From patchwork Fri Nov 28 11:34:28 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Leif Lindholm X-Patchwork-Id: 41696 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wg0-f71.google.com (mail-wg0-f71.google.com [74.125.82.71]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 7343B244C2 for ; Fri, 28 Nov 2014 11:34:38 +0000 (UTC) Received: by mail-wg0-f71.google.com with SMTP id l18sf3994376wgh.2 for ; Fri, 28 Nov 2014 03:34:37 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:date:from:to:cc:subject:message-id :references:mime-version:content-type:content-disposition :in-reply-to:user-agent:sender:precedence:list-id:x-original-sender :x-original-authentication-results:mailing-list:list-post:list-help :list-archive:list-unsubscribe; bh=eulNCpQ4+FTHeGVovBCoknW+WTWnjRt8VM1BhIBauqY=; b=CR0XeKG99QeWsC3u1Br6HkyHYK7yMKg40eU+60AkMjHm0YbNaWtR8UJsuC8H/lK9NE qLLUgqfzhALOfG7zqCygVk+5bE6/OCRFoZNtA7cZ/qH+ZinIdUGykpaezRH9BabWH7D4 lbQEte7QiSle03fj6CLH7XaaRpeXkr+CUKk8wuZAgOTDVdpJeQNwIZUIdRkCjtjhpT11 8wpFybcDwoqbKgcU0v7krAqrNfLcCcO9mBfdWIB5ToHrBuS7/LURpvI+sNeLLB7iL8e1 FNOOLG634Cb9O3kS9DPqHYaRZHsbh5KjgVXH6W+5cJeltWuKegSICoHXIRhaMCaMxfqD HS/w== X-Gm-Message-State: ALoCoQlvu/Om/MA382tVz4C4pMEdSrUlbLFCOu1fAH4QJIR/X+1G3V0/+6mSmBE3fTGyPCJX79MF X-Received: by 10.152.43.50 with SMTP id t18mr309129lal.8.1417174477734; Fri, 28 Nov 2014 03:34:37 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.87.176 with SMTP id az16ls746700lab.87.gmail; Fri, 28 Nov 2014 03:34:37 -0800 (PST) X-Received: by 10.152.7.71 with SMTP id h7mr42365914laa.68.1417174477431; Fri, 28 Nov 2014 03:34:37 -0800 (PST) Received: from mail-lb0-f176.google.com (mail-lb0-f176.google.com. [209.85.217.176]) by mx.google.com with ESMTPS id m2si9771702laf.6.2014.11.28.03.34.37 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 28 Nov 2014 03:34:37 -0800 (PST) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.176 as permitted sender) client-ip=209.85.217.176; Received: by mail-lb0-f176.google.com with SMTP id p9so5370487lbv.21 for ; Fri, 28 Nov 2014 03:34:37 -0800 (PST) X-Received: by 10.152.43.103 with SMTP id v7mr19991863lal.29.1417174477331; Fri, 28 Nov 2014 03:34:37 -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.112.184.201 with SMTP id ew9csp74495lbc; Fri, 28 Nov 2014 03:34:36 -0800 (PST) X-Received: by 10.68.57.144 with SMTP id i16mr71663556pbq.86.1417174475777; Fri, 28 Nov 2014 03:34:35 -0800 (PST) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id id7si15714049pbc.167.2014.11.28.03.34.35 for ; Fri, 28 Nov 2014 03:34:35 -0800 (PST) Received-SPF: none (google.com: devicetree-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 S1751566AbaK1Lee (ORCPT + 4 others); Fri, 28 Nov 2014 06:34:34 -0500 Received: from mail-wi0-f181.google.com ([209.85.212.181]:55338 "EHLO mail-wi0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751046AbaK1Led (ORCPT ); Fri, 28 Nov 2014 06:34:33 -0500 Received: by mail-wi0-f181.google.com with SMTP id r20so10954616wiv.8 for ; Fri, 28 Nov 2014 03:34:32 -0800 (PST) X-Received: by 10.180.211.79 with SMTP id na15mr53329302wic.36.1417174472022; Fri, 28 Nov 2014 03:34:32 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by mx.google.com with ESMTPSA id wb9sm1313098wic.20.2014.11.28.03.34.30 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 28 Nov 2014 03:34:31 -0800 (PST) Date: Fri, 28 Nov 2014 11:34:28 +0000 From: Leif Lindholm To: Grant Likely Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, mark.rutland@arm.com, robh+dt@kernel.org, plagnioj@jcrosoft.com, ijc@debian.org, andrew@lunn.ch, s.hauer@pengutronix.de Subject: Re: [PATCH v3 2/3] of: add optional options parameter to of_find_node_by_path() Message-ID: <20141128113428.GM2361@bivouac.eciton.net> References: <1417110967-16284-1-git-send-email-leif.lindholm@linaro.org> <1417110967-16284-3-git-send-email-leif.lindholm@linaro.org> <20141128004403.08D90C40A7F@trevor.secretlab.ca> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20141128004403.08D90C40A7F@trevor.secretlab.ca> User-Agent: Mutt/1.5.21 (2010-09-15) 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: leif.lindholm@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.217.176 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, Nov 28, 2014 at 12:44:03AM +0000, Grant Likely wrote: > On Thu, 27 Nov 2014 17:56:06 +0000 > , Leif Lindholm > wrote: > > Update of_find_node_by_path(): > > 1) Rename function to of_find_node_opts_by_path(), adding an optional > > pointer argument. Provide a static inline wrapper version of > > of_find_node_by_path() which calls the new function with NULL as > > the optional argument. > > 2) Ignore any part of the path beyond and including the ':' separator. > > 3) Set the new provided pointer argument to the beginning of the string > > following the ':' separator. > > 4: Add tests. > > > > Signed-off-by: Leif Lindholm > > --- > > drivers/of/base.c | 21 +++++++++++++++++---- > > drivers/of/selftest.c | 12 ++++++++++++ > > include/linux/of.h | 14 +++++++++++++- > > 3 files changed, 42 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/of/base.c b/drivers/of/base.c > > index 3823edf..7f0e5f7 100644 > > --- a/drivers/of/base.c > > +++ b/drivers/of/base.c > > @@ -699,10 +699,15 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent, > > { > > struct device_node *child; > > int len = strchrnul(path, '/') - path; > > + int term; > > > > if (!len) > > return NULL; > > > > + term = strchrnul(path, ':') - path; > > + if (term < len) > > + len = term; > > + > > __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)) > > @@ -715,11 +720,14 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent, > > } > > > > /** > > - * of_find_node_by_path - Find a node matching a full OF path > > + * of_find_node_opts_by_path - Find a node matching a full OF path > > * @path: Either the full path to match, or if the path does not > > * 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. > > + * @opts: Address of a pointer into which to store the start of > > + * an options string appended to the end of the path with > > + * a ':' separator. > > * > > * Valid paths: > > * /foo/bar Full path > > @@ -729,19 +737,24 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent, > > * Returns a node pointer with refcount incremented, use > > * of_node_put() on it when done. > > */ > > -struct device_node *of_find_node_by_path(const char *path) > > +struct device_node *of_find_node_opts_by_path(const char *path, const char **opts) > > { > > struct device_node *np = NULL; > > struct property *pp; > > unsigned long flags; > > + char *separator; > > > > if (strcmp(path, "/") == 0) > > return of_node_get(of_allnodes); > > > > + separator = strchr(path, ':'); > > + if (separator && opts) > > + *opts = separator + 1; > > + > > What about when there are no opts? Do we require the caller to make sure > opts is NULL before calling the function (which sounds like a good > source of bugs) or do we clear it on successful return? > > I think if opts is passed in, but there are no options, then it should > set *opts = NULL. Yeah, oops. > There should be test cases for this also. Must set opts to NULL on > successful return, and (I think) should leave opts alone on an > unsuccessful search. I would actually argue for always nuking the opts - since that could (theoretically) prevent something working by accident in spite of error conditions. How about the below? / Leif >From 2e1a44e539967d96366d074ae158092900e0c822 Mon Sep 17 00:00:00 2001 From: Leif Lindholm Date: Thu, 27 Nov 2014 09:24:31 +0000 Subject: [PATCH] of: add optional options parameter to of_find_node_by_path() Update of_find_node_by_path(): 1) Rename function to of_find_node_opts_by_path(), adding an optional pointer argument. Provide a static inline wrapper version of of_find_node_by_path() which calls the new function with NULL as the optional argument. 2) Ignore any part of the path beyond and including the ':' separator. 3) Set the new provided pointer argument to the beginning of the string following the ':' separator. 4: Add tests. Signed-off-by: Leif Lindholm --- drivers/of/base.c | 21 +++++++++++++++++---- drivers/of/selftest.c | 17 +++++++++++++++++ include/linux/of.h | 14 +++++++++++++- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 3823edf..99f0fd9 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -699,10 +699,15 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent, { struct device_node *child; int len = strchrnul(path, '/') - path; + int term; if (!len) return NULL; + term = strchrnul(path, ':') - path; + if (term < len) + len = term; + __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)) @@ -715,11 +720,14 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent, } /** - * of_find_node_by_path - Find a node matching a full OF path + * of_find_node_opts_by_path - Find a node matching a full OF path * @path: Either the full path to match, or if the path does not * 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. + * @opts: Address of a pointer into which to store the start of + * an options string appended to the end of the path with + * a ':' separator. * * Valid paths: * /foo/bar Full path @@ -729,19 +737,24 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent, * Returns a node pointer with refcount incremented, use * of_node_put() on it when done. */ -struct device_node *of_find_node_by_path(const char *path) +struct device_node *of_find_node_opts_by_path(const char *path, const char **opts) { struct device_node *np = NULL; struct property *pp; unsigned long flags; + char *separator; if (strcmp(path, "/") == 0) return of_node_get(of_allnodes); + separator = strchr(path, ':'); + if (opts) + *opts = separator ? separator + 1 : NULL; + /* The path could begin with an alias */ if (*path != '/') { char *p = strchrnul(path, '/'); - int len = p - path; + int len = separator ? separator - path : p - path; /* of_aliases must not be NULL */ if (!of_aliases) @@ -770,7 +783,7 @@ struct device_node *of_find_node_by_path(const char *path) raw_spin_unlock_irqrestore(&devtree_lock, flags); return np; } -EXPORT_SYMBOL(of_find_node_by_path); +EXPORT_SYMBOL(of_find_node_opts_by_path); /** * of_find_node_by_name - Find a node by its "name" property diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c index e2d79af..721b2a4 100644 --- a/drivers/of/selftest.c +++ b/drivers/of/selftest.c @@ -43,6 +43,7 @@ static bool selftest_live_tree; static void __init of_selftest_find_node_by_name(void) { struct device_node *np; + const char *options; np = of_find_node_by_path("/testcase-data"); selftest(np && !strcmp("/testcase-data", np->full_name), @@ -83,6 +84,22 @@ static void __init of_selftest_find_node_by_name(void) np = of_find_node_by_path("testcase-alias/missing-path"); selftest(!np, "non-existent alias with relative path returned node %s\n", np->full_name); of_node_put(np); + + np = of_find_node_opts_by_path("/testcase-data:testoption", &options); + selftest(np && !strcmp("testoption", options), + "option path test failed\n"); + of_node_put(np); + + np = of_find_node_opts_by_path("testcase-alias:testaliasoption", + &options); + selftest(np && !strcmp("testaliasoption", options), + "option alias path test failed\n"); + of_node_put(np); + + options = "testoption"; + np = of_find_node_opts_by_path("testcase-alias", &options); + selftest(np && !options, "option clearing test failed\n"); + of_node_put(np); } static void __init of_selftest_dynamic(void) diff --git a/include/linux/of.h b/include/linux/of.h index 29f0adc..a36be70 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -228,7 +228,13 @@ extern struct device_node *of_find_matching_node_and_match( const struct of_device_id *matches, const struct of_device_id **match); -extern struct device_node *of_find_node_by_path(const char *path); +extern struct device_node *of_find_node_opts_by_path(const char *path, + const char **opts); +static inline struct device_node *of_find_node_by_path(const char *path) +{ + return of_find_node_opts_by_path(path, NULL); +} + extern struct device_node *of_find_node_by_phandle(phandle handle); extern struct device_node *of_get_parent(const struct device_node *node); extern struct device_node *of_get_next_parent(struct device_node *node); @@ -385,6 +391,12 @@ static inline struct device_node *of_find_node_by_path(const char *path) return NULL; } +static inline struct device_node *of_find_node_opts_by_path(const char *path, + const char **opts) +{ + return NULL; +} + static inline struct device_node *of_get_parent(const struct device_node *node) { return NULL;