Thread (7 messages) 7 messages, 3 authors, 2021-12-15

Re: [PATCH v2] of/fdt: Rework early_init_dt_scan_memory() to call directly

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2021-12-14 11:18:31
Also in: linux-devicetree, linuxppc-dev, lkml
Subsystem: linux for powerpc (32-bit and 64-bit), the rest · Maintainers: Madhavan Srinivasan, Michael Ellerman, Linus Torvalds

Rob Herring [off-list ref] writes:
On Mon, Dec 13, 2021 at 6:47 AM Michael Ellerman [off-list ref] wrote:
quoted
Rob Herring [off-list ref] writes:
quoted
Use of the of_scan_flat_dt() function predates libfdt and is discouraged
as libfdt provides a nicer set of APIs. Rework
early_init_dt_scan_memory() to be called directly and use libfdt.
...
quoted
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 6e1a106f02eb..63762a3b75e8 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -532,19 +532,19 @@ static int  __init early_init_drmem_lmb(struct drmem_lmb *lmb,
 }
 #endif /* CONFIG_PPC_PSERIES */

-static int __init early_init_dt_scan_memory_ppc(unsigned long node,
-                                             const char *uname,
-                                             int depth, void *data)
+static int __init early_init_dt_scan_memory_ppc(void)
 {
 #ifdef CONFIG_PPC_PSERIES
-     if (depth == 1 &&
-         strcmp(uname, "ibm,dynamic-reconfiguration-memory") == 0) {
+     const void *fdt = initial_boot_params;
+     int node = fdt_path_offset(fdt, "/ibm,dynamic-reconfiguration-memory");
+
+     if (node > 0) {
              walk_drmem_lmbs_early(node, NULL, early_init_drmem_lmb);
              return 0;
      }
It's that return that is the problem.

Now that early_init_dt_scan_memory_ppc() is only called once, that
return causes us to skip scanning regular memory nodes if there is an
"ibm,dynamic-reconfiguration-memory" property present.

So the fix is just:
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 1098de3b172f..125661e5fcf3 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -538,10 +538,8 @@ static int __init early_init_dt_scan_memory_ppc(void)
 	const void *fdt = initial_boot_params;
 	int node = fdt_path_offset(fdt, "/ibm,dynamic-reconfiguration-memory");
 
-	if (node > 0) {
+	if (node > 0)
 		walk_drmem_lmbs_early(node, NULL, early_init_drmem_lmb);
-		return 0;
-	}
 #endif
 	
 	return early_init_dt_scan_memory();

The only thing I see is now there is an assumption that 'memory' nodes
are off the root node only. Before they could be anywhere.
I don't know of any machines where that would be a problem. But given
all the wild and wonderful device trees out there, who really knows :)

Maybe we should continue to allow memory nodes to be anywhere, and print
a warning for any that aren't at the root. Then if no one reports any
hits for the warning we could switch to only allowing them at the root?

cheers

quoted hunk ↗ jump to hunk
index a835c458f50a..97d7607625ec 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -1083,16 +1083,13 @@ int __init early_init_dt_scan_memory(void)
        int node;
        const void *fdt = initial_boot_params;

-       fdt_for_each_subnode(node, fdt, 0) {
-               const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
+       for (node = fdt_node_offset_by_prop_value(fdt, -1, "device_type", "memory", 6);
+            node != -FDT_ERR_NOTFOUND;
+            node = fdt_node_offset_by_prop_value(fdt, node, "device_type", "memory", 6)) {
                const __be32 *reg, *endp;
                int l;
                bool hotpluggable;

-               /* We are scanning "memory" nodes only */
-               if (type == NULL || strcmp(type, "memory") != 0)
-                       continue;
-
                reg = of_get_flat_dt_prop(node, "linux,usable-memory", &l);
                if (reg == NULL)
                        reg = of_get_flat_dt_prop(node, "reg", &l);
Rob
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help