linux-kernel - Re: [PATCH RFC v6 10/10] iio: adc: ad7380: add support for resolution boost

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]

Message-ID: <CAMknhBHOXaff__QyU-wFSNNENvs23vDX5n_ddH-Dw3s6-sQ9sg@mail.gmail.com>
Date: Mon, 6 May 2024 09:44:25 -0500
From: David Lechner <dlechner@...libre.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Nuno Sá <noname.nuno@...il.com>, 
	Julien Stephan <jstephan@...libre.com>, Lars-Peter Clausen <lars@...afoo.de>, 
	Michael Hennerich <Michael.Hennerich@...log.com>, Nuno Sá <nuno.sa@...log.com>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, 
	Conor Dooley <conor+dt@...nel.org>, Liam Girdwood <lgirdwood@...il.com>, 
	Mark Brown <broonie@...nel.org>, kernel test robot <lkp@...el.com>, linux-iio@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC v6 10/10] iio: adc: ad7380: add support for resolution boost

FYI, Julien is AFK for a bit so I'll try to respond to the non-trivial comments.

On Mon, May 6, 2024 at 8:46 AM Jonathan Cameron <jic23@...nel.org> wrote:
>
> On Mon, 06 May 2024 10:55:46 +0200
> Nuno Sá <noname.nuno@...il.com> wrote:
>
> > On Wed, 2024-05-01 at 16:55 +0200, Julien Stephan wrote:
> > > ad738x chips are able to use an additional 2 bits of resolution when
> > > using oversampling. The 14-bits chips can have up to 16 bits of
> > > resolution and the 16-bits chips can have up to 18 bits of resolution.
> > >
> > > In order to dynamically allow to enable/disable the resolution boost
> > > feature, we have to set iio realbits/storagebits to the maximum per chips.
> > > This means that for iio, data will always be on the higher resolution
> > > available, and to cope with that we adjust the iio scale and iio offset
> > > depending on the resolution boost status.
> > >
> > > The available scales can be displayed using the regular _scale_available
> > > attributes and can be set by writing the regular _scale attribute.
> > > The available scales depend on the oversampling status.
> > >
> > > Signed-off-by: Julien Stephan <jstephan@...libre.com>
> > >
> > > ---
> > >
> > > In order to support the resolution boost (additional 2 bits of resolution)
> > > we need to set realbits/storagebits to the maximum value i.e :
> > >   - realbits = 16 + 2 = 18, and storagebits = 32 for 16-bits chips
> > >   - realbits = 14 + 2 = 16, and storagebits = 16 for 14-bits chips
> > >
> > > For 14-bits chips this does not have a major impact, but this
> > > has the drawback of forcing 16-bits chip users to always use 32-bits
> > > words in buffers even if they are not using oversampling and resolution
> > > boost. Is there a better way of implementing this? For example
> > > implementing dynamic scan_type?
> > >
> >
> > Yeah, I don't think it's that bad in this case. But maybe dynamic scan types is
> > something we may need at some point yes (or IOW that I would like to see supported
> > :)). We do some ADCs (eg: ad4630) where we use questionably use FW properties to set
> > a specific operating mode exactly because we have a different data layout (scan
> > elements) depending on the mode.
>
> Yeah. Fixed scan modes were somewhat of a bad design decision a long time back.
> However, the big advantage is that it got people to think hard about whether it is
> worth supporting low precision modes. For slow devices it very rarely is and
> forcing people to make a decision and the advantage we never supported
> low precision on those parts.
>
> Having said that there are good reasons for dynamic resolution changing
> (the main one being the storage case you have here) so I'd be happy to
> see some patches adding it. It might be easier than I've always thought
> to bolt on.
>
> This and inkernel event consumers have been the two significant features
> where I keep expecting it to happen, but every time people decide they really
> don't care enough to make them work :(
>

Supposing we knew someone willing and able :-) ...

Do you have any specific requirements for how dynamic resolution
changing should work? Any particular sticky points we should watch out
for?

I'm assuming this would just affect the bufferY/*_type attributes,
i.e. if you write a channel scale attribute to change the resolution,
then the scan_type info may change and the bufferY/*_type would need
to be read again.