linux-kernel - Re: [PATCH v3 2/6] iio: improve doc for available_scan

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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 25 Sep 2023 14:17:26 +0100
From:   Jonathan Cameron <Jonathan.Cameron@...wei.com>
To:     Matti Vaittinen <mazziesaccount@...il.com>
CC:     Jonathan Cameron <jic23@...nel.org>,
        Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Angel Iglesias <ang.iglesiasg@...il.com>,
        Andreas Klinger <ak@...klinger.de>,
        "Christophe JAILLET" <christophe.jaillet@...adoo.fr>,
        Benjamin Bara <bbara93@...il.com>, <linux-iio@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/6] iio: improve doc for available_scan_mask

On Mon, 25 Sep 2023 12:50:46 +0300
Matti Vaittinen <mazziesaccount@...il.com> wrote:

> On 9/24/23 18:59, Jonathan Cameron wrote:
> > On Fri, 22 Sep 2023 14:16:57 +0300
> > Matti Vaittinen <mazziesaccount@...il.com> wrote:
> >   
> >> The available_scan_mask is an array of bitmaps representing the channels
> >> which can be simultaneously enabled by the driver. In many cases the
> >> hardware can offer more channels than what the user is interested in
> >> obtaining. In such cases it may be preferred that only subset of
> >> channels are enabled, and driver reads only a subset of the channels from
> >> the hardware.
> >>
> >> Some devices can't support all channel combinations. For example the
> >> BM1390 pressure sensor must always read the pressure data in order to
> >> acknowledge the watermark IRQ, while reading temperature can be omitted.
> >> So, the available scan mask would be 'pressure and temperature' and
> >> 'pressure only'.
> >>
> >> When IIO seatchs for the scan mask it asks the driver to use, it will  
> > 
> > Spell check description.  searches  
> 
> Oh, right. Thanks!
> 
> >   
> >> pick the first suitable one from the 'available_scan_mask' array. Hence,
> >> ordering the masks in the array makes difference. We should 'prefer'
> >> reading just the pressure from the hardware (as it is cheaper operation
> >> than reading both pressure and temperature) over reading both pressure
> >> and temperature. Hence, we should set the 'only pressure' as first scan
> >> mask in available_scan_mask array. If we set the 'pressure and
> >> temperature' as first in array, then the 'only temperature' will never
> >> get used as 'pressure and temperature' can always serve the user's
> >> needs.
> >>
> >> Add (minimal) kerneldoc to the 'available_scan_mask' to hint the user
> >> that ordering of masks matters.
> >>
> >> Signed-off-by: Matti Vaittinen <mazziesaccount@...il.com>
> >> ---
> >>   include/linux/iio/iio.h | 4 +++-
> >>   1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> >> index 202e55b0a28b..7bfa1b9bc8a2 100644
> >> --- a/include/linux/iio/iio.h
> >> +++ b/include/linux/iio/iio.h
> >> @@ -556,7 +556,9 @@ struct iio_buffer_setup_ops {
> >>    *			and owner
> >>    * @buffer:		[DRIVER] any buffer present
> >>    * @scan_bytes:		[INTERN] num bytes captured to be fed to buffer demux
> >> - * @available_scan_masks: [DRIVER] optional array of allowed bitmasks
> >> + * @available_scan_masks: [DRIVER] optional array of allowed bitmasks. Sort the
> >> + *			   array in order of preference, the most preferred
> >> + *			   masks first.  
> > 
> > LGTM  
> 
> I'll try to spell check the commit message and then I treat this as an 
> ack. Please, let me know if it's not Ok.
Given I'll be picking these up anyway, not a formal Ack that you should carry
with the next version, but looks fine to me.

J

> 
> >>    * @masklength:		[INTERN] the length of the mask established from
> >>    *			channels
> >>    * @active_scan_mask:	[INTERN] union of all scan masks requested by buffers  
> >   
>