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

From: Chang Yu

Date: Thu Sep 24 2026 - 20:52:13 EST


On Fri, Sep 25, 2026 at 12:02:34AM +0200, Jose A. Perez de Azpillaga wrote:
> On Sat, Sep 19, 2026 at 04:51:45PM -0700, Chang Yu wrote:
> > +static int as7343_read_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + int *val, int *val2, long mask)
> > +{
> > + struct as7343_data *data = iio_priv(indio_dev);
> > + struct regmap *map = data->regmap;
> > + struct device *dev = regmap_get_device(map);
> > + unsigned int unused;
> > + __le16 result;
> > + int ret;
> > +
> > + PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
> > + ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> > + if (ret)
> > + return ret;
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_RAW: {
> > + /* Wait until integration time passes for all 3 cycles. */
> > + msleep(160);
>
> three integration periods hardcoded next to a hardcoded integration
> time, so nothing keeps them in sync. make ATIME/ASTEP writable and the
> fixed wait no longer covers a readout. as73211, which this is based on,
> computes the timeout and polls NDATA. STATUS2 (0x90) bit 6 is AVALID is
> defined and never read, does it cover all three cycles? the datasheet
> does not say.
>
Configurable ATIME/ASTEP will be added in future patches since I want to
keep the initial driver lean. I'll switch to dynamically computing wait
time in those patches. For now I'm inclined to leave the value hardcoded
just to keep things simple.

The documentation on AVALID is indeed pretty poorly worded and unclear.
I'll test on hardware when I have time and see if it covers all three.
I think it's fine even if AVALID turns out to be unreliable? Worst case
scenario the userspace reads a stale value, which is OK for my use case
at least.