All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] ARM: OMAP: omap_device: keep track of driver bound status
@ 2012-07-10 23:02 ` Kevin Hilman
  0 siblings, 0 replies; 10+ messages in thread
From: Kevin Hilman @ 2012-07-10 23:02 UTC (permalink / raw)
  To: linux-omap, Paul Walmsley; +Cc: linux-arm-kernel

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

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..e0486cb 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.
  *
@@ -78,6 +79,7 @@ struct omap_device {
 	u8				hwmods_cnt;
 	u8				_state;
 	u8                              flags;
+	u8				_driver_status;
 };
 
 /* Device driver interface (call via platform_data fn ptrs) */
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


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

* [PATCH 1/3] ARM: OMAP: omap_device: keep track of driver bound status
@ 2012-07-10 23:02 ` Kevin Hilman
  0 siblings, 0 replies; 10+ messages in thread
From: Kevin Hilman @ 2012-07-10 23:02 UTC (permalink / raw)
  To: linux-arm-kernel

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

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..e0486cb 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.
  *
@@ -78,6 +79,7 @@ struct omap_device {
 	u8				hwmods_cnt;
 	u8				_state;
 	u8                              flags;
+	u8				_driver_status;
 };
 
 /* Device driver interface (call via platform_data fn ptrs) */
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

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

* [PATCH 2/3] ARM: OMAP: omap_device: don't attempt late suspend if no driver bound
  2012-07-10 23:02 ` Kevin Hilman
@ 2012-07-10 23:02   ` Kevin Hilman
  -1 siblings, 0 replies; 10+ messages in thread
From: Kevin Hilman @ 2012-07-10 23:02 UTC (permalink / raw)
  To: linux-omap, Paul Walmsley; +Cc: linux-arm-kernel

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


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

* [PATCH 2/3] ARM: OMAP: omap_device: don't attempt late suspend if no driver bound
@ 2012-07-10 23:02   ` Kevin Hilman
  0 siblings, 0 replies; 10+ messages in thread
From: Kevin Hilman @ 2012-07-10 23:02 UTC (permalink / raw)
  To: linux-arm-kernel

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

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

* [PATCH 3/3] ARM: OMAP: omap_device: idle devices with no driver bound
  2012-07-10 23:02 ` Kevin Hilman
@ 2012-07-10 23:02   ` Kevin Hilman
  -1 siblings, 0 replies; 10+ messages in thread
From: Kevin Hilman @ 2012-07-10 23:02 UTC (permalink / raw)
  To: linux-omap, Paul Walmsley; +Cc: linux-arm-kernel

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 |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 150112e..28ab6af 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -1133,3 +1133,33 @@ static int __init omap_device_init(void)
 	return 0;
 }
 core_initcall(omap_device_init);
+
+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


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

* [PATCH 3/3] ARM: OMAP: omap_device: idle devices with no driver bound
@ 2012-07-10 23:02   ` Kevin Hilman
  0 siblings, 0 replies; 10+ messages in thread
From: Kevin Hilman @ 2012-07-10 23:02 UTC (permalink / raw)
  To: linux-arm-kernel

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 |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 150112e..28ab6af 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -1133,3 +1133,33 @@ static int __init omap_device_init(void)
 	return 0;
 }
 core_initcall(omap_device_init);
+
+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

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

* Re: [PATCH 1/3] ARM: OMAP: omap_device: keep track of driver bound status
  2012-07-10 23:02 ` Kevin Hilman
@ 2012-08-07 21:26   ` Paul Walmsley
  -1 siblings, 0 replies; 10+ messages in thread
From: Paul Walmsley @ 2012-08-07 21:26 UTC (permalink / raw)
  To: Kevin Hilman; +Cc: linux-omap, linux-arm-kernel

Hi Kevin

a couple of minor comments

On Tue, 10 Jul 2012, Kevin Hilman wrote:

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

"_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..e0486cb 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.
>   *
> @@ -78,6 +79,7 @@ struct omap_device {
>  	u8				hwmods_cnt;
>  	u8				_state;
>  	u8                              flags;
> +	u8				_driver_status;
>  };
>  
>  /* Device driver interface (call via platform_data fn ptrs) */
> 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;

_driver_status is a u8, but event is an unsigned long.  Might be worth 
adding a WARN() to complain if event is greater than (2^8)-1.

>  	}
>  
>  	return NOTIFY_DONE;
> -- 
> 1.7.9.2
> 


- Paul

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

* [PATCH 1/3] ARM: OMAP: omap_device: keep track of driver bound status
@ 2012-08-07 21:26   ` Paul Walmsley
  0 siblings, 0 replies; 10+ messages in thread
From: Paul Walmsley @ 2012-08-07 21:26 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Kevin

a couple of minor comments

On Tue, 10 Jul 2012, Kevin Hilman wrote:

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

"_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..e0486cb 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.
>   *
> @@ -78,6 +79,7 @@ struct omap_device {
>  	u8				hwmods_cnt;
>  	u8				_state;
>  	u8                              flags;
> +	u8				_driver_status;
>  };
>  
>  /* Device driver interface (call via platform_data fn ptrs) */
> 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;

_driver_status is a u8, but event is an unsigned long.  Might be worth 
adding a WARN() to complain if event is greater than (2^8)-1.

>  	}
>  
>  	return NOTIFY_DONE;
> -- 
> 1.7.9.2
> 


- Paul

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

* Re: [PATCH 1/3] ARM: OMAP: omap_device: keep track of driver bound status
  2012-08-07 21:26   ` Paul Walmsley
@ 2012-08-07 21:49     ` Kevin Hilman
  -1 siblings, 0 replies; 10+ messages in thread
From: Kevin Hilman @ 2012-08-07 21:49 UTC (permalink / raw)
  To: Paul Walmsley; +Cc: linux-omap, linux-arm-kernel

Paul Walmsley <paul@pwsan.com> writes:

> Hi Kevin
>
> a couple of minor comments
>
> On Tue, 10 Jul 2012, Kevin Hilman wrote:
>
>> Use the bus notifier to keep track of driver bound status by adding a
>> new internal field to struct omap_device: _driver_staus.
>
> "_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..e0486cb 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.
>>   *
>> @@ -78,6 +79,7 @@ struct omap_device {
>>  	u8				hwmods_cnt;
>>  	u8				_state;
>>  	u8                              flags;
>> +	u8				_driver_status;
>>  };
>>  
>>  /* Device driver interface (call via platform_data fn ptrs) */
>> 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;
>
> _driver_status is a u8, but event is an unsigned long.  Might be worth 
> adding a WARN() to complain if event is greater than (2^8)-1.
>

It's probably more future proof if I just make _driver_status an
unsigned long, in case the values of those events ever change in the
driver core.

Thanks for the review,

Kevin


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

* [PATCH 1/3] ARM: OMAP: omap_device: keep track of driver bound status
@ 2012-08-07 21:49     ` Kevin Hilman
  0 siblings, 0 replies; 10+ messages in thread
From: Kevin Hilman @ 2012-08-07 21:49 UTC (permalink / raw)
  To: linux-arm-kernel

Paul Walmsley <paul@pwsan.com> writes:

> Hi Kevin
>
> a couple of minor comments
>
> On Tue, 10 Jul 2012, Kevin Hilman wrote:
>
>> Use the bus notifier to keep track of driver bound status by adding a
>> new internal field to struct omap_device: _driver_staus.
>
> "_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..e0486cb 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.
>>   *
>> @@ -78,6 +79,7 @@ struct omap_device {
>>  	u8				hwmods_cnt;
>>  	u8				_state;
>>  	u8                              flags;
>> +	u8				_driver_status;
>>  };
>>  
>>  /* Device driver interface (call via platform_data fn ptrs) */
>> 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;
>
> _driver_status is a u8, but event is an unsigned long.  Might be worth 
> adding a WARN() to complain if event is greater than (2^8)-1.
>

It's probably more future proof if I just make _driver_status an
unsigned long, in case the values of those events ever change in the
driver core.

Thanks for the review,

Kevin

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

end of thread, other threads:[~2012-08-07 21:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-10 23:02 [PATCH 1/3] ARM: OMAP: omap_device: keep track of driver bound status Kevin Hilman
2012-07-10 23:02 ` Kevin Hilman
2012-07-10 23:02 ` [PATCH 2/3] ARM: OMAP: omap_device: don't attempt late suspend if no driver bound Kevin Hilman
2012-07-10 23:02   ` Kevin Hilman
2012-07-10 23:02 ` [PATCH 3/3] ARM: OMAP: omap_device: idle devices with " Kevin Hilman
2012-07-10 23:02   ` Kevin Hilman
2012-08-07 21:26 ` [PATCH 1/3] ARM: OMAP: omap_device: keep track of driver bound status Paul Walmsley
2012-08-07 21:26   ` Paul Walmsley
2012-08-07 21:49   ` Kevin Hilman
2012-08-07 21:49     ` Kevin Hilman

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.