Re: [PATCH] iio: light: apds9999: register standby action after enabling device

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 26 Jul 2026 20:50:30 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 24 Jul 2026 08:28:02 +0530
> kr494167@gmail.com wrote:
> 
> > From: Surendra Singh Chouhan <kr494167@gmail.com>
> > 
> > apds9999_init() called devm_add_action_or_reset() at the start
> > of the function, before register configuration and before enabling the
> > device via APDS9999_MAIN_CTRL_LS_EN.
> > 
> > If register initialization failed during apds9999_init() (e.g. SMBus
> > write failures on LS_MEAS_RATE or LS_GAIN), devm_add_action_or_reset()
> > immediately triggered apds9999_standby(), writing to the control
> > register on a device that failed initialization and was never enabled.
> > 
> > Fix this by registering devm_add_action_or_reset() only after
> > APDS9999_MAIN_CTRL_LS_EN is successfully written to the control
> > register.
> > 
> > Fixes: 5f9363e52300 ("iio: light: add support for APDS9999 sensor")  
> 
> Given the side effect of this is write that is harmless, I don't
> think the fixes tag is justified. 
> 
> Having said that this is a logical improvement.
> 
> I would like Jose to have taken a look though before I consider picking
> it up so let us leave it on list for now.
> 
Seems they are busy or missed this.  So let me gamble...

Applied to the testing branch of iio.git which I will be rebasing on rc1
once available. Note that in the meantime I am happy to rebase so late
comments or tags are fine.

thanks

Jonathan

> Thanks,
> 
> Jonathan
> 
> > Signed-off-by: Surendra Singh Chouhan <kr494167@gmail.com>
> > ---
> >  drivers/iio/light/apds9999.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/iio/light/apds9999.c b/drivers/iio/light/apds9999.c
> > index 43fa9992c9c2..62620c4a2fbc 100644
> > --- a/drivers/iio/light/apds9999.c
> > +++ b/drivers/iio/light/apds9999.c
> > @@ -100,10 +100,6 @@ static int apds9999_init(struct apds9999_data *data)
> >  	u8 regval;
> >  	int ret;
> >  
> > -	ret = devm_add_action_or_reset(dev, apds9999_standby, client);
> > -	if (ret)
> > -		return ret;
> > -
> >  	guard(mutex)(&data->lock);
> >  
> >  	regval = FIELD_PREP(APDS9999_LS_RES_MASK, APDS9999_RES_18BIT) |
> > @@ -121,8 +117,12 @@ static int apds9999_init(struct apds9999_data *data)
> >  		return ret;
> >  	data->als_gain_idx = APDS9999_GAIN_3X;
> >  
> > -	return i2c_smbus_write_byte_data(client, APDS9999_REG_MAIN_CTRL,
> > -					 APDS9999_MAIN_CTRL_LS_EN);
> > +	ret = i2c_smbus_write_byte_data(client, APDS9999_REG_MAIN_CTRL,
> > +					APDS9999_MAIN_CTRL_LS_EN);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return devm_add_action_or_reset(dev, apds9999_standby, client);
> >  }
> >  
> >  static int apds9999_read_channel(struct apds9999_data *data, u8 reg,  
> 
>