Re: [PATCH 1/2] iio: ltc2983: add support for optional reset gpio

[Alexandru Ardelean <ardeleanalex@gmail.com>]

On Fri, Aug 20, 2021 at 12:29 PM Sa, Nuno <Nuno.Sa@analog.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Alexandru Ardelean <ardeleanalex@gmail.com>
> > Sent: Friday, August 20, 2021 10:21 AM
> > To: Sa, Nuno <Nuno.Sa@analog.com>
> > Cc: linux-iio <linux-iio@vger.kernel.org>; Jonathan Cameron
> > <jic23@kernel.org>; Lars-Peter Clausen <lars@metafoo.de>; Drew
> > Fustini <drew@pdp7.com>
> > Subject: Re: [PATCH 1/2] iio: ltc2983: add support for optional reset
> > gpio
> >
> > [External]
> >
> > On Fri, Aug 20, 2021 at 9:53 AM Nuno Sá <nuno.sa@analog.com>
> > wrote:
> > >
> > > Check if an optional reset gpio is present and if so, make sure to
> > reset
> > > the device.
> > >
> >
> > Just one note/question inline.
> >
> > > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > > ---
> > >  drivers/iio/temperature/ltc2983.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > > diff --git a/drivers/iio/temperature/ltc2983.c
> > b/drivers/iio/temperature/ltc2983.c
> > > index 3b4a0e60e605..37903e9fb90f 100644
> > > --- a/drivers/iio/temperature/ltc2983.c
> > > +++ b/drivers/iio/temperature/ltc2983.c
> > > @@ -1470,6 +1470,7 @@ static int ltc2983_probe(struct spi_device
> > *spi)
> > >  {
> > >         struct ltc2983_data *st;
> > >         struct iio_dev *indio_dev;
> > > +       struct gpio_desc *gpio;
> > >         const char *name = spi_get_device_id(spi)->name;
> > >         int ret;
> > >
> > > @@ -1494,6 +1495,16 @@ static int ltc2983_probe(struct spi_device
> > *spi)
> > >         if (ret)
> > >                 return ret;
> > >
> > > +       gpio = devm_gpiod_get_optional(&st->spi->dev, "reset",
> > GPIOD_OUT_HIGH);
> > > +       if (IS_ERR(gpio))
> > > +               return PTR_ERR(gpio);
> > > +
> > > +       if (gpio) {
> > > +               /* bring device out of reset */
> > > +               usleep_range(1000, 1005);
> > > +               gpiod_set_value_cansleep(gpio, 0);
> >
> > Datasheet mentions that it takes up to 100 ms for the device to fully
> > start-up.
> > It also mentions that the (command) status register will be
> > unavailable to the user before this point.
> > Page 16, Conversion State Details section, second paragraph.
> >
> > I think there should probably be a sleep here of 100 ms.
> >
> > Other than that change looks good.
> >
>
> In the setup function we do a polled read on the status register until
> we get the indication we are up. This was actually a fix sent recently
> [1].
>

Yes, I saw that.
But I did not have energy to look at it too in-depth [at that moment in time].
Apologies.

But the question is: is the statement on page 16 valid?
i.e. do we need to wait 100ms after the reset pin goes low? because it states:

In the first phase of the start-up state all critical analog circuits
are powered up.
This includes the LDO, reference, charge pump and ADCs.
***During this first phase, the com-mand status register will be
inaccessible to the user.***
***This phase takes a maximum of 100mS to complete. ***
Once this phase completes, the command status register will be
accessible and return
a value of 0x80 until the LTC2983 is completely initialized.

> [1]: https://patchwork.kernel.org/project/linux-iio/patch/20210811133220.190264-2-nuno.sa@analog.com/
> - Nuno Sá
>
> > > +       }
> > > +
> > >         ret = ltc2983_setup(st, true);
> > >         if (ret)
> > >                 return ret;
> > > --
> > > 2.33.0
> > >