Replace the ARCH_NR_GPIOS-sized static array of GPIO descriptors by
dynamically-allocated arrays for each GPIO chip.
This change makes gpio_to_desc() perform in O(n) (where n is the number
of GPIO chips registered) instead of O(1), however since n is rarely
bigger than 1 or 2 no noticeable performance issue is expected.
Besides this provides more incentive for GPIO consumers to move to the
gpiod interface. One could use a O(log(n)) structure to link the GPIO
chips together, but considering the low limit of n the hypothetical
performance benefit is probably not worth the added complexity.
This patch uses kcalloc() in gpiochip_add(), which removes the ability
to add a chip before kcalloc() can operate. I am not aware of such
cases, but if someone bisects up to this patch then I will be proven
wrong...
Signed-off-by: Alexandre Courbot <[email protected]>
---
drivers/gpio/gpiolib.c | 47 +++++++++++++++++++++++++++++++----------------
1 file changed, 31 insertions(+), 16 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index eb739a51e774..5619922ebf44 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -47,8 +47,6 @@
*/
DEFINE_SPINLOCK(gpio_lock);
-static struct gpio_desc gpio_desc[ARCH_NR_GPIOS];
-
#define GPIO_OFFSET_VALID(chip, offset) (offset >= 0 && offset < chip->ngpio)
static DEFINE_MUTEX(gpio_lookup_lock);
@@ -65,10 +63,22 @@ static inline void desc_set_label(struct gpio_desc *d, const char *label)
*/
struct gpio_desc *gpio_to_desc(unsigned gpio)
{
- if (WARN(!gpio_is_valid(gpio), "invalid GPIO %d\n", gpio))
- return NULL;
- else
- return &gpio_desc[gpio];
+ struct gpio_chip *chip;
+ unsigned long flags;
+
+ spin_lock_irqsave(&gpio_lock, flags);
+
+ list_for_each_entry(chip, &gpio_chips, list) {
+ if (chip->base <= gpio && chip->base + chip->ngpio > gpio) {
+ spin_unlock_irqrestore(&gpio_lock, flags);
+ return &chip->desc[gpio - chip->base];
+ }
+ }
+
+ spin_unlock_irqrestore(&gpio_lock, flags);
+
+ WARN(1, "invalid GPIO %d\n", gpio);
+ return NULL;
}
EXPORT_SYMBOL_GPL(gpio_to_desc);
@@ -91,7 +101,7 @@ struct gpio_desc *gpiochip_get_desc(struct gpio_chip *chip,
*/
int desc_to_gpio(const struct gpio_desc *desc)
{
- return desc - &gpio_desc[0];
+ return desc->chip->base + (desc - &desc->chip->desc[0]);
}
EXPORT_SYMBOL_GPL(desc_to_gpio);
@@ -206,7 +216,7 @@ static int gpiochip_add_to_list(struct gpio_chip *chip)
/**
* gpiochip_add() - register a gpio_chip
* @chip: the chip to register, with chip->base initialized
- * Context: potentially before irqs or kmalloc will work
+ * Context: potentially before irqs will work
*
* Returns a negative errno if the chip can't be registered, such as
* because the chip->base is invalid or already associated with a
@@ -226,12 +236,11 @@ int gpiochip_add(struct gpio_chip *chip)
int status = 0;
unsigned id;
int base = chip->base;
+ struct gpio_desc *descs;
- if ((!gpio_is_valid(base) || !gpio_is_valid(base + chip->ngpio - 1))
- && base >= 0) {
- status = -EINVAL;
- goto fail;
- }
+ descs = kcalloc(chip->ngpio, sizeof(descs[0]), GFP_KERNEL);
+ if (!descs)
+ return -ENOMEM;
spin_lock_irqsave(&gpio_lock, flags);
@@ -247,10 +256,8 @@ int gpiochip_add(struct gpio_chip *chip)
status = gpiochip_add_to_list(chip);
if (status == 0) {
- chip->desc = &gpio_desc[chip->base];
-
for (id = 0; id < chip->ngpio; id++) {
- struct gpio_desc *desc = &chip->desc[id];
+ struct gpio_desc *desc = &descs[id];
desc->chip = chip;
/* REVISIT: most hardware initializes GPIOs as
@@ -266,6 +273,8 @@ int gpiochip_add(struct gpio_chip *chip)
}
}
+ chip->desc = descs;
+
spin_unlock_irqrestore(&gpio_lock, flags);
#ifdef CONFIG_PINCTRL
@@ -291,6 +300,9 @@ int gpiochip_add(struct gpio_chip *chip)
unlock:
spin_unlock_irqrestore(&gpio_lock, flags);
fail:
+ kfree(descs);
+ chip->desc = NULL;
+
/* failures here can mean systems won't boot... */
pr_err("%s: GPIOs %d..%d (%s) failed to register\n", __func__,
chip->base, chip->base + chip->ngpio - 1,
@@ -331,6 +343,9 @@ void gpiochip_remove(struct gpio_chip *chip)
list_del(&chip->list);
spin_unlock_irqrestore(&gpio_lock, flags);
gpiochip_unexport(chip);
+
+ kfree(chip->desc);
+ chip->desc = NULL;
}
EXPORT_SYMBOL_GPL(gpiochip_remove);
--
2.1.3
On Wed, Nov 19, 2014 at 8:51 AM, Alexandre Courbot <[email protected]> wrote:
> Replace the ARCH_NR_GPIOS-sized static array of GPIO descriptors by
> dynamically-allocated arrays for each GPIO chip.
>
> This change makes gpio_to_desc() perform in O(n) (where n is the number
> of GPIO chips registered) instead of O(1), however since n is rarely
> bigger than 1 or 2 no noticeable performance issue is expected.
> Besides this provides more incentive for GPIO consumers to move to the
> gpiod interface. One could use a O(log(n)) structure to link the GPIO
> chips together, but considering the low limit of n the hypothetical
> performance benefit is probably not worth the added complexity.
>
> This patch uses kcalloc() in gpiochip_add(), which removes the ability
> to add a chip before kcalloc() can operate. I am not aware of such
> cases, but if someone bisects up to this patch then I will be proven
> wrong...
>
> Signed-off-by: Alexandre Courbot <[email protected]>
OK patch applied. Let's see if the world explodes.
Yours,
Linus Walleij
Hi Linus, Alexandre,
On Fri, Nov 28, 2014 at 11:29 AM, Linus Walleij
<[email protected]> wrote:
> On Wed, Nov 19, 2014 at 8:51 AM, Alexandre Courbot <[email protected]> wrote:
>> Replace the ARCH_NR_GPIOS-sized static array of GPIO descriptors by
>> dynamically-allocated arrays for each GPIO chip.
>>
>> This change makes gpio_to_desc() perform in O(n) (where n is the number
>> of GPIO chips registered) instead of O(1), however since n is rarely
>> bigger than 1 or 2 no noticeable performance issue is expected.
>> Besides this provides more incentive for GPIO consumers to move to the
>> gpiod interface. One could use a O(log(n)) structure to link the GPIO
>> chips together, but considering the low limit of n the hypothetical
>> performance benefit is probably not worth the added complexity.
>>
>> This patch uses kcalloc() in gpiochip_add(), which removes the ability
>> to add a chip before kcalloc() can operate. I am not aware of such
>> cases, but if someone bisects up to this patch then I will be proven
>> wrong...
>>
>> Signed-off-by: Alexandre Courbot <[email protected]>
>
> OK patch applied. Let's see if the world explodes.
This patch changes a return value from -EPROBE_DEFER to -EINVAL in
regulator-gpio when a GPIO cannot be found yet, causing probe failures
on r8a7791/koelsch:
of_get_named_gpiod_flags: can't parse 'enable-gpio' property of node
'/regulator@1[0]'
of_get_named_gpiod_flags: parsed 'gpios' property of node
'/regulator@1[0]' - status (-517)
-gpio-0 (?): gpiod_request: status -517
-gpio-regulator regulator@1: Could not obtain regulator setting GPIOs: -517
-platform regulator@1: Driver gpio-regulator requests probe deferral
+------------[ cut here ]------------
+WARNING: CPU: 0 PID: 1 at
/scratch/geert/linux/linux-renesas/drivers/gpio/gpiolib.c:80
gpio_to_desc+0x94/0xac()
+invalid GPIO 0
+Modules linked in:
+CPU: 0 PID: 1 Comm: swapper/0 Not tainted
3.18.0-rc7-koelsch-05803-ge7a5d822abfc727e #634
+Hardware name: Generic R8A7791 (Flattened Device Tree)
+Backtrace:
+[<c0012118>] (dump_backtrace) from [<c0012338>] (show_stack+0x18/0x1c)
+ r6:c04608f7 r5:00000009 r4:00000000 r3:00200040
+[<c0012320>] (show_stack) from [<c0386e48>] (dump_stack+0x78/0x94)
+[<c0386dd0>] (dump_stack) from [<c0026b84>] (warn_slowpath_common+0x70/0x94)
+ r4:eec53d10 r3:c04f3390
+[<c0026b14>] (warn_slowpath_common) from [<c0026be0>]
(warn_slowpath_fmt+0x38/0x40)
+ r8:eed4a600 r7:00000001 r6:00000000 r5:00000000 r4:c04f9430
+[<c0026bac>] (warn_slowpath_fmt) from [<c01dd700>] (gpio_to_desc+0x94/0xac)
+ r3:00000000 r2:c0460931
+[<c01dd66c>] (gpio_to_desc) from [<c01df200>] (gpio_request_one+0x18/0xc4)
+ r5:00000000 r4:00000000
+[<c01df1e8>] (gpio_request_one) from [<c01df2d4>]
(gpio_request_array+0x28/0x60)
+ r6:00000004 r5:00000000 r4:eed5a4c0 r3:00000002
+[<c01df2ac>] (gpio_request_array) from [<c0201b20>]
(gpio_regulator_probe+0x424/0x5a4)
+ r7:eed4a610 r6:00000004 r5:eed464d0 r4:eed5a5d0
+[<c02016fc>] (gpio_regulator_probe) from [<c022a81c>]
(platform_drv_probe+0x50/0xa0)
+ r10:00000000 r9:c0509f80 r8:c04fa648 r7:00000000 r6:c04fa648 r5:eed4a610
+ r4:ffffffed
+[<c022a7cc>] (platform_drv_probe) from [<c0229000>]
(driver_probe_device+0xc0/0x204)
+ r6:00000000 r5:c054ae0c r4:eed4a610 r3:c022a7cc
+[<c0228f40>] (driver_probe_device) from [<c0229200>]
(__driver_attach+0x70/0x94)
+ r8:c04ee560 r7:c04fe4b0 r6:c04fa648 r5:eed4a644 r4:eed4a610 r3:00000000
+[<c0229190>] (__driver_attach) from [<c02276bc>] (bus_for_each_dev+0x74/0x98)
+ r6:c0229190 r5:c04fa648 r4:00000000 r3:00000001
+[<c0227648>] (bus_for_each_dev) from [<c0228b5c>] (driver_attach+0x20/0x28)
+ r6:eed52300 r5:00000000 r4:c04fa648
+[<c0228b3c>] (driver_attach) from [<c02287f4>] (bus_add_driver+0xe8/0x1d0)
+[<c022870c>] (bus_add_driver) from [<c022989c>] (driver_register+0xa4/0xe8)
+ r7:c04ee560 r6:00000000 r5:c04c0498 r4:c04fa648
+[<c02297f8>] (driver_register) from [<c022a750>]
(__platform_driver_register+0x50/0x64)
+ r5:c04c0498 r4:eed5a800
+[<c022a700>] (__platform_driver_register) from [<c04c04b0>]
(gpio_regulator_init+0x18/0x20)
+[<c04c0498>] (gpio_regulator_init) from [<c0009778>]
(do_one_initcall+0x108/0x1b8)
+[<c0009670>] (do_one_initcall) from [<c04a9e10>]
(kernel_init_freeable+0x11c/0x1e4)
+ r9:c0509f80 r8:c0509f80 r7:c04dfcf0 r6:c04d7c2c r5:0000006d r4:00000004
+[<c04a9cf4>] (kernel_init_freeable) from [<c0384538>] (kernel_init+0x10/0xec)
+ r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c0384528 r4:00000000
+[<c0384528>] (kernel_init) from [<c000ee58>] (ret_from_fork+0x14/0x3c)
+ r4:00000000 r3:eec52000
+---[ end trace e95579bdd1fdbe74 ]---
+gpiod_request: invalid GPIO
+gpio-regulator regulator@1: Could not obtain regulator setting GPIOs: -22
+gpio-regulator: probe of regulator@1 failed with error -22
Reverting commit 14e85c0e69d5c7fdbd963edbbec1dc5cdd385200
("gpio: remove gpio_descs global array") fixes the issue.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On Tue, Dec 2, 2014 at 7:32 PM, Geert Uytterhoeven <[email protected]> wrote:
> Hi Linus, Alexandre,
>
> On Fri, Nov 28, 2014 at 11:29 AM, Linus Walleij
> <[email protected]> wrote:
>> On Wed, Nov 19, 2014 at 8:51 AM, Alexandre Courbot <[email protected]> wrote:
>>> Replace the ARCH_NR_GPIOS-sized static array of GPIO descriptors by
>>> dynamically-allocated arrays for each GPIO chip.
>>>
>>> This change makes gpio_to_desc() perform in O(n) (where n is the number
>>> of GPIO chips registered) instead of O(1), however since n is rarely
>>> bigger than 1 or 2 no noticeable performance issue is expected.
>>> Besides this provides more incentive for GPIO consumers to move to the
>>> gpiod interface. One could use a O(log(n)) structure to link the GPIO
>>> chips together, but considering the low limit of n the hypothetical
>>> performance benefit is probably not worth the added complexity.
>>>
>>> This patch uses kcalloc() in gpiochip_add(), which removes the ability
>>> to add a chip before kcalloc() can operate. I am not aware of such
>>> cases, but if someone bisects up to this patch then I will be proven
>>> wrong...
>>>
>>> Signed-off-by: Alexandre Courbot <[email protected]>
>>
>> OK patch applied. Let's see if the world explodes.
>
> This patch changes a return value from -EPROBE_DEFER to -EINVAL in
> regulator-gpio when a GPIO cannot be found yet, causing probe failures
> on r8a7791/koelsch:
Hi Geert,
Thanks for signaling this. I think I understand what is going wrong
and will send a fixup patch in a few minutes.
Cheers,
Alex.
On Tue, Dec 2, 2014 at 2:25 PM, Alexandre Courbot <[email protected]> wrote:
> On Tue, Dec 2, 2014 at 7:32 PM, Geert Uytterhoeven <[email protected]> wrote:
>> This patch changes a return value from -EPROBE_DEFER to -EINVAL in
>> regulator-gpio when a GPIO cannot be found yet, causing probe failures
>> on r8a7791/koelsch:
>
> Hi Geert,
>
> Thanks for signaling this. I think I understand what is going wrong
> and will send a fixup patch in a few minutes.
OK standing by.
Yours,
Linus Walleij
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 <[email protected]>
Signed-off-by: Alexandre Courbot <[email protected]>
---
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(-)
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);
--
2.1.3
Hi Alexandre,
On Tue, Dec 2, 2014 at 2:42 PM, Alexandre Courbot <[email protected]> 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 <[email protected]>
> Signed-off-by: Alexandre Courbot <[email protected]>
Thanks, this restored functionality. There's still a loud
WARN(1, "invalid GPIO %d\n", gpio) in gpio_to_desc().
Tested-by: Geert Uytterhoeven <[email protected]>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On Tue, Dec 2, 2014 at 2:42 PM, Alexandre Courbot <[email protected]> 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 <[email protected]>
> Signed-off-by: Alexandre Courbot <[email protected]>
Patch applied with Geert's tested tag.
Yours,
Linus Walleij
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 <[email protected]>
Signed-off-by: Alexandre Courbot <[email protected]>
---
v2:
- Add Geert's Tested-by
- Only display scary warning on gpio_to_desc() is the GPIO is outside
the valid range (pre gpio_desc array removal behavior)
drivers/gpio/gpiolib-legacy.c | 12 +++++++++++-
drivers/gpio/gpiolib.c | 4 +++-
2 files changed, 14 insertions(+), 2 deletions(-)
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);
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 0b271ef8..56b7c5d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -77,7 +77,9 @@ struct gpio_desc *gpio_to_desc(unsigned gpio)
spin_unlock_irqrestore(&gpio_lock, flags);
- WARN(1, "invalid GPIO %d\n", gpio);
+ if (!gpio_is_valid(gpio))
+ WARN(1, "invalid GPIO %d\n", gpio);
+
return NULL;
}
EXPORT_SYMBOL_GPL(gpio_to_desc);
--
2.1.3
On Tue, Dec 2, 2014 at 11:19 PM, Linus Walleij <[email protected]> wrote:
> On Tue, Dec 2, 2014 at 2:42 PM, Alexandre Courbot <[email protected]> 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 <[email protected]>
>> Signed-off-by: Alexandre Courbot <[email protected]>
>
> 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).
On Tue, Dec 2, 2014 at 11:20 PM, Alexandre Courbot <[email protected]> wrote:
> On Tue, Dec 2, 2014 at 11:19 PM, Linus Walleij <[email protected]> wrote:
>> On Tue, Dec 2, 2014 at 2:42 PM, Alexandre Courbot <[email protected]> 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 <[email protected]>
>>> Signed-off-by: Alexandre Courbot <[email protected]>
>>
>> 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... -_- )
Hi Alexander,
On Tue, Dec 2, 2014 at 3:15 PM, Alexandre Courbot <[email protected]> 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 <[email protected]>
> Signed-off-by: Alexandre Courbot <[email protected]>
> ---
> v2:
> - Add Geert's Tested-by
> - Only display scary warning on gpio_to_desc() is the GPIO is outside
> the valid range (pre gpio_desc array removal behavior)
Thanks, the scary warnings are gone, together with a few superfluous
"gpio-0 (?): gpiod_request: status -517" messages.
Have a good night!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On Tue, Dec 2, 2014 at 3:15 PM, Alexandre Courbot <[email protected]> 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 <[email protected]>
> Signed-off-by: Alexandre Courbot <[email protected]>
> ---
> v2:
OK threw out v1 and applied this instead.
Yours,
Linus Walleij