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

[Kever Yang <kever.yang@rock-chips.com>]

Hi Johan,

     This update will cause below error on evb-ast2500/2600:

$ 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
>