Re: [PATCH v2 6/6] iio: adc: ti-ads1298: add ADS1299 support

[Stephan Costa Barros]

On Sun, 25 Oct 2026 23:34:29 +0000, Jonathan Cameron wrote:
> A few things in here feel like they should be in precursor
> patches because the implication is they apply just as much
> to improving support for the ads1298 as they do for the
> new device.

Agreed. DVDD and PWDN become ADS129x patches, binding and driver, before
the ADS1299 ones.

> > +	if (FIELD_GET(ADS1299_MASK_ID_DEVICE, id) != ADS1299_ID_DEVICE ||
> > +	    nu_ch > 2)
> > +		return dev_err_probe(dev, -ENODEV, "Unknown ID: 0x%x\n", id);
> How do backwards compatible parts with fallback compatible of ads1299 work?
>
> Generally we don't reject on simply an who am I value mismatch.
> Note the existing driver may need relaxing on that point - we used
> to do this before DT maitainers pointed out what it broke!

Today that works only within a family: the channel count comes from the
ID, so ti,ads1299 serves the -4 and -6, and ti,ads1298 the ADS1294/6
and the R parts. A part with a new ID is refused, as the existing driver
does too.

For v3 I'd do, in a precursor patch for the existing driver: the
compatible's chip info is what the driver trusts. If the ID is a known
part of that family, it still gives the channel count and the name; if
not (another family, or a reserved channel code), the driver warns and
goes on with the compatible's chip info. The channel count from the ID
has to stay, since ti,ads1298 has been the only compatible and existing
boards with an ADS1294 or ADS1296 depend on it.

Does that match what you have in mind?

> > +	/* The ADS1299 comes out of reset with its inputs shorted */
>
> What is the case for the ADS1298 - is this just harmless or does
> that have similar behavior?

Harmless. On the ADS129x CHnSET resets to 00h, normal electrode input,
so the write changes nothing. On the ADS1299 it resets to 61h, input
shorted, and without the write each channel reads its own shorted input.
I'll make it ADS1299 only, through chip info, unless you prefer it
unconditional.

> > +	for (i = 0; i < indio_dev->num_channels; i++) {
>
> 	for (unsigned int i = 0;...

Will do.

Thanks,
Stephan