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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Sat, Oct 24, 2026 at 08:13:35PM -0300, Stephan Costa Barros wrote:
> The ADS1299, ADS1299-6 and ADS1299-4 are 8, 6 and 4-channel, 24-bit
> biopotential ADCs for EEG. They share the register map, the commands
> and the data format of the ADS129x, and differ in:
> 
> - the PGA gains, 1, 2, 4, 6, 8, 12 and 24;
> - the data rates, f_CLK / 2^(7 + DR) with no HR bit, from 16 kSPS to
>   250 SPS, and the oversampling ratio, 64 * 2^DR;
> - the internal reference, a fixed 4.5 V, and the reserved bits 6:5 of
>   CONFIG3;
> - the ID register, which holds the channel count in bits 1:0;
> - the register map, which ends at CONFIG4;
> - the inputs, which are shorted out of reset.
> 
> Add the chip info for the ADS1299, and connect the inputs of every
> channel at init, which changes nothing on the ADS129x, whose inputs are
> already connected out of reset. The channel count is read from the ID
> register, so ti,ads1299 also serves the ADS1299-4 and ADS1299-6 through
> their fallback.
> 
> For both families, enable the DVDD supply when the device tree gives
> one, and hold PWDN asserted, like RESET, until the supplies and the
> clock are up. Nothing changes for a device tree that has neither.

...

>  config TI_ADS1298
> -	tristate "Texas Instruments ADS1298"
> +	tristate "Texas Instruments ADS1298 and ADS1299"
>  	depends on SPI
>  	select IIO_BUFFER
>  	select IIO_KFIFO_BUF
>  	select REGMAP
>  	help
> -	  If you say yes here you get support for Texas Instruments ADS1298
> -	  medical ADC chips
> +	  If you say yes here you get support for Texas Instruments ADS1294,
> +	  ADS1296, ADS1298 and ADS1299 medical ADC chips, and their variants

Make this a list, see how other drivers (recently updated to support more HW)
do it.


>  	  This driver can also be built as a module. If so, the module will be
>  	  called ti-ads1298.

...

>  // SPDX-License-Identifier: GPL-2.0
> -/* TI ADS1298 chip family driver
> +/* TI ADS1298 and ADS1299 chip family driver
>   * Copyright (C) 2023 - 2024 Topic Embedded Products
> + * Copyright (C) 2026 Stephan Costa Barros <stephanbrrs8@gmail.com>
>   */

Fix multi-line comment style, make the text of tristate in Kconfig, this one,
and in the MODULE_DESCRIPTION() the same.

...

> +	/* TI sells the 8-channel part as plain ADS1299 */
> +	if (indio_dev->num_channels == ADS1298_MAX_CHANNELS)
> +		indio_dev->name = "ads1299";
> +	else
> +		indio_dev->name = devm_kasprintf(dev, GFP_KERNEL, "ads1299-%u",
> +						 indio_dev->num_channels);

Why do we need this? Can't we derive it from IIO ABI?

> +	if (!indio_dev->name)
> +		return -ENOMEM;

...

>  static int ads1298_init(struct iio_dev *indio_dev)
>  {
>  	struct ads1298_private *priv = iio_priv(indio_dev);
>  	unsigned int val;
> +	unsigned int i;

It's not used outside of the loop, make it local to that for-loop.

>  	int ret;
>  
>  	/* Device initializes into RDATAC mode, which we don't want */

>  	if (!priv->reg_vref)
>  		msleep(ADS1298_REF_START_UP_MS);
>  
> +	/* The ADS1299 comes out of reset with its inputs shorted */
> +	for (i = 0; i < indio_dev->num_channels; i++) {
> +		ret = regmap_update_bits(priv->regmap, ADS1298_REG_CHnSET(i),
> +					 ADS1298_MASK_CH_MUX,
> +					 FIELD_PREP(ADS1298_MASK_CH_MUX,
> +						    ADS1298_MUX_NORMAL));
> +		if (ret)
> +			return ret;
> +	}
> +
>  	return 0;
>  }

-- 
With Best Regards,
Andy Shevchenko