Re: [PATCH 1/8] iio: adc: ad9467: fix reset gpio handling

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 05 Dec 2023 18:06:41 +0100
Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:

> From: Nuno Sa <nuno.sa@analog.com>
> 
> The reset gpio was being requested with GPIOD_OUT_LOW which means, not
> asserted. Then it was being asserted but never de-asserted which means
> the devices was left in reset. Fix it by de-asserting the gpio.

If I understand this correctly, it's really just inverted polarity
compared to the expectation?  If so just call it that rather than
discussing what happens in detail.

> 
> While at it, moved the handling to it's own function and dropped
> 'reset_gpio' from the 'struct ad9467_state' as we only need it during
> probe. On top of that, refactored things so that we now request the gpio
> asserted (i.e in reset) and then de-assert it. Also note that we now use
> gpiod_set_value_cansleep() instead of gpiod_direction_output() as we
> already request the pin as output.
> 
> Fixes: ad6797120238 ("iio: adc: ad9467: add support AD9467 ADC")
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> ---
>  drivers/iio/adc/ad9467.c | 33 ++++++++++++++++++++-------------
>  1 file changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
> index 39eccc28debe..5ecf486bf5d1 100644
> --- a/drivers/iio/adc/ad9467.c
> +++ b/drivers/iio/adc/ad9467.c
> @@ -121,7 +121,6 @@ struct ad9467_state {
>  	unsigned int			output_mode;
>  
>  	struct gpio_desc		*pwrdown_gpio;
> -	struct gpio_desc		*reset_gpio;
>  };
>  
>  static int ad9467_spi_read(struct spi_device *spi, unsigned int reg)
> @@ -378,6 +377,23 @@ static int ad9467_preenable_setup(struct adi_axi_adc_conv *conv)
>  	return ad9467_outputmode_set(st->spi, st->output_mode);
>  }
>  
> +static int ad9467_reset(struct device *dev)
> +{
> +	struct gpio_desc *gpio;
> +
> +	gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(gpio))
> +		return PTR_ERR(gpio);
> +	if (!gpio)
> +		return 0;
> +
> +	fsleep(1);
> +	gpiod_set_value_cansleep(gpio, 0);
> +	fsleep(10 * USEC_PER_MSEC);
> +
> +	return 0;
> +}
> +
>  static int ad9467_probe(struct spi_device *spi)
>  {
>  	const struct ad9467_chip_info *info;
> @@ -408,18 +424,9 @@ static int ad9467_probe(struct spi_device *spi)
>  	if (IS_ERR(st->pwrdown_gpio))
>  		return PTR_ERR(st->pwrdown_gpio);
>  
> -	st->reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset",
> -						 GPIOD_OUT_LOW);
> -	if (IS_ERR(st->reset_gpio))
> -		return PTR_ERR(st->reset_gpio);
> -
> -	if (st->reset_gpio) {
> -		udelay(1);
> -		ret = gpiod_direction_output(st->reset_gpio, 1);
> -		if (ret)
> -			return ret;
> -		mdelay(10);
> -	}
> +	ret = ad9467_reset(&spi->dev);
> +	if (ret)
> +		return ret;
>  
>  	conv->chip_info = &info->axi_adc_info;
>  
>