| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Z9mQPJwnKAkPHriT@kekkonen.localdomain>
Date: Tue, 18 Mar 2025 15:24:44 +0000
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Matti Vaittinen <mazziesaccount@...il.com>
Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
Jonathan Cameron <jic23@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Daniel Scally <djrscally@...il.com>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Danilo Krummrich <dakr@...nel.org>,
Claudiu Manoil <claudiu.manoil@....com>,
Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
netdev@...r.kernel.org
Subject: Re: [PATCH v8 02/10] property: Add functions to iterate named child
Moi,
On Mon, Mar 17, 2025 at 05:50:38PM +0200, Matti Vaittinen wrote:
> There are a few use-cases where child nodes with a specific name need to
> be parsed. Code like:
>
> fwnode_for_each_child_node()
> if (fwnode_name_eq())
> ...
>
> can be found from a various drivers/subsystems. Adding a macro for this
> can simplify things a bit.
>
> In a few cases the data from the found nodes is later added to an array,
> which is allocated based on the number of found nodes. One example of
> such use is the IIO subsystem's ADC channel nodes, where the relevant
> nodes are named as channel[@N].
>
> Add helpers for iterating and counting device's sub-nodes with certain
> name instead of open-coding this in every user.
>
> Suggested-by: Jonathan Cameron <jic23@...nel.org>
> Signed-off-by: Matti Vaittinen <mazziesaccount@...il.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@...il.com>
> ---
> Revision history:
> v7 => v8:
> - Fix the example in fwnode_get_named_child_node_count() documentation
> to use the fwnode_get_named_child_node_count() and not the
> device_get_named_child_node_count()
> - Fix the rest of the new macro's indentiations
> v6 => v7:
> - Improve kerneldoc
> - Inline device_get_named_child_node_count() and change it to call
> fwnode_get_named_child_node_count() inside
> - Fix indentiation of the new macros
> v5 => v6:
> - Add helpers to also iterate through the nodes.
> v4 => v5:
> - Use given name instead of string 'channel' when counting the nodes
> - Add also fwnode_get_child_node_count_named() as suggested by Rob.
> v3 => v4:
> - New patch as suggested by Jonathan, see discussion in:
> https://lore.kernel.org/lkml/20250223161338.5c896280@jic23-huawei/
> ---
> drivers/base/property.c | 27 +++++++++++++++++++++++++++
> include/linux/property.h | 24 ++++++++++++++++++++++++
> 2 files changed, 51 insertions(+)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index c1392743df9c..f42f32ff45fc 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -945,6 +945,33 @@ unsigned int device_get_child_node_count(const struct device *dev)
> }
> EXPORT_SYMBOL_GPL(device_get_child_node_count);
>
> +/**
> + * fwnode_get_named_child_node_count - number of child nodes with given name
> + * @fwnode: Node which child nodes are counted.
> + * @name: String to match child node name against.
> + *
> + * Scan child nodes and count all the nodes with a specific name. Potential
> + * 'number' -ending after the 'at sign' for scanned names is ignored.
> + * E.g.::
> + * fwnode_get_named_child_node_count(fwnode, "channel");
> + * would match all the nodes::
> + * channel { }, channel@0 {}, channel@...bba {}...
> + *
> + * Return: the number of child nodes with a matching name for a given device.
> + */
> +unsigned int fwnode_get_named_child_node_count(const struct fwnode_handle *fwnode,
> + const char *name)
> +{
> + struct fwnode_handle *child;
> + unsigned int count = 0;
> +
> + fwnode_for_each_named_child_node(fwnode, child, name)
> + count++;
> +
> + return count;
> +}
> +EXPORT_SYMBOL_GPL(fwnode_get_named_child_node_count);
> +
> bool device_dma_supported(const struct device *dev)
> {
> return fwnode_call_bool_op(dev_fwnode(dev), device_dma_supported);
> diff --git a/include/linux/property.h b/include/linux/property.h
> index e214ecd241eb..a1856e6b714c 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -167,10 +167,18 @@ struct fwnode_handle *fwnode_get_next_available_child_node(
> for (child = fwnode_get_next_child_node(fwnode, NULL); child; \
> child = fwnode_get_next_child_node(fwnode, child))
>
> +#define fwnode_for_each_named_child_node(fwnode, child, name) \
> + fwnode_for_each_child_node(fwnode, child) \
> + if (!fwnode_name_eq(child, name)) { } else
> +
> #define fwnode_for_each_available_child_node(fwnode, child) \
> for (child = fwnode_get_next_available_child_node(fwnode, NULL); child;\
> child = fwnode_get_next_available_child_node(fwnode, child))
>
> +#define fwnode_for_each_available_named_child_node(fwnode, child, name) \
> + fwnode_for_each_available_child_node(fwnode, child) \
> + if (!fwnode_name_eq(child, name)) { } else
> +
OF only enumerates available nodes via the fwnode API, software nodes don't
have the concept but on ACPI I guess you could have a difference in nodes
where you have device sub-nodes that aren't available. Still, these ACPI
device nodes don't have meaningful names in this context (they're
4-character object names) so you wouldn't use them like this anyway.
So my question is: is it useful to provide this besides
fwnode_for_each_named_child_node(), given that both are effectively the
same?
> struct fwnode_handle *device_get_next_child_node(const struct device *dev,
> struct fwnode_handle *child);
>
> @@ -178,11 +186,19 @@ struct fwnode_handle *device_get_next_child_node(const struct device *dev,
> for (child = device_get_next_child_node(dev, NULL); child; \
> child = device_get_next_child_node(dev, child))
>
> +#define device_for_each_named_child_node(dev, child, name) \
> + device_for_each_child_node(dev, child) \
> + if (!fwnode_name_eq(child, name)) { } else
> +
> #define device_for_each_child_node_scoped(dev, child) \
> for (struct fwnode_handle *child __free(fwnode_handle) = \
> device_get_next_child_node(dev, NULL); \
> child; child = device_get_next_child_node(dev, child))
>
> +#define device_for_each_named_child_node_scoped(dev, child, name) \
> + device_for_each_child_node_scoped(dev, child) \
> + if (!fwnode_name_eq(child, name)) { } else
> +
> struct fwnode_handle *fwnode_get_named_child_node(const struct fwnode_handle *fwnode,
> const char *childname);
> struct fwnode_handle *device_get_named_child_node(const struct device *dev,
> @@ -210,6 +226,14 @@ int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name);
>
> unsigned int device_get_child_node_count(const struct device *dev);
>
> +unsigned int fwnode_get_named_child_node_count(const struct fwnode_handle *fwnode,
> + const char *name);
> +static inline unsigned int device_get_named_child_node_count(const struct device *dev,
> + const char *name)
> +{
> + return fwnode_get_named_child_node_count(dev_fwnode(dev), name);
> +}
> +
> static inline int device_property_read_u8(const struct device *dev,
> const char *propname, u8 *val)
> {
--
Terveisin,
Sakari Ailus
Powered by blists - more mailing lists