[PATCH v1 07/15] iio: adc: ad7768-1: Add reset gpio

Jonathan Santos posted 15 patches 11 months, 1 week ago
There is a newer version of this series
[PATCH v1 07/15] iio: adc: ad7768-1: Add reset gpio
Posted by Jonathan Santos 11 months, 1 week ago
From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>

Depending on the controller, the default state of a gpio can vary. This
change excludes the probability that the dafult state of the ADC reset
gpio will be HIGH if it will be passed as reference in the deivcetree.

Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
---
 drivers/iio/adc/ad7768-1.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index 881446462ff5..f73b9aec8b0f 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -161,6 +161,7 @@ struct ad7768_state {
 	struct completion completion;
 	struct iio_trigger *trig;
 	struct gpio_desc *gpio_sync_in;
+	struct gpio_desc *gpio_reset;
 	const char *labels[ARRAY_SIZE(ad7768_channels)];
 	/*
 	 * DMA (thus cache coherency maintenance) may require the
@@ -441,6 +442,18 @@ static int ad7768_setup(struct ad7768_state *st)
 {
 	int ret;
 
+	st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
+						 GPIOD_OUT_LOW);
+	if (IS_ERR(st->gpio_reset))
+		return PTR_ERR(st->gpio_reset);
+
+	if (st->gpio_reset) {
+		gpiod_direction_output(st->gpio_reset, 1);
+		usleep_range(10, 15);
+		gpiod_direction_output(st->gpio_reset, 0);
+		usleep_range(10, 15);
+	}
+
 	/*
 	 * Two writes to the SPI_RESET[1:0] bits are required to initiate
 	 * a software reset. The bits must first be set to 11, and then
-- 
2.34.1
Re: [PATCH v1 07/15] iio: adc: ad7768-1: Add reset gpio
Posted by David Lechner 11 months, 1 week ago
On 1/7/25 9:25 AM, Jonathan Santos wrote:
> From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> 
> Depending on the controller, the default state of a gpio can vary. This
> change excludes the probability that the dafult state of the ADC reset
> gpio will be HIGH if it will be passed as reference in the deivcetree.
> 
> Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> ---
>  drivers/iio/adc/ad7768-1.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> index 881446462ff5..f73b9aec8b0f 100644
> --- a/drivers/iio/adc/ad7768-1.c
> +++ b/drivers/iio/adc/ad7768-1.c
> @@ -161,6 +161,7 @@ struct ad7768_state {
>  	struct completion completion;
>  	struct iio_trigger *trig;
>  	struct gpio_desc *gpio_sync_in;
> +	struct gpio_desc *gpio_reset;
>  	const char *labels[ARRAY_SIZE(ad7768_channels)];
>  	/*
>  	 * DMA (thus cache coherency maintenance) may require the
> @@ -441,6 +442,18 @@ static int ad7768_setup(struct ad7768_state *st)
>  {
>  	int ret;
>  
> +	st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
> +						 GPIOD_OUT_LOW);

Could be simplified by setting this to GPIOD_OUT_HIGH and drop

	gpiod_direction_output(st->gpio_reset, 1);

> +	if (IS_ERR(st->gpio_reset))
> +		return PTR_ERR(st->gpio_reset);
> +
> +	if (st->gpio_reset) {
> +		gpiod_direction_output(st->gpio_reset, 1);
> +		usleep_range(10, 15);
> +		gpiod_direction_output(st->gpio_reset, 0);
> +		usleep_range(10, 15);

prefer fsleep()

> +	}
> +

We can move the code below in an else since we don't need to do 2 resets.

>  	/*
>  	 * Two writes to the SPI_RESET[1:0] bits are required to initiate
>  	 * a software reset. The bits must first be set to 11, and then
Re: [PATCH v1 07/15] iio: adc: ad7768-1: Add reset gpio
Posted by Jonathan Cameron 11 months, 1 week ago
On Tue, 7 Jan 2025 17:40:53 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 1/7/25 9:25 AM, Jonathan Santos wrote:
> > From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> > 
> > Depending on the controller, the default state of a gpio can vary. This
> > change excludes the probability that the dafult state of the ADC reset
> > gpio will be HIGH if it will be passed as reference in the deivcetree.

devicetree

> > 
> > Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> > ---
> >  drivers/iio/adc/ad7768-1.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> > index 881446462ff5..f73b9aec8b0f 100644
> > --- a/drivers/iio/adc/ad7768-1.c
> > +++ b/drivers/iio/adc/ad7768-1.c
> > @@ -161,6 +161,7 @@ struct ad7768_state {
> >  	struct completion completion;
> >  	struct iio_trigger *trig;
> >  	struct gpio_desc *gpio_sync_in;
> > +	struct gpio_desc *gpio_reset;
> >  	const char *labels[ARRAY_SIZE(ad7768_channels)];
> >  	/*
> >  	 * DMA (thus cache coherency maintenance) may require the
> > @@ -441,6 +442,18 @@ static int ad7768_setup(struct ad7768_state *st)
> >  {
> >  	int ret;
> >  
> > +	st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
> > +						 GPIOD_OUT_LOW);  
> 
> Could be simplified by setting this to GPIOD_OUT_HIGH and drop
> 
> 	gpiod_direction_output(st->gpio_reset, 1);
> 
> > +	if (IS_ERR(st->gpio_reset))
> > +		return PTR_ERR(st->gpio_reset);
> > +
> > +	if (st->gpio_reset) {
> > +		gpiod_direction_output(st->gpio_reset, 1);

It's an output already, gpiod_set_value() should work I think?
Possibly even then sleep allowing variants


> > +		usleep_range(10, 15);
> > +		gpiod_direction_output(st->gpio_reset, 0);
> > +		usleep_range(10, 15);  
> 
> prefer fsleep()
> 
> > +	}
> > +  
> 
> We can move the code below in an else since we don't need to do 2 resets.
> 
> >  	/*
> >  	 * Two writes to the SPI_RESET[1:0] bits are required to initiate
> >  	 * a software reset. The bits must first be set to 11, and then  
>