From: Mark Rutland <mark.rutland@arm.com>
To: Stephen Warren <swarren@wwwdotorg.org>
Cc: Linus Walleij <linus.walleij@linaro.org>,
"rob.herring@calxeda.com" <rob.herring@calxeda.com>,
"grant.likely@linaro.org" <grant.likely@linaro.org>,
"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
Pawel Moll <Pawel.Moll@arm.com>,
Ian Campbell <ian.campbell@citrix.com>,
Kumar Gala <galak@kernel.crashing.org>,
Stephen Warren <swarren@nvidia.com>
Subject: Re: [PATCH V5 4/6] of: call __of_parse_phandle_with_args from of_parse_phandle
Date: Tue, 13 Aug 2013 17:29:28 +0100 [thread overview]
Message-ID: <20130813162928.GS27165@e106331-lin.cambridge.arm.com> (raw)
In-Reply-To: <520A569A.5040805@wwwdotorg.org>
On Tue, Aug 13, 2013 at 04:54:02PM +0100, Stephen Warren wrote:
> On 08/13/2013 03:08 AM, Mark Rutland wrote:
> > On Mon, Aug 12, 2013 at 06:36:30PM +0100, Stephen Warren wrote:
> >> From: Stephen Warren <swarren@nvidia.com>
> >>
> >> The simplest case of __of_parse_phandle_with_args() implements
> >> of_parse_phandle(), except that it doesn't return the node referenced by
> >> the phandle. Modify it to do so, and then rewrite of_parse_phandle() to
> >> call __of_parse_phandle_with_args() rather than open-coding the simple
> >> case.
> >
> > That commit message doesn't seem to match the patch (which doesn't
> > modify __of_parse_phandle_with_args).
> >
> > Rather, now that __of_parse_phandle_with_args can handle parsing with a
> > fixed number of argument cells, it's possible to write of_parse_phandle
> > in terms of it.
>
> True. I originally hadn't realized that __of_parse_phandle_with_args()
> does already return the node and so started to add that feature, then
> forgot to re-write the commit description. How about:
>
> -----
> of: call __of_parse_phandle_with_args from of_parse_phandle
>
> The simplest case of __of_parse_phandle_with_args() now implements the
> semantics of of_parse_phandle(). Rewrite of_parse_phandle() to call
> __of_parse_phandle_with_args() rather than open-coding the simple case.
> -----
Sounds good to me!
>
> > What's the overhead over the old of_parse_phandle? It looks like this is
> > going to do a lot of pointless work beyond what it already does --
> > parsing each prior entry in the list, and for each prior entry walking
> > the tree in of_find_node_by_phandle. Maybe we don't use long enough
> > phandle lists anywhere for that to be noticeable.
>
> I think the overhead is pretty minimal. The main difference is that the
> new code will loop over the property cell by cell rather than directly
> jump into the required index. That's not likely to be much work for
> typical properties. In particular, no extra DT property lookups are
> performed, since of_parse_phandle() passes in cells_name=NULL,
> cell_count=0, so the cells_name property is not looked up.
I thought even with your patch we still call of_find_node_by_phandle on
each (phandle) cell as we go over the property, before we hit the check
for cells_name?
Given that of_find_node_by_phandle does a pretty naive linear search of
the of_allnodes list, that could get significant, especially if all the
elements referred to in the property are near the end of the of_allnodes
list.
>
> Besides, Grant told me to do this change:-)
>
A Likely story... :)
Thanks,
Mark.
next prev parent reply other threads:[~2013-08-13 16:30 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-12 17:36 [PATCH V5 1/6] of: move documentation of of_parse_phandle_with_args Stephen Warren
2013-08-12 17:36 ` [PATCH V5 2/6] of: move of_parse_phandle() Stephen Warren
2013-08-14 16:05 ` Mark Rutland
2013-08-12 17:36 ` [PATCH V5 3/6] of: introduce of_parse_phandle_with_fixed_args Stephen Warren
2013-08-12 17:36 ` [PATCH V5 4/6] of: call __of_parse_phandle_with_args from of_parse_phandle Stephen Warren
2013-08-13 9:08 ` Mark Rutland
2013-08-13 15:54 ` Stephen Warren
2013-08-13 16:29 ` Mark Rutland [this message]
2013-08-13 18:12 ` Stephen Warren
2013-08-14 16:04 ` Mark Rutland
2013-08-12 17:36 ` [PATCH V5 5/6] gpio: clean up gpio-ranges documentation Stephen Warren
2013-08-12 17:36 ` [PATCH V5 6/6] gpio: implement gpio-ranges binding document fix Stephen Warren
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=20130813162928.GS27165@e106331-lin.cambridge.arm.com \
--to=mark.rutland@arm.com \
--cc=Pawel.Moll@arm.com \
--cc=devicetree@vger.kernel.org \
--cc=galak@kernel.crashing.org \
--cc=grant.likely@linaro.org \
--cc=ian.campbell@citrix.com \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=rob.herring@calxeda.com \
--cc=swarren@nvidia.com \
--cc=swarren@wwwdotorg.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.