lkml.org 
[lkml]   [2013]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] OF: base: match each node compatible against all given matches first
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 <sebastian.hesselbarth@gmail.com>
> ---
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/of/base.c | 47 ++++++++++++++++++++++++++++++++---------------
> 1 file changed, 32 insertions(+), 15 deletions(-)

Hi Sebastian,

I think you're one in a long line of people attempting to fix this. I
tried myself over a year ago (commit 107a84e61cdd 'of: match by
compatible property first') but it caused a subtle regression late in
the release cycle and was reverted (commit bc51b0c22ceb 'Revert "of:
match by compatible property first").

Only recently there was another attempt [0] but it's pretty much
equivalent to what I did back then.

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.

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;
> +
> + while (cplen > 0) {
> + 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])
> + match &= !of_compat_cmp(cp, m->compatible,
> + strlen(m->compatible));
> + if (match)
> + return m;
> + m++;
> + }
> + l = strlen(cp) + 1;
> + cp += l;
> + cplen -= l;
> }
> return NULL;
> }
> @@ -739,7 +753,10 @@ const struct of_device_id *__of_match_node(const struct of_device_id *matches,
> * @matches: array of of device match structures to search in
> * @node: the of device structure to match against
> *
> - * Low level utility function used by device matching.
> + * Low level utility function used by device matching. Matching order
> + * is to compare each of the node's compatibles with all given matches
> + * first. This implies node's compatible is sorted from specific to
> + * generic while matches can be in any order.
> */
> const struct of_device_id *of_match_node(const struct of_device_id *matches,
> const struct device_node *node)
> --
> 1.7.10.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2013-12-02 13:21    [W:0.120 / U:0.160 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site