
On 02/24/2016 04:30 PM, Simon Glass wrote:
Hi York,
On 24 February 2016 at 15:55, york sun york.sun@nxp.com wrote:
On 02/16/2016 08:02 AM, Simon Glass wrote:
Hi York,
On 12 February 2016 at 13:59, York Sun york.sun@nxp.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.
nit: How about saying "fix this by adding support for 64-bit addresses" or similar
Sure. I can fix that.
Signed-off-by: York Sun york.sun@nxp.com
Changes in v4: Separate ulong to phys_addr_t change to another patch.
Changes in v3: Define PRIpa for host and target in common/image-fit.c so printf works properly for 32-, 64-bit targets and host tools.
Changes in v2: Make a common function for both load and entry addresses. Simplify calculation of addresses in a similar way as fdtdec_get_number() fdtdec_get_number() is not used, or too many files need to be included and/or twisted for host tool Continue to use %08llx for print format for load and entry addresses because %pa does not always work for host tool (mkimage)
common/image-fit.c | 54 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 23 deletions(-)
diff --git a/common/image-fit.c b/common/image-fit.c index bfa76a2..c000475 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -433,7 +433,7 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)
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");
@@ -675,6 +675,34 @@ int fit_image_get_comp(const void *fit, int noffset, uint8_t *comp) return 0; }
+static int fit_image_get_address(const void *fit, int noffset, char *name,
phys_addr_t *load)
+{
int len, cell_len;
const fdt32_t *cell;
unsigned long long load64 = 0;
cell = fdt_getprop(fit, noffset, name, &len);
if (cell == NULL) {
fit_get_debug(fit, noffset, name, len);
return -1;
}
if (len > sizeof(phys_addr_t)) {
printf("Unsupported %s address size\n", name);
return -1;
}
cell_len = len >> 2;
/* Use load64 to avoid compiling warning for 32-bit target */
while (cell_len--) {
load64 = (load64 << 32) | uimage_to_cpu(*cell);
cell++;
}
*load = (phys_addr_t)load64;
return 0;
+} /**
- fit_image_get_load() - get load addr property for given component image node
- @fit: pointer to the FIT format image header
@@ -690,17 +718,7 @@ int fit_image_get_comp(const void *fit, int noffset, uint8_t *comp) */ int fit_image_get_load(const void *fit, int noffset, phys_addr_t *load) {
int len;
const uint32_t *data;
data = fdt_getprop(fit, noffset, FIT_LOAD_PROP, &len);
if (data == NULL) {
fit_get_debug(fit, noffset, FIT_LOAD_PROP, len);
return -1;
}
*load = uimage_to_cpu(*data);
return 0;
return fit_image_get_address(fit, noffset, FIT_LOAD_PROP, load);
I think it would make sense to have your new fit_image_get_address() in one patch, and the enhancement to support more address sizes in another.
The new fit_image_get_address() gets correct address. The rest of change is to use the new function. I don't think they can be separated. Maybe I don't understand your comment.
I am preparing a new version. Please comment on that if you still feel the same.
I mean:
- patch 1: take the existing 32-bit-only code and put it in a new
fit_image_get_address() functoin
- patch 2: enhance your new function to support 64-bit
At present you have these two things co-mingled which I don't think is ideal.
I see. Let me work on it.
York