[PATCH 0/4] Introduce deferred probing

[PATCH 0/4] Introduce deferred probing

[Sebastian Hesselbarth]

As expected, the Lenovo ix4 has an poor-man's GPIO expander (74x163)
connected by (guess what) GPIO bitbang SPI. This causes driver probing
to fail when the GPIO expander is probed before GPIO SPI driver.

This patch set introduces deferred probing support into base/driver.c
by moving drivers that request probe deferral to an extra device list
that will be reprobed later. As -EPROBE_DEFER is an error in general,
the rest of the patch set amends gpio and led parts to deal with this
special case.

The corresponding drivers and support for Lenovo ix4 will be added in
a third patch set.

Sebastian Hesselbarth (4):
  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

 drivers/base/driver.c       | 42 +++++++++++++++++++++++++++++++++++++++---
 drivers/gpio/gpiolib.c      |  5 ++++-
 drivers/led/led-gpio.c      | 15 ++++++++++++++-
 drivers/of/of_gpio.c        |  5 +++--
 include/asm-generic/errno.h |  1 +
 5 files changed, 61 insertions(+), 7 deletions(-)

---
Cc: barebox@lists.infradead.org
-- 
2.1.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/4] base: Introduce deferred probing

[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 <sebastian.hesselbarth@gmail.com>
---
Cc: barebox@lists.infradead.org
---
 drivers/base/driver.c       | 42 +++++++++++++++++++++++++++++++++++++++---
 include/asm-generic/errno.h |  1 +
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index 590c97c96424..5eac21f3c950 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,33 @@ int unregister_device(struct device_d *old_dev)
 }
 EXPORT_SYMBOL(unregister_device);
 
+static int device_probe_deferred(void)
+{
+	struct device_d *dev, *tmp;
+	struct driver_d *drv;
+	int retries = 10;
+
+	do {
+		if (list_empty(&deferred))
+			break;
+
+		list_for_each_entry_safe(dev, tmp, &deferred, active) {
+			list_del(&dev->active);
+
+			if (dev->bus) {
+				bus_for_each_driver(dev->bus, drv) {
+					if (!match(drv, dev))
+						break;
+				}
+				device_probe(dev);
+			}
+		}
+	} while (retries--);
+
+	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 */
 
-- 
2.1.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/4] base: Introduce deferred probing

[Sascha Hauer]

On Fri, Apr 10, 2015 at 03:02:43AM +0200, Sebastian Hesselbarth wrote:
> 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 <sebastian.hesselbarth@gmail.com>
> ---
> Cc: barebox@lists.infradead.org
> ---
>  drivers/base/driver.c       | 42 +++++++++++++++++++++++++++++++++++++++---
>  include/asm-generic/errno.h |  1 +
>  2 files changed, 40 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index 590c97c96424..5eac21f3c950 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,33 @@ int unregister_device(struct device_d *old_dev)
>  }
>  EXPORT_SYMBOL(unregister_device);
>  
> +static int device_probe_deferred(void)
> +{
> +	struct device_d *dev, *tmp;
> +	struct driver_d *drv;
> +	int retries = 10;
> +
> +	do {
> +		if (list_empty(&deferred))
> +			break;
> +
> +		list_for_each_entry_safe(dev, tmp, &deferred, active) {
> +			list_del(&dev->active);
> +
> +			if (dev->bus) {
> +				bus_for_each_driver(dev->bus, drv) {
> +					if (!match(drv, dev))
> +						break;
> +				}
> +				device_probe(dev);
> +			}
> +		}
> +	} while (retries--);

Instead of a hardcoded loop counter I think this should be "while at least
one device successfully probed". Also if probe fails and the return
value is still -EPROBE_DEFER you have to add the device to the deferred
list again.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/4] base: Introduce deferred probing

[Sebastian Hesselbarth]

On 13.04.2015 08:54, Sascha Hauer wrote:
> On Fri, Apr 10, 2015 at 03:02:43AM +0200, Sebastian Hesselbarth wrote:
>> 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.
[...}
>> +static int device_probe_deferred(void)
>> +{
>> +	struct device_d *dev, *tmp;
>> +	struct driver_d *drv;
>> +	int retries = 10;
>> +
>> +	do {
>> +		if (list_empty(&deferred))
>> +			break;
>> +
>> +		list_for_each_entry_safe(dev, tmp, &deferred, active) {
>> +			list_del(&dev->active);
>> +
>> +			if (dev->bus) {
>> +				bus_for_each_driver(dev->bus, drv) {
>> +					if (!match(drv, dev))
>> +						break;
>> +				}
>> +				device_probe(dev);
>> +			}
>> +		}
>> +	} while (retries--);
>
> Instead of a hardcoded loop counter I think this should be "while at least
> one device successfully probed". Also if probe fails and the return
> value is still -EPROBE_DEFER you have to add the device to the deferred
> list again.

Sascha,

agreed. I'll have another look at how deferred probing is handled here
and resend once I have implemented your comments above.

Sebastian


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/4] gpio: Return -EPROBE_DEFER on gpio_get_num()

[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 <sebastian.hesselbarth@gmail.com>
---
Cc: barebox@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
-- 
2.1.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/4] OF: gpio: Silence error message on -EPROBE_DEFER

[Sebastian Hesselbarth]

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

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: barebox@lists.infradead.org
---
 drivers/of/of_gpio.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/of/of_gpio.c b/drivers/of/of_gpio.c
index 6738a220a5a3..470ccfa1ba77 100644
--- a/drivers/of/of_gpio.c
+++ b/drivers/of/of_gpio.c
@@ -39,8 +39,9 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,
 
 	ret = gpio_get_num(dev, out_args.args[0]);
 	if (ret < 0) {
-		pr_err("%s: unable to get gpio num of device %s: %d\n",
-			__func__, dev_name(dev), ret);
+		if (ret != -EPROBE_DEFER)
+			pr_err("%s: unable to get gpio num of device %s: %d\n",
+			       __func__, dev_name(dev), ret);
 		return ret;
 	}
 
-- 
2.1.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/4] led: gpio: Properly deal with deferred probing

[Sebastian Hesselbarth]

GPIO LEDs can suffer from deferred probing due to failing gpio request.
If of_get_named_gpio_flags returns -EPROBE_DEFER, skip current LED and
request deferred probing later. Since not all LEDs have to fail, build
a mask of already registered LEDs to be skipped later on deferred probe.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: barebox@lists.infradead.org
---
 drivers/led/led-gpio.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/led/led-gpio.c b/drivers/led/led-gpio.c
index ae3f13f45b6c..91e5d5b76085 100644
--- a/drivers/led/led-gpio.c
+++ b/drivers/led/led-gpio.c
@@ -201,6 +201,8 @@ void led_gpio_rgb_unregister(struct gpio_led *led)
 static int led_gpio_of_probe(struct device_d *dev)
 {
 	struct device_node *child;
+	static u32 registered;
+	int ret = 0, n = 0;
 
 	for_each_child_of_node(dev->device_node, child) {
 		struct gpio_led *gled;
@@ -209,7 +211,15 @@ static int led_gpio_of_probe(struct device_d *dev)
 		int gpio;
 		const char *label;
 
+		/* On deferred probing, skip already registered LEDs */
+		if (registered & BIT(n)) {
+			n++;
+			continue;
+		}
+
 		gpio = of_get_named_gpio_flags(child, "gpios", 0, &flags);
+		if (gpio == -EPROBE_DEFER)
+			ret = -EPROBE_DEFER;
 		if (gpio < 0)
 			continue;
 
@@ -233,9 +243,12 @@ static int led_gpio_of_probe(struct device_d *dev)
 			else if (!strcmp(default_state, "off"))
 				led_gpio_set(&gled->led, 0);
 		}
+
+		registered |= BIT(n);
+		n++;
 	}
 
-	return 0;
+	return ret;
 }
 
 static struct of_device_id led_gpio_of_ids[] = {
-- 
2.1.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/4] Introduce deferred probing

[Sascha Hauer]

On Fri, Apr 10, 2015 at 03:02:42AM +0200, Sebastian Hesselbarth wrote:
> As expected, the Lenovo ix4 has an poor-man's GPIO expander (74x163)
> connected by (guess what) GPIO bitbang SPI. This causes driver probing
> to fail when the GPIO expander is probed before GPIO SPI driver.

Hard to get this right without deferred probing. I agree that we reached
the point where deferred probing gets necessary.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox