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

[Andy Shevchenko]

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:

...

> > > +	if (val != 0x81)
> > > +		dev_info(dev, "Unknown device ID: %x\n", val);
> > 
> > Wouldn't be better to define 0x81 with meaningful name?
> 
> If I remeber correctly Jonathan prefers just putting 0x81 inline. I'm
> OK with either style so I'll defer to the judgement to Jonathan here.

OK!

...

> > > +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.

-- 
With Best Regards,
Andy Shevchenko