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

Hi Jonathan,
Thank you for stepping in and providing the perspective!

On Sun, Aug 2, 2026 at 6:59=E2=80=AFAM Jonathan Cameron <[email protected]> =
wrote:
>
> 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-ads1=
12c04.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-aab016=
8c3c01%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.
Point well taken. I have reverted the IRQ flags parameter back to `0`
in the `devm_request_irq()`
call to let the driver naturally inherit the trigger type from the
device tree, rather than hardcoding
`IRQF_TRIGGER_FALLING` just to appease the bot's edge-case warning.
I've also made sure the expected edge-falling behavior is clearly
documented in the DT binding example.
>
> 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 tha=
t
> transaction.
>
> > > +static int ads112c04_read_reg(struct i2c_client *client, u8 reg, u8 =
*val)
> > > +{
> > > +   u8 cmd =3D ADS112C04_CMD_RREG(reg);
> > > +   int ret;
> > > +
> > > +   ret =3D 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 possi=
ble.
Taking both your and David's advice, I have completely replaced the
raw `i2c_master_send/recv`
transfers with `i2c_smbus_read_byte_data`, `i2c_smbus_write_byte_data`, and
`i2c_smbus_read_word_data`.

This elegantly handles the transactions and completely side-steps the
headache of parsing
those obscure `0` return values from `i2c_master_send`.

Thanks again for the guidance!

Best regards,
Kyle Hsieh
> >
> > > +   if (ret < 0)
> > > +           return ret;
> > > +
> > > +   ret =3D i2c_master_recv(client, val, 1);
> > > +   return ret < 0 ? ret : 0;
> > > +}