Re: [PATCH v6 3/4] iio: light: veml6031x00: add support for triggered buffers
From: Andy Shevchenko
Date: Thu Aug 13 2026 - 04:37:27 EST
On Thu, Aug 13, 2026 at 02:09:58AM +0100, Jonathan Cameron wrote:
> On Wed, 12 Aug 2026 22:27:42 +0200
> Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> wrote:
...
> > +static irqreturn_t veml6031x00_trig_handler(int irq, void *p)
> > +{
> > + struct iio_poll_func *pf = p;
> > + struct iio_dev *iio = pf->indio_dev;
> > + struct veml6031x00_data *data = iio_priv(iio);
> > + IIO_DECLARE_BUFFER_WITH_TS(__le16, scan, 2) = { };
> > + unsigned int i = 0;
> > + int ch, ret;
> > +
> > + if (test_bit(VEML6031X00_SCAN_ALS, iio->active_scan_mask) &&
> > + test_bit(VEML6031X00_SCAN_IR, iio->active_scan_mask)) {
> > + ret = regmap_bulk_read(data->regmap,
> > + VEML6031X00_REG_ALS_L,
> > + scan,
> > + 2 * sizeof(*scan));
> I don't care that much, but you could combine parameters on one line in
> a few more places to reduced the code length a little.
> e.g.
> I'd be tempted to put the two lines above on one line or even
>
> ret = regmap_bulk_read(data->regmap, VEML6031X00_REG_ALS_L,
> scan, 2 * sizeof(*scan));
I usually suggest to combine that with
struct regmap *map = data->regmap;
so it will become
ret = regmap_bulk_read(map, VEML6031X00_REG_ALS_L,
scan, 2 * sizeof(*scan));
> > + if (ret)
> > + goto done;
> > + } else {
> > + iio_for_each_active_channel(iio, ch) {
> > + ret = regmap_bulk_read(data->regmap,
> > + iio->channels[ch].address,
> > + &scan[i++],
> > + sizeof(*scan));
And here it will help
ret = regmap_bulk_read(map, iio->channels[ch].address,
&scan[i++], sizeof(*scan));
> > + if (ret)
> > + goto done;
> > + }
But I would even go with
ret = regmap_bulk_read(map, iio->channels[ch].address,
&scan[i], sizeof(scan[i]));
if (ret)
goto done;
i++;
> > + }
> > +
> > + iio_push_to_buffers_with_ts(iio, scan, sizeof(scan), pf->timestamp);
> > +
> > +done:
> > + iio_trigger_notify_done(iio->trig);
> > +
> > + return IRQ_HANDLED;
> > +}
--
With Best Regards,
Andy Shevchenko