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

[Jose A. Perez de Azpillaga]

On 10/13/26 1:56 PM, Chang Yu wrote:

found a few things before I can give a tag, correct me if I'm wrong.

> The driver exposes 12 spectral channels (11 visible light and 1
> near-infrared) via sysfs. Runtime PM is implemented to stop
> measurements when the device is suspended or torn down. Power is
> never cut (PON=1 always) to preserve register values.

PON=0 is SLEEP, not power off. per the datasheet, entering SLEEP doesn't
change register settings. idle is 40 uA vs 0.7 uA in sleep. worth
clearing PON in the teardown action, and probably runtime suspend too.

if this goes in as v6, Jonathan, could you drop the "to preserve
register values" part of the commit message when applying?

[snip]

> +/* AS7343 channel indices. MUST match data register order above. */

nothing depends on this. drop the MUST, just say it follows the DATA
register order for auto_smux = 3.

[snip]

> +static inline int as7343_atime_steps(unsigned int step, u8 *atime)

no inline in .c files. same for the other helper.

[snip]

> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW: {
> +		/* Wait until integration time passes for all 3 cycles. */
> +		msleep(160);

derive this from the ATIME/ASTEP defaults, or spell out the math.

> +
> +		/*
> +		 * Reading ASTATUS latches all data registers to this read.
> +		 * We don't care about the returned saturation/gain status for
> +		 * now.
> +		 */
> +		guard(mutex)(&data->mutex);
> +
> +		ret = regmap_read(map, AS7343_ASTATUS_REG, &unused);

ASAT_STATUS (bit 7) is thrown away, so saturated counts get returned as
valid, while the datasheet says to discard them. ideally fixed before
this hits a release, since changing what read_raw returns after that is
user-visible. Jonathan, any preference on what it should return then?

[snip]

> +	/*
> +	 * Setting a 600 * 2.78us * 30 = 50ms integration time as the default
> +	 * for now.
> +	 */

cite the datasheet's typical config, that also answers Andy's "magic 600".

-- 
cheers,
jose a. p-a