Re: [PATCH v4 3/3] iio: adc: ti-ads1298: Remove unnecessary CONFIG2 write during init

[Jonathan Cameron <jic23@kernel.org>]

On Sat, 9 May 2026 15:36:19 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 5/9/26 10:19 AM, Md Shofiqul Islam wrote:
> > The driver was enabling the internal test signal (INT_TEST), double
> > amplitude (TEST_AMP), and fast frequency (TEST_FREQ_FAST) bits in
> > CONFIG2 during initialization. These bits activate an internal square
> > wave generator intended for device testing and calibration, not normal
> > ECG operation.
> > 
> > CONFIG2 defaults to having only the RESERVED bit set after reset, which
> > is the correct value for normal operation. Remove the write entirely
> > since it would just be writing the reset default value.
> > 
> > Suggested-by: Mike Looijmans <mike.looijmans@topic.nl>
> > Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
> > ---
> >  drivers/iio/adc/ti-ads1298.c | 9 ---------
> >  1 file changed, 9 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
> > index 186bda3087..8957e873e1 100644
> > --- a/drivers/iio/adc/ti-ads1298.c
> > +++ b/drivers/iio/adc/ti-ads1298.c
> > @@ -615,15 +615,6 @@ static int ads1298_init(struct iio_dev *indio_dev)
> >  	if (!indio_dev->name)
> >  		return -ENOMEM;
> >  
> > -	/* Enable internal test signal, double amplitude, double frequency */  
> 
> This is why comments should say why and not what. If the original author
> had a plan for this, we don't know what it was.
> 
Mike, any recollection?

Anyhow I'm going to apply it anyway given Mike suggested it and fairly 
sure it was Mikes' code ;)

Applied.
> I look at the datasheet and the driver. Seems OK since the test signals
> aren't used anywhere in the driver right now. 
> 
> > -	ret = regmap_write(priv->regmap, ADS1298_REG_CONFIG2,
> > -			   ADS1298_MASK_CONFIG2_RESERVED |
> > -			   ADS1298_MASK_CONFIG2_INT_TEST |
> > -			   ADS1298_MASK_CONFIG2_TEST_AMP |
> > -			   ADS1298_MASK_CONFIG2_TEST_FREQ_FAST);
> > -	if (ret)
> > -		return ret;
> > -
> >  	val = ADS1298_MASK_CONFIG3_RESERVED; /* Must write 1 always */
> >  	if (!priv->reg_vref) {
> >  		/* Enable internal reference */  
>