[PATCH 3/5] of/device: Make of_get_next_child() check status properties
Hollis Blanchard
hollis at penguinppc.org
Thu Dec 16 05:35:06 EST 2010
On Wed, Dec 8, 2010 at 7:09 PM, David Gibson
<david at gibson.dropbear.id.au> wrote:
> On Thu, Dec 09, 2010 at 12:33:22PM +1100, Michael Ellerman wrote:
>> On Wed, 2010-12-08 at 15:01 -0600, Scott Wood wrote:
>> > On Wed, 8 Dec 2010 11:29:44 -0800
>> > Deepak Saxena <deepak_saxena at mentor.com> wrote:
>> >
>> > > We only return the next child if the device is available.
>> > >
>> > > Signed-off-by: Hollis Blanchard <hollis_blanchard at mentor.com>
>> > > Signed-off-by: Deepak Saxena <deepak_saxena at mentor.com>
>> > > ---
>> > > drivers/of/base.c | 4 +++-
>> > > 1 files changed, 3 insertions(+), 1 deletions(-)
>> > >
>> > > diff --git a/drivers/of/base.c b/drivers/of/base.c
>> > > index 5d269a4..81b2601 100644
>> > > --- a/drivers/of/base.c
>> > > +++ b/drivers/of/base.c
>> > > @@ -321,6 +321,8 @@ struct device_node *of_get_next_parent(struct device_node *node)
>> > > *
>> > > * Returns a node pointer with refcount incremented, use
>> > > * of_node_put() on it when done.
>> > > + *
>> > > + * Does not return nodes marked unavailable by a status property.
>> > > */
>> > > struct device_node *of_get_next_child(const struct device_node *node,
>> > > struct device_node *prev)
>> > > @@ -330,7 +332,7 @@ struct device_node *of_get_next_child(const struct device_node *node,
>> > > read_lock(&devtree_lock);
>> > > next = prev ? prev->sibling : node->child;
>> > > for (; next; next = next->sibling)
>> > > - if (of_node_get(next))
>> > > + if (of_device_is_available(next) && of_node_get(next))
>> > > break;
>> > > of_node_put(prev);
>> > > read_unlock(&devtree_lock);
>> >
>> > This seems like too low-level a place to put this. Some code may know
>> > how to un-disable a device in certain situations, or it may be part of
>> > debug code trying to dump the whole device tree, etc. Looking
>> > further[1], I see a raw version of this function, but not other things
>> > like of_find_compatible_node.
>>
>> Yeah I agree. I think we'll eventually end up with __ versions of all or
>> lots of them. Not to mention there might be cases you've missed where
>> code expects to see unavailable nodes. The right approach is to add
>> _new_ routines that don't return unavailable nodes, and convert code
>> that you know wants to use them.
>
> Actually, I don't think we really want these status-skipping
> iterators at all. The device tree iterators should give us the device
> tree, as it is. Those old-style drivers which seach for a node rather
> than using the bus probing logic can keep individual checks of the
> status property until they're converted to the new scheme.
So the patch should look something like this?
@@ -321,6 +321,8 @@ struct device_node *of_get_next_parent(struct
device_node *node)
*
* Returns a node pointer with refcount incremented, use
* of_node_put() on it when done.
+ *
+ * Do not use this function.
*/
struct device_node *of_get_next_child(const struct device_node *node,
struct device_node *prev)
...
+ struct device_node *of_get_next_available_child(const struct
device_node *node,
+ struct device_node *prev)
+ ...
+ }
And then (almost) all the of_get_next_child() sites should be changed
to call the new function?
-Hollis
More information about the devicetree-discuss
mailing list