lkml.org 
[lkml]   [2022]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH 5/5] iio: addac: ad74413r: add support for reset-gpio
Date


> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Saturday, November 12, 2022 7:07 PM
> To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Cc: Tanislav, Cosmin <Cosmin.Tanislav@analog.com>; Lars-Peter Clausen
> <lars@metafoo.de>; Hennerich, Michael <Michael.Hennerich@analog.com>;
> devicetree@vger.kernel.org; Rob Herring <robh+dt@kernel.org>; linux-
> iio@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 5/5] iio: addac: ad74413r: add support for reset-gpio
>
> [External]
>
> On Fri, 11 Nov 2022 15:39:21 +0100
> Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
>
> > We have a board where the reset pin of the ad74412 is connected to a
> > gpio, but also pulled low by default. Hence to get the chip out of
> > reset, the driver needs to know about that gpio and set it high before
> > attempting to communicate with it.
>
> I'm a little confused on polarity here. The pin is a !reset so
> we need to drive it low briefly to trigger a reset.
> I'm guessing for your board the pin is set to active low? (an example
> in the dt would have made that clearer) Hence the pulse
> in here to 1 is actually briefly driving it low before restoring to high?
>
> For a pin documented as !reset that seems backwards though you have
> called it reset so that is fine, but this description doesn't make that
> celar.

My opinion is that the driver shouldn't exactly know the polarity of the reset,
and just assume that setting the reset GPIO to 1 means putting it in reset,
and setting it to 0 means bringing out of reset.

>
> Perhaps just add some more description here to make it clear the GPIO
> is active low, and then refer to setting it to true and false to avoid
> the confusing high / low terminology which are inverted...
>
> >
> > When a reset-gpio is given in device tree, use that instead of the
> > software reset. According to the data sheet, the two methods are
> > functionally equivalent.
> >
> > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > ---
> > drivers/iio/addac/ad74413r.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
> > index 9f77d2f514de..af09d43f921c 100644
> > --- a/drivers/iio/addac/ad74413r.c
> > +++ b/drivers/iio/addac/ad74413r.c
> > @@ -71,6 +71,7 @@ struct ad74413r_state {
> > struct regmap *regmap;
> > struct device *dev;
> > struct iio_trigger *trig;
> > + struct gpio_desc *reset_gpio;
> >
> > size_t adc_active_channels;
> > struct spi_message adc_samples_msg;
> > @@ -393,6 +394,13 @@ static int ad74413r_reset(struct ad74413r_state
> *st)
> > {
> > int ret;
> >
> > + if (st->reset_gpio) {
> > + gpiod_set_value_cansleep(st->reset_gpio, 1);
> > + fsleep(50);
> > + gpiod_set_value_cansleep(st->reset_gpio, 0);
> > + return 0;
> > + }
> > +
> > ret = regmap_write(st->regmap, AD74413R_REG_CMD_KEY,
> > AD74413R_CMD_KEY_RESET1);
> > if (ret)
> > @@ -1316,6 +1324,10 @@ static int ad74413r_probe(struct spi_device *spi)
> > if (IS_ERR(st->regmap))
> > return PTR_ERR(st->regmap);
> >
> > + st->reset_gpio = devm_gpiod_get_optional(st->dev, "reset",
> GPIOD_OUT_LOW);
> > + if (IS_ERR(st->reset_gpio))
> > + return PTR_ERR(st->reset_gpio);
> > +
> > st->refin_reg = devm_regulator_get_optional(st->dev, "refin");
> > if (IS_ERR(st->refin_reg)) {
> > ret = PTR_ERR(st->refin_reg);

\
 
 \ /
  Last update: 2022-11-14 14:53    [W:0.096 / U:0.264 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site