gpio: remove gpio_descs global array

PATCH In review Alexandre Courbot 4 participants 13 messages Nov 19, 2014 – Dec 2, 2014

Subsystems (2)

GPIO SuperH

lore

Share & tools

lore

Alexandre Courbot

author

2014-11-19 07:51 UTC #0 lore raw

PATCH 1/1 gpio: remove gpio_descs global array

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

drivers/gpio/gpiolib.c +31 ~~-16~~

index eb739a51e774..5619922ebf44 100644
@@ -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);

Signature · 1 line

2.1.3

Linus Walleij

2014-11-28 10:29 UTC #1 lore raw

Quoted 11 lines · Alexandre Courbot

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.

Quoted 6 lines

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

Geert Uytterhoeven

2014-12-02 10:32 UTC #2 lore raw

Hi Linus, Alexandre,

Quoted 12 lines · Linus Walleij

Quoted 11 lines · Alexandre Courbot
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.

Quoted 8 lines

Quoted 6 lines
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

Alexandre Courbot

2014-12-02 13:25 UTC #3 lore raw

Quoted 21 lines · Geert Uytterhoeven

Hi Linus, Alexandre,
Quoted 17 lines · Linus Walleij
Quoted 16 lines · Alexandre Courbot
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...

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
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.

Linus Walleij

2014-12-02 13:26 UTC #4 lore raw

Quoted 10 lines · Alexandre Courbot

Quoted 3 lines · Geert Uytterhoeven
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

Alexandre Courbot

author

2014-12-02 13:42 UTC #5 lore raw

PATCH 1/1 gpio: fix deferred probe detection for legacy API

In reply to Alexandre Courbot

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

drivers/gpio/gpiolib-legacy.c +11 -1

index 078ae6c..8b83099 100644
@@ -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);

Signature · 1 line

2.1.3

Geert Uytterhoeven

Tested-by on v1 1/1

2014-12-02 14:11 UTC #6 lore raw

Re: [PATCH] gpio: fix deferred probe detection for legacy API

Hi Alexandre,

Quoted 12 lines · Alexandre Courbot

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.

Quoted 6 lines

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

Linus Walleij

2014-12-02 14:19 UTC #7 lore raw

Re: [PATCH] gpio: fix deferred probe detection for legacy API

In reply to Alexandre Courbot

Quoted 12 lines · Alexandre Courbot

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.

Quoted 6 lines

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

Alexandre Courbot

2014-12-02 14:20 UTC #8 lore raw

Re: [PATCH] gpio: fix deferred probe detection for legacy API

Quoted 14 lines · Linus Walleij

Quoted 12 lines · Alexandre Courbot
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.

Quoted 8 lines

Quoted 6 lines
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).

Alexandre Courbot

2014-12-02 14:22 UTC #9 lore raw

Re: [PATCH] gpio: fix deferred probe detection for legacy API

Quoted 19 lines · Alexandre Courbot

Quoted 18 lines · Linus Walleij
Quoted 16 lines · Alexandre Courbot
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.

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
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... -_- )

Alexandre Courbot

author

2014-12-02 14:20 UTC #10 lore raw

PATCH v2 1/1 gpio: fix deferred probe detection for legacy API

In reply to Alexandre Courbot

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

drivers/gpio/gpiolib-legacy.c +11 -1

index 078ae6c..8b83099 100644
@@ -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);

drivers/gpio/gpiolib.c +3 -1

index 0b271ef8..56b7c5d 100644
@@ -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);

Signature · 1 line

2.1.3

Geert Uytterhoeven

2014-12-02 14:30 UTC #11 lore raw

Re: [PATCH v2] gpio: fix deferred probe detection for legacy API

Hi Alexander,

Quoted 16 lines · Alexandre Courbot

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.

Quoted 7 lines

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

Linus Walleij

2014-12-02 14:48 UTC #12 lore raw

Re: [PATCH v2] gpio: fix deferred probe detection for legacy API

In reply to Alexandre Courbot

Quoted 12 lines · Alexandre Courbot

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.

Quoted 8 lines

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