[PATCH] gpiolib: use better errno if get_direction is not available

[PATCH] gpiolib: use better errno if get_direction is not available

[Wolfram Sang]

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(-)

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

Re: [PATCH] gpiolib: use better errno if get_direction is not available

[Geert Uytterhoeven]

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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] gpiolib: use better errno if get_direction is not available

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 65 bytes --]


> Do you want me to implement it?

Yes, please.

Thanks a lot!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] gpiolib: use better errno if get_direction is not available

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2014 bytes --]

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] gpiolib: use better errno if get_direction is not available

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 225 bytes --]


> 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.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] gpiolib: use better errno if get_direction is not available

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 112 bytes --]


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

Linus, do you have time to comment on this?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] gpiolib: use better errno if get_direction is not available

[Linus Walleij]

On Wed, Jul 25, 2018 at 10:20 PM Wolfram Sang <wsa@the-dreams.de> wrote:

> > That all being said, I think this patch is still useful as is.
>
> Linus, do you have time to comment on this?

This looks like a good solution to me.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH] gpiolib: use better errno if get_direction is not available

[Geert Uytterhoeven]

Hi Linus,

On Sun, Jul 29, 2018 at 11:33 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Jul 25, 2018 at 10:20 PM Wolfram Sang <wsa@the-dreams.de> wrote:
> > > That all being said, I think this patch is still useful as is.
> >
> > Linus, do you have time to comment on this?
>
> This looks like a good solution to me.
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Any plans to apply it? drivers/gpio/gpiolib.c is your territory, I believe ;-)

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] gpiolib: use better errno if get_direction is not available

[Linus Walleij]

On Tue, Sep 11, 2018 at 2:04 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Sun, Jul 29, 2018 at 11:33 PM Linus Walleij <linus.walleij@linaro.org> wrote:> > On Wed, Jul 25, 2018 at 10:20 PM Wolfram Sang <wsa@the-dreams.de> wrote:
> > > > That all being said, I think this patch is still useful as is.
> > >
> > > Linus, do you have time to comment on this?
> >
> > This looks like a good solution to me.
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> Any plans to apply it? drivers/gpio/gpiolib.c is your territory, I believe ;-)

Ooops sorry.

Patch applied.

Yours,
Linus Walleij

Re: [PATCH] gpiolib: use better errno if get_direction is not available

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 155 bytes --]


> > Any plans to apply it? drivers/gpio/gpiolib.c is your territory, I believe ;-)
> 
> Ooops sorry.
> 
> Patch applied.

Thanks Geert & Linus!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] gpiolib: use better errno if get_direction is not available

[Simon Horman]

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
>