On Thu, Jun 26, 2008 at 05:48:01PM -1000, Mitch Bradley wrote:
>
>
> David Gibson wrote:
>> On Thu, Jun 26, 2008 at 05:28:42PM -1000, Mitch Bradley wrote:
>>
>>> David Gibson wrote:
>>>
>>>> On Thu, Jun 26, 2008 at 01:50:40PM -1000, Mitch Bradley wrote:
>>>>
>> [snip]
>>
>>>>> + const u_int32_t *propval;
>>>>> + u_int32_t addrcells = 0, sizecells = 0;
>>>>> int len;
>>>>>
>>>>> - reg = of_get_property(pp, "reg", &len);
>>>>> - if (!reg || (len != 2 * sizeof(u32))) {
>>>>> + /*
>>>>> + * Determine the layout of a "reg" entry based on the
>>>>> parent
>>>>> + * node's properties, if it hasn't been done already.
>>>>> + */
>>>>> +
>>>>> + if (addrcells == 0)
>>>>>
>>>> Redundant 'if'; you've just initialized this variable to zero.
>>>>
>>> The intention is that the body of the "if" should only be executed
>>> once during the loop, since the parent node is the same for all
>>> children.
>>>
>>
>> But the initialization is within the loop body as well, so this won't
>> do it. Just factor the code getting addr and size cells right out of
>> the loop, instead.
>>
>>
>
> Hmmm. Perhaps it's better to move the declaration of the variables out of
> the loop instead.
>
> Moving the of_n_*_cells() calls outside the loop requires redundant calls
> to of_get_child() and of_node_put(), because of_n_*_cells() implicitly
> reach up to the parent node. That is almost certainly more expensive
> than the "if".
This is not exactly a hot path; clarity and (source) code size are
more important than expense. But going down to the child then back up
is ugly too. Maybe you should just directly pull #address-cells and
#size-cells from the parent node. In fact, of_n_*_cells() are wrong
by the OF spec, since they assume the values are inherited, which is
not how it's supposed to work.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
