Thread (8 messages) 8 messages, 3 authors, 2011-02-23

Re: [PATCH] of/pdt: allow DT device matching by fixing 'name' brokenness

From: Andres Salomon <dilinger@queued.net>
Date: 2011-02-23 19:54:20
Also in: lkml, sparclinux

On Wed, 23 Feb 2011 12:43:52 -0700
Grant Likely [off-list ref] wrote:
On Fri, Feb 18, 2011 at 07:06:59PM -0800, Andres Salomon wrote:
quoted
On Fri, 18 Feb 2011 23:42:57 +0000
Daniel Drake [off-list ref] wrote:
quoted
On 16 February 2011 22:44, David Woodhouse [off-list ref]
wrote:
quoted
On Wed, 2011-02-16 at 22:28 +0000, Daniel Drake wrote:
quoted
+static int __init add_common_platform_devices(void)
+{
+       struct platform_device *pdev;
+
+       pdev = platform_device_register_simple("olpc-battery",
-1, NULL, 0);
+       if (IS_ERR(pdev))
+               return PTR_ERR(pdev);
+
+       return 0;
+}
+
Still kind of sucks that you have to do this, and can't bind to
something in the device-tree.
OK, feel free to put this patch on hold for now. I started
looking at the device tree approach today. It looks doable but
first we have to fix a DT bug/inconsistency that is preventing us
from correctly binding to the tree's devices.

Daniel

Mea culpa.  The patch below fixes a bug I introduced earlier.
Cc'ing the sparc folks, as this probably affects them
(although I would think that it fixes broken behavior for them..?)
Wait; why are you binding to a device based on name?  Binding by name
and/or device_type is strongly discouraged for new code.  Use
compatible instead.
Daniel posted a separate patch showing his code, would you mind
commenting on that?  I noticed he didn't cc you though, here's the
patch:

https://patchwork.kernel.org/patch/574901/

As for this patch, comments below...
quoted
From: Andres Salomon <dilinger@queued.net>

Commit e2f2a93b changed dp->name from using the 'name' property to
using package-to-path.  This fixed /proc/device-tree creation by
eliminating conflicts between names (the 'name' property provides
names like 'battery', whereas package-to-path provides names like
'/foo/bar/battery@0', which we stripped to 'battery@0').  However,
it also breaks of_device_id table matching.

The fix that we _really_ wanted was to keep dp->name based upon
the name property ('battery'), but based dp->full_name upon
package-to-path ('battery@0').  This patch does just that.

Signed-off-by: Andres Salomon <dilinger@queued.net>
Reported-by: Daniel Drake <redacted>
From what I can tell, this only affects OLPC, correct?  It looks like
SPARC's implementation of of_pdt_node_name() will sidestep most of
this name retrieval code.

This affects sparc as well; it changes behavior back for dp->name to
what it used to be.  dp->full_name behavior is still the same for sparc.

quoted
---
 drivers/of/pdt.c |   42 +++++++++++++++++++-----------------------
 1 files changed, 19 insertions(+), 23 deletions(-)
diff --git a/drivers/of/pdt.c b/drivers/of/pdt.c
index 28295d0..b39d584 100644
--- a/drivers/of/pdt.c
+++ b/drivers/of/pdt.c
@@ -48,7 +48,7 @@ static inline void irq_trans_init(struct
device_node *dp) { } 
 static inline const char *of_pdt_node_name(struct device_node *dp)
 {
-	return dp->name;
+	return NULL;
 }
Rather than using this hook; perhaps the sparc .path_component_name
should be enabled for all architectures.  It is a useful data item to
keep a pointer to, and it would simplify the of_pdt code.
quoted
 
Yeah, I considered that, but pkg2path works even better for our
purposes.


I'll combine patches and resend, thanks.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help