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

[Marcelo Schmitt <marcelo.schmitt1@gmail.com>]

On 04/09, David Lechner wrote:
> On Tue, Apr 9, 2024 at 11:09 AM Marcelo Schmitt
> <marcelo.schmitt1@gmail.com> wrote:
> >
> > On 04/08, David Lechner wrote:
> > > On Mon, Apr 8, 2024 at 9:32 AM Marcelo Schmitt
> > > <marcelo.schmitt@analog.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.