linux-kernel - Re: [PATCH 9/9] iio: adc: ad7606: fix standby gpio state to match the documentation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240623164920.48dda649@jic23-huawei>
Date: Sun, 23 Jun 2024 16:49:20 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Guillaume Stols <gstols@...libre.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich
 <Michael.Hennerich@...log.com>, Rob Herring <robh@...nel.org>, Krzysztof
 Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Beniamin Bia <beniamin.bia@...log.com>, Stefan Popa
 <stefan.popa@...log.com>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-fbdev@...r.kernel.org,
 devicetree@...r.kernel.org, Jonathan Cameron <Jonathan.Cameron@...wei.com>,
 jstephan@...libre.com, dlechner@...libre.com
Subject: Re: [PATCH 9/9] iio: adc: ad7606: fix standby gpio state to match
 the documentation

On Tue, 18 Jun 2024 14:02:41 +0000
Guillaume Stols <gstols@...libre.com> wrote:

> The binding's documentation specifies that "As the line is active low, it
> should be marked GPIO_ACTIVE_LOW". However, in the driver, it was handled
> the opposite way. This commit sets the driver's behaviour in sync with the
> documentation
> 
> Fixes: 722407a4e8c0 ("staging:iio:ad7606: Use GPIO descriptor API")
> Signed-off-by: Guillaume Stols <gstols@...libre.com>

This sound dangerous.  If anyone is using the driver before this an it's
working they indeed have the pin inverted wrt to the docs, but
as it works for them this will be a regression.

So messy corner - do we fix the docs or the driver?  I'm not sure which
is more painful. In theory the DT binding might be in use by another
OS or similar which might have a non broken driver, but I suspect it isn't.
Whereas perhaps the driver as it stands is in use on Linux.

AD folk: You will get the support calls, do you want to risk them or
should we change the docs (and maybe add a note on it being 'odd' wrt
to the documentation as we are treating it as an active !standy pin)
> ---
>  drivers/iio/adc/ad7606.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> index 502344e019e0..05addea105f0 100644
> --- a/drivers/iio/adc/ad7606.c
> +++ b/drivers/iio/adc/ad7606.c
> @@ -438,7 +438,7 @@ static int ad7606_request_gpios(struct ad7606_state *st)
>  		return PTR_ERR(st->gpio_range);
>  
>  	st->gpio_standby = devm_gpiod_get_optional(dev, "standby",
> -						   GPIOD_OUT_HIGH);
> +						   GPIOD_OUT_LOW);
>  	if (IS_ERR(st->gpio_standby))
>  		return PTR_ERR(st->gpio_standby);
>  
> @@ -681,7 +681,7 @@ static int ad7606_suspend(struct device *dev)
>  
>  	if (st->gpio_standby) {
>  		gpiod_set_value(st->gpio_range, 1);
> -		gpiod_set_value(st->gpio_standby, 0);
> +		gpiod_set_value(st->gpio_standby, 1);
>  	}
>  
>  	return 0;
>