Re: [PATCH v4 01/11] include: fdtdec: decouple fdt_addr_t and phys_addr_t size

[Johan Jonker <jbx6244@gmail.com>]

On 2/25/23 10:15, Michael Nazzareno Trimarchi wrote:
> Hi Johan
> 
> On Sat, Feb 25, 2023 at 10:11 AM Johan Jonker <jbx6244@gmail.com> wrote:
>>
>>
>>
>> On 2/23/23 01:47, Kever Yang wrote:
>>> Hi Johan,
>>>
>>>     This update will cause below error on evb-ast2500/2600:
>>
>> Hi Kever, Simon,
>>
>> For "spi-aspeed-smc.c" I have submitted a patch proposal.
>> The changes in patch 1 possible involves a lot more boards that might need a change to which the external MAINTAINERS may or may not respond to.
>> As that becomes a lengthy time consuming process. Is it maybe possible to skip patch 1 for now and get at least the more essential patches 2-11 merged?
>> Please advise what's the best option here to get forward.
>>
>> Johan
> 

> Please point to the patch you mentioned.

[PATCH v1] spi: spi-aspeed-smc: convert devfdt_get_addr_index output to phys_addr_t
https://lore.kernel.org/u-boot/f3dba231-4a55-0a94-dfab-5cab1419d132@gmail.com/T/#m167646537956e20dc9ae60957f6d54f8a00866d7

> 
> Kever if you are going to pick the nand patches part let me know
> 
> Michael
> 
>>
>>
>>>
>>> $ tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -E -W -e --board ${TEST_PY_BD} ${OVERRIDE}
>>> Building current source for 1 boards (1 thread, 24 jobs per thread)
>>>        arm:  +   evb-ast2600
>>> +drivers/spi/spi-aspeed-smc.c: In function 'apseed_spi_of_to_plat':
>>> +drivers/spi/spi-aspeed-smc.c:1154:22: error: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'fdt_size_t' {aka 'unsigned int'} [-Werror=format=]
>>> + 1154 |         dev_dbg(bus, "ctrl_base = 0x%x, ahb_base = 0x%p, size = 0x%lx\n",
>>> +      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> + 1155 |                 (u32)priv->regs, plat->ahb_base, plat->ahb_sz);
>>> +      | ~~~~~~~~~~~~
>>> +      |                                                      |
>>> +      | fdt_size_t {aka unsigned int}
>>> +include/dm/device_compat.h:50:24: note: in definition of macro 'dev_printk_emit'
>>> +   50 |                 printf(fmt, ##__VA_ARGS__); \
>>> +      |                        ^~~
>>> +include/dm/device_compat.h:117:9: note: in expansion of macro '__dev_printk'
>>> +  117 |         __dev_printk(LOGL_DEBUG, dev, fmt, ##__VA_ARGS__)
>>> +      |         ^~~~~~~~~~~~
>>> +drivers/spi/spi-aspeed-smc.c:1154:9: note: in expansion of macro 'dev_dbg'
>>> +      |         ^~~~~~~
>>> +drivers/spi/spi-aspeed-smc.c:1154:69: note: format string is defined here
>>> + | ~~^
>>> + | |
>>> + | long unsigned int
>>> + | %x
>>> +In file included from include/linux/printk.h:4,
>>> +                 from include/linux/kernel.h:5,
>>> +                 from arch/arm/include/asm/io.h:14,
>>> +                 from drivers/spi/spi-aspeed-smc.c:13:
>>> +include/log.h:222:24: note: in definition of macro 'log'
>>> +  222 |                 printf(_fmt, ##_args); \
>>> +      |                        ^~~~
>>> +include/dm/device_compat.h:85:17: note: in expansion of macro 'dev_printk_emit'
>>> +   85 |                 dev_printk_emit(LOG_CATEGORY, level, fmt, ##__VA_ARGS__); \
>>> +      |                 ^~~~~~~~~~~~~~~
>>> +include/dm/device_compat.h:54:24: note: in definition of macro 'dev_printk_emit'
>>> +   54 |                 printf(fmt, ##__VA_ARGS__); \
>>> +include/dm/device_compat.h:91:41: error: format '%lx' expects argument of type 'long unsigned int', but argument 6 has type 'fdt_size_t' {aka 'unsigned int'} [-Werror=format=]
>>> +   91 |                                         "%s %s: " fmt, \
>>> +      |                                         ^~~~~~~~~
>>> +include/dm/device_compat.h:90:25: note: in expansion of macro 'dev_printk_emit'
>>> +   90 | dev_printk_emit(__dev->driver->id, level, \
>>> +      |                         ^~~~~~~~~~~~~~~
>>> +include/dm/device_compat.h:96:41: error: format '%lx' expects argument of type 'long unsigned int', but argument 6 has type 'fdt_size_t' {aka 'unsigned int'} [-Werror=format=]
>>> +   96 |                                         "%s %s: " fmt, \
>>> +include/dm/device_compat.h:95:25: note: in expansion of macro 'dev_printk_emit'
>>> +   95 |                         dev_printk_emit(LOG_CATEGORY, level, \
>>> +cc1: all warnings being treated as errors
>>> +make[3]: *** [scripts/Makefile.build:256: drivers/spi/spi-aspeed-smc.o] Error 1
>>> +make[2]: *** [scripts/Makefile.build:397: drivers/spi] Error 2
>>> +make[1]: *** [Makefile:1846: drivers] Error 2
>>> +make: *** [Makefile:177: sub-make] Error 2
>>>     0    0    1 /1              evb-ast2600
>>>
>>>
>>> Thanks,
>>>
>>> - Kever
>>>
>>> On 2023/2/18 23:26, Johan Jonker wrote:
>>>> The DT specification supports CPUs with both 32-bit and 64-bit addressing
>>>> capabilities. In U-boot the fdt_addr_t and phys_addr_t size are coupled
>>>> by a typedef. The MTD NAND drivers for 32-bit CPU's can describe partitions
>>>> with a 64-bit reg property. These partitions synced from Linux end up with
>>>> the wrong offset and sizes when only the lower 32-bit is passed.
>>>> Decouple the fdt_addr_t and phys_addr_t size as they don't necessary
>>>> match.
>>>>
>>>> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
>>>> ---
>>>>
>>>> Changed V2:
>>>>    reword
>>>>
>>>> ---
>>>>
>>>> Note:
>>>>    Most drivers still assume that FDT and CPU capabilities are identical.
>>>>    In order to use these variables a cast is needed.
>>>> ---
>>>>   Kconfig          |  8 ++++++++
>>>>   include/fdtdec.h | 13 +++++++++----
>>>>   2 files changed, 17 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Kconfig b/Kconfig
>>>> index a75cce7e..7697dade 100644
>>>> --- a/Kconfig
>>>> +++ b/Kconfig
>>>> @@ -422,11 +422,19 @@ endif # EXPERT
>>>>
>>>>   config PHYS_64BIT
>>>>       bool "64bit physical address support"
>>>> +    select FDT_64BIT
>>>>       help
>>>>         Say Y here to support 64bit physical memory address.
>>>>         This can be used not only for 64bit SoCs, but also for
>>>>         large physical address extension on 32bit SoCs.
>>>>
>>>> +config FDT_64BIT
>>>> +    bool "64bit fdt address support"
>>>> +    help
>>>> +      Say Y here to support 64bit fdt addresses.
>>>> +      This can be used not only for 64bit SoCs, but also
>>>> +      for large address extensions on 32bit SoCs.
>>>> +
>>>>   config HAS_ROM
>>>>       bool
>>>>       select BINMAN
>>>> diff --git a/include/fdtdec.h b/include/fdtdec.h
>>>> index 12355afd..af29ac0c 100644
>>>> --- a/include/fdtdec.h
>>>> +++ b/include/fdtdec.h
>>>> @@ -18,15 +18,18 @@
>>>>   #include <pci.h>
>>>>
>>>>   /*
>>>> - * A typedef for a physical address. Note that fdt data is always big
>>>> + * Support for 64bit fdt addresses.
>>>> + * This can be used not only for 64bit SoCs, but also
>>>> + * for large address extensions on 32bit SoCs.
>>>> + * Note that fdt data is always big
>>>>    * endian even on a litle endian machine.
>>>>    */
>>>> -typedef phys_addr_t fdt_addr_t;
>>>> -typedef phys_size_t fdt_size_t;
>>>>
>>>>   #define FDT_SIZE_T_NONE (-1U)
>>>>
>>>> -#ifdef CONFIG_PHYS_64BIT
>>>> +#ifdef CONFIG_FDT_64BIT
>>>> +typedef u64 fdt_addr_t;
>>>> +typedef u64 fdt_size_t;
>>>>   #define FDT_ADDR_T_NONE ((ulong)(-1))
>>>>
>>>>   #define fdt_addr_to_cpu(reg) be64_to_cpu(reg)
>>>> @@ -35,6 +38,8 @@ typedef phys_size_t fdt_size_t;
>>>>   #define cpu_to_fdt_size(reg) cpu_to_be64(reg)
>>>>   typedef fdt64_t fdt_val_t;
>>>>   #else
>>>> +typedef u32 fdt_addr_t;
>>>> +typedef u32 fdt_size_t;
>>>>   #define FDT_ADDR_T_NONE (-1U)
>>>>
>>>>   #define fdt_addr_to_cpu(reg) be32_to_cpu(reg)
>>>> --
>>>> 2.20.1
>>>>
> 
> 
>