Re: [RFC PATCH v1 1/2] include: fdtdec: decouple fdt_addr_t and phys_addr_t size

4 Feb 2023

Hi Johan,
On Thu, 2 Feb 2023 at 10:58, Johan Jonker jbx6244@gmail.com 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
Note for Tom Rini or others:
fdt_addr_t is referenced in 230 files and fdt_size_t in 50 files.
Most drivers mix up FDT and CPU capabilities.
Please advise how to move forward with proper DT parsing.

This is related to a possible future serie of bug fixes
for the Rockchip nfc driver.

Kconfig          | 8 ++++++++
 include/fdtdec.h | 8 +++++---
 2 files changed, 13 insertions(+), 3 deletions(-)
I wonder what the impact of this might be? So long as fdt_addr_t is at
least as big as phys_addr_t then perhaps this is OK.
But at present, changing fdt_addr_t affects fdtdec_get_addr_size().
Perhaps that doesn't matter, so long as '#address-cells' is respected.
So I'm going to go with:
Reviewed-by: Simon Glass sjg@chromium.org
...

diff --git a/Kconfig b/Kconfig
index a75cce7e..8101f1a6 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 memory address.


    This can be used not only for 64bit SoCs, but also for


    large physical address extension on 32bit SoCs.




config HAS_ROM
        bool
        select BINMAN
diff --git a/include/fdtdec.h b/include/fdtdec.h
index 12355afd..0adde92a 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -21,12 +21,12 @@

A typedef for a physical address. 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 +35,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)
Could you add some comments about what fdt_addr/size_t are for and
what it means.
The original intent was to avoid using 64-bit values on a 32-bit machine.
...
#define fdt_addr_to_cpu(reg) be32_to_cpu(reg)
2.20.1
Regards,
Simon

    