Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.

On 02/12/14 11:31, Kevin Hao wrote:

On Wed, Feb 12, 2014 at 09:30:00AM +0100, Sebastian Hesselbarth wrote:

quoted

On 02/12/2014 06:28 AM, Kevin Hao wrote:

quoted

On Wed, Feb 12, 2014 at 10:21:58AM +1000, Stephen N Chivers wrote:

quoted

But, the Interrupt Controller (MPIC)
goes AWOL and it is down hill from there.

The MPIC is specified in the DTS as:

         mpic: pic@40000 {
                         interrupt-controller;
                         #address-cells = <0>;
                         #interrupt-cells = <2>;
                         reg = <0x40000 0x40000>;
                         compatible = "chrp,open-pic";
                         device_type = "open-pic";
                         big-endian;
                 };

The board support file has the standard mechanism for allocating
the PIC:

         struct mpic *mpic;

         mpic = mpic_alloc(NULL, 0, 0, 0, 256, " OpenPIC  ");
         BUG_ON(mpic == NULL);

         mpic_init(mpic);

I checked for damage in applying the patch and it has applied
correctly.

How about the following fix?

diff --git a/drivers/of/base.c b/drivers/of/base.c
index ff85450d5683..ca91984d3c4b 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c

@@ -730,32 +730,40 @@ out:
  }
  EXPORT_SYMBOL(of_find_node_with_property);

+static int of_match_type_name(const struct device_node *node,
+				const struct of_device_id *m)

I am fine with having a sub-function here, but it should rather be
named of_match_type_or_name.

OK.

quoted

quoted

+{
+	int match = 1;
+
+	if (m->name[0])
+		match &= node->name && !strcmp(m->name, node->name);
+
+	if (m->type[0])
+		match &= node->type && !strcmp(m->type, node->type);
+
+	return match;
+}

[...]

quoted

+	/* Check against matches without compatible string */
+	m = matches;
+	while (!m->compatible[0] && (m->name[0] || m->type[0])) {

We shouldn't check for anything else than the sentinel here.
Although I guess yours will not quit early as mine did but that
way we don't have to worry about it.

Yes, this is still buggy. I will change something like this:

	m = matches;
	/* Check against matches without compatible string */
	while (m->name[0] || m->type[0] || m->compatible[0]) {
		if (m->compatible[0]) {
			m++;
			continue;
		}

		match = of_match_type_name(node, m);
		if (match)
			return m;
		m++;
	}

You can cook it down to:

	m = matches;
	/* Check against matches without compatible string */
	while (m->name[0] || m->type[0] || m->compatible[0]) {
		if (!m->compatible[0] && of_match_type_or_name(node, m)
			return m;
		m++;
	}





--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html