gpiolib: use better errno if get_direction is not available

Message ID 20180711163319.28004-1-wsa+renesas@sang-engineering.com
State New
Headers show
Series
  • gpiolib: use better errno if get_direction is not available
Related show

Commit Message

Wolfram Sang July 11, 2018, 4:33 p.m.
EINVAL is very generic, use ENOTSUPP in case the gpiochip does not
provide this function. While removing the assignment from the 'status'
variable, use better indentation in the declaration block.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

I got puzzled by the EINVAL until I found out that gpio-rcar simply does not
implement it.

@Geert: any reason gpio-rcar is missing it? I would need it for the
i2c-gpio-fault-injector.

 drivers/gpio/gpiolib.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Geert Uytterhoeven July 11, 2018, 5:24 p.m. | #1
Hi Wolfram,

On Wed, Jul 11, 2018 at 6:33 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> @Geert: any reason gpio-rcar is missing it? I would need it for the
> i2c-gpio-fault-injector.

Because so far no one had a need for it?
/me thought gpiolib would return a cached value from a previous
set_direction_{in,out}put(), but apparently that must have been a dream...

Do you want me to implement it?

Gr{oetje,eeting}s,

                        Geert
Wolfram Sang July 11, 2018, 5:29 p.m. | #2
> Do you want me to implement it?

Yes, please.

Thanks a lot!
Wolfram Sang July 12, 2018, 3:51 p.m. | #3
On Wed, Jul 11, 2018 at 06:33:19PM +0200, Wolfram Sang wrote:
> EINVAL is very generic, use ENOTSUPP in case the gpiochip does not
> provide this function. While removing the assignment from the 'status'
> variable, use better indentation in the declaration block.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

So, Geert implemented get_direction for gpio-rcar, but this doesn't help
my case sadly. Because I2C is open drain, get_direction returns 'input'
:( As mentioned in a previous discussion, returning '0' and '1' from
get_direction() is confusing. But as it looks now, it also is not
enough. Or we'd need another function from which I could determine the
OUT_OPEN_DRAIN state.

For the short term, I will remove the sanity check from the I2C core and
hope the user properly set up the GPIO. It would be nice to check for it
somewhen in the future, though.

That all being said, I think this patch is still useful as is.

Thanks,

   Wolfram

> ---
> 
> I got puzzled by the EINVAL until I found out that gpio-rcar simply does not
> implement it.
> 
> @Geert: any reason gpio-rcar is missing it? I would need it for the
> i2c-gpio-fault-injector.
> 
>  drivers/gpio/gpiolib.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index e11a3bb03820..18719f64e80b 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -210,15 +210,15 @@ static int gpiochip_find_base(int ngpio)
>   */
>  int gpiod_get_direction(struct gpio_desc *desc)
>  {
> -	struct gpio_chip	*chip;
> -	unsigned		offset;
> -	int			status = -EINVAL;
> +	struct gpio_chip *chip;
> +	unsigned offset;
> +	int status;
>  
>  	chip = gpiod_to_chip(desc);
>  	offset = gpio_chip_hwgpio(desc);
>  
>  	if (!chip->get_direction)
> -		return status;
> +		return -ENOTSUPP;
>  
>  	status = chip->get_direction(chip, offset);
>  	if (status > 0) {
> -- 
> 2.11.0
>
Wolfram Sang July 12, 2018, 4:31 p.m. | #4
> For the short term, I will remove the sanity check from the I2C core and
> hope the user properly set up the GPIO.

For the record, this patch is here:

http://patchwork.ozlabs.org/patch/943115/

Forgot to cc linux-gpio.
Simon Horman July 13, 2018, 8:01 a.m. | #5
On Wed, Jul 11, 2018 at 06:33:19PM +0200, Wolfram Sang wrote:
> EINVAL is very generic, use ENOTSUPP in case the gpiochip does not
> provide this function. While removing the assignment from the 'status'
> variable, use better indentation in the declaration block.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> ---
> 
> I got puzzled by the EINVAL until I found out that gpio-rcar simply does not
> implement it.
> 
> @Geert: any reason gpio-rcar is missing it? I would need it for the
> i2c-gpio-fault-injector.
> 
>  drivers/gpio/gpiolib.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index e11a3bb03820..18719f64e80b 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -210,15 +210,15 @@ static int gpiochip_find_base(int ngpio)
>   */
>  int gpiod_get_direction(struct gpio_desc *desc)
>  {
> -	struct gpio_chip	*chip;
> -	unsigned		offset;
> -	int			status = -EINVAL;
> +	struct gpio_chip *chip;
> +	unsigned offset;
> +	int status;
>  
>  	chip = gpiod_to_chip(desc);
>  	offset = gpio_chip_hwgpio(desc);
>  
>  	if (!chip->get_direction)
> -		return status;
> +		return -ENOTSUPP;
>  
>  	status = chip->get_direction(chip, offset);
>  	if (status > 0) {
> -- 
> 2.11.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e11a3bb03820..18719f64e80b 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -210,15 +210,15 @@  static int gpiochip_find_base(int ngpio)
  */
 int gpiod_get_direction(struct gpio_desc *desc)
 {
-	struct gpio_chip	*chip;
-	unsigned		offset;
-	int			status = -EINVAL;
+	struct gpio_chip *chip;
+	unsigned offset;
+	int status;
 
 	chip = gpiod_to_chip(desc);
 	offset = gpio_chip_hwgpio(desc);
 
 	if (!chip->get_direction)
-		return status;
+		return -ENOTSUPP;
 
 	status = chip->get_direction(chip, offset);
 	if (status > 0) {