diff mbox

gpio: fix deferred probe detection for legacy API

Message ID 1417527724-24960-1-git-send-email-acourbot@nvidia.com
State Changes Requested
Headers show

Commit Message

Alexandre Courbot Dec. 2, 2014, 1:42 p.m. UTC
Commit 14e85c0e69d5 ("gpio: remove gpio_descs global array") changed
gpio_to_desc()'s behavior to return NULL not only for GPIOs numbers
not in the valid range, but also for all GPIOs whose controller has not
been probed yet. Although this behavior is more correct (nothing hints
that these GPIO numbers will be populated later), this affects
gpio_request() and gpio_request_one() which call gpiod_request() with a
NULL descriptor, causing it to return -EINVAL instead of the expected
-EPROBE_DEFER for a non-probed GPIO.

gpiod_request() is only called with a descriptor obtained from
gpio_to_desc() from these two functions, so address the issue there.

Other ways to obtain GPIOs rely on well-defined mappings and can thus
return -EPROBE_DEFER only for relevant GPIOs, and are thus not affected
by this issue.

Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Hi Geert,

Could we have your Tested-by to confirm this fixes the issue? Thanks!

Hi Linus,

Once Geert confirms this does the trick, Please feel free to squash this
patch into the gpio_descs array removal one if it is not too late for that.

 drivers/gpio/gpiolib-legacy.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Geert Uytterhoeven Dec. 2, 2014, 2:11 p.m. UTC | #1
Hi Alexandre,

On Tue, Dec 2, 2014 at 2:42 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> Commit 14e85c0e69d5 ("gpio: remove gpio_descs global array") changed
> gpio_to_desc()'s behavior to return NULL not only for GPIOs numbers
> not in the valid range, but also for all GPIOs whose controller has not
> been probed yet. Although this behavior is more correct (nothing hints
> that these GPIO numbers will be populated later), this affects
> gpio_request() and gpio_request_one() which call gpiod_request() with a
> NULL descriptor, causing it to return -EINVAL instead of the expected
> -EPROBE_DEFER for a non-probed GPIO.
>
> gpiod_request() is only called with a descriptor obtained from
> gpio_to_desc() from these two functions, so address the issue there.
>
> Other ways to obtain GPIOs rely on well-defined mappings and can thus
> return -EPROBE_DEFER only for relevant GPIOs, and are thus not affected
> by this issue.
>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>

Thanks, this restored functionality. There's still a loud
WARN(1, "invalid GPIO %d\n", gpio) in gpio_to_desc().

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

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
--
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
Linus Walleij Dec. 2, 2014, 2:19 p.m. UTC | #2
On Tue, Dec 2, 2014 at 2:42 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:

> Commit 14e85c0e69d5 ("gpio: remove gpio_descs global array") changed
> gpio_to_desc()'s behavior to return NULL not only for GPIOs numbers
> not in the valid range, but also for all GPIOs whose controller has not
> been probed yet. Although this behavior is more correct (nothing hints
> that these GPIO numbers will be populated later), this affects
> gpio_request() and gpio_request_one() which call gpiod_request() with a
> NULL descriptor, causing it to return -EINVAL instead of the expected
> -EPROBE_DEFER for a non-probed GPIO.
>
> gpiod_request() is only called with a descriptor obtained from
> gpio_to_desc() from these two functions, so address the issue there.
>
> Other ways to obtain GPIOs rely on well-defined mappings and can thus
> return -EPROBE_DEFER only for relevant GPIOs, and are thus not affected
> by this issue.
>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>

Patch applied with Geert's tested tag.

Yours,
Linus Walleij
--
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
Alexandre Courbot Dec. 2, 2014, 2:20 p.m. UTC | #3
On Tue, Dec 2, 2014 at 11:19 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Dec 2, 2014 at 2:42 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
>
>> Commit 14e85c0e69d5 ("gpio: remove gpio_descs global array") changed
>> gpio_to_desc()'s behavior to return NULL not only for GPIOs numbers
>> not in the valid range, but also for all GPIOs whose controller has not
>> been probed yet. Although this behavior is more correct (nothing hints
>> that these GPIO numbers will be populated later), this affects
>> gpio_request() and gpio_request_one() which call gpiod_request() with a
>> NULL descriptor, causing it to return -EINVAL instead of the expected
>> -EPROBE_DEFER for a non-probed GPIO.
>>
>> gpiod_request() is only called with a descriptor obtained from
>> gpio_to_desc() from these two functions, so address the issue there.
>>
>> Other ways to obtain GPIOs rely on well-defined mappings and can thus
>> return -EPROBE_DEFER only for relevant GPIOs, and are thus not affected
>> by this issue.
>>
>> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>
> Patch applied with Geert's tested tag.

I just send a v2 which only prints the warning if the GPIO is outside
of the valid range (better for legacy API).
--
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
Alexandre Courbot Dec. 2, 2014, 2:22 p.m. UTC | #4
On Tue, Dec 2, 2014 at 11:20 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Tue, Dec 2, 2014 at 11:19 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Tue, Dec 2, 2014 at 2:42 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
>>
>>> Commit 14e85c0e69d5 ("gpio: remove gpio_descs global array") changed
>>> gpio_to_desc()'s behavior to return NULL not only for GPIOs numbers
>>> not in the valid range, but also for all GPIOs whose controller has not
>>> been probed yet. Although this behavior is more correct (nothing hints
>>> that these GPIO numbers will be populated later), this affects
>>> gpio_request() and gpio_request_one() which call gpiod_request() with a
>>> NULL descriptor, causing it to return -EINVAL instead of the expected
>>> -EPROBE_DEFER for a non-probed GPIO.
>>>
>>> gpiod_request() is only called with a descriptor obtained from
>>> gpio_to_desc() from these two functions, so address the issue there.
>>>
>>> Other ways to obtain GPIOs rely on well-defined mappings and can thus
>>> return -EPROBE_DEFER only for relevant GPIOs, and are thus not affected
>>> by this issue.
>>>
>>> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>
>> Patch applied with Geert's tested tag.
>
> I just send a v2 which only prints the warning if the GPIO is outside
> of the valid range (better for legacy API).

... although contrary to what the log says I forgot to add Geerts
Tested-by tag. Sorry for the noise.

Alex (Zzzz... -_- )
--
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
diff mbox

Patch

diff --git a/drivers/gpio/gpiolib-legacy.c b/drivers/gpio/gpiolib-legacy.c
index 078ae6c..8b83099 100644
--- a/drivers/gpio/gpiolib-legacy.c
+++ b/drivers/gpio/gpiolib-legacy.c
@@ -24,6 +24,10 @@  int gpio_request_one(unsigned gpio, unsigned long flags, const char *label)
 
 	desc = gpio_to_desc(gpio);
 
+	/* Compatibility: assume unavailable "valid" GPIOs will appear later */
+	if (!desc && gpio_is_valid(gpio))
+		return -EPROBE_DEFER;
+
 	err = gpiod_request(desc, label);
 	if (err)
 		return err;
@@ -62,7 +66,13 @@  EXPORT_SYMBOL_GPL(gpio_request_one);
 
 int gpio_request(unsigned gpio, const char *label)
 {
-	return gpiod_request(gpio_to_desc(gpio), label);
+	struct gpio_desc *desc = gpio_to_desc(gpio);
+
+	/* Compatibility: assume unavailable "valid" GPIOs will appear later */
+	if (!desc && gpio_is_valid(gpio))
+		return -EPROBE_DEFER;
+
+	return gpiod_request(desc, label);
 }
 EXPORT_SYMBOL_GPL(gpio_request);