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

[Jose A. Perez de Azpillaga]

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@xxxxxxxxx>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> Signed-off-by: Chang Yu <marcus.yu.56@xxxxxxxxx>


[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

> +			return ret ? ret : -EIO;

-EIO reads like a bus error. as73211 returns -EOVERFLOW for this,
wouldn't that fit better?

		if (status & AS7343_ASTATUS_ASAT_STATUS)
			return -EOVERFLOW;

-- 
cheers,
jose a. p-a