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".

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Reply via email to