linux-kernel - Re: [PATCH v2 2/2] iio: adc: Add support for AD4000

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: <ZhWE-hQkRZ4JcJA6@debian-BULLSEYE-live-builder-AMD64>
Date: Tue, 9 Apr 2024 15:12:10 -0300
From: Marcelo Schmitt <marcelo.schmitt1@...il.com>
To: David Lechner <dlechner@...libre.com>
Cc: Marcelo Schmitt <marcelo.schmitt@...log.com>, lars@...afoo.de,
	Michael.Hennerich@...log.com, jic23@...nel.org, robh+dt@...nel.org,
	krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
	linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] iio: adc: Add support for AD4000

On 04/09, David Lechner wrote:
> On Tue, Apr 9, 2024 at 11:09 AM Marcelo Schmitt
> <marcelo.schmitt1@...il.com> wrote:
> >
> > On 04/08, David Lechner wrote:
> > > On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
> > > <marcelo.schmitt@...log.com> wrote:
> > > >
> 
..
> 
> 
> > > > +static void ad4000_config(struct ad4000_state *st)
> > > > +{
> > > > +       unsigned int reg_val;
> > > > +       int ret;
> > > > +
> > > > +       reg_val = FIELD_PREP(AD4000_TURBO, 1);
> > >
> > > Since the driver in it's current state can get anywhere near the max
> > > sample rate of ~1MSPS, I don't think it makes sense to enable turbo at
> > > this point.
> > >
> >
> > This is just enabling turbo at start up. If not enabling turbo during probe,
> > we would want(need?) to provide some interface for that, which might not be
> > much desired.
> >
> 
> TURBO is only needed to achieve the max sample rate of 500k/1M/2MSPS
> on the various chips by skipping powering down some circuitry between
> samples. We can't get anywhere close to that in Linux without some
> sort of SPI offloading. So, for now, we might as well leave it
> disabled and save some power.
> 

Humm, ad4000 datasheets don't really mention power usage differences for turbo
mode like ad7944 datasheet do. Though, they should be similar.
Yeah, will leave turbo disabled until providing offload support.

> 
> > > > +
> > > > +       st->pin_gain = AD4000_1_GAIN;
> > > > +       if (device_property_present(&spi->dev, "adi,gain-milli")) {
> > > > +               u32 val;
> > >
> > > Should it be an error if adi,gain-milli is set on non-adaq chips?
> >
> > Maybe. We should not change the scale if it's a chip that don't have the
> > amplifier in front of the ADC. I think the best handling would be to just
> > ignore adi,gain-milli if it's not an ADAQ device. Maybe better add a DT
> > constraint,
> >   - if:
> >       properties:
> >         compatible:
> >           contains:
> >             enum:
> >               - adi,adaq4001
> >               - adi,adaq4003
> >     then:
> >       properties:
> >         adi,gain-milli: false
> > ?
> 
> I think this is missing a not:, but otherwise yes this should be in
> the DT bindings.

Oops, yeap, was missing the not:.

> 
> Even with that though, I would still be helpful to readers of the
> driver to at least have a comment here pointing out that this property
> and related gain scaling only applies to ADAQ chips.

ok, will add a comment.