[PATCH v2 0/6] Introduce deferred probing

[Sebastian Hesselbarth]

This is round two of the introduction of deferred probing by moving
drivers that request probe deferral to an extra device list that will
be re-probed later.

Compared to v1 there are the following changes:
- Loop over deferred device list until none succeeds
(Suggested by Sascha Hauer)
- (Now) properly deal with deferred probing in led-gpio
- Free GPIOs claimed by leds on foo_led_unregister()
- Convert Orion GPIO to real platform driver
Each patch also contains an individual changelog.

It would be great to give this patch series a little more coverage.
Also, several other subsystems could use some support -EPROBE_DEFER
but IMHO we should wait for someone to actually force probe deferral
there.

Sebastian Hesselbarth (6):
base: Introduce deferred probing
gpio: Return -EPROBE_DEFER on gpio_get_num()
OF: gpio: Silence error message on -EPROBE_DEFER
led: gpio: Properly deal with deferred probing
led: gpio: Free GPIOs on unregister()
gpio: orion: Convert to platform_driver

drivers/base/driver.c | 56 ++++++++++++++++++++++++++++++++++++++++++---
drivers/gpio/gpio-orion.c | 7 +-----
drivers/gpio/gpiolib.c | 5 +++-
drivers/led/led-gpio.c | 36 +++++++++++++++++++++++++----
drivers/of/of_gpio.c | 8 ++++---
include/asm-generic/errno.h | 1 +
6 files changed, 96 insertions(+), 17 deletions(-)

---
Cc: ***@lists.infradead.org

[Sebastian Hesselbarth]

GPIO drivers can be registered quite late in registration process
causing dependant devices to fail probing. If we know gpio_get_num
will be called with a non-NULL device, return -EPROBE_DEFER instead
of -ENODEV to allow re-probing later.

Signed-off-by: Sebastian Hesselbarth <***@gmail.com>
---
Changelog:
v1->v2:
- none

Cc: ***@lists.infradead.org
---
drivers/gpio/gpiolib.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 611e41ea5606..1f57c76ec16d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -292,12 +292,15 @@ int gpio_get_num(struct device_d *dev, int gpio)
{
struct gpio_chip *chip;

+ if (!dev)
+ return -ENODEV;
+
list_for_each_entry(chip, &chip_list, list) {
if (chip->dev == dev)
return chip->base + gpio;
}

- return -ENODEV;
+ return -EPROBE_DEFER;
}

#ifdef CONFIG_CMD_GPIO

[Sebastian Hesselbarth]

As expected, we would need deferred probing sooner or later. This is
a first approach to allow devices to return -EPROBE_DEFER and get
sorted into a list of deferred devices that will be re-probed later.

Signed-off-by: Sebastian Hesselbarth <***@gmail.com>
---
Changelog:
v1->v2:
- Rework deferred probing to loop until no previously deferred device
probing succeeds. (Suggested by Sascha Hauer)
- Print an error message for devices stuck in deferred list.
- Add a comment how deferred probing and re-assignment to deferred
list works.

Cc: ***@lists.infradead.org
---
drivers/base/driver.c | 56 ++++++++++++++++++++++++++++++++++++++++++---
include/asm-generic/errno.h | 1 +
2 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index 590c97c96424..6112b4953ba6 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -29,6 +29,7 @@
#include <console.h>
#include <linux/ctype.h>
#include <errno.h>
+#include <init.h>
#include <fs.h>
#include <of.h>
#include <linux/list.h>
@@ -43,6 +44,7 @@ LIST_HEAD(driver_list);
EXPORT_SYMBOL(driver_list);

static LIST_HEAD(active);
+static LIST_HEAD(deferred);

struct device_d *get_device_by_name(const char *name)
{
@@ -88,13 +90,20 @@ int device_probe(struct device_d *dev)
list_add(&dev->active, &active);

ret = dev->bus->probe(dev);
- if (ret) {
+ if (ret == 0)
+ return 0;
+
+ if (ret == -EPROBE_DEFER) {
list_del(&dev->active);
- dev_err(dev, "probe failed: %s\n", strerror(-ret));
+ list_add(&dev->active, &deferred);
+ dev_dbg(dev, "probe deferred\n");
return ret;
}

- return 0;
+ list_del(&dev->active);
+ dev_err(dev, "probe failed: %s\n", strerror(-ret));
+
+ return ret;
}

int device_detect(struct device_d *dev)
@@ -213,6 +222,47 @@ int unregister_device(struct device_d *old_dev)
}
EXPORT_SYMBOL(unregister_device);

+/*
+ * Loop over list of deferred devices as long as at least one
+ * device is successfully probed. Devices that again request
+ * deferral are re-added to deferred list in device_probe().
+ * For devices finally left in deferred list -EPROBE_DEFER
+ * becomes a fatal error.
+ */
+static int device_probe_deferred(void)
+{
+ struct device_d *dev, *tmp;
+ struct driver_d *drv;
+ bool success;
+
+ do {
+ success = false;
+
+ if (list_empty(&deferred))
+ break;
+
+ list_for_each_entry_safe(dev, tmp, &deferred, active) {
+ list_del(&dev->active);
+ dev_dbg(dev, "re-probe device\n");
+ bus_for_each_driver(dev->bus, drv) {
+ if (match(drv, dev))
+ continue;
+ success = true;
+ break;
+ }
+ }
+ } while (success);
+
+ if (list_empty(&deferred))
+ return 0;
+
+ list_for_each_entry(dev, &deferred, active)
+ dev_err(dev, "probe permanently deferred\n");
+
+ return 0;
+}
+late_initcall(device_probe_deferred);
+
struct driver_d *get_driver_by_name(const char *name)
{
struct driver_d *drv;
diff --git a/include/asm-generic/errno.h b/include/asm-generic/errno.h
index bbf493c373ae..6072f7b605bb 100644
--- a/include/asm-generic/errno.h
+++ b/include/asm-generic/errno.h
@@ -132,6 +132,7 @@
#define ERESTARTNOINTR 513
#define ERESTARTNOHAND 514 /* restart if no handler.. */
#define ENOIOCTLCMD 515 /* No ioctl command */
+#define EPROBE_DEFER 517 /* Driver requests probe retry */

#define ENOTSUPP 524 /* Operation is not supported */

[Sebastian Hesselbarth]

With deferred probing, -EPROBE_DEFER is not worth spilling an error.

Signed-off-by: Sebastian Hesselbarth <***@gmail.com>
---
Changelog:
v1->v2:
- Also return early on gpio_get_num() == -EPROBE_DEFER

Cc: ***@lists.infradead.org
---
drivers/of/of_gpio.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/of/of_gpio.c b/drivers/of/of_gpio.c
index 6738a220a5a3..dc8ae2277641 100644
--- a/drivers/of/of_gpio.c
+++ b/drivers/of/of_gpio.c
@@ -32,12 +32,14 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,

dev = of_find_device_by_node(out_args.np);
if (!dev) {
- pr_err("%s: unable to find device of node %s\n",
- __func__, out_args.np->full_name);
- return -ENODEV;
+ pr_debug("%s: unable to find device of node %s\n",
+ __func__, out_args.np->full_name);
+ return -EPROBE_DEFER;
}

ret = gpio_get_num(dev, out_args.args[0]);
+ if (ret == -EPROBE_DEFER)
+ return ret;
if (ret < 0) {
pr_err("%s: unable to get gpio num of device %s: %d\n",
__func__, dev_name(dev), ret);

[Sebastian Hesselbarth]

GPIO LEDs can suffer from deferred probing due to failing gpio request.
Instead of registering each gpio led independently, pre-allocate an
array of struct gpio_led for all and tear it down properly if probing
of one leds fails. While at it, silence error messages on -EPROBE_DEFER.

Signed-off-by: Sebastian Hesselbarth <***@gmail.com>
---
Changelog:
v1->v2:
- Drop static registered bitmap as two "gpio-leds" nodes will both
call led_gpio_of_probe(). The second call will find the static
variable modified and skip some leds.
- Use a pre-allocated array of struct gpio_led instead that will
be torn down on failure.

Cc: ***@lists.infradead.org
---
drivers/led/led-gpio.c | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/led/led-gpio.c b/drivers/led/led-gpio.c
index ae3f13f45b6c..807251721fd3 100644
--- a/drivers/led/led-gpio.c
+++ b/drivers/led/led-gpio.c
@@ -20,6 +20,7 @@
#include <common.h>
#include <init.h>
#include <led.h>
+#include <malloc.h>
#include <gpio.h>
#include <of_gpio.h>

@@ -201,19 +202,32 @@ void led_gpio_rgb_unregister(struct gpio_led *led)
static int led_gpio_of_probe(struct device_d *dev)
{
struct device_node *child;
+ struct gpio_led *leds;
+ int num_leds;
+ int ret = 0, n = 0;
+
+ num_leds = of_get_child_count(dev->device_node);
+ if (num_leds <= 0)
+ return num_leds;
+
+ leds = xzalloc(num_leds * sizeof(struct gpio_led));

for_each_child_of_node(dev->device_node, child) {
- struct gpio_led *gled;
+ struct gpio_led *gled = &leds[n];
const char *default_state;
enum of_gpio_flags flags;
int gpio;
const char *label;

gpio = of_get_named_gpio_flags(child, "gpios", 0, &flags);
- if (gpio < 0)
- continue;
+ if (gpio < 0) {
+ if (gpio != -EPROBE_DEFER)
+ dev_err(dev, "failed to get gpio for %s: %d\n",
+ child->full_name, gpio);
+ ret = gpio;
+ goto err;
+ }

- gled = xzalloc(sizeof(*gled));
if (of_property_read_string(child, "label", &label))
label = child->name;
gled->led.name = xstrdup(label);
@@ -233,9 +247,17 @@ static int led_gpio_of_probe(struct device_d *dev)
else if (!strcmp(default_state, "off"))
led_gpio_set(&gled->led, 0);
}
+
+ n++;
}

return 0;
+
+err:
+ for (n = n - 1; n >= 0; n--)
+ led_gpio_unregister(&leds[n]);
+ free(leds);
+ return ret;
}

static struct of_device_id led_gpio_of_ids[] = {

[Sebastian Hesselbarth]

With support for deferred probing, we can now relax driver
registration for Marvell Orion GPIO driver from postcore_initcall()
to normal platform_driver.

Signed-off-by: Sebastian Hesselbarth <***@gmail.com>
---
Changelog:
v1->v2:
- new patch

Cc: ***@lists.infradead.org
---
drivers/gpio/gpio-orion.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpio/gpio-orion.c b/drivers/gpio/gpio-orion.c
index 855763ea665a..3deeac126f25 100644
--- a/drivers/gpio/gpio-orion.c
+++ b/drivers/gpio/gpio-orion.c
@@ -124,9 +124,4 @@ static struct driver_d orion_gpio_driver = {
.probe = orion_gpio_probe,
.of_compatible = DRV_OF_COMPAT(orion_gpio_dt_ids),
};
-
-static int orion_gpio_init(void)
-{
- return platform_driver_register(&orion_gpio_driver);
-}
-postcore_initcall(orion_gpio_init);
+device_platform_driver(orion_gpio_driver);

[Sebastian Hesselbarth]

Free requested GPIOs on unregistration of mono-, bi-, and tri-color
GPIO leds.

Signed-off-by: Sebastian Hesselbarth <***@gmail.com>
---
Changelog:
v1->v2:
- new patch

Cc: ***@lists.infradead.org
---
drivers/led/led-gpio.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/led/led-gpio.c b/drivers/led/led-gpio.c
index 807251721fd3..74be2304e868 100644
--- a/drivers/led/led-gpio.c
+++ b/drivers/led/led-gpio.c
@@ -64,6 +64,7 @@ int led_gpio_register(struct gpio_led *led)
void led_gpio_unregister(struct gpio_led *led)
{
led_unregister(&led->led);
+ gpio_free(led->gpio);
}

#ifdef CONFIG_LED_GPIO_BICOLOR
@@ -131,6 +132,8 @@ err_gpio_c0:
void led_gpio_bicolor_unregister(struct gpio_bicolor_led *led)
{
led_unregister(&led->led);
+ gpio_free(led->gpio_c1);
+ gpio_free(led->gpio_c0);
}
#endif

@@ -195,6 +198,9 @@ err_gpio_r:
void led_gpio_rgb_unregister(struct gpio_led *led)
{
led_unregister(&led->led);
+ gpio_free(led->gpio_b);
+ gpio_free(led->gpio_g);
+ gpio_free(led->gpio_r);
}
#endif /* CONFIG_LED_GPIO_RGB */

[Sascha Hauer]

Post by Sebastian Hesselbarth
This is round two of the introduction of deferred probing by moving
drivers that request probe deferral to an extra device list that will
be re-probed later.
- Loop over deferred device list until none succeeds
(Suggested by Sascha Hauer)
- (Now) properly deal with deferred probing in led-gpio
- Free GPIOs claimed by leds on foo_led_unregister()
- Convert Orion GPIO to real platform driver
Each patch also contains an individual changelog.
It would be great to give this patch series a little more coverage.
Also, several other subsystems could use some support -EPROBE_DEFER
but IMHO we should wait for someone to actually force probe deferral
there.
base: Introduce deferred probing
gpio: Return -EPROBE_DEFER on gpio_get_num()
OF: gpio: Silence error message on -EPROBE_DEFER
led: gpio: Properly deal with deferred probing
led: gpio: Free GPIOs on unregister()
gpio: orion: Convert to platform_driver

Applied, thanks

Sascha