public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andres Salomon <dilinger@queued.net>
To: Daniel Drake <dsd@laptop.org>
Cc: David Woodhouse <dwmw2@infradead.org>,
	cbou@mail.ru, linux-kernel@vger.kernel.org, x86@kernel.org,
	tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com,
	dmitry.torokhov@gmail.com,
	Grant Likely <grant.likely@secretlab.ca>,
	devicetree-discuss@lists.ozlabs.org,
	"David S. Miller" <davem@davemloft.net>,
	sparclinux@vger.kernel.org
Subject: [PATCH] of/pdt: allow DT device matching by fixing 'name' brokenness
Date: Fri, 18 Feb 2011 19:06:59 -0800	[thread overview]
Message-ID: <20110218190659.7f955e4c@queued.net> (raw)
In-Reply-To: <AANLkTikAF2X1A5jfoNfCPRhZkZ7qPdZ57c8B5b9z4+qr@mail.gmail.com>

On Fri, 18 Feb 2011 23:42:57 +0000
Daniel Drake <dsd@laptop.org> wrote:

> On 16 February 2011 22:44, David Woodhouse <dwmw2@infradead.org>
> wrote:
> > On Wed, 2011-02-16 at 22:28 +0000, Daniel Drake wrote:
> >>
> >> +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..?)





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 <dsd@laptop.org>
---
 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;
 }
 
 #endif /* !CONFIG_SPARC */
@@ -156,23 +156,6 @@ static char * __init of_pdt_try_pkg2path(phandle node)
 	return res+1;
 }
 
-/*
- * When fetching the node's name, first try using package-to-path; if
- * that fails (either because the arch hasn't supplied a PROM callback,
- * or some other random failure), fall back to just looking at the node's
- * 'name' property.
- */
-static char * __init of_pdt_build_name(phandle node)
-{
-	char *buf;
-
-	buf = of_pdt_try_pkg2path(node);
-	if (!buf)
-		buf = of_pdt_get_one_property(node, "name");
-
-	return buf;
-}
-
 static struct device_node * __init of_pdt_create_node(phandle node,
 						    struct device_node *parent)
 {
@@ -187,7 +170,7 @@ static struct device_node * __init of_pdt_create_node(phandle node,
 
 	kref_init(&dp->kref);
 
-	dp->name = of_pdt_build_name(node);
+	dp->name = of_pdt_get_one_property(node, "name");
 	dp->type = of_pdt_get_one_property(node, "device_type");
 	dp->phandle = node;
 
@@ -198,13 +181,26 @@ static struct device_node * __init of_pdt_create_node(phandle node,
 	return dp;
 }
 
-static char * __init of_pdt_build_full_name(struct device_node *dp)
+static char * __init of_pdt_build_full_name(struct device_node *dp,
+		phandle node)
 {
 	int len, ourlen, plen;
+	const char *name;
 	char *n;
 
+	/*
+	 * When fetching the full name, rather than what we see with the
+	 * name property (ie, 'battery'), we want the name we see with
+	 * package-to-path (ie, 'battery@0').
+	 */
+	name = of_pdt_node_name(dp);
+	if (!name)
+		name = of_pdt_try_pkg2path(node);
+	if (!name)
+		name = dp->name;
+
 	plen = strlen(dp->parent->full_name);
-	ourlen = strlen(of_pdt_node_name(dp));
+	ourlen = strlen(name);
 	len = ourlen + plen + 2;
 
 	n = prom_early_alloc(len);
@@ -213,7 +209,7 @@ static char * __init of_pdt_build_full_name(struct device_node *dp)
 		strcpy(n + plen, "/");
 		plen++;
 	}
-	strcpy(n + plen, of_pdt_node_name(dp));
+	strcpy(n + plen, name);
 
 	return n;
 }
@@ -243,7 +239,7 @@ static struct device_node * __init of_pdt_build_tree(struct device_node *parent,
 #if defined(CONFIG_SPARC)
 		dp->path_component_name = build_path_component(dp);
 #endif
-		dp->full_name = of_pdt_build_full_name(dp);
+		dp->full_name = of_pdt_build_full_name(dp, node);
 
 		dp->child = of_pdt_build_tree(dp,
 				of_pdt_prom_ops->getchild(node), nextp);
-- 
1.7.2.3


  reply	other threads:[~2011-02-19  3:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-16 22:28 [PATCH v3] olpc_battery: convert to platform device Daniel Drake
2011-02-16 22:34 ` Dmitry Torokhov
2011-02-16 22:44 ` David Woodhouse
2011-02-16 23:39   ` H. Peter Anvin
2011-02-18 23:42   ` Daniel Drake
2011-02-19  3:06     ` Andres Salomon [this message]
2011-02-19  3:12       ` [PATCH] of/pdt: don't bother parsing pkg2path results, return as-is Andres Salomon
2011-02-23 19:45         ` Grant Likely
2011-02-23 19:43       ` [PATCH] of/pdt: allow DT device matching by fixing 'name' brokenness Grant Likely
2011-02-23 19:54         ` Andres Salomon
2011-02-23 20:06           ` Daniel Drake
2011-02-23 20:37             ` Grant Likely
2011-02-23 20:35           ` Grant Likely

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110218190659.7f955e4c@queued.net \
    --to=dilinger@queued.net \
    --cc=cbou@mail.ru \
    --cc=davem@davemloft.net \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=dsd@laptop.org \
    --cc=dwmw2@infradead.org \
    --cc=grant.likely@secretlab.ca \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=sparclinux@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox