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

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 29 Oct 2026 21:59:28 +0100
"Jose A. Perez de Azpillaga" <azpijr@gmail.com> wrote:

> On 10/29/26 5:38 PM, Chang Yu wrote:
> > On Thu, Oct 29, 2026 at 12:20:24PM +0100, Jose A. Perez de Azpillaga wrote:  
> >> On 10/29/26 8:48 AM, Chang Yu wrote:  
> >>> Add a driver for the AMS AS7343 14-channel multi-spectral sensor.
> >>>
> >>> The AS7343 is a 14-channel spectral sensor featuring 11 visible
> >>> channels, 1 near-infrared channel, 1 clear channel (VIS), and 1
> >>> flicker detection channel.
> >>>
> >>> The driver exposes 12 spectral channels (11 visible light and 1
> >>> near-infrared) via sysfs. Runtime PM is implemented to stop
> >>> measurements and enter SLEEP state when the device is suspended
> >>> or torn down.
> >>>
> >>> Future patches will add configurable gain and integration time,
> >>> interrupt support, buffered reads, VIS channel, and flicker
> >>> detection.
> >>>
> >>> Datasheet: https://look.ams-osram.com/m/5f2d27fff9a874d2/original/AS7343-14-Channel-Multi-Spectral-Sensor.pdf
> >>> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> >>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> >>> Signed-off-by: Chang Yu <marcus.yu.56@gmail.com>  
> >>
> >>
> >> [snip]
> >>  
> >>> +		/* Check for digital saturation. */
> >>> +		if (status & AS7343_ASTATUS_ASAT_STATUS) {
> >>> +			/*
> >>> +			 * Per the datasheet, ASTATUS is self-clearing: writing
> >>> +			 * the value back clears the entire register.
> >>> +			 */
> >>> +			ret = regmap_write(map, AS7343_ASTATUS_REG, status);  
> >>
> >>
> >> isn't that the STATUS (0x93) text? the datasheet says ASTATUS is read
> >> only and ASAT_STATUS clears on access, so the write shouldn't be needed.
> >> it also covers analog saturation, not only digital
> >>  
> > Aw man, how did I not notice that?  
> 
> easy to mix up, STATUS and ASTATUS are one letter apart :)
> 
> >   
> >>> +			return ret ? ret : -EIO;  
> >>
> >> -EIO reads like a bus error. as73211 returns -EOVERFLOW for this,
> >> wouldn't that fit better?  
> > 
> > Sounds good to me. Jonathan, if v7 goes in, can you replace this if
> > block with:
> > 
> > 		/* Check for saturation. */
> > 		if (status & AS7343_ASTATUS_ASAT_STATUS)
> > 			return -EOVERFLOW;
Diff I applied is:
diff --git a/drivers/iio/light/as7343.c b/drivers/iio/light/as7343.c
index a26f2cd571f9..aff0cba2239c 100644
--- a/drivers/iio/light/as7343.c
+++ b/drivers/iio/light/as7343.c
@@ -195,15 +195,9 @@ static int as7343_read_raw(struct iio_dev *indio_dev,
                if (ret)
                        return ret;
 
-               /* Check for digital saturation. */
-               if (status & AS7343_ASTATUS_ASAT_STATUS) {
-                       /*
-                        * Per the datasheet, ASTATUS is self-clearing: writing
-                        * the value back clears the entire register.
-                        */
-                       ret = regmap_write(map, AS7343_ASTATUS_REG, status);
-                       return ret ? ret : -EIO;
-               }
+               /* Check for saturation. */
+               if (status & AS7343_ASTATUS_ASAT_STATUS)
+                       return -EOVERFLOW;
 
                ret = regmap_bulk_read(map, chan->address,
                                       &result, sizeof(result));

With that applied to the testing branch of iio.git.

Shout if I misunderstood this discussion.

Thanks,

Jonathan


> > 
> > I've tested the change on hardware and everything works fine. Or do you
> > prefer me sending over a v8 instead?  
> 
> with that fixup, LGTM so:
> 
> Reviewed-by: Jose A. Perez de Azpillaga <azpijr@gmail.com>
>