Re: [PATCH v2 2/2] iio: adc: ti-ads112c04: Add support for TI ADS112C04

On Fri, 31 Jul 2026 11:27:50 +0200
Joshua Crofts <[email protected]> wrote:

> On Fri, 31 Jul 2026 10:58:25 +0800
> Kyle Hsieh <[email protected]> wrote:
> > diff --git a/drivers/iio/adc/ti-ads112c04.c b/drivers/iio/adc/ti-ads112c04.c
> > new file mode 100644
> > index 000000000000..28d3be81934f
> > --- /dev/null
> > +++ b/drivers/iio/adc/ti-ads112c04.c  
> 
> Hi Kyle, quick review from me, comments inline. Additionally, please
> check Sashiko's review as there are some move severe issues (mostly
> I2C stuff), see it here:
> https://sashiko.dev/#/patchset/20260731-ti-ads112c04-driver-v2-0-aab0168c3c01%40gmail.com
Be careful with these.   Some may be misleading or the correct
response may be in a very different place to sashiko suggests.
For example we wouldn't typically bother to defend against nonsense
interrupt types from DT, so if level isn't a plausible type then
state what is as a comment in the DT.

Also, multi master doesn't seems like something we should worry
too much about.  That's not to say there isn't a better way to handle that
transaction.

> > +static int ads112c04_read_reg(struct i2c_client *client, u8 reg, u8 *val)
> > +{
> > +	u8 cmd = ADS112C04_CMD_RREG(reg);
> > +	int ret;
> > +
> > +	ret = i2c_master_send(client, &cmd, 1);  
> 
> i2c_master_send returns either the amount of bytes sent or an error
> code. If the device NACKs, the function will return 0 (zero bytes sent)
> but this will be interpreted as success.

This one is fun and not necessarily that simple. (I used to assume it was!)
Mostly nacks will result in error codes, but there are other obscure
reasons it might return 0.  

Anyhow, whatever the reason, agreed we annoyingly have to check these to see if
they are not 0.

Or, better, as per David's comment use the smbus command if that is possible.
> 
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = i2c_master_recv(client, val, 1);
> > +	return ret < 0 ? ret : 0;
> > +}