Re: [RESEND PATCH] iio: adis: set GPIO reset pin direction - Lars-Peter Clausen

From: Lars-Peter Clausen <lars@metafoo.de>
To: "Sa, Nuno" <Nuno.Sa@analog.com>,
	"Antti Keränen" <detegr@rbx.email>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>
Cc: Hannu Hartikainen <hannu@hrtk.in>,
	"Hennerich, Michael" <Michael.Hennerich@analog.com>,
	Jonathan Cameron <jic23@kernel.org>
Subject: Re: [RESEND PATCH] iio: adis: set GPIO reset pin direction
Date: Wed, 7 Jul 2021 14:32:10 +0200	[thread overview]
Message-ID: <6f2a3147-11df-c85e-849e-f7a9d92a71fd@metafoo.de> (raw)
In-Reply-To: <PH0PR03MB63661EDEE130924514602A84991A9@PH0PR03MB6366.namprd03.prod.outlook.com>

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.