Re: [PATCH v4 2/2] iio: light: add AS7343 multi-spectral sensor driver

[Chang Yu]

On Tue, Sep 15, 2026 at 10:34:06AM +0300, Andy Shevchenko wrote:
> On Mon, Sep 14, 2026 at 07:49:36PM -0700, Chang Yu wrote:
> > On Mon, Sep 14, 2026 at 11:00:18AM +0300, Andy Shevchenko wrote:
> > > On Fri, Sep 11, 2026 at 06:39:12PM -0700, Chang Yu wrote:
> 
> ...
> 
> > > > +static int as7343_suspend(struct device *dev)
> > > > +{
> > > > +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> > > > +	struct as7343_data *data = iio_priv(indio_dev);
> > > > +	struct regmap *map = data->regmap;
> > > 
> > > > +	return regmap_clear_bits(map, AS7343_ENABLE, AS7343_ENABLE_SP_EN);
> > > 
> > > Can this mess up the raw read? If so, also needs a mutex to be held.
> > 
> > People familiar with runtime PM correct me if I'm wrong, but I believe
> > PM_RUNTIME_ACQUIRE waits for any ongoing suspend callback to finish
> > before returning? If so I think there is no risk of a race here. In
> > the case of system suspend, worst case scenario SP_EN gets cleared
> > before we read ASTATUS or the data register, which for my use case
> > at least is OK since the entire system is suspending anyway.
> 
> Yes, if the raw read is guarded. What about _setup()? Is it guaranteed
> to be free from races? If so, we are okay.
> 
Currently in setup_device() I'm enabling runtime PM before flipping SP_EN
which is a bit problematic. The fix is simply enable SP_EN first before
enaling runtime PM like Jonathan has suggested. Then I think we will be OK.