Re: [PATCH v6 07/16] iio: core: add hierarchical channel relationships

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 18 Jun 2026 14:27:23 +0100
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:

> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> 
> Add parent-child relationship between iio channels by creating a parent
> pointer field in iio_chan_spec struct and exposing a sysfs attribute that
> returns the parent channel label.
> 
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
Generally I like this.  A couple of trivial suggestions inline.

Jonathan

> ---
>  drivers/iio/industrialio-core.c | 44 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/iio/iio.h         |  5 +++++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 9373006235c8..3d12269f26f4 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -854,6 +854,21 @@ static ssize_t iio_read_channel_label(struct device *dev,
>  					 to_iio_dev_attr(attr)->c, buf);
>  }
>  
> +static ssize_t iio_read_channel_parent(struct device *dev,
> +				       struct device_attribute *attr,
> +				       char *buf)
> +{
> +	const struct iio_chan_spec *parent = to_iio_dev_attr(attr)->c->parent;
> +	int len;
> +
> +	len = __iio_chan_prefix_emit(parent, IIO_SEPARATE, buf, PAGE_SIZE - 1);
> +	if (len < 0)
> +		return len;
> +
> +	buf[len - 1] = '\n'; /* replace underscore termination with newline */

That's irritating.  Can we instead make __iio_chan_prefix_emit() not include the _ ?
That might mean a wrapper to just do the last bit of logic in there.


> +	return len;
> +}
> +
>  static ssize_t iio_read_channel_info(struct device *dev,
>  				     struct device_attribute *attr,
>  				     char *buf)
> @@ -1263,6 +1278,30 @@ static int iio_device_add_channel_label(struct iio_dev *indio_dev,
>  	return 1;
>  }
>  
> +static int iio_device_add_channel_parent(struct iio_dev *indio_dev,
> +					 struct iio_chan_spec const *chan)
> +{
> +	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
> +	int ret;
> +
> +	if (!chan->parent)
> +		return 0;
> +
> +	ret = __iio_add_chan_devattr("parent",
> +				     chan,
> +				     &iio_read_channel_parent,
> +				     NULL,
> +				     0,
> +				     IIO_SEPARATE,
> +				     &indio_dev->dev,
> +				     NULL,
> +				     &iio_dev_opaque->channel_attr_list);

Probably younger me style, but can we group a few of those into logical
sets to reduce the number of lines?

> +	if (ret < 0)
> +		return ret;
> +
> +	return 1;

Hmm. Can we make it explicit this is about an attr in the function name.
I think that would make the positive return more something we might expect
than something that sounds like it is adding something non countable.

> +}
> +
>  static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
>  					 struct iio_chan_spec const *chan,
>  					 enum iio_shared_by shared_by,
> @@ -1401,6 +1440,11 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
>  		return ret;
>  	attrcount += ret;
>  
> +	ret = iio_device_add_channel_parent(indio_dev, chan);
> +	if (ret < 0)
> +		return ret;
> +	attrcount += ret;
> +
>  	if (chan->ext_info) {
>  		unsigned int i = 0;
>  
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 1c7d12af22da..9470ab8eb726 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -264,6 +264,10 @@ struct iio_scan_type {
>   * @ext_info:		Array of extended info attributes for this channel.
>   *			The array is NULL terminated, the last element should
>   *			have its name field set to NULL.
> + * @parent:		Optional pointer to the parent channel spec for
> + *			hierarchical channel relationships. When set, a read-only
> + *			"parent" sysfs attribute is created containing the
> + *			parent channel's sysfs name prefix (e.g. "in_voltage0").
>   * @extend_name:	Allows labeling of channel attributes with an
>   *			informative name. Note this has no effect codes etc,
>   *			unlike modifiers.
> @@ -309,6 +313,7 @@ struct iio_chan_spec {
>  	const struct iio_event_spec *event_spec;
>  	unsigned int		num_event_specs;
>  	const struct iio_chan_spec_ext_info *ext_info;
> +	const struct iio_chan_spec *parent;
>  	const char		*extend_name;
>  	const char		*datasheet_name;
>  	unsigned int		modified:1;
>