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

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(-)
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) -- 2.20.1

When fdt_addr_t and phys_addr_t are split it turns out that the header don't match the functions, so fix the headers.
Signed-off-by: Johan Jonker jbx6244@gmail.com --- include/dm/ofnode.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h index fa986560..287b0c35 100644 --- a/include/dm/ofnode.h +++ b/include/dm/ofnode.h @@ -678,8 +678,8 @@ int ofnode_read_size(ofnode node, const char *propname); * @size: Pointer to size of the address * Return: address, or FDT_ADDR_T_NONE if not present or invalid */ -phys_addr_t ofnode_get_addr_size_index(ofnode node, int index, - fdt_size_t *size); +fdt_addr_t ofnode_get_addr_size_index(ofnode node, int index, + fdt_size_t *size);
/** * ofnode_get_addr_size_index_notrans() - get an address/size from a node @@ -695,8 +695,8 @@ phys_addr_t ofnode_get_addr_size_index(ofnode node, int index, * @size: Pointer to size of the address * Return: address, or FDT_ADDR_T_NONE if not present or invalid */ -phys_addr_t ofnode_get_addr_size_index_notrans(ofnode node, int index, - fdt_size_t *size); +fdt_addr_t ofnode_get_addr_size_index_notrans(ofnode node, int index, + fdt_size_t *size);
/** * ofnode_get_addr_index() - get an address from a node @@ -707,7 +707,7 @@ phys_addr_t ofnode_get_addr_size_index_notrans(ofnode node, int index, * @index: Index of address to read (0 for first) * Return: address, or FDT_ADDR_T_NONE if not present or invalid */ -phys_addr_t ofnode_get_addr_index(ofnode node, int index); +fdt_addr_t ofnode_get_addr_index(ofnode node, int index);
/** * ofnode_get_addr() - get an address from a node @@ -717,7 +717,7 @@ phys_addr_t ofnode_get_addr_index(ofnode node, int index); * @node: node to read from * Return: address, or FDT_ADDR_T_NONE if not present or invalid */ -phys_addr_t ofnode_get_addr(ofnode node); +fdt_addr_t ofnode_get_addr(ofnode node);
/** * ofnode_get_size() - get size from a node @@ -1055,8 +1055,8 @@ const void *ofprop_get_property(const struct ofprop *prop, * @sizep: place to put size value (on success) * Return: address value, or FDT_ADDR_T_NONE on error */ -phys_addr_t ofnode_get_addr_size(ofnode node, const char *propname, - phys_size_t *sizep); +fdt_addr_t ofnode_get_addr_size(ofnode node, const char *propname, + fdt_size_t *sizep);
/** * ofnode_read_u8_array_ptr() - find an 8-bit array -- 2.20.1

On Thu, 2 Feb 2023 at 10:59, Johan Jonker jbx6244@gmail.com wrote:
When fdt_addr_t and phys_addr_t are split it turns out that the header don't match the functions, so fix the headers.
Signed-off-by: Johan Jonker jbx6244@gmail.com
include/dm/ofnode.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

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
participants (2)
-
Johan Jonker
-
Simon Glass