Re: [PATCH v3 3/4] iio: adc: ad4691: add triggered buffer support

From: Andy Shevchenko

Date: Fri Mar 13 2026 - 10:40:45 EST


On Fri, Mar 13, 2026 at 12:09:55PM +0000, Sabau, Radu bogdan wrote:
> > -----Original Message-----
> > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> > Sent: Friday, March 13, 2026 11:13 AM
> > On Fri, Mar 13, 2026 at 12:07:27PM +0200, Radu Sabau via B4 Relay wrote:

...

> > > -#define AD4691_ACC_COUNT_VAL 0x3F
> > > +#define AD4691_ACC_COUNT_VAL 0x01
> >
> > No ping-pong, and actually this was not used at all. So, make sure you add
> > constants when they are really started being used.
>
> This value is being used in the buffer_postenable in order to make
> sure we don't encounter oversampling, since Manual Mode doesn't
> oversample, and per Jonathan's review, there is no reason to support
> both oversampled and raw readings at the same time.

Yes, but it wasn't used before this patch.

...

> > > +static int ad4691_setup_triggered_buffer(struct iio_dev *indio_dev,
> > > + struct ad4691_state *st)
> > > +{
> > > + struct device *dev = regmap_get_device(st->regmap);
> > > + struct spi_device *spi = to_spi_device(dev);
> > > + int irq, ret;
> > > +
> > > + st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
> > > + indio_dev->name,
> > > + iio_device_id(indio_dev));
> >
> > It seems you ignored some of my comments. Please go back and read carefully
> > what I commented on previous version of the series.
>
> I am very sorry for this. I may have misunderstood the comments, and thus
> seem like I ignored them, but this wasn't my intention at all.

When in such a situation, ask! A request is free and acceptable.

> If you refer to the trigger_alloc comment, I did talk to my senior colleagues
> and they referred to the fact -ENOMEM return should have been enough,
> but perhaps this was wrong too. Could you please clarify?

I'm talking about the room of the previous lines

st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name,
iio_device_id(indio_dev));

fits 80 limit, for example. OTOH this is less logical, perhaps your variant
is okay.

...

> Radu

When answering, remove the context you are agree with and not going to discuss.
I had to drop over 200 lines just "for fun" (no).

--
With Best Regards,
Andy Shevchenko