> From: Sa, Nuno <Nuno.Sa@xxxxxxxxxx>
> Sent: Monday, August 23, 2021 9:05 AM
> To: Alexandru Ardelean <ardeleanalex@xxxxxxxxx>
> 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]
>
>
>
> > -----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.
So, I did consulted internally about this and the status register is 0
initialized during the initial powert-up state which means we don't really
have to wait 100ms here....
Anyways, I will still send a v2 with a improved range for
'usleep_range()'.
- Nuno Sá
