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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20240814195604.6a871c6c@jic23-huawei>
Date: Wed, 14 Aug 2024 19:56:04 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Nuno Sá <noname.nuno@...il.com>
Cc: David Lechner <dlechner@...libre.com>, Michael Hennerich
 <Michael.Hennerich@...log.com>, Nuno Sá 
 <nuno.sa@...log.com>, Jonathan Corbet <corbet@....net>,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org
Subject: Re: [PATCH 1/2] iio: adc: ad4695: implement triggered buffer

On Tue, 13 Aug 2024 09:28:06 +0200
Nuno Sá <noname.nuno@...il.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@...libre.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@...libre.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á