Re: [PATCH v2 4/6] iio: adc: ti-ads1298: prepare for more chip families

[Stephan Costa Barros]

On Sun, 25 Oct 2026 23:26:08 +0000, Jonathan Cameron wrote:
> > +/* What differs between the chip families the driver supports */
>
> This is very standard naming and purpose. I'd drop the comment.

Will drop it, and the other generic ones. dr and hr will be spelled
out, and vref_mv becomes internal_vref_mV.

> > +	/* CONFIG3 has the VREF_4V bit, which selects a 4 V internal reference */
> > +	bool has_vref_4v;
>
> I'm lost here. If it's about the bit, then name it after the bit.
> I suspect it's about internal reference that can be disabled or not?
> In that case can you name it for that functionality instead?
>
> From a quick look below, seems it is about what of two modes can be
> used?  If so how does that fit with just one vref_mv above?

It is about the internal reference. On the ADS129x it is 2.4 V, or 4 V
when CONFIG3 VREF_4V is set, which the driver does when AVDD is 4.4 V or
more. On the ADS1299 it is a fixed 4.5 V, and that bit is reserved. So
vref_mv was the default internal reference, and has_vref_4v said that
the 4 V option exists.

For v3: chip info has internal_vref_mV (2400 and 4500) and a bool named
for the 4 V option; init chooses the reference once and keeps it, so
read_raw no longer reads CONFIG3. Andy asked for the same.

> > +	/* Reserved bits of CONFIG3 that must be written as 1 */
>
> This one is interesting. Normally we deal with these by just using
> RMW cycles on such a register.   Can we do that here?

Yes. CONFIG3 resets to 40h on the ADS129x and 60h on the ADS1299, with
the reserved bits set, and the regmap has no defaults, so
regmap_update_bits() reads them from the chip. config3_reserved goes.

> Please use FIELD_GET() and fix the family masks to be values in the field
> rather than the shifted version.  You might want to do that
> as a separate precursor patch.  Looks like there may be other similar
> changes in driver around fields vs value defines.

Will do

Thanks,
Stephan