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

On Wed, Aug 12, 2026 at 10:58:48AM +0800, Kyle Hsieh wrote:
> On Tue, Aug 11, 2026 at 5:39 PM Andy Shevchenko
> <[email protected]> wrote:
> > On Tue, Aug 11, 2026 at 10:48:38AM +0800, Kyle Hsieh wrote:

...

You quoted tons of comments to which you haven't replied. Are you agree with
them?

...

> > > +                     if (pair[0] == 0 && pair[1] == 1)
> > > +                             spec->address = 0x00;
> > > +                     else if (pair[0] == 0 && pair[1] == 2)
> > > +                             spec->address = 0x01;
> > > +                     else if (pair[0] == 0 && pair[1] == 3)
> > > +                             spec->address = 0x02;
> > > +                     else if (pair[0] == 1 && pair[1] == 0)
> > > +                             spec->address = 0x03;
> > > +                     else if (pair[0] == 1 && pair[1] == 2)
> > > +                             spec->address = 0x04;
> > > +                     else if (pair[0] == 1 && pair[1] == 3)
> > > +                             spec->address = 0x05;
> > > +                     else if (pair[0] == 2 && pair[1] == 3)
> > > +                             spec->address = 0x06;
> > > +                     else if (pair[0] == 3 && pair[1] == 2)
> > > +                             spec->address = 0x07;
> >
> > I would do this as a 4x4 table
> >
> >         -1, 0, 1, 2,
> >         3, -1, 4, 5,
> >         -1, -1, -1, 6,
> >         -1, -1, 7, -1,
> >
> > With that done you can even supported the swapped cases
> >
> >         -1, 0, 1, 2,
> >         3, -1, 4, 5,
> >         1, 4, -1, 6,
> >         2, 5, 7, -1,
> >
> > (but I haven't studied the code if it's toughly relies on the pair[0]/pair[1]
> >  values to be in a strong order after the address being assigned).
> I will apply your first 4x4 table for diff-channels - it maps directly to
> the MUX encoding and is much more readable than the if-else chain.
> 
> I did not apply the second (swapped) table. Entries like [2][0] would
> map to MUX value 1, which is AINP=AIN0/AINN=AIN2, i.e. the opposite
> polarity, so it would silently return negated values. Supporting swapped
> pairs properly would need a per-channel inversion flag and negation
> after conversion, which I'd rather leave out of this initial minimal
> implementation.

I see, thanks for the information. I agree with your choice.

...

> > > +     /* Requesting OUT_HIGH asserts the active-low reset pin immediately */
> > > +     reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> > > +     if (IS_ERR(reset_gpio))
> > > +             return PTR_ERR(reset_gpio);
> >
> > Why reset-gpio driver can't be used instead?
> David suggested the reset controller framework on v1, but after I moved
> to gpiod in v2 he suggested GPIOD_OUT_HIGH to simplify it, so I kept
> gpiod. Switching now would mean changing the binding, which already has
> a Reviewed-by - happy to do it if you both prefer.

I'm not sure how the binding might be affected by that. reset-gpio should be
direct replacement of the custom "reset" GPIO handling in the driver. If there
is a problem with reset framework (on initial state), perhaps it needs to be
addressed there?

-- 
With Best Regards,
Andy Shevchenko