On 08/13/2013 10:29 AM, Mark Rutland wrote:

On Tue, Aug 13, 2013 at 04:54:02PM +0100, Stephen Warren wrote:

quoted

On 08/13/2013 03:08 AM, Mark Rutland wrote:

quoted

On Mon, Aug 12, 2013 at 06:36:30PM +0100, Stephen Warren wrote:

quoted

From: Stephen Warren <redacted>

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.

...

quoted

quoted

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.

Oh yes, that is true.

I suppose it'd be possible to make the call to of_find_node_by_phandle()
conditional; it'd be needed:

if (cells_name /* Need to get property from it */ ||
	cur_index == index /* Need to return it */)

Do you think it's worth making that change?