From patchwork Sun May 18 09:27:30 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Grant Likely X-Patchwork-Id: 30354 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ob0-f199.google.com (mail-ob0-f199.google.com [209.85.214.199]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 3810E20491 for ; Sun, 18 May 2014 09:27:58 +0000 (UTC) Received: by mail-ob0-f199.google.com with SMTP id wm4sf23691713obc.2 for ; Sun, 18 May 2014 02:27:57 -0700 (PDT) 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=L1Gp8zu1VhDn0A2E4GeoErXaL+QRXlnFSO/uQ20KpNA=; b=CcajTSf34QRBnjReZ1iO/j8NpwHTHDM8PJHwLJIe7PP5+ZwKgmsglilCM367qRjNG8 gfznbxXlB1fRnLB1ysVS31OriWpRVdth/9vkf3GviiEAU5Ezii5yP59BYRt/cXuHYTNM NdiB0HvGp5uk6my2yf2+zlY61D4zqehLjtnFgS9kx1QMuu9zMDbCvmuD77wxROq69cvE IM0P01xU72CwQKrf7WU9t1Zw7wk0HgOEVq6yTzVZvHJxLrbR5/HHwzILrK8Ng7E59kog l+teDfiB30pj6Cq3/sENby1Co0MxvYgvAK2WyzbaaGgayAbwtqp0HlnyLLdv3/eqWu60 jUbA== X-Gm-Message-State: ALoCoQnjJzxtDiX+kXIPX7BloIwlGruMqhzL2fZ4XPDxf8W8mYcrxCTTEsMQ8uIFIZHau0xb35Vx X-Received: by 10.50.141.161 with SMTP id rp1mr4157019igb.2.1400405277352; Sun, 18 May 2014 02:27:57 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.98.52 with SMTP id n49ls1294849qge.29.gmail; Sun, 18 May 2014 02:27:57 -0700 (PDT) X-Received: by 10.221.26.10 with SMTP id rk10mr7726444vcb.0.1400405277249; Sun, 18 May 2014 02:27:57 -0700 (PDT) Received: from mail-vc0-f182.google.com (mail-vc0-f182.google.com [209.85.220.182]) by mx.google.com with ESMTPS id oh5si3053115vcb.140.2014.05.18.02.27.57 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Sun, 18 May 2014 02:27:57 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.182 as permitted sender) client-ip=209.85.220.182; Received: by mail-vc0-f182.google.com with SMTP id la4so8457452vcb.27 for ; Sun, 18 May 2014 02:27:57 -0700 (PDT) X-Received: by 10.221.34.7 with SMTP id sq7mr7817295vcb.5.1400405276942; Sun, 18 May 2014 02:27:56 -0700 (PDT) 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.221.72 with SMTP id ib8csp171997vcb; Sun, 18 May 2014 02:27:56 -0700 (PDT) X-Received: by 10.66.232.166 with SMTP id tp6mr34344578pac.127.1400405275887; Sun, 18 May 2014 02:27:55 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id pt4si15270299pac.241.2014.05.18.02.27.54; Sun, 18 May 2014 02:27:55 -0700 (PDT) Received-SPF: none (google.com: linux-kernel-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750877AbaERJ1m (ORCPT + 27 others); Sun, 18 May 2014 05:27:42 -0400 Received: from mail-we0-f176.google.com ([74.125.82.176]:43884 "EHLO mail-we0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750712AbaERJ1j (ORCPT ); Sun, 18 May 2014 05:27:39 -0400 Received: by mail-we0-f176.google.com with SMTP id q59so4320848wes.35 for ; Sun, 18 May 2014 02:27:38 -0700 (PDT) X-Received: by 10.194.189.116 with SMTP id gh20mr23297770wjc.41.1400405258056; Sun, 18 May 2014 02:27:38 -0700 (PDT) Received: from trevor.secretlab.ca ([37.205.61.203]) by mx.google.com with ESMTPSA id ek4sm8422127wib.9.2014.05.18.02.27.35 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 18 May 2014 02:27:36 -0700 (PDT) Received: by trevor.secretlab.ca (Postfix, from userid 1000) id 109AAC40B8A; Sun, 18 May 2014 10:27:30 +0100 (BST) From: Grant Likely Subject: Re: [PATCH 2/3] of: Make of_find_node_by_path() handle /aliases To: frowand.list@gmail.com Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, David Daney , Pantelis Antoniou In-Reply-To: <20140516105444.C1DB7C42277@trevor.secretlab.ca> References: <1399993115-21552-1-git-send-email-grant.likely@linaro.org> <1399993115-21552-3-git-send-email-grant.likely@linaro.org> <53757D25.1060005@gmail.com> <20140516105444.C1DB7C42277@trevor.secretlab.ca> Date: Sun, 18 May 2014 10:27:30 +0100 Message-Id: <20140518092730.109AAC40B8A@trevor.secretlab.ca> Sender: linux-kernel-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-kernel@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=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.182 as permitted sender) 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 Fri, 16 May 2014 11:54:44 +0100, Grant Likely wrote: > On Thu, 15 May 2014 19:51:17 -0700, Frank Rowand wrote: > > On 5/13/2014 7:58 AM, Grant Likely wrote: > > > Make of_find_node_by_path() handle aliases as prefixes. To make this > > > work the name search is refactored to search by path component instead > > > of by full string. This should be a more efficient search, and it makes > > > it possible to start a search at a subnode of a tree. > > > > > > Signed-off-by: David Daney > > > Signed-off-by: Pantelis Antoniou > > > [grant.likely: Rework to not require allocating at runtime] > > > Acked-by: Rob Herring > > > Signed-off-by: Grant Likely > > > --- > > > drivers/of/base.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++---- > > > 1 file changed, 56 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/of/base.c b/drivers/of/base.c > > > index 6e240698353b..60089b9a3014 100644 > > > --- a/drivers/of/base.c > > > +++ b/drivers/of/base.c > > > @@ -771,9 +771,38 @@ struct device_node *of_get_child_by_name(const struct device_node *node, > > > } > > > EXPORT_SYMBOL(of_get_child_by_name); > > > > > > +static struct device_node *__of_find_node_by_path(struct device_node *parent, > > > + const char *path) > > > +{ > > > + struct device_node *child; > > > + int len = strchrnul(path, '/') - path; > > > + > > > + if (!len) > > > + return parent; > > > > (!len) is true if the the final character of the path passed into of_find_node_by_path() > > was "/". Strictly speaking, ->full_name will never end with "/", so the return value > > should be NULL, indicating that the match fails. > > Ah, good catch. I should add a test case for that. In my testing this looks okay. The while loop that calls into __of_find_node_by_path() looks like this: while (np && *path == '/') { path++; /* Increment past '/' delimiter */ np = __of_find_node_by_path(np, path); path = strchrnul(path, '/'); } If the path ends with a '/', then the loop will go around one more time. The pointer will be incremented to point at the null character and len will be null because strchrnul() will point at the last item. I've added a couple of test cases to make sure it works correctly: g. > > > > > > + > > > + for_each_child_of_node(parent, child) { > > > + const char *name = strrchr(child->full_name, '/'); > > > + if (WARN(!name, "malformed device_node %s\n", child->full_name)) > > > + continue; > > > + name++; > > > > Why go to the effort of finding the final component of child->full_name instead > > of just using child->name? > > Because child->name and the final component of child->full_name is not > the same thing. child->name has the unit address stripped off. It is > part of how OpenFirmware expects to be used. New drivers are never > supposed to be matching by node name, but I don't currently have a good > way to go through any driver depending on the OpenFirmware behaviour. > > Ideally I'd like to get rid of the difference. > > > > > > + if (strncmp(path, name, len) == 0 && (strlen(name) == len)) > > > + return child; > > > + } > > > + return NULL; > > > +} > > > + > > > /** > > > * of_find_node_by_path - Find a node matching a full OF path > > > * @path: The full path to match > > > + * @path: Either the full path to match, or if the path does not > > > > Delete the old @path description. > > Oops, thanks. > > > > > > > > + * start with '/', the name of a property of the /aliases > > > + * node (an alias). In the case of an alias, the node > > > + * matching the alias' value will be returned. > > > + * > > > + * Valid paths: > > > + * /foo/bar Full path > > > + * foo Valid alias > > > + * foo/bar Valid alias + relative path > > > * > > > * Returns a node pointer with refcount incremented, use > > > * of_node_put() on it when done. > > > @@ -781,13 +810,36 @@ EXPORT_SYMBOL(of_get_child_by_name); > > > struct device_node *of_find_node_by_path(const char *path) > > > { > > > struct device_node *np = of_allnodes; > > > + struct property *pp; > > > unsigned long flags; > > > > > > + /* The path could begin with an alias */ > > > + if (*path != '/') { > > > + char *p = strchrnul(path, '/'); > > > + int len = p - path; > > > + > > > + /* of_aliases must not be NULL */ > > > + if (!of_aliases) > > > + return NULL; > > > + > > > + np = NULL; > > > + for_each_property_of_node(of_aliases, pp) { > > > + if (strlen(pp->name) == len && !strncmp(pp->name, path, len)) { > > > + np = of_find_node_by_path(pp->value); > > > + break; > > > + } > > > + } > > > + if (!np) > > > + return NULL; > > > + path = p; > > > + } > > > + > > > + /* Step down the tree matching path components */ > > > raw_spin_lock_irqsave(&devtree_lock, flags); > > > - for (; np; np = np->allnext) { > > > - if (np->full_name && (of_node_cmp(np->full_name, path) == 0) > > > - && of_node_get(np)) > > > - break; > > > + while (np && *path == '/') { > > > + path++; /* Increment past '/' delimiter */ > > > + np = __of_find_node_by_path(np, path); > > > + path = strchrnul(path, '/'); > > > } > > > raw_spin_unlock_irqrestore(&devtree_lock, flags); > > > return np; > > > > > > --- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c index a9d00e8c17ea..10900b18fc06 100644 --- a/drivers/of/selftest.c +++ b/drivers/of/selftest.c @@ -40,6 +40,12 @@ static void __init of_selftest_find_node_by_name(void) "find /testcase-data failed\n"); of_node_put(np); + /* Test if trailing '/' works */ + np = of_find_node_by_path("/testcase-data/"); + selftest(np && !strcmp("/testcase-data", np->full_name), + "find /testcase-data/ failed\n"); + of_node_put(np); + np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); selftest(np && !strcmp("/testcase-data/phandle-tests/consumer-a", np->full_name), "find /testcase-data/phandle-tests/consumer-a failed\n"); @@ -50,6 +56,12 @@ static void __init of_selftest_find_node_by_name(void) "find testcase-alias failed\n"); of_node_put(np); + /* Test if trailing '/' works on aliases */ + np = of_find_node_by_path("testcase-alias/"); + selftest(np && !strcmp("/testcase-data", np->full_name), + "find testcase-alias/ failed\n"); + of_node_put(np); + np = of_find_node_by_path("testcase-alias/phandle-tests/consumer-a"); selftest(np && !strcmp("/testcase-data/phandle-tests/consumer-a", np->full_name), "find testcase-alias/phandle-tests/consumer-a failed\n");