Re: [PATCH 1/2] iio: adc: ad4695: implement triggered buffer

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 13 Aug 2024 09:28:06 +0200
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Mon, 2024-08-12 at 12:03 -0500, David Lechner wrote:
> > On 8/10/24 4:35 AM, Jonathan Cameron wrote:  
> > > On Wed,  7 Aug 2024 15:02:10 -0500
> > > David Lechner <dlechner@baylibre.com> wrote:
> > >   
> > > > This implements buffered reads for the ad4695 driver using the typical
> > > > triggered buffer implementation, including adding a soft timestamp
> > > > channel.
> > > > 
> > > > The chip has 4 different modes for doing conversions. The driver is
> > > > using the advanced sequencer mode since that is the only mode that
> > > > allows individual configuration of all aspects each channel (e.g.
> > > > bipolar config currently and oversampling to be added in the future).
> > > > 
> > > > Signed-off-by: David Lechner <dlechner@baylibre.com>  
> > > 
> > > Main thing in here is I think you can use available_scan_masks
> > > to avoid the need for the error path on just the temperature channel
> > > being enabled.
> > >   
> > I had not thought about doing it that way, but now that I am
> > thinking about it, it seems like we would need to have a scan
> > mask in the list for every possible combination of channels.
> > This would be 10s of thousands of possible scan masks for 16
> > channel chips so that doesn't seem like the best way to go.

Indeed not my best suggestion.

> > 
> > But adding some special handling to make the temperature
> > channel just work should be easy enough to add.
> >   
> 
> Not sure if the following is meaningful to this usecase but I used to think like you
> but then realized that iio_scan_mask_match() will do bitmap_subset(). So you only
> need to enable a subset of the available scan mask for things to work (and with that
> you should no longer need an insane number of combinations). The core will then take
> care of demuxing the actual enabled channels. AFAIR, strict scan matching is only
> used for HW buffering.

Hmm. The validate_scan_mask callback also doesn't work as that's really about
restricting cases where we are onehot or similar (so restricting number of channels
or that sort of thing).

So, I think this is a driver problem to hide it.

Just have some logic in the driver that enables a dummy channel if only the
temperature is enabled and throw it away (probably fine to put it in the
data passed to iio_push_to_buffers() and rely on the it either being
treated as garbage, or dropped depending on whether it is in a hole, or
on the end of the scan.  That should give the intuitive interface
we expect (no restrictions to bite us that don't have to be there - see onehot
case where we have no choice) without adding too much complexity.

Jonathan

> 
> - Nuno Sá