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]
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;
> 


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ