From: ijc@debian.org (Ian Campbell)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] of: support passing console options with stdout-path
Date: Tue, 25 Nov 2014 15:24:31 +0000 [thread overview]
Message-ID: <1416929071.16769.1.camel@debian.org> (raw)
In-Reply-To: <CACxGe6v2i3iJGjK9s6HSBxV6bUwVrAtGYj=FAOb0AFEx3KVYEg@mail.gmail.com>
On Tue, 2014-11-25 at 15:20 +0000, Grant Likely wrote:
> On Tue, Nov 25, 2014 at 3:15 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Tue, Nov 25, 2014 at 02:58:54PM +0000, Grant Likely wrote:
> >> > + len = strchrnul(path, ':') - path;
> >> > +
> >> > 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);
> >>
> >> Can you add a testcase to drivers/of/unittests.c for this new path
> >> parsing? Then we know it's correct!
> >
> > Will do.
> >
> >> > @@ -1830,8 +1835,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
> >> > name = of_get_property(of_chosen, "linux,stdout-path", NULL);
> >> > if (IS_ENABLED(CONFIG_PPC) && !name)
> >> > name = of_get_property(of_aliases, "stdout", NULL);
> >> > - if (name)
> >> > + if (name) {
> >> > of_stdout = of_find_node_by_path(name);
> >> > + of_stdout_options = strchr(name, ':');
> >> > + if (of_stdout_options != NULL)
> >> > + of_stdout_options++;
> >> > + }
> >>
> >> Not so good. The ':' should actually be a generic part of
> >> of_find_node_by_path(), it doesn't have to be limited to stdout parsing.
> >> There are other places that would use it. I would add a second,
> >> optional, argument to of_find_node_by_path() that will update a pointer
> >> to the arguments after the ':'.
> >
> > So, I agree this would be nicer...
> >
> > However, I'm counting 157 callers of this function outside of
> > drivers/of and 43 inside. Any chance you'd let me get away with a
> > separate of_find_extra_options_by_path()?
>
> I'd rather not. It is a trivial change and can be pretty much done
> mechanically. Something like (I think; I always have to look up the
> sed syntax):
>
> sed s/of_find_node_by_path\((.*)\)/of_find_node_by_path\(\1, NULL\)
Coccinelle rules for this sort of transformation...
Ian.
WARNING: multiple messages have this Message-ID (diff)
From: Ian Campbell <ijc@debian.org>
To: Grant Likely <grant.likely@linaro.org>
Cc: Leif Lindholm <leif.lindholm@linaro.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Rob Herring <robh+dt@kernel.org>,
Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Subject: Re: [PATCH] of: support passing console options with stdout-path
Date: Tue, 25 Nov 2014 15:24:31 +0000 [thread overview]
Message-ID: <1416929071.16769.1.camel@debian.org> (raw)
In-Reply-To: <CACxGe6v2i3iJGjK9s6HSBxV6bUwVrAtGYj=FAOb0AFEx3KVYEg@mail.gmail.com>
On Tue, 2014-11-25 at 15:20 +0000, Grant Likely wrote:
> On Tue, Nov 25, 2014 at 3:15 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Tue, Nov 25, 2014 at 02:58:54PM +0000, Grant Likely wrote:
> >> > + len = strchrnul(path, ':') - path;
> >> > +
> >> > 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);
> >>
> >> Can you add a testcase to drivers/of/unittests.c for this new path
> >> parsing? Then we know it's correct!
> >
> > Will do.
> >
> >> > @@ -1830,8 +1835,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
> >> > name = of_get_property(of_chosen, "linux,stdout-path", NULL);
> >> > if (IS_ENABLED(CONFIG_PPC) && !name)
> >> > name = of_get_property(of_aliases, "stdout", NULL);
> >> > - if (name)
> >> > + if (name) {
> >> > of_stdout = of_find_node_by_path(name);
> >> > + of_stdout_options = strchr(name, ':');
> >> > + if (of_stdout_options != NULL)
> >> > + of_stdout_options++;
> >> > + }
> >>
> >> Not so good. The ':' should actually be a generic part of
> >> of_find_node_by_path(), it doesn't have to be limited to stdout parsing.
> >> There are other places that would use it. I would add a second,
> >> optional, argument to of_find_node_by_path() that will update a pointer
> >> to the arguments after the ':'.
> >
> > So, I agree this would be nicer...
> >
> > However, I'm counting 157 callers of this function outside of
> > drivers/of and 43 inside. Any chance you'd let me get away with a
> > separate of_find_extra_options_by_path()?
>
> I'd rather not. It is a trivial change and can be pretty much done
> mechanically. Something like (I think; I always have to look up the
> sed syntax):
>
> sed s/of_find_node_by_path\((.*)\)/of_find_node_by_path\(\1, NULL\)
Coccinelle rules for this sort of transformation...
Ian.
next prev parent reply other threads:[~2014-11-25 15:24 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-24 22:23 [PATCH] of: support passing console options with stdout-path Leif Lindholm
2014-11-24 22:23 ` Leif Lindholm
2014-11-24 23:00 ` Andrew Lunn
2014-11-24 23:00 ` Andrew Lunn
2014-11-24 23:00 ` Andrew Lunn
2014-11-25 0:12 ` Leif Lindholm
2014-11-25 0:12 ` Leif Lindholm
2014-11-25 0:12 ` Leif Lindholm
2014-11-25 6:49 ` Sascha Hauer
2014-11-25 6:49 ` Sascha Hauer
2014-11-25 6:49 ` Sascha Hauer
2014-11-25 14:44 ` Grant Likely
2014-11-25 14:44 ` Grant Likely
2014-11-25 14:44 ` Grant Likely
2014-11-25 16:21 ` Andrew Lunn
2014-11-25 16:21 ` Andrew Lunn
2014-11-25 16:21 ` Andrew Lunn
2014-11-25 10:35 ` Mark Rutland
2014-11-25 10:35 ` Mark Rutland
2014-11-25 10:35 ` Mark Rutland
2014-11-25 11:17 ` Leif Lindholm
2014-11-25 11:17 ` Leif Lindholm
2014-11-25 12:07 ` Ian Campbell
2014-11-25 12:07 ` Ian Campbell
2014-11-25 12:07 ` Ian Campbell
2014-11-25 14:35 ` Leif Lindholm
2014-11-25 14:35 ` Leif Lindholm
2014-11-25 14:35 ` Leif Lindholm
2014-11-25 14:55 ` Grant Likely
2014-11-25 14:55 ` Grant Likely
2014-11-25 12:54 ` Sascha Hauer
2014-11-25 12:54 ` Sascha Hauer
2014-11-25 12:54 ` Sascha Hauer
2014-11-25 14:58 ` Grant Likely
2014-11-25 14:58 ` Grant Likely
2014-11-25 14:58 ` Grant Likely
2014-11-25 15:15 ` Leif Lindholm
2014-11-25 15:15 ` Leif Lindholm
2014-11-25 15:20 ` Grant Likely
2014-11-25 15:20 ` Grant Likely
2014-11-25 15:24 ` Ian Campbell [this message]
2014-11-25 15:24 ` Ian Campbell
2014-11-25 15:39 ` Grant Likely
2014-11-25 15:39 ` Grant Likely
2014-11-25 15:39 ` Grant Likely
2014-11-25 15:41 ` Ian Campbell
2014-11-25 15:41 ` Ian Campbell
2014-11-25 15:41 ` Ian Campbell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1416929071.16769.1.camel@debian.org \
--to=ijc@debian.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.