RE: [PATCH v12 5/6] iio: adc: ad4691: add oversampling support

[Sabau, Radu bogdan]

> -----Original Message-----
> From: Jonathan Cameron <jic23@xxxxxxxxxx>
> Sent: Friday, May 22, 2026 2:16 PM

...

> > >
> > > +	iio_for_each_active_channel(indio_dev, bit) {
> > > +		ret = regmap_write(st->regmap,
> > > AD4691_ACC_DEPTH_IN(bit), st->osr[bit]);
> >
> > Unfortunately enough, I think a v13 will come, too...
> >
> > Had a look again on what Sashiko had to say, and seeing the sampling
> frequency
> > shared_by_all comment again made me have a deeper look see how the
> code could
> > be commented so he wouldn't complain about this anymore, and...
> >
> > Perhaps he is a bit right after all. I found a section stating that in standard
> > sequencer mode (which the driver uses right now), all the channels actually
> use
> > the ACC_DEPTH_IN0 for osr, and so changing ACC_DEPTH_INn for other
> channels
> > doesn't really do much. And so I tested this selecting both voltage0 and
> voltage1
> > for sampling with osr4 for voltage0 and osr1 for voltage1 and with a 100kHz
> osc freq
> > indeed DR fell after approximately 80us which points out both channels were
> actually
> > using OSR of 4. Perhaps the OSR should be shared by all and therefore the
> > sampling frequency would also be shared by all, right?
> 
> I kind of lost track on the modes. What are the chances we later move to or
> add
> support for a mode where the different OSRs do matter?  If that's a possibility
> we should avoid ABI change by allowing for it from the start.
> 
> Then if we are in this mode, they'll have separate controls but change any,
> changes
> them all, if we are in a different mode that connection breaks.
> If that's the case, just throw in a comment saying something to the effect this
> may change.
> 
> It's not wrong ABI to do this, it's just less intuitive for users which is why
> we prefer the shared_by stuff where there isn't a disadvantage.  That is at
> most
> a hint to what actually happens.   A simple example is where different
> channels have one OSR field but they aren't the same - i.e. channel 1 is twice
> the OSR of channel 2.  Hence we can't share the attribute but any change
> effects
> both.
> 

Hi Jonathan,

I don't think a mode where different OSR will matter will be added in the future. Better
yet, this advanced sequencer functionality is not really mode dependent and is actually
something that allows you to manually rearrange channels and samples in the
sequence, and unless this functionality is active (it is not by default nor is it used by
the driver, since we use the standard sequencer).

Personally, I don't see any reason to have this advanced sequencer stuff implemented
since DR is only falling at the end of the sequence no matter if it is standard config or not,
the only "disadvantage" to say so is that the standard sequencer uses the same OSR field
for all channels. But that advanced sequencer stuff would only complicate the buffer
enable/disable functions even more, which I don't think it's worth the effort.

So, with this in mind. Letting the driver use standard sequencer would ultimately mean
that the osr would be the same for all the channels, and then effective rate the same for
all channels, which I suggest having it like that from initial driver patch to the end, so no
ABI change mid-patch series. This change will simplify the driver.

Radu