Re: [PATCH V5 4/6] of: call __of_parse_phandle_with_args from of_parse_phandle

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 08/13/2013 10:29 AM, Mark Rutland wrote:
> 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@xxxxxxxxxx>
>>>>
>>>> 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.
...
>>> 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?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux