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

[mike.looijmans]

On 11-05-2026 18:06, Jonathan Cameron wrote:
> On Sat, 9 May 2026 15:36:19 -0500
> David Lechner <dlechner@xxxxxxxxxxxx> 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@xxxxxxxx>
> > > Signed-off-by: Md Shofiqul Islam <shofiqtest@xxxxxxxxx>
> > > ---
> > >   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 ;)

I think the idea was to have "some" test signal when setting the mux 
into "test" mode.

But indeed, never actually used and as it raises questions, it's better 
to leave it out. Hence my suggestion.


> 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 */


--
Mike Looijmans
System Expert

TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@xxxxxxxx
W: www.topic.nl