Re: [PATCH v2 8/8] iio: adc: ti-ads112c14: add measurement channel support

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 25 Jun 2026 16:55:10 -0500
"David Lechner (TI)" <dlechner@baylibre.com> wrote:

> Add support for parsing devicetree properties for measurement channels
> and doing direct reads on these.
> 
> There are quite a lot of conditions that have to be met for each
> measurement to be made, so quite a bit of state and algorithms are
> required to handle it.
> 
> Channels are created dynamically since the number of possibilities is
> unreasonably large.
> 
> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>

Just a few minor things inline.

Thanks,

Jonathan
> 
> diff --git a/drivers/iio/adc/ti-ads112c14.c b/drivers/iio/adc/ti-ads112c14.c
> index 0e775dbc8d50..05d9670c72a4 100644
> --- a/drivers/iio/adc/ti-ads112c14.c
> +++ b/drivers/iio/adc/ti-ads112c14.c

> @@ -441,6 +594,16 @@ static int ads112c14_read_avail(struct iio_dev *indio_dev,
>  {
>  	struct ads112c14_data *data = iio_priv(indio_dev);
>  
> +	if (chan->channel < 100) {

Perhaps we need a define for that magic channel number.

> +
> +static int ads112c14_parse_channels(struct iio_dev *indio_dev,
> +				    bool *need_avdd_ref, bool *need_ext_ref)
> +{
> +	struct ads112c14_data *data = iio_priv(indio_dev);
> +	struct device *dev = indio_dev->dev.parent;
> +	struct iio_chan_spec *channels;
> +	u32 num_child_nodes, i, pair[2];
> +	int ret;
> +
> +	*need_avdd_ref = false;
> +	*need_ext_ref = false;
> +
> +	num_child_nodes = device_get_named_child_node_count(dev, "channel");
> +
> +	data->measurements = devm_kcalloc(dev, num_child_nodes,
> +					  sizeof(*data->measurements), GFP_KERNEL);
> +	if (!data->measurements)
> +		return -ENOMEM;
> +
> +	channels = devm_kcalloc(dev, num_child_nodes +
> +				ARRAY_SIZE(ads112c14_sys_mon_channels),
> +				sizeof(*channels), GFP_KERNEL);
> +	if (!channels)
> +		return -ENOMEM;
> +
> +	i = 0;
> +	device_for_each_named_child_node_scoped(dev, child, "channel") {
> +		struct ads112c14_measurement *measurement = &data->measurements[i];
> +		struct iio_chan_spec *spec = &channels[i];
> +
> +		if (!fwnode_device_is_available(child))
> +			continue;
> +
> +		spec->indexed = 1;
> +		spec->scan_index = i;
> +		measurement->gain_val = 1;
> +
> +		fwnode_property_read_string(child, "label", &measurement->label);

Hmm.  Should check if this exists and then error out on an a parsing error
when it is there but not valid?  Seems like that would be more consistent
with other properties.

> +
> +		if (fwnode_property_present(child, "single-channel")) {
> +			ret = fwnode_property_read_u32(child, "single-channel",
> +						       &pair[0]);
> +			if (ret)
> +				return dev_err_probe(dev, ret,
> +						     "failed to read single-channel property\n");
> +
> +			if (pair[0] >= 8)
> +				return dev_err_probe(dev, -EINVAL,
> +						     "single-channel value must be between 0 and 7\n");
> +
> +			spec->channel = pair[0];
> +			/* NB: channel2 is unused by iio core code in this case. */

True, but maybe should also say why you are stuffing something in it.
What makes that only useful for single channel cases?

> +			spec->channel2 = ADS112C14_MUX_CFG_AIN_GND;
> +		} else if (fwnode_property_present(child, "diff-channels")) {
> +			ret = fwnode_property_read_u32_array(child, "diff-channels",
> +							     pair, ARRAY_SIZE(pair));
> +			if (ret)
> +				return dev_err_probe(dev, ret,
> +						     "failed to read diff-channels property\n");
> +
> +			if (pair[0] >= 8 || pair[1] >= 8)
> +				return dev_err_probe(dev, -EINVAL,
> +						     "diff-channels values must be between 0 and 7\n");
> +
> +			spec->differential = 1;
> +			spec->channel = pair[0];
> +			spec->channel2 = pair[1];
> +		} else {
> +			return dev_err_probe(dev, -EINVAL,
> +					     "channel node missing channel type property\n");
> +		}
> +
> +
...