On 02/16/2016 08:02 AM, Simon Glass wrote:
> Hi York,
>
> On 12 February 2016 at 13:59, York Sun <york....@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....@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.
York
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
