| Message ID | 20240904090016.2841572-3-wenst@chromium.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | platform/chrome: Introduce DT hardware prober | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | fail | build log |
On Wed, Sep 04, 2024 at 05:00:04PM +0800, Chen-Yu Tsai wrote: > There are cases where drivers would go through child device nodes and > operate on only the ones whose node name starts with a given prefix. > > Provide a helper for these users. This will mainly be used in a > subsequent patch that implements a hardware component prober for I2C > busses. ... > +#define for_each_child_of_node_with_prefix_scoped(parent, child, prefix) \ > + for (struct device_node *child __free(device_node) = \ > + of_get_next_child_with_prefix(parent, NULL, prefix); \ > + child != NULL; \ > + child = of_get_next_child_with_prefix(parent, child, prefix)) I'm wondering if we may drop _scoped from day 1. Yeah, probably a bit confusing as the rest of APIs without that suffix do require reference count handling on the loop abrupt.
On Wed, Sep 04, 2024 at 04:03:47PM +0300, Andy Shevchenko wrote: > On Wed, Sep 04, 2024 at 05:00:04PM +0800, Chen-Yu Tsai wrote: > > There are cases where drivers would go through child device nodes and > > operate on only the ones whose node name starts with a given prefix. > > > > Provide a helper for these users. This will mainly be used in a > > subsequent patch that implements a hardware component prober for I2C > > busses. > > ... > > > +#define for_each_child_of_node_with_prefix_scoped(parent, child, prefix) \ > > + for (struct device_node *child __free(device_node) = \ > > + of_get_next_child_with_prefix(parent, NULL, prefix); \ > > + child != NULL; \ > > + child = of_get_next_child_with_prefix(parent, child, prefix)) > > I'm wondering if we may drop _scoped from day 1. Yeah, probably a bit confusing > as the rest of APIs without that suffix do require reference count handling on > the loop abrupt. Yes, please drop. We have other new ones coming and they don't have "_scoped". I was on the fence, but if you use a scoped one like a non-scoped one (declaring child outside or using it outside the loop) you will get a compiler error. Rob
diff --git a/drivers/of/base.c b/drivers/of/base.c index 20603d3c9931..d3c123b3261a 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -628,6 +628,41 @@ struct device_node *of_get_next_child(const struct device_node *node, } EXPORT_SYMBOL(of_get_next_child); +/** + * of_get_next_child_with_prefix - Find the next child node with prefix + * @node: parent node + * @prev: previous child of the parent node, or NULL to get first + * + * This function is like of_get_next_child(), except that it automatically + * skips any nodes whose name doesn't have the given prefix. + * + * Return: A node pointer with refcount incremented, use + * of_node_put() on it when done. + */ +struct device_node *of_get_next_child_with_prefix(const struct device_node *node, + struct device_node *prev, + const char *prefix) +{ + struct device_node *next; + unsigned long flags; + + if (!node) + return NULL; + + raw_spin_lock_irqsave(&devtree_lock, flags); + next = prev ? prev->sibling : node->child; + for (; next; next = next->sibling) { + if (!of_node_name_prefix(next, prefix)) + continue; + if (of_node_get(next)) + break; + } + of_node_put(prev); + raw_spin_unlock_irqrestore(&devtree_lock, flags); + return next; +} +EXPORT_SYMBOL(of_get_next_child_with_prefix); + static struct device_node *of_get_next_status_child(const struct device_node *node, struct device_node *prev, bool (*checker)(const struct device_node *)) diff --git a/include/linux/of.h b/include/linux/of.h index 046283be1cd3..acc0d5b98417 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -289,6 +289,9 @@ extern struct device_node *of_get_parent(const struct device_node *node); extern struct device_node *of_get_next_parent(struct device_node *node); extern struct device_node *of_get_next_child(const struct device_node *node, struct device_node *prev); +extern struct device_node *of_get_next_child_with_prefix(const struct device_node *node, + struct device_node *prev, + const char *prefix); extern struct device_node *of_get_next_available_child( const struct device_node *node, struct device_node *prev); extern struct device_node *of_get_next_reserved_child( @@ -1468,6 +1471,12 @@ static inline int of_property_read_s32(const struct device_node *np, child != NULL; \ child = of_get_next_child(parent, child)) +#define for_each_child_of_node_with_prefix_scoped(parent, child, prefix) \ + for (struct device_node *child __free(device_node) = \ + of_get_next_child_with_prefix(parent, NULL, prefix); \ + child != NULL; \ + child = of_get_next_child_with_prefix(parent, child, prefix)) + #define for_each_available_child_of_node(parent, child) \ for (child = of_get_next_available_child(parent, NULL); child != NULL; \ child = of_get_next_available_child(parent, child))
There are cases where drivers would go through child device nodes and operate on only the ones whose node name starts with a given prefix. Provide a helper for these users. This will mainly be used in a subsequent patch that implements a hardware component prober for I2C busses. Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> --- Changes since v5: - New patch --- drivers/of/base.c | 35 +++++++++++++++++++++++++++++++++++ include/linux/of.h | 9 +++++++++ 2 files changed, 44 insertions(+)