Re: [PATCH v2] staging: iio: adc: ad7816: Use devm_gpiod_get_optional() for busy GPIO

[Jonathan Cameron]

On Wed,  3 Jun 2026 12:33:33 +0000
Taha Narimani <tahanarimani3443@xxxxxxxxx> wrote:

> The driver currently utilizes devm_gpiod_get() for the 'busy' line,
> which makes the GPIO mandatory. However, the busy pin is hardware-optional
> depending on the specific board configuration.
> 
> Switch to devm_gpiod_get_optional() to allow boards that do not have
> this pin wired up to still probe the driver successfully, and remove
> the redundant conditional chip-ID check since the optional API handles
> missing descriptors gracefully.
> 
> Signed-off-by: Taha Narimani <tahanarimani3443@xxxxxxxxx>
I tried to pick this up, but it doesn't apply. I suspect that's because
you've put it on top of your previous patch which modified the checks on chip->id

Please send it as a single patch. Also this is fixing a false assumption in the
driver so it should have an appropriate Fixes tag.

Trivial comment inline.

Thanks,

Jonathan

> ---
> Changes in v2:
>   - Fixed trailing whitespace and missing newline at the end of the file.
>   - Converted the file format to Unix (LF) to remove carriage returns.
>   - Removed the explicit chip-ID check around the busy pin logic.
>   - Improved the commit message to provide clear architectural justification.
> 
>  drivers/staging/iio/adc/ad7816.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
> index 0eac484..039b34d 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -84,7 +84,7 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
>  		gpiod_set_value(chip->convert_pin, 1);
>  	}
>  
> -	if (chip->id == ID_AD7817) {
> +	if (chip->busy_pin) {
>  		while (gpiod_get_value(chip->busy_pin))
>  			cpu_relax();
>  	}
> @@ -380,15 +380,14 @@ static int ad7816_probe(struct spi_device *spi_dev)
>  			ret);
>  		return ret;
>  	}
> -	if (chip->id == ID_AD7817) {
> -		chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy",
> -						GPIOD_IN);
> -		if (IS_ERR(chip->busy_pin)) {
> -			ret = PTR_ERR(chip->busy_pin);
> -			dev_err(&spi_dev->dev, "Failed to request busy GPIO: %d\n",
> -				ret);
> -			return ret;
> -		}
> +
> +	chip->busy_pin = devm_gpiod_get_optional(&spi_dev->dev, "busy",
> +						 GPIOD_IN);
Trivial:  We are more relaxed on line lengths these days so for cases like this
where it would only go a little past 80 chars to have it on one line I would
generally prefer it that way.

> +	if (IS_ERR(chip->busy_pin)) {
> +		ret = PTR_ERR(chip->busy_pin);
> +		dev_err(&spi_dev->dev, "Failed to request busy GPIO: %d\n",
> +			ret);
> +		return ret;
>  	}
>  
>  	indio_dev->name = spi_get_device_id(spi_dev)->name;