[PATCH] gpiolib: Keep returning EPROBE_DEFER when we should

[PATCH] gpiolib: Keep returning EPROBE_DEFER when we should

[Maxime Ripard]

Commits c85823390215 ("gpio: of: Support SPI nonstandard GPIO properties")
and 6a537d48461d ("gpio: of: Support regulator nonstandard GPIO
properties") have introduced a regression in the way error codes from
of_get_named_gpiod_flags are handled.

Previously, those errors codes were returned immediately, but the two
commits mentioned above are now overwriting the error pointer, meaning that
whatever value has been returned will be dropped in favor of whatever the
two new functions will return.

This might not be a big deal except for EPROBE_DEFER, on which GPIOlib
customers will depend on, and that will now be returned as an hard error
which means that they will not probe anymore, instead of gently deferring
their probe.

Since EPROBE_DEFER basically means that we have found a valid property but
there was no GPIO controller registered to handle it, fix this issues by
returning it as soon as we encounter it.

Fixes: c85823390215 ("gpio: of: Support SPI nonstandard GPIO properties")
Fixes: 6a537d48461d ("gpio: of: Support regulator nonstandard GPIO properties")
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpio/gpiolib-of.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 564bb7a31da4..c92847ff5d0a 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -243,6 +243,19 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 						&of_flags);
 		if (!IS_ERR(desc) || (PTR_ERR(desc) != -ENOENT))
 			break;
+
+		/*
+		 * -EPROBE_DEFER in our case means that we found a
+		 * valid GPIO property, but no controller has been
+		 * registered so far.
+		 *
+		 * This means we don't need to look any further for
+		 * alternate name conventions, and we should really
+		 * preserve the return code for our user to be able to
+		 * retry probing later.
+		 */
+		if (IS_ERR(desc) && PTR_ERR(desc) == -EPROBE_DEFER)
+			return desc;
 	}
 
 	/* Special handling for SPI GPIOs if used */
-- 
2.14.3

Re: [PATCH] gpiolib: Keep returning EPROBE_DEFER when we should

[Linus Walleij]

On Wed, Feb 21, 2018 at 9:11 AM, Maxime Ripard
<maxime.ripard@bootlin.com> wrote:

> Commits c85823390215 ("gpio: of: Support SPI nonstandard GPIO properties")
> and 6a537d48461d ("gpio: of: Support regulator nonstandard GPIO
> properties") have introduced a regression in the way error codes from
> of_get_named_gpiod_flags are handled.
>
> Previously, those errors codes were returned immediately, but the two
> commits mentioned above are now overwriting the error pointer, meaning that
> whatever value has been returned will be dropped in favor of whatever the
> two new functions will return.
>
> This might not be a big deal except for EPROBE_DEFER, on which GPIOlib
> customers will depend on, and that will now be returned as an hard error
> which means that they will not probe anymore, instead of gently deferring
> their probe.
>
> Since EPROBE_DEFER basically means that we have found a valid property but
> there was no GPIO controller registered to handle it, fix this issues by
> returning it as soon as we encounter it.
>
> Fixes: c85823390215 ("gpio: of: Support SPI nonstandard GPIO properties")
> Fixes: 6a537d48461d ("gpio: of: Support regulator nonstandard GPIO properties")
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>

Whoops sorry for that, patch applied for fixes.

Thanks you for your help!

Yours,
Linus Walleij

Re: [PATCH] gpiolib: Keep returning EPROBE_DEFER when we should

[Chen-Yu Tsai]

Hi Linus

On Wed, Feb 21, 2018 at 4:17 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Feb 21, 2018 at 9:11 AM, Maxime Ripard
> <maxime.ripard@bootlin.com> wrote:
>
>> Commits c85823390215 ("gpio: of: Support SPI nonstandard GPIO properties")
>> and 6a537d48461d ("gpio: of: Support regulator nonstandard GPIO
>> properties") have introduced a regression in the way error codes from
>> of_get_named_gpiod_flags are handled.
>>
>> Previously, those errors codes were returned immediately, but the two
>> commits mentioned above are now overwriting the error pointer, meaning that
>> whatever value has been returned will be dropped in favor of whatever the
>> two new functions will return.
>>
>> This might not be a big deal except for EPROBE_DEFER, on which GPIOlib
>> customers will depend on, and that will now be returned as an hard error
>> which means that they will not probe anymore, instead of gently deferring
>> their probe.
>>
>> Since EPROBE_DEFER basically means that we have found a valid property but
>> there was no GPIO controller registered to handle it, fix this issues by
>> returning it as soon as we encounter it.
>>
>> Fixes: c85823390215 ("gpio: of: Support SPI nonstandard GPIO properties")
>> Fixes: 6a537d48461d ("gpio: of: Support regulator nonstandard GPIO properties")
>> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
>
> Whoops sorry for that, patch applied for fixes.

Would this patch be any better?

https://www.spinics.net/lists/linux-gpio/msg28160.html

Not sure if of_find_spi_gpio() and of_find_regulator_gpio() return
EPROBE_DEFER as well.

ChenYu

Re: [PATCH] gpiolib: Keep returning EPROBE_DEFER when we should

[Linus Walleij]

On Wed, Feb 21, 2018 at 9:19 AM, Chen-Yu Tsai <wens@csie.org> wrote:
> On Wed, Feb 21, 2018 at 4:17 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Wed, Feb 21, 2018 at 9:11 AM, Maxime Ripard
>> <maxime.ripard@bootlin.com> wrote:
>>
>>> Commits c85823390215 ("gpio: of: Support SPI nonstandard GPIO properties")
>>> and 6a537d48461d ("gpio: of: Support regulator nonstandard GPIO
>>> properties") have introduced a regression in the way error codes from
>>> of_get_named_gpiod_flags are handled.
>>>
>>> Previously, those errors codes were returned immediately, but the two
>>> commits mentioned above are now overwriting the error pointer, meaning that
>>> whatever value has been returned will be dropped in favor of whatever the
>>> two new functions will return.
>>>
>>> This might not be a big deal except for EPROBE_DEFER, on which GPIOlib
>>> customers will depend on, and that will now be returned as an hard error
>>> which means that they will not probe anymore, instead of gently deferring
>>> their probe.
>>>
>>> Since EPROBE_DEFER basically means that we have found a valid property but
>>> there was no GPIO controller registered to handle it, fix this issues by
>>> returning it as soon as we encounter it.
>>>
>>> Fixes: c85823390215 ("gpio: of: Support SPI nonstandard GPIO properties")
>>> Fixes: 6a537d48461d ("gpio: of: Support regulator nonstandard GPIO properties")
>>> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
>>
>> Whoops sorry for that, patch applied for fixes.
>
> Would this patch be any better?
>
> https://www.spinics.net/lists/linux-gpio/msg28160.html
>
> Not sure if of_find_spi_gpio() and of_find_regulator_gpio() return
> EPROBE_DEFER as well.

Ah sorry I missed the patch, too much patches in my Inbox
again, I wish I was better at this :/

I think Maxime's patch is nice since it clearly bails out of the
loop as soon as we get deferral.

But we want the second part of your patch as well. If
of_find_spi_gpio() returns -EPROBE_DEFER it should
return.

I will look into it.

Yours,
Linus Walleij

Re: [PATCH] gpiolib: Keep returning EPROBE_DEFER when we should

[Maxime Ripard]

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

On Wed, Feb 21, 2018 at 09:17:15AM +0100, Linus Walleij wrote:
> On Wed, Feb 21, 2018 at 9:11 AM, Maxime Ripard
> <maxime.ripard@bootlin.com> wrote:
> 
> > Commits c85823390215 ("gpio: of: Support SPI nonstandard GPIO properties")
> > and 6a537d48461d ("gpio: of: Support regulator nonstandard GPIO
> > properties") have introduced a regression in the way error codes from
> > of_get_named_gpiod_flags are handled.
> >
> > Previously, those errors codes were returned immediately, but the two
> > commits mentioned above are now overwriting the error pointer, meaning that
> > whatever value has been returned will be dropped in favor of whatever the
> > two new functions will return.
> >
> > This might not be a big deal except for EPROBE_DEFER, on which GPIOlib
> > customers will depend on, and that will now be returned as an hard error
> > which means that they will not probe anymore, instead of gently deferring
> > their probe.
> >
> > Since EPROBE_DEFER basically means that we have found a valid property but
> > there was no GPIO controller registered to handle it, fix this issues by
> > returning it as soon as we encounter it.
> >
> > Fixes: c85823390215 ("gpio: of: Support SPI nonstandard GPIO properties")
> > Fixes: 6a537d48461d ("gpio: of: Support regulator nonstandard GPIO properties")
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> 
> Whoops sorry for that, patch applied for fixes.
> 
> Thanks you for your help!

This is slightly embarrassing, but that patch doesn't actually fix
anything. I tried to be smart and rework it to look nicer, and
obviously failed.

Can you squash
http://code.bulix.org/z1vkt1-286673

in the commit?

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

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

Re: [PATCH] gpiolib: Keep returning EPROBE_DEFER when we should

[Linus Walleij]

On Wed, Feb 21, 2018 at 10:16 AM, Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
> On Wed, Feb 21, 2018 at 09:17:15AM +0100, Linus Walleij wrote:
>> On Wed, Feb 21, 2018 at 9:11 AM, Maxime Ripard
>> <maxime.ripard@bootlin.com> wrote:
>>
> This is slightly embarrassing, but that patch doesn't actually fix
> anything. I tried to be smart and rework it to look nicer, and
> obviously failed.
>
> Can you squash
> http://code.bulix.org/z1vkt1-286673
>
> in the commit?

This site is down. Can you send a new version of the patch or
just reply with the change inline here?

Yours,
Linus Walleij

Re: [PATCH] gpiolib: Keep returning EPROBE_DEFER when we should

[Maxime Ripard]

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

On Mon, Feb 26, 2018 at 11:24:35AM +0100, Linus Walleij wrote:
> On Wed, Feb 21, 2018 at 10:16 AM, Maxime Ripard
> <maxime.ripard@bootlin.com> wrote:
> > On Wed, Feb 21, 2018 at 09:17:15AM +0100, Linus Walleij wrote:
> >> On Wed, Feb 21, 2018 at 9:11 AM, Maxime Ripard
> >> <maxime.ripard@bootlin.com> wrote:
> >>
> > This is slightly embarrassing, but that patch doesn't actually fix
> > anything. I tried to be smart and rework it to look nicer, and
> > obviously failed.
> >
> > Can you squash
> > http://code.bulix.org/z1vkt1-286673
> >
> > in the commit?
> 
> This site is down.

Sorry :/

> Can you send a new version of the patch or just reply with the
> change inline here?

Here it is:

---------8<----------------
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index c92847ff5d0a..ae70f679459c 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -241,8 +241,6 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 
                desc = of_get_named_gpiod_flags(dev->of_node, prop_name, idx,
                                                &of_flags);
-               if (!IS_ERR(desc) || (PTR_ERR(desc) != -ENOENT))
-                       break;
 
                /*
                 * -EPROBE_DEFER in our case means that we found a
@@ -256,6 +254,9 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
                 */
                if (IS_ERR(desc) && PTR_ERR(desc) == -EPROBE_DEFER)
                        return desc;
+
+               if (!IS_ERR(desc) || (PTR_ERR(desc) != -ENOENT))
+                       break;
        }
 
        /* Special handling for SPI GPIOs if used */
----------8<-----------------

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

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