Message ID | 873cd740-1909-f0d3-5fef-ff033aed558e@gmail.com |
---|---|
State | RFC |
Delegated to: | Simon Glass |
Headers | show |
Series | [RFC,v1,1/2] include: fdtdec: decouple fdt_addr_t and phys_addr_t size | expand |
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
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) #define fdt_addr_to_cpu(reg) be32_to_cpu(reg)
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(-) -- 2.20.1