On 09/02/2015 09:05 AM, Simon Glass wrote: > Hi York, > > On 1 September 2015 at 22:01, York Sun <york...@freescale.com> wrote: >> FIT image supports more than 32 bits in addresses by using #address-cell >> field. However the address length is not handled when parsing FIT images. >> Beside, the variable used to host address has "ulong" type. It is OK for >> the target, but not always enough for host tools such as mkimage. This >> patch replaces "ulong" with "phys_addr_t" to make sure the address is >> correct for both the target and the host. > > This looks right to me but I have a few comments. > >> >> Signed-off-by: York Sun <york...@freescale.com> >> >> --- >> >> common/bootm.c | 13 +++++++------ >> common/image-fit.c | 55 >> +++++++++++++++++++++++++++++++++++++--------------- >> include/bootm.h | 6 +++--- >> include/image.h | 12 ++++++++---- >> 4 files changed, 57 insertions(+), 29 deletions(-) >> >> diff --git a/common/bootm.c b/common/bootm.c >> index 667c934..0672e73 100644 >> --- a/common/bootm.c >> +++ b/common/bootm.c >> @@ -325,9 +325,9 @@ static int handle_decomp_error(int comp_type, size_t >> uncomp_size, >> return BOOTM_ERR_RESET; >> } >> >> -int bootm_decomp_image(int comp, ulong load, ulong image_start, int type, >> - void *load_buf, void *image_buf, ulong image_len, >> - uint unc_len, ulong *load_end) >> +int bootm_decomp_image(int comp, phys_addr_t load, phys_addr_t image_start, >> + int type, void *load_buf, void *image_buf, >> + ulong image_len, uint unc_len, ulong *load_end) >> { >> int ret = 0; >> >> @@ -883,7 +883,8 @@ void memmove_wd(void *to, void *from, size_t len, ulong >> chunksz) >> static int bootm_host_load_image(const void *fit, int req_image_type) >> { >> const char *fit_uname_config = NULL; >> - ulong data, len; >> + phys_addr_t *data = NULL; >> + ulong len; >> bootm_headers_t images; >> int noffset; >> ulong load_end; >> @@ -897,7 +898,7 @@ static int bootm_host_load_image(const void *fit, int >> req_image_type) >> noffset = fit_image_load(&images, (ulong)fit, >> NULL, &fit_uname_config, >> IH_ARCH_DEFAULT, req_image_type, -1, >> - FIT_LOAD_IGNORED, &data, &len); >> + FIT_LOAD_IGNORED, data, &len); >> if (noffset < 0) >> return noffset; >> if (fit_image_get_type(fit, noffset, &image_type)) { >> @@ -912,7 +913,7 @@ static int bootm_host_load_image(const void *fit, int >> req_image_type) >> >> /* Allow the image to expand by a factor of 4, should be safe */ >> load_buf = malloc((1 << 20) + len * 4); >> - ret = bootm_decomp_image(imape_comp, 0, data, image_type, load_buf, >> + ret = bootm_decomp_image(imape_comp, 0, *data, image_type, load_buf, >> (void *)data, len, CONFIG_SYS_BOOTM_LEN, >> &load_end); >> free(load_buf); >> diff --git a/common/image-fit.c b/common/image-fit.c >> index 28f7aa8..513cfdc 100644 >> --- a/common/image-fit.c >> +++ b/common/image-fit.c >> @@ -358,7 +358,7 @@ void fit_image_print(const void *fit, int image_noffset, >> const char *p) >> char *desc; >> uint8_t type, arch, os, comp; >> size_t size; >> - ulong load, entry; >> + phys_addr_t load, entry; >> const void *data; >> int noffset; >> int ndepth; >> @@ -428,17 +428,17 @@ void fit_image_print(const void *fit, int >> image_noffset, const char *p) >> if (ret) >> printf("unavailable\n"); >> else >> - printf("0x%08lx\n", load); >> + printf("0x%08llx\n", (uint64_t)load); >> } >> >> if ((type == IH_TYPE_KERNEL) || (type == IH_TYPE_STANDALONE) || >> (type == IH_TYPE_RAMDISK)) { >> - fit_image_get_entry(fit, image_noffset, &entry); >> + ret = fit_image_get_entry(fit, image_noffset, &entry); >> printf("%s Entry Point: ", p); >> if (ret) >> printf("unavailable\n"); >> else >> - printf("0x%08lx\n", entry); >> + printf("0x%08llx\n", (uint64_t)entry); > > If the address is 32-bit we cast it to 64-bit and print 8 digits. If > it is 64-bit we print as many digits as we can find. > > I think this behaviour is reasonable - and avoids hopelessly confusing > 16-character hex strings with lots of leading zeros. > > But the code looks a bit odd. Do you think we should add a % formatter > to print a phys_addr_t? >
Do you mean %pa? I tried and the result looks weird on mkimage. I didn't spend time on it, thinking it could be host CC issue. I will work on your other comments. York _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot