> -----Original Message-----
> From: Alexandru Ardelean <ardeleanalex@xxxxxxxxx>
> Sent: Friday, August 20, 2021 8:59 PM
> To: Sa, Nuno <Nuno.Sa@xxxxxxxxxx>
> Cc: linux-iio <linux-iio@xxxxxxxxxxxxxxx>; Jonathan Cameron
> <jic23@xxxxxxxxxx>; Lars-Peter Clausen <lars@xxxxxxxxxx>; Drew
> Fustini <drew@xxxxxxxx>
> Subject: Re: [PATCH 1/2] iio: ltc2983: add support for optional reset
> gpio
>
> On Fri, Aug 20, 2021 at 12:29 PM Sa, Nuno <Nuno.Sa@xxxxxxxxxx>
> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Alexandru Ardelean <ardeleanalex@xxxxxxxxx>
> > > Sent: Friday, August 20, 2021 10:21 AM
> > > To: Sa, Nuno <Nuno.Sa@xxxxxxxxxx>
> > > Cc: linux-iio <linux-iio@xxxxxxxxxxxxxxx>; Jonathan Cameron
> > > <jic23@xxxxxxxxxx>; Lars-Peter Clausen <lars@xxxxxxxxxx>; Drew
> > > Fustini <drew@xxxxxxxx>
> > > 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@xxxxxxxxxx>
> > > 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@xxxxxxxxxx>
> > > > ---
> > > > 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.
Yes, I saw that on the datasheet. My interpretation is that the status
register is just not available so we just get 0x00 or 0xff (whatever the
default state of the SDO line). In my tests, it took 3 iterations to acknowledge
the device as up. Reading 0x00, 0x80 and 0x40...
Being this said, let's see if someone else has something to say about this.
I'm more than fine in adding the 100ms as I also wondered about this to
be honest.
- Nuno Sá
