Re: [PATCH v2 2/2] iio: adc: ti-ads1015: Add support for label

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `ads1015_read_label()` function passes a potentially NULL pointer to `sysfs_emit()`, causing userspace to read `"(null)\n"` for unconfigured channels.
--

commit 9084b6f4b1c391e596d3456f4a84598bb430a6e6
Author: Flaviu Nistor <flaviu.nistor@gmail.com>
Subject: iio: adc: ti-ads1015: Add support for label

Add support for label sysfs attribute similar to other adc devices. This is
particularly useful because the label can match the schematic signal name,
identifying channel voltages easier if label is defined via device tree.

Link: https://lore.kernel.org/all/20260901184302.8127-2-flaviu.nistor@gmail.com/

> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index 0fbfa4e499aa8..8be1f0c32ca9d 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
[ ... ]
> @@ -586,6 +587,15 @@ static int ads1015_read_raw(struct iio_dev *indio_dev,
>  	}
>  }
>  
> +static int ads1015_read_label(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan, char *label)
> +{
> +	struct ads1015_data *data = iio_priv(indio_dev);
> +	const char *name = data->channel_data[chan->address].label;
> +
> +	return sysfs_emit(label, "%s\n", name);

[Severity: Medium]
Will this return "(null)\n" to userspace for channels without a configured
label?

If a channel is not explicitly defined in the firmware nodes, or if the driver
falls back to the default configuration via ads1015_get_channels_config(),
the label field will remain NULL. Passing a NULL pointer to sysfs_emit()
results in an incorrect literal "(null)\n" output at the UAPI boundary.

> +}
> +
>  static int ads1015_write_raw(struct iio_dev *indio_dev,
>  			     struct iio_chan_spec const *chan, int val,
>  			     int val2, long mask)
[ ... ]
> @@ -897,6 +911,17 @@ static int ads1015_client_get_channels_config(struct i2c_client *client)
>  			}
>  		}
>  
> +		if (fwnode_property_present(node, "label")) {
> +			ret = fwnode_property_read_string(node, "label", &label);
> +			if (ret) {
> +				dev_err(dev, "invalid label on %pfw\n", node);
> +				return ret;
> +			}
> +			data->channel_data[channel].label = label;
> +		} else {
> +			data->channel_data[channel].label = "";
> +		}
> +

[Severity: Medium]
Are there cases where some channels might be skipped in this loop?

Since this initialization is contained within the child node iteration loop,
it only covers channels that are explicitly defined in the firmware nodes.
Channels omitted from the device tree will remain zero-initialized, leaving
their label field as NULL.

>  		data->channel_data[channel].pga = pga;
>  		data->channel_data[channel].data_rate = data_rate;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902172230.5234-1-flaviu.nistor@gmail.com?part=2