Re: [PATCH v4 2/4] of: reimplement the matching method for __of_match_node()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Wed, Feb 19, 2014 at 8:14 AM, Grant Likely <grant.likely@xxxxxxxxxx> wrote:
> From: Kevin Hao <haokexin@xxxxxxxxx>
>
> In the current implementation of __of_match_node(), it will compare
> each given match entry against all the 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, we define a following priority order for the match, and
> then scan all the entries to find the best match.
>   1. specific compatible && type && name
>   2. specific compatible && type
>   3. specific compatible && name
>   4. specific compatible
>   5. general compatible && type && name
>   6. general compatible && type
>   7. general compatible && name
>   8. general compatible
>   9. type && name
>   10. type
>   11. name

While I agree this should be the right order, I worry that this may be
changing the matching in some case as all previous attempts have done.

>
> v4: Short-circuit failure cases instead of mucking with score, and
>     remove extra __of_device_is_compatible() wrapper stub.
>     Move scoring logic directly into __of_device_is_compatible()
>
> v3: Also need to bail out when there does have a compatible member in match
>     entry, but it doesn't match with the device node's compatible.
>
> v2: Fix the bug such as we get the same score for the following two match
>     entries with the empty node 'name2 { };'
>         struct of_device_id matches[] = {
>                 {.name = "name2", },
>                 {.name = "name2", .type = "type1", },
>                 {}
>         };
>
> Signed-off-by: Kevin Hao <haokexin@xxxxxxxxx>
> [grant.likely: added v4 changes]
> Signed-off-by: Grant Likely <grant.likely@xxxxxxxxxx>
> ---
>  drivers/of/base.c | 108 +++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 74 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index ba195fbce4c6..30ffc291d0a0 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -342,27 +342,73 @@ struct device_node *of_get_cpu_node(int cpu, unsigned int *thread)
>  }
>  EXPORT_SYMBOL(of_get_cpu_node);
>
> -/** Checks if the given "compat" string matches one of the strings in
> - * the device's "compatible" property
> +/**
> + * __of_device_is_compatible() - Check if the node matches given constraints
> + * @device: pointer to node
> + * @compat: required compatible string, NULL or "" for any match
> + * @type: required device_type value, NULL or "" for any match
> + * @name: required node name, NULL or "" for any match
> + *
> + * Checks if the given @compat, @type and @name strings match the
> + * properties of the given @device. A constraints can be skipped by
> + * passing NULL or an empty string as the constraint.
> + *
> + * Returns 0 for no match, and a positive integer on match. The return
> + * value is a relative score with larger values indicating better
> + * matches. The score is weighted for the most specific compatible value
> + * to get the highest score. Matching type is next, followed by matching
> + * name. Practically speaking, this results in the following priority
> + * order for matches:
> + *
> + * 1. specific compatible && type && name
> + * 2. specific compatible && type
> + * 3. specific compatible && name
> + * 4. specific compatible
> + * 5. general compatible && type && name
> + * 6. general compatible && type
> + * 7. general compatible && name
> + * 8. general compatible
> + * 9. type && name
> + * 10. type
> + * 11. name
>   */
>  static int __of_device_is_compatible(const struct device_node *device,
> -                                    const char *compat)
> +                                    const char *compat, const char *type, const char *name)
>  {
>         const char* cp;
> -       int cplen, l;
> +       int cplen, l, index, score = 0;
>
> -       cp = __of_get_property(device, "compatible", &cplen);
> -       if (cp == NULL)
> -               return 0;
> -       while (cplen > 0) {
> -               if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
> -                       return 1;
> -               l = strlen(cp) + 1;
> -               cp += l;
> -               cplen -= l;
> +       /* Compatible match has highest priority */
> +       if (compat && compat[0]) {
> +               cp = __of_get_property(device, "compatible", &cplen);
> +               if (!cp)
> +                       return 0;
> +               for (index = 0; cplen > 0; index++, cp += l, cplen -= l) {
> +                       l = strlen(cp) + 1;

This could use of_property_for_each_string.

> +                       if (of_compat_cmp(cp, compat, strlen(compat)) == 0) {
> +                               score = INT_MAX/2 - (index << 2);
> +                               break;
> +                       }
> +               }
> +               if (!score)
> +                       return 0;
>         }
>
> -       return 0;
> +       /* Matching type is better than matching name */
> +       if (type && type[0]) {
> +               if (!device->type || of_node_cmp(type, device->type))
> +                       return 0;
> +               score += 2;
> +       }
> +
> +       /* Matching name is a bit better than not */
> +       if (name && name[0]) {
> +               if (!device->name || of_node_cmp(name, device->name))
> +                       return 0;
> +               score++;
> +       }
> +
> +       return score;
>  }
>
>  /** Checks if the given "compat" string matches one of the strings in
> @@ -375,7 +421,7 @@ int of_device_is_compatible(const struct device_node *device,
>         int res;
>
>         raw_spin_lock_irqsave(&devtree_lock, flags);
> -       res = __of_device_is_compatible(device, compat);
> +       res = __of_device_is_compatible(device, compat, NULL, NULL);
>         raw_spin_unlock_irqrestore(&devtree_lock, flags);
>         return res;
>  }
> @@ -681,10 +727,7 @@ struct device_node *of_find_compatible_node(struct device_node *from,
>         raw_spin_lock_irqsave(&devtree_lock, flags);
>         np = from ? from->allnext : of_allnodes;
>         for (; np; np = np->allnext) {
> -               if (type
> -                   && !(np->type && (of_node_cmp(np->type, type) == 0)))
> -                       continue;
> -               if (__of_device_is_compatible(np, compatible) &&
> +               if (__of_device_is_compatible(np, compatible, type, NULL) &&
>                     of_node_get(np))
>                         break;
>         }
> @@ -734,25 +777,22 @@ static
>  const struct of_device_id *__of_match_node(const struct of_device_id *matches,
>                                            const struct device_node *node)
>  {
> +       const struct of_device_id *best_match = NULL;
> +       int score, best_score = 0;
> +
>         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++;
> +       for (; matches->name[0] || matches->type[0] || matches->compatible[0]; matches++) {
> +               score = __of_device_is_compatible(node, matches->compatible,
> +                                                 matches->type, matches->name);
> +               if (score > best_score) {
> +                       best_match = matches;
> +                       best_score = score;
> +               }
>         }
> -       return NULL;
> +
> +       return best_match;
>  }
>
>  /**
> --
> 1.8.3.2
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux