Skip to contentSkip to conversation

of/fdt: Some bug fixes and cleanups

PATCH In review Yuntao Wang 4 participants 14 messages Nov 15, 2025 – Nov 20, 2025 Device tree

lore
Share & tools
lore
#0 lore raw

v3 cover of/fdt: Some bug fixes and cleanups

This patch series fixes several bugs related to dt_root_addr_cells and dt_root_size_cells, and performs some cleanup.

Changelog:

v2 -> v3:
- Use of_flat_dt_ instead of of_fdt_ as the prefix for the newly added helper functions

- Improve the internal logic of of_flat_dt_get_addr_size_prop() and

    of_flat_dt_get_addr_size()

- Introduce the entry_index parameter for of_flat_dt_read_addr_size()

- Fix some warnings and coding-style issues

v1 -> v2:
- Consolidate duplicate code into helper functions

Links to previous patch series:

v2: https://lore.kernel.org/linux-devicetree/[email protected]/
v1: https://lore.kernel.org/linux-devicetree/[email protected]/

Yuntao Wang (8):
of/fdt: Consolidate duplicate code into helper functions of/fdt: Fix the len check in early_init_dt_check_for_elfcorehdr() of/fdt: Fix the len check in
early_init_dt_check_for_usable_mem_range()
of/fdt: Fix incorrect use of dt_root_addr_cells in
early_init_dt_check_kho()
of/fdt: Simplify the logic of early_init_dt_scan_memory() of/reserved_mem: Simplify the logic of __reserved_mem_reserve_reg() of/reserved_mem: Simplify the logic of
fdt_scan_reserved_mem_reg_nodes()
of/reserved_mem: Simplify the logic of __reserved_mem_alloc_size()

 drivers/of/fdt.c             | 101 ++++++++++++++++++++++-------------
 drivers/of/of_reserved_mem.c |  69 ++++++++----------------
 include/linux/of_fdt.h       |   9 ++++
 3 files changed, 97 insertions(+), 82 deletions(-)

--
2.51.0

#1 lore raw

PATCH v3 1/8 of/fdt: Consolidate duplicate code into helper functions

Currently, there are many pieces of nearly identical code scattered across different places. Consolidate the duplicate code into helper functions to improve maintainability and reduce the likelihood of errors.

---
 drivers/of/fdt.c       | 41 +++++++++++++++++++++++++++++++++++++++++
 include/linux/of_fdt.h |  9 +++++++++
 2 files changed, 50 insertions(+)
drivers/of/fdt.c +41 -0
index 0edd639898a6..0c18bdefbbee 100644
@@ -625,6 +625,47 @@ const void *__init of_get_flat_dt_prop(unsigned long node, const char *name,
 	return fdt_getprop(initial_boot_params, node, name, size);
 }
 
+const __be32 *__init of_flat_dt_get_addr_size_prop(unsigned long node,
+						   const char *name,
+						   int *entries)
+{
+	const __be32 *prop;
+	int len, elen = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
+
+	prop = of_get_flat_dt_prop(node, name, &len);
+	if (!prop || len % elen) {
+		*entries = 0;
+		return NULL;
+	}
+
+	*entries = len / elen;
+	return prop;
+}
+
+bool __init of_flat_dt_get_addr_size(unsigned long node, const char *name,
+				     u64 *addr, u64 *size)
+{
+	const __be32 *prop;
+	int entries;
+
+	prop = of_flat_dt_get_addr_size_prop(node, name, &entries);
+	if (!prop || entries != 1)
+		return false;
+
+	of_flat_dt_read_addr_size(prop, 0, addr, size);
+	return true;
+}
+
+void __init of_flat_dt_read_addr_size(const __be32 *prop, int entry_index,
+				      u64 *addr, u64 *size)
+{
+	int entry_cells = dt_root_addr_cells + dt_root_size_cells;
+	prop += entry_cells * entry_index;
+
+	*addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
+	*size = dt_mem_next_cell(dt_root_size_cells, &prop);
+}
+
 /**
  * of_fdt_is_compatible - Return true if given node from the given blob has
  * compat in its compatible list
include/linux/of_fdt.h +9 -0
index b8d6c0c20876..51dadbaa3d63 100644
@@ -55,6 +55,15 @@ extern int of_get_flat_dt_subnode_by_name(unsigned long node,
 					  const char *uname);
 extern const void *of_get_flat_dt_prop(unsigned long node, const char *name,
 				       int *size);
+
+extern const __be32 *of_flat_dt_get_addr_size_prop(unsigned long node,
+						   const char *name,
+						   int *entries);
+extern bool of_flat_dt_get_addr_size(unsigned long node, const char *name,
+				     u64 *addr, u64 *size);
+extern void of_flat_dt_read_addr_size(const __be32 *prop, int entry_index,
+				      u64 *addr, u64 *size);
+
 extern int of_flat_dt_is_compatible(unsigned long node, const char *name);
 extern unsigned long of_get_flat_dt_root(void);
 extern uint32_t of_get_flat_dt_phandle(unsigned long node);
Signature · 1 line
2.51.0
#2 lore raw

Re: [PATCH v3 1/8] of/fdt: Consolidate duplicate code into helper functions

Quoted 3 lines · Yuntao Wang

Currently, there are many pieces of nearly identical code scattered across different places. Consolidate the duplicate code into helper functions to improve maintainability and reduce the likelihood of errors.

Not much improved. Please go to previous version and read the comments.

Best regards,
Krzysztof

#3 lore raw

Re: [PATCH v3 1/8] of/fdt: Consolidate duplicate code into helper functions

Quoted 9 lines · Krzysztof Kozlowski
Quoted 3 lines · Yuntao Wang

Currently, there are many pieces of nearly identical code scattered across different places. Consolidate the duplicate code into helper functions to improve maintainability and reduce the likelihood of errors.

Not much improved. Please go to previous version and read the comments.

Best regards,
Krzysztof

Hi Krzysztof,

scripts/checkpatch.pl indeed still reports some warnings. I noticed them, but I intentionally didn't fix them.

Below is a list of all the warnings, along with my reasons for leaving them unaddressed.

1. WARNING: Missing a blank line after declarations #60: FILE: drivers/of/fdt.c:663:
+ int entry_cells = dt_root_addr_cells + dt_root_size_cells;
+ prop += entry_cells * entry_index;

The function that triggers this warning is:

void __init of_flat_dt_read_addr_size(const __be32 *prop, int entry_index,

				      u64 *addr, u64 *size)
{
	int entry_cells = dt_root_addr_cells + dt_root_size_cells;
	prop += entry_cells * entry_index;

	*addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
	*size = dt_mem_next_cell(dt_root_size_cells, &prop);
}

The warning suggests adding a blank line before

	prop += entry_cells * entry_index;

I didn't add it because, logically,

	int entry_cells = dt_root_addr_cells + dt_root_size_cells;
	prop += entry_cells * entry_index;

forms a single block, just like

	*addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
	*size = dt_mem_next_cell(dt_root_size_cells, &prop);

I think the code is more readable without the blank line.

In fact, I initially combined these two lines

	int entry_cells = dt_root_addr_cells + dt_root_size_cells;
	prop += entry_cells * entry_index;

into a single line:

	prop += (dt_root_addr_cells + dt_root_size_cells) * entry_index;

I added the entry_cells local variable specifically to improve readability.

2. CHECK: extern prototypes should be avoided in .h files
#78: FILE: include/linux/of_fdt.h:59:
+extern const __be32 *of_flat_dt_get_addr_size_prop(unsigned long node,

This is another warning reported by scripts/checkpatch.pl --strict.

This warning says that extern should be removed.

The reason I didn't remove it was to maintain consistency with the existing function declarations.

In include/linux/of_fdt.h, all function declarations use the extern keyword.

Yes, the Linux kernel coding style document

https://docs.kernel.org/process/coding-style.html#function-prototypes

emphasizes

Do not use the extern keyword with function declarations as this makes lines longer and isn’t strictly necessary.

I agree with this.

However, if extern needs to be removed, I think it should be done in a separate patch that removes all instances of extern.

Thanks,
Yuntao

#4 lore raw

Re: [PATCH v3 1/8] of/fdt: Consolidate duplicate code into helper functions

Hi Yuntao,

Quoted 54 lines · Yuntao Wang
Quoted 9 lines · Krzysztof Kozlowski
Quoted 3 lines · Yuntao Wang

Currently, there are many pieces of nearly identical code scattered across different places. Consolidate the duplicate code into helper functions to improve maintainability and reduce the likelihood of errors.

Not much improved. Please go to previous version and read the comments.

Best regards,
Krzysztof

Hi Krzysztof,

scripts/checkpatch.pl indeed still reports some warnings. I noticed them, but I intentionally didn't fix them.

Below is a list of all the warnings, along with my reasons for leaving them unaddressed.

1. WARNING: Missing a blank line after declarations #60: FILE: drivers/of/fdt.c:663:
+ int entry_cells = dt_root_addr_cells + dt_root_size_cells;
+ prop += entry_cells * entry_index;

The function that triggers this warning is:

void __init of_flat_dt_read_addr_size(const __be32 *prop, int entry_index,

                                      u64 *addr, u64 *size)
{
        int entry_cells = dt_root_addr_cells + dt_root_size_cells;
        prop += entry_cells * entry_index;

        *addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
        *size = dt_mem_next_cell(dt_root_size_cells, &prop);
}

The warning suggests adding a blank line before

        prop += entry_cells * entry_index;

I didn't add it because, logically,

        int entry_cells = dt_root_addr_cells + dt_root_size_cells;
        prop += entry_cells * entry_index;

forms a single block, just like

        *addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
        *size = dt_mem_next_cell(dt_root_size_cells, &prop);

I think the code is more readable without the blank line.

In fact, I initially combined these two lines

Quoted 8 lines
        int entry_cells = dt_root_addr_cells + dt_root_size_cells;
        prop += entry_cells * entry_index;

into a single line:

        prop += (dt_root_addr_cells + dt_root_size_cells) * entry_index;

I added the entry_cells local variable specifically to improve readability.

What about:

    void __init of_flat_dt_read_addr_size(const __be32 *prop, int entry_index,
                                          u64 *addr, u64 *size)
    {
            int entry_cells = dt_root_addr_cells + dt_root_size_cells;

            prop += entry_cells * entry_index;
            *addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
            *size = dt_mem_next_cell(dt_root_size_cells, &prop);
    }

?

1. entry_cells is an intermediate variable,
2. prop is prepared just before its use.

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
#5 lore raw

Re: [PATCH v3 1/8] of/fdt: Consolidate duplicate code into helper functions

Quoted 78 lines · Geert Uytterhoeven

Hi Yuntao,

Quoted 54 lines · Yuntao Wang
Quoted 9 lines · Krzysztof Kozlowski
Quoted 3 lines · Yuntao Wang

Currently, there are many pieces of nearly identical code scattered across different places. Consolidate the duplicate code into helper functions to improve maintainability and reduce the likelihood of errors.

Not much improved. Please go to previous version and read the comments.

Best regards,
Krzysztof

Hi Krzysztof,

scripts/checkpatch.pl indeed still reports some warnings. I noticed them, but I intentionally didn't fix them.

Below is a list of all the warnings, along with my reasons for leaving them unaddressed.

1. WARNING: Missing a blank line after declarations #60: FILE: drivers/of/fdt.c:663:
+ int entry_cells = dt_root_addr_cells + dt_root_size_cells;
+ prop += entry_cells * entry_index;

The function that triggers this warning is:

void __init of_flat_dt_read_addr_size(const __be32 *prop, int entry_index,

                                      u64 *addr, u64 *size)
{
        int entry_cells = dt_root_addr_cells + dt_root_size_cells;
        prop += entry_cells * entry_index;

        *addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
        *size = dt_mem_next_cell(dt_root_size_cells, &prop);
}

The warning suggests adding a blank line before

        prop += entry_cells * entry_index;

I didn't add it because, logically,

        int entry_cells = dt_root_addr_cells + dt_root_size_cells;
        prop += entry_cells * entry_index;

forms a single block, just like

        *addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
        *size = dt_mem_next_cell(dt_root_size_cells, &prop);

I think the code is more readable without the blank line.

In fact, I initially combined these two lines

Quoted 8 lines
        int entry_cells = dt_root_addr_cells + dt_root_size_cells;
        prop += entry_cells * entry_index;

into a single line:

        prop += (dt_root_addr_cells + dt_root_size_cells) * entry_index;

I added the entry_cells local variable specifically to improve readability.

What about:

    void __init of_flat_dt_read_addr_size(const __be32 *prop, int entry_index,
                                          u64 *addr, u64 *size)
    {
            int entry_cells = dt_root_addr_cells + dt_root_size_cells;

            prop += entry_cells * entry_index;
            *addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
            *size = dt_mem_next_cell(dt_root_size_cells, &prop);
    }
Quoted 4 lines

?

1. entry_cells is an intermediate variable,
2. prop is prepared just before its use.

Hi Geert,

Yes, if this warning really needs to be fixed, that's exactly how it should be done.

Thanks,
Yuntao

Quoted 11 lines

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#6 lore raw

PATCH v3 2/8 of/fdt: Fix the len check in early_init_dt_check_for_elfcorehdr()

In reply to Yuntao Wang

The len value is in bytes, while dt_root_addr_cells + dt_root_size_cells is in cells (4 bytes per cell). Comparing them directly is incorrect.

Use a helper function to simplify the code and address this issue.

Fixes f7e7ce93aac1 ("of: fdt: Add generic support for handling elf core headers property")
Fixes e62aaeac426ab1dd ("arm64: kdump: provide /proc/vmcore file")
---
 drivers/of/fdt.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)
drivers/of/fdt.c +2 -8
index 0c18bdefbbee..b45f60dccd7c 100644
@@ -853,21 +853,15 @@ static void __init early_init_dt_check_for_initrd(unsigned long node)
  */
 static void __init early_init_dt_check_for_elfcorehdr(unsigned long node)
 {
-	const __be32 *prop;
-	int len;
-
 	if (!IS_ENABLED(CONFIG_CRASH_DUMP))
 		return;
 
 	pr_debug("Looking for elfcorehdr property... ");
 
-	prop = of_get_flat_dt_prop(node, "linux,elfcorehdr", &len);
-	if (!prop || (len < (dt_root_addr_cells + dt_root_size_cells)))
+	if (!of_flat_dt_get_addr_size(node, "linux,elfcorehdr",
+				      &elfcorehdr_addr, &elfcorehdr_size))
 		return;
 
-	elfcorehdr_addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
-	elfcorehdr_size = dt_mem_next_cell(dt_root_size_cells, &prop);
-
 	pr_debug("elfcorehdr_start=0x%llx elfcorehdr_size=0x%llx\n",
 		 elfcorehdr_addr, elfcorehdr_size);
 }
Signature · 1 line
2.51.0
#7 lore raw

PATCH v3 3/8 of/fdt: Fix the len check in early_init_dt_check_for_usable_mem_range()

In reply to Yuntao Wang

The len value is in bytes, while dt_root_addr_cells + dt_root_size_cells is in cells (4 bytes per cell). Modulo calculation between them is incorrect, the units must be converted first.

Use helper functions to simplify the code and fix this issue.

Fixes fb319e77a0e7 ("of: fdt: Add memory for devices by DT property "linux,usable-memory-range"")
Fixes 2af2b50acf9b9c38 ("of: fdt: Add generic support for handling usable memory range property")
Fixes 8f579b1c4e347b23 ("arm64: limit memory regions based on DT property, usable-memory-range")
---
 drivers/of/fdt.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)
drivers/of/fdt.c +11 -7
index b45f60dccd7c..ea37ba694bcd 100644
@@ -884,8 +884,9 @@ static unsigned long chosen_node_offset = -FDT_ERR_NOTFOUND;
 void __init early_init_dt_check_for_usable_mem_range(void)
 {
 	struct memblock_region rgn[MAX_USABLE_RANGES] = {0};
-	const __be32 *prop, *endp;
+	const __be32 *prop;
 	int len, i;
+	u64 base, size;
 	unsigned long node = chosen_node_offset;
 
 	if ((long)node < 0)
@@ -893,14 +894,17 @@ void __init early_init_dt_check_for_usable_mem_range(void)
 
 	pr_debug("Looking for usable-memory-range property... ");
 
-	prop = of_get_flat_dt_prop(node, "linux,usable-memory-range", &len);
-	if (!prop || (len % (dt_root_addr_cells + dt_root_size_cells)))
+	prop = of_flat_dt_get_addr_size_prop(node, "linux,usable-memory-range",
+					     &len);
+	if (!prop)
 		return;
 
-	endp = prop + (len / sizeof(__be32));
-	for (i = 0; i < MAX_USABLE_RANGES && prop < endp; i++) {
-		rgn[i].base = dt_mem_next_cell(dt_root_addr_cells, &prop);
-		rgn[i].size = dt_mem_next_cell(dt_root_size_cells, &prop);
+	len = min(len, MAX_USABLE_RANGES);
+
+	for (i = 0; i < len; i++) {
+		of_flat_dt_read_addr_size(prop, i, &base, &size);
+		rgn[i].base = base;
+		rgn[i].size = size;
 
 		pr_debug("cap_mem_regions[%d]: base=%pa, size=%pa\n",
 			 i, &rgn[i].base, &rgn[i].size);
Signature · 1 line
2.51.0
#8 lore raw

PATCH v3 4/8 of/fdt: Fix incorrect use of dt_root_addr_cells in early_init_dt_check_kho()

In reply to Yuntao Wang

When reading the fdt_size value, the argument passed to dt_mem_next_cell() is dt_root_addr_cells, but it should be dt_root_size_cells.

The same issue occurs when reading the scratch_size value.

Use a helper function to simplify the code and fix these issues.

Fixes 274cdcb1c004 ("arm64: add KHO support")
---
 drivers/of/fdt.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)
drivers/of/fdt.c +4 -12
index ea37ba694bcd..fdaee4906836 100644
@@ -922,26 +922,18 @@ static void __init early_init_dt_check_kho(void)
 {
 	unsigned long node = chosen_node_offset;
 	u64 fdt_start, fdt_size, scratch_start, scratch_size;
-	const __be32 *p;
-	int l;
 
 	if (!IS_ENABLED(CONFIG_KEXEC_HANDOVER) || (long)node < 0)
 		return;
 
-	p = of_get_flat_dt_prop(node, "linux,kho-fdt", &l);
-	if (l != (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32))
+	if (!of_flat_dt_get_addr_size(node, "linux,kho-fdt",
+				      &fdt_start, &fdt_size))
 		return;
 
-	fdt_start = dt_mem_next_cell(dt_root_addr_cells, &p);
-	fdt_size = dt_mem_next_cell(dt_root_addr_cells, &p);
-
-	p = of_get_flat_dt_prop(node, "linux,kho-scratch", &l);
-	if (l != (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32))
+	if (!of_flat_dt_get_addr_size(node, "linux,kho-scratch",
+				      &scratch_start, &scratch_size))
 		return;
 
-	scratch_start = dt_mem_next_cell(dt_root_addr_cells, &p);
-	scratch_size = dt_mem_next_cell(dt_root_addr_cells, &p);
-
 	kho_populate(fdt_start, fdt_size, scratch_start, scratch_size);
 }
 
Signature · 1 line
2.51.0
#9 lore raw

PATCH v3 5/8 of/fdt: Simplify the logic of early_init_dt_scan_memory()

In reply to Yuntao Wang

Use the existing helper functions to simplify the logic of early_init_dt_scan_memory()

---
 drivers/of/fdt.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)
drivers/of/fdt.c +7 -9
index fdaee4906836..d378d4b4109f 100644
@@ -1033,8 +1033,8 @@ int __init early_init_dt_scan_memory(void)
 
 	fdt_for_each_subnode(node, fdt, 0) {
 		const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
-		const __be32 *reg, *endp;
-		int l;
+		const __be32 *reg;
+		int i, l;
 		bool hotpluggable;
 
 		/* We are scanning "memory" nodes only */
@@ -1044,23 +1044,21 @@ int __init early_init_dt_scan_memory(void)
 		if (!of_fdt_device_is_available(fdt, node))
 			continue;
 
-		reg = of_get_flat_dt_prop(node, "linux,usable-memory", &l);
+		reg = of_flat_dt_get_addr_size_prop(node, "linux,usable-memory", &l);
 		if (reg == NULL)
-			reg = of_get_flat_dt_prop(node, "reg", &l);
+			reg = of_flat_dt_get_addr_size_prop(node, "reg", &l);
 		if (reg == NULL)
 			continue;
 
-		endp = reg + (l / sizeof(__be32));
 		hotpluggable = of_get_flat_dt_prop(node, "hotpluggable", NULL);
 
-		pr_debug("memory scan node %s, reg size %d,\n",
+		pr_debug("memory scan node %s, reg {addr,size} entries %d,\n",
 			 fdt_get_name(fdt, node, NULL), l);
 
-		while ((endp - reg) >= (dt_root_addr_cells + dt_root_size_cells)) {
+		for (i = 0; i < l; i++) {
 			u64 base, size;
 
-			base = dt_mem_next_cell(dt_root_addr_cells, &reg);
-			size = dt_mem_next_cell(dt_root_size_cells, &reg);
+			of_flat_dt_read_addr_size(reg, i, &base, &size);
 
 			if (size == 0)
 				continue;
Signature · 1 line
2.51.0
#10 lore raw

PATCH v3 6/8 of/reserved_mem: Simplify the logic of __reserved_mem_reserve_reg()

In reply to Yuntao Wang

Use the existing helper functions to simplify the logic of __reserved_mem_reserve_reg()

---
 drivers/of/of_reserved_mem.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)
drivers/of/of_reserved_mem.c +9 -14
index 2e9ea751ed2d..2f1e4450785e 100644
@@ -154,27 +154,24 @@ static int __init early_init_dt_reserve_memory(phys_addr_t base,
 static int __init __reserved_mem_reserve_reg(unsigned long node,
 					     const char *uname)
 {
-	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
 	phys_addr_t base, size;
-	int len;
+	int i, len;
 	const __be32 *prop;
 	bool nomap;
 
-	prop = of_get_flat_dt_prop(node, "reg", &len);
+	prop = of_flat_dt_get_addr_size_prop(node, "reg", &len);
 	if (!prop)
 		return -ENOENT;
 
-	if (len && len % t_len != 0) {
-		pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
-		       uname);
-		return -EINVAL;
-	}
-
 	nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
 
-	while (len >= t_len) {
-		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
-		size = dt_mem_next_cell(dt_root_size_cells, &prop);
+	for (i = 0; i < len; i++) {
+		u64 b, s;
+
+		of_flat_dt_read_addr_size(prop, i, &b, &s);
+
+		base = b;
+		size = s;
 
 		if (size && early_init_dt_reserve_memory(base, size, nomap) == 0) {
 			/* Architecture specific contiguous memory fixup. */
@@ -187,8 +184,6 @@ static int __init __reserved_mem_reserve_reg(unsigned long node,
 			pr_err("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
 			       uname, &base, (unsigned long)(size / SZ_1M));
 		}
-
-		len -= t_len;
 	}
 	return 0;
 }
Signature · 1 line
2.51.0
#11 lore raw

PATCH v3 7/8 of/reserved_mem: Simplify the logic of fdt_scan_reserved_mem_reg_nodes()

In reply to Yuntao Wang

Use the existing helper functions to simplify the logic of fdt_scan_reserved_mem_reg_nodes()

---
 drivers/of/of_reserved_mem.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)
drivers/of/of_reserved_mem.c +7 -18
index 2f1e4450785e..0ceb096c17e6 100644
@@ -225,12 +225,9 @@ static void __init __rmem_check_for_overlap(void);
  */
 void __init fdt_scan_reserved_mem_reg_nodes(void)
 {
-	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
 	const void *fdt = initial_boot_params;
 	phys_addr_t base, size;
-	const __be32 *prop;
 	int node, child;
-	int len;
 
 	if (!fdt)
 		return;
@@ -251,29 +248,21 @@ void __init fdt_scan_reserved_mem_reg_nodes(void)
 
 	fdt_for_each_subnode(child, fdt, node) {
 		const char *uname;
+		u64 b, s;
 
-		prop = of_get_flat_dt_prop(child, "reg", &len);
-		if (!prop)
-			continue;
 		if (!of_fdt_device_is_available(fdt, child))
 			continue;
 
-		uname = fdt_get_name(fdt, child, NULL);
-		if (len && len % t_len != 0) {
-			pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
-			       uname);
+		if (!of_flat_dt_get_addr_size(child, "reg", &b, &s))
 			continue;
-		}
-
-		if (len > t_len)
-			pr_warn("%s() ignores %d regions in node '%s'\n",
-				__func__, len / t_len - 1, uname);
 
-		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
-		size = dt_mem_next_cell(dt_root_size_cells, &prop);
+		base = b;
+		size = s;
 
-		if (size)
+		if (size) {
+			uname = fdt_get_name(fdt, child, NULL);
 			fdt_reserved_mem_save_node(child, uname, base, size);
+		}
 	}
 
 	/* check for overlapping reserved regions */
Signature · 1 line
2.51.0
#12 lore raw

PATCH v3 8/8 of/reserved_mem: Simplify the logic of __reserved_mem_alloc_size()

In reply to Yuntao Wang

Use the existing helper functions to simplify the logic of __reserved_mem_alloc_size()

---
 drivers/of/of_reserved_mem.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)
drivers/of/of_reserved_mem.c +7 -14
index 0ceb096c17e6..5619ec917858 100644
@@ -385,10 +385,9 @@ static int __init __reserved_mem_alloc_in_range(phys_addr_t size,
  */
 static int __init __reserved_mem_alloc_size(unsigned long node, const char *uname)
 {
-	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
 	phys_addr_t start = 0, end = 0;
 	phys_addr_t base = 0, align = 0, size;
-	int len;
+	int i, len;
 	const __be32 *prop;
 	bool nomap;
 	int ret;
@@ -422,19 +421,15 @@ static int __init __reserved_mem_alloc_size(unsigned long node, const char *unam
 	    && !nomap)
 		align = max_t(phys_addr_t, align, CMA_MIN_ALIGNMENT_BYTES);
 
-	prop = of_get_flat_dt_prop(node, "alloc-ranges", &len);
+	prop = of_flat_dt_get_addr_size_prop(node, "alloc-ranges", &len);
 	if (prop) {
+		for (i = 0; i < len; i++) {
+			u64 b, s;
 
-		if (len % t_len != 0) {
-			pr_err("invalid alloc-ranges property in '%s', skipping node.\n",
-			       uname);
-			return -EINVAL;
-		}
+			of_flat_dt_read_addr_size(prop, i, &b, &s);
 
-		while (len > 0) {
-			start = dt_mem_next_cell(dt_root_addr_cells, &prop);
-			end = start + dt_mem_next_cell(dt_root_size_cells,
-						       &prop);
+			start = b;
+			end = b + s;
 
 			base = 0;
 			ret = __reserved_mem_alloc_in_range(size, align,
@@ -445,9 +440,7 @@ static int __init __reserved_mem_alloc_size(unsigned long node, const char *unam
 					(unsigned long)(size / SZ_1M));
 				break;
 			}
-			len -= t_len;
 		}
-
 	} else {
 		ret = early_init_dt_alloc_reserved_memory_arch(size, align,
 							0, 0, nomap, &base);
Signature · 1 line
2.51.0
maintainer
#13 lore raw
In reply to Yuntao Wang
Quoted 24 lines · Yuntao Wang

This patch series fixes several bugs related to dt_root_addr_cells and dt_root_size_cells, and performs some cleanup.

Changelog:

v2 -> v3:
- Use of_flat_dt_ instead of of_fdt_ as the prefix for the newly added helper functions

- Improve the internal logic of of_flat_dt_get_addr_size_prop() and

    of_flat_dt_get_addr_size()

- Introduce the entry_index parameter for of_flat_dt_read_addr_size()

- Fix some warnings and coding-style issues

v1 -> v2:
- Consolidate duplicate code into helper functions

Links to previous patch series:

v2: https://lore.kernel.org/linux-devicetree/[email protected]/
v1: https://lore.kernel.org/linux-devicetree/[email protected]/

Quoted 12 lines

Yuntao Wang (8):
of/fdt: Consolidate duplicate code into helper functions of/fdt: Fix the len check in early_init_dt_check_for_elfcorehdr() of/fdt: Fix the len check in
early_init_dt_check_for_usable_mem_range()
of/fdt: Fix incorrect use of dt_root_addr_cells in
early_init_dt_check_kho()
of/fdt: Simplify the logic of early_init_dt_scan_memory() of/reserved_mem: Simplify the logic of __reserved_mem_reserve_reg() of/reserved_mem: Simplify the logic of
fdt_scan_reserved_mem_reg_nodes()
of/reserved_mem: Simplify the logic of __reserved_mem_alloc_size()

I've applied the series. I was going to send the first 4 for 6.18, but since we're pretty close to the merge window, I decided to keep it all together for 6.19.

Thanks for all the clean-ups.

Rob