[PATCH v2 0/3] ARM: OMAP: omap_device: track driver bound status

[PATCH v2 0/3] ARM: OMAP: omap_device: track driver bound status

[Kevin Hilman]

Add support for the omap_device layer to keep track of driver bound status.

This will be used to track whether or not a driver is bound in order
to make intelligent omap_device enable/idle decisions (e.g. ensuring
omap_devices are idled when no driver is present, or driver probe
fails.)

Also, the omap_device layer implements PM domains for all
omap_devices, and knowing whether or not a driver is bound is needed
to know whether or not a drivers callbacks are valid, especially in
cases of failed driver probe.

Kevin Hilman (3):
  ARM: OMAP: omap_device: keep track of driver bound status
  ARM: OMAP: omap_device: don't attempt late suspend if no driver bound
  ARM: OMAP: omap_device: idle devices with no driver bound

 arch/arm/plat-omap/include/plat/omap_device.h |    2 +
 arch/arm/plat-omap/omap_device.c              |   56 ++++++++++++++++++++++---
 2 files changed, 53 insertions(+), 5 deletions(-)

-- 
1.7.9.2

[PATCH v2 1/3] ARM: OMAP: omap_device: keep track of driver bound status

[Kevin Hilman]

Use the bus notifier to keep track of driver bound status by adding a
new internal field to struct omap_device: _driver_status.

This will be useful for follow-up patches which need to know whether
or not a driver is bound in order to make intelligent omap_device
enable/idle decisions.

Cc: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/plat-omap/include/plat/omap_device.h |    2 ++
 arch/arm/plat-omap/omap_device.c              |   14 +++++++++-----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
index 4327b2c..e7259c0 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -60,6 +60,7 @@ extern struct dev_pm_domain omap_device_pm_domain;
  * @_dev_wakeup_lat_limit: dev wakeup latency limit in nsec - set by OMAP PM
  * @_state: one of OMAP_DEVICE_STATE_* (see above)
  * @flags: device flags
+ * @_driver_status: one of BUS_NOTIFY_*_DRIVER from <linux/device.h>
  *
  * Integrates omap_hwmod data into Linux platform_device.
  *
@@ -73,6 +74,7 @@ struct omap_device {
 	struct omap_device_pm_latency	*pm_lats;
 	u32				dev_wakeup_lat;
 	u32				_dev_wakeup_lat_limit;
+	unsigned long			_driver_status;
 	u8				pm_lats_cnt;
 	s8				pm_lat_level;
 	u8				hwmods_cnt;
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index c490240..1d1b5ff 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -385,17 +385,21 @@ static int _omap_device_notifier_call(struct notifier_block *nb,
 				      unsigned long event, void *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
+	struct omap_device *od;
 
 	switch (event) {
-	case BUS_NOTIFY_ADD_DEVICE:
-		if (pdev->dev.of_node)
-			omap_device_build_from_dt(pdev);
-		break;
-
 	case BUS_NOTIFY_DEL_DEVICE:
 		if (pdev->archdata.od)
 			omap_device_delete(pdev->archdata.od);
 		break;
+	case BUS_NOTIFY_ADD_DEVICE:
+		if (pdev->dev.of_node)
+			omap_device_build_from_dt(pdev);
+		/* fall through */
+	default:
+		od = to_omap_device(pdev);
+		if (od)
+			od->_driver_status = event;
 	}
 
 	return NOTIFY_DONE;
-- 
1.7.9.2

[PATCH v2 2/3] ARM: OMAP: omap_device: don't attempt late suspend if no driver bound

[Kevin Hilman]

Currently, the omap_device PM domain layer uses the late suspend and
early resume callbacks to ensure devices are in their low power
states.

However, this is attempted even in cases where a driver probe has
failed.  If a driver's ->probe() method fails, the driver is likely in
a state where it is not expecting its runtime PM callbacks to be
called, yet currently the omap_device PM domain code attempts to call
the drivers callbacks.

To fix, use the omap_device driver_status field to check whether a
driver is bound to the omap_device before attempting to trigger driver
callbacks.

Cc: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/plat-omap/omap_device.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 1d1b5ff..150112e 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -756,6 +756,10 @@ static int _od_suspend_noirq(struct device *dev)
 	struct omap_device *od = to_omap_device(pdev);
 	int ret;
 
+	/* Don't attempt late suspend on a driver that is not bound */
+	if (od->_driver_status != BUS_NOTIFY_BOUND_DRIVER)
+		return 0;
+
 	ret = pm_generic_suspend_noirq(dev);
 
 	if (!ret && !pm_runtime_status_suspended(dev)) {
-- 
1.7.9.2

[PATCH v2 3/3] ARM: OMAP: omap_device: idle devices with no driver bound

[Kevin Hilman]

Under some circumstances, drivers may leave an omap_device enabled due
to driver programming errors, or due to a failure in the drivers
probe method.

Using the recently added omap_device driver_status field, we can
detect conditions where an omap_device is enabled but has no driver
bound and then ensure that the device is properly idled until it can
be probed again.

The goal of this feature is not only to detect and warn on these error
conditions, but also to ensure that devices are properly put in
low-power states so they do not prevent SoC-wide low-power states.

Cc: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/plat-omap/omap_device.c |   38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 150112e..0693260 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -1133,3 +1133,41 @@ static int __init omap_device_init(void)
 	return 0;
 }
 core_initcall(omap_device_init);
+
+/**
+ * omap_device_late_idle - idle devices without drivers
+ * @dev: struct device * associated with omap_device
+ * @data: unused
+ *
+ * Check the driver bound status of this device, and idle it
+ * if there is no driver attached.
+ */
+static int __init omap_device_late_idle(struct device *dev, void *data)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct omap_device *od = to_omap_device(pdev);
+
+	if (!od)
+		return 0;
+
+	/*
+	 * If omap_device state is enabled, but has no driver bound,
+	 * idle it.
+	 */
+	if (od->_driver_status != BUS_NOTIFY_BOUND_DRIVER) {
+		if (od->_state == OMAP_DEVICE_STATE_ENABLED) {
+			dev_warn(dev, "%s: enabled but no driver.  Idling\n",
+				__func__);
+			omap_device_idle(pdev);
+		}
+	}
+
+	return 0;
+}
+
+static int __init omap_device_late_init(void)
+{
+	bus_for_each_dev(&platform_bus_type, NULL, NULL, omap_device_late_idle);
+	return 0;
+}
+late_initcall(omap_device_late_init);
-- 
1.7.9.2

Re: [PATCH v2 3/3] ARM: OMAP: omap_device: idle devices with no driver bound

[Paul Walmsley]

On Fri, 10 Aug 2012, Kevin Hilman wrote:

> Under some circumstances, drivers may leave an omap_device enabled due
> to driver programming errors, or due to a failure in the drivers
> probe method.
> 
> Using the recently added omap_device driver_status field, we can
> detect conditions where an omap_device is enabled but has no driver
> bound and then ensure that the device is properly idled until it can
> be probed again.
> 
> The goal of this feature is not only to detect and warn on these error
> conditions, but also to ensure that devices are properly put in
> low-power states so they do not prevent SoC-wide low-power states.
> 
> Cc: Paul Walmsley <paul@pwsan.com>
> Signed-off-by: Kevin Hilman <khilman@ti.com>

Here's the queued version of this one.


- Paul

From: Kevin Hilman <khilman@ti.com>
Date: Fri, 10 Aug 2012 11:39:13 -0700
Subject: [PATCH] ARM: OMAP: omap_device: idle devices with no driver bound

Under some circumstances, drivers may leave an omap_device enabled due
to driver programming errors, or due to a failure in the drivers
probe method.

Using the recently added omap_device driver_status field, we can
detect conditions where an omap_device is enabled but has no driver
bound and then ensure that the device is properly idled until it can
be probed again.

The goal of this feature is not only to detect and warn on these error
conditions, but also to ensure that devices are properly put in
low-power states so they do not prevent SoC-wide low-power states.

Cc: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Kevin Hilman <khilman@ti.com>
[paul@pwsan.com: fixed checkpatch issue; removed blank line at top of 
 file]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/plat-omap/omap_device.c |   39 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 150112e..1f8d9c9 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -1,4 +1,3 @@
-
 /*
  * omap_device implementation
  *
@@ -1133,3 +1132,41 @@ static int __init omap_device_init(void)
 	return 0;
 }
 core_initcall(omap_device_init);
+
+/**
+ * omap_device_late_idle - idle devices without drivers
+ * @dev: struct device * associated with omap_device
+ * @data: unused
+ *
+ * Check the driver bound status of this device, and idle it
+ * if there is no driver attached.
+ */
+static int __init omap_device_late_idle(struct device *dev, void *data)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct omap_device *od = to_omap_device(pdev);
+
+	if (!od)
+		return 0;
+
+	/*
+	 * If omap_device state is enabled, but has no driver bound,
+	 * idle it.
+	 */
+	if (od->_driver_status != BUS_NOTIFY_BOUND_DRIVER) {
+		if (od->_state == OMAP_DEVICE_STATE_ENABLED) {
+			dev_warn(dev, "%s: enabled but no driver.  Idling\n",
+				 __func__);
+			omap_device_idle(pdev);
+		}
+	}
+
+	return 0;
+}
+
+static int __init omap_device_late_init(void)
+{
+	bus_for_each_dev(&platform_bus_type, NULL, NULL, omap_device_late_idle);
+	return 0;
+}
+late_initcall(omap_device_late_init);
-- 
1.7.10.4

Re: [PATCH v2 3/3] ARM: OMAP: omap_device: idle devices with no driver bound

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120810 15:59]:
> On Fri, 10 Aug 2012, Kevin Hilman wrote:
> 
> > Under some circumstances, drivers may leave an omap_device enabled due
> > to driver programming errors, or due to a failure in the drivers
> > probe method.
> > 
> > Using the recently added omap_device driver_status field, we can
> > detect conditions where an omap_device is enabled but has no driver
> > bound and then ensure that the device is properly idled until it can
> > be probed again.
> > 
> > The goal of this feature is not only to detect and warn on these error
> > conditions, but also to ensure that devices are properly put in
> > low-power states so they do not prevent SoC-wide low-power states.
> > 
> > Cc: Paul Walmsley <paul@pwsan.com>
> > Signed-off-by: Kevin Hilman <khilman@ti.com>
> 
> Here's the queued version of this one.

Nice to see this happening :)

Tony

Re: [PATCH v2 0/3] ARM: OMAP: omap_device: track driver bound status

[Paul Walmsley]

Hi Kevin,

On Fri, 10 Aug 2012, Kevin Hilman wrote:

> Add support for the omap_device layer to keep track of driver bound status.
> 
> This will be used to track whether or not a driver is bound in order
> to make intelligent omap_device enable/idle decisions (e.g. ensuring
> omap_devices are idled when no driver is present, or driver probe
> fails.)
> 
> Also, the omap_device layer implements PM domains for all
> omap_devices, and knowing whether or not a driver is bound is needed
> to know whether or not a drivers callbacks are valid, especially in
> cases of failed driver probe.
> 
> Kevin Hilman (3):
>   ARM: OMAP: omap_device: keep track of driver bound status
>   ARM: OMAP: omap_device: don't attempt late suspend if no driver bound
>   ARM: OMAP: omap_device: idle devices with no driver bound
> 
>  arch/arm/plat-omap/include/plat/omap_device.h |    2 +
>  arch/arm/plat-omap/omap_device.c              |   56 ++++++++++++++++++++++---
>  2 files changed, 53 insertions(+), 5 deletions(-)

Thanks, these have been queued for 3.7 with a couple of changes to patch 
3.  Those changes fix a checkpatch issue, and drop the superfluous blank 
line at the top of omap_device.c added by an earlier commit.


- Paul