Patchwork of: require a match on all fields of of_device_id

login
register
mail settings
Submitter Scott Wood
Date July 18, 2012, 1:11 a.m.
Message ID <20120718011151.GA6119@tyr.buserror.net>
Download mbox | patch
Permalink /patch/171585/
State Not Applicable
Headers show

Comments

Scott Wood - July 18, 2012, 1:11 a.m.
Commit 107a84e61cdd3406c842a0e4be7efffd3a05dba6 ("of: match by compatible
property first") breaks the gianfar ethernet driver found on various
Freescale PPC chips.

There are, for unfortunate historical reasons, two nodes with a
compatible of "gianfar".  One has a device_type of "network" and the
other has device_type of "mdio".  The match entries look like this:

>         {
>                 .type = "mdio",
>                 .compatible = "gianfar",
>         },

and

>         {
>                 .type = "network",
>                 .compatible = "gianfar",
>         },

With the above patch, both nodes get probed by the first driver, because
nothing else in the match struct is looked at if there's a compatible
match.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 drivers/of/base.c |   44 ++++++++++++++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 12 deletions(-)
Tabi Timur-B04825 - July 18, 2012, 1:57 a.m.
On Tue, Jul 17, 2012 at 8:11 PM, Scott Wood <scottwood@freescale.com> wrote:
> Commit 107a84e61cdd3406c842a0e4be7efffd3a05dba6 ("of: match by compatible
> property first") breaks the gianfar ethernet driver found on various
> Freescale PPC chips.
>
> There are, for unfortunate historical reasons, two nodes with a
> compatible of "gianfar".

Would it be worth updating the binding for the two nodes to make the
compatible property different?  We could do something like this:

ethernet@24000 {
                        reg = <0x24000 0x1000>;
                        compatible = "fsl,gianfar-eth", "gianfar";
                        ...
};

(and something similar for MDIO nodes)

and update all the drivers to look for both strings.  After a few
years, we can delete "gianfar".

Patch

diff --git a/drivers/of/base.c b/drivers/of/base.c
index bc86ea2..4e707cc 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -511,14 +511,37 @@  out:
 }
 EXPORT_SYMBOL(of_find_node_with_property);
 
-static const struct of_device_id *of_match_compat(const struct of_device_id *matches,
-						  const char *compat)
+/*
+ * Tell if an device_node matches the non-compatible fields of
+ * a specific of_match element.
+ */
+static bool of_match_one_noncompat(const struct of_device_id *match,
+				   const struct device_node *node)
+{
+	bool is_match = true;
+
+	if (match->name[0])
+		is_match &= node->name && !strcmp(match->name, node->name);
+	if (match->type[0])
+		is_match &= node->type && !strcmp(match->type, node->type);
+
+	return is_match;
+}
+
+/*
+ * Find an OF match using the supplied compatible string, rather than
+ * the node's entire string list.
+ */
+static const struct of_device_id *of_match_compat(
+	const struct of_device_id *matches, const char *compat,
+	const struct device_node *node)
 {
 	while (matches->name[0] || matches->type[0] || matches->compatible[0]) {
 		const char *cp = matches->compatible;
 		int len = strlen(cp);
 
-		if (len > 0 && of_compat_cmp(compat, cp, len) == 0)
+		if (len > 0 && of_compat_cmp(compat, cp, len) == 0 &&
+		    of_match_one_noncompat(matches, node))
 			return matches;
 
 		matches++;
@@ -544,23 +567,20 @@  const struct of_device_id *of_match_node(const struct of_device_id *matches,
 		return NULL;
 
 	of_property_for_each_string(node, "compatible", prop, cp) {
-		const struct of_device_id *match = of_match_compat(matches, cp);
+		const struct of_device_id *match =
+			of_match_compat(matches, cp, node);
 		if (match)
 			return match;
 	}
 
 	while (matches->name[0] || matches->type[0] || matches->compatible[0]) {
-		int match = 1;
-		if (matches->name[0])
-			match &= node->name
-				&& !strcmp(matches->name, node->name);
-		if (matches->type[0])
-			match &= node->type
-				&& !strcmp(matches->type, node->type);
-		if (match && !matches->compatible[0])
+		if (of_match_one_noncompat(matches, node) &&
+		    !matches->compatible[0])
 			return matches;
+
 		matches++;
 	}
+
 	return NULL;
 }
 EXPORT_SYMBOL(of_match_node);