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
Attachment:
pgpmm9q34bLeU.pgp
Description: PGP signature
