All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-22 18:51 ` Grant Likely
  0 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-09-22 18:51 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Greg Kroah-Hartman, Dilan Lee, Mark Brown, Manjunath GKondaiah,
	Arnd Bergmann

Allow drivers to report at probe time that they cannot get all the resources
required by the device, and should be retried at a later time.

This should completely solve the problem of getting devices
initialized in the right order.  Right now this is mostly handled by
mucking about with initcall ordering which is a complete hack, and
doesn't even remotely handle the case where device drivers are in
modules.  This approach completely sidesteps the issues by allowing
driver registration to occur in any order, and any driver can request
to be retried after a few more other drivers get probed.

v3: - Hold off workqueue scheduling until late_initcall so that the bulk
      of driver probes are complete before we start retrying deferred devices.
    - Tested with simple use cases.  Still needs more testing though.
      Using it to get rid of the gpio early_initcall madness, or to replace
      the ASoC internal probe deferral code would be ideal.
v2: - added locking so it should no longer be utterly broken in that regard
    - remove device from deferred list at device_del time.
    - Still completely untested with any real use case, but has been
      boot tested.

TODO: - Create a separate singlethread_workqueue so that drivers can't
        mess things up by calling flush_work().
      - Maybe this should be wrapped with a kconfig symbol so it can
        be compiled out on systems that don't care.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Greg Kroah-Hartman <greg@kroah.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Dilan Lee <dilee@nvidia.com>
Cc: Manjunath GKondaiah <manjunath.gkondaiah@linaro.org>
---

Hi Manjunath,

Here's the current state of the patch.  The major think that needs to
be done is to convert it to use a separate workqueue as described in
the TODO above.  It also needs some users adapted to it.  One of the
gpio drivers would work; preferably one of the newer drivers that
doesn't have a lot of drivers depending on the early_initcall()
behaviour yet.

Mark Brown may also be able to suggest specific examples.

For everyone else; this is the current state of the patch.  I think it
is in pretty good shape other than the TODO item above.  I'm turning
it over to Manjunath to  polish up for merging.  I would appreciate
any feedback.

g.


 drivers/base/base.h    |    1 
 drivers/base/core.c    |    2 +
 drivers/base/dd.c      |  134 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/device.h |    5 ++
 4 files changed, 141 insertions(+), 1 deletions(-)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index a34dca0..9641309 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -105,6 +105,7 @@ extern void bus_remove_driver(struct device_driver *drv);
 
 extern void driver_detach(struct device_driver *drv);
 extern int driver_probe_device(struct device_driver *drv, struct device *dev);
+extern void driver_deferred_probe_del(struct device *dev);
 static inline int driver_match_device(struct device_driver *drv,
 				      struct device *dev)
 {
diff --git a/drivers/base/core.c b/drivers/base/core.c
index bc8729d..0d37e18 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -588,6 +588,7 @@ void device_initialize(struct device *dev)
 {
 	dev->kobj.kset = devices_kset;
 	kobject_init(&dev->kobj, &device_ktype);
+	INIT_LIST_HEAD(&dev->deferred_probe);
 	INIT_LIST_HEAD(&dev->dma_pools);
 	mutex_init(&dev->mutex);
 	lockdep_set_novalidate_class(&dev->mutex);
@@ -1119,6 +1120,7 @@ void device_del(struct device *dev)
 	device_remove_file(dev, &uevent_attr);
 	device_remove_attrs(dev);
 	bus_remove_device(dev);
+	driver_deferred_probe_del(dev);
 
 	/*
 	 * Some platform devices are driven without driver attached
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 6658da7..8be5b33 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -28,6 +28,129 @@
 #include "base.h"
 #include "power/power.h"
 
+/*
+ * Deferred Probe infrastructure.
+ *
+ * Sometimes driver probe order matters, but the kernel doesn't always have
+ * dependency information which means some drivers will get probed before a
+ * resource it depends on is available.  For example, an SDHCI driver may
+ * first need a GPIO line from an i2c GPIO controller before it can be
+ * initialized.  If a required resource is not available yet, a driver can
+ * request probing to be deferred by returning -EAGAIN from its probe hook
+ *
+ * Deferred probe maintains two lists of devices, a pending list and an active
+ * list.  A driver returning -EAGAIN causes the device to be added to the
+ * pending list.
+ *
+ * The deferred_probe_mutex *must* be held any time the deferred_probe_*_list
+ * of the (struct device*)->deferred_probe pointers are manipulated
+ */
+static DEFINE_MUTEX(deferred_probe_mutex);
+static LIST_HEAD(deferred_probe_pending_list);
+static LIST_HEAD(deferred_probe_active_list);
+
+/**
+ * deferred_probe_work_func() - Retry probing devices in the active list.
+ */
+static void deferred_probe_work_func(struct work_struct *work)
+{
+	struct device *dev;
+	/*
+	 * This bit is tricky.  We want to process every device in the
+	 * deferred list, but devices can be removed from the list at any
+	 * time while inside this for-each loop.  There are two things that
+	 * need to be protected against:
+	 * - if the device is removed from the deferred_probe_list, then we
+	 *   loose our place in the loop.  Since any device can be removed
+	 *   asynchronously, list_for_each_entry_safe() wouldn't make things
+	 *   much better.  Simplest solution is to restart walking the list
+	 *   whenever the current device gets removed.  Not the most efficient,
+	 *   but is simple to implement and easy to audit for correctness.
+	 * - if the device is unregistered, and freed, then there is a risk
+	 *   of a null pointer dereference.  This code uses get/put_device()
+	 *   to ensure the device cannot disappear from under our feet.
+	 */
+	mutex_lock(&deferred_probe_mutex);
+	while (!list_empty(&deferred_probe_active_list)) {
+		dev = list_first_entry(&deferred_probe_active_list,
+					typeof(*dev), deferred_probe);
+		list_del_init(&dev->deferred_probe);
+
+		get_device(dev);
+
+		/* Drop the mutex while probing each device; the probe path
+		 * may manipulate the deferred list */
+		mutex_unlock(&deferred_probe_mutex);
+		dev_dbg(dev, "Retrying from deferred list\n");
+		bus_probe_device(dev);
+		mutex_lock(&deferred_probe_mutex);
+
+		put_device(dev);
+	}
+	mutex_unlock(&deferred_probe_mutex);
+}
+static DECLARE_WORK(deferred_probe_work, deferred_probe_work_func);
+
+static void driver_deferred_probe_add(struct device *dev)
+{
+	mutex_lock(&deferred_probe_mutex);
+	if (list_empty(&dev->deferred_probe)) {
+		dev_dbg(dev, "Added to deferred list\n");
+		list_add(&dev->deferred_probe, &deferred_probe_pending_list);
+	}
+	mutex_unlock(&deferred_probe_mutex);
+}
+
+void driver_deferred_probe_del(struct device *dev)
+{
+	mutex_lock(&deferred_probe_mutex);
+	if (!list_empty(&dev->deferred_probe)) {
+		dev_dbg(dev, "Removed from deferred list\n");
+		list_del_init(&dev->deferred_probe);
+	}
+	mutex_unlock(&deferred_probe_mutex);
+}
+
+static bool driver_deferred_probe_enable = false;
+/**
+ * driver_deferred_probe_trigger() - Kick off re-probing deferred devices
+ *
+ * This functions moves all devices from the pending list to the active
+ * list and schedules the deferred probe workqueue to process them.  It
+ * should be called anytime a driver is successfully bound to a device.
+ */
+static void driver_deferred_probe_trigger(void)
+{
+	if (!driver_deferred_probe_enable)
+		return;
+
+	/* A successful probe means that all the devices in the pending list
+	 * should be triggered to be reprobed.  Move all the deferred devices
+	 * into the active list so they can be retried by the workqueue */
+	mutex_lock(&deferred_probe_mutex);
+	list_splice_tail_init(&deferred_probe_pending_list,
+			      &deferred_probe_active_list);
+	mutex_unlock(&deferred_probe_mutex);
+
+	/* Kick the re-probe thread.  It may already be scheduled, but
+	 * it is safe to kick it again. */
+	schedule_work(&deferred_probe_work);
+}
+
+/**
+ * deferred_probe_initcall() - Enable probing of deferred devices
+ *
+ * We don't want to get in the way when the bulk of drivers are getting probed.
+ * Instead, this initcall makes sure that deferred probing is delayed until
+ * late_initcall time.
+ */
+static int deferred_probe_initcall(void)
+{
+	driver_deferred_probe_enable = true;
+	driver_deferred_probe_trigger();
+	return 0;
+}
+late_initcall(deferred_probe_initcall);
 
 static void driver_bound(struct device *dev)
 {
@@ -42,6 +165,11 @@ static void driver_bound(struct device *dev)
 
 	klist_add_tail(&dev->p->knode_driver, &dev->driver->p->klist_devices);
 
+	/* Make sure the device is no longer in one of the deferred lists
+	 * and kick off retrying all pending devices */
+	driver_deferred_probe_del(dev);
+	driver_deferred_probe_trigger();
+
 	if (dev->bus)
 		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
 					     BUS_NOTIFY_BOUND_DRIVER, dev);
@@ -142,7 +270,11 @@ probe_failed:
 	driver_sysfs_remove(dev);
 	dev->driver = NULL;
 
-	if (ret != -ENODEV && ret != -ENXIO) {
+	if (ret == -EAGAIN) {
+		/* Driver requested deferred probing */
+		dev_info(dev, "Driver %s requests probe deferral\n", drv->name);
+		driver_deferred_probe_add(dev);
+	} else if (ret != -ENODEV && ret != -ENXIO) {
 		/* driver matched but the probe failed */
 		printk(KERN_WARNING
 		       "%s: probe of %s failed with error %d\n",
diff --git a/include/linux/device.h b/include/linux/device.h
index c20dfbf..dab93a4 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -506,6 +506,10 @@ struct device_dma_parameters {
  * @mutex:	Mutex to synchronize calls to its driver.
  * @bus:	Type of bus device is on.
  * @driver:	Which driver has allocated this
+ * @deferred_probe: entry in deferred_probe_list which is used to retry the
+ * 		binding of drivers which were unable to get all the resources
+ * 		needed by the device; typically because it depends on another
+ * 		driver getting probed first.
  * @platform_data: Platform data specific to the device.
  * 		Example: For devices on custom boards, as typical of embedded
  * 		and SOC based hardware, Linux often uses platform_data to point
@@ -564,6 +568,7 @@ struct device {
 	struct bus_type	*bus;		/* type of bus device is on */
 	struct device_driver *driver;	/* which driver has allocated this
 					   device */
+	struct list_head	deferred_probe;
 	void		*platform_data;	/* Platform specific data, device
 					   core doesn't touch it */
 	struct dev_pm_info	power;


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-22 18:51 ` Grant Likely
  0 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-09-22 18:51 UTC (permalink / raw)
  To: linux-arm-kernel

Allow drivers to report at probe time that they cannot get all the resources
required by the device, and should be retried at a later time.

This should completely solve the problem of getting devices
initialized in the right order.  Right now this is mostly handled by
mucking about with initcall ordering which is a complete hack, and
doesn't even remotely handle the case where device drivers are in
modules.  This approach completely sidesteps the issues by allowing
driver registration to occur in any order, and any driver can request
to be retried after a few more other drivers get probed.

v3: - Hold off workqueue scheduling until late_initcall so that the bulk
      of driver probes are complete before we start retrying deferred devices.
    - Tested with simple use cases.  Still needs more testing though.
      Using it to get rid of the gpio early_initcall madness, or to replace
      the ASoC internal probe deferral code would be ideal.
v2: - added locking so it should no longer be utterly broken in that regard
    - remove device from deferred list at device_del time.
    - Still completely untested with any real use case, but has been
      boot tested.

TODO: - Create a separate singlethread_workqueue so that drivers can't
        mess things up by calling flush_work().
      - Maybe this should be wrapped with a kconfig symbol so it can
        be compiled out on systems that don't care.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Greg Kroah-Hartman <greg@kroah.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Dilan Lee <dilee@nvidia.com>
Cc: Manjunath GKondaiah <manjunath.gkondaiah@linaro.org>
---

Hi Manjunath,

Here's the current state of the patch.  The major think that needs to
be done is to convert it to use a separate workqueue as described in
the TODO above.  It also needs some users adapted to it.  One of the
gpio drivers would work; preferably one of the newer drivers that
doesn't have a lot of drivers depending on the early_initcall()
behaviour yet.

Mark Brown may also be able to suggest specific examples.

For everyone else; this is the current state of the patch.  I think it
is in pretty good shape other than the TODO item above.  I'm turning
it over to Manjunath to  polish up for merging.  I would appreciate
any feedback.

g.


 drivers/base/base.h    |    1 
 drivers/base/core.c    |    2 +
 drivers/base/dd.c      |  134 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/device.h |    5 ++
 4 files changed, 141 insertions(+), 1 deletions(-)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index a34dca0..9641309 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -105,6 +105,7 @@ extern void bus_remove_driver(struct device_driver *drv);
 
 extern void driver_detach(struct device_driver *drv);
 extern int driver_probe_device(struct device_driver *drv, struct device *dev);
+extern void driver_deferred_probe_del(struct device *dev);
 static inline int driver_match_device(struct device_driver *drv,
 				      struct device *dev)
 {
diff --git a/drivers/base/core.c b/drivers/base/core.c
index bc8729d..0d37e18 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -588,6 +588,7 @@ void device_initialize(struct device *dev)
 {
 	dev->kobj.kset = devices_kset;
 	kobject_init(&dev->kobj, &device_ktype);
+	INIT_LIST_HEAD(&dev->deferred_probe);
 	INIT_LIST_HEAD(&dev->dma_pools);
 	mutex_init(&dev->mutex);
 	lockdep_set_novalidate_class(&dev->mutex);
@@ -1119,6 +1120,7 @@ void device_del(struct device *dev)
 	device_remove_file(dev, &uevent_attr);
 	device_remove_attrs(dev);
 	bus_remove_device(dev);
+	driver_deferred_probe_del(dev);
 
 	/*
 	 * Some platform devices are driven without driver attached
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 6658da7..8be5b33 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -28,6 +28,129 @@
 #include "base.h"
 #include "power/power.h"
 
+/*
+ * Deferred Probe infrastructure.
+ *
+ * Sometimes driver probe order matters, but the kernel doesn't always have
+ * dependency information which means some drivers will get probed before a
+ * resource it depends on is available.  For example, an SDHCI driver may
+ * first need a GPIO line from an i2c GPIO controller before it can be
+ * initialized.  If a required resource is not available yet, a driver can
+ * request probing to be deferred by returning -EAGAIN from its probe hook
+ *
+ * Deferred probe maintains two lists of devices, a pending list and an active
+ * list.  A driver returning -EAGAIN causes the device to be added to the
+ * pending list.
+ *
+ * The deferred_probe_mutex *must* be held any time the deferred_probe_*_list
+ * of the (struct device*)->deferred_probe pointers are manipulated
+ */
+static DEFINE_MUTEX(deferred_probe_mutex);
+static LIST_HEAD(deferred_probe_pending_list);
+static LIST_HEAD(deferred_probe_active_list);
+
+/**
+ * deferred_probe_work_func() - Retry probing devices in the active list.
+ */
+static void deferred_probe_work_func(struct work_struct *work)
+{
+	struct device *dev;
+	/*
+	 * This bit is tricky.  We want to process every device in the
+	 * deferred list, but devices can be removed from the list at any
+	 * time while inside this for-each loop.  There are two things that
+	 * need to be protected against:
+	 * - if the device is removed from the deferred_probe_list, then we
+	 *   loose our place in the loop.  Since any device can be removed
+	 *   asynchronously, list_for_each_entry_safe() wouldn't make things
+	 *   much better.  Simplest solution is to restart walking the list
+	 *   whenever the current device gets removed.  Not the most efficient,
+	 *   but is simple to implement and easy to audit for correctness.
+	 * - if the device is unregistered, and freed, then there is a risk
+	 *   of a null pointer dereference.  This code uses get/put_device()
+	 *   to ensure the device cannot disappear from under our feet.
+	 */
+	mutex_lock(&deferred_probe_mutex);
+	while (!list_empty(&deferred_probe_active_list)) {
+		dev = list_first_entry(&deferred_probe_active_list,
+					typeof(*dev), deferred_probe);
+		list_del_init(&dev->deferred_probe);
+
+		get_device(dev);
+
+		/* Drop the mutex while probing each device; the probe path
+		 * may manipulate the deferred list */
+		mutex_unlock(&deferred_probe_mutex);
+		dev_dbg(dev, "Retrying from deferred list\n");
+		bus_probe_device(dev);
+		mutex_lock(&deferred_probe_mutex);
+
+		put_device(dev);
+	}
+	mutex_unlock(&deferred_probe_mutex);
+}
+static DECLARE_WORK(deferred_probe_work, deferred_probe_work_func);
+
+static void driver_deferred_probe_add(struct device *dev)
+{
+	mutex_lock(&deferred_probe_mutex);
+	if (list_empty(&dev->deferred_probe)) {
+		dev_dbg(dev, "Added to deferred list\n");
+		list_add(&dev->deferred_probe, &deferred_probe_pending_list);
+	}
+	mutex_unlock(&deferred_probe_mutex);
+}
+
+void driver_deferred_probe_del(struct device *dev)
+{
+	mutex_lock(&deferred_probe_mutex);
+	if (!list_empty(&dev->deferred_probe)) {
+		dev_dbg(dev, "Removed from deferred list\n");
+		list_del_init(&dev->deferred_probe);
+	}
+	mutex_unlock(&deferred_probe_mutex);
+}
+
+static bool driver_deferred_probe_enable = false;
+/**
+ * driver_deferred_probe_trigger() - Kick off re-probing deferred devices
+ *
+ * This functions moves all devices from the pending list to the active
+ * list and schedules the deferred probe workqueue to process them.  It
+ * should be called anytime a driver is successfully bound to a device.
+ */
+static void driver_deferred_probe_trigger(void)
+{
+	if (!driver_deferred_probe_enable)
+		return;
+
+	/* A successful probe means that all the devices in the pending list
+	 * should be triggered to be reprobed.  Move all the deferred devices
+	 * into the active list so they can be retried by the workqueue */
+	mutex_lock(&deferred_probe_mutex);
+	list_splice_tail_init(&deferred_probe_pending_list,
+			      &deferred_probe_active_list);
+	mutex_unlock(&deferred_probe_mutex);
+
+	/* Kick the re-probe thread.  It may already be scheduled, but
+	 * it is safe to kick it again. */
+	schedule_work(&deferred_probe_work);
+}
+
+/**
+ * deferred_probe_initcall() - Enable probing of deferred devices
+ *
+ * We don't want to get in the way when the bulk of drivers are getting probed.
+ * Instead, this initcall makes sure that deferred probing is delayed until
+ * late_initcall time.
+ */
+static int deferred_probe_initcall(void)
+{
+	driver_deferred_probe_enable = true;
+	driver_deferred_probe_trigger();
+	return 0;
+}
+late_initcall(deferred_probe_initcall);
 
 static void driver_bound(struct device *dev)
 {
@@ -42,6 +165,11 @@ static void driver_bound(struct device *dev)
 
 	klist_add_tail(&dev->p->knode_driver, &dev->driver->p->klist_devices);
 
+	/* Make sure the device is no longer in one of the deferred lists
+	 * and kick off retrying all pending devices */
+	driver_deferred_probe_del(dev);
+	driver_deferred_probe_trigger();
+
 	if (dev->bus)
 		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
 					     BUS_NOTIFY_BOUND_DRIVER, dev);
@@ -142,7 +270,11 @@ probe_failed:
 	driver_sysfs_remove(dev);
 	dev->driver = NULL;
 
-	if (ret != -ENODEV && ret != -ENXIO) {
+	if (ret == -EAGAIN) {
+		/* Driver requested deferred probing */
+		dev_info(dev, "Driver %s requests probe deferral\n", drv->name);
+		driver_deferred_probe_add(dev);
+	} else if (ret != -ENODEV && ret != -ENXIO) {
 		/* driver matched but the probe failed */
 		printk(KERN_WARNING
 		       "%s: probe of %s failed with error %d\n",
diff --git a/include/linux/device.h b/include/linux/device.h
index c20dfbf..dab93a4 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -506,6 +506,10 @@ struct device_dma_parameters {
  * @mutex:	Mutex to synchronize calls to its driver.
  * @bus:	Type of bus device is on.
  * @driver:	Which driver has allocated this
+ * @deferred_probe: entry in deferred_probe_list which is used to retry the
+ * 		binding of drivers which were unable to get all the resources
+ * 		needed by the device; typically because it depends on another
+ * 		driver getting probed first.
  * @platform_data: Platform data specific to the device.
  * 		Example: For devices on custom boards, as typical of embedded
  * 		and SOC based hardware, Linux often uses platform_data to point
@@ -564,6 +568,7 @@ struct device {
 	struct bus_type	*bus;		/* type of bus device is on */
 	struct device_driver *driver;	/* which driver has allocated this
 					   device */
+	struct list_head	deferred_probe;
 	void		*platform_data;	/* Platform specific data, device
 					   core doesn't touch it */
 	struct dev_pm_info	power;

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-22 18:51 ` Grant Likely
@ 2011-09-22 18:58   ` Joe Perches
  -1 siblings, 0 replies; 63+ messages in thread
From: Joe Perches @ 2011-09-22 18:58 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, linux-arm-kernel, Greg Kroah-Hartman, Dilan Lee,
	Manjunath GKondaiah, Arnd Bergmann, Mark Brown

On Thu, 2011-09-22 at 12:51 -0600, Grant Likely wrote:
> Allow drivers to report at probe time that they cannot get all the resources
> required by the device, and should be retried at a later time.
[]
> I would appreciate
> any feedback.

trivia:

> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
[]
> +static void driver_deferred_probe_add(struct device *dev)
> +{
> +	mutex_lock(&deferred_probe_mutex);
> +	if (list_empty(&dev->deferred_probe)) {
> +		dev_dbg(dev, "Added to deferred list\n");

These messages might be more intelligible as
"Added to deferred probe list\n"

> +void driver_deferred_probe_del(struct device *dev)
> +{
> +	mutex_lock(&deferred_probe_mutex);
> +	if (!list_empty(&dev->deferred_probe)) {
> +		dev_dbg(dev, "Removed from deferred list\n");

"Removed from deferred probe list\n"



^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-22 18:58   ` Joe Perches
  0 siblings, 0 replies; 63+ messages in thread
From: Joe Perches @ 2011-09-22 18:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 2011-09-22 at 12:51 -0600, Grant Likely wrote:
> Allow drivers to report at probe time that they cannot get all the resources
> required by the device, and should be retried at a later time.
[]
> I would appreciate
> any feedback.

trivia:

> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
[]
> +static void driver_deferred_probe_add(struct device *dev)
> +{
> +	mutex_lock(&deferred_probe_mutex);
> +	if (list_empty(&dev->deferred_probe)) {
> +		dev_dbg(dev, "Added to deferred list\n");

These messages might be more intelligible as
"Added to deferred probe list\n"

> +void driver_deferred_probe_del(struct device *dev)
> +{
> +	mutex_lock(&deferred_probe_mutex);
> +	if (!list_empty(&dev->deferred_probe)) {
> +		dev_dbg(dev, "Removed from deferred list\n");

"Removed from deferred probe list\n"

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-22 18:51 ` Grant Likely
  (?)
  (?)
@ 2011-09-22 19:28 ` David Daney
  -1 siblings, 0 replies; 63+ messages in thread
From: David Daney @ 2011-09-22 19:28 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, linux-arm-kernel, Greg Kroah-Hartman, Dilan Lee,
	Mark Brown, Manjunath GKondaiah, Arnd Bergmann

Thanks Grant, This is sorely needed.

On 09/22/2011 11:51 AM, Grant Likely wrote:
> Allow drivers to report at probe time that they cannot get all the resources
> required by the device, and should be retried at a later time.
>
> This should completely solve the problem of getting devices
> initialized in the right order.  Right now this is mostly handled by
> mucking about with initcall ordering which is a complete hack, and
> doesn't even remotely handle the case where device drivers are in
> modules.  This approach completely sidesteps the issues by allowing
> driver registration to occur in any order, and any driver can request
> to be retried after a few more other drivers get probed.
>
> v3: - Hold off workqueue scheduling until late_initcall so that the bulk
>        of driver probes are complete before we start retrying deferred devices.
>      - Tested with simple use cases.  Still needs more testing though.
>        Using it to get rid of the gpio early_initcall madness, or to replace
>        the ASoC internal probe deferral code would be ideal.
> v2: - added locking so it should no longer be utterly broken in that regard
>      - remove device from deferred list at device_del time.
>      - Still completely untested with any real use case, but has been
>        boot tested.
>
> TODO: - Create a separate singlethread_workqueue so that drivers can't
>          mess things up by calling flush_work().
>        - Maybe this should be wrapped with a kconfig symbol so it can
>          be compiled out on systems that don't care.
>
> Signed-off-by: Grant Likely<grant.likely@secretlab.ca>
> Cc: Greg Kroah-Hartman<greg@kroah.com>
> Cc: Mark Brown<broonie@opensource.wolfsonmicro.com>
> Cc: Arnd Bergmann<arnd@arndb.de>
> Cc: Dilan Lee<dilee@nvidia.com>
> Cc: Manjunath GKondaiah<manjunath.gkondaiah@linaro.org>

FWIW:

Acked-by: David Daney <david.daney@cavium.com>


> ---
>
> Hi Manjunath,
>
> Here's the current state of the patch.  The major think that needs to
> be done is to convert it to use a separate workqueue as described in
> the TODO above.  It also needs some users adapted to it.  One of the
> gpio drivers would work; preferably one of the newer drivers that
> doesn't have a lot of drivers depending on the early_initcall()
> behaviour yet.
>
> Mark Brown may also be able to suggest specific examples.
>

I know I have an example:

My MDIO and I2C bus multiplexer patches are not yet merged, but they are 
relevant to this.


Consider a Ethernet device driver, it needs to communicate with PHY 
devices and its driver cannot be be initialized until the PHY drivers 
have been initialized.

However these PHYs are on an multiplexed MDIO bus controlled by GPIO 
pins.  We cannot initialize the PHY drivers until the MDIO bus is 
initialized.

Wait there is more...  The GPIO pins controlling the MDIO bus 
multiplexer are on an I2C controlled GPIO expander and cannot be used 
until the I2C system is up and the drivers initialized.

So we have this driver initialization dependency:

Ethernet driver -> PHY/MDIO -> GPIO/I2C

We cannot really have a static initialization order because the 
relationships are board dependent and there are more levels than can be 
achieved with *_initcall().  On some boards the PHYs are directly 
connected so there is no multiplexer.  On others there is a MDIO 
multiplexer, but it is controlled by GPIO pins on the SOC and are thus 
available when the GPIO subsystem is up rather than I2C.


Without something like this patch, we have a house of cards just waiting 
to collapse.


> For everyone else; this is the current state of the patch.  I think it
> is in pretty good shape other than the TODO item above.  I'm turning
> it over to Manjunath to  polish up for merging.  I would appreciate
> any feedback.
>
> g.
>
>
>   drivers/base/base.h    |    1
>   drivers/base/core.c    |    2 +
>   drivers/base/dd.c      |  134 ++++++++++++++++++++++++++++++++++++++++++++++++
>   include/linux/device.h |    5 ++
>   4 files changed, 141 insertions(+), 1 deletions(-)
[...]


^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-22 18:51 ` Grant Likely
@ 2011-09-22 20:29   ` Alan Cox
  -1 siblings, 0 replies; 63+ messages in thread
From: Alan Cox @ 2011-09-22 20:29 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, linux-arm-kernel, Greg Kroah-Hartman, Dilan Lee,
	Mark Brown, Manjunath GKondaiah, Arnd Bergmann

Definitely what is needed for some of the x86 SoC stuff and would let us
rip out some of the special case magic for the SCU discovery.

First thing that strikes me is driver_bound kicks the processing queue
again. That seems odd - surely this isn't needed because any driver that
does initialise this time and may allow something else to get going will
queue the kick itself. Thus this seems to just add overhead.

It all looks a bit O(N²) if we don't expect the drivers that might
trigger something else binding to just say 'hey I'm one of the
troublemakers'

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-22 20:29   ` Alan Cox
  0 siblings, 0 replies; 63+ messages in thread
From: Alan Cox @ 2011-09-22 20:29 UTC (permalink / raw)
  To: linux-arm-kernel

Definitely what is needed for some of the x86 SoC stuff and would let us
rip out some of the special case magic for the SCU discovery.

First thing that strikes me is driver_bound kicks the processing queue
again. That seems odd - surely this isn't needed because any driver that
does initialise this time and may allow something else to get going will
queue the kick itself. Thus this seems to just add overhead.

It all looks a bit O(N?) if we don't expect the drivers that might
trigger something else binding to just say 'hey I'm one of the
troublemakers'

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-22 20:29   ` Alan Cox
@ 2011-09-22 21:19     ` Grant Likely
  -1 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-09-22 21:19 UTC (permalink / raw)
  To: Alan Cox
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Mark Brown,
	Manjunath GKondaiah, linux-kernel, Dilan Lee, linux-arm-kernel

On Thu, Sep 22, 2011 at 2:29 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> Definitely what is needed for some of the x86 SoC stuff and would let us
> rip out some of the special case magic for the SCU discovery.
>
> First thing that strikes me is driver_bound kicks the processing queue
> again. That seems odd - surely this isn't needed because any driver that
> does initialise this time and may allow something else to get going will
> queue the kick itself. Thus this seems to just add overhead.
>
> It all looks a bit O(N²) if we don't expect the drivers that might
> trigger something else binding to just say 'hey I'm one of the
> troublemakers'

The way I read it, absolute worst case is when every device but one
depends on another device.  In that case I believe it will be
O(Nlog(N)).  (Every device gets probed on the first pass, but only the
last one gets probed.  Then it goes through N-1 devices to the result
of only 1 more device getting probed, then N-2, etc.).  Actual usage
won't be anywhere near that complexity because there tends to be a
small number of devices on these SoCs that a lot of other devices
depend on, so I expect that there will be somewhere on the order of
3-4 passes for the typical embedded SoCs that I've seen.

driver_bound is used to kick off the process because anytime a new
device becomes active (bound to a driver), there is a high likelyhood
that one of the devices in the pending list can now be probed.
However, the re-probing is performed in a workqueue to make sure that
it is somewhat serialized to prevent the *entire* pending list to be
processed for every driver_bound() call.  Instead, by kicking a
workqueue, it guarantees that all the pending items are retried, but
kicking the workqueue 6 times doesn't mean that the list is going to
be processed 6 times.

In other words, driver_bound will kick off the process, but it doesn't
actually do the re-probing work.

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-22 21:19     ` Grant Likely
  0 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-09-22 21:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 22, 2011 at 2:29 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> Definitely what is needed for some of the x86 SoC stuff and would let us
> rip out some of the special case magic for the SCU discovery.
>
> First thing that strikes me is driver_bound kicks the processing queue
> again. That seems odd - surely this isn't needed because any driver that
> does initialise this time and may allow something else to get going will
> queue the kick itself. Thus this seems to just add overhead.
>
> It all looks a bit O(N?) if we don't expect the drivers that might
> trigger something else binding to just say 'hey I'm one of the
> troublemakers'

The way I read it, absolute worst case is when every device but one
depends on another device.  In that case I believe it will be
O(Nlog(N)).  (Every device gets probed on the first pass, but only the
last one gets probed.  Then it goes through N-1 devices to the result
of only 1 more device getting probed, then N-2, etc.).  Actual usage
won't be anywhere near that complexity because there tends to be a
small number of devices on these SoCs that a lot of other devices
depend on, so I expect that there will be somewhere on the order of
3-4 passes for the typical embedded SoCs that I've seen.

driver_bound is used to kick off the process because anytime a new
device becomes active (bound to a driver), there is a high likelyhood
that one of the devices in the pending list can now be probed.
However, the re-probing is performed in a workqueue to make sure that
it is somewhat serialized to prevent the *entire* pending list to be
processed for every driver_bound() call.  Instead, by kicking a
workqueue, it guarantees that all the pending items are retried, but
kicking the workqueue 6 times doesn't mean that the list is going to
be processed 6 times.

In other words, driver_bound will kick off the process, but it doesn't
actually do the re-probing work.

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-22 20:29   ` Alan Cox
  (?)
  (?)
@ 2011-09-22 21:19   ` David Daney
  2011-09-22 22:47       ` Alan Cox
  -1 siblings, 1 reply; 63+ messages in thread
From: David Daney @ 2011-09-22 21:19 UTC (permalink / raw)
  To: Alan Cox
  Cc: Grant Likely, linux-kernel, linux-arm-kernel, Greg Kroah-Hartman,
	Dilan Lee, Mark Brown, Manjunath GKondaiah, Arnd Bergmann

On 09/22/2011 01:29 PM, Alan Cox wrote:
> Definitely what is needed for some of the x86 SoC stuff and would let us
> rip out some of the special case magic for the SCU discovery.
>
> First thing that strikes me is driver_bound kicks the processing queue
> again. That seems odd - surely this isn't needed because any driver that
> does initialise this time and may allow something else to get going will
> queue the kick itself. Thus this seems to just add overhead.
>

Do you mean explicitly kick the queue?

How would a given driver know that something else is waiting for it?  Or 
would we add the explicit kick to each and every driver in the tree?


> It all looks a bit O(N²) if we don't expect the drivers that might
> trigger something else binding to just say 'hey I'm one of the
> troublemakers'

I think it probably is O(N²), but N is small...



> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>


^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-22 21:19   ` David Daney
@ 2011-09-22 22:47       ` Alan Cox
  0 siblings, 0 replies; 63+ messages in thread
From: Alan Cox @ 2011-09-22 22:47 UTC (permalink / raw)
  To: David Daney
  Cc: Grant Likely, linux-kernel, linux-arm-kernel, Greg Kroah-Hartman,
	Dilan Lee, Mark Brown, Manjunath GKondaiah, Arnd Bergmann

> How would a given driver know that something else is waiting for it?  Or 
> would we add the explicit kick to each and every driver in the tree?

I think there are very very few drivers that have this property and don't
already implicitly cause a probe by creating a new bus or device.

Those drivers that set something up for another device really should
know what is going on because they are making a guarantee that they are
ready for the other device to call into them or whatever is going on at
some point, either explicitly in the kick or implicitly in returning from
their probe method.

I know which I think is clearer and easier for a 3rd party to see and not
miss completely when updating code.

Alan

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-22 22:47       ` Alan Cox
  0 siblings, 0 replies; 63+ messages in thread
From: Alan Cox @ 2011-09-22 22:47 UTC (permalink / raw)
  To: linux-arm-kernel

> How would a given driver know that something else is waiting for it?  Or 
> would we add the explicit kick to each and every driver in the tree?

I think there are very very few drivers that have this property and don't
already implicitly cause a probe by creating a new bus or device.

Those drivers that set something up for another device really should
know what is going on because they are making a guarantee that they are
ready for the other device to call into them or whatever is going on at
some point, either explicitly in the kick or implicitly in returning from
their probe method.

I know which I think is clearer and easier for a 3rd party to see and not
miss completely when updating code.

Alan

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-22 22:47       ` Alan Cox
@ 2011-09-23  5:02         ` Grant Likely
  -1 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-09-23  5:02 UTC (permalink / raw)
  To: Alan Cox
  Cc: David Daney, Arnd Bergmann, Greg Kroah-Hartman, Mark Brown,
	linux-kernel, Manjunath GKondaiah, Dilan Lee, linux-arm-kernel

On Thu, Sep 22, 2011 at 11:47:37PM +0100, Alan Cox wrote:
> > How would a given driver know that something else is waiting for it?  Or 
> > would we add the explicit kick to each and every driver in the tree?
> 
> I think there are very very few drivers that have this property and don't
> already implicitly cause a probe by creating a new bus or device.
> 
> Those drivers that set something up for another device really should
> know what is going on because they are making a guarantee that they are
> ready for the other device to call into them or whatever is going on at
> some point, either explicitly in the kick or implicitly in returning from
> their probe method.

I disagree.  There are lots of situations we're hitting where the
providing driver and the consuming driver don't have any details about
each other except for the name/reference of the resource (be it gpio,
phy, clock, regulator; each with a different API).

Example 1: ALSA SoC complex.  A codec sitting on an i2c bus that gets
bound to an i2c_driver and attached to a memory-mapped I2S controller
driven by a platform_driver.  The sound device cannot be created until
both the i2c codec and the i2s controller are bound to their drivers.

Example 2: an SDHCI controller which uses a couple of GPIO lines for
card detect and write protect, but the board designer may choose to
use any available gpio line, such as on an spi gpio expander.

Example 3: An Ethernet controller attached to a phy on a completely
independent MDIO bus.  The Ethernet driver uses the phylib api to
access the phy, but cannot do so until after the phy device is bound
to a driver.

These are three very quickly off the top of my head.  I can easily
come up with a lot more.

The whole point is that the consumers *don't* and *should not* know anything
how its required resources are provided. The consumer only knows how
to request them.  Similarly, providers don't know anything about which
devices are going to be requesting resources from them.  They can only
publish what they provide.

Now, if I'm understanding what you're suggesting, we could cause the
re-probe to be triggered only when new gpio banks, phys, clocks,
regulators, audio codecs, audio busses, interrupt controllers, and dma
engines are registered.  This means adding the trigger call to all the
registration paths we think are relevant.  Personally, I really don't
think this is going to be worth it given the projected order of
complexity.  It is because the dependencies could be any interface
provided by another driver, I don't want to start with limitations on
exactly which interfaces can use probe deferral.

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-23  5:02         ` Grant Likely
  0 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-09-23  5:02 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 22, 2011 at 11:47:37PM +0100, Alan Cox wrote:
> > How would a given driver know that something else is waiting for it?  Or 
> > would we add the explicit kick to each and every driver in the tree?
> 
> I think there are very very few drivers that have this property and don't
> already implicitly cause a probe by creating a new bus or device.
> 
> Those drivers that set something up for another device really should
> know what is going on because they are making a guarantee that they are
> ready for the other device to call into them or whatever is going on at
> some point, either explicitly in the kick or implicitly in returning from
> their probe method.

I disagree.  There are lots of situations we're hitting where the
providing driver and the consuming driver don't have any details about
each other except for the name/reference of the resource (be it gpio,
phy, clock, regulator; each with a different API).

Example 1: ALSA SoC complex.  A codec sitting on an i2c bus that gets
bound to an i2c_driver and attached to a memory-mapped I2S controller
driven by a platform_driver.  The sound device cannot be created until
both the i2c codec and the i2s controller are bound to their drivers.

Example 2: an SDHCI controller which uses a couple of GPIO lines for
card detect and write protect, but the board designer may choose to
use any available gpio line, such as on an spi gpio expander.

Example 3: An Ethernet controller attached to a phy on a completely
independent MDIO bus.  The Ethernet driver uses the phylib api to
access the phy, but cannot do so until after the phy device is bound
to a driver.

These are three very quickly off the top of my head.  I can easily
come up with a lot more.

The whole point is that the consumers *don't* and *should not* know anything
how its required resources are provided. The consumer only knows how
to request them.  Similarly, providers don't know anything about which
devices are going to be requesting resources from them.  They can only
publish what they provide.

Now, if I'm understanding what you're suggesting, we could cause the
re-probe to be triggered only when new gpio banks, phys, clocks,
regulators, audio codecs, audio busses, interrupt controllers, and dma
engines are registered.  This means adding the trigger call to all the
registration paths we think are relevant.  Personally, I really don't
think this is going to be worth it given the projected order of
complexity.  It is because the dependencies could be any interface
provided by another driver, I don't want to start with limitations on
exactly which interfaces can use probe deferral.

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-22 22:47       ` Alan Cox
@ 2011-09-23 16:55         ` David Daney
  -1 siblings, 0 replies; 63+ messages in thread
From: David Daney @ 2011-09-23 16:55 UTC (permalink / raw)
  To: Alan Cox
  Cc: Grant Likely, linux-kernel, linux-arm-kernel, Greg Kroah-Hartman,
	Dilan Lee, Mark Brown, Manjunath GKondaiah, Arnd Bergmann

On 09/22/2011 03:47 PM, Alan Cox wrote:
>> How would a given driver know that something else is waiting for it?  Or
>> would we add the explicit kick to each and every driver in the tree?
>
> I think there are very very few drivers that have this property and don't
> already implicitly cause a probe by creating a new bus or device.
>

These are precisely the drivers of concern.  However it is not 
individual drivers, but whole classes of drivers.  In my case we are 
talking about GPIO drivers.

If there is a dependency on GPIO devices, we don't know which 
device/driver will be providing the GPIO services, so at a minimum *all* 
GPIO drivers would have to add the explicit kick.

> Those drivers that set something up for another device really should
> know what is going on because they are making a guarantee that they are
> ready for the other device to call into them or whatever is going on at
> some point, either explicitly in the kick or implicitly in returning from
> their probe method.

Really the driver framework is there to do all this already.  Once the 
probe method is called, the device is usually presented as 'ready' in 
some sense.

The problem this patch solves is to make it work when there are ad hoc 
relationships between the devices that cannot be represented in a tree 
like structure presented by the bus/driver topology of the driver framework.

If there are no deferred probes necessary, the only overhead is a single 
check to see if work needs to be done.  Since in this case, nothing 
needs to be done...  Nothing is done.


>
> I know which I think is clearer and easier for a 3rd party to see and not
> miss completely when updating code.

?? I don't understand that statement.

A handful of lines of code in the driver core vs. having to wonder, and 
then get it right, for each and every driver if there could be a 
dependency outside of the bus framework.


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-23 16:55         ` David Daney
  0 siblings, 0 replies; 63+ messages in thread
From: David Daney @ 2011-09-23 16:55 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/22/2011 03:47 PM, Alan Cox wrote:
>> How would a given driver know that something else is waiting for it?  Or
>> would we add the explicit kick to each and every driver in the tree?
>
> I think there are very very few drivers that have this property and don't
> already implicitly cause a probe by creating a new bus or device.
>

These are precisely the drivers of concern.  However it is not 
individual drivers, but whole classes of drivers.  In my case we are 
talking about GPIO drivers.

If there is a dependency on GPIO devices, we don't know which 
device/driver will be providing the GPIO services, so at a minimum *all* 
GPIO drivers would have to add the explicit kick.

> Those drivers that set something up for another device really should
> know what is going on because they are making a guarantee that they are
> ready for the other device to call into them or whatever is going on at
> some point, either explicitly in the kick or implicitly in returning from
> their probe method.

Really the driver framework is there to do all this already.  Once the 
probe method is called, the device is usually presented as 'ready' in 
some sense.

The problem this patch solves is to make it work when there are ad hoc 
relationships between the devices that cannot be represented in a tree 
like structure presented by the bus/driver topology of the driver framework.

If there are no deferred probes necessary, the only overhead is a single 
check to see if work needs to be done.  Since in this case, nothing 
needs to be done...  Nothing is done.


>
> I know which I think is clearer and easier for a 3rd party to see and not
> miss completely when updating code.

?? I don't understand that statement.

A handful of lines of code in the driver core vs. having to wonder, and 
then get it right, for each and every driver if there could be a 
dependency outside of the bus framework.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-22 21:19     ` Grant Likely
@ 2011-09-23 17:50       ` Valdis.Kletnieks at vt.edu
  -1 siblings, 0 replies; 63+ messages in thread
From: Valdis.Kletnieks @ 2011-09-23 17:50 UTC (permalink / raw)
  To: Grant Likely
  Cc: Alan Cox, Arnd Bergmann, Greg Kroah-Hartman, Mark Brown,
	Manjunath GKondaiah, linux-kernel, Dilan Lee, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 1309 bytes --]

On Thu, 22 Sep 2011 15:19:01 MDT, Grant Likely said:
> On Thu, Sep 22, 2011 at 2:29 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > Definitely what is needed for some of the x86 SoC stuff and would let us
> > rip out some of the special case magic for the SCU discovery.
> >
> > First thing that strikes me is driver_bound kicks the processing queue
> > again. That seems odd - surely this isn't needed because any driver that
> > does initialise this time and may allow something else to get going will
> > queue the kick itself. Thus this seems to just add overhead.
> >
> > It all looks a bit O(N²) if we don't expect the drivers that might
> > trigger something else binding to just say 'hey I'm one of the
> > troublemakers'
> 
> The way I read it, absolute worst case is when every device but one
> depends on another device.  In that case I believe it will be
> O(Nlog(N)).  (Every device gets probed on the first pass, but only the
> last one gets probed.  Then it goes through N-1 devices to the result
> of only 1 more device getting probed, then N-2, etc.). 

That is indeed O(N**2) not Nlog(N).  The total number of probes is (N+1)(N)/2
To get it to O(Nlog(N)), you'd have to probe N devices the first pass, N/2 devices
on the second pass, N/4 on the third, and so on.


[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-23 17:50       ` Valdis.Kletnieks at vt.edu
  0 siblings, 0 replies; 63+ messages in thread
From: Valdis.Kletnieks at vt.edu @ 2011-09-23 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 22 Sep 2011 15:19:01 MDT, Grant Likely said:
> On Thu, Sep 22, 2011 at 2:29 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > Definitely what is needed for some of the x86 SoC stuff and would let us
> > rip out some of the special case magic for the SCU discovery.
> >
> > First thing that strikes me is driver_bound kicks the processing queue
> > again. That seems odd - surely this isn't needed because any driver that
> > does initialise this time and may allow something else to get going will
> > queue the kick itself. Thus this seems to just add overhead.
> >
> > It all looks a bit O(N?) if we don't expect the drivers that might
> > trigger something else binding to just say 'hey I'm one of the
> > troublemakers'
> 
> The way I read it, absolute worst case is when every device but one
> depends on another device.  In that case I believe it will be
> O(Nlog(N)).  (Every device gets probed on the first pass, but only the
> last one gets probed.  Then it goes through N-1 devices to the result
> of only 1 more device getting probed, then N-2, etc.). 

That is indeed O(N**2) not Nlog(N).  The total number of probes is (N+1)(N)/2
To get it to O(Nlog(N)), you'd have to probe N devices the first pass, N/2 devices
on the second pass, N/4 on the third, and so on.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110923/f61d03c2/attachment.sig>

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-23 17:50       ` Valdis.Kletnieks at vt.edu
@ 2011-09-23 23:18         ` Grant Likely
  -1 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-09-23 23:18 UTC (permalink / raw)
  To: Valdis.Kletnieks
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Mark Brown, linux-kernel,
	Manjunath GKondaiah, linux-arm-kernel, Dilan Lee, Alan Cox

On Fri, Sep 23, 2011 at 01:50:23PM -0400, Valdis.Kletnieks@vt.edu wrote:
> On Thu, 22 Sep 2011 15:19:01 MDT, Grant Likely said:
> > On Thu, Sep 22, 2011 at 2:29 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > > Definitely what is needed for some of the x86 SoC stuff and would let us
> > > rip out some of the special case magic for the SCU discovery.
> > >
> > > First thing that strikes me is driver_bound kicks the processing queue
> > > again. That seems odd - surely this isn't needed because any driver that
> > > does initialise this time and may allow something else to get going will
> > > queue the kick itself. Thus this seems to just add overhead.
> > >
> > > It all looks a bit O(N²) if we don't expect the drivers that might
> > > trigger something else binding to just say 'hey I'm one of the
> > > troublemakers'
> > 
> > The way I read it, absolute worst case is when every device but one
> > depends on another device.  In that case I believe it will be
> > O(Nlog(N)).  (Every device gets probed on the first pass, but only the
> > last one gets probed.  Then it goes through N-1 devices to the result
> > of only 1 more device getting probed, then N-2, etc.). 
> 
> That is indeed O(N**2) not Nlog(N).  The total number of probes is (N+1)(N)/2
> To get it to O(Nlog(N)), you'd have to probe N devices the first pass, N/2 devices
> on the second pass, N/4 on the third, and so on.
> 

Ah, indeed, you are correct.  It's been too long since my engineering
CS class.

Still, I'm not even remotely worried about this algorithm for the
kernel.  The complexity is bounded by the number of levels of
dependencies, not the number of devices requesting deferral.  I'd be
very surprised if the nested dependencies ever get to half a dozen.

Note: these are only dependencies outside of the existing
parent-child dependencies in the Linux device model, which further
reduces the number of sources of nesting.

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-23 23:18         ` Grant Likely
  0 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-09-23 23:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Sep 23, 2011 at 01:50:23PM -0400, Valdis.Kletnieks at vt.edu wrote:
> On Thu, 22 Sep 2011 15:19:01 MDT, Grant Likely said:
> > On Thu, Sep 22, 2011 at 2:29 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > > Definitely what is needed for some of the x86 SoC stuff and would let us
> > > rip out some of the special case magic for the SCU discovery.
> > >
> > > First thing that strikes me is driver_bound kicks the processing queue
> > > again. That seems odd - surely this isn't needed because any driver that
> > > does initialise this time and may allow something else to get going will
> > > queue the kick itself. Thus this seems to just add overhead.
> > >
> > > It all looks a bit O(N?) if we don't expect the drivers that might
> > > trigger something else binding to just say 'hey I'm one of the
> > > troublemakers'
> > 
> > The way I read it, absolute worst case is when every device but one
> > depends on another device.  In that case I believe it will be
> > O(Nlog(N)).  (Every device gets probed on the first pass, but only the
> > last one gets probed.  Then it goes through N-1 devices to the result
> > of only 1 more device getting probed, then N-2, etc.). 
> 
> That is indeed O(N**2) not Nlog(N).  The total number of probes is (N+1)(N)/2
> To get it to O(Nlog(N)), you'd have to probe N devices the first pass, N/2 devices
> on the second pass, N/4 on the third, and so on.
> 

Ah, indeed, you are correct.  It's been too long since my engineering
CS class.

Still, I'm not even remotely worried about this algorithm for the
kernel.  The complexity is bounded by the number of levels of
dependencies, not the number of devices requesting deferral.  I'd be
very surprised if the nested dependencies ever get to half a dozen.

Note: these are only dependencies outside of the existing
parent-child dependencies in the Linux device model, which further
reduces the number of sources of nesting.

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-22 18:51 ` Grant Likely
@ 2011-09-26 14:16   ` Mark Brown
  -1 siblings, 0 replies; 63+ messages in thread
From: Mark Brown @ 2011-09-26 14:16 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, linux-arm-kernel, Greg Kroah-Hartman, Dilan Lee,
	Manjunath GKondaiah, Arnd Bergmann

On Thu, Sep 22, 2011 at 12:51:23PM -0600, Grant Likely wrote:
> Allow drivers to report at probe time that they cannot get all the resources
> required by the device, and should be retried at a later time.

> This should completely solve the problem of getting devices
> initialized in the right order.  Right now this is mostly handled by
> mucking about with initcall ordering which is a complete hack, and
> doesn't even remotely handle the case where device drivers are in
> modules.  This approach completely sidesteps the issues by allowing
> driver registration to occur in any order, and any driver can request
> to be retried after a few more other drivers get probed.

So, one issue I did think of the other day while putting some support in
the regulator core for using this: what happens with devices which can
optionally use a resource but don't rely on it?  One example here is
that a lot of the MMC drivers have an optional regulator to control some
of the supplies for the cards.  If the reglator isn't there it won't be
used but it's not a blocker for anything.  Devices doing this would need
some way to figure out if they should -EBUSY or fail otherwise.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-26 14:16   ` Mark Brown
  0 siblings, 0 replies; 63+ messages in thread
From: Mark Brown @ 2011-09-26 14:16 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 22, 2011 at 12:51:23PM -0600, Grant Likely wrote:
> Allow drivers to report at probe time that they cannot get all the resources
> required by the device, and should be retried at a later time.

> This should completely solve the problem of getting devices
> initialized in the right order.  Right now this is mostly handled by
> mucking about with initcall ordering which is a complete hack, and
> doesn't even remotely handle the case where device drivers are in
> modules.  This approach completely sidesteps the issues by allowing
> driver registration to occur in any order, and any driver can request
> to be retried after a few more other drivers get probed.

So, one issue I did think of the other day while putting some support in
the regulator core for using this: what happens with devices which can
optionally use a resource but don't rely on it?  One example here is
that a lot of the MMC drivers have an optional regulator to control some
of the supplies for the cards.  If the reglator isn't there it won't be
used but it's not a blocker for anything.  Devices doing this would need
some way to figure out if they should -EBUSY or fail otherwise.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-26 14:16   ` Mark Brown
@ 2011-09-26 15:12     ` Russell King - ARM Linux
  -1 siblings, 0 replies; 63+ messages in thread
From: Russell King - ARM Linux @ 2011-09-26 15:12 UTC (permalink / raw)
  To: Mark Brown
  Cc: Grant Likely, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
	Manjunath GKondaiah, Dilan Lee, linux-arm-kernel

On Mon, Sep 26, 2011 at 03:16:43PM +0100, Mark Brown wrote:
> On Thu, Sep 22, 2011 at 12:51:23PM -0600, Grant Likely wrote:
> > Allow drivers to report at probe time that they cannot get all the resources
> > required by the device, and should be retried at a later time.
> 
> > This should completely solve the problem of getting devices
> > initialized in the right order.  Right now this is mostly handled by
> > mucking about with initcall ordering which is a complete hack, and
> > doesn't even remotely handle the case where device drivers are in
> > modules.  This approach completely sidesteps the issues by allowing
> > driver registration to occur in any order, and any driver can request
> > to be retried after a few more other drivers get probed.
> 
> So, one issue I did think of the other day while putting some support in
> the regulator core for using this: what happens with devices which can
> optionally use a resource but don't rely on it?  One example here is
> that a lot of the MMC drivers have an optional regulator to control some
> of the supplies for the cards.  If the reglator isn't there it won't be
> used but it's not a blocker for anything.  Devices doing this would need
> some way to figure out if they should -EBUSY or fail otherwise.

Just to avoid confusion - ITYM -EAGAIN there.  -EBUSY is already used
by drivers to mean "someone else claimed a resource I need" be it the
IO region or an IRQ resource...

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-26 15:12     ` Russell King - ARM Linux
  0 siblings, 0 replies; 63+ messages in thread
From: Russell King - ARM Linux @ 2011-09-26 15:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 26, 2011 at 03:16:43PM +0100, Mark Brown wrote:
> On Thu, Sep 22, 2011 at 12:51:23PM -0600, Grant Likely wrote:
> > Allow drivers to report at probe time that they cannot get all the resources
> > required by the device, and should be retried at a later time.
> 
> > This should completely solve the problem of getting devices
> > initialized in the right order.  Right now this is mostly handled by
> > mucking about with initcall ordering which is a complete hack, and
> > doesn't even remotely handle the case where device drivers are in
> > modules.  This approach completely sidesteps the issues by allowing
> > driver registration to occur in any order, and any driver can request
> > to be retried after a few more other drivers get probed.
> 
> So, one issue I did think of the other day while putting some support in
> the regulator core for using this: what happens with devices which can
> optionally use a resource but don't rely on it?  One example here is
> that a lot of the MMC drivers have an optional regulator to control some
> of the supplies for the cards.  If the reglator isn't there it won't be
> used but it's not a blocker for anything.  Devices doing this would need
> some way to figure out if they should -EBUSY or fail otherwise.

Just to avoid confusion - ITYM -EAGAIN there.  -EBUSY is already used
by drivers to mean "someone else claimed a resource I need" be it the
IO region or an IRQ resource...

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-26 15:12     ` Russell King - ARM Linux
@ 2011-09-26 15:26       ` Mark Brown
  -1 siblings, 0 replies; 63+ messages in thread
From: Mark Brown @ 2011-09-26 15:26 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Grant Likely, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
	Manjunath GKondaiah, Dilan Lee, linux-arm-kernel

On Mon, Sep 26, 2011 at 04:12:10PM +0100, Russell King - ARM Linux wrote:
> On Mon, Sep 26, 2011 at 03:16:43PM +0100, Mark Brown wrote:

> > used but it's not a blocker for anything.  Devices doing this would need
> > some way to figure out if they should -EBUSY or fail otherwise.

> Just to avoid confusion - ITYM -EAGAIN there.  -EBUSY is already used
> by drivers to mean "someone else claimed a resource I need" be it the
> IO region or an IRQ resource...

Yes, I do - sorry.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-26 15:26       ` Mark Brown
  0 siblings, 0 replies; 63+ messages in thread
From: Mark Brown @ 2011-09-26 15:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 26, 2011 at 04:12:10PM +0100, Russell King - ARM Linux wrote:
> On Mon, Sep 26, 2011 at 03:16:43PM +0100, Mark Brown wrote:

> > used but it's not a blocker for anything.  Devices doing this would need
> > some way to figure out if they should -EBUSY or fail otherwise.

> Just to avoid confusion - ITYM -EAGAIN there.  -EBUSY is already used
> by drivers to mean "someone else claimed a resource I need" be it the
> IO region or an IRQ resource...

Yes, I do - sorry.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-26 15:26       ` Mark Brown
@ 2011-09-26 15:48         ` Grant Likely
  -1 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-09-26 15:48 UTC (permalink / raw)
  To: Mark Brown
  Cc: Russell King - ARM Linux, Arnd Bergmann, Greg Kroah-Hartman,
	linux-kernel, Manjunath GKondaiah, Dilan Lee, linux-arm-kernel

On Mon, Sep 26, 2011 at 9:26 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Sep 26, 2011 at 04:12:10PM +0100, Russell King - ARM Linux wrote:
>> On Mon, Sep 26, 2011 at 03:16:43PM +0100, Mark Brown wrote:
>
>> > used but it's not a blocker for anything.  Devices doing this would need
>> > some way to figure out if they should -EBUSY or fail otherwise.
>
>> Just to avoid confusion - ITYM -EAGAIN there.  -EBUSY is already used
>> by drivers to mean "someone else claimed a resource I need" be it the
>> IO region or an IRQ resource...
>
> Yes, I do - sorry.

Actually, in the next iteration, I'm thinking it would be a good idea
to create a new #define to only be used by probe deferral.  I want it
to be easy to find all the drivers that are using this mechanism
without needing to filter all the unrelated hits.  However, this is a
kernel-only thing so it is probably not appropriate to add it to
include/asm-generic/errno.h.  I could use some guidance/advice as to
the best way to handle this.

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-26 15:48         ` Grant Likely
  0 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-09-26 15:48 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 26, 2011 at 9:26 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Sep 26, 2011 at 04:12:10PM +0100, Russell King - ARM Linux wrote:
>> On Mon, Sep 26, 2011 at 03:16:43PM +0100, Mark Brown wrote:
>
>> > used but it's not a blocker for anything. ?Devices doing this would need
>> > some way to figure out if they should -EBUSY or fail otherwise.
>
>> Just to avoid confusion - ITYM -EAGAIN there. ?-EBUSY is already used
>> by drivers to mean "someone else claimed a resource I need" be it the
>> IO region or an IRQ resource...
>
> Yes, I do - sorry.

Actually, in the next iteration, I'm thinking it would be a good idea
to create a new #define to only be used by probe deferral.  I want it
to be easy to find all the drivers that are using this mechanism
without needing to filter all the unrelated hits.  However, this is a
kernel-only thing so it is probably not appropriate to add it to
include/asm-generic/errno.h.  I could use some guidance/advice as to
the best way to handle this.

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-26 15:48         ` Grant Likely
@ 2011-09-27 13:50           ` Arnd Bergmann
  -1 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2011-09-27 13:50 UTC (permalink / raw)
  To: Grant Likely
  Cc: Mark Brown, Russell King - ARM Linux, Greg Kroah-Hartman,
	linux-kernel, Manjunath GKondaiah, Dilan Lee, linux-arm-kernel

On Monday 26 September 2011, Grant Likely wrote:
> Actually, in the next iteration, I'm thinking it would be a good idea
> to create a new #define to only be used by probe deferral.  I want it
> to be easy to find all the drivers that are using this mechanism
> without needing to filter all the unrelated hits.  However, this is a
> kernel-only thing so it is probably not appropriate to add it to
> include/asm-generic/errno.h.  I could use some guidance/advice as to
> the best way to handle this.

include/linux/errno.h already has a bunch of kernel-internal error
codes that are never supposed to be seen in user space. Just
add one there, the next free one right now is 517, after
ERESTART_RESTARTBLOCK.

	Arnd

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-27 13:50           ` Arnd Bergmann
  0 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2011-09-27 13:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Monday 26 September 2011, Grant Likely wrote:
> Actually, in the next iteration, I'm thinking it would be a good idea
> to create a new #define to only be used by probe deferral.  I want it
> to be easy to find all the drivers that are using this mechanism
> without needing to filter all the unrelated hits.  However, this is a
> kernel-only thing so it is probably not appropriate to add it to
> include/asm-generic/errno.h.  I could use some guidance/advice as to
> the best way to handle this.

include/linux/errno.h already has a bunch of kernel-internal error
codes that are never supposed to be seen in user space. Just
add one there, the next free one right now is 517, after
ERESTART_RESTARTBLOCK.

	Arnd

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-27 13:50           ` Arnd Bergmann
@ 2011-09-27 21:08             ` Grant Likely
  -1 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-09-27 21:08 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Russell King - ARM Linux, Greg Kroah-Hartman, Mark Brown,
	linux-kernel, Manjunath GKondaiah, Dilan Lee, linux-arm-kernel

On Tue, Sep 27, 2011 at 03:50:36PM +0200, Arnd Bergmann wrote:
> On Monday 26 September 2011, Grant Likely wrote:
> > Actually, in the next iteration, I'm thinking it would be a good idea
> > to create a new #define to only be used by probe deferral.  I want it
> > to be easy to find all the drivers that are using this mechanism
> > without needing to filter all the unrelated hits.  However, this is a
> > kernel-only thing so it is probably not appropriate to add it to
> > include/asm-generic/errno.h.  I could use some guidance/advice as to
> > the best way to handle this.
> 
> include/linux/errno.h already has a bunch of kernel-internal error
> codes that are never supposed to be seen in user space. Just
> add one there, the next free one right now is 517, after
> ERESTART_RESTARTBLOCK.

Okay, will do.  How does EPROBE_DEFER 518 sound?

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-27 21:08             ` Grant Likely
  0 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-09-27 21:08 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 27, 2011 at 03:50:36PM +0200, Arnd Bergmann wrote:
> On Monday 26 September 2011, Grant Likely wrote:
> > Actually, in the next iteration, I'm thinking it would be a good idea
> > to create a new #define to only be used by probe deferral.  I want it
> > to be easy to find all the drivers that are using this mechanism
> > without needing to filter all the unrelated hits.  However, this is a
> > kernel-only thing so it is probably not appropriate to add it to
> > include/asm-generic/errno.h.  I could use some guidance/advice as to
> > the best way to handle this.
> 
> include/linux/errno.h already has a bunch of kernel-internal error
> codes that are never supposed to be seen in user space. Just
> add one there, the next free one right now is 517, after
> ERESTART_RESTARTBLOCK.

Okay, will do.  How does EPROBE_DEFER 518 sound?

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-27 21:08             ` Grant Likely
@ 2011-09-27 22:13               ` Mark Brown
  -1 siblings, 0 replies; 63+ messages in thread
From: Mark Brown @ 2011-09-27 22:13 UTC (permalink / raw)
  To: Grant Likely
  Cc: Arnd Bergmann, Russell King - ARM Linux, Greg Kroah-Hartman,
	linux-kernel, Manjunath GKondaiah, Dilan Lee, linux-arm-kernel

On Tue, Sep 27, 2011 at 03:08:49PM -0600, Grant Likely wrote:

> Okay, will do.  How does EPROBE_DEFER 518 sound?

Note that I'm not sure this answers the issue I was raising - the issue
isn't that the caller doesn't know what the error code means, the issue
is that in some cases the driver needs to take a decision about what
failure to get a resource means.  Does it mean that the driver can work
fine and be slightly less featureful or should it cause a deferral?

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-27 22:13               ` Mark Brown
  0 siblings, 0 replies; 63+ messages in thread
From: Mark Brown @ 2011-09-27 22:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 27, 2011 at 03:08:49PM -0600, Grant Likely wrote:

> Okay, will do.  How does EPROBE_DEFER 518 sound?

Note that I'm not sure this answers the issue I was raising - the issue
isn't that the caller doesn't know what the error code means, the issue
is that in some cases the driver needs to take a decision about what
failure to get a resource means.  Does it mean that the driver can work
fine and be slightly less featureful or should it cause a deferral?

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-27 22:13               ` Mark Brown
@ 2011-09-28 13:04                 ` Arnd Bergmann
  -1 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2011-09-28 13:04 UTC (permalink / raw)
  To: Mark Brown
  Cc: Grant Likely, Russell King - ARM Linux, Greg Kroah-Hartman,
	linux-kernel, Manjunath GKondaiah, Dilan Lee, linux-arm-kernel

On Wednesday 28 September 2011, Mark Brown wrote:
> On Tue, Sep 27, 2011 at 03:08:49PM -0600, Grant Likely wrote:
> 
> > Okay, will do.  How does EPROBE_DEFER 518 sound?
> 
> Note that I'm not sure this answers the issue I was raising - the issue
> isn't that the caller doesn't know what the error code means, the issue
> is that in some cases the driver needs to take a decision about what
> failure to get a resource means.  Does it mean that the driver can work
> fine and be slightly less featureful or should it cause a deferral?

Can you think of cases where this information cannot be put into the
device tree or platform_data? If a board provides an optional feature,
I think that should be a property of the device that the driver gets,
so it can return an error when that feature is not around, or continue
when it knows that the feature will never become available.

	Arnd

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-28 13:04                 ` Arnd Bergmann
  0 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2011-09-28 13:04 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 28 September 2011, Mark Brown wrote:
> On Tue, Sep 27, 2011 at 03:08:49PM -0600, Grant Likely wrote:
> 
> > Okay, will do.  How does EPROBE_DEFER 518 sound?
> 
> Note that I'm not sure this answers the issue I was raising - the issue
> isn't that the caller doesn't know what the error code means, the issue
> is that in some cases the driver needs to take a decision about what
> failure to get a resource means.  Does it mean that the driver can work
> fine and be slightly less featureful or should it cause a deferral?

Can you think of cases where this information cannot be put into the
device tree or platform_data? If a board provides an optional feature,
I think that should be a property of the device that the driver gets,
so it can return an error when that feature is not around, or continue
when it knows that the feature will never become available.

	Arnd

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-28 13:04                 ` Arnd Bergmann
@ 2011-09-28 13:20                   ` Mark Brown
  -1 siblings, 0 replies; 63+ messages in thread
From: Mark Brown @ 2011-09-28 13:20 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Grant Likely, Russell King - ARM Linux, Greg Kroah-Hartman,
	linux-kernel, Manjunath GKondaiah, Dilan Lee, linux-arm-kernel

On Wed, Sep 28, 2011 at 03:04:34PM +0200, Arnd Bergmann wrote:
> On Wednesday 28 September 2011, Mark Brown wrote:

> > Note that I'm not sure this answers the issue I was raising - the issue
> > isn't that the caller doesn't know what the error code means, the issue
> > is that in some cases the driver needs to take a decision about what
> > failure to get a resource means.  Does it mean that the driver can work
> > fine and be slightly less featureful or should it cause a deferral?

> Can you think of cases where this information cannot be put into the
> device tree or platform_data? If a board provides an optional feature,
> I think that should be a property of the device that the driver gets,
> so it can return an error when that feature is not around, or continue
> when it knows that the feature will never become available.

Not off the top of my head, most of the cases I'm aware of were cases
where the supply is mandatory but soft control is optional so don't need
to make this decision in the driver at all.  In the MMC case I didn't
push this as working with the people concerned was extremely painful.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-28 13:20                   ` Mark Brown
  0 siblings, 0 replies; 63+ messages in thread
From: Mark Brown @ 2011-09-28 13:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 28, 2011 at 03:04:34PM +0200, Arnd Bergmann wrote:
> On Wednesday 28 September 2011, Mark Brown wrote:

> > Note that I'm not sure this answers the issue I was raising - the issue
> > isn't that the caller doesn't know what the error code means, the issue
> > is that in some cases the driver needs to take a decision about what
> > failure to get a resource means.  Does it mean that the driver can work
> > fine and be slightly less featureful or should it cause a deferral?

> Can you think of cases where this information cannot be put into the
> device tree or platform_data? If a board provides an optional feature,
> I think that should be a property of the device that the driver gets,
> so it can return an error when that feature is not around, or continue
> when it knows that the feature will never become available.

Not off the top of my head, most of the cases I'm aware of were cases
where the supply is mandatory but soft control is optional so don't need
to make this decision in the driver at all.  In the MMC case I didn't
push this as working with the people concerned was extremely painful.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-27 22:13               ` Mark Brown
@ 2011-09-28 23:14                 ` Grant Likely
  -1 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-09-28 23:14 UTC (permalink / raw)
  To: Mark Brown
  Cc: Arnd Bergmann, Russell King - ARM Linux, Greg Kroah-Hartman,
	linux-kernel, Manjunath GKondaiah, Dilan Lee, linux-arm-kernel

On Tue, Sep 27, 2011 at 11:13:10PM +0100, Mark Brown wrote:
> On Tue, Sep 27, 2011 at 03:08:49PM -0600, Grant Likely wrote:
> 
> > Okay, will do.  How does EPROBE_DEFER 518 sound?
> 
> Note that I'm not sure this answers the issue I was raising - the issue
> isn't that the caller doesn't know what the error code means, the issue
> is that in some cases the driver needs to take a decision about what
> failure to get a resource means.  Does it mean that the driver can work
> fine and be slightly less featureful or should it cause a deferral?

Right. That was answering a different question.

For your question, I still think it is the driver that gets to make
the decision.  If it can proceed without a resource, then it should go
ahead and succeed on the probe, and then arrange to either be notified
of new gpio controller (or whatever) registrations, or poll for the
resource to be set up.

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-28 23:14                 ` Grant Likely
  0 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-09-28 23:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 27, 2011 at 11:13:10PM +0100, Mark Brown wrote:
> On Tue, Sep 27, 2011 at 03:08:49PM -0600, Grant Likely wrote:
> 
> > Okay, will do.  How does EPROBE_DEFER 518 sound?
> 
> Note that I'm not sure this answers the issue I was raising - the issue
> isn't that the caller doesn't know what the error code means, the issue
> is that in some cases the driver needs to take a decision about what
> failure to get a resource means.  Does it mean that the driver can work
> fine and be slightly less featureful or should it cause a deferral?

Right. That was answering a different question.

For your question, I still think it is the driver that gets to make
the decision.  If it can proceed without a resource, then it should go
ahead and succeed on the probe, and then arrange to either be notified
of new gpio controller (or whatever) registrations, or poll for the
resource to be set up.

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-28 23:14                 ` Grant Likely
@ 2011-09-29 11:00                   ` Mark Brown
  -1 siblings, 0 replies; 63+ messages in thread
From: Mark Brown @ 2011-09-29 11:00 UTC (permalink / raw)
  To: Grant Likely
  Cc: Arnd Bergmann, Russell King - ARM Linux, Greg Kroah-Hartman,
	linux-kernel, Manjunath GKondaiah, Dilan Lee, linux-arm-kernel

On Wed, Sep 28, 2011 at 06:14:10PM -0500, Grant Likely wrote:

> For your question, I still think it is the driver that gets to make
> the decision.  If it can proceed without a resource, then it should go
> ahead and succeed on the probe, and then arrange to either be notified
> of new gpio controller (or whatever) registrations, or poll for the
> resource to be set up.

Right, I do tend to agree.  This is something we'll have to bear in mind
when deploying this stuff - drivers that are doing this sort of stuff
are going to get surprised.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-09-29 11:00                   ` Mark Brown
  0 siblings, 0 replies; 63+ messages in thread
From: Mark Brown @ 2011-09-29 11:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 28, 2011 at 06:14:10PM -0500, Grant Likely wrote:

> For your question, I still think it is the driver that gets to make
> the decision.  If it can proceed without a resource, then it should go
> ahead and succeed on the probe, and then arrange to either be notified
> of new gpio controller (or whatever) registrations, or poll for the
> resource to be set up.

Right, I do tend to agree.  This is something we'll have to bear in mind
when deploying this stuff - drivers that are doing this sort of stuff
are going to get surprised.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-22 18:51 ` Grant Likely
@ 2011-10-03 23:02   ` Kevin Hilman
  -1 siblings, 0 replies; 63+ messages in thread
From: Kevin Hilman @ 2011-10-03 23:02 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, linux-arm-kernel, Greg Kroah-Hartman, Dilan Lee,
	Mark Brown, Manjunath GKondaiah, Arnd Bergmann

Grant Likely <grant.likely@secretlab.ca> writes:

> Allow drivers to report at probe time that they cannot get all the resources
> required by the device, and should be retried at a later time.
>
> This should completely solve the problem of getting devices
> initialized in the right order.  Right now this is mostly handled by
> mucking about with initcall ordering which is a complete hack, and
> doesn't even remotely handle the case where device drivers are in
> modules.  This approach completely sidesteps the issues by allowing
> driver registration to occur in any order, and any driver can request
> to be retried after a few more other drivers get probed.

This is great work, thanks!

For the TODO list:

While the proposed patch should solve probe order dependencies, I don't
think it will solve the suspend/resume ordering dependencies, which are
typically the same.

Currenly suspend/resume order is based on the order devices are *added*
(device_add() -> device_pm_add() -> device added to dpm_list), so
unfortunately, deferring probe isn't going to affect suspend/resume
ordering.

Extending this to also address suspend/resume ordering by also changing
when the device is added to the dpm_list (or possibly creating another
list) should probably be explored as well.

Kevin

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-10-03 23:02   ` Kevin Hilman
  0 siblings, 0 replies; 63+ messages in thread
From: Kevin Hilman @ 2011-10-03 23:02 UTC (permalink / raw)
  To: linux-arm-kernel

Grant Likely <grant.likely@secretlab.ca> writes:

> Allow drivers to report at probe time that they cannot get all the resources
> required by the device, and should be retried at a later time.
>
> This should completely solve the problem of getting devices
> initialized in the right order.  Right now this is mostly handled by
> mucking about with initcall ordering which is a complete hack, and
> doesn't even remotely handle the case where device drivers are in
> modules.  This approach completely sidesteps the issues by allowing
> driver registration to occur in any order, and any driver can request
> to be retried after a few more other drivers get probed.

This is great work, thanks!

For the TODO list:

While the proposed patch should solve probe order dependencies, I don't
think it will solve the suspend/resume ordering dependencies, which are
typically the same.

Currenly suspend/resume order is based on the order devices are *added*
(device_add() -> device_pm_add() -> device added to dpm_list), so
unfortunately, deferring probe isn't going to affect suspend/resume
ordering.

Extending this to also address suspend/resume ordering by also changing
when the device is added to the dpm_list (or possibly creating another
list) should probably be explored as well.

Kevin

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-22 18:51 ` Grant Likely
  (?)
@ 2011-10-04 14:51   ` G, Manjunath Kondaiah
  -1 siblings, 0 replies; 63+ messages in thread
From: G, Manjunath Kondaiah @ 2011-10-04 14:51 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, linux-arm-kernel, Greg Kroah-Hartman, Dilan Lee,
	Mark Brown, Manjunath GKondaiah, Arnd Bergmann, linux-omap

Hi Grant,

On Thu, Sep 22, 2011 at 12:51:23PM -0600, Grant Likely wrote:
> Allow drivers to report at probe time that they cannot get all the resources
> required by the device, and should be retried at a later time.
> 
> This should completely solve the problem of getting devices
> initialized in the right order.  Right now this is mostly handled by
> mucking about with initcall ordering which is a complete hack, and
> doesn't even remotely handle the case where device drivers are in
> modules.  This approach completely sidesteps the issues by allowing
> driver registration to occur in any order, and any driver can request
> to be retried after a few more other drivers get probed.
> 
> v3: - Hold off workqueue scheduling until late_initcall so that the bulk
>       of driver probes are complete before we start retrying deferred devices.
>     - Tested with simple use cases.  Still needs more testing though.
>       Using it to get rid of the gpio early_initcall madness, or to replace
>       the ASoC internal probe deferral code would be ideal.
> v2: - added locking so it should no longer be utterly broken in that regard
>     - remove device from deferred list at device_del time.
>     - Still completely untested with any real use case, but has been
>       boot tested.
> 
> TODO: - Create a separate singlethread_workqueue so that drivers can't
>         mess things up by calling flush_work().
>       - Maybe this should be wrapped with a kconfig symbol so it can
>         be compiled out on systems that don't care.
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Cc: Greg Kroah-Hartman <greg@kroah.com>
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Dilan Lee <dilee@nvidia.com>
> Cc: Manjunath GKondaiah <manjunath.gkondaiah@linaro.org>
> ---
> 
> Hi Manjunath,
> 
> Here's the current state of the patch.  The major think that needs to
> be done is to convert it to use a separate workqueue as described in
> the TODO above.  It also needs some users adapted to it.  One of the
> gpio drivers would work; preferably one of the newer drivers that
> doesn't have a lot of drivers depending on the early_initcall()
> behaviour yet.

I have tested this patch on omap3 beagle board by making:
1. omap-gpio driver init as late_initcall instead of postcore_initcall
2. mmc driver probe will request gpio through gpio_request and gpio driver
returns -EDEFER_PROBE which in turn makes mmc driver to request deferral probe.
3. When deferral probe gets activated, it scans driver entries and it will not 
find any match for mmc driver probe.

After step 3, mmc driver probe will not get called due to device and driver
mismatch during "bus_for_each_drv" execution.

The behaviour is no different when it is used with singlethread_workqueue.

Here is bootlog:
Texas Instruments X-Loader 1.4.2 (Feb 19 2009 - 12:01:24)
Reading boot sector
Loading u-boot.bin from mmc


U-Boot 2009.11 (Feb 23 2010 - 15:33:48)

OMAP3530-GP ES3.0, CPU-OPP2 L3-165MHz
OMAP3 Beagle board + LPDDR/NAND
I2C:   ready
DRAM:  128 MB
NAND:  256 MiB
In:    serial
Out:   serial
Err:   serial
Board revision Ax/Bx
Die ID #5ac400030000000004013f8901001001
Hit any key to stop autoboot:  0 
mmc0 is available
mmc - MMC sub-system

Usage:
mmc init [dev] - init MMC sub system
mmc device [dev] - show or set current device
reading uImage

3237432 bytes read
## Booting kernel from Legacy Image at 80000000 ...
      Image Name:   Linux-3.1.0-rc8-00001-g4dc9843-d
      Image Type:   ARM Linux Kernel Image (uncompressed)
      Data Size:    3237368 Bytes =  3.1 MB
      Load Address: 80008000
      Entry Point:  80008000
      Verifying Checksum ... OK
      Kernel Image ... OK
OK

Starting kernel ...

Uncompressing Linux... done, booting the kernel.
[    0.000000] Linux version 3.1.0-rc8-00001-g4dc9843-dirty (manju@manju-desktop) (gcc version 4.5.2 (Ubuntu/Linaro4.5.2-8ubuntu3) ) #7 SMP Tue Oct 4 19:19:49 IST 2011
[    0.000000] CPU: ARMv7 Processor [411fc083] revision 3 (ARMv7), cr=10c53c7f
[    0.000000] CPU: VIPT nonaliasing data cache, VIPT nonaliasing instruction cache
[    0.000000] Machine: OMAP3 Beagle Board
[    0.000000] Memory policy: ECC disabled, Data cache writeback
[    0.000000] OMAP3430/3530 ES3.0 (l2cache iva sgx neon isp)

...

[    3.554656] driver_probe_device: drv->name: omap_hsmmc
[    3.560241] omap_hsmmc_probe: ...........Enter *********************
[    3.566955] omap_hsmmc_gpio_init: ..... enter .........!!!!
[    3.572906] platform omap_hsmmc.0: Driver omap_hsmmc requests probe deferral

...

[    3.712097] driver_probe_device: drv->name: omap_gpio
[    3.717681] omap_device: omap_gpio.0: new worst case activate latency 0: 30517
[    3.727813] OMAP GPIO hardware version 2.5
[    3.732421] driver_probe_device: drv->name: omap_gpio
[    3.739776] driver_probe_device: drv->name: omap_gpio
[    3.747039] driver_probe_device: drv->name: omap_gpio
[    3.754241] driver_probe_device: drv->name: omap_gpio
[    3.761413] driver_probe_device: drv->name: omap_gpio
...
[    3.800170] platform omap_hsmmc.0: Retrying from deferred list
[    3.806304] bus_probe_device: device autoprobe 
[    3.811096] device_attach: ...........test-1
[    3.815643] __device_attach: ...........test-4
[    3.820312] __device_attach: ...........test-4
[    3.825012] __device_attach: ...........test-4
[    3.829681] __device_attach: ...........test-4
[    3.834411] __device_attach: ...........test-4
[    3.839111] __device_attach: ...........test-4
[    3.843749] __device_attach: ...........test-4
[    3.848480] __device_attach: ...........test-4
[    3.853118] __device_attach: ...........test-4
[    3.857818] __device_attach: ...........test-4
[    3.862548] __device_attach: ...........test-4
[    3.867218] __device_attach: ...........test-4
[    3.871917] __device_attach: ...........test-4
[    3.876556] __device_attach: ...........test-4
[    3.881286] __device_attach: ...........test-4
[    3.885986] __device_attach: ...........test-4
[    3.890655] __device_attach: ...........test-4
[    3.895355] __device_attach: ...........test-4
[    3.900024] __device_attach: ...........test-4
[    3.904724] __device_attach: ...........test-4
[    3.909423] __device_attach: ...........test-4
[    3.914093] __device_attach: ...........test-4
[    3.918792] __device_attach: ...........test-4
[    3.923461] __device_attach: ...........test-4
[    3.928192] __device_attach: ...........test-4
[    3.932891] __device_attach: ...........test-4
[    3.937530] __device_attach: ...........test-4

[    3.944458] input: gpio-keys as /devices/platform/gpio-keys/input/input1
[    3.955657] twl_rtc twl_rtc: setting system clock to 2000-01-01 00:01:59 UTC (946684919)
[    3.969390] Waiting for root device /dev/mmcblk0p2...

Here it hangs since MMC driver is not ready!

-Manjunath


> 
> Mark Brown may also be able to suggest specific examples.
> 
> For everyone else; this is the current state of the patch.  I think it
> is in pretty good shape other than the TODO item above.  I'm turning
> it over to Manjunath to  polish up for merging.  I would appreciate
> any feedback.
> 
> g.
> 
> 
>  drivers/base/base.h    |    1 
>  drivers/base/core.c    |    2 +
>  drivers/base/dd.c      |  134 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/device.h |    5 ++
>  4 files changed, 141 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index a34dca0..9641309 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -105,6 +105,7 @@ extern void bus_remove_driver(struct device_driver *drv);
>  
>  extern void driver_detach(struct device_driver *drv);
>  extern int driver_probe_device(struct device_driver *drv, struct device *dev);
> +extern void driver_deferred_probe_del(struct device *dev);
>  static inline int driver_match_device(struct device_driver *drv,
>  				      struct device *dev)
>  {
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index bc8729d..0d37e18 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -588,6 +588,7 @@ void device_initialize(struct device *dev)
>  {
>  	dev->kobj.kset = devices_kset;
>  	kobject_init(&dev->kobj, &device_ktype);
> +	INIT_LIST_HEAD(&dev->deferred_probe);
>  	INIT_LIST_HEAD(&dev->dma_pools);
>  	mutex_init(&dev->mutex);
>  	lockdep_set_novalidate_class(&dev->mutex);
> @@ -1119,6 +1120,7 @@ void device_del(struct device *dev)
>  	device_remove_file(dev, &uevent_attr);
>  	device_remove_attrs(dev);
>  	bus_remove_device(dev);
> +	driver_deferred_probe_del(dev);
>  
>  	/*
>  	 * Some platform devices are driven without driver attached
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 6658da7..8be5b33 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -28,6 +28,129 @@
>  #include "base.h"
>  #include "power/power.h"
>  
> +/*
> + * Deferred Probe infrastructure.
> + *
> + * Sometimes driver probe order matters, but the kernel doesn't always have
> + * dependency information which means some drivers will get probed before a
> + * resource it depends on is available.  For example, an SDHCI driver may
> + * first need a GPIO line from an i2c GPIO controller before it can be
> + * initialized.  If a required resource is not available yet, a driver can
> + * request probing to be deferred by returning -EAGAIN from its probe hook
> + *
> + * Deferred probe maintains two lists of devices, a pending list and an active
> + * list.  A driver returning -EAGAIN causes the device to be added to the
> + * pending list.
> + *
> + * The deferred_probe_mutex *must* be held any time the deferred_probe_*_list
> + * of the (struct device*)->deferred_probe pointers are manipulated
> + */
> +static DEFINE_MUTEX(deferred_probe_mutex);
> +static LIST_HEAD(deferred_probe_pending_list);
> +static LIST_HEAD(deferred_probe_active_list);
> +
> +/**
> + * deferred_probe_work_func() - Retry probing devices in the active list.
> + */
> +static void deferred_probe_work_func(struct work_struct *work)
> +{
> +	struct device *dev;
> +	/*
> +	 * This bit is tricky.  We want to process every device in the
> +	 * deferred list, but devices can be removed from the list at any
> +	 * time while inside this for-each loop.  There are two things that
> +	 * need to be protected against:
> +	 * - if the device is removed from the deferred_probe_list, then we
> +	 *   loose our place in the loop.  Since any device can be removed
> +	 *   asynchronously, list_for_each_entry_safe() wouldn't make things
> +	 *   much better.  Simplest solution is to restart walking the list
> +	 *   whenever the current device gets removed.  Not the most efficient,
> +	 *   but is simple to implement and easy to audit for correctness.
> +	 * - if the device is unregistered, and freed, then there is a risk
> +	 *   of a null pointer dereference.  This code uses get/put_device()
> +	 *   to ensure the device cannot disappear from under our feet.
> +	 */
> +	mutex_lock(&deferred_probe_mutex);
> +	while (!list_empty(&deferred_probe_active_list)) {
> +		dev = list_first_entry(&deferred_probe_active_list,
> +					typeof(*dev), deferred_probe);
> +		list_del_init(&dev->deferred_probe);
> +
> +		get_device(dev);
> +
> +		/* Drop the mutex while probing each device; the probe path
> +		 * may manipulate the deferred list */
> +		mutex_unlock(&deferred_probe_mutex);
> +		dev_dbg(dev, "Retrying from deferred list\n");
> +		bus_probe_device(dev);
> +		mutex_lock(&deferred_probe_mutex);
> +
> +		put_device(dev);
> +	}
> +	mutex_unlock(&deferred_probe_mutex);
> +}
> +static DECLARE_WORK(deferred_probe_work, deferred_probe_work_func);
> +
> +static void driver_deferred_probe_add(struct device *dev)
> +{
> +	mutex_lock(&deferred_probe_mutex);
> +	if (list_empty(&dev->deferred_probe)) {
> +		dev_dbg(dev, "Added to deferred list\n");
> +		list_add(&dev->deferred_probe, &deferred_probe_pending_list);
> +	}
> +	mutex_unlock(&deferred_probe_mutex);
> +}
> +
> +void driver_deferred_probe_del(struct device *dev)
> +{
> +	mutex_lock(&deferred_probe_mutex);
> +	if (!list_empty(&dev->deferred_probe)) {
> +		dev_dbg(dev, "Removed from deferred list\n");
> +		list_del_init(&dev->deferred_probe);
> +	}
> +	mutex_unlock(&deferred_probe_mutex);
> +}
> +
> +static bool driver_deferred_probe_enable = false;
> +/**
> + * driver_deferred_probe_trigger() - Kick off re-probing deferred devices
> + *
> + * This functions moves all devices from the pending list to the active
> + * list and schedules the deferred probe workqueue to process them.  It
> + * should be called anytime a driver is successfully bound to a device.
> + */
> +static void driver_deferred_probe_trigger(void)
> +{
> +	if (!driver_deferred_probe_enable)
> +		return;
> +
> +	/* A successful probe means that all the devices in the pending list
> +	 * should be triggered to be reprobed.  Move all the deferred devices
> +	 * into the active list so they can be retried by the workqueue */
> +	mutex_lock(&deferred_probe_mutex);
> +	list_splice_tail_init(&deferred_probe_pending_list,
> +			      &deferred_probe_active_list);
> +	mutex_unlock(&deferred_probe_mutex);
> +
> +	/* Kick the re-probe thread.  It may already be scheduled, but
> +	 * it is safe to kick it again. */
> +	schedule_work(&deferred_probe_work);
> +}
> +
> +/**
> + * deferred_probe_initcall() - Enable probing of deferred devices
> + *
> + * We don't want to get in the way when the bulk of drivers are getting probed.
> + * Instead, this initcall makes sure that deferred probing is delayed until
> + * late_initcall time.
> + */
> +static int deferred_probe_initcall(void)
> +{
> +	driver_deferred_probe_enable = true;
> +	driver_deferred_probe_trigger();
> +	return 0;
> +}
> +late_initcall(deferred_probe_initcall);
>  
>  static void driver_bound(struct device *dev)
>  {
> @@ -42,6 +165,11 @@ static void driver_bound(struct device *dev)
>  
>  	klist_add_tail(&dev->p->knode_driver, &dev->driver->p->klist_devices);
>  
> +	/* Make sure the device is no longer in one of the deferred lists
> +	 * and kick off retrying all pending devices */
> +	driver_deferred_probe_del(dev);
> +	driver_deferred_probe_trigger();
> +
>  	if (dev->bus)
>  		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
>  					     BUS_NOTIFY_BOUND_DRIVER, dev);
> @@ -142,7 +270,11 @@ probe_failed:
>  	driver_sysfs_remove(dev);
>  	dev->driver = NULL;
>  
> -	if (ret != -ENODEV && ret != -ENXIO) {
> +	if (ret == -EAGAIN) {
> +		/* Driver requested deferred probing */
> +		dev_info(dev, "Driver %s requests probe deferral\n", drv->name);
> +		driver_deferred_probe_add(dev);
> +	} else if (ret != -ENODEV && ret != -ENXIO) {
>  		/* driver matched but the probe failed */
>  		printk(KERN_WARNING
>  		       "%s: probe of %s failed with error %d\n",
> diff --git a/include/linux/device.h b/include/linux/device.h
> index c20dfbf..dab93a4 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -506,6 +506,10 @@ struct device_dma_parameters {
>   * @mutex:	Mutex to synchronize calls to its driver.
>   * @bus:	Type of bus device is on.
>   * @driver:	Which driver has allocated this
> + * @deferred_probe: entry in deferred_probe_list which is used to retry the
> + * 		binding of drivers which were unable to get all the resources
> + * 		needed by the device; typically because it depends on another
> + * 		driver getting probed first.
>   * @platform_data: Platform data specific to the device.
>   * 		Example: For devices on custom boards, as typical of embedded
>   * 		and SOC based hardware, Linux often uses platform_data to point
> @@ -564,6 +568,7 @@ struct device {
>  	struct bus_type	*bus;		/* type of bus device is on */
>  	struct device_driver *driver;	/* which driver has allocated this
>  					   device */
> +	struct list_head	deferred_probe;
>  	void		*platform_data;	/* Platform specific data, device
>  					   core doesn't touch it */
>  	struct dev_pm_info	power;
> 

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-10-04 14:51   ` G, Manjunath Kondaiah
  0 siblings, 0 replies; 63+ messages in thread
From: G, Manjunath Kondaiah @ 2011-10-04 14:51 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-omap, Arnd Bergmann, Greg Kroah-Hartman, Mark Brown,
	Manjunath GKondaiah, linux-kernel, Dilan Lee, linux-arm-kernel

Hi Grant,

On Thu, Sep 22, 2011 at 12:51:23PM -0600, Grant Likely wrote:
> Allow drivers to report at probe time that they cannot get all the resources
> required by the device, and should be retried at a later time.
> 
> This should completely solve the problem of getting devices
> initialized in the right order.  Right now this is mostly handled by
> mucking about with initcall ordering which is a complete hack, and
> doesn't even remotely handle the case where device drivers are in
> modules.  This approach completely sidesteps the issues by allowing
> driver registration to occur in any order, and any driver can request
> to be retried after a few more other drivers get probed.
> 
> v3: - Hold off workqueue scheduling until late_initcall so that the bulk
>       of driver probes are complete before we start retrying deferred devices.
>     - Tested with simple use cases.  Still needs more testing though.
>       Using it to get rid of the gpio early_initcall madness, or to replace
>       the ASoC internal probe deferral code would be ideal.
> v2: - added locking so it should no longer be utterly broken in that regard
>     - remove device from deferred list at device_del time.
>     - Still completely untested with any real use case, but has been
>       boot tested.
> 
> TODO: - Create a separate singlethread_workqueue so that drivers can't
>         mess things up by calling flush_work().
>       - Maybe this should be wrapped with a kconfig symbol so it can
>         be compiled out on systems that don't care.
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Cc: Greg Kroah-Hartman <greg@kroah.com>
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Dilan Lee <dilee@nvidia.com>
> Cc: Manjunath GKondaiah <manjunath.gkondaiah@linaro.org>
> ---
> 
> Hi Manjunath,
> 
> Here's the current state of the patch.  The major think that needs to
> be done is to convert it to use a separate workqueue as described in
> the TODO above.  It also needs some users adapted to it.  One of the
> gpio drivers would work; preferably one of the newer drivers that
> doesn't have a lot of drivers depending on the early_initcall()
> behaviour yet.

I have tested this patch on omap3 beagle board by making:
1. omap-gpio driver init as late_initcall instead of postcore_initcall
2. mmc driver probe will request gpio through gpio_request and gpio driver
returns -EDEFER_PROBE which in turn makes mmc driver to request deferral probe.
3. When deferral probe gets activated, it scans driver entries and it will not 
find any match for mmc driver probe.

After step 3, mmc driver probe will not get called due to device and driver
mismatch during "bus_for_each_drv" execution.

The behaviour is no different when it is used with singlethread_workqueue.

Here is bootlog:
Texas Instruments X-Loader 1.4.2 (Feb 19 2009 - 12:01:24)
Reading boot sector
Loading u-boot.bin from mmc


U-Boot 2009.11 (Feb 23 2010 - 15:33:48)

OMAP3530-GP ES3.0, CPU-OPP2 L3-165MHz
OMAP3 Beagle board + LPDDR/NAND
I2C:   ready
DRAM:  128 MB
NAND:  256 MiB
In:    serial
Out:   serial
Err:   serial
Board revision Ax/Bx
Die ID #5ac400030000000004013f8901001001
Hit any key to stop autoboot:  0 
mmc0 is available
mmc - MMC sub-system

Usage:
mmc init [dev] - init MMC sub system
mmc device [dev] - show or set current device
reading uImage

3237432 bytes read
## Booting kernel from Legacy Image at 80000000 ...
      Image Name:   Linux-3.1.0-rc8-00001-g4dc9843-d
      Image Type:   ARM Linux Kernel Image (uncompressed)
      Data Size:    3237368 Bytes =  3.1 MB
      Load Address: 80008000
      Entry Point:  80008000
      Verifying Checksum ... OK
      Kernel Image ... OK
OK

Starting kernel ...

Uncompressing Linux... done, booting the kernel.
[    0.000000] Linux version 3.1.0-rc8-00001-g4dc9843-dirty (manju@manju-desktop) (gcc version 4.5.2 (Ubuntu/Linaro4.5.2-8ubuntu3) ) #7 SMP Tue Oct 4 19:19:49 IST 2011
[    0.000000] CPU: ARMv7 Processor [411fc083] revision 3 (ARMv7), cr=10c53c7f
[    0.000000] CPU: VIPT nonaliasing data cache, VIPT nonaliasing instruction cache
[    0.000000] Machine: OMAP3 Beagle Board
[    0.000000] Memory policy: ECC disabled, Data cache writeback
[    0.000000] OMAP3430/3530 ES3.0 (l2cache iva sgx neon isp)

...

[    3.554656] driver_probe_device: drv->name: omap_hsmmc
[    3.560241] omap_hsmmc_probe: ...........Enter *********************
[    3.566955] omap_hsmmc_gpio_init: ..... enter .........!!!!
[    3.572906] platform omap_hsmmc.0: Driver omap_hsmmc requests probe deferral

...

[    3.712097] driver_probe_device: drv->name: omap_gpio
[    3.717681] omap_device: omap_gpio.0: new worst case activate latency 0: 30517
[    3.727813] OMAP GPIO hardware version 2.5
[    3.732421] driver_probe_device: drv->name: omap_gpio
[    3.739776] driver_probe_device: drv->name: omap_gpio
[    3.747039] driver_probe_device: drv->name: omap_gpio
[    3.754241] driver_probe_device: drv->name: omap_gpio
[    3.761413] driver_probe_device: drv->name: omap_gpio
...
[    3.800170] platform omap_hsmmc.0: Retrying from deferred list
[    3.806304] bus_probe_device: device autoprobe 
[    3.811096] device_attach: ...........test-1
[    3.815643] __device_attach: ...........test-4
[    3.820312] __device_attach: ...........test-4
[    3.825012] __device_attach: ...........test-4
[    3.829681] __device_attach: ...........test-4
[    3.834411] __device_attach: ...........test-4
[    3.839111] __device_attach: ...........test-4
[    3.843749] __device_attach: ...........test-4
[    3.848480] __device_attach: ...........test-4
[    3.853118] __device_attach: ...........test-4
[    3.857818] __device_attach: ...........test-4
[    3.862548] __device_attach: ...........test-4
[    3.867218] __device_attach: ...........test-4
[    3.871917] __device_attach: ...........test-4
[    3.876556] __device_attach: ...........test-4
[    3.881286] __device_attach: ...........test-4
[    3.885986] __device_attach: ...........test-4
[    3.890655] __device_attach: ...........test-4
[    3.895355] __device_attach: ...........test-4
[    3.900024] __device_attach: ...........test-4
[    3.904724] __device_attach: ...........test-4
[    3.909423] __device_attach: ...........test-4
[    3.914093] __device_attach: ...........test-4
[    3.918792] __device_attach: ...........test-4
[    3.923461] __device_attach: ...........test-4
[    3.928192] __device_attach: ...........test-4
[    3.932891] __device_attach: ...........test-4
[    3.937530] __device_attach: ...........test-4

[    3.944458] input: gpio-keys as /devices/platform/gpio-keys/input/input1
[    3.955657] twl_rtc twl_rtc: setting system clock to 2000-01-01 00:01:59 UTC (946684919)
[    3.969390] Waiting for root device /dev/mmcblk0p2...

Here it hangs since MMC driver is not ready!

-Manjunath


> 
> Mark Brown may also be able to suggest specific examples.
> 
> For everyone else; this is the current state of the patch.  I think it
> is in pretty good shape other than the TODO item above.  I'm turning
> it over to Manjunath to  polish up for merging.  I would appreciate
> any feedback.
> 
> g.
> 
> 
>  drivers/base/base.h    |    1 
>  drivers/base/core.c    |    2 +
>  drivers/base/dd.c      |  134 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/device.h |    5 ++
>  4 files changed, 141 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index a34dca0..9641309 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -105,6 +105,7 @@ extern void bus_remove_driver(struct device_driver *drv);
>  
>  extern void driver_detach(struct device_driver *drv);
>  extern int driver_probe_device(struct device_driver *drv, struct device *dev);
> +extern void driver_deferred_probe_del(struct device *dev);
>  static inline int driver_match_device(struct device_driver *drv,
>  				      struct device *dev)
>  {
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index bc8729d..0d37e18 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -588,6 +588,7 @@ void device_initialize(struct device *dev)
>  {
>  	dev->kobj.kset = devices_kset;
>  	kobject_init(&dev->kobj, &device_ktype);
> +	INIT_LIST_HEAD(&dev->deferred_probe);
>  	INIT_LIST_HEAD(&dev->dma_pools);
>  	mutex_init(&dev->mutex);
>  	lockdep_set_novalidate_class(&dev->mutex);
> @@ -1119,6 +1120,7 @@ void device_del(struct device *dev)
>  	device_remove_file(dev, &uevent_attr);
>  	device_remove_attrs(dev);
>  	bus_remove_device(dev);
> +	driver_deferred_probe_del(dev);
>  
>  	/*
>  	 * Some platform devices are driven without driver attached
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 6658da7..8be5b33 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -28,6 +28,129 @@
>  #include "base.h"
>  #include "power/power.h"
>  
> +/*
> + * Deferred Probe infrastructure.
> + *
> + * Sometimes driver probe order matters, but the kernel doesn't always have
> + * dependency information which means some drivers will get probed before a
> + * resource it depends on is available.  For example, an SDHCI driver may
> + * first need a GPIO line from an i2c GPIO controller before it can be
> + * initialized.  If a required resource is not available yet, a driver can
> + * request probing to be deferred by returning -EAGAIN from its probe hook
> + *
> + * Deferred probe maintains two lists of devices, a pending list and an active
> + * list.  A driver returning -EAGAIN causes the device to be added to the
> + * pending list.
> + *
> + * The deferred_probe_mutex *must* be held any time the deferred_probe_*_list
> + * of the (struct device*)->deferred_probe pointers are manipulated
> + */
> +static DEFINE_MUTEX(deferred_probe_mutex);
> +static LIST_HEAD(deferred_probe_pending_list);
> +static LIST_HEAD(deferred_probe_active_list);
> +
> +/**
> + * deferred_probe_work_func() - Retry probing devices in the active list.
> + */
> +static void deferred_probe_work_func(struct work_struct *work)
> +{
> +	struct device *dev;
> +	/*
> +	 * This bit is tricky.  We want to process every device in the
> +	 * deferred list, but devices can be removed from the list at any
> +	 * time while inside this for-each loop.  There are two things that
> +	 * need to be protected against:
> +	 * - if the device is removed from the deferred_probe_list, then we
> +	 *   loose our place in the loop.  Since any device can be removed
> +	 *   asynchronously, list_for_each_entry_safe() wouldn't make things
> +	 *   much better.  Simplest solution is to restart walking the list
> +	 *   whenever the current device gets removed.  Not the most efficient,
> +	 *   but is simple to implement and easy to audit for correctness.
> +	 * - if the device is unregistered, and freed, then there is a risk
> +	 *   of a null pointer dereference.  This code uses get/put_device()
> +	 *   to ensure the device cannot disappear from under our feet.
> +	 */
> +	mutex_lock(&deferred_probe_mutex);
> +	while (!list_empty(&deferred_probe_active_list)) {
> +		dev = list_first_entry(&deferred_probe_active_list,
> +					typeof(*dev), deferred_probe);
> +		list_del_init(&dev->deferred_probe);
> +
> +		get_device(dev);
> +
> +		/* Drop the mutex while probing each device; the probe path
> +		 * may manipulate the deferred list */
> +		mutex_unlock(&deferred_probe_mutex);
> +		dev_dbg(dev, "Retrying from deferred list\n");
> +		bus_probe_device(dev);
> +		mutex_lock(&deferred_probe_mutex);
> +
> +		put_device(dev);
> +	}
> +	mutex_unlock(&deferred_probe_mutex);
> +}
> +static DECLARE_WORK(deferred_probe_work, deferred_probe_work_func);
> +
> +static void driver_deferred_probe_add(struct device *dev)
> +{
> +	mutex_lock(&deferred_probe_mutex);
> +	if (list_empty(&dev->deferred_probe)) {
> +		dev_dbg(dev, "Added to deferred list\n");
> +		list_add(&dev->deferred_probe, &deferred_probe_pending_list);
> +	}
> +	mutex_unlock(&deferred_probe_mutex);
> +}
> +
> +void driver_deferred_probe_del(struct device *dev)
> +{
> +	mutex_lock(&deferred_probe_mutex);
> +	if (!list_empty(&dev->deferred_probe)) {
> +		dev_dbg(dev, "Removed from deferred list\n");
> +		list_del_init(&dev->deferred_probe);
> +	}
> +	mutex_unlock(&deferred_probe_mutex);
> +}
> +
> +static bool driver_deferred_probe_enable = false;
> +/**
> + * driver_deferred_probe_trigger() - Kick off re-probing deferred devices
> + *
> + * This functions moves all devices from the pending list to the active
> + * list and schedules the deferred probe workqueue to process them.  It
> + * should be called anytime a driver is successfully bound to a device.
> + */
> +static void driver_deferred_probe_trigger(void)
> +{
> +	if (!driver_deferred_probe_enable)
> +		return;
> +
> +	/* A successful probe means that all the devices in the pending list
> +	 * should be triggered to be reprobed.  Move all the deferred devices
> +	 * into the active list so they can be retried by the workqueue */
> +	mutex_lock(&deferred_probe_mutex);
> +	list_splice_tail_init(&deferred_probe_pending_list,
> +			      &deferred_probe_active_list);
> +	mutex_unlock(&deferred_probe_mutex);
> +
> +	/* Kick the re-probe thread.  It may already be scheduled, but
> +	 * it is safe to kick it again. */
> +	schedule_work(&deferred_probe_work);
> +}
> +
> +/**
> + * deferred_probe_initcall() - Enable probing of deferred devices
> + *
> + * We don't want to get in the way when the bulk of drivers are getting probed.
> + * Instead, this initcall makes sure that deferred probing is delayed until
> + * late_initcall time.
> + */
> +static int deferred_probe_initcall(void)
> +{
> +	driver_deferred_probe_enable = true;
> +	driver_deferred_probe_trigger();
> +	return 0;
> +}
> +late_initcall(deferred_probe_initcall);
>  
>  static void driver_bound(struct device *dev)
>  {
> @@ -42,6 +165,11 @@ static void driver_bound(struct device *dev)
>  
>  	klist_add_tail(&dev->p->knode_driver, &dev->driver->p->klist_devices);
>  
> +	/* Make sure the device is no longer in one of the deferred lists
> +	 * and kick off retrying all pending devices */
> +	driver_deferred_probe_del(dev);
> +	driver_deferred_probe_trigger();
> +
>  	if (dev->bus)
>  		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
>  					     BUS_NOTIFY_BOUND_DRIVER, dev);
> @@ -142,7 +270,11 @@ probe_failed:
>  	driver_sysfs_remove(dev);
>  	dev->driver = NULL;
>  
> -	if (ret != -ENODEV && ret != -ENXIO) {
> +	if (ret == -EAGAIN) {
> +		/* Driver requested deferred probing */
> +		dev_info(dev, "Driver %s requests probe deferral\n", drv->name);
> +		driver_deferred_probe_add(dev);
> +	} else if (ret != -ENODEV && ret != -ENXIO) {
>  		/* driver matched but the probe failed */
>  		printk(KERN_WARNING
>  		       "%s: probe of %s failed with error %d\n",
> diff --git a/include/linux/device.h b/include/linux/device.h
> index c20dfbf..dab93a4 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -506,6 +506,10 @@ struct device_dma_parameters {
>   * @mutex:	Mutex to synchronize calls to its driver.
>   * @bus:	Type of bus device is on.
>   * @driver:	Which driver has allocated this
> + * @deferred_probe: entry in deferred_probe_list which is used to retry the
> + * 		binding of drivers which were unable to get all the resources
> + * 		needed by the device; typically because it depends on another
> + * 		driver getting probed first.
>   * @platform_data: Platform data specific to the device.
>   * 		Example: For devices on custom boards, as typical of embedded
>   * 		and SOC based hardware, Linux often uses platform_data to point
> @@ -564,6 +568,7 @@ struct device {
>  	struct bus_type	*bus;		/* type of bus device is on */
>  	struct device_driver *driver;	/* which driver has allocated this
>  					   device */
> +	struct list_head	deferred_probe;
>  	void		*platform_data;	/* Platform specific data, device
>  					   core doesn't touch it */
>  	struct dev_pm_info	power;
> 

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-10-04 14:51   ` G, Manjunath Kondaiah
  0 siblings, 0 replies; 63+ messages in thread
From: G, Manjunath Kondaiah @ 2011-10-04 14:51 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Grant,

On Thu, Sep 22, 2011 at 12:51:23PM -0600, Grant Likely wrote:
> Allow drivers to report at probe time that they cannot get all the resources
> required by the device, and should be retried at a later time.
> 
> This should completely solve the problem of getting devices
> initialized in the right order.  Right now this is mostly handled by
> mucking about with initcall ordering which is a complete hack, and
> doesn't even remotely handle the case where device drivers are in
> modules.  This approach completely sidesteps the issues by allowing
> driver registration to occur in any order, and any driver can request
> to be retried after a few more other drivers get probed.
> 
> v3: - Hold off workqueue scheduling until late_initcall so that the bulk
>       of driver probes are complete before we start retrying deferred devices.
>     - Tested with simple use cases.  Still needs more testing though.
>       Using it to get rid of the gpio early_initcall madness, or to replace
>       the ASoC internal probe deferral code would be ideal.
> v2: - added locking so it should no longer be utterly broken in that regard
>     - remove device from deferred list at device_del time.
>     - Still completely untested with any real use case, but has been
>       boot tested.
> 
> TODO: - Create a separate singlethread_workqueue so that drivers can't
>         mess things up by calling flush_work().
>       - Maybe this should be wrapped with a kconfig symbol so it can
>         be compiled out on systems that don't care.
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Cc: Greg Kroah-Hartman <greg@kroah.com>
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Dilan Lee <dilee@nvidia.com>
> Cc: Manjunath GKondaiah <manjunath.gkondaiah@linaro.org>
> ---
> 
> Hi Manjunath,
> 
> Here's the current state of the patch.  The major think that needs to
> be done is to convert it to use a separate workqueue as described in
> the TODO above.  It also needs some users adapted to it.  One of the
> gpio drivers would work; preferably one of the newer drivers that
> doesn't have a lot of drivers depending on the early_initcall()
> behaviour yet.

I have tested this patch on omap3 beagle board by making:
1. omap-gpio driver init as late_initcall instead of postcore_initcall
2. mmc driver probe will request gpio through gpio_request and gpio driver
returns -EDEFER_PROBE which in turn makes mmc driver to request deferral probe.
3. When deferral probe gets activated, it scans driver entries and it will not 
find any match for mmc driver probe.

After step 3, mmc driver probe will not get called due to device and driver
mismatch during "bus_for_each_drv" execution.

The behaviour is no different when it is used with singlethread_workqueue.

Here is bootlog:
Texas Instruments X-Loader 1.4.2 (Feb 19 2009 - 12:01:24)
Reading boot sector
Loading u-boot.bin from mmc


U-Boot 2009.11 (Feb 23 2010 - 15:33:48)

OMAP3530-GP ES3.0, CPU-OPP2 L3-165MHz
OMAP3 Beagle board + LPDDR/NAND
I2C:   ready
DRAM:  128 MB
NAND:  256 MiB
In:    serial
Out:   serial
Err:   serial
Board revision Ax/Bx
Die ID #5ac400030000000004013f8901001001
Hit any key to stop autoboot:  0 
mmc0 is available
mmc - MMC sub-system

Usage:
mmc init [dev] - init MMC sub system
mmc device [dev] - show or set current device
reading uImage

3237432 bytes read
## Booting kernel from Legacy Image at 80000000 ...
      Image Name:   Linux-3.1.0-rc8-00001-g4dc9843-d
      Image Type:   ARM Linux Kernel Image (uncompressed)
      Data Size:    3237368 Bytes =  3.1 MB
      Load Address: 80008000
      Entry Point:  80008000
      Verifying Checksum ... OK
      Kernel Image ... OK
OK

Starting kernel ...

Uncompressing Linux... done, booting the kernel.
[    0.000000] Linux version 3.1.0-rc8-00001-g4dc9843-dirty (manju at manju-desktop) (gcc version 4.5.2 (Ubuntu/Linaro4.5.2-8ubuntu3) ) #7 SMP Tue Oct 4 19:19:49 IST 2011
[    0.000000] CPU: ARMv7 Processor [411fc083] revision 3 (ARMv7), cr=10c53c7f
[    0.000000] CPU: VIPT nonaliasing data cache, VIPT nonaliasing instruction cache
[    0.000000] Machine: OMAP3 Beagle Board
[    0.000000] Memory policy: ECC disabled, Data cache writeback
[    0.000000] OMAP3430/3530 ES3.0 (l2cache iva sgx neon isp)

...

[    3.554656] driver_probe_device: drv->name: omap_hsmmc
[    3.560241] omap_hsmmc_probe: ...........Enter *********************
[    3.566955] omap_hsmmc_gpio_init: ..... enter .........!!!!
[    3.572906] platform omap_hsmmc.0: Driver omap_hsmmc requests probe deferral

...

[    3.712097] driver_probe_device: drv->name: omap_gpio
[    3.717681] omap_device: omap_gpio.0: new worst case activate latency 0: 30517
[    3.727813] OMAP GPIO hardware version 2.5
[    3.732421] driver_probe_device: drv->name: omap_gpio
[    3.739776] driver_probe_device: drv->name: omap_gpio
[    3.747039] driver_probe_device: drv->name: omap_gpio
[    3.754241] driver_probe_device: drv->name: omap_gpio
[    3.761413] driver_probe_device: drv->name: omap_gpio
...
[    3.800170] platform omap_hsmmc.0: Retrying from deferred list
[    3.806304] bus_probe_device: device autoprobe 
[    3.811096] device_attach: ...........test-1
[    3.815643] __device_attach: ...........test-4
[    3.820312] __device_attach: ...........test-4
[    3.825012] __device_attach: ...........test-4
[    3.829681] __device_attach: ...........test-4
[    3.834411] __device_attach: ...........test-4
[    3.839111] __device_attach: ...........test-4
[    3.843749] __device_attach: ...........test-4
[    3.848480] __device_attach: ...........test-4
[    3.853118] __device_attach: ...........test-4
[    3.857818] __device_attach: ...........test-4
[    3.862548] __device_attach: ...........test-4
[    3.867218] __device_attach: ...........test-4
[    3.871917] __device_attach: ...........test-4
[    3.876556] __device_attach: ...........test-4
[    3.881286] __device_attach: ...........test-4
[    3.885986] __device_attach: ...........test-4
[    3.890655] __device_attach: ...........test-4
[    3.895355] __device_attach: ...........test-4
[    3.900024] __device_attach: ...........test-4
[    3.904724] __device_attach: ...........test-4
[    3.909423] __device_attach: ...........test-4
[    3.914093] __device_attach: ...........test-4
[    3.918792] __device_attach: ...........test-4
[    3.923461] __device_attach: ...........test-4
[    3.928192] __device_attach: ...........test-4
[    3.932891] __device_attach: ...........test-4
[    3.937530] __device_attach: ...........test-4

[    3.944458] input: gpio-keys as /devices/platform/gpio-keys/input/input1
[    3.955657] twl_rtc twl_rtc: setting system clock to 2000-01-01 00:01:59 UTC (946684919)
[    3.969390] Waiting for root device /dev/mmcblk0p2...

Here it hangs since MMC driver is not ready!

-Manjunath


> 
> Mark Brown may also be able to suggest specific examples.
> 
> For everyone else; this is the current state of the patch.  I think it
> is in pretty good shape other than the TODO item above.  I'm turning
> it over to Manjunath to  polish up for merging.  I would appreciate
> any feedback.
> 
> g.
> 
> 
>  drivers/base/base.h    |    1 
>  drivers/base/core.c    |    2 +
>  drivers/base/dd.c      |  134 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/device.h |    5 ++
>  4 files changed, 141 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index a34dca0..9641309 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -105,6 +105,7 @@ extern void bus_remove_driver(struct device_driver *drv);
>  
>  extern void driver_detach(struct device_driver *drv);
>  extern int driver_probe_device(struct device_driver *drv, struct device *dev);
> +extern void driver_deferred_probe_del(struct device *dev);
>  static inline int driver_match_device(struct device_driver *drv,
>  				      struct device *dev)
>  {
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index bc8729d..0d37e18 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -588,6 +588,7 @@ void device_initialize(struct device *dev)
>  {
>  	dev->kobj.kset = devices_kset;
>  	kobject_init(&dev->kobj, &device_ktype);
> +	INIT_LIST_HEAD(&dev->deferred_probe);
>  	INIT_LIST_HEAD(&dev->dma_pools);
>  	mutex_init(&dev->mutex);
>  	lockdep_set_novalidate_class(&dev->mutex);
> @@ -1119,6 +1120,7 @@ void device_del(struct device *dev)
>  	device_remove_file(dev, &uevent_attr);
>  	device_remove_attrs(dev);
>  	bus_remove_device(dev);
> +	driver_deferred_probe_del(dev);
>  
>  	/*
>  	 * Some platform devices are driven without driver attached
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 6658da7..8be5b33 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -28,6 +28,129 @@
>  #include "base.h"
>  #include "power/power.h"
>  
> +/*
> + * Deferred Probe infrastructure.
> + *
> + * Sometimes driver probe order matters, but the kernel doesn't always have
> + * dependency information which means some drivers will get probed before a
> + * resource it depends on is available.  For example, an SDHCI driver may
> + * first need a GPIO line from an i2c GPIO controller before it can be
> + * initialized.  If a required resource is not available yet, a driver can
> + * request probing to be deferred by returning -EAGAIN from its probe hook
> + *
> + * Deferred probe maintains two lists of devices, a pending list and an active
> + * list.  A driver returning -EAGAIN causes the device to be added to the
> + * pending list.
> + *
> + * The deferred_probe_mutex *must* be held any time the deferred_probe_*_list
> + * of the (struct device*)->deferred_probe pointers are manipulated
> + */
> +static DEFINE_MUTEX(deferred_probe_mutex);
> +static LIST_HEAD(deferred_probe_pending_list);
> +static LIST_HEAD(deferred_probe_active_list);
> +
> +/**
> + * deferred_probe_work_func() - Retry probing devices in the active list.
> + */
> +static void deferred_probe_work_func(struct work_struct *work)
> +{
> +	struct device *dev;
> +	/*
> +	 * This bit is tricky.  We want to process every device in the
> +	 * deferred list, but devices can be removed from the list at any
> +	 * time while inside this for-each loop.  There are two things that
> +	 * need to be protected against:
> +	 * - if the device is removed from the deferred_probe_list, then we
> +	 *   loose our place in the loop.  Since any device can be removed
> +	 *   asynchronously, list_for_each_entry_safe() wouldn't make things
> +	 *   much better.  Simplest solution is to restart walking the list
> +	 *   whenever the current device gets removed.  Not the most efficient,
> +	 *   but is simple to implement and easy to audit for correctness.
> +	 * - if the device is unregistered, and freed, then there is a risk
> +	 *   of a null pointer dereference.  This code uses get/put_device()
> +	 *   to ensure the device cannot disappear from under our feet.
> +	 */
> +	mutex_lock(&deferred_probe_mutex);
> +	while (!list_empty(&deferred_probe_active_list)) {
> +		dev = list_first_entry(&deferred_probe_active_list,
> +					typeof(*dev), deferred_probe);
> +		list_del_init(&dev->deferred_probe);
> +
> +		get_device(dev);
> +
> +		/* Drop the mutex while probing each device; the probe path
> +		 * may manipulate the deferred list */
> +		mutex_unlock(&deferred_probe_mutex);
> +		dev_dbg(dev, "Retrying from deferred list\n");
> +		bus_probe_device(dev);
> +		mutex_lock(&deferred_probe_mutex);
> +
> +		put_device(dev);
> +	}
> +	mutex_unlock(&deferred_probe_mutex);
> +}
> +static DECLARE_WORK(deferred_probe_work, deferred_probe_work_func);
> +
> +static void driver_deferred_probe_add(struct device *dev)
> +{
> +	mutex_lock(&deferred_probe_mutex);
> +	if (list_empty(&dev->deferred_probe)) {
> +		dev_dbg(dev, "Added to deferred list\n");
> +		list_add(&dev->deferred_probe, &deferred_probe_pending_list);
> +	}
> +	mutex_unlock(&deferred_probe_mutex);
> +}
> +
> +void driver_deferred_probe_del(struct device *dev)
> +{
> +	mutex_lock(&deferred_probe_mutex);
> +	if (!list_empty(&dev->deferred_probe)) {
> +		dev_dbg(dev, "Removed from deferred list\n");
> +		list_del_init(&dev->deferred_probe);
> +	}
> +	mutex_unlock(&deferred_probe_mutex);
> +}
> +
> +static bool driver_deferred_probe_enable = false;
> +/**
> + * driver_deferred_probe_trigger() - Kick off re-probing deferred devices
> + *
> + * This functions moves all devices from the pending list to the active
> + * list and schedules the deferred probe workqueue to process them.  It
> + * should be called anytime a driver is successfully bound to a device.
> + */
> +static void driver_deferred_probe_trigger(void)
> +{
> +	if (!driver_deferred_probe_enable)
> +		return;
> +
> +	/* A successful probe means that all the devices in the pending list
> +	 * should be triggered to be reprobed.  Move all the deferred devices
> +	 * into the active list so they can be retried by the workqueue */
> +	mutex_lock(&deferred_probe_mutex);
> +	list_splice_tail_init(&deferred_probe_pending_list,
> +			      &deferred_probe_active_list);
> +	mutex_unlock(&deferred_probe_mutex);
> +
> +	/* Kick the re-probe thread.  It may already be scheduled, but
> +	 * it is safe to kick it again. */
> +	schedule_work(&deferred_probe_work);
> +}
> +
> +/**
> + * deferred_probe_initcall() - Enable probing of deferred devices
> + *
> + * We don't want to get in the way when the bulk of drivers are getting probed.
> + * Instead, this initcall makes sure that deferred probing is delayed until
> + * late_initcall time.
> + */
> +static int deferred_probe_initcall(void)
> +{
> +	driver_deferred_probe_enable = true;
> +	driver_deferred_probe_trigger();
> +	return 0;
> +}
> +late_initcall(deferred_probe_initcall);
>  
>  static void driver_bound(struct device *dev)
>  {
> @@ -42,6 +165,11 @@ static void driver_bound(struct device *dev)
>  
>  	klist_add_tail(&dev->p->knode_driver, &dev->driver->p->klist_devices);
>  
> +	/* Make sure the device is no longer in one of the deferred lists
> +	 * and kick off retrying all pending devices */
> +	driver_deferred_probe_del(dev);
> +	driver_deferred_probe_trigger();
> +
>  	if (dev->bus)
>  		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
>  					     BUS_NOTIFY_BOUND_DRIVER, dev);
> @@ -142,7 +270,11 @@ probe_failed:
>  	driver_sysfs_remove(dev);
>  	dev->driver = NULL;
>  
> -	if (ret != -ENODEV && ret != -ENXIO) {
> +	if (ret == -EAGAIN) {
> +		/* Driver requested deferred probing */
> +		dev_info(dev, "Driver %s requests probe deferral\n", drv->name);
> +		driver_deferred_probe_add(dev);
> +	} else if (ret != -ENODEV && ret != -ENXIO) {
>  		/* driver matched but the probe failed */
>  		printk(KERN_WARNING
>  		       "%s: probe of %s failed with error %d\n",
> diff --git a/include/linux/device.h b/include/linux/device.h
> index c20dfbf..dab93a4 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -506,6 +506,10 @@ struct device_dma_parameters {
>   * @mutex:	Mutex to synchronize calls to its driver.
>   * @bus:	Type of bus device is on.
>   * @driver:	Which driver has allocated this
> + * @deferred_probe: entry in deferred_probe_list which is used to retry the
> + * 		binding of drivers which were unable to get all the resources
> + * 		needed by the device; typically because it depends on another
> + * 		driver getting probed first.
>   * @platform_data: Platform data specific to the device.
>   * 		Example: For devices on custom boards, as typical of embedded
>   * 		and SOC based hardware, Linux often uses platform_data to point
> @@ -564,6 +568,7 @@ struct device {
>  	struct bus_type	*bus;		/* type of bus device is on */
>  	struct device_driver *driver;	/* which driver has allocated this
>  					   device */
> +	struct list_head	deferred_probe;
>  	void		*platform_data;	/* Platform specific data, device
>  					   core doesn't touch it */
>  	struct dev_pm_info	power;
> 

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-10-03 23:02   ` Kevin Hilman
@ 2011-10-04 15:52     ` Grant Likely
  -1 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-10-04 15:52 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: linux-kernel, linux-arm-kernel, Greg Kroah-Hartman, Dilan Lee,
	Mark Brown, Manjunath GKondaiah, Arnd Bergmann

On Mon, Oct 3, 2011 at 5:02 PM, Kevin Hilman <khilman@ti.com> wrote:
> Grant Likely <grant.likely@secretlab.ca> writes:
>
>> Allow drivers to report at probe time that they cannot get all the resources
>> required by the device, and should be retried at a later time.
>>
>> This should completely solve the problem of getting devices
>> initialized in the right order.  Right now this is mostly handled by
>> mucking about with initcall ordering which is a complete hack, and
>> doesn't even remotely handle the case where device drivers are in
>> modules.  This approach completely sidesteps the issues by allowing
>> driver registration to occur in any order, and any driver can request
>> to be retried after a few more other drivers get probed.
>
> This is great work, thanks!
>
> For the TODO list:
>
> While the proposed patch should solve probe order dependencies, I don't
> think it will solve the suspend/resume ordering dependencies, which are
> typically the same.
>
> Currenly suspend/resume order is based on the order devices are *added*
> (device_add() -> device_pm_add() -> device added to dpm_list), so
> unfortunately, deferring probe isn't going to affect suspend/resume
> ordering.
>
> Extending this to also address suspend/resume ordering by also changing
> when the device is added to the dpm_list (or possibly creating another
> list) should probably be explored as well.

Hmm, yes, I think this is worth exploring.  It doesn't help with
runtime pm dependencies, but it has the potential to make PM just work
if the list order is updated each time a device is successfully bound
to a driver.  Manjunath, can you investigate what it would take to do
this? (after getting the core deferral patch finalized; I don't want
to block that work)?

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-10-04 15:52     ` Grant Likely
  0 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-10-04 15:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 3, 2011 at 5:02 PM, Kevin Hilman <khilman@ti.com> wrote:
> Grant Likely <grant.likely@secretlab.ca> writes:
>
>> Allow drivers to report at probe time that they cannot get all the resources
>> required by the device, and should be retried at a later time.
>>
>> This should completely solve the problem of getting devices
>> initialized in the right order. ?Right now this is mostly handled by
>> mucking about with initcall ordering which is a complete hack, and
>> doesn't even remotely handle the case where device drivers are in
>> modules. ?This approach completely sidesteps the issues by allowing
>> driver registration to occur in any order, and any driver can request
>> to be retried after a few more other drivers get probed.
>
> This is great work, thanks!
>
> For the TODO list:
>
> While the proposed patch should solve probe order dependencies, I don't
> think it will solve the suspend/resume ordering dependencies, which are
> typically the same.
>
> Currenly suspend/resume order is based on the order devices are *added*
> (device_add() -> device_pm_add() -> device added to dpm_list), so
> unfortunately, deferring probe isn't going to affect suspend/resume
> ordering.
>
> Extending this to also address suspend/resume ordering by also changing
> when the device is added to the dpm_list (or possibly creating another
> list) should probably be explored as well.

Hmm, yes, I think this is worth exploring.  It doesn't help with
runtime pm dependencies, but it has the potential to make PM just work
if the list order is updated each time a device is successfully bound
to a driver.  Manjunath, can you investigate what it would take to do
this? (after getting the core deferral patch finalized; I don't want
to block that work)?

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-10-04 14:51   ` G, Manjunath Kondaiah
  (?)
@ 2011-10-04 15:58     ` Grant Likely
  -1 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-10-04 15:58 UTC (permalink / raw)
  To: G, Manjunath Kondaiah
  Cc: linux-kernel, linux-arm-kernel, Greg Kroah-Hartman, Dilan Lee,
	Mark Brown, Manjunath GKondaiah, Arnd Bergmann, linux-omap

On Tue, Oct 4, 2011 at 8:51 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote:
> On Thu, Sep 22, 2011 at 12:51:23PM -0600, Grant Likely wrote:
>> Hi Manjunath,
>>
>> Here's the current state of the patch.  The major think that needs to
>> be done is to convert it to use a separate workqueue as described in
>> the TODO above.  It also needs some users adapted to it.  One of the
>> gpio drivers would work; preferably one of the newer drivers that
>> doesn't have a lot of drivers depending on the early_initcall()
>> behaviour yet.
>
> I have tested this patch on omap3 beagle board by making:
> 1. omap-gpio driver init as late_initcall instead of postcore_initcall
> 2. mmc driver probe will request gpio through gpio_request and gpio driver
> returns -EDEFER_PROBE which in turn makes mmc driver to request deferral probe.
> 3. When deferral probe gets activated, it scans driver entries and it will not
> find any match for mmc driver probe.

Looks like drivers/mmc/host/omap.c is using platform_driver_probe()
instead of platform_driver_register().  Add the probe hook to the
platform_driver structure and change it to call
platform_driver_register() and it should work.  Don't forget to change
mmc_omap_probe from __init to __devinit.

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-10-04 15:58     ` Grant Likely
  0 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-10-04 15:58 UTC (permalink / raw)
  To: G, Manjunath Kondaiah
  Cc: linux-kernel, linux-arm-kernel, Greg Kroah-Hartman, Dilan Lee,
	Mark Brown, Manjunath GKondaiah, Arnd Bergmann, linux-omap

On Tue, Oct 4, 2011 at 8:51 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote:
> On Thu, Sep 22, 2011 at 12:51:23PM -0600, Grant Likely wrote:
>> Hi Manjunath,
>>
>> Here's the current state of the patch.  The major think that needs to
>> be done is to convert it to use a separate workqueue as described in
>> the TODO above.  It also needs some users adapted to it.  One of the
>> gpio drivers would work; preferably one of the newer drivers that
>> doesn't have a lot of drivers depending on the early_initcall()
>> behaviour yet.
>
> I have tested this patch on omap3 beagle board by making:
> 1. omap-gpio driver init as late_initcall instead of postcore_initcall
> 2. mmc driver probe will request gpio through gpio_request and gpio driver
> returns -EDEFER_PROBE which in turn makes mmc driver to request deferral probe.
> 3. When deferral probe gets activated, it scans driver entries and it will not
> find any match for mmc driver probe.

Looks like drivers/mmc/host/omap.c is using platform_driver_probe()
instead of platform_driver_register().  Add the probe hook to the
platform_driver structure and change it to call
platform_driver_register() and it should work.  Don't forget to change
mmc_omap_probe from __init to __devinit.

g.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-10-04 15:58     ` Grant Likely
  0 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-10-04 15:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Oct 4, 2011 at 8:51 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote:
> On Thu, Sep 22, 2011 at 12:51:23PM -0600, Grant Likely wrote:
>> Hi Manjunath,
>>
>> Here's the current state of the patch. ?The major think that needs to
>> be done is to convert it to use a separate workqueue as described in
>> the TODO above. ?It also needs some users adapted to it. ?One of the
>> gpio drivers would work; preferably one of the newer drivers that
>> doesn't have a lot of drivers depending on the early_initcall()
>> behaviour yet.
>
> I have tested this patch on omap3 beagle board by making:
> 1. omap-gpio driver init as late_initcall instead of postcore_initcall
> 2. mmc driver probe will request gpio through gpio_request and gpio driver
> returns -EDEFER_PROBE which in turn makes mmc driver to request deferral probe.
> 3. When deferral probe gets activated, it scans driver entries and it will not
> find any match for mmc driver probe.

Looks like drivers/mmc/host/omap.c is using platform_driver_probe()
instead of platform_driver_register().  Add the probe hook to the
platform_driver structure and change it to call
platform_driver_register() and it should work.  Don't forget to change
mmc_omap_probe from __init to __devinit.

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-10-04 15:58     ` Grant Likely
@ 2011-10-04 18:35       ` G, Manjunath Kondaiah
  -1 siblings, 0 replies; 63+ messages in thread
From: G, Manjunath Kondaiah @ 2011-10-04 18:35 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, linux-arm-kernel, Greg Kroah-Hartman, Dilan Lee,
	Mark Brown, Manjunath GKondaiah, Arnd Bergmann, linux-omap

On Tue, Oct 04, 2011 at 09:58:10AM -0600, Grant Likely wrote:
> On Tue, Oct 4, 2011 at 8:51 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote:
> > On Thu, Sep 22, 2011 at 12:51:23PM -0600, Grant Likely wrote:
> >> Hi Manjunath,
> >>
> >> Here's the current state of the patch.  The major think that needs to
> >> be done is to convert it to use a separate workqueue as described in
> >> the TODO above.  It also needs some users adapted to it.  One of the
> >> gpio drivers would work; preferably one of the newer drivers that
> >> doesn't have a lot of drivers depending on the early_initcall()
> >> behaviour yet.
> >
> > I have tested this patch on omap3 beagle board by making:
> > 1. omap-gpio driver init as late_initcall instead of postcore_initcall
> > 2. mmc driver probe will request gpio through gpio_request and gpio driver
> > returns -EDEFER_PROBE which in turn makes mmc driver to request deferral probe.
> > 3. When deferral probe gets activated, it scans driver entries and it will not
> > find any match for mmc driver probe.
> 
> Looks like drivers/mmc/host/omap.c is using platform_driver_probe()
> instead of platform_driver_register().  Add the probe hook to the
> platform_driver structure and change it to call
> platform_driver_register() and it should work.  Don't forget to change
> mmc_omap_probe from __init to __devinit.

Yes. After changing it into platform_driver_register, I can see mmc probe is
getting completed from deferred probe list. But, MMC upper layer will check 
probe and it waits for ever since probe_count is not getting incremented.

Log:

[    1.807830] platform omap_hsmmc.0: Driver omap_hsmmc requests probe deferral

...

[    1.948760] omap_device: omap_gpio.0: new worst case activate latency 0:30517
[    1.959259] OMAP GPIO hardware version 2.5

...

[    2.000488] platform omap_hsmmc.0: Retrying from deferred list
[    2.008026] omap_device: omap_hsmmc.0: new worst case activate latency 0:244140
[    2.020080] input: gpio-keys as /devices/platform/gpio-keys/input/input1
[    2.035827] omap_hsmmc omap_hsmmc.0: Probe success...
[    2.042083] twl_rtc twl_rtc: setting system clock to 2000-01-01 01:06:35 UTC
(946688795)
[    2.056030] Waiting for root device /dev/mmcblk0p2...
[    2.061492] driver_probe_done: probe_count = 0
[    2.168518] driver_probe_done: probe_count = 0
[    2.277832] driver_probe_done: probe_count = 0
[    2.387207] driver_probe_done: probe_count = 0
[    2.496582] driver_probe_done: probe_count = 0

Waits for ever in init/do_mount.c:
	if ((ROOT_DEV == 0) && root_wait) {
		printk(KERN_INFO "Waiting for root device %s...\n",
			saved_root_name);
		while (driver_probe_done() != 0 ||
			(ROOT_DEV = name_to_dev_t(saved_root_name)) == 0)
			msleep(100);
		async_synchronize_full();
	}

-Manjunath

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-10-04 18:35       ` G, Manjunath Kondaiah
  0 siblings, 0 replies; 63+ messages in thread
From: G, Manjunath Kondaiah @ 2011-10-04 18:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Oct 04, 2011 at 09:58:10AM -0600, Grant Likely wrote:
> On Tue, Oct 4, 2011 at 8:51 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote:
> > On Thu, Sep 22, 2011 at 12:51:23PM -0600, Grant Likely wrote:
> >> Hi Manjunath,
> >>
> >> Here's the current state of the patch. ?The major think that needs to
> >> be done is to convert it to use a separate workqueue as described in
> >> the TODO above. ?It also needs some users adapted to it. ?One of the
> >> gpio drivers would work; preferably one of the newer drivers that
> >> doesn't have a lot of drivers depending on the early_initcall()
> >> behaviour yet.
> >
> > I have tested this patch on omap3 beagle board by making:
> > 1. omap-gpio driver init as late_initcall instead of postcore_initcall
> > 2. mmc driver probe will request gpio through gpio_request and gpio driver
> > returns -EDEFER_PROBE which in turn makes mmc driver to request deferral probe.
> > 3. When deferral probe gets activated, it scans driver entries and it will not
> > find any match for mmc driver probe.
> 
> Looks like drivers/mmc/host/omap.c is using platform_driver_probe()
> instead of platform_driver_register().  Add the probe hook to the
> platform_driver structure and change it to call
> platform_driver_register() and it should work.  Don't forget to change
> mmc_omap_probe from __init to __devinit.

Yes. After changing it into platform_driver_register, I can see mmc probe is
getting completed from deferred probe list. But, MMC upper layer will check 
probe and it waits for ever since probe_count is not getting incremented.

Log:

[    1.807830] platform omap_hsmmc.0: Driver omap_hsmmc requests probe deferral

...

[    1.948760] omap_device: omap_gpio.0: new worst case activate latency 0:30517
[    1.959259] OMAP GPIO hardware version 2.5

...

[    2.000488] platform omap_hsmmc.0: Retrying from deferred list
[    2.008026] omap_device: omap_hsmmc.0: new worst case activate latency 0:244140
[    2.020080] input: gpio-keys as /devices/platform/gpio-keys/input/input1
[    2.035827] omap_hsmmc omap_hsmmc.0: Probe success...
[    2.042083] twl_rtc twl_rtc: setting system clock to 2000-01-01 01:06:35 UTC
(946688795)
[    2.056030] Waiting for root device /dev/mmcblk0p2...
[    2.061492] driver_probe_done: probe_count = 0
[    2.168518] driver_probe_done: probe_count = 0
[    2.277832] driver_probe_done: probe_count = 0
[    2.387207] driver_probe_done: probe_count = 0
[    2.496582] driver_probe_done: probe_count = 0

Waits for ever in init/do_mount.c:
	if ((ROOT_DEV == 0) && root_wait) {
		printk(KERN_INFO "Waiting for root device %s...\n",
			saved_root_name);
		while (driver_probe_done() != 0 ||
			(ROOT_DEV = name_to_dev_t(saved_root_name)) == 0)
			msleep(100);
		async_synchronize_full();
	}

-Manjunath

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-10-04 18:35       ` G, Manjunath Kondaiah
@ 2011-10-04 23:35         ` Grant Likely
  -1 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-10-04 23:35 UTC (permalink / raw)
  To: G, Manjunath Kondaiah
  Cc: linux-kernel, linux-arm-kernel, Greg Kroah-Hartman, Dilan Lee,
	Mark Brown, Manjunath GKondaiah, Arnd Bergmann, linux-omap

On Wed, Oct 05, 2011 at 12:05:16AM +0530, G, Manjunath Kondaiah wrote:
> On Tue, Oct 04, 2011 at 09:58:10AM -0600, Grant Likely wrote:
> > On Tue, Oct 4, 2011 at 8:51 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote:
> > > On Thu, Sep 22, 2011 at 12:51:23PM -0600, Grant Likely wrote:
> > >> Hi Manjunath,
> > >>
> > >> Here's the current state of the patch.  The major think that needs to
> > >> be done is to convert it to use a separate workqueue as described in
> > >> the TODO above.  It also needs some users adapted to it.  One of the
> > >> gpio drivers would work; preferably one of the newer drivers that
> > >> doesn't have a lot of drivers depending on the early_initcall()
> > >> behaviour yet.
> > >
> > > I have tested this patch on omap3 beagle board by making:
> > > 1. omap-gpio driver init as late_initcall instead of postcore_initcall
> > > 2. mmc driver probe will request gpio through gpio_request and gpio driver
> > > returns -EDEFER_PROBE which in turn makes mmc driver to request deferral probe.
> > > 3. When deferral probe gets activated, it scans driver entries and it will not
> > > find any match for mmc driver probe.
> > 
> > Looks like drivers/mmc/host/omap.c is using platform_driver_probe()
> > instead of platform_driver_register().  Add the probe hook to the
> > platform_driver structure and change it to call
> > platform_driver_register() and it should work.  Don't forget to change
> > mmc_omap_probe from __init to __devinit.
> 
> Yes. After changing it into platform_driver_register, I can see mmc probe is
> getting completed from deferred probe list. But, MMC upper layer will check 
> probe and it waits for ever since probe_count is not getting incremented.
> 
> Log:
> 
> [    1.807830] platform omap_hsmmc.0: Driver omap_hsmmc requests probe deferral
> 
> ...
> 
> [    1.948760] omap_device: omap_gpio.0: new worst case activate latency 0:30517
> [    1.959259] OMAP GPIO hardware version 2.5
> 
> ...
> 
> [    2.000488] platform omap_hsmmc.0: Retrying from deferred list
> [    2.008026] omap_device: omap_hsmmc.0: new worst case activate latency 0:244140
> [    2.020080] input: gpio-keys as /devices/platform/gpio-keys/input/input1
> [    2.035827] omap_hsmmc omap_hsmmc.0: Probe success...
> [    2.042083] twl_rtc twl_rtc: setting system clock to 2000-01-01 01:06:35 UTC
> (946688795)
> [    2.056030] Waiting for root device /dev/mmcblk0p2...
> [    2.061492] driver_probe_done: probe_count = 0
> [    2.168518] driver_probe_done: probe_count = 0
> [    2.277832] driver_probe_done: probe_count = 0
> [    2.387207] driver_probe_done: probe_count = 0
> [    2.496582] driver_probe_done: probe_count = 0
> 
> Waits for ever in init/do_mount.c:
> 	if ((ROOT_DEV == 0) && root_wait) {
> 		printk(KERN_INFO "Waiting for root device %s...\n",
> 			saved_root_name);
> 		while (driver_probe_done() != 0 ||
> 			(ROOT_DEV = name_to_dev_t(saved_root_name)) == 0)
> 			msleep(100);
> 		async_synchronize_full();
> 	}

Okay, it looks like the driver deferral workqueue needs to get kicked
off before the kernel starts waiting for the root filesystem.  Check
the initcall level that is being used by do_mount, and make sure the
deferred probe starts before that...

Hmmm.... wait, that doesn't make sense because the probe is still
successful.  What triggers the exit conditions in that while loop?
How would it be different from when the driver can probe successfully
without deferral?

g.

> 
> -Manjunath

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-10-04 23:35         ` Grant Likely
  0 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-10-04 23:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Oct 05, 2011 at 12:05:16AM +0530, G, Manjunath Kondaiah wrote:
> On Tue, Oct 04, 2011 at 09:58:10AM -0600, Grant Likely wrote:
> > On Tue, Oct 4, 2011 at 8:51 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote:
> > > On Thu, Sep 22, 2011 at 12:51:23PM -0600, Grant Likely wrote:
> > >> Hi Manjunath,
> > >>
> > >> Here's the current state of the patch. ?The major think that needs to
> > >> be done is to convert it to use a separate workqueue as described in
> > >> the TODO above. ?It also needs some users adapted to it. ?One of the
> > >> gpio drivers would work; preferably one of the newer drivers that
> > >> doesn't have a lot of drivers depending on the early_initcall()
> > >> behaviour yet.
> > >
> > > I have tested this patch on omap3 beagle board by making:
> > > 1. omap-gpio driver init as late_initcall instead of postcore_initcall
> > > 2. mmc driver probe will request gpio through gpio_request and gpio driver
> > > returns -EDEFER_PROBE which in turn makes mmc driver to request deferral probe.
> > > 3. When deferral probe gets activated, it scans driver entries and it will not
> > > find any match for mmc driver probe.
> > 
> > Looks like drivers/mmc/host/omap.c is using platform_driver_probe()
> > instead of platform_driver_register().  Add the probe hook to the
> > platform_driver structure and change it to call
> > platform_driver_register() and it should work.  Don't forget to change
> > mmc_omap_probe from __init to __devinit.
> 
> Yes. After changing it into platform_driver_register, I can see mmc probe is
> getting completed from deferred probe list. But, MMC upper layer will check 
> probe and it waits for ever since probe_count is not getting incremented.
> 
> Log:
> 
> [    1.807830] platform omap_hsmmc.0: Driver omap_hsmmc requests probe deferral
> 
> ...
> 
> [    1.948760] omap_device: omap_gpio.0: new worst case activate latency 0:30517
> [    1.959259] OMAP GPIO hardware version 2.5
> 
> ...
> 
> [    2.000488] platform omap_hsmmc.0: Retrying from deferred list
> [    2.008026] omap_device: omap_hsmmc.0: new worst case activate latency 0:244140
> [    2.020080] input: gpio-keys as /devices/platform/gpio-keys/input/input1
> [    2.035827] omap_hsmmc omap_hsmmc.0: Probe success...
> [    2.042083] twl_rtc twl_rtc: setting system clock to 2000-01-01 01:06:35 UTC
> (946688795)
> [    2.056030] Waiting for root device /dev/mmcblk0p2...
> [    2.061492] driver_probe_done: probe_count = 0
> [    2.168518] driver_probe_done: probe_count = 0
> [    2.277832] driver_probe_done: probe_count = 0
> [    2.387207] driver_probe_done: probe_count = 0
> [    2.496582] driver_probe_done: probe_count = 0
> 
> Waits for ever in init/do_mount.c:
> 	if ((ROOT_DEV == 0) && root_wait) {
> 		printk(KERN_INFO "Waiting for root device %s...\n",
> 			saved_root_name);
> 		while (driver_probe_done() != 0 ||
> 			(ROOT_DEV = name_to_dev_t(saved_root_name)) == 0)
> 			msleep(100);
> 		async_synchronize_full();
> 	}

Okay, it looks like the driver deferral workqueue needs to get kicked
off before the kernel starts waiting for the root filesystem.  Check
the initcall level that is being used by do_mount, and make sure the
deferred probe starts before that...

Hmmm.... wait, that doesn't make sense because the probe is still
successful.  What triggers the exit conditions in that while loop?
How would it be different from when the driver can probe successfully
without deferral?

g.

> 
> -Manjunath

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-10-04 23:35         ` Grant Likely
@ 2011-10-07  3:31           ` G, Manjunath Kondaiah
  -1 siblings, 0 replies; 63+ messages in thread
From: G, Manjunath Kondaiah @ 2011-10-07  3:31 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, linux-arm-kernel, Greg Kroah-Hartman, Dilan Lee,
	Mark Brown, Manjunath GKondaiah, Arnd Bergmann, linux-omap

On Tue, Oct 04, 2011 at 05:35:04PM -0600, Grant Likely wrote:
> On Wed, Oct 05, 2011 at 12:05:16AM +0530, G, Manjunath Kondaiah wrote:
> > On Tue, Oct 04, 2011 at 09:58:10AM -0600, Grant Likely wrote:
> > > On Tue, Oct 4, 2011 at 8:51 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote:
> > > > On Thu, Sep 22, 2011 at 12:51:23PM -0600, Grant Likely wrote:
> > > >> Hi Manjunath,
> > > >>
> > > >> Here's the current state of the patch.  The major think that needs to
> > > >> be done is to convert it to use a separate workqueue as described in
> > > >> the TODO above.  It also needs some users adapted to it.  One of the
> > > >> gpio drivers would work; preferably one of the newer drivers that
> > > >> doesn't have a lot of drivers depending on the early_initcall()
> > > >> behaviour yet.
> > > >
> > > > I have tested this patch on omap3 beagle board by making:
> > > > 1. omap-gpio driver init as late_initcall instead of postcore_initcall
> > > > 2. mmc driver probe will request gpio through gpio_request and gpio driver
> > > > returns -EDEFER_PROBE which in turn makes mmc driver to request deferral probe.
> > > > 3. When deferral probe gets activated, it scans driver entries and it will not
> > > > find any match for mmc driver probe.
> > > 
> > > Looks like drivers/mmc/host/omap.c is using platform_driver_probe()
> > > instead of platform_driver_register().  Add the probe hook to the
> > > platform_driver structure and change it to call
> > > platform_driver_register() and it should work.  Don't forget to change
> > > mmc_omap_probe from __init to __devinit.
> > 
> > Yes. After changing it into platform_driver_register, I can see mmc probe is
> > getting completed from deferred probe list. But, MMC upper layer will check 
> > probe and it waits for ever since probe_count is not getting incremented.
> > 
> > Log:
> > 
> > [    1.807830] platform omap_hsmmc.0: Driver omap_hsmmc requests probe deferral
> > 
> > ...
> > 
> > [    1.948760] omap_device: omap_gpio.0: new worst case activate latency 0:30517
> > [    1.959259] OMAP GPIO hardware version 2.5
> > 
> > ...
> > 
> > [    2.000488] platform omap_hsmmc.0: Retrying from deferred list
> > [    2.008026] omap_device: omap_hsmmc.0: new worst case activate latency 0:244140
> > [    2.020080] input: gpio-keys as /devices/platform/gpio-keys/input/input1
> > [    2.035827] omap_hsmmc omap_hsmmc.0: Probe success...
> > [    2.042083] twl_rtc twl_rtc: setting system clock to 2000-01-01 01:06:35 UTC
> > (946688795)
> > [    2.056030] Waiting for root device /dev/mmcblk0p2...
> > [    2.061492] driver_probe_done: probe_count = 0
> > [    2.168518] driver_probe_done: probe_count = 0
> > [    2.277832] driver_probe_done: probe_count = 0
> > [    2.387207] driver_probe_done: probe_count = 0
> > [    2.496582] driver_probe_done: probe_count = 0
> > 
> > Waits for ever in init/do_mount.c:
> > 	if ((ROOT_DEV == 0) && root_wait) {
> > 		printk(KERN_INFO "Waiting for root device %s...\n",
> > 			saved_root_name);
> > 		while (driver_probe_done() != 0 ||
> > 			(ROOT_DEV = name_to_dev_t(saved_root_name)) == 0)
> > 			msleep(100);
> > 		async_synchronize_full();
> > 	}
> 
> Okay, it looks like the driver deferral workqueue needs to get kicked
> off before the kernel starts waiting for the root filesystem.  Check
> the initcall level that is being used by do_mount, and make sure the
> deferred probe starts before that...
> 
> Hmmm.... wait, that doesn't make sense because the probe is still
> successful.  What triggers the exit conditions in that while loop?
> How would it be different from when the driver can probe successfully
> without deferral?

Looks like OMAP MMC uses TWL GPIO for card detect. I manually changed this 
twl gpio to omap gpio for testing purpose which is causing this problem. 
I have not tried to root cause further, I reverted back to twl gpio and requested
omap gpio in mmc driver probe. With omap gpio request and making gpio driver
init as lateinit, deferral probe works fine for MMC driver.

I will post updated patch series.

-Manjunath

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-10-07  3:31           ` G, Manjunath Kondaiah
  0 siblings, 0 replies; 63+ messages in thread
From: G, Manjunath Kondaiah @ 2011-10-07  3:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Oct 04, 2011 at 05:35:04PM -0600, Grant Likely wrote:
> On Wed, Oct 05, 2011 at 12:05:16AM +0530, G, Manjunath Kondaiah wrote:
> > On Tue, Oct 04, 2011 at 09:58:10AM -0600, Grant Likely wrote:
> > > On Tue, Oct 4, 2011 at 8:51 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote:
> > > > On Thu, Sep 22, 2011 at 12:51:23PM -0600, Grant Likely wrote:
> > > >> Hi Manjunath,
> > > >>
> > > >> Here's the current state of the patch. ?The major think that needs to
> > > >> be done is to convert it to use a separate workqueue as described in
> > > >> the TODO above. ?It also needs some users adapted to it. ?One of the
> > > >> gpio drivers would work; preferably one of the newer drivers that
> > > >> doesn't have a lot of drivers depending on the early_initcall()
> > > >> behaviour yet.
> > > >
> > > > I have tested this patch on omap3 beagle board by making:
> > > > 1. omap-gpio driver init as late_initcall instead of postcore_initcall
> > > > 2. mmc driver probe will request gpio through gpio_request and gpio driver
> > > > returns -EDEFER_PROBE which in turn makes mmc driver to request deferral probe.
> > > > 3. When deferral probe gets activated, it scans driver entries and it will not
> > > > find any match for mmc driver probe.
> > > 
> > > Looks like drivers/mmc/host/omap.c is using platform_driver_probe()
> > > instead of platform_driver_register().  Add the probe hook to the
> > > platform_driver structure and change it to call
> > > platform_driver_register() and it should work.  Don't forget to change
> > > mmc_omap_probe from __init to __devinit.
> > 
> > Yes. After changing it into platform_driver_register, I can see mmc probe is
> > getting completed from deferred probe list. But, MMC upper layer will check 
> > probe and it waits for ever since probe_count is not getting incremented.
> > 
> > Log:
> > 
> > [    1.807830] platform omap_hsmmc.0: Driver omap_hsmmc requests probe deferral
> > 
> > ...
> > 
> > [    1.948760] omap_device: omap_gpio.0: new worst case activate latency 0:30517
> > [    1.959259] OMAP GPIO hardware version 2.5
> > 
> > ...
> > 
> > [    2.000488] platform omap_hsmmc.0: Retrying from deferred list
> > [    2.008026] omap_device: omap_hsmmc.0: new worst case activate latency 0:244140
> > [    2.020080] input: gpio-keys as /devices/platform/gpio-keys/input/input1
> > [    2.035827] omap_hsmmc omap_hsmmc.0: Probe success...
> > [    2.042083] twl_rtc twl_rtc: setting system clock to 2000-01-01 01:06:35 UTC
> > (946688795)
> > [    2.056030] Waiting for root device /dev/mmcblk0p2...
> > [    2.061492] driver_probe_done: probe_count = 0
> > [    2.168518] driver_probe_done: probe_count = 0
> > [    2.277832] driver_probe_done: probe_count = 0
> > [    2.387207] driver_probe_done: probe_count = 0
> > [    2.496582] driver_probe_done: probe_count = 0
> > 
> > Waits for ever in init/do_mount.c:
> > 	if ((ROOT_DEV == 0) && root_wait) {
> > 		printk(KERN_INFO "Waiting for root device %s...\n",
> > 			saved_root_name);
> > 		while (driver_probe_done() != 0 ||
> > 			(ROOT_DEV = name_to_dev_t(saved_root_name)) == 0)
> > 			msleep(100);
> > 		async_synchronize_full();
> > 	}
> 
> Okay, it looks like the driver deferral workqueue needs to get kicked
> off before the kernel starts waiting for the root filesystem.  Check
> the initcall level that is being used by do_mount, and make sure the
> deferred probe starts before that...
> 
> Hmmm.... wait, that doesn't make sense because the probe is still
> successful.  What triggers the exit conditions in that while loop?
> How would it be different from when the driver can probe successfully
> without deferral?

Looks like OMAP MMC uses TWL GPIO for card detect. I manually changed this 
twl gpio to omap gpio for testing purpose which is causing this problem. 
I have not tried to root cause further, I reverted back to twl gpio and requested
omap gpio in mmc driver probe. With omap gpio request and making gpio driver
init as lateinit, deferral probe works fine for MMC driver.

I will post updated patch series.

-Manjunath

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-09-22 18:51 ` Grant Likely
@ 2011-10-11 20:47   ` Andrew Morton
  -1 siblings, 0 replies; 63+ messages in thread
From: Andrew Morton @ 2011-10-11 20:47 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, linux-arm-kernel, Greg Kroah-Hartman, Dilan Lee,
	Mark Brown, Manjunath GKondaiah, Arnd Bergmann

On Thu, 22 Sep 2011 12:51:23 -0600
Grant Likely <grant.likely@secretlab.ca> wrote:

> Allow drivers to report at probe time that they cannot get all the resources
> required by the device, and should be retried at a later time.
> 
> This should completely solve the problem of getting devices
> initialized in the right order.  Right now this is mostly handled by
> mucking about with initcall ordering which is a complete hack, and
> doesn't even remotely handle the case where device drivers are in
> modules.  This approach completely sidesteps the issues by allowing
> driver registration to occur in any order, and any driver can request
> to be retried after a few more other drivers get probed.

What happens is there is a circular dependency, or if a driver's
preconditions are never met?  AFAICT the code keeps running the probe
function for ever.

If so: bad.  The kernel should detect such situations, should
exhaustively report them and if possible, fix them up and struggle
onwards.

>
> ...
>
> +	 * This bit is tricky.  We want to process every device in the
> +	 * deferred list, but devices can be removed from the list at any
> +	 * time while inside this for-each loop.  There are two things that
> +	 * need to be protected against:
> +	 * - if the device is removed from the deferred_probe_list, then we
> +	 *   loose our place in the loop.  Since any device can be removed

s/loose/lose/

> +	 *   asynchronously, list_for_each_entry_safe() wouldn't make things
> +	 *   much better.  Simplest solution is to restart walking the list
> +	 *   whenever the current device gets removed.  Not the most efficient,
> +	 *   but is simple to implement and easy to audit for correctness.
> +	 * - if the device is unregistered, and freed, then there is a risk
> +	 *   of a null pointer dereference.  This code uses get/put_device()
> +	 *   to ensure the device cannot disappear from under our feet.
> +	 */
>
> ...
>
> +		/* Drop the mutex while probing each device; the probe path
> +		 * may manipulate the deferred list */

Please don't invent new coding styles.  Like this:

		/*
		 * Drop the mutex while probing each device; the probe path
		 * may manipulate the deferred list
		 */

(entire patch)

>
> ...
>

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-10-11 20:47   ` Andrew Morton
  0 siblings, 0 replies; 63+ messages in thread
From: Andrew Morton @ 2011-10-11 20:47 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 22 Sep 2011 12:51:23 -0600
Grant Likely <grant.likely@secretlab.ca> wrote:

> Allow drivers to report at probe time that they cannot get all the resources
> required by the device, and should be retried at a later time.
> 
> This should completely solve the problem of getting devices
> initialized in the right order.  Right now this is mostly handled by
> mucking about with initcall ordering which is a complete hack, and
> doesn't even remotely handle the case where device drivers are in
> modules.  This approach completely sidesteps the issues by allowing
> driver registration to occur in any order, and any driver can request
> to be retried after a few more other drivers get probed.

What happens is there is a circular dependency, or if a driver's
preconditions are never met?  AFAICT the code keeps running the probe
function for ever.

If so: bad.  The kernel should detect such situations, should
exhaustively report them and if possible, fix them up and struggle
onwards.

>
> ...
>
> +	 * This bit is tricky.  We want to process every device in the
> +	 * deferred list, but devices can be removed from the list at any
> +	 * time while inside this for-each loop.  There are two things that
> +	 * need to be protected against:
> +	 * - if the device is removed from the deferred_probe_list, then we
> +	 *   loose our place in the loop.  Since any device can be removed

s/loose/lose/

> +	 *   asynchronously, list_for_each_entry_safe() wouldn't make things
> +	 *   much better.  Simplest solution is to restart walking the list
> +	 *   whenever the current device gets removed.  Not the most efficient,
> +	 *   but is simple to implement and easy to audit for correctness.
> +	 * - if the device is unregistered, and freed, then there is a risk
> +	 *   of a null pointer dereference.  This code uses get/put_device()
> +	 *   to ensure the device cannot disappear from under our feet.
> +	 */
>
> ...
>
> +		/* Drop the mutex while probing each device; the probe path
> +		 * may manipulate the deferred list */

Please don't invent new coding styles.  Like this:

		/*
		 * Drop the mutex while probing each device; the probe path
		 * may manipulate the deferred list
		 */

(entire patch)

>
> ...
>

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-10-11 20:47   ` Andrew Morton
  (?)
@ 2011-10-11 21:07   ` David Daney
  2011-10-13  4:19       ` Grant Likely
  -1 siblings, 1 reply; 63+ messages in thread
From: David Daney @ 2011-10-11 21:07 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Grant Likely, linux-kernel, linux-arm-kernel, Greg Kroah-Hartman,
	Dilan Lee, Mark Brown, Manjunath GKondaiah, Arnd Bergmann

On 10/11/2011 01:47 PM, Andrew Morton wrote:
> On Thu, 22 Sep 2011 12:51:23 -0600
> Grant Likely<grant.likely@secretlab.ca>  wrote:
>
>> Allow drivers to report at probe time that they cannot get all the resources
>> required by the device, and should be retried at a later time.
>>
>> This should completely solve the problem of getting devices
>> initialized in the right order.  Right now this is mostly handled by
>> mucking about with initcall ordering which is a complete hack, and
>> doesn't even remotely handle the case where device drivers are in
>> modules.  This approach completely sidesteps the issues by allowing
>> driver registration to occur in any order, and any driver can request
>> to be retried after a few more other drivers get probed.
>
> What happens is there is a circular dependency, or if a driver's
> preconditions are never met?  AFAICT the code keeps running the probe
> function for ever.
>

The deferred probe functions are only run once per (other) driver 
binding event.  So once you quit registering new drivers, no further 
probing is done.  There is no endless loop happening here.

If the preconditions are never met, the driver will just sit in the list 
waiting.


> If so: bad.  The kernel should detect such situations, should
> exhaustively report them and if possible, fix them up and struggle
> onwards.
>

I don't think we should actively report anything, but being able to 
inspect the deferred probe list from user space might be useful for 
diagnosing problems


David Daney


^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
  2011-10-11 21:07   ` David Daney
@ 2011-10-13  4:19       ` Grant Likely
  0 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-10-13  4:19 UTC (permalink / raw)
  To: David Daney
  Cc: Andrew Morton, linux-kernel, linux-arm-kernel,
	Greg Kroah-Hartman, Dilan Lee, Mark Brown, Manjunath GKondaiah,
	Arnd Bergmann

On Tue, Oct 11, 2011 at 02:07:41PM -0700, David Daney wrote:
> On 10/11/2011 01:47 PM, Andrew Morton wrote:
> >On Thu, 22 Sep 2011 12:51:23 -0600
> >Grant Likely<grant.likely@secretlab.ca>  wrote:
> >
> >>Allow drivers to report at probe time that they cannot get all the resources
> >>required by the device, and should be retried at a later time.
> >>
> >>This should completely solve the problem of getting devices
> >>initialized in the right order.  Right now this is mostly handled by
> >>mucking about with initcall ordering which is a complete hack, and
> >>doesn't even remotely handle the case where device drivers are in
> >>modules.  This approach completely sidesteps the issues by allowing
> >>driver registration to occur in any order, and any driver can request
> >>to be retried after a few more other drivers get probed.
> >
> >What happens is there is a circular dependency, or if a driver's
> >preconditions are never met?  AFAICT the code keeps running the probe
> >function for ever.
> >
> 
> The deferred probe functions are only run once per (other) driver
> binding event.  So once you quit registering new drivers, no further
> probing is done.  There is no endless loop happening here.
> 
> If the preconditions are never met, the driver will just sit in the
> list waiting.

Plus, as an optimization, walking the deferred list doesn't begin
until late initcall time so that the first pass over the device
drivers proceeds completely before starting retries.

> >If so: bad.  The kernel should detect such situations, should
> >exhaustively report them and if possible, fix them up and struggle
> >onwards.

The kernel won't get stalled on deferred probe.  It may end up with
stale devices in the deferred list, but that only means those
particular devices won't get bound to a driver.  It isn't fatal.

> I don't think we should actively report anything, but being able to
> inspect the deferred probe list from user space might be useful for
> diagnosing problems

Well, we could dump out the remaining deferred devices in sysfs.
Alternately the kernel could dump them out to the console log after
userspace starts.  That would catch conditions where built-in drivers
aren't able to initialize their devices.

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [RFC PATCH v3] drivercore: Add driver probe deferral mechanism
@ 2011-10-13  4:19       ` Grant Likely
  0 siblings, 0 replies; 63+ messages in thread
From: Grant Likely @ 2011-10-13  4:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Oct 11, 2011 at 02:07:41PM -0700, David Daney wrote:
> On 10/11/2011 01:47 PM, Andrew Morton wrote:
> >On Thu, 22 Sep 2011 12:51:23 -0600
> >Grant Likely<grant.likely@secretlab.ca>  wrote:
> >
> >>Allow drivers to report at probe time that they cannot get all the resources
> >>required by the device, and should be retried at a later time.
> >>
> >>This should completely solve the problem of getting devices
> >>initialized in the right order.  Right now this is mostly handled by
> >>mucking about with initcall ordering which is a complete hack, and
> >>doesn't even remotely handle the case where device drivers are in
> >>modules.  This approach completely sidesteps the issues by allowing
> >>driver registration to occur in any order, and any driver can request
> >>to be retried after a few more other drivers get probed.
> >
> >What happens is there is a circular dependency, or if a driver's
> >preconditions are never met?  AFAICT the code keeps running the probe
> >function for ever.
> >
> 
> The deferred probe functions are only run once per (other) driver
> binding event.  So once you quit registering new drivers, no further
> probing is done.  There is no endless loop happening here.
> 
> If the preconditions are never met, the driver will just sit in the
> list waiting.

Plus, as an optimization, walking the deferred list doesn't begin
until late initcall time so that the first pass over the device
drivers proceeds completely before starting retries.

> >If so: bad.  The kernel should detect such situations, should
> >exhaustively report them and if possible, fix them up and struggle
> >onwards.

The kernel won't get stalled on deferred probe.  It may end up with
stale devices in the deferred list, but that only means those
particular devices won't get bound to a driver.  It isn't fatal.

> I don't think we should actively report anything, but being able to
> inspect the deferred probe list from user space might be useful for
> diagnosing problems

Well, we could dump out the remaining deferred devices in sysfs.
Alternately the kernel could dump them out to the console log after
userspace starts.  That would catch conditions where built-in drivers
aren't able to initialize their devices.

g.

^ permalink raw reply	[flat|nested] 63+ messages in thread

end of thread, other threads:[~2011-10-13  4:19 UTC | newest]

Thread overview: 63+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-22 18:51 [RFC PATCH v3] drivercore: Add driver probe deferral mechanism Grant Likely
2011-09-22 18:51 ` Grant Likely
2011-09-22 18:58 ` Joe Perches
2011-09-22 18:58   ` Joe Perches
2011-09-22 19:28 ` David Daney
2011-09-22 20:29 ` Alan Cox
2011-09-22 20:29   ` Alan Cox
2011-09-22 21:19   ` Grant Likely
2011-09-22 21:19     ` Grant Likely
2011-09-23 17:50     ` Valdis.Kletnieks
2011-09-23 17:50       ` Valdis.Kletnieks at vt.edu
2011-09-23 23:18       ` Grant Likely
2011-09-23 23:18         ` Grant Likely
2011-09-22 21:19   ` David Daney
2011-09-22 22:47     ` Alan Cox
2011-09-22 22:47       ` Alan Cox
2011-09-23  5:02       ` Grant Likely
2011-09-23  5:02         ` Grant Likely
2011-09-23 16:55       ` David Daney
2011-09-23 16:55         ` David Daney
2011-09-26 14:16 ` Mark Brown
2011-09-26 14:16   ` Mark Brown
2011-09-26 15:12   ` Russell King - ARM Linux
2011-09-26 15:12     ` Russell King - ARM Linux
2011-09-26 15:26     ` Mark Brown
2011-09-26 15:26       ` Mark Brown
2011-09-26 15:48       ` Grant Likely
2011-09-26 15:48         ` Grant Likely
2011-09-27 13:50         ` Arnd Bergmann
2011-09-27 13:50           ` Arnd Bergmann
2011-09-27 21:08           ` Grant Likely
2011-09-27 21:08             ` Grant Likely
2011-09-27 22:13             ` Mark Brown
2011-09-27 22:13               ` Mark Brown
2011-09-28 13:04               ` Arnd Bergmann
2011-09-28 13:04                 ` Arnd Bergmann
2011-09-28 13:20                 ` Mark Brown
2011-09-28 13:20                   ` Mark Brown
2011-09-28 23:14               ` Grant Likely
2011-09-28 23:14                 ` Grant Likely
2011-09-29 11:00                 ` Mark Brown
2011-09-29 11:00                   ` Mark Brown
2011-10-03 23:02 ` Kevin Hilman
2011-10-03 23:02   ` Kevin Hilman
2011-10-04 15:52   ` Grant Likely
2011-10-04 15:52     ` Grant Likely
2011-10-04 14:51 ` G, Manjunath Kondaiah
2011-10-04 14:51   ` G, Manjunath Kondaiah
2011-10-04 14:51   ` G, Manjunath Kondaiah
2011-10-04 15:58   ` Grant Likely
2011-10-04 15:58     ` Grant Likely
2011-10-04 15:58     ` Grant Likely
2011-10-04 18:35     ` G, Manjunath Kondaiah
2011-10-04 18:35       ` G, Manjunath Kondaiah
2011-10-04 23:35       ` Grant Likely
2011-10-04 23:35         ` Grant Likely
2011-10-07  3:31         ` G, Manjunath Kondaiah
2011-10-07  3:31           ` G, Manjunath Kondaiah
2011-10-11 20:47 ` Andrew Morton
2011-10-11 20:47   ` Andrew Morton
2011-10-11 21:07   ` David Daney
2011-10-13  4:19     ` Grant Likely
2011-10-13  4:19       ` Grant Likely

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.