From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sebastian Hesselbarth Subject: Re: [PATCH] OF: base: match each node compatible against all given matches first Date: Mon, 02 Dec 2013 15:09:54 +0100 Message-ID: <529C94B2.3040408@gmail.com> References: <1385663785-8643-1-git-send-email-sebastian.hesselbarth@gmail.com> <20131202120300.GA12793@ulmo.nvidia.com> <529C927A.9000303@gmail.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <529C927A.9000303-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Rob Herring , Thierry Reding Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Russell King , Benjamin Herrenschmidt , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Grant Likely , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Meelis Roos , Marc Kleine-Budde , Scott Wood List-Id: devicetree@vger.kernel.org On 12/02/13 15:00, Rob Herring wrote: > On 12/02/2013 06:03 AM, Thierry Reding wrote: >> On Thu, Nov 28, 2013 at 07:36:25PM +0100, Sebastian Hesselbarth >> wrote: >>> Currently, of_match_node compares each given match against all >>> node's compatible strings with of_device_is_compatible. >>> >>> To achieve multiple compatible strings per node with ordering >>> from specific to generic, this requires given matches to be >>> ordered from specific to generic. For most of the drivers this >>> is not true and also an alphabetical ordering is more sane >>> there. >>> >>> Therefore, this patch modifies of_match_node to match each of >>> the node's compatible strings against all given matches first, >>> before checking the next compatible string. This implies that >>> node's compatibles are ordered from specific to generic while >>> given matches can be in any order. >>> >>> Signed-off-by: Sebastian Hesselbarth [...] >> That said, I think you might actually have nailed it with this >> patch. From what I remember all earlier attempt failed because >> they didn't match all compatible/name/type combinations properly. >> I'm adding a few people on Cc who were involved with the other >> patches, perhaps they can give your patch a spin and see if it >> fixes things for them. > > I agree and the change here is simple enough I think the chances of a > regression are low. But there is one issue... > >> >> Thierry >> >> [0]: https://lkml.org/lkml/2013/10/3/585 >> >>> diff --git a/drivers/of/base.c b/drivers/of/base.c index >>> 7d4c70f..183a9c7 100644 --- a/drivers/of/base.c +++ >>> b/drivers/of/base.c @@ -713,23 +713,37 @@ static const struct >>> of_device_id *__of_match_node(const struct of_device_id *matches, >>> const struct device_node *node) { + const char *cp; + int cplen, >>> l; + if (!matches) return NULL; >>> >>> - 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 (matches->compatible[0]) - match &= >>> __of_device_is_compatible(node, - matches->compatible); >>> - if (match) - return matches; - matches++; + cp = >>> __of_get_property(node, "compatible", &cplen); + if (!cp) + >>> return NULL; > > No compatible property and matching on name and/or type only is valid. > Remove the if here and changing the following should work: > >>> + + while (cplen > 0) { > > do { > >>> + const struct of_device_id *m = matches; + + while >>> (m->name[0] || m->type[0] || m->compatible[0]) { + 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); + if >>> (m->compatible[0]) > > if (cp && m->compatible[0]) > >>> + match &= !of_compat_cmp(cp, m->compatible, + >>> strlen(m->compatible)); + if (match) + return m; + m++; + >>> } + l = strlen(cp) + 1; + cp += l; + cplen -= l; } > > > } while (cp && (cplen > 0)); Rob, although quoting somehow got messed up, I see the point. The pointer arith right above loop end condition needs to be skipped too if !cp. I will rework the loop to allow name/type matching with cp == NULL and resend in a day or two. Sebastian -- 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