Patchwork device tree variations

login
register
mail settings
Submitter Mike Ditto
Date Oct. 18, 2008, 2:14 a.m.
Message ID <48F9466D.8060603@consentry.com>
Download mbox | patch
Permalink /patch/4922/
State Superseded
Delegated to: David Gibson
Headers show

Comments

Mike Ditto - Oct. 18, 2008, 2:14 a.m.
David Gibson wrote:
> Deleting the irrelevant parts or picking a device tree to pass to
> fdt_init() are both reasonable solutions.  libfdt which is included in
> the bootwrapper has functions for removing unwanted nodes: either
> fdt_nop_node() or fdt_del_node() will suffice.  There isn't currently
> a dt_ops hook to call though to those functions though.  You could
> either add one, or (knowing that your platform always has a flat dt)
> bypass the dt_ops hooks and call libfdt directly.

Thanks.  The fdt_del_node approach works pretty nicely.  I added a
dt_ops hook since fdt is static in libfdt-wrapper.c.

At first I tried fdt_nop_node, fearing that find_node_by_prop_value()
and fdt_del_node() would interact badly when deleting in mid-traversal,
but it turns out that fdt_nop_node() upsets find_node_by_prop_value()
anyway.  Plus, the kernel prints harmless but strange messages when
there are NOPs in the tree.  So I use fdt_del_node() and rescan from
the top each time I delete a node and it works fine.

	/* Find all product-dependent nodes and delete inapplicable ones. */
	dev = NULL;
	while ((dev = find_node_by_prop_value(dev,
					      "product-dependent",
					      "", 0)) != NULL) {
		u32 mask;
		int len;

		len = getprop(dev, "productmask", &mask, sizeof mask);
		if (len == sizeof mask) {
			if ((mask & (1 << product_id)) == 0) {
				del_node(dev);
				/* Have to restart from the beginning. */
				dev = NULL;
			}
		}
	}

I had to fix the memcmp bug I mentioned in an earlier posting to allow
searching for a boolean (empty) property.

If anyone cares, below is a trivial patch to expose the del_node()
operation via dt_ops.

Thanks again,
					-=] Mike [=-
David Gibson - Oct. 21, 2008, 1:49 a.m.
On Fri, Oct 17, 2008 at 07:14:05PM -0700, Mike Ditto wrote:
> David Gibson wrote:
> > Deleting the irrelevant parts or picking a device tree to pass to
> > fdt_init() are both reasonable solutions.  libfdt which is included in
> > the bootwrapper has functions for removing unwanted nodes: either
> > fdt_nop_node() or fdt_del_node() will suffice.  There isn't currently
> > a dt_ops hook to call though to those functions though.  You could
> > either add one, or (knowing that your platform always has a flat dt)
> > bypass the dt_ops hooks and call libfdt directly.
> 
> Thanks.  The fdt_del_node approach works pretty nicely.  I added a
> dt_ops hook since fdt is static in libfdt-wrapper.c.
> 
> At first I tried fdt_nop_node, fearing that find_node_by_prop_value()
> and fdt_del_node() would interact badly when deleting in
> mid-traversal,

Ah, yes, it would.

> but it turns out that fdt_nop_node() upsets find_node_by_prop_value()
> anyway.

Oh... it shouldn't, I think that's a bug.  I'll try to look into that
and fix it.

>  Plus, the kernel prints harmless but strange messages when
> there are NOPs in the tree.

We should probably fix that too.

>  So I use fdt_del_node() and rescan from
> the top each time I delete a node and it works fine.

Kind of ugly, but ok.
Benjamin Herrenschmidt - Oct. 21, 2008, 6 a.m.
> Thanks.  The fdt_del_node approach works pretty nicely.  I added a
> dt_ops hook since fdt is static in libfdt-wrapper.c.

 .../...

David says your patch is ok, However it's not in the right form. Could
you repost it please with a proper changeset comment and signed-off-by:
line ?

Thanks !

Cheers,
Ben.

Patch

Index: arch/powerpc/boot/ops.h
===================================================================
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 ops.h
--- arch/powerpc/boot/ops.h	11 Oct 2008 02:51:35 -0000	1.1.1.1
+++ arch/powerpc/boot/ops.h	18 Oct 2008 02:06:45 -0000
@@ -40,6 +40,7 @@ 
 			const int buflen);
 	int	(*setprop)(const void *phandle, const char *name,
 			const void *buf, const int buflen);
+	int (*del_node)(const void *phandle);
 	void *(*get_parent)(const void *phandle);
 	/* The node must not already exist. */
 	void *(*create_node)(const void *parent, const char *name);
@@ -124,6 +125,11 @@ 
 		return dt_ops.setprop(devp, name, buf, strlen(buf) + 1);

 	return -1;
+}
+
+static inline int del_node(const void *devp)
+{
+	return dt_ops.del_node ? dt_ops.del_node(devp) : -1;
 }

 static inline void *get_parent(const char *devp)
Index: arch/powerpc/boot/libfdt-wrapper.c
===================================================================
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 libfdt-wrapper.c
--- arch/powerpc/boot/libfdt-wrapper.c	11 Oct 2008 02:51:35 -0000	1.1.1.1
+++ arch/powerpc/boot/libfdt-wrapper.c	17 Oct 2008 22:08:44 -0000
@@ -105,6 +105,11 @@ 
 	return check_err(rc);
 }

+static int fdt_wrapper_del_node(const void *devp)
+{
+	return fdt_del_node(fdt, devp_offset(devp));
+}
+
 static void *fdt_wrapper_get_parent(const void *devp)
 {
 	return offset_devp(fdt_parent_offset(fdt, devp_offset(devp)));
@@ -173,6 +178,7 @@ 
 	dt_ops.create_node = fdt_wrapper_create_node;
 	dt_ops.find_node_by_prop_value = fdt_wrapper_find_node_by_prop_value;
 	dt_ops.find_node_by_compatible = fdt_wrapper_find_node_by_compatible;
+	dt_ops.del_node = fdt_wrapper_del_node;
 	dt_ops.get_path = fdt_wrapper_get_path;
 	dt_ops.finalize = fdt_wrapper_finalize;