From patchwork Wed Feb 19 12:48:13 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Grant Likely X-Patchwork-Id: 24954 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ie0-f198.google.com (mail-ie0-f198.google.com [209.85.223.198]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id EC21120143 for ; Wed, 19 Feb 2014 12:48:43 +0000 (UTC) Received: by mail-ie0-f198.google.com with SMTP id at1sf1236410iec.9 for ; Wed, 19 Feb 2014 04:48:43 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:delivered-to:from:subject:to:cc :in-reply-to:references:date:message-id:sender:precedence:list-id :x-original-sender:x-original-authentication-results:mailing-list :list-post:list-help:list-archive:list-unsubscribe; bh=H/mcaYYJ3eb3P4qOTn+3OOY3rElGYEJMewvoMmLtWVg=; b=X9Nax2Am2JIdXvLOsPMVebZZOdBB4b3UOyQoA/LtBqWSzzUNyX0JiWwRKN2ak1ZDsa qyCHKvbESx/dbVE4NVbLlKLDk7j0zjObF6GD+q7c1H1dTWQCfrBGrxqzjV1o250RLl8O nK4axd+U+1nDvPdiAfMJcY4PZTP8ypMN63Gib+rI3ZVpP6G8Fbt5tcIf1O1XfsatFmdu 4cVOWCC2VWOGDkFKjXNJEDy4hvOA1KZNnW+WWwSy0MBtJt5CQyBj+lT/IB4W4WWEHhTW QYoPtvexIUwykEoKV7RvG1qa+b4zzMdbfUUU+KAxbef0bv9fslpBLuZSOYA4zMFPGMdy BR9Q== X-Gm-Message-State: ALoCoQnSW10pDzOW4E3M4e9iVn60eteUwQkZtJAGxP62Dp/fkJN3VLFdlfryyMuglfNR/7a2NgQF X-Received: by 10.50.164.198 with SMTP id ys6mr533532igb.2.1392814123247; Wed, 19 Feb 2014 04:48:43 -0800 (PST) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.109.101 with SMTP id k92ls66621qgf.78.gmail; Wed, 19 Feb 2014 04:48:43 -0800 (PST) X-Received: by 10.52.61.133 with SMTP id p5mr21155186vdr.4.1392814123122; Wed, 19 Feb 2014 04:48:43 -0800 (PST) Received: from mail-ve0-f181.google.com (mail-ve0-f181.google.com [209.85.128.181]) by mx.google.com with ESMTPS id l2si34565vcf.43.2014.02.19.04.48.43 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 19 Feb 2014 04:48:43 -0800 (PST) Received-SPF: neutral (google.com: 209.85.128.181 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.128.181; Received: by mail-ve0-f181.google.com with SMTP id jw12so309978veb.12 for ; Wed, 19 Feb 2014 04:48:43 -0800 (PST) X-Received: by 10.52.106.166 with SMTP id gv6mr272517vdb.86.1392814123027; Wed, 19 Feb 2014 04:48:43 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.220.174.196 with SMTP id u4csp294649vcz; Wed, 19 Feb 2014 04:48:42 -0800 (PST) X-Received: by 10.66.164.70 with SMTP id yo6mr2077686pab.85.1392814122052; Wed, 19 Feb 2014 04:48:42 -0800 (PST) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id tq3si120412pab.38.2014.02.19.04.48.41; Wed, 19 Feb 2014 04:48:41 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of devicetree-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753505AbaBSMse (ORCPT + 9 others); Wed, 19 Feb 2014 07:48:34 -0500 Received: from mail-we0-f182.google.com ([74.125.82.182]:47597 "EHLO mail-we0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752931AbaBSMsV (ORCPT ); Wed, 19 Feb 2014 07:48:21 -0500 Received: by mail-we0-f182.google.com with SMTP id u57so286972wes.27 for ; Wed, 19 Feb 2014 04:48:20 -0800 (PST) X-Received: by 10.180.7.130 with SMTP id j2mr1254283wia.25.1392814100351; Wed, 19 Feb 2014 04:48:20 -0800 (PST) Received: from trevor.secretlab.ca (host109-153-23-84.range109-153.btcentralplus.com. [109.153.23.84]) by mx.google.com with ESMTPSA id t6sm51605171wix.4.2014.02.19.04.48.18 for (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 19 Feb 2014 04:48:19 -0800 (PST) Received: by trevor.secretlab.ca (Postfix, from userid 1000) id 8FD66C4088D; Wed, 19 Feb 2014 12:48:13 +0000 (GMT) From: Grant Likely Subject: Re: [PATCH v3 2/4] of: reimplement the matching method for __of_match_node() To: Kevin Hao , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Kevin Hao , Rob Herring , Sebastian Hesselbarth In-Reply-To: <1392797745-7561-1-git-send-email-haokexin@gmail.com> References: <20140219075812.GE14031@pek-khao-d1.corp.ad.wrs.com> < 1392797745-7561-1-git-send-email-haokexin@gmail.com> Date: Wed, 19 Feb 2014 12:48:13 +0000 Message-Id: <20140219124813.8FD66C4088D@trevor.secretlab.ca> Sender: devicetree-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: devicetree@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: grant.likely@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.128.181 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , On Wed, 19 Feb 2014 16:15:45 +0800, Kevin Hao wrote: > 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 > > This is based on some pseudo-codes provided by Grant Likely. > > Signed-off-by: Kevin Hao > [grant.likely: Changed multiplier to 4 which makes more sense] > Signed-off-by: Grant Likely > --- > 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: > name2 { } > > struct of_device_id matches[] = { > {.name = "name2", }, > {.name = "name2", .type = "type1", }, > {} > }; > > drivers/of/base.c | 96 ++++++++++++++++++++++++++++++++++++++++++++----------- > 1 file changed, 77 insertions(+), 19 deletions(-) > > diff --git a/drivers/of/base.c b/drivers/of/base.c > index ba195fbce4c6..8f79f006d86f 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -342,21 +342,28 @@ 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 > +/* > + * Compare with the __of_device_is_compatible, this will return a score for the > + * matching strings. The smaller value indicates the match for the more specific > + * compatible string. > */ > -static int __of_device_is_compatible(const struct device_node *device, > - const char *compat) > +static int __of_device_is_compatible_score(const struct device_node *device, > + const char *compat, int *pscore) > { > const char* cp; > int cplen, l; > + int 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) > + score++; > + if (of_compat_cmp(cp, compat, strlen(compat)) == 0) { > + if (pscore) > + *pscore = score; > return 1; > + } > l = strlen(cp) + 1; > cp += l; > cplen -= l; > @@ -368,6 +375,15 @@ static int __of_device_is_compatible(const struct device_node *device, > /** Checks if the given "compat" string matches one of the strings in > * the device's "compatible" property > */ > +static int __of_device_is_compatible(const struct device_node *device, > + const char *compat) > +{ > + return __of_device_is_compatible_score(device, compat, NULL); > +} > + > +/** Checks if the given "compat" string matches one of the strings in > + * the device's "compatible" property > + */ > int of_device_is_compatible(const struct device_node *device, > const char *compat) > { > @@ -734,25 +750,55 @@ 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 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; > + int score = 0; > + > + /* > + * Matching compatible is better than matching type and name, > + * and the specific compatible is better than the general. > + */ > + if (matches->compatible[0]) { > + if (__of_device_is_compatible_score(node, > + matches->compatible, &score)) > + score = INT_MAX - 4 * score; > + else > + score = INT_MIN; > + } > + > + /* > + * Matching type is better than matching name, but matching > + * both is even better than that. > + */ > + if (matches->type[0]) { > + if (node->type && !strcmp(matches->type, node->type)) > + score += 2; > + else > + score = INT_MIN; > + } > + > + /* Matching name is a bit better than not */ > + if (matches->name[0]) { > + if (node->name && !strcmp(matches->name, node->name)) > + score++; > + else > + score = INT_MIN; > + } All that mucking about with setting the score to INT_MIN is pretty ugly. I've reworked the patch to 'continue;' whenever one of the above matches fail. That completely eliminates any possibility of accepting an entry that has a failed match. Here's my diff. I'll merge this change into the patch before committing. All of the test cases still pass with my rework. --- -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/of/base.c b/drivers/of/base.c index 8f79f006d86f..fcb65d27d071 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -756,7 +756,7 @@ const struct of_device_id *__of_match_node(const struct of_device_id *matches, if (!matches) return NULL; - while (matches->name[0] || matches->type[0] || matches->compatible[0]) { + for (; matches->name[0] || matches->type[0] || matches->compatible[0]; matches++) { int score = 0; /* @@ -764,11 +764,10 @@ const struct of_device_id *__of_match_node(const struct of_device_id *matches, * and the specific compatible is better than the general. */ if (matches->compatible[0]) { - if (__of_device_is_compatible_score(node, + if (!__of_device_is_compatible_score(node, matches->compatible, &score)) - score = INT_MAX - 4 * score; - else - score = INT_MIN; + continue; + score = INT_MAX - 4 * score; } /* @@ -776,26 +775,22 @@ const struct of_device_id *__of_match_node(const struct of_device_id *matches, * both is even better than that. */ if (matches->type[0]) { - if (node->type && !strcmp(matches->type, node->type)) - score += 2; - else - score = INT_MIN; + if (!node->type || strcmp(matches->type, node->type)) + continue; + score += 2; } /* Matching name is a bit better than not */ if (matches->name[0]) { - if (node->name && !strcmp(matches->name, node->name)) - score++; - else - score = INT_MIN; + if (!node->name || strcmp(matches->name, node->name)) + continue; + score++; } if (score > best_score) { best_match = matches; best_score = score; } - - matches++; } return best_match;