[U-Boot] [PATCH v3] common: Fix load and entry addresses in FIT image

[York Sun <yorksun@freescale.com>]

On 11/06/2015 04:06 AM, Simon Glass wrote:
> Hi York,
> 
> On 29 October 2015 at 11:21, York Sun <yorksun@freescale.com> wrote:
>>
>>
>> On 10/29/2015 10:16 AM, Simon Glass wrote:
>>> Hi York,
>>>
>>> On 19 October 2015 at 11:30, York Sun <yorksun@freescale.com> wrote:
>>>>
>>>>
>>>> On 10/18/2015 04:15 PM, Simon Glass wrote:
>>>>> Hi,
>>>>>
>>>>> On 18 October 2015 at 06:18, Simon Glass <sjg@chromium.org> wrote:
>>>>>> On 9 September 2015 at 12:07, Simon Glass <sjg@chromium.org> wrote:
>>>>>>>
>>>>>>> On Friday, 4 September 2015, 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.
>>>>>>>>
>>>>>>>> Signed-off-by: York Sun <yorksun@freescale.com>
>>>>>>>>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> 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/bootm.c     |   13 +++++----
>>>>>>>>  common/image-fit.c |   81 +++++++++++++++++++++++++++++-----------------------
>>>>>>>>  include/bootm.h    |    6 ++--
>>>>>>>>  include/image.h    |   12 +++++---
>>>>>>>>  4 files changed, 63 insertions(+), 49 deletions(-)
>>>>>>>
>>>>>>
>>>>>> Acked-by: Simon Glass <sjg@chromium.org>
>>>>>
>>>>> Unfortunately this produces lots of warnings on sandbox. Can you
>>>>> please take a look?
>>>>>
>>>>> /usr/local/google/c/cosarm/src/third_party/u-boot/files/common/bootm.c:
>>>>> In function ?bootm_find_os?:
>>>>> /usr/local/google/c/cosarm/src/third_party/u-boot/files/common/bootm.c:146:12:
>>>>> warning: passing argument 3 of ?fit_image_get_load? from incompatible
>>>>> pointer type [enabled by default]
>>>>>             &images.os.load)) {
>>>>>             ^
>>>>> In file included from
>>>>> /usr/local/google/c/cosarm/src/third_party/u-boot/files/include/common.h:82:0,
>>>>>                  from
>>>>> /usr/local/google/c/cosarm/src/third_party/u-boot/files/common/bootm.c:9:
>>>>> /usr/local/google/c/cosarm/src/third_party/u-boot/files/include/image.h:851:5:
>>>>> note: expected ?phys_addr_t *? but argument is of type ?ulong *?
>>>>>  int fit_image_get_load(const void *fit, int noffset, phys_addr_t *load);
>>>>>      ^
>>>>
>>>> Simon,
>>>>
>>>> This warning is buried by tons of other warnings when compiling sandbox. I
>>>> believe it is caused by the typedef of phys_addr_t for sandbox. How about a fix
>>>> like this?
>>>>
>>>> diff --git a/arch/sandbox/include/asm/types.h b/arch/sandbox/include/asm/types.h
>>>> index 42c09e2..c3bb76e 100644
>>>> --- a/arch/sandbox/include/asm/types.h
>>>> +++ b/arch/sandbox/include/asm/types.h
>>>> @@ -53,8 +53,8 @@ typedef __UINT64_TYPE__ u64;
>>>>  #define BITS_PER_LONG  CONFIG_SANDBOX_BITS_PER_LONG
>>>>
>>>>  typedef unsigned long dma_addr_t;
>>>> -typedef u32 phys_addr_t;
>>>> -typedef u32 phys_size_t;
>>>> +typedef unsigned long phys_addr_t;
>>>> +typedef unsigned long phys_size_t;
>>>
>>> Sandbox is supposed to use a 32-bit memory model regardless of the
>>> host. What other warnings do you see? I don't see any when I build it.
>>>
>>
>> Maybe I did something wrong. Here is my log
>>
>> $ make sandbox_defconfig
>>   HOSTCC  scripts/basic/fixdep
>>   HOSTCC  scripts/kconfig/conf.o
>>   SHIPPED scripts/kconfig/zconf.tab.c
>>   SHIPPED scripts/kconfig/zconf.lex.c
>>   SHIPPED scripts/kconfig/zconf.hash.c
>>   HOSTCC  scripts/kconfig/zconf.tab.o
>>   HOSTLD  scripts/kconfig/conf
>> #
>> # configuration written to .config
>> #
>> $ make -s -j8
>> disk/part_efi.c: In function ?alloc_read_gpt_entries?:
>> disk/part_efi.c:752:2: warning: format ?%zu? expects argument of type ?size_t?,
>> but argument 5 has type ?long unsigned int? [-Wformat]
>> disk/part_efi.c:765:4: warning: format ?%zX? expects argument of type ?size_t?,
>> but argument 3 has type ?long unsigned int? [-Wformat]
>> drivers/mtd/spi/sf.c: In function ?spi_flash_read_write?:
>> drivers/mtd/spi/sf.c:30:3: warning: format ?%zu? expects argument of type
>> ?size_t?, but argument 2 has type ?long unsigned int? [-Wformat]
>> drivers/mtd/spi/sf.c:36:4: warning: format ?%zu? expects argument of type
>> ?size_t?, but argument 2 has type ?long unsigned int? [-Wformat]
>> drivers/mtd/spi/sf_ops.c: In function ?spi_flash_cmd_write_ops?:
>> drivers/mtd/spi/sf_ops.c:343:3: warning: format ?%zu? expects argument of type
>> ?size_t?, but argument 7 has type ?long unsigned int? [-Wformat]
>> drivers/mtd/spi/sf_ops.c: In function ?sst_write_wp?:
>> drivers/mtd/spi/sf_ops.c:538:2: warning: format ?%zu? expects argument of type
>> ?size_t?, but argument 3 has type ?long unsigned int? [-Wformat]
>> drivers/mtd/spi/sf_ops.c:538:2: warning: format ?%zx? expects argument of type
>> ?size_t?, but argument 4 has type ?long unsigned int? [-Wformat]
>> drivers/mtd/spi/sf_ops.c: In function ?sst_write_bp?:
>> drivers/mtd/spi/sf_ops.c:569:2: warning: format ?%zu? expects argument of type
>> ?size_t?, but argument 3 has type ?long unsigned int? [-Wformat]
>> drivers/mtd/spi/sf_ops.c:569:2: warning: format ?%zx? expects argument of type
>> ?size_t?, but argument 4 has type ?long unsigned int? [-Wformat]
>> drivers/tpm/tpm-uclass.c: In function ?tpm_xfer?:
>> drivers/tpm/tpm-uclass.c:92:3: warning: format ?%zx? expects argument of type
>> ?size_t?, but argument 3 has type ?long unsigned int? [-Wformat]
>> drivers/tpm/tpm_tis_sandbox.c: In function ?sandbox_tpm_xfer?:
>> drivers/tpm/tpm_tis_sandbox.c:152:9: warning: format ?%zd? expects argument of
>> type ?signed size_t?, but argument 2 has type ?size_t? [-Wformat]
>> drivers/tpm/tpm_tis_sandbox.c:152:9: warning: format ?%zd? expects argument of
>> type ?signed size_t?, but argument 3 has type ?size_t? [-Wformat]
>> common/cmd_pxe.c: In function ?format_mac_pxe?:
>> common/cmd_pxe.c:68:3: warning: format ?%zd? expects argument of type ?signed
>> size_t?, but argument 2 has type ?size_t? [-Wformat]
>> common/cmd_pxe.c: In function ?get_bootfile_path?:
>> common/cmd_pxe.c:114:5: warning: format ?%zd? expects argument of type ?signed
>> size_t?, but argument 2 has type ?size_t? [-Wformat]
>> common/cmd_pxe.c:114:5: warning: format ?%zd? expects argument of type ?signed
>> size_t?, but argument 3 has type ?size_t? [-Wformat]
>> common/cmd_pxe.c: In function ?label_boot?:
>> common/cmd_pxe.c:690:11: warning: format ?%zd? expects argument of type ?signed
>> size_t?, but argument 2 has type ?__kernel_size_t? [-Wformat]
>> common/cmd_pxe.c:690:11: warning: format ?%zd? expects argument of type ?signed
>> size_t?, but argument 3 has type ?__kernel_size_t? [-Wformat]
>> common/cmd_pxe.c:690:11: warning: format ?%zd? expects argument of type ?signed
>> size_t?, but argument 4 has type ?__kernel_size_t? [-Wformat]
>> common/cmd_sf.c: In function ?spi_flash_update_block?:
>> common/cmd_sf.c:174:2: warning: format ?%zx? expects argument of type ?size_t?,
>> but argument 4 has type ?long unsigned int? [-Wformat]
>> common/cmd_sf.c:181:3: warning: format ?%zx? expects argument of type ?size_t?,
>> but argument 3 has type ?long unsigned int? [-Wformat]
>> common/cmd_sf.c: In function ?spi_flash_update?:
>> common/cmd_sf.c:236:9: warning: format ?%zu? expects argument of type ?size_t?,
>> but argument 2 has type ?long unsigned int? [-Wformat]
>> common/cmd_sf.c:254:9: warning: format ?%zu? expects argument of type ?size_t?,
>> but argument 2 has type ?long unsigned int? [-Wformat]
>> common/cmd_sf.c:254:9: warning: format ?%zu? expects argument of type ?size_t?,
>> but argument 3 has type ?long unsigned int? [-Wformat]
>> common/cmd_sf.c: In function ?do_spi_flash_read_write?:
>> common/cmd_sf.c:307:10: warning: format ?%zu? expects argument of type ?size_t?,
>> but argument 2 has type ?long unsigned int? [-Wformat]
>> common/cmd_sf.c: In function ?do_spi_flash_erase?:
>> common/cmd_sf.c:346:9: warning: format ?%zu? expects argument of type ?size_t?,
>> but argument 2 has type ?ulong? [-Wformat]
>> common/cmd_nvedit.c: In function ?do_env_export?:
>> common/cmd_nvedit.c:929:3: warning: format ?%zX? expects argument of type
>> ?size_t?, but argument 3 has type ?long unsigned int? [-Wformat]
>> common/cmd_nvedit.c: In function ?do_env_import?:
>> common/cmd_nvedit.c:1062:3: warning: format ?%zu? expects argument of type
>> ?size_t?, but argument 2 has type ?long unsigned int? [-Wformat]
>> common/cmd_nvedit.c:1062:3: warning: format ?%zX? expects argument of type
>> ?size_t?, but argument 3 has type ?long unsigned int? [-Wformat]
>> common/malloc_simple.c: In function ?malloc_simple?:
>> common/malloc_simple.c:22:2: warning: format ?%zx? expects argument of type
>> ?size_t?, but argument 3 has type ?long unsigned int? [-Wformat]
>> lib/lzma/LzmaTools.c: In function ?lzmaBuffToBuffDecompress?:
>> lib/lzma/LzmaTools.c:94:5: warning: format ?%zx? expects argument of type
>> ?size_t?, but argument 2 has type ?SizeT? [-Wformat]
>> lib/lzma/LzmaTools.c:95:5: warning: format ?%zx? expects argument of type
>> ?size_t?, but argument 2 has type ?SizeT? [-Wformat]
>> lib/lzma/LzmaTools.c:115:5: warning: format ?%zx? expects argument of type
>> ?size_t?, but argument 2 has type ?SizeT? [-Wformat]
>> common/cli_simple.c: In function ?cli_simple_process_macros?:
>> common/cli_simple.c:73:2: warning: format ?%zd? expects argument of type ?signed
>> size_t?, but argument 2 has type ?__kernel_size_t? [-Wformat]
>> common/cli_simple.c:162:2: warning: format ?%zd? expects argument of type
>> ?signed size_t?, but argument 2 has type ?__kernel_size_t? [-Wformat]
>> lib/lz4_wrapper.c: In function ?ulz4fn?:
>> lib/lz4_wrapper.c:111:18: warning: comparison of distinct pointer types lacks a
>> cast [enabled by default]
>> lib/hashtable.c: In function ?hexport_r?:
>> lib/hashtable.c:605:2: warning: format ?%zu? expects argument of type ?size_t?,
>> but argument 5 has type ?long unsigned int? [-Wformat]
>> lib/hashtable.c:661:5: warning: format ?%zu? expects argument of type ?size_t?,
>> but argument 2 has type ?long unsigned int? [-Wformat]
>> lib/hashtable.c:661:5: warning: format ?%zu? expects argument of type ?size_t?,
>> but argument 3 has type ?long unsigned int? [-Wformat]
>> lib/hashtable.c: In function ?himport_r?:
>> lib/hashtable.c:793:3: warning: format ?%zu? expects argument of type ?size_t?,
>> but argument 2 has type ?long unsigned int? [-Wformat]
>>
>> I don't need to specify ARCH or CROSS_COMPILE to build sandbox, do I?
> 
> No. I just tried the same procedure and get no warnings. I am using a
> 64-bit Ubuntu machine. What kind of platform are you using?

I am using 32-bit Ubuntu.

York