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

[Chang Yu]

On Sun, Sep 13, 2026 at 03:47:14AM +0100, Jonathan Cameron wrote:
> On Fri, 11 Sep 2026 18:39:12 -0700
> Chang Yu <marcus.yu.56@xxxxxxxxx> 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);
> > +	unsigned int unused;
> > +	struct regmap *map;
> 	struct regmap *map = data->regmap;
> 	struct device *dev = regmap_get_device(map);
> 
> Neither is checked so no point in waiting until a few lines
> later to initialize them.
> 

I was trying to preserve the reverse christmas tree order and
was unsure how the rules apply here. Is the following OK:

  struct as7343_data *data = iio_priv(indio_dev);
  struct regmap *map = data->regmap;
  struct device *dev = regmap_get_device(map);

  struct device *dev;
  __le16 result;
  int ret;

> ...
> > +	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);
> > +
> > +		/*
> > +		 * Reading ASTATUS latches all data registers to this read.
> > +		 * We don't care about the returned saturation/gain status for
> > +		 * now.
> 
> Why do we care given only reading one channel and...
> 
> > +		 */
> > +		guard(mutex)(&data->mutex);
> > +		ret = regmap_read(map, AS7343_ASTATUS, &unused);
> > +		if (ret)
> > +			return ret;
> > +
> > +		ret = regmap_bulk_read(map, chan->address,
> .. it seems that if you read the low byte first (as this does) it is latched anyway.
> There is a statement about this in the i2c intro part section 9.
> 
> so we get a consistent register pair.  Thus not needing the latch astatus
> gives us.
> With that in place reads can't stomp on each other as only
> one regmap read is required, so the mutex isn't needed either.
> It may be necessary once you add more features - hard to tell yet.
> 
When testing on hardware I discovered, for whatever reason, register
values for other channels won't refresh unless I read ASTATUS or the
FZ channel first. The only thing I can find in the datasheet pertaining
to this is 10.2.7 page 41.

"Reading the ASTATUS register (0x94) latches all 36 spectral data bytes
to that status read. Reading these bytes consecutively (0x94 to 0xB8)
ensures that the data is concurrent."

My most charitable interpretation is that by "latches" they also mean
"updates". I have no idea why reading FZ also works though. Could be
a hardware bug since ASTATUS (0x94) and FZ DATA_0_L (0x95) are next
to each other.