[RESEND PATCH] iio: adis: set GPIO reset pin direction

[RESEND PATCH] iio: adis: set GPIO reset pin direction

[Antti Keränen]

Use GPIOD_OUT_LOW instead of GPIOD_ASIS as the reset pin needs to be an
active low output pin.

Suggested-by: Hannu Hartikainen <hannu@hrtk.in>
Signed-off-by: Antti Keränen <detegr@rbx.email>
---
The documentation of GPIO consumer interface states:

Be aware that there is no default direction for GPIOs. Therefore,
**using a GPIO without setting its direction first is illegal and will
result in undefined behavior!**

Therefore the direction of the reset GPIO pin should be set as output.

 drivers/iio/imu/adis.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 319b64b2fd88..7f13b3763732 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -415,7 +415,7 @@ int __adis_initial_startup(struct adis *adis)
 	int ret;
 
 	/* check if the device has rst pin low */
-	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset", GPIOD_ASIS);
+	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset", GPIOD_OUT_LOW);
 	if (IS_ERR(gpio))
 		return PTR_ERR(gpio);
 
-- 
2.31.1

Re: [RESEND PATCH] iio: adis: set GPIO reset pin direction

[Lars-Peter Clausen]

On 7/6/21 11:29 AM, Antti Keränen wrote:
> Use GPIOD_OUT_LOW instead of GPIOD_ASIS as the reset pin needs to be an
> active low output pin.
>
> Suggested-by: Hannu Hartikainen <hannu@hrtk.in>
> Signed-off-by: Antti Keränen <detegr@rbx.email>

Hi,

Thanks for the patch, this looks good.

How about requesting it as GPIOD_OUT_HIGH and removing the 
gpiod_set_value_cansleep(gpio, 1) to avoid unnecessary toggling of the pin.

> ---
> The documentation of GPIO consumer interface states:
>
> Be aware that there is no default direction for GPIOs. Therefore,
> **using a GPIO without setting its direction first is illegal and will
> result in undefined behavior!**
>
> Therefore the direction of the reset GPIO pin should be set as output.
>
>   drivers/iio/imu/adis.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index 319b64b2fd88..7f13b3763732 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -415,7 +415,7 @@ int __adis_initial_startup(struct adis *adis)
>   	int ret;
>   
>   	/* check if the device has rst pin low */
> -	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset", GPIOD_ASIS);
> +	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset", GPIOD_OUT_LOW);
>   	if (IS_ERR(gpio))
>   		return PTR_ERR(gpio);
>   

RE: [RESEND PATCH] iio: adis: set GPIO reset pin direction

[Sa, Nuno]

> From: Antti Keränen <detegr@rbx.email>
> Sent: Tuesday, July 6, 2021 11:29 AM
> To: linux-iio@vger.kernel.org
> Cc: Antti Keränen <detegr@rbx.email>; Hannu Hartikainen
> <hannu@hrtk.in>; Lars-Peter Clausen <lars@metafoo.de>; Hennerich,
> Michael <Michael.Hennerich@analog.com>; Sa, Nuno
> <Nuno.Sa@analog.com>; Jonathan Cameron <jic23@kernel.org>
> Subject: [RESEND PATCH] iio: adis: set GPIO reset pin direction
> 
> Use GPIOD_OUT_LOW instead of GPIOD_ASIS as the reset pin needs
> to be an
> active low output pin.
> 
> Suggested-by: Hannu Hartikainen <hannu@hrtk.in>
> Signed-off-by: Antti Keränen <detegr@rbx.email>
> ---
> The documentation of GPIO consumer interface states:
> 
> Be aware that there is no default direction for GPIOs. Therefore,
> **using a GPIO without setting its direction first is illegal and will
> result in undefined behavior!**
> 
> Therefore the direction of the reset GPIO pin should be set as output.
> 
>  drivers/iio/imu/adis.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index 319b64b2fd88..7f13b3763732 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -415,7 +415,7 @@ int __adis_initial_startup(struct adis *adis)
>  	int ret;
> 
>  	/* check if the device has rst pin low */
> -	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset",
> GPIOD_ASIS);
> +	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset",
> GPIOD_OUT_LOW);
>  	if (IS_ERR(gpio))
>  		return PTR_ERR(gpio);
> 

Hi,

Thanks for the patch. Forcing the device reset was intentional
(thus the GPIO_ASIS). But what Lars is suggesting is a good idea
and a neat improvement here.

- Nuno Sá

RE: [RESEND PATCH] iio: adis: set GPIO reset pin direction

[Hannu Hartikainen]

Hi!

Thanks for reviewing the patch. I'll also chime in. We were working on
an ADIS device together with Antti so I've read some of the relevant
code, documentation and datasheets.

On Wed, 7 Jul 2021 08:36:47 +0000, "Sa, Nuno" <Nuno.Sa@analog.com> wrote:
> Thanks for the patch. Forcing the device reset was intentional
> (thus the GPIO_ASIS). But what Lars is suggesting is a good idea
> and a neat improvement here.

I don't understand what you mean. The GPIO consumer documentation[0]
states that if GPIO_ASIS is used, the pin direction must be set in
driver code later. AFAICT that doesn't happen.

If a pin was defined as input by default by the manufacturer, I don't
think there's a way to make an ADIS device work with RST on it without
patching the driver. Device trees couldn't be used to do that IIRC. I.e.
this patch is needed so the device reset works.

On Wed, 7 Jul 2021 10:18:57 +0200, Lars-Peter Clausen <lars@metafoo.de> wrote:
> How about requesting it as GPIOD_OUT_HIGH and removing the 
> gpiod_set_value_cansleep(gpio, 1) to avoid unnecessary toggling of the pin.

GPIOD_OUT_LOW and GPIOD_OUT_HIGH have different semantics.[1] Is setting
the pin to use wrong semantics to save one line of code and possibly
toggle the pin one time less worth it? (The ADIS devices whose
datasheets I've read have the RST pin as active low, ie. GPIOD_OUT_LOW
is semantically correct.)

If we really want that, I think the better choice is to use GPIO_ASIS,
gpiod_direction_output and gpiod_set_raw_value_cansleep and skip
semantically describing the pin altogether.

Hannu

[0]: https://www.kernel.org/doc/html/latest/driver-api/gpio/consumer.html
> GPIOD_ASIS or 0 to not initialize the GPIO at all. The direction must
> be set later with one of the dedicated functions.
[1]: https://www.kernel.org/doc/html/latest/driver-api/gpio/consumer.html#the-active-low-and-open-drain-semantics

Re: [RESEND PATCH] iio: adis: set GPIO reset pin direction

[Lars-Peter Clausen]

On 7/7/21 1:53 PM, Hannu Hartikainen wrote:
> [...]
> GPIOD_OUT_LOW and GPIOD_OUT_HIGH have different semantics.[1] Is setting
> the pin to use wrong semantics to save one line of code and possibly
> toggle the pin one time less worth it? (The ADIS devices whose
> datasheets I've read have the RST pin as active low, ie. GPIOD_OUT_LOW
> is semantically correct.)

GPIOD_OUT_LOW does not mean that the GPIO is active low. OUT_LOW just 
means that the GPIO is configured as output in the non-asserted 
state[1]. Whether it is active low or active high is configured through 
the flags associated with the GPIO descriptor. E.g. when using 
devicetree this is typically the field after the GPIO offset.

>
> If we really want that, I think the better choice is to use GPIO_ASIS,
> gpiod_direction_output and gpiod_set_raw_value_cansleep and skip
> semantically describing the pin altogether.
Requesting as output is the right solution.


[1] 
https://www.kernel.org/doc/html/latest/driver-api/gpio/consumer.html#obtaining-and-disposing-gpios

Re: [RESEND PATCH] iio: adis: set GPIO reset pin direction

[Hannu Hartikainen]

On Wed, 7 Jul 2021 14:25:06 +0200, Lars-Peter Clausen <lars@metafoo.de> wrote:
> GPIOD_OUT_LOW does not mean that the GPIO is active low. OUT_LOW just 
> means that the GPIO is configured as output in the non-asserted 
> state[1]. Whether it is active low or active high is configured through 
> the flags associated with the GPIO descriptor. E.g. when using 
> devicetree this is typically the field after the GPIO offset.

Oh, I misunderstood that. Thanks for pointing it out and sorry for the
confusion!

Your original suggestion of using GPIOD_OUT_HIGH sounds good to me,
then.

Hannu

[PATCH v2] iio: adis: set GPIO reset pin direction

[Antti Keränen]

Set reset pin direction to output as the reset pin needs to be an active
low output pin.

Co-developed-by: Hannu Hartikainen <hannu@hrtk.in>
Signed-off-by: Hannu Hartikainen <hannu@hrtk.in>
Signed-off-by: Antti Keränen <detegr@rbx.email>
---
Removed unnecessary toggling of the pin as requested by Lars-Peter. I
missed out on the conversation, but I agree this is better.

 drivers/iio/imu/adis.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 319b64b2fd88..f8b7837d8b8f 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -415,12 +415,11 @@ int __adis_initial_startup(struct adis *adis)
 	int ret;
 
 	/* check if the device has rst pin low */
-	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset", GPIOD_ASIS);
+	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(gpio))
 		return PTR_ERR(gpio);
 
 	if (gpio) {
-		gpiod_set_value_cansleep(gpio, 1);
 		msleep(10);
 		/* bring device out of reset */
 		gpiod_set_value_cansleep(gpio, 0);
-- 
2.32.0

RE: [PATCH v2] iio: adis: set GPIO reset pin direction

[Sa, Nuno]

> -----Original Message-----
> From: Antti Keränen <detegr@rbx.email>
> Sent: Thursday, July 8, 2021 11:54 AM
> To: linux-iio@vger.kernel.org
> Cc: Antti Keränen <detegr@rbx.email>; Hannu Hartikainen
> <hannu@hrtk.in>; Lars-Peter Clausen <lars@metafoo.de>; Hennerich,
> Michael <Michael.Hennerich@analog.com>; Sa, Nuno
> <Nuno.Sa@analog.com>; Jonathan Cameron <jic23@kernel.org>;
> open list <linux-kernel@vger.kernel.org>
> Subject: [PATCH v2] iio: adis: set GPIO reset pin direction
> 
> Set reset pin direction to output as the reset pin needs to be an active
> low output pin.
> 
> Co-developed-by: Hannu Hartikainen <hannu@hrtk.in>
> Signed-off-by: Hannu Hartikainen <hannu@hrtk.in>
> Signed-off-by: Antti Keränen <detegr@rbx.email>
> ---
> Removed unnecessary toggling of the pin as requested by Lars-Peter. I
> missed out on the conversation, but I agree this is better.
> 
>  drivers/iio/imu/adis.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index 319b64b2fd88..f8b7837d8b8f 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -415,12 +415,11 @@ int __adis_initial_startup(struct adis *adis)
>  	int ret;
> 
>  	/* check if the device has rst pin low */
> -	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset",
> GPIOD_ASIS);
> +	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset",
> GPIOD_OUT_HIGH);
>  	if (IS_ERR(gpio))
>  		return PTR_ERR(gpio);
> 
>  	if (gpio) {
> -		gpiod_set_value_cansleep(gpio, 1);
>  		msleep(10);
>  		/* bring device out of reset */
>  		gpiod_set_value_cansleep(gpio, 0);
> --

Reviewed-by: Nuno Sá <nuno.sa@analog.com> 


Thanks!

- Nuno Sá

Re: [PATCH v2] iio: adis: set GPIO reset pin direction

[Jonathan Cameron]

On Thu,  8 Jul 2021 12:54:29 +0300
Antti Keränen <detegr@rbx.email> wrote:

> Set reset pin direction to output as the reset pin needs to be an active
> low output pin.
> 
> Co-developed-by: Hannu Hartikainen <hannu@hrtk.in>
> Signed-off-by: Hannu Hartikainen <hannu@hrtk.in>
> Signed-off-by: Antti Keränen <detegr@rbx.email>

Hi Antti,

For future reference (in IIO anyway as not all maintainers agree on this!)
please don't send a v2 in reply to v1.  It gets lots if anyone is using
a threaded email client as it's buried deep.  If I'd not been following
the discussion I'd probably have missed this.

Seems correct to me, but will leave on list a few more days as rc1 isn't
out yet so I don't have a good based to start collecting fixes on a the
moment.

Jonathan

> ---
> Removed unnecessary toggling of the pin as requested by Lars-Peter. I
> missed out on the conversation, but I agree this is better.
> 
>  drivers/iio/imu/adis.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index 319b64b2fd88..f8b7837d8b8f 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -415,12 +415,11 @@ int __adis_initial_startup(struct adis *adis)
>  	int ret;
>  
>  	/* check if the device has rst pin low */
> -	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset", GPIOD_ASIS);
> +	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset", GPIOD_OUT_HIGH);
>  	if (IS_ERR(gpio))
>  		return PTR_ERR(gpio);
>  
>  	if (gpio) {
> -		gpiod_set_value_cansleep(gpio, 1);
>  		msleep(10);
>  		/* bring device out of reset */
>  		gpiod_set_value_cansleep(gpio, 0);

Re: [PATCH v2] iio: adis: set GPIO reset pin direction

[Jonathan Cameron]

On Thu,  8 Jul 2021 12:54:29 +0300
Antti Keränen <detegr@rbx.email> wrote:

> Set reset pin direction to output as the reset pin needs to be an active
> low output pin.
> 
> Co-developed-by: Hannu Hartikainen <hannu@hrtk.in>
> Signed-off-by: Hannu Hartikainen <hannu@hrtk.in>
> Signed-off-by: Antti Keränen <detegr@rbx.email>

So this sits on the boundary of whether it is a fix or not.
Do we want this to go into rc1 + stable?

If so a fixes tag would be great.

Thanks,

Jonathan

> ---
> Removed unnecessary toggling of the pin as requested by Lars-Peter. I
> missed out on the conversation, but I agree this is better.
> 
>  drivers/iio/imu/adis.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index 319b64b2fd88..f8b7837d8b8f 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -415,12 +415,11 @@ int __adis_initial_startup(struct adis *adis)
>  	int ret;
>  
>  	/* check if the device has rst pin low */
> -	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset", GPIOD_ASIS);
> +	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset", GPIOD_OUT_HIGH);
>  	if (IS_ERR(gpio))
>  		return PTR_ERR(gpio);
>  
>  	if (gpio) {
> -		gpiod_set_value_cansleep(gpio, 1);
>  		msleep(10);
>  		/* bring device out of reset */
>  		gpiod_set_value_cansleep(gpio, 0);

Re: [PATCH v2] iio: adis: set GPIO reset pin direction

[Antti Keränen]

On Tue, Jul 13, 2021 at 06:53:11PM +0100, Jonathan Cameron wrote:
> So this sits on the boundary of whether it is a fix or not.
> Do we want this to go into rc1 + stable?

I'm not familiar enough with kernel development to give any opinions of
where this should go, but I would say this is a fix as the current code
won't work with certain hardware configurations.

> If so a fixes tag would be great.

This would mean a v3 patch with fixes tag included into the commit
message, right?

-- 
Antti

Re: [PATCH v2] iio: adis: set GPIO reset pin direction

[Jonathan Cameron]

On Wed, 14 Jul 2021 13:04:45 +0300
Antti Keränen <detegr@rbx.email> wrote:

> On Tue, Jul 13, 2021 at 06:53:11PM +0100, Jonathan Cameron wrote:
> > So this sits on the boundary of whether it is a fix or not.
> > Do we want this to go into rc1 + stable?  
> 
> I'm not familiar enough with kernel development to give any opinions of
> where this should go, but I would say this is a fix as the current code
> won't work with certain hardware configurations.
> 
> > If so a fixes tag would be great.  
> 
> This would mean a v3 patch with fixes tag included into the commit
> message, right?
> 

Send the fixes tag in reply to this and I'll make sure it's added.
No need to bother with a v3 for just that.

Thanks,

Jonathan

Re: [PATCH v2] iio: adis: set GPIO reset pin direction

[Antti Keränen]

On Wed, Jul 14, 2021 at 01:40:05PM +0100, Jonathan Cameron wrote:
> Send the fixes tag in reply to this and I'll make sure it's added.
> No need to bother with a v3 for just that.

Fixes: ecb010d44108 ("iio: imu: adis: Refactor adis_initial_startup")

-- 
Antti

Re: [PATCH v2] iio: adis: set GPIO reset pin direction

[Jonathan Cameron]

On Wed, 14 Jul 2021 21:25:07 +0300
Antti Keränen <detegr@rbx.email> wrote:

> On Wed, Jul 14, 2021 at 01:40:05PM +0100, Jonathan Cameron wrote:
> > Send the fixes tag in reply to this and I'll make sure it's added.
> > No need to bother with a v3 for just that.  
> 
> Fixes: ecb010d44108 ("iio: imu: adis: Refactor adis_initial_startup")
Great.

Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks,

Jonathan

> 

RE: [RESEND PATCH] iio: adis: set GPIO reset pin direction

[Sa, Nuno]

Hi,

> From: Hannu Hartikainen <hannu@hrtk.in>
> Sent: Wednesday, July 7, 2021 1:53 PM
> To: Sa, Nuno <Nuno.Sa@analog.com>; Antti Keränen
> <detegr@rbx.email>; linux-iio@vger.kernel.org
> Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Jonathan Cameron
> <jic23@kernel.org>
> Subject: RE: [RESEND PATCH] iio: adis: set GPIO reset pin direction
> 
> Hi!
> 
> Thanks for reviewing the patch. I'll also chime in. We were working on
> an ADIS device together with Antti so I've read some of the relevant
> code, documentation and datasheets.
> 
> On Wed, 7 Jul 2021 08:36:47 +0000, "Sa, Nuno"
> <Nuno.Sa@analog.com> wrote:
> > Thanks for the patch. Forcing the device reset was intentional
> > (thus the GPIO_ASIS). But what Lars is suggesting is a good idea
> > and a neat improvement here.
> 
> I don't understand what you mean. The GPIO consumer
> documentation[0]
> states that if GPIO_ASIS is used, the pin direction must be set in
> driver code later. AFAICT that doesn't happen.
> 
> If a pin was defined as input by default by the manufacturer, I don't
> think there's a way to make an ADIS device work with RST on it without
> patching the driver. Device trees couldn't be used to do that IIRC. I.e.
> this patch is needed so the device reset works.

Yes, you're right. This is pretty much assuming that the pin is already an output
one. Though you can typically make sure that the pin will be configured as
an output one (through pinctrl) which is what we are doing in the devicetree
overlays for the adis16475 device for example (in ADI trees).

> On Wed, 7 Jul 2021 10:18:57 +0200, Lars-Peter Clausen
> <lars@metafoo.de> wrote:
> > How about requesting it as GPIOD_OUT_HIGH and removing the
> > gpiod_set_value_cansleep(gpio, 1) to avoid unnecessary toggling of
> the pin.
> 
> GPIOD_OUT_LOW and GPIOD_OUT_HIGH have different
> semantics.[1] Is setting
> the pin to use wrong semantics to save one line of code and possibly
> toggle the pin one time less worth it? (The ADIS devices whose
> datasheets I've read have the RST pin as active low, ie.
> GPIOD_OUT_LOW
> is semantically correct.)

Well, AFAIK all the ADIS devices so far have the RST pin active low
so this a very fair assumption to make and use GPIOD_OUT_HIGH.
It can always be changed afterwards or even add a flag to the adis_data
structure...

> If we really want that, I think the better choice is to use GPIO_ASIS,
> gpiod_direction_output and gpiod_set_raw_value_cansleep and skip
> semantically describing the pin altogether.

Yes, we want to make sure a reset is done on the device. So, personally,
I'm fine with either approach. Either using GPIOD_OUT_HIGH or GPIO_ASIS
with the extra 'gpiod_direction_output()'...

- Nuno Sá

Re: [RESEND PATCH] iio: adis: set GPIO reset pin direction

[Lars-Peter Clausen]

On 7/7/21 10:36 AM, Sa, Nuno wrote:
>> From: Antti Keränen <detegr@rbx.email>
>> Sent: Tuesday, July 6, 2021 11:29 AM
>> To: linux-iio@vger.kernel.org
>> Cc: Antti Keränen <detegr@rbx.email>; Hannu Hartikainen
>> <hannu@hrtk.in>; Lars-Peter Clausen <lars@metafoo.de>; Hennerich,
>> Michael <Michael.Hennerich@analog.com>; Sa, Nuno
>> <Nuno.Sa@analog.com>; Jonathan Cameron <jic23@kernel.org>
>> Subject: [RESEND PATCH] iio: adis: set GPIO reset pin direction
>>
>> Use GPIOD_OUT_LOW instead of GPIOD_ASIS as the reset pin needs
>> to be an
>> active low output pin.
>>
>> Suggested-by: Hannu Hartikainen <hannu@hrtk.in>
>> Signed-off-by: Antti Keränen <detegr@rbx.email>
>> ---
>> The documentation of GPIO consumer interface states:
>>
>> Be aware that there is no default direction for GPIOs. Therefore,
>> **using a GPIO without setting its direction first is illegal and will
>> result in undefined behavior!**
>>
>> Therefore the direction of the reset GPIO pin should be set as output.
>>
>>   drivers/iio/imu/adis.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
>> index 319b64b2fd88..7f13b3763732 100644
>> --- a/drivers/iio/imu/adis.c
>> +++ b/drivers/iio/imu/adis.c
>> @@ -415,7 +415,7 @@ int __adis_initial_startup(struct adis *adis)
>>   	int ret;
>>
>>   	/* check if the device has rst pin low */
>> -	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset",
>> GPIOD_ASIS);
>> +	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset",
>> GPIOD_OUT_LOW);
>>   	if (IS_ERR(gpio))
>>   		return PTR_ERR(gpio);
>>
> Hi,
>
> Thanks for the patch. Forcing the device reset was intentional
> (thus the GPIO_ASIS). But what Lars is suggesting is a good idea
> and a neat improvement here.
>
GPIO_ASIS leaves the direction of the GPIO untouched. If the default is 
input, the GPIO will stay as an input, in which case triggering the 
reset does not work.