On Wed, Feb 12, 2014 at 09:30:00AM +0100, Sebastian Hesselbarth wrote:
> On 02/12/2014 06:28 AM, Kevin Hao wrote:
> >On Wed, Feb 12, 2014 at 10:21:58AM +1000, Stephen N Chivers wrote:
> >>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.

> 
> >+{
> >+	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;
> >+}
> [...]
> >+	/* 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++;
	}

Thanks,
Kevin