Re: [PATCH v4 6/8] iio: adc: add ti-ads112c14 driver

[Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>]

On Tue, 14 Jul 2026 18:21:28 -0500
"David Lechner (TI)" <dlechner@baylibre.com> wrote:

> Add a new driver for the TI ADS112C14/ADS122C14 ADC chips.
> 
> This first step is adding a very basic driver that only supports power
> on/reset and reading the system monitor channels.
> 
> ADS112C14_SYS_MON_CHANNEL_SHORT is the last channel rather than being in
> logical order by address to keep the voltage channels together and in
> case we find we need to add variants of this channel with different
> voltage reference later.
> 
> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
I think sashiko is sending us on a wild goose chase on this one.

"When ads112c14_single_conversion() calls i2c_smbus_read_i2c_block_data(), it
returns the number of bytes read upon success. A short read (e.g. 1 or 2 bytes)
would return a positive value, bypassing the (ret < 0) error check in
ads112c14_read_raw()."

An i2c_smbus_read_i2c_block_data() response doesn't contain a length
unlike i2c_smbus_read_block_data() which does. 

Maybe there is a controller driver out there that messes with block[0]
rather than returning an error on failure to do the read part of
the sequence. I checked a few and didn't find one.

It might make sense to make it clear this doesn't happen by
adding checks in the i2c core.  One to consider after the other
ones on my list!

However, one small related comment inline about keeping postive
return values meaning success as local as possible in the code!

Jonathan




> diff --git a/drivers/iio/adc/ti-ads112c14.c b/drivers/iio/adc/ti-ads112c14.c
> new file mode 100644
> index 000000000000..a69c595ab518
> --- /dev/null
> +++ b/drivers/iio/adc/ti-ads112c14.c

...

> +
> +static int ads112c14_single_conversion(struct ads112c14_data *data,
> +				       const struct iio_chan_spec *chan,
> +				       u8 *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(regmap_get_device(data->regmap));
> +	u32 reg_val;
> +	int ret;
> +
> +	if (chan->channel < ADS112C14_SYS_MON_CHANNEL_BASE) {
> +		/* Not implemented yet. */
> +		return -EINVAL;
> +	} else {
> +		ret = ads112c14_prepare_sys_mon_channel(data, chan);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = regmap_write(data->regmap, ADS112C14_REG_CONVERSION_CTRL,
> +			   ADS112C14_CONVERSION_CTRL_START);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read_poll_timeout(data->regmap,
> +				       ADS112C14_REG_STATUS_MSB, reg_val,
> +				       FIELD_GET(ADS112C14_STATUS_MSB_DRDY, reg_val),
> +				       1 * USEC_PER_MSEC, 100 * USEC_PER_MSEC);
> +	if (ret)
> +		return ret;
> +
> +	return i2c_smbus_read_i2c_block_data(client, ADS112C14_CMD_RDATA,
> +					     BITS_TO_BYTES(data->chip_info->resolution_bits),
> +					     buf);
With all that stuff above about this not returning short, I'd still be tempted to do
an if (ret < 0) return ret; return 0; sequence in here so we don't propogate
confusing positive returns beyond where we can see their source.

Having done that make the outer check if (ret) 


> +}