On 09/02/2015 09:05 AM, Simon Glass wrote:
Hi York,
On 1 September 2015 at 22:01, York Sun yorksun@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 yorksun@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