[RFC/PATCH 0/7] decouple platform_device from omap_device

[RFC/PATCH 0/7] decouple platform_device from omap_device

[Kevin Hilman]

Here's a first whack, proof-of-concept on how we could start to
decouple the platform_device from an omap_device.

The main RFC is in the last patch, and everything leading up to it are
misc. omap_device cleanups that make the last patch cleaner and
clearer.  It's really the last patch that does the decoupling.

This will be necessary if we're going to decouple the platform_device
creation from the omap_device/omap_hwmod creation etc.  This patch
leaves them both done in omap_device_build(), but shows that they can
be decoupled.


Kevin Hilman (7):
  OMAP: omap_device: replace debug/warning/error prints with dev_*
    macros
  OMAP3: beagle: don't touch omap_device internals
  OMAP: McBSP: use existing macros for converting between devices
  OMAP: omap_device: remove internal functions from omap_device.h
  OMAP: omap_device: when building return platform_device instead of
    omap_device
  OMAP: omap_device: device register functions now take platform_device
    pointer
  WIP: HACK/RFC: omap_device: begin to decouple platform_device from
    omap_device

 arch/arm/mach-omap2/board-omap3beagle.c       |   23 ++--
 arch/arm/mach-omap2/devices.c                 |   44 ++++----
 arch/arm/mach-omap2/display.c                 |    6 +-
 arch/arm/mach-omap2/dma.c                     |   16 ++--
 arch/arm/mach-omap2/gpio.c                    |    8 +-
 arch/arm/mach-omap2/hsmmc.c                   |    8 +-
 arch/arm/mach-omap2/hwspinlock.c              |    8 +-
 arch/arm/mach-omap2/mcbsp.c                   |    8 +-
 arch/arm/mach-omap2/opp.c                     |    2 +-
 arch/arm/mach-omap2/pm.c                      |    8 +-
 arch/arm/mach-omap2/serial.c                  |   12 +-
 arch/arm/plat-omap/i2c.c                      |    8 +-
 arch/arm/plat-omap/include/plat/omap_device.h |   14 +--
 arch/arm/plat-omap/mcbsp.c                    |    6 +-
 arch/arm/plat-omap/omap_device.c              |  165 ++++++++++++++-----------
 drivers/base/dd.c                             |    2 +-
 16 files changed, 173 insertions(+), 165 deletions(-)

-- 
1.7.6

[PATCH] OMAP: omap_device: replace _find_by_pdev() with to_omap_device()

[Kevin Hilman]

The omap_device layer currently has two ways of getting an omap_device
pointer from a platform_device pointer.

Replace current usage of _find_by_pdev() with to_omap_device() since
to_omap_device() is more familiar to the existing to_platform_device()
used when getting a platform_device pointer from a struct device pointer.

Cc: Felipe Balbi <balbi@ti.com>
Signed-off-by: Kevin Hilman <khilman@ti.com>
---
Paul, please consider this minor cleanup for the v3.1 merge window.
Applies to v3.0-c6.

 arch/arm/plat-omap/omap_device.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 49fc0df..c8b9cd1 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -236,11 +236,6 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
 	return 0;
 }
 
-static inline struct omap_device *_find_by_pdev(struct platform_device *pdev)
-{
-	return container_of(pdev, struct omap_device, pdev);
-}
-
 /**
  * _add_optional_clock_clkdev - Add clkdev entry for hwmod optional clocks
  * @od: struct omap_device *od
@@ -316,7 +311,7 @@ u32 omap_device_get_context_loss_count(struct platform_device *pdev)
 	struct omap_device *od;
 	u32 ret = 0;
 
-	od = _find_by_pdev(pdev);
+	od = to_omap_device(pdev);
 
 	if (od->hwmods_cnt)
 		ret = omap_hwmod_get_context_loss_count(od->hwmods[0]);
@@ -611,7 +606,7 @@ int omap_device_enable(struct platform_device *pdev)
 	int ret;
 	struct omap_device *od;
 
-	od = _find_by_pdev(pdev);
+	od = to_omap_device(pdev);
 
 	if (od->_state == OMAP_DEVICE_STATE_ENABLED) {
 		WARN(1, "omap_device: %s.%d: %s() called from invalid state %d\n",
@@ -650,7 +645,7 @@ int omap_device_idle(struct platform_device *pdev)
 	int ret;
 	struct omap_device *od;
 
-	od = _find_by_pdev(pdev);
+	od = to_omap_device(pdev);
 
 	if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
 		WARN(1, "omap_device: %s.%d: %s() called from invalid state %d\n",
@@ -681,7 +676,7 @@ int omap_device_shutdown(struct platform_device *pdev)
 	int ret, i;
 	struct omap_device *od;
 
-	od = _find_by_pdev(pdev);
+	od = to_omap_device(pdev);
 
 	if (od->_state != OMAP_DEVICE_STATE_ENABLED &&
 	    od->_state != OMAP_DEVICE_STATE_IDLE) {
@@ -722,7 +717,7 @@ int omap_device_align_pm_lat(struct platform_device *pdev,
 	int ret = -EINVAL;
 	struct omap_device *od;
 
-	od = _find_by_pdev(pdev);
+	od = to_omap_device(pdev);
 
 	if (new_wakeup_lat_limit == od->dev_wakeup_lat)
 		return 0;
-- 
1.7.6

[PATCH] OMAP: omap_device: replace _find_by_pdev() with to_omap_device()

[Felipe Balbi]

On Thu, Jul 21, 2011 at 04:52:11PM -0700, Kevin Hilman wrote:
> The omap_device layer currently has two ways of getting an omap_device
> pointer from a platform_device pointer.
> 
> Replace current usage of _find_by_pdev() with to_omap_device() since
> to_omap_device() is more familiar to the existing to_platform_device()
> used when getting a platform_device pointer from a struct device pointer.
> 

FWIW:

you can use either Reported-by: Felipe Balbi <balbi@ti.com> or
Reviewed-by: Felipe Balbi <balbi@ti.com>

thanks for posting the patch Kevin.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110722/51abee0b/attachment.sig>

[RFC/PATCH 1/7] OMAP: omap_device: replace debug/warning/error prints with dev_* macros

[Kevin Hilman]

For consistency in kernel printk output for devices, use dev_dbg(),
dev_warn(), dev_err() instead of pr_debug(), pr_warning() and
pr_err(), some of which currently use direct access of name from
platform_device and others of which use dev_name().  Using the dev_*
versions uses the standard device naming from the driver core.

Some pr_* prints were not converted with this patch since they are
used before the platform_device and struct device are created so
neither the dev_* prints or dev_name() is valid.

Reported-by: Russell King <rmk+kernel@arm.linux.org.uk>
Reviewed-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/plat-omap/omap_device.c |   85 +++++++++++++++++++-------------------
 1 files changed, 42 insertions(+), 43 deletions(-)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index be45147..7d83301 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -114,7 +114,7 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat)
 {
 	struct timespec a, b, c;
 
-	pr_debug("omap_device: %s: activating\n", od->pdev.name);
+	dev_dbg(&od->pdev.dev, "omap_device: activating\n");
 
 	while (od->pm_lat_level > 0) {
 		struct omap_device_pm_latency *odpl;
@@ -138,25 +138,24 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat)
 		c = timespec_sub(b, a);
 		act_lat = timespec_to_ns(&c);
 
-		pr_debug("omap_device: %s: pm_lat %d: activate: elapsed time "
-			 "%llu nsec\n", od->pdev.name, od->pm_lat_level,
-			 act_lat);
+		dev_dbg(&od->pdev.dev,
+			"omap_device: pm_lat %d: activate: elapsed time "
+			"%llu nsec\n", od->pm_lat_level, act_lat);
 
 		if (act_lat > odpl->activate_lat) {
 			odpl->activate_lat_worst = act_lat;
 			if (odpl->flags & OMAP_DEVICE_LATENCY_AUTO_ADJUST) {
 				odpl->activate_lat = act_lat;
-				pr_warning("omap_device: %s.%d: new worst case "
-					   "activate latency %d: %llu\n",
-					   od->pdev.name, od->pdev.id,
-					   od->pm_lat_level, act_lat);
+				dev_warn(&od->pdev.dev,
+					 "new worst case activate latency "
+					 "%d: %llu\n",
+					 od->pm_lat_level, act_lat);
 			} else
-				pr_warning("omap_device: %s.%d: activate "
-					   "latency %d higher than exptected. "
-					   "(%llu > %d)\n",
-					   od->pdev.name, od->pdev.id,
-					   od->pm_lat_level, act_lat,
-					   odpl->activate_lat);
+				dev_warn(&od->pdev.dev,
+					 "activate latency %d "
+					 "higher than exptected. (%llu > %d)\n",
+					 od->pm_lat_level, act_lat,
+					 odpl->activate_lat);
 		}
 
 		od->dev_wakeup_lat -= odpl->activate_lat;
@@ -183,7 +182,7 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
 {
 	struct timespec a, b, c;
 
-	pr_debug("omap_device: %s: deactivating\n", od->pdev.name);
+	dev_dbg(&od->pdev.dev, "omap_device: deactivating\n");
 
 	while (od->pm_lat_level < od->pm_lats_cnt) {
 		struct omap_device_pm_latency *odpl;
@@ -206,28 +205,26 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
 		c = timespec_sub(b, a);
 		deact_lat = timespec_to_ns(&c);
 
-		pr_debug("omap_device: %s: pm_lat %d: deactivate: elapsed time "
-			 "%llu nsec\n", od->pdev.name, od->pm_lat_level,
-			 deact_lat);
+		dev_dbg(&od->pdev.dev,
+			"omap_device: pm_lat %d: deactivate: elapsed time "
+			"%llu nsec\n", od->pm_lat_level, deact_lat);
 
 		if (deact_lat > odpl->deactivate_lat) {
 			odpl->deactivate_lat_worst = deact_lat;
 			if (odpl->flags & OMAP_DEVICE_LATENCY_AUTO_ADJUST) {
 				odpl->deactivate_lat = deact_lat;
-				pr_warning("omap_device: %s.%d: new worst case "
-					   "deactivate latency %d: %llu\n",
-					   od->pdev.name, od->pdev.id,
-					   od->pm_lat_level, deact_lat);
+				dev_warn(&od->pdev.dev,
+					 "new worst case deactivate latency "
+					 "%d: %llu\n",
+					 od->pm_lat_level, deact_lat);
 			} else
-				pr_warning("omap_device: %s.%d: deactivate "
-					   "latency %d higher than exptected. "
-					   "(%llu > %d)\n",
-					   od->pdev.name, od->pdev.id,
-					   od->pm_lat_level, deact_lat,
-					   odpl->deactivate_lat);
+				dev_warn(&od->pdev.dev,
+					 "deactivate latency %d "
+					 "higher than exptected. (%llu > %d)\n",
+					 od->pm_lat_level, deact_lat,
+					 odpl->deactivate_lat);
 		}
 
-
 		od->dev_wakeup_lat += odpl->activate_lat;
 
 		od->pm_lat_level++;
@@ -245,28 +242,27 @@ static void _add_clkdev(struct omap_device *od, const char *clk_alias,
 	if (!clk_alias || !clk_name)
 		return;
 
-	pr_debug("omap_device: %s: Creating %s -> %s\n",
-		 dev_name(&od->pdev.dev), clk_alias, clk_name);
+	dev_dbg(&od->pdev.dev, "Creating %s -> %s\n", clk_alias, clk_name);
 
 	r = clk_get_sys(dev_name(&od->pdev.dev), clk_alias);
 	if (!IS_ERR(r)) {
-		pr_warning("omap_device: %s: alias %s already exists\n",
-			   dev_name(&od->pdev.dev), clk_alias);
+		dev_warn(&od->pdev.dev,
+			 "alias %s already exists\n", clk_alias);
 		clk_put(r);
 		return;
 	}
 
 	r = omap_clk_get_by_name(clk_name);
 	if (IS_ERR(r)) {
-		pr_err("omap_device: %s: omap_clk_get_by_name for %s failed\n",
-		       dev_name(&od->pdev.dev), clk_name);
+		dev_err(&od->pdev.dev,
+			"omap_clk_get_by_name for %s failed\n", clk_name);
 		return;
 	}
 
 	l = clkdev_alloc(r, clk_alias, dev_name(&od->pdev.dev));
 	if (!l) {
-		pr_err("omap_device: %s: clkdev_alloc for %s failed\n",
-		       dev_name(&od->pdev.dev), clk_alias);
+		dev_err(&od->pdev.dev,
+			"clkdev_alloc for %s failed\n", clk_alias);
 		return;
 	}
 
@@ -624,8 +620,9 @@ int omap_device_enable(struct platform_device *pdev)
 	od = to_omap_device(pdev);
 
 	if (od->_state == OMAP_DEVICE_STATE_ENABLED) {
-		WARN(1, "omap_device: %s.%d: %s() called from invalid state %d\n",
-		     od->pdev.name, od->pdev.id, __func__, od->_state);
+		dev_warn(&pdev->dev,
+			 "omap_device: %s() called from invalid state %d\n",
+			 __func__, od->_state);
 		return -EINVAL;
 	}
 
@@ -663,8 +660,9 @@ int omap_device_idle(struct platform_device *pdev)
 	od = to_omap_device(pdev);
 
 	if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
-		WARN(1, "omap_device: %s.%d: %s() called from invalid state %d\n",
-		     od->pdev.name, od->pdev.id, __func__, od->_state);
+		dev_warn(&pdev->dev, 
+			 "omap_device: %s() called from invalid state %d\n",
+			 __func__, od->_state);
 		return -EINVAL;
 	}
 
@@ -695,8 +693,9 @@ int omap_device_shutdown(struct platform_device *pdev)
 
 	if (od->_state != OMAP_DEVICE_STATE_ENABLED &&
 	    od->_state != OMAP_DEVICE_STATE_IDLE) {
-		WARN(1, "omap_device: %s.%d: %s() called from invalid state %d\n",
-		     od->pdev.name, od->pdev.id, __func__, od->_state);
+		dev_warn(&pdev->dev,
+			 "omap_device: %s() called from invalid state %d\n",
+			 __func__, od->_state);
 		return -EINVAL;
 	}
 
-- 
1.7.6

[RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals

[Kevin Hilman]

Board code should not touch omap_device internals.  To get the MPU/IVA devices,
use existing APIs: omap2_get_mpu_device(), omap2_get_iva_device().

Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/board-omap3beagle.c |   23 ++++++++++-------------
 1 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
index 32f5f89..3ae16b4 100644
--- a/arch/arm/mach-omap2/board-omap3beagle.c
+++ b/arch/arm/mach-omap2/board-omap3beagle.c
@@ -491,23 +491,22 @@ static void __init beagle_opp_init(void)
 
 	/* Custom OPP enabled for all xM versions */
 	if (cpu_is_omap3630()) {
-		struct omap_hwmod *mh = omap_hwmod_lookup("mpu");
-		struct omap_hwmod *dh = omap_hwmod_lookup("iva");
-		struct device *dev;
+		struct device *mpu_dev, *iva_dev;
 
-		if (!mh || !dh) {
+		mpu_dev = omap2_get_mpuss_device();
+		iva_dev = omap2_get_iva_device();
+
+		if (!mpu_dev || !iva_dev) {
 			pr_err("%s: Aiee.. no mpu/dsp devices? %p %p\n",
-				__func__, mh, dh);
+				__func__, mpu_dev, iva_dev);
 			return;
 		}
 		/* Enable MPU 1GHz and lower opps */
-		dev = &mh->od->pdev.dev;
-		r = opp_enable(dev, 800000000);
+		r = opp_enable(mpu_dev, 800000000);
 		/* TODO: MPU 1GHz needs SR and ABB */
 
 		/* Enable IVA 800MHz and lower opps */
-		dev = &dh->od->pdev.dev;
-		r |= opp_enable(dev, 660000000);
+		r |= opp_enable(iva_dev, 660000000);
 		/* TODO: DSP 800MHz needs SR and ABB */
 		if (r) {
 			pr_err("%s: failed to enable higher opp %d\n",
@@ -516,10 +515,8 @@ static void __init beagle_opp_init(void)
 			 * Cleanup - disable the higher freqs - we dont care
 			 * about the results
 			 */
-			dev = &mh->od->pdev.dev;
-			opp_disable(dev, 800000000);
-			dev = &dh->od->pdev.dev;
-			opp_disable(dev, 660000000);
+			opp_disable(mpu_dev, 800000000);
+			opp_disable(iva_dev, 660000000);
 		}
 	}
 	return;
-- 
1.7.6

[RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals

[Felipe Balbi]

Hi,

On Thu, Jul 21, 2011 at 04:52:13PM -0700, Kevin Hilman wrote:
> Board code should not touch omap_device internals.  To get the MPU/IVA devices,
> use existing APIs: omap2_get_mpu_device(), omap2_get_iva_device().
> 
> Signed-off-by: Kevin Hilman <khilman@ti.com>
> ---
>  arch/arm/mach-omap2/board-omap3beagle.c |   23 ++++++++++-------------
>  1 files changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
> index 32f5f89..3ae16b4 100644
> --- a/arch/arm/mach-omap2/board-omap3beagle.c
> +++ b/arch/arm/mach-omap2/board-omap3beagle.c
> @@ -491,23 +491,22 @@ static void __init beagle_opp_init(void)
>  
>  	/* Custom OPP enabled for all xM versions */
>  	if (cpu_is_omap3630()) {
> -		struct omap_hwmod *mh = omap_hwmod_lookup("mpu");
> -		struct omap_hwmod *dh = omap_hwmod_lookup("iva");
> -		struct device *dev;
> +		struct device *mpu_dev, *iva_dev;
>  
> -		if (!mh || !dh) {
> +		mpu_dev = omap2_get_mpuss_device();
> +		iva_dev = omap2_get_iva_device();

out of curiosity again, nothing to do with this patch.

Maybe it would be nicer to have an api such as:

omap2_get_device(name);

there are already four devices to be gotten, if that number grows any
bigger, so will the number of helper functions.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110722/139764a3/attachment.sig>

[RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals

[Nishanth Menon]

On 11:57-20110722, Felipe Balbi wrote:
[...]
> >  	/* Custom OPP enabled for all xM versions */
> >  	if (cpu_is_omap3630()) {
> > -		struct omap_hwmod *mh = omap_hwmod_lookup("mpu");
> > -		struct omap_hwmod *dh = omap_hwmod_lookup("iva");
> > -		struct device *dev;
> > +		struct device *mpu_dev, *iva_dev;
> >  
> > -		if (!mh || !dh) {
> > +		mpu_dev = omap2_get_mpuss_device();
> > +		iva_dev = omap2_get_iva_device();
> 
> out of curiosity again, nothing to do with this patch.
> 
> Maybe it would be nicer to have an api such as:
> 
> omap2_get_device(name);
> 
> there are already four devices to be gotten, if that number grows any
> bigger, so will the number of helper functions.
I agree, in fact, on a different topic, I hit the same requirement
here is the patch I had done:

[RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals

[Russell King - ARM Linux]

On Thu, Jul 28, 2011 at 12:53:47AM -0500, Nishanth Menon wrote:
> +struct device *omap_hwmod_to_device(const char *oh_name)
> +{
> +	struct omap_hwmod *oh;
> +
> +	if (!oh_name) {
> +		WARN(1, "%s: no hwmod name!\n", __func__);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	oh = _lookup(oh_name);
> +	if (IS_ERR_OR_NULL(oh)) {
> +		WARN(1, "%s: no hwmod for %s\n", __func__,
> +			oh_name);
> +		return ERR_PTR(-ENODEV);

It is good practice to always propagate back the error codes from functions
which failed.  So you may need something like:

		return ERR_PTR(oh ? PTR_ERR(oh) : -ENODEV);

here.

> +	}
> +	if (IS_ERR_OR_NULL(oh->od)) {
> +		WARN(1, "%s: no omap_device for %s\n", __func__,
> +			oh_name);
> +		return ERR_PTR(-ENODEV);

Same here.

[RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals

[Cousson, Benoit]

Hi Nishanth,

On 7/28/2011 7:53 AM, Menon, Nishanth wrote:
> On 11:57-20110722, Felipe Balbi wrote:
> [...]
>>>   	/* Custom OPP enabled for all xM versions */
>>>   	if (cpu_is_omap3630()) {
>>> -		struct omap_hwmod *mh = omap_hwmod_lookup("mpu");
>>> -		struct omap_hwmod *dh = omap_hwmod_lookup("iva");
>>> -		struct device *dev;
>>> +		struct device *mpu_dev, *iva_dev;
>>>
>>> -		if (!mh || !dh) {
>>> +		mpu_dev = omap2_get_mpuss_device();
>>> +		iva_dev = omap2_get_iva_device();
>>
>> out of curiosity again, nothing to do with this patch.
>>
>> Maybe it would be nicer to have an api such as:
>>
>> omap2_get_device(name);
>>
>> there are already four devices to be gotten, if that number grows any
>> bigger, so will the number of helper functions.
> I agree, in fact, on a different topic, I hit the same requirement
> here is the patch I had done:
>  From 9f226def811bd50e4bac02f427604034cef77706 Mon Sep 17 00:00:00 2001
> From: Nishanth Menon<nm@ti.com>
> Date: Wed, 27 Jul 2011 15:02:32 -0500
> Subject: [PATCH] OMAP: hwmod: add omap_hwmod_to_device
>
> omap_hwmod_to_device is useful for drivers when they need to
> look up the device associated with a hwmod name to map back
> into the device structure pointers. These ideally should
> be used by drivers in mach directory. This could in effect
> replace apis such as omap2_get_mpuss_device,
> omap2_get_iva_device, omap2_get_l3_device, omap4_get_dsp_device
> etc..
>
> Signed-off-by: Nishanth Menon<nm@ti.com>
> ---
>   arch/arm/mach-omap2/omap_hwmod.c             |   33 ++++++++++++++++++++++++++
>   arch/arm/plat-omap/include/plat/omap_hwmod.h |    2 +
>   2 files changed, 35 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 293fa6c..77d01a2 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -142,6 +142,7 @@
>   #include "powerdomain.h"
>   #include<plat/clock.h>
>   #include<plat/omap_hwmod.h>
> +#include<plat/omap_device.h>

I'd rather put that code inside the omap_device.c instead of here.
The omap_device layer is on top of the omap_hwmod.
In order to minimize the dependencies from the low HW description layer 
to the omap_device layer, you should maybe define a 
omap_device_from_hwmod() function or something similar.

That being said, do we really need to get the device from the hwmod 
name? Cannot we use the device name instead?
I do not know all the usecases, that why I'm asking.

Regards,
Benoit

[RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals

[Felipe Balbi]

Hi,

On Thu, Jul 28, 2011 at 02:57:03PM +0200, Cousson, Benoit wrote:
> Hi Nishanth,
> 
> On 7/28/2011 7:53 AM, Menon, Nishanth wrote:
> >On 11:57-20110722, Felipe Balbi wrote:
> >[...]
> >>>  	/* Custom OPP enabled for all xM versions */
> >>>  	if (cpu_is_omap3630()) {
> >>>-		struct omap_hwmod *mh = omap_hwmod_lookup("mpu");
> >>>-		struct omap_hwmod *dh = omap_hwmod_lookup("iva");
> >>>-		struct device *dev;
> >>>+		struct device *mpu_dev, *iva_dev;
> >>>
> >>>-		if (!mh || !dh) {
> >>>+		mpu_dev = omap2_get_mpuss_device();
> >>>+		iva_dev = omap2_get_iva_device();
> >>
> >>out of curiosity again, nothing to do with this patch.
> >>
> >>Maybe it would be nicer to have an api such as:
> >>
> >>omap2_get_device(name);
> >>
> >>there are already four devices to be gotten, if that number grows any
> >>bigger, so will the number of helper functions.
> >I agree, in fact, on a different topic, I hit the same requirement
> >here is the patch I had done:
> > From 9f226def811bd50e4bac02f427604034cef77706 Mon Sep 17 00:00:00 2001
> >From: Nishanth Menon<nm@ti.com>
> >Date: Wed, 27 Jul 2011 15:02:32 -0500
> >Subject: [PATCH] OMAP: hwmod: add omap_hwmod_to_device
> >
> >omap_hwmod_to_device is useful for drivers when they need to
> >look up the device associated with a hwmod name to map back
> >into the device structure pointers. These ideally should
> >be used by drivers in mach directory. This could in effect
> >replace apis such as omap2_get_mpuss_device,
> >omap2_get_iva_device, omap2_get_l3_device, omap4_get_dsp_device
> >etc..
> >
> >Signed-off-by: Nishanth Menon<nm@ti.com>
> >---
> >  arch/arm/mach-omap2/omap_hwmod.c             |   33 ++++++++++++++++++++++++++
> >  arch/arm/plat-omap/include/plat/omap_hwmod.h |    2 +
> >  2 files changed, 35 insertions(+), 0 deletions(-)
> >
> >diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> >index 293fa6c..77d01a2 100644
> >--- a/arch/arm/mach-omap2/omap_hwmod.c
> >+++ b/arch/arm/mach-omap2/omap_hwmod.c
> >@@ -142,6 +142,7 @@
> >  #include "powerdomain.h"
> >  #include<plat/clock.h>
> >  #include<plat/omap_hwmod.h>
> >+#include<plat/omap_device.h>
> 
> I'd rather put that code inside the omap_device.c instead of here.
> The omap_device layer is on top of the omap_hwmod.
> In order to minimize the dependencies from the low HW description
> layer to the omap_device layer, you should maybe define a
> omap_device_from_hwmod() function or something similar.
> 
> That being said, do we really need to get the device from the hwmod
> name? Cannot we use the device name instead?
> I do not know all the usecases, that why I'm asking.

that's a good question, I only suggested the above given the fact that
we already have four functions to grab four different devices. It was
only a way to combine all of those with a simple argument.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110728/3cec6a79/attachment.sig>

[RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals

[Menon, Nishanth]

On Thu, Jul 28, 2011 at 07:57, Cousson, Benoit <b-cousson@ti.com> wrote:
[...]
>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c
>> b/arch/arm/mach-omap2/omap_hwmod.c
>> index 293fa6c..77d01a2 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>> @@ -142,6 +142,7 @@
>> ?#include "powerdomain.h"
>> ?#include<plat/clock.h>
>> ?#include<plat/omap_hwmod.h>
>> +#include<plat/omap_device.h>
>
> I'd rather put that code inside the omap_device.c instead of here.
> The omap_device layer is on top of the omap_hwmod.
> In order to minimize the dependencies from the low HW description layer to
> the omap_device layer, you should maybe define a omap_device_from_hwmod()
> function or something similar.
Thanks for the review. will check on this.

>
> That being said, do we really need to get the device from the hwmod name?
> Cannot we use the device name instead?
> I do not know all the usecases, that why I'm asking.
mpu.0 , are the device names - which probably lets me walk the kernel
data structrues instead of omap database to get to the right device,
Vs using the common names like "mpu" " make things a little easier to
deal with from driver perspective.

as an example, some of the dev_driver_string(dev):dev_name(dev) (in
bracket hwmod name) I collected from OMAP4 are:
platform:mpu.0 ("mpu")
platform:l3_main_1.0 ('l3_main_1")
pvrsrvkm:pvrsrvkm.0 ("gpu")
rpres:fdif.0 ("fdif")
omap_hsi:omap_hsi.0 ("hsi")
platform:iss.0 ("iss")
etc..

I mean I have'nt been keeping track of the device tree discussions so
dont know if this function could be better done - but I think I agree
with the overall idea that instead of spawning off get_xyz_device() we
need to have some uniform approach to get to the device scaling
silicon - I hoped we could consider the hwmod database/what ever
replacing it to do that.

Regards,
Nishanth Menon

[RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals

[Nishanth Menon]

On 08:31-20110728, Menon, Nishanth wrote:
> On Thu, Jul 28, 2011 at 07:57, Cousson, Benoit <b-cousson@ti.com> wrote:
> [...]
> >> diff --git a/arch/arm/mach-omap2/omap_hwmod.c
> >> b/arch/arm/mach-omap2/omap_hwmod.c
> >> index 293fa6c..77d01a2 100644
> >> --- a/arch/arm/mach-omap2/omap_hwmod.c
> >> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> >> @@ -142,6 +142,7 @@
> >> ?#include "powerdomain.h"
> >> ?#include<plat/clock.h>
> >> ?#include<plat/omap_hwmod.h>
> >> +#include<plat/omap_device.h>
> >
> > I'd rather put that code inside the omap_device.c instead of here.
> > The omap_device layer is on top of the omap_hwmod.
> > In order to minimize the dependencies from the low HW description layer to
> > the omap_device layer, you should maybe define a omap_device_from_hwmod()
> > function or something similar.
> Thanks for the review. will check on this.
> 
> >
> > That being said, do we really need to get the device from the hwmod name?
> > Cannot we use the device name instead?
> > I do not know all the usecases, that why I'm asking.
> mpu.0 , are the device names - which probably lets me walk the kernel
> data structrues instead of omap database to get to the right device,
> Vs using the common names like "mpu" " make things a little easier to
> deal with from driver perspective.
> 
> as an example, some of the dev_driver_string(dev):dev_name(dev) (in
> bracket hwmod name) I collected from OMAP4 are:
> platform:mpu.0 ("mpu")
> platform:l3_main_1.0 ('l3_main_1")
> pvrsrvkm:pvrsrvkm.0 ("gpu")
> rpres:fdif.0 ("fdif")
> omap_hsi:omap_hsi.0 ("hsi")
> platform:iss.0 ("iss")
> etc..
> 
> I mean I have'nt been keeping track of the device tree discussions so
> dont know if this function could be better done - but I think I agree
> with the overall idea that instead of spawning off get_xyz_device() we
> need to have some uniform approach to get to the device scaling
> silicon - I hoped we could consider the hwmod database/what ever
> replacing it to do that.
following are a couple of reference patches how this could be done
with omap_device

[RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals

[Felipe Balbi]

hi,

On Fri, Jul 29, 2011 at 08:49:34AM -0500, Nishanth Menon wrote:
> From f03490456e24f72ca5272303c95a6f0b212494d5 Mon Sep 17 00:00:00 2001
> From: Nishanth Menon <nm@ti.com>
> Date: Wed, 27 Jul 2011 15:02:32 -0500
> Subject: [PATCH 1/2] OMAP: PM: omap_device: add omap_hwmod_name_get_odev
> 
> An API which translates a standard hwmod name to corresponding
> omap_device is useful for drivers when they need to look up the
> device associated with a hwmod name to map back into the device
> structure pointers. These ideally should be used by drivers in
> mach directory. Using a generic hwmod name like "gpu" instead of
> the actual device name which could change in the future, allows
> us to:
> a) Could in effect help replace apis such as omap2_get_mpuss_device,
> omap2_get_iva_device, omap2_get_l3_device, omap4_get_dsp_device,
> etc..
> b) Scale to more devices rather than be restricted to named functions
> c) Simplify driver's platform_data from passing additional fields
> all doing the same thing with different function pointer names
> just for accessing a different device name.
> 
> Change-Id: Ib42d22b4a929982c87a79866e3d7dc6e41073a6c
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  arch/arm/plat-omap/include/plat/omap_device.h |    1 +
>  arch/arm/plat-omap/omap_device.c              |   32 +++++++++++++++++++++++++
>  2 files changed, 33 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
> index 70d31d0..7a3c046 100644
> --- a/arch/arm/plat-omap/include/plat/omap_device.h
> +++ b/arch/arm/plat-omap/include/plat/omap_device.h
> @@ -102,6 +102,7 @@ int omap_device_register(struct omap_device *od);
>  int omap_early_device_register(struct omap_device *od);
>  
>  void __iomem *omap_device_get_rt_va(struct omap_device *od);
> +struct omap_device *omap_hwmod_name_get_odev(const char *oh_name);
>  
>  /* OMAP PM interface */
>  int omap_device_align_pm_lat(struct platform_device *pdev,
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index 92b4496..21df532 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -780,6 +780,38 @@ void __iomem *omap_device_get_rt_va(struct omap_device *od)
>  	return omap_hwmod_get_mpu_rt_va(od->hwmods[0]);
>  }
>  
> +/**
> + * omap_hwmod_name_get_odev() - convert a hwmod name to omap_device pointer
> + * @oh_name: name of the hwmod device
> + *
> + * returns back a struct omap_device * pointer associated with a hwmod
> + * device represented by a hwmod_name
> + */
> +struct omap_device *omap_hwmod_name_get_odev(const char *oh_name)
> +{
> +	struct omap_hwmod *oh;
> +
> +	if (!oh_name) {
> +		WARN(1, "%s: no hwmod name!\n", __func__);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	oh = omap_hwmod_lookup(oh_name);
> +	if (IS_ERR_OR_NULL(oh)) {
> +		WARN(1, "%s: no hwmod for %s\n", __func__,
> +			oh_name);
> +		return ERR_PTR(oh ? PTR_ERR(oh) : -ENODEV);
> +	}
> +	if (IS_ERR_OR_NULL(oh->od)) {
> +		WARN(1, "%s: no omap_device for %s\n", __func__,
> +			oh_name);
> +		return ERR_PTR(oh->od ? PTR_ERR(oh->od) : -ENODEV);
> +	}
> +
> +	return oh->od;
> +}
> +EXPORT_SYMBOL(omap_hwmod_name_get_odev);

maybe EXPORT_SYMBOL_GPL() ?? Not sure we want non-GPL code to access
this ;-)

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110729/627d1615/attachment-0001.sig>

[RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals

[Menon, Nishanth]

On Fri, Jul 29, 2011 at 09:05, Felipe Balbi <balbi@ti.com> wrote:
>
>> +}
>> +EXPORT_SYMBOL(omap_hwmod_name_get_odev);
>
> maybe EXPORT_SYMBOL_GPL() ?? Not sure we want non-GPL code to access
> this ;-)

Sure.. but is this the way we want to go? if yes, I can post the
series in a formal way to the list.

Regards,
Nishanth Menon

[RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals

[Felipe Balbi]

Hi,

On Fri, Jul 29, 2011 at 06:07:17PM -0500, Menon, Nishanth wrote:
> On Fri, Jul 29, 2011 at 09:05, Felipe Balbi <balbi@ti.com> wrote:
> >
> >> +}
> >> +EXPORT_SYMBOL(omap_hwmod_name_get_odev);
> >
> > maybe EXPORT_SYMBOL_GPL() ?? Not sure we want non-GPL code to access
> > this ;-)
> 
> Sure.. but is this the way we want to go? if yes, I can post the
> series in a formal way to the list.

I would expect so, but if there are concerns of other non-GPL drivers
needing this, you can at least use:

EXPORT_SYMBOL_GPL_FUTURE();

this would allow you to catch non-GPL drivers accessing this symbol.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110801/09caadd1/attachment-0001.sig>

[RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals

[Jean Pihet]

Hi,

On Fri, Jul 22, 2011 at 10:57 AM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Thu, Jul 21, 2011 at 04:52:13PM -0700, Kevin Hilman wrote:
>> Board code should not touch omap_device internals. ?To get the MPU/IVA devices,
>> use existing APIs: omap2_get_mpu_device(), omap2_get_iva_device().
>>
>> Signed-off-by: Kevin Hilman <khilman@ti.com>
>> ---
>> ?arch/arm/mach-omap2/board-omap3beagle.c | ? 23 ++++++++++-------------
>> ?1 files changed, 10 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
>> index 32f5f89..3ae16b4 100644
>> --- a/arch/arm/mach-omap2/board-omap3beagle.c
>> +++ b/arch/arm/mach-omap2/board-omap3beagle.c
>> @@ -491,23 +491,22 @@ static void __init beagle_opp_init(void)
>>
>> ? ? ? /* Custom OPP enabled for all xM versions */
>> ? ? ? if (cpu_is_omap3630()) {
>> - ? ? ? ? ? ? struct omap_hwmod *mh = omap_hwmod_lookup("mpu");
>> - ? ? ? ? ? ? struct omap_hwmod *dh = omap_hwmod_lookup("iva");
>> - ? ? ? ? ? ? struct device *dev;
>> + ? ? ? ? ? ? struct device *mpu_dev, *iva_dev;
>>
>> - ? ? ? ? ? ? if (!mh || !dh) {
>> + ? ? ? ? ? ? mpu_dev = omap2_get_mpuss_device();
>> + ? ? ? ? ? ? iva_dev = omap2_get_iva_device();
>
> out of curiosity again, nothing to do with this patch.
>
> Maybe it would be nicer to have an api such as:
>
> omap2_get_device(name);
>
> there are already four devices to be gotten, if that number grows any
> bigger, so will the number of helper functions.

I agree with this.
The API is also helpful for the PM QoS devices constraints API, which
requires the pointers to devices structs. At the present time only a
few devices can be used and it would be good to have a generic
implementation.

Regards,
Jean

>
> --
> balbi
>

[RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals

[Jean Pihet]

Hi,

On Fri, Jul 22, 2011 at 10:57 AM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Thu, Jul 21, 2011 at 04:52:13PM -0700, Kevin Hilman wrote:
>> Board code should not touch omap_device internals. ?To get the MPU/IVA devices,
>> use existing APIs: omap2_get_mpu_device(), omap2_get_iva_device().
>>
>> Signed-off-by: Kevin Hilman <khilman@ti.com>
>> ---
>> ?arch/arm/mach-omap2/board-omap3beagle.c | ? 23 ++++++++++-------------
>> ?1 files changed, 10 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
>> index 32f5f89..3ae16b4 100644
>> --- a/arch/arm/mach-omap2/board-omap3beagle.c
>> +++ b/arch/arm/mach-omap2/board-omap3beagle.c
>> @@ -491,23 +491,22 @@ static void __init beagle_opp_init(void)
>>
>> ? ? ? /* Custom OPP enabled for all xM versions */
>> ? ? ? if (cpu_is_omap3630()) {
>> - ? ? ? ? ? ? struct omap_hwmod *mh = omap_hwmod_lookup("mpu");
>> - ? ? ? ? ? ? struct omap_hwmod *dh = omap_hwmod_lookup("iva");
>> - ? ? ? ? ? ? struct device *dev;
>> + ? ? ? ? ? ? struct device *mpu_dev, *iva_dev;
>>
>> - ? ? ? ? ? ? if (!mh || !dh) {
>> + ? ? ? ? ? ? mpu_dev = omap2_get_mpuss_device();
>> + ? ? ? ? ? ? iva_dev = omap2_get_iva_device();
>
> out of curiosity again, nothing to do with this patch.
>
> Maybe it would be nicer to have an api such as:
>
> omap2_get_device(name);
>
> there are already four devices to be gotten, if that number grows any
> bigger, so will the number of helper functions.

I agree with this.
The API is also helpful for the PM QoS devices constraints API, which
requires the pointers to devices structs. At the present time only a
few devices can be used and it would be good to have a generic
implementation.

Regards,
Jean

>
> --
> balbi
>

[RFC/PATCH 3/7] OMAP: McBSP: use existing macros for converting between devices

[Kevin Hilman]

For converting from struct device to platform_device, and from
platform_device to struct device, there are existing macros.  Use
them instead of manual use of container_of().

Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/plat-omap/mcbsp.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index 6c62af1..02199e8 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -260,9 +260,9 @@ EXPORT_SYMBOL(omap_mcbsp_dma_reg_params);
 #ifdef CONFIG_ARCH_OMAP3
 static struct omap_device *find_omap_device_by_dev(struct device *dev)
 {
-	struct platform_device *pdev = container_of(dev,
-					struct platform_device, dev);
-	return container_of(pdev, struct omap_device, pdev);
+	struct platform_device *pdev = to_platform_device(dev);
+
+	return to_omap_device(pdev);
 }
 
 static void omap_st_on(struct omap_mcbsp *mcbsp)
-- 
1.7.6

[RFC/PATCH 3/7] OMAP: McBSP: use existing macros for converting between devices

[Felipe Balbi]

On Thu, Jul 21, 2011 at 04:52:14PM -0700, Kevin Hilman wrote:
> For converting from struct device to platform_device, and from
> platform_device to struct device, there are existing macros.  Use
> them instead of manual use of container_of().
> 
> Signed-off-by: Kevin Hilman <khilman@ti.com>
> ---
>  arch/arm/plat-omap/mcbsp.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
> index 6c62af1..02199e8 100644
> --- a/arch/arm/plat-omap/mcbsp.c
> +++ b/arch/arm/plat-omap/mcbsp.c
> @@ -260,9 +260,9 @@ EXPORT_SYMBOL(omap_mcbsp_dma_reg_params);
>  #ifdef CONFIG_ARCH_OMAP3
>  static struct omap_device *find_omap_device_by_dev(struct device *dev)
>  {
> -	struct platform_device *pdev = container_of(dev,
> -					struct platform_device, dev);
> -	return container_of(pdev, struct omap_device, pdev);
> +	struct platform_device *pdev = to_platform_device(dev);
> +
> +	return to_omap_device(pdev);

could simplify to:

	return to_omap_device(to_platform_device(dev));

but it won't change much. Only decrease readability, probably.

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

[RFC/PATCH 3/7] OMAP: McBSP: use existing macros for converting between devices

[Sergei Shtylyov]

Hello.

On 22-07-2011 3:52, Kevin Hilman wrote:

> For converting from struct device to platform_device, and from
> platform_device to struct device,

    You mean from platform_device to omap_device?

> there are existing macros.  Use
> them instead of manual use of container_of().

> Signed-off-by: Kevin Hilman<khilman@ti.com>
> ---
>   arch/arm/plat-omap/mcbsp.c |    6 +++---
>   1 files changed, 3 insertions(+), 3 deletions(-)

> diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
> index 6c62af1..02199e8 100644
> --- a/arch/arm/plat-omap/mcbsp.c
> +++ b/arch/arm/plat-omap/mcbsp.c
> @@ -260,9 +260,9 @@ EXPORT_SYMBOL(omap_mcbsp_dma_reg_params);
>   #ifdef CONFIG_ARCH_OMAP3
>   static struct omap_device *find_omap_device_by_dev(struct device *dev)
>   {
> -	struct platform_device *pdev = container_of(dev,
> -					struct platform_device, dev);
> -	return container_of(pdev, struct omap_device, pdev);
> +	struct platform_device *pdev = to_platform_device(dev);
> +
> +	return to_omap_device(pdev);
>   }
>
>   static void omap_st_on(struct omap_mcbsp *mcbsp)

WBR, Sergei

[RFC/PATCH 3/7] OMAP: McBSP: use existing macros for converting between devices

[Kevin Hilman]

Hi Sergei,

Sergei Shtylyov <sshtylyov@mvista.com> writes:

> On 22-07-2011 3:52, Kevin Hilman wrote:
>
>> For converting from struct device to platform_device, and from
>> platform_device to struct device,
>
>    You mean from platform_device to omap_device?
>

Yes indeed, thanks.  Updated patch below (only changelog changed)

Kevin

[RFC/PATCH 4/7] OMAP: omap_device: remove internal functions from omap_device.h

[Kevin Hilman]

The *_device_register() functions and the count/fill resources functions
are internal to omap_device and do not need to be in the header.

Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/plat-omap/include/plat/omap_device.h |    6 ------
 arch/arm/plat-omap/omap_device.c              |   12 ++++++++----
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
index e4c349f..0a53a0a 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -83,9 +83,6 @@ int omap_device_shutdown(struct platform_device *pdev);
 
 /* Core code interface */
 
-int omap_device_count_resources(struct omap_device *od);
-int omap_device_fill_resources(struct omap_device *od, struct resource *res);
-
 struct omap_device *omap_device_build(const char *pdev_name, int pdev_id,
 				      struct omap_hwmod *oh, void *pdata,
 				      int pdata_len,
@@ -98,9 +95,6 @@ struct omap_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
 					 struct omap_device_pm_latency *pm_lats,
 					 int pm_lats_cnt, int is_early_device);
 
-int omap_device_register(struct omap_device *od);
-int omap_early_device_register(struct omap_device *od);
-
 void __iomem *omap_device_get_rt_va(struct omap_device *od);
 
 /* OMAP PM interface */
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 7d83301..4e4b36c 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -94,6 +94,9 @@
 #define USE_WAKEUP_LAT			0
 #define IGNORE_WAKEUP_LAT		1
 
+static int omap_device_register(struct omap_device *od);
+static int omap_early_device_register(struct omap_device *od);
+
 /* Private functions */
 
 /**
@@ -339,7 +342,7 @@ u32 omap_device_get_context_loss_count(struct platform_device *pdev)
  * much memory to allocate before calling
  * omap_device_fill_resources().  Returns the count.
  */
-int omap_device_count_resources(struct omap_device *od)
+static int omap_device_count_resources(struct omap_device *od)
 {
 	int c = 0;
 	int i;
@@ -370,7 +373,8 @@ int omap_device_count_resources(struct omap_device *od)
  * functions to get device resources.  Hacking around the existing
  * platform_device code wastes memory.  Returns 0.
  */
-int omap_device_fill_resources(struct omap_device *od, struct resource *res)
+static int omap_device_fill_resources(struct omap_device *od,
+				      struct resource *res)
 {
 	int c = 0;
 	int i, r;
@@ -534,7 +538,7 @@ odbs_exit1:
  * platform_early_add_device() on the underlying platform_device.
  * Returns 0 by default.
  */
-int omap_early_device_register(struct omap_device *od)
+static int omap_early_device_register(struct omap_device *od)
 {
 	struct platform_device *devices[1];
 
@@ -587,7 +591,7 @@ static struct dev_power_domain omap_device_power_domain = {
  * platform_device_register() on the underlying platform_device.
  * Returns the return value of platform_device_register().
  */
-int omap_device_register(struct omap_device *od)
+static int omap_device_register(struct omap_device *od)
 {
 	pr_debug("omap_device: %s: registering\n", od->pdev.name);
 
-- 
1.7.6

[RFC/PATCH 5/7] OMAP: omap_device: when building return platform_device instead of omap_device

[Kevin Hilman]

All of the device init and device driver interaction with omap_device
is done using platform_device pointers.  To make this more explicit,
have omap_device return a platform_device pointer instead of an
omap_device pointer.

All current users of the omap_device pointer were only using it to get
at the platform_device pointer or struct device pointer, so fixing all
of the users was trivial.

This also makes it more difficult for device init code to directly
access members of struct omap_device, and allows for easier changing
of omap_device internals.

Cc: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/devices.c                 |   44 ++++++++++++------------
 arch/arm/mach-omap2/display.c                 |    6 ++--
 arch/arm/mach-omap2/dma.c                     |   16 ++++----
 arch/arm/mach-omap2/gpio.c                    |    8 ++--
 arch/arm/mach-omap2/hsmmc.c                   |    8 ++--
 arch/arm/mach-omap2/hwspinlock.c              |    8 ++--
 arch/arm/mach-omap2/mcbsp.c                   |    8 ++--
 arch/arm/mach-omap2/pm.c                      |    8 ++--
 arch/arm/mach-omap2/serial.c                  |   12 +++---
 arch/arm/plat-omap/i2c.c                      |    8 ++--
 arch/arm/plat-omap/include/plat/omap_device.h |    4 +-
 arch/arm/plat-omap/omap_device.c              |    6 ++--
 12 files changed, 68 insertions(+), 68 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 1077ad6..10adf66 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -44,7 +44,7 @@ static int __init omap3_l3_init(void)
 {
 	int l;
 	struct omap_hwmod *oh;
-	struct omap_device *od;
+	struct platform_device *pdev;
 	char oh_name[L3_MODULES_MAX_LEN];
 
 	/*
@@ -61,12 +61,12 @@ static int __init omap3_l3_init(void)
 	if (!oh)
 		pr_err("could not look up %s\n", oh_name);
 
-	od = omap_device_build("omap_l3_smx", 0, oh, NULL, 0,
+	pdev = omap_device_build("omap_l3_smx", 0, oh, NULL, 0,
 							   NULL, 0, 0);
 
-	WARN(IS_ERR(od), "could not build omap_device for %s\n", oh_name);
+	WARN(IS_ERR(pdev), "could not build omap_device for %s\n", oh_name);
 
-	return IS_ERR(od) ? PTR_ERR(od) : 0;
+	return IS_ERR(pdev) ? PTR_ERR(pdev) : 0;
 }
 postcore_initcall(omap3_l3_init);
 
@@ -74,7 +74,7 @@ static int __init omap4_l3_init(void)
 {
 	int l, i;
 	struct omap_hwmod *oh[3];
-	struct omap_device *od;
+	struct platform_device *pdev;
 	char oh_name[L3_MODULES_MAX_LEN];
 
 	/*
@@ -92,12 +92,12 @@ static int __init omap4_l3_init(void)
 			pr_err("could not look up %s\n", oh_name);
 	}
 
-	od = omap_device_build_ss("omap_l3_noc", 0, oh, 3, NULL,
+	pdev = omap_device_build_ss("omap_l3_noc", 0, oh, 3, NULL,
 						     0, NULL, 0, 0);
 
-	WARN(IS_ERR(od), "could not build omap_device for %s\n", oh_name);
+	WARN(IS_ERR(pdev), "could not build omap_device for %s\n", oh_name);
 
-	return IS_ERR(od) ? PTR_ERR(od) : 0;
+	return IS_ERR(pdev) ? PTR_ERR(pdev) : 0;
 }
 postcore_initcall(omap4_l3_init);
 
@@ -232,7 +232,7 @@ struct omap_device_pm_latency omap_keyboard_latency[] = {
 int __init omap4_keyboard_init(struct omap4_keypad_platform_data
 			*sdp4430_keypad_data, struct omap_board_data *bdata)
 {
-	struct omap_device *od;
+	struct platform_device *pdev;
 	struct omap_hwmod *oh;
 	struct omap4_keypad_platform_data *keypad_data;
 	unsigned int id = -1;
@@ -247,15 +247,15 @@ int __init omap4_keyboard_init(struct omap4_keypad_platform_data
 
 	keypad_data = sdp4430_keypad_data;
 
-	od = omap_device_build(name, id, oh, keypad_data,
+	pdev = omap_device_build(name, id, oh, keypad_data,
 			sizeof(struct omap4_keypad_platform_data),
 			omap_keyboard_latency,
 			ARRAY_SIZE(omap_keyboard_latency), 0);
 
-	if (IS_ERR(od)) {
+	if (IS_ERR(pdev)) {
 		WARN(1, "Can't build omap_device for %s:%s.\n",
 						name, oh->name);
-		return PTR_ERR(od);
+		return PTR_ERR(pdev);
 	}
 	oh->mux = omap_hwmod_mux_init(bdata->pads, bdata->pads_cnt);
 
@@ -274,7 +274,7 @@ static struct omap_device_pm_latency mbox_latencies[] = {
 static inline void omap_init_mbox(void)
 {
 	struct omap_hwmod *oh;
-	struct omap_device *od;
+	struct platform_device *pdev;
 
 	oh = omap_hwmod_lookup("mailbox");
 	if (!oh) {
@@ -282,10 +282,10 @@ static inline void omap_init_mbox(void)
 		return;
 	}
 
-	od = omap_device_build("omap-mailbox", -1, oh, NULL, 0,
+	pdev = omap_device_build("omap-mailbox", -1, oh, NULL, 0,
 				mbox_latencies, ARRAY_SIZE(mbox_latencies), 0);
-	WARN(IS_ERR(od), "%s: could not build device, err %ld\n",
-						__func__, PTR_ERR(od));
+	WARN(IS_ERR(pdev), "%s: could not build device, err %ld\n",
+						__func__, PTR_ERR(pdev));
 }
 #else
 static inline void omap_init_mbox(void) { }
@@ -344,7 +344,7 @@ struct omap_device_pm_latency omap_mcspi_latency[] = {
 
 static int omap_mcspi_init(struct omap_hwmod *oh, void *unused)
 {
-	struct omap_device *od;
+	struct platform_device *pdev;
 	char *name = "omap2_mcspi";
 	struct omap2_mcspi_platform_config *pdata;
 	static int spi_num;
@@ -371,10 +371,10 @@ static int omap_mcspi_init(struct omap_hwmod *oh, void *unused)
 	}
 
 	spi_num++;
-	od = omap_device_build(name, spi_num, oh, pdata,
+	pdev = omap_device_build(name, spi_num, oh, pdata,
 				sizeof(*pdata),	omap_mcspi_latency,
 				ARRAY_SIZE(omap_mcspi_latency), 0);
-	WARN(IS_ERR(od), "Can't build omap_device for %s:%s\n",
+	WARN(IS_ERR(pdev), "Can't build omap_device for %s:%s\n",
 				name, oh->name);
 	kfree(pdata);
 	return 0;
@@ -709,7 +709,7 @@ static struct omap_device_pm_latency omap_wdt_latency[] = {
 static int __init omap_init_wdt(void)
 {
 	int id = -1;
-	struct omap_device *od;
+	struct platform_device *pdev;
 	struct omap_hwmod *oh;
 	char *oh_name = "wd_timer2";
 	char *dev_name = "omap_wdt";
@@ -723,10 +723,10 @@ static int __init omap_init_wdt(void)
 		return -EINVAL;
 	}
 
-	od = omap_device_build(dev_name, id, oh, NULL, 0,
+	pdev = omap_device_build(dev_name, id, oh, NULL, 0,
 				omap_wdt_latency,
 				ARRAY_SIZE(omap_wdt_latency), 0);
-	WARN(IS_ERR(od), "Can't build omap_device for %s:%s.\n",
+	WARN(IS_ERR(pdev), "Can't build omap_device for %s:%s.\n",
 				dev_name, oh->name);
 	return 0;
 }
diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
index a5b7a23..18693f6 100644
--- a/arch/arm/mach-omap2/display.c
+++ b/arch/arm/mach-omap2/display.c
@@ -78,7 +78,7 @@ int __init omap_display_init(struct omap_dss_board_info *board_data)
 {
 	int r = 0;
 	struct omap_hwmod *oh;
-	struct omap_device *od;
+	struct platform_device *pdev;
 	int i, oh_count;
 	struct omap_display_platform_data pdata;
 	const struct omap_dss_hwmod_data *curr_dss_hwmod;
@@ -108,13 +108,13 @@ int __init omap_display_init(struct omap_dss_board_info *board_data)
 			return -ENODEV;
 		}
 
-		od = omap_device_build(curr_dss_hwmod[i].dev_name,
+		pdev = omap_device_build(curr_dss_hwmod[i].dev_name,
 				curr_dss_hwmod[i].id, oh, &pdata,
 				sizeof(struct omap_display_platform_data),
 				omap_dss_latency,
 				ARRAY_SIZE(omap_dss_latency), 0);
 
-		if (WARN((IS_ERR(od)), "Could not build omap_device for %s\n",
+		if (WARN((IS_ERR(pdev)), "Could not build omap_device for %s\n",
 				curr_dss_hwmod[i].oh_name))
 			return -ENODEV;
 	}
diff --git a/arch/arm/mach-omap2/dma.c b/arch/arm/mach-omap2/dma.c
index c9ff0e7..ae8cb3f 100644
--- a/arch/arm/mach-omap2/dma.c
+++ b/arch/arm/mach-omap2/dma.c
@@ -228,7 +228,7 @@ static u32 configure_dma_errata(void)
 /* One time initializations */
 static int __init omap2_system_dma_init_dev(struct omap_hwmod *oh, void *unused)
 {
-	struct omap_device			*od;
+	struct platform_device			*pdev;
 	struct omap_system_dma_plat_info	*p;
 	struct resource				*mem;
 	char					*name = "omap_dma_system";
@@ -258,23 +258,23 @@ static int __init omap2_system_dma_init_dev(struct omap_hwmod *oh, void *unused)
 
 	p->errata		= configure_dma_errata();
 
-	od = omap_device_build(name, 0, oh, p, sizeof(*p),
+	pdev = omap_device_build(name, 0, oh, p, sizeof(*p),
 			omap2_dma_latency, ARRAY_SIZE(omap2_dma_latency), 0);
 	kfree(p);
-	if (IS_ERR(od)) {
+	if (IS_ERR(pdev)) {
 		pr_err("%s: Can't build omap_device for %s:%s.\n",
 			__func__, name, oh->name);
-		return PTR_ERR(od);
+		return PTR_ERR(pdev);
 	}
 
-	mem = platform_get_resource(&od->pdev, IORESOURCE_MEM, 0);
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!mem) {
-		dev_err(&od->pdev.dev, "%s: no mem resource\n", __func__);
+		dev_err(&pdev->dev, "%s: no mem resource\n", __func__);
 		return -EINVAL;
 	}
 	dma_base = ioremap(mem->start, resource_size(mem));
 	if (!dma_base) {
-		dev_err(&od->pdev.dev, "%s: ioremap fail\n", __func__);
+		dev_err(&pdev->dev, "%s: ioremap fail\n", __func__);
 		return -ENOMEM;
 	}
 
@@ -283,7 +283,7 @@ static int __init omap2_system_dma_init_dev(struct omap_hwmod *oh, void *unused)
 					(d->lch_count), GFP_KERNEL);
 
 	if (!d->chan) {
-		dev_err(&od->pdev.dev, "%s: kzalloc fail\n", __func__);
+		dev_err(&pdev->dev, "%s: kzalloc fail\n", __func__);
 		return -ENOMEM;
 	}
 	return 0;
diff --git a/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c
index 9a46d77..f4f25c1 100644
--- a/arch/arm/mach-omap2/gpio.c
+++ b/arch/arm/mach-omap2/gpio.c
@@ -34,7 +34,7 @@ static struct omap_device_pm_latency omap_gpio_latency[] = {
 
 static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 {
-	struct omap_device *od;
+	struct platform_device *pdev;
 	struct omap_gpio_platform_data *pdata;
 	struct omap_gpio_dev_attr *dev_attr;
 	char *name = "omap_gpio";
@@ -107,16 +107,16 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 		return -EINVAL;
 	}
 
-	od = omap_device_build(name, id - 1, oh, pdata,
+	pdev = omap_device_build(name, id - 1, oh, pdata,
 				sizeof(*pdata),	omap_gpio_latency,
 				ARRAY_SIZE(omap_gpio_latency),
 				false);
 	kfree(pdata);
 
-	if (IS_ERR(od)) {
+	if (IS_ERR(pdev)) {
 		WARN(1, "Can't build omap_device for %s:%s.\n",
 					name, oh->name);
-		return PTR_ERR(od);
+		return PTR_ERR(pdev);
 	}
 
 	gpio_bank_count++;
diff --git a/arch/arm/mach-omap2/hsmmc.c b/arch/arm/mach-omap2/hsmmc.c
index a9b45c7..cc87919 100644
--- a/arch/arm/mach-omap2/hsmmc.c
+++ b/arch/arm/mach-omap2/hsmmc.c
@@ -430,7 +430,7 @@ static struct omap_device_pm_latency omap_hsmmc_latency[] = {
 void __init omap_init_hsmmc(struct omap2_hsmmc_info *hsmmcinfo, int ctrl_nr)
 {
 	struct omap_hwmod *oh;
-	struct omap_device *od;
+	struct platform_device *pdev;
 	struct omap_device_pm_latency *ohl;
 	char oh_name[MAX_OMAP_MMC_HWMOD_NAME_LEN];
 	struct omap_mmc_platform_data *mmc_data;
@@ -471,9 +471,9 @@ void __init omap_init_hsmmc(struct omap2_hsmmc_info *hsmmcinfo, int ctrl_nr)
 		mmc_data->controller_flags = mmc_dev_attr->flags;
 	}
 
-	od = omap_device_build(name, ctrl_nr - 1, oh, mmc_data,
+	pdev = omap_device_build(name, ctrl_nr - 1, oh, mmc_data,
 		sizeof(struct omap_mmc_platform_data), ohl, ohl_cnt, false);
-	if (IS_ERR(od)) {
+	if (IS_ERR(pdev)) {
 		WARN(1, "Can't build omap_device for %s:%s.\n", name, oh->name);
 		kfree(mmc_data->slots[0].name);
 		goto done;
@@ -482,7 +482,7 @@ void __init omap_init_hsmmc(struct omap2_hsmmc_info *hsmmcinfo, int ctrl_nr)
 	 * return device handle to board setup code
 	 * required to populate for regulator framework structure
 	 */
-	hsmmcinfo->dev = &od->pdev.dev;
+	hsmmcinfo->dev = &pdev->dev;
 
 done:
 	kfree(mmc_data);
diff --git a/arch/arm/mach-omap2/hwspinlock.c b/arch/arm/mach-omap2/hwspinlock.c
index 06d4a80..0b3ae9d 100644
--- a/arch/arm/mach-omap2/hwspinlock.c
+++ b/arch/arm/mach-omap2/hwspinlock.c
@@ -35,7 +35,7 @@ int __init hwspinlocks_init(void)
 {
 	int retval = 0;
 	struct omap_hwmod *oh;
-	struct omap_device *od;
+	struct platform_device *pdev;
 	const char *oh_name = "spinlock";
 	const char *dev_name = "omap_hwspinlock";
 
@@ -48,13 +48,13 @@ int __init hwspinlocks_init(void)
 	if (oh == NULL)
 		return -EINVAL;
 
-	od = omap_device_build(dev_name, 0, oh, NULL, 0,
+	pdev = omap_device_build(dev_name, 0, oh, NULL, 0,
 				omap_spinlock_latency,
 				ARRAY_SIZE(omap_spinlock_latency), false);
-	if (IS_ERR(od)) {
+	if (IS_ERR(pdev)) {
 		pr_err("Can't build omap_device for %s:%s\n", dev_name,
 								oh_name);
-		retval = PTR_ERR(od);
+		retval = PTR_ERR(pdev);
 	}
 
 	return retval;
diff --git a/arch/arm/mach-omap2/mcbsp.c b/arch/arm/mach-omap2/mcbsp.c
index 4a6ef6a..7a42f32 100644
--- a/arch/arm/mach-omap2/mcbsp.c
+++ b/arch/arm/mach-omap2/mcbsp.c
@@ -116,7 +116,7 @@ static int omap_init_mcbsp(struct omap_hwmod *oh, void *unused)
 	char *name = "omap-mcbsp";
 	struct omap_hwmod *oh_device[2];
 	struct omap_mcbsp_platform_data *pdata = NULL;
-	struct omap_device *od;
+	struct platform_device *pdev;
 
 	sscanf(oh->name, "mcbsp%d", &id);
 
@@ -144,14 +144,14 @@ static int omap_init_mcbsp(struct omap_hwmod *oh, void *unused)
 		(struct omap_mcbsp_dev_attr *)(oh->dev_attr))->sidetone);
 		count++;
 	}
-	od = omap_device_build_ss(name, id, oh_device, count, pdata,
+	pdev = omap_device_build_ss(name, id, oh_device, count, pdata,
 				sizeof(*pdata), omap2_mcbsp_latency,
 				ARRAY_SIZE(omap2_mcbsp_latency), false);
 	kfree(pdata);
-	if (IS_ERR(od))  {
+	if (IS_ERR(pdev))  {
 		pr_err("%s: Can't build omap_device for %s:%s.\n", __func__,
 					name, oh->name);
-		return PTR_ERR(od);
+		return PTR_ERR(pdev);
 	}
 	omap_mcbsp_count++;
 	return 0;
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 3feb359..54281e5 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -60,19 +60,19 @@ EXPORT_SYMBOL(omap4_get_dsp_device);
 static int _init_omap_device(char *name, struct device **new_dev)
 {
 	struct omap_hwmod *oh;
-	struct omap_device *od;
+	struct platform_device *pdev;
 
 	oh = omap_hwmod_lookup(name);
 	if (WARN(!oh, "%s: could not find omap_hwmod for %s\n",
 		 __func__, name))
 		return -ENODEV;
 
-	od = omap_device_build(oh->name, 0, oh, NULL, 0, pm_lats, 0, false);
-	if (WARN(IS_ERR(od), "%s: could not build omap_device for %s\n",
+	pdev = omap_device_build(oh->name, 0, oh, NULL, 0, pm_lats, 0, false);
+	if (WARN(IS_ERR(pdev), "%s: could not build omap_device for %s\n",
 		 __func__, name))
 		return -ENODEV;
 
-	*new_dev = &od->pdev.dev;
+	*new_dev = &pdev->dev;
 
 	return 0;
 }
diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 1ac361b..3781bcb 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -711,7 +711,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
 {
 	struct omap_uart_state *uart;
 	struct omap_hwmod *oh;
-	struct omap_device *od;
+	struct platform_device *pdev;
 	void *pdata = NULL;
 	u32 pdata_size = 0;
 	char *name;
@@ -799,10 +799,10 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
 	if (WARN_ON(!oh))
 		return;
 
-	od = omap_device_build(name, uart->num, oh, pdata, pdata_size,
+	pdev = omap_device_build(name, uart->num, oh, pdata, pdata_size,
 			       omap_uart_latency,
 			       ARRAY_SIZE(omap_uart_latency), false);
-	WARN(IS_ERR(od), "Could not build omap_device for %s: %s.\n",
+	WARN(IS_ERR(pdev), "Could not build omap_device for %s: %s.\n",
 	     name, oh->name);
 
 	oh->mux = omap_hwmod_mux_init(bdata->pads, bdata->pads_cnt);
@@ -811,7 +811,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
 	uart->regshift = 2;
 	uart->mapbase = oh->slaves[0]->addr->pa_start;
 	uart->membase = omap_hwmod_get_mpu_rt_va(oh);
-	uart->pdev = &od->pdev;
+	uart->pdev = pdev;
 
 	oh->dev_attr = uart;
 
@@ -845,8 +845,8 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
 
 	if ((cpu_is_omap34xx() && uart->padconf) ||
 	    (uart->wk_en && uart->wk_mask)) {
-		device_init_wakeup(&od->pdev.dev, true);
-		DEV_CREATE_FILE(&od->pdev.dev, &dev_attr_sleep_timeout);
+		device_init_wakeup(&pdev->dev, true);
+		DEV_CREATE_FILE(&pdev->dev, &dev_attr_sleep_timeout);
 	}
 
 	/* Enable the MDR1 errata for OMAP3 */
diff --git a/arch/arm/plat-omap/i2c.c b/arch/arm/plat-omap/i2c.c
index 3341ca4..0c7caf2 100644
--- a/arch/arm/plat-omap/i2c.c
+++ b/arch/arm/plat-omap/i2c.c
@@ -135,7 +135,7 @@ static inline int omap2_i2c_add_bus(int bus_id)
 {
 	int l;
 	struct omap_hwmod *oh;
-	struct omap_device *od;
+	struct platform_device *pdev;
 	char oh_name[MAX_OMAP_I2C_HWMOD_NAME_LEN];
 	struct omap_i2c_bus_platform_data *pdata;
 
@@ -160,12 +160,12 @@ static inline int omap2_i2c_add_bus(int bus_id)
 	 */
 	if (cpu_is_omap34xx())
 		pdata->set_mpu_wkup_lat = omap_pm_set_max_mpu_wakeup_lat_compat;
-	od = omap_device_build(name, bus_id, oh, pdata,
+	pdev = omap_device_build(name, bus_id, oh, pdata,
 			sizeof(struct omap_i2c_bus_platform_data),
 			omap_i2c_latency, ARRAY_SIZE(omap_i2c_latency), 0);
-	WARN(IS_ERR(od), "Could not build omap_device for %s\n", name);
+	WARN(IS_ERR(pdev), "Could not build omap_device for %s\n", name);
 
-	return PTR_ERR(od);
+	return PTR_ERR(pdev);
 }
 #else
 static inline int omap2_i2c_add_bus(int bus_id)
diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
index 0a53a0a..7a3ec4f 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -83,13 +83,13 @@ int omap_device_shutdown(struct platform_device *pdev);
 
 /* Core code interface */
 
-struct omap_device *omap_device_build(const char *pdev_name, int pdev_id,
+struct platform_device *omap_device_build(const char *pdev_name, int pdev_id,
 				      struct omap_hwmod *oh, void *pdata,
 				      int pdata_len,
 				      struct omap_device_pm_latency *pm_lats,
 				      int pm_lats_cnt, int is_early_device);
 
-struct omap_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
+struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
 					 struct omap_hwmod **oh, int oh_cnt,
 					 void *pdata, int pdata_len,
 					 struct omap_device_pm_latency *pm_lats,
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 4e4b36c..932a7e6 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -405,7 +405,7 @@ static int omap_device_fill_resources(struct omap_device *od,
  * information.  Returns ERR_PTR(-EINVAL) if @oh is NULL; otherwise,
  * passes along the return value of omap_device_build_ss().
  */
-struct omap_device *omap_device_build(const char *pdev_name, int pdev_id,
+struct platform_device *omap_device_build(const char *pdev_name, int pdev_id,
 				      struct omap_hwmod *oh, void *pdata,
 				      int pdata_len,
 				      struct omap_device_pm_latency *pm_lats,
@@ -438,7 +438,7 @@ struct omap_device *omap_device_build(const char *pdev_name, int pdev_id,
  * platform_device record.  Returns an ERR_PTR() on error, or passes
  * along the return value of omap_device_register().
  */
-struct omap_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
+struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
 					 struct omap_hwmod **ohs, int oh_cnt,
 					 void *pdata, int pdata_len,
 					 struct omap_device_pm_latency *pm_lats,
@@ -513,7 +513,7 @@ struct omap_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
 	if (ret)
 		goto odbs_exit4;
 
-	return od;
+	return &od->pdev;
 
 odbs_exit4:
 	kfree(res);
-- 
1.7.6

[RFC/PATCH 6/7] OMAP: omap_device: device register functions now take platform_device pointer

[Kevin Hilman]

The internal device register functions do not need or use any omap_device
internals, so pass in a platform_device pointer instead of an omap_device
pointer.

Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/plat-omap/omap_device.c |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 932a7e6..c420b94 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -94,8 +94,8 @@
 #define USE_WAKEUP_LAT			0
 #define IGNORE_WAKEUP_LAT		1
 
-static int omap_device_register(struct omap_device *od);
-static int omap_early_device_register(struct omap_device *od);
+static int omap_device_register(struct platform_device *pdev);
+static int omap_early_device_register(struct platform_device *pdev);
 
 /* Private functions */
 
@@ -501,9 +501,9 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
 	od->pm_lats_cnt = pm_lats_cnt;
 
 	if (is_early_device)
-		ret = omap_early_device_register(od);
+		ret = omap_early_device_register(&od->pdev);
 	else
-		ret = omap_device_register(od);
+		ret = omap_device_register(&od->pdev);
 
 	for (i = 0; i < oh_cnt; i++) {
 		hwmods[i]->od = od;
@@ -538,11 +538,11 @@ odbs_exit1:
  * platform_early_add_device() on the underlying platform_device.
  * Returns 0 by default.
  */
-static int omap_early_device_register(struct omap_device *od)
+static int omap_early_device_register(struct platform_device *pdev)
 {
 	struct platform_device *devices[1];
 
-	devices[0] = &(od->pdev);
+	devices[0] = pdev;
 	early_platform_add_devices(devices, 1);
 	return 0;
 }
@@ -591,13 +591,13 @@ static struct dev_power_domain omap_device_power_domain = {
  * platform_device_register() on the underlying platform_device.
  * Returns the return value of platform_device_register().
  */
-static int omap_device_register(struct omap_device *od)
+static int omap_device_register(struct platform_device *pdev)
 {
-	pr_debug("omap_device: %s: registering\n", od->pdev.name);
+	pr_debug("omap_device: %s: registering\n", pdev->name);
 
-	od->pdev.dev.parent = &omap_device_parent;
-	od->pdev.dev.pwr_domain = &omap_device_power_domain;
-	return platform_device_register(&od->pdev);
+	pdev->dev.parent = &omap_device_parent;
+	pdev->dev.pwr_domain = &omap_device_power_domain;
+	return platform_device_add(pdev);
 }
 
 
-- 
1.7.6

[RFC/PATCH 6/7] OMAP: omap_device: device register functions now take platform_device pointer

[Grant Likely]

On Thu, Jul 21, 2011 at 04:52:17PM -0700, Kevin Hilman wrote:
> The internal device register functions do not need or use any omap_device
> internals, so pass in a platform_device pointer instead of an omap_device
> pointer.
> 
> Signed-off-by: Kevin Hilman <khilman@ti.com>

FWIW, all 6 of these cleanups look sane.

Reviewed-by: Grant Likely <grant.likely@secretlab.ca>

[RFC/PATCH 7/7] WIP: HACK/RFC: omap_device: begin to decouple platform_device from omap_device

[Kevin Hilman]

Rather than embedding a struct platform_device inside a struct
omap_device, decouple them, leaving only a pointer to the
platform_device inside the omap_device.

This patch uses devres to allocate and attach the omap_device to the
struct device, so finding an omap_device from a struct device means
using devres_find(), and the to_omap_device() helper function was
modified accordingly.

RFC/Hack alert:

Currently the driver core (drivers/base/dd.c) doesn't expect any
devres resources to exist before the driver's ->probe() is called.  In
this patch, I just comment out the warning, but we'll need to
understand why that limitation exists, and if it's a real limitation.
A first glance suggests that it's not really needed.  If this is a
true limitation, we'll need to find some way other than devres to
attach an omap_device to a struct device.

On OMAP, we will need an omap_device attached to a struct device
before probe because device HW may be disabled in probe and drivers
are expected to use runtime PM in ->probe() to activate the hardware
before access.  Because the runtime PM API calls use omap_device (via
our PM domain layer), we need omap_device attached to a
platform_device before probe.
---
 arch/arm/mach-omap2/opp.c                     |    2 +-
 arch/arm/plat-omap/include/plat/omap_device.h |    4 +-
 arch/arm/plat-omap/omap_device.c              |   78 +++++++++++++++----------
 drivers/base/dd.c                             |    2 +-
 4 files changed, 50 insertions(+), 36 deletions(-)

diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c
index ab8b35b..9262a6b 100644
--- a/arch/arm/mach-omap2/opp.c
+++ b/arch/arm/mach-omap2/opp.c
@@ -69,7 +69,7 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def,
 				opp_def->hwmod_name, i);
 			return -EINVAL;
 		}
-		dev = &oh->od->pdev.dev;
+		dev = &oh->od->pdev->dev;
 
 		r = opp_add(dev, opp_def->freq, opp_def->u_volt);
 		if (r) {
diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
index 7a3ec4f..6bd4f6f 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -64,7 +64,7 @@ extern struct device omap_device_parent;
  *
  */
 struct omap_device {
-	struct platform_device		pdev;
+	struct platform_device		*pdev;
 	struct omap_hwmod		**hwmods;
 	struct omap_device_pm_latency	*pm_lats;
 	u32				dev_wakeup_lat;
@@ -142,6 +142,6 @@ struct omap_device_pm_latency {
 #define OMAP_DEVICE_LATENCY_AUTO_ADJUST BIT(1)
 
 /* Get omap_device pointer from platform_device pointer */
-#define to_omap_device(x) container_of((x), struct omap_device, pdev)
+struct omap_device *to_omap_device(struct platform_device *pdev);
 
 #endif
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index c420b94..52ce013 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -117,7 +117,7 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat)
 {
 	struct timespec a, b, c;
 
-	dev_dbg(&od->pdev.dev, "omap_device: activating\n");
+	dev_dbg(&od->pdev->dev, "omap_device: activating\n");
 
 	while (od->pm_lat_level > 0) {
 		struct omap_device_pm_latency *odpl;
@@ -141,7 +141,7 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat)
 		c = timespec_sub(b, a);
 		act_lat = timespec_to_ns(&c);
 
-		dev_dbg(&od->pdev.dev,
+		dev_dbg(&od->pdev->dev,
 			"omap_device: pm_lat %d: activate: elapsed time "
 			"%llu nsec\n", od->pm_lat_level, act_lat);
 
@@ -149,12 +149,12 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat)
 			odpl->activate_lat_worst = act_lat;
 			if (odpl->flags & OMAP_DEVICE_LATENCY_AUTO_ADJUST) {
 				odpl->activate_lat = act_lat;
-				dev_warn(&od->pdev.dev,
+				dev_warn(&od->pdev->dev,
 					 "new worst case activate latency "
 					 "%d: %llu\n",
 					 od->pm_lat_level, act_lat);
 			} else
-				dev_warn(&od->pdev.dev,
+				dev_warn(&od->pdev->dev,
 					 "activate latency %d "
 					 "higher than exptected. (%llu > %d)\n",
 					 od->pm_lat_level, act_lat,
@@ -185,7 +185,7 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
 {
 	struct timespec a, b, c;
 
-	dev_dbg(&od->pdev.dev, "omap_device: deactivating\n");
+	dev_dbg(&od->pdev->dev, "omap_device: deactivating\n");
 
 	while (od->pm_lat_level < od->pm_lats_cnt) {
 		struct omap_device_pm_latency *odpl;
@@ -208,7 +208,7 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
 		c = timespec_sub(b, a);
 		deact_lat = timespec_to_ns(&c);
 
-		dev_dbg(&od->pdev.dev,
+		dev_dbg(&od->pdev->dev,
 			"omap_device: pm_lat %d: deactivate: elapsed time "
 			"%llu nsec\n", od->pm_lat_level, deact_lat);
 
@@ -216,12 +216,12 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
 			odpl->deactivate_lat_worst = deact_lat;
 			if (odpl->flags & OMAP_DEVICE_LATENCY_AUTO_ADJUST) {
 				odpl->deactivate_lat = deact_lat;
-				dev_warn(&od->pdev.dev,
+				dev_warn(&od->pdev->dev,
 					 "new worst case deactivate latency "
 					 "%d: %llu\n",
 					 od->pm_lat_level, deact_lat);
 			} else
-				dev_warn(&od->pdev.dev,
+				dev_warn(&od->pdev->dev,
 					 "deactivate latency %d "
 					 "higher than exptected. (%llu > %d)\n",
 					 od->pm_lat_level, deact_lat,
@@ -245,11 +245,11 @@ static void _add_clkdev(struct omap_device *od, const char *clk_alias,
 	if (!clk_alias || !clk_name)
 		return;
 
-	dev_dbg(&od->pdev.dev, "Creating %s -> %s\n", clk_alias, clk_name);
+	dev_dbg(&od->pdev->dev, "Creating %s -> %s\n", clk_alias, clk_name);
 
-	r = clk_get_sys(dev_name(&od->pdev.dev), clk_alias);
+	r = clk_get_sys(dev_name(&od->pdev->dev), clk_alias);
 	if (!IS_ERR(r)) {
-		dev_warn(&od->pdev.dev,
+		dev_warn(&od->pdev->dev,
 			 "alias %s already exists\n", clk_alias);
 		clk_put(r);
 		return;
@@ -257,14 +257,14 @@ static void _add_clkdev(struct omap_device *od, const char *clk_alias,
 
 	r = omap_clk_get_by_name(clk_name);
 	if (IS_ERR(r)) {
-		dev_err(&od->pdev.dev,
+		dev_err(&od->pdev->dev,
 			"omap_clk_get_by_name for %s failed\n", clk_name);
 		return;
 	}
 
-	l = clkdev_alloc(r, clk_alias, dev_name(&od->pdev.dev));
+	l = clkdev_alloc(r, clk_alias, dev_name(&od->pdev->dev));
 	if (!l) {
-		dev_err(&od->pdev.dev,
+		dev_err(&od->pdev->dev,
 			"clkdev_alloc for %s failed\n", clk_alias);
 		return;
 	}
@@ -351,7 +351,7 @@ static int omap_device_count_resources(struct omap_device *od)
 		c += omap_hwmod_count_resources(od->hwmods[i]);
 
 	pr_debug("omap_device: %s: counted %d total resources across %d "
-		 "hwmods\n", od->pdev.name, c, od->hwmods_cnt);
+		 "hwmods\n", od->pdev->name, c, od->hwmods_cnt);
 
 	return c;
 }
@@ -388,6 +388,21 @@ static int omap_device_fill_resources(struct omap_device *od,
 	return 0;
 }
 
+static void _od_dr_release(struct device *dev, void *res)
+{
+	kfree(res);
+}
+
+struct omap_device *to_omap_device(struct platform_device *pdev)
+{
+	void *res;
+
+	res = devres_find(&pdev->dev, _od_dr_release, NULL, NULL);
+	WARN_ON(!res);
+
+	return (struct omap_device *)res;
+}
+
 /**
  * omap_device_build - build and register an omap_device with one omap_hwmod
  * @pdev_name: name of the platform_device driver to use
@@ -445,8 +460,8 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
 					 int pm_lats_cnt, int is_early_device)
 {
 	int ret = -ENOMEM;
+	struct platform_device *pdev;
 	struct omap_device *od;
-	char *pdev_name2;
 	struct resource *res = NULL;
 	int i, res_count;
 	struct omap_hwmod **hwmods;
@@ -460,7 +475,8 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
 	pr_debug("omap_device: %s: building with %d hwmods\n", pdev_name,
 		 oh_cnt);
 
-	od = kzalloc(sizeof(struct omap_device), GFP_KERNEL);
+	od = devres_alloc(_od_dr_release, sizeof(struct omap_device),
+			  GFP_KERNEL);
 	if (!od)
 		return ERR_PTR(-ENOMEM);
 
@@ -474,13 +490,9 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
 	memcpy(hwmods, ohs, sizeof(struct omap_hwmod *) * oh_cnt);
 	od->hwmods = hwmods;
 
-	pdev_name2 = kzalloc(strlen(pdev_name) + 1, GFP_KERNEL);
-	if (!pdev_name2)
+	pdev = platform_device_alloc(pdev_name, pdev_id);
+	if (!pdev)
 		goto odbs_exit2;
-	strcpy(pdev_name2, pdev_name);
-
-	od->pdev.name = pdev_name2;
-	od->pdev.id = pdev_id;
 
 	res_count = omap_device_count_resources(od);
 	if (res_count > 0) {
@@ -490,35 +502,37 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
 	}
 	omap_device_fill_resources(od, res);
 
-	od->pdev.num_resources = res_count;
-	od->pdev.resource = res;
+	pdev->num_resources = res_count;
+	pdev->resource = res;
 
-	ret = platform_device_add_data(&od->pdev, pdata, pdata_len);
+	ret = platform_device_add_data(pdev, pdata, pdata_len);
 	if (ret)
 		goto odbs_exit4;
 
 	od->pm_lats = pm_lats;
 	od->pm_lats_cnt = pm_lats_cnt;
 
-	if (is_early_device)
-		ret = omap_early_device_register(&od->pdev);
-	else
-		ret = omap_device_register(&od->pdev);
+	od->pdev = pdev;
+	devres_add(&pdev->dev, od);
 
 	for (i = 0; i < oh_cnt; i++) {
 		hwmods[i]->od = od;
 		_add_hwmod_clocks_clkdev(od, hwmods[i]);
 	}
 
+	if (is_early_device)
+		ret = omap_early_device_register(pdev);
+	else
+		ret = omap_device_register(pdev);
+
 	if (ret)
 		goto odbs_exit4;
 
-	return &od->pdev;
+	return pdev;
 
 odbs_exit4:
 	kfree(res);
 odbs_exit3:
-	kfree(pdev_name2);
 odbs_exit2:
 	kfree(hwmods);
 odbs_exit1:
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 6658da7..9289dac 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -112,7 +112,7 @@ static int really_probe(struct device *dev, struct device_driver *drv)
 	atomic_inc(&probe_count);
 	pr_debug("bus: '%s': %s: probing driver %s with device %s\n",
 		 drv->bus->name, __func__, drv->name, dev_name(dev));
-	WARN_ON(!list_empty(&dev->devres_head));
+	/* WARN_ON(!list_empty(&dev->devres_head)); */
 
 	dev->driver = drv;
 	if (driver_sysfs_add(dev)) {
-- 
1.7.6

[RFC/PATCH 7/7] WIP: HACK/RFC: omap_device: begin to decouple platform_device from omap_device

[Grant Likely]

On Thu, Jul 21, 2011 at 5:52 PM, Kevin Hilman <khilman@ti.com> wrote:
> Rather than embedding a struct platform_device inside a struct
> omap_device, decouple them, leaving only a pointer to the
> platform_device inside the omap_device.
>
> This patch uses devres to allocate and attach the omap_device to the
> struct device, so finding an omap_device from a struct device means
> using devres_find(), and the to_omap_device() helper function was
> modified accordingly.
>
> RFC/Hack alert:
>
> Currently the driver core (drivers/base/dd.c) doesn't expect any
> devres resources to exist before the driver's ->probe() is called. ?In
> this patch, I just comment out the warning, but we'll need to
> understand why that limitation exists, and if it's a real limitation.
> A first glance suggests that it's not really needed. ?If this is a
> true limitation, we'll need to find some way other than devres to
> attach an omap_device to a struct device.

The devres lifetime is scoped to binding a driver; it is added at
probe time and removed at release.  For this use-case, it needs to be
scoped to the lifetime of the struct device.  To make this work,
you'll need to add a flag to devres so that the driver core can
differentiate between driver-scoped and device-scoped lifetimes (which
I do think is the right thing to do).  Without fixing this, the driver
core will remove all the devres when a driver gets unbound, or if a
.probe() hook fails, which completely breaks the driver model.

g.

>
> On OMAP, we will need an omap_device attached to a struct device
> before probe because device HW may be disabled in probe and drivers
> are expected to use runtime PM in ->probe() to activate the hardware
> before access. ?Because the runtime PM API calls use omap_device (via
> our PM domain layer), we need omap_device attached to a
> platform_device before probe.
> ---
> ?arch/arm/mach-omap2/opp.c ? ? ? ? ? ? ? ? ? ? | ? ?2 +-
> ?arch/arm/plat-omap/include/plat/omap_device.h | ? ?4 +-
> ?arch/arm/plat-omap/omap_device.c ? ? ? ? ? ? ?| ? 78 +++++++++++++++----------
> ?drivers/base/dd.c ? ? ? ? ? ? ? ? ? ? ? ? ? ? | ? ?2 +-
> ?4 files changed, 50 insertions(+), 36 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c
> index ab8b35b..9262a6b 100644
> --- a/arch/arm/mach-omap2/opp.c
> +++ b/arch/arm/mach-omap2/opp.c
> @@ -69,7 +69,7 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?opp_def->hwmod_name, i);
> ? ? ? ? ? ? ? ? ? ? ? ?return -EINVAL;
> ? ? ? ? ? ? ? ?}
> - ? ? ? ? ? ? ? dev = &oh->od->pdev.dev;
> + ? ? ? ? ? ? ? dev = &oh->od->pdev->dev;
>
> ? ? ? ? ? ? ? ?r = opp_add(dev, opp_def->freq, opp_def->u_volt);
> ? ? ? ? ? ? ? ?if (r) {
> diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
> index 7a3ec4f..6bd4f6f 100644
> --- a/arch/arm/plat-omap/include/plat/omap_device.h
> +++ b/arch/arm/plat-omap/include/plat/omap_device.h
> @@ -64,7 +64,7 @@ extern struct device omap_device_parent;
> ?*
> ?*/
> ?struct omap_device {
> - ? ? ? struct platform_device ? ? ? ? ?pdev;
> + ? ? ? struct platform_device ? ? ? ? ?*pdev;
> ? ? ? ?struct omap_hwmod ? ? ? ? ? ? ? **hwmods;
> ? ? ? ?struct omap_device_pm_latency ? *pm_lats;
> ? ? ? ?u32 ? ? ? ? ? ? ? ? ? ? ? ? ? ? dev_wakeup_lat;
> @@ -142,6 +142,6 @@ struct omap_device_pm_latency {
> ?#define OMAP_DEVICE_LATENCY_AUTO_ADJUST BIT(1)
>
> ?/* Get omap_device pointer from platform_device pointer */
> -#define to_omap_device(x) container_of((x), struct omap_device, pdev)
> +struct omap_device *to_omap_device(struct platform_device *pdev);
>
> ?#endif
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index c420b94..52ce013 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -117,7 +117,7 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat)
> ?{
> ? ? ? ?struct timespec a, b, c;
>
> - ? ? ? dev_dbg(&od->pdev.dev, "omap_device: activating\n");
> + ? ? ? dev_dbg(&od->pdev->dev, "omap_device: activating\n");
>
> ? ? ? ?while (od->pm_lat_level > 0) {
> ? ? ? ? ? ? ? ?struct omap_device_pm_latency *odpl;
> @@ -141,7 +141,7 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat)
> ? ? ? ? ? ? ? ?c = timespec_sub(b, a);
> ? ? ? ? ? ? ? ?act_lat = timespec_to_ns(&c);
>
> - ? ? ? ? ? ? ? dev_dbg(&od->pdev.dev,
> + ? ? ? ? ? ? ? dev_dbg(&od->pdev->dev,
> ? ? ? ? ? ? ? ? ? ? ? ?"omap_device: pm_lat %d: activate: elapsed time "
> ? ? ? ? ? ? ? ? ? ? ? ?"%llu nsec\n", od->pm_lat_level, act_lat);
>
> @@ -149,12 +149,12 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat)
> ? ? ? ? ? ? ? ? ? ? ? ?odpl->activate_lat_worst = act_lat;
> ? ? ? ? ? ? ? ? ? ? ? ?if (odpl->flags & OMAP_DEVICE_LATENCY_AUTO_ADJUST) {
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?odpl->activate_lat = act_lat;
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dev_warn(&od->pdev.dev,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dev_warn(&od->pdev->dev,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "new worst case activate latency "
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "%d: %llu\n",
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? od->pm_lat_level, act_lat);
> ? ? ? ? ? ? ? ? ? ? ? ?} else
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dev_warn(&od->pdev.dev,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dev_warn(&od->pdev->dev,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "activate latency %d "
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "higher than exptected. (%llu > %d)\n",
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? od->pm_lat_level, act_lat,
> @@ -185,7 +185,7 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
> ?{
> ? ? ? ?struct timespec a, b, c;
>
> - ? ? ? dev_dbg(&od->pdev.dev, "omap_device: deactivating\n");
> + ? ? ? dev_dbg(&od->pdev->dev, "omap_device: deactivating\n");
>
> ? ? ? ?while (od->pm_lat_level < od->pm_lats_cnt) {
> ? ? ? ? ? ? ? ?struct omap_device_pm_latency *odpl;
> @@ -208,7 +208,7 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
> ? ? ? ? ? ? ? ?c = timespec_sub(b, a);
> ? ? ? ? ? ? ? ?deact_lat = timespec_to_ns(&c);
>
> - ? ? ? ? ? ? ? dev_dbg(&od->pdev.dev,
> + ? ? ? ? ? ? ? dev_dbg(&od->pdev->dev,
> ? ? ? ? ? ? ? ? ? ? ? ?"omap_device: pm_lat %d: deactivate: elapsed time "
> ? ? ? ? ? ? ? ? ? ? ? ?"%llu nsec\n", od->pm_lat_level, deact_lat);
>
> @@ -216,12 +216,12 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
> ? ? ? ? ? ? ? ? ? ? ? ?odpl->deactivate_lat_worst = deact_lat;
> ? ? ? ? ? ? ? ? ? ? ? ?if (odpl->flags & OMAP_DEVICE_LATENCY_AUTO_ADJUST) {
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?odpl->deactivate_lat = deact_lat;
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dev_warn(&od->pdev.dev,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dev_warn(&od->pdev->dev,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "new worst case deactivate latency "
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "%d: %llu\n",
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? od->pm_lat_level, deact_lat);
> ? ? ? ? ? ? ? ? ? ? ? ?} else
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dev_warn(&od->pdev.dev,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dev_warn(&od->pdev->dev,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "deactivate latency %d "
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "higher than exptected. (%llu > %d)\n",
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? od->pm_lat_level, deact_lat,
> @@ -245,11 +245,11 @@ static void _add_clkdev(struct omap_device *od, const char *clk_alias,
> ? ? ? ?if (!clk_alias || !clk_name)
> ? ? ? ? ? ? ? ?return;
>
> - ? ? ? dev_dbg(&od->pdev.dev, "Creating %s -> %s\n", clk_alias, clk_name);
> + ? ? ? dev_dbg(&od->pdev->dev, "Creating %s -> %s\n", clk_alias, clk_name);
>
> - ? ? ? r = clk_get_sys(dev_name(&od->pdev.dev), clk_alias);
> + ? ? ? r = clk_get_sys(dev_name(&od->pdev->dev), clk_alias);
> ? ? ? ?if (!IS_ERR(r)) {
> - ? ? ? ? ? ? ? dev_warn(&od->pdev.dev,
> + ? ? ? ? ? ? ? dev_warn(&od->pdev->dev,
> ? ? ? ? ? ? ? ? ? ? ? ? "alias %s already exists\n", clk_alias);
> ? ? ? ? ? ? ? ?clk_put(r);
> ? ? ? ? ? ? ? ?return;
> @@ -257,14 +257,14 @@ static void _add_clkdev(struct omap_device *od, const char *clk_alias,
>
> ? ? ? ?r = omap_clk_get_by_name(clk_name);
> ? ? ? ?if (IS_ERR(r)) {
> - ? ? ? ? ? ? ? dev_err(&od->pdev.dev,
> + ? ? ? ? ? ? ? dev_err(&od->pdev->dev,
> ? ? ? ? ? ? ? ? ? ? ? ?"omap_clk_get_by_name for %s failed\n", clk_name);
> ? ? ? ? ? ? ? ?return;
> ? ? ? ?}
>
> - ? ? ? l = clkdev_alloc(r, clk_alias, dev_name(&od->pdev.dev));
> + ? ? ? l = clkdev_alloc(r, clk_alias, dev_name(&od->pdev->dev));
> ? ? ? ?if (!l) {
> - ? ? ? ? ? ? ? dev_err(&od->pdev.dev,
> + ? ? ? ? ? ? ? dev_err(&od->pdev->dev,
> ? ? ? ? ? ? ? ? ? ? ? ?"clkdev_alloc for %s failed\n", clk_alias);
> ? ? ? ? ? ? ? ?return;
> ? ? ? ?}
> @@ -351,7 +351,7 @@ static int omap_device_count_resources(struct omap_device *od)
> ? ? ? ? ? ? ? ?c += omap_hwmod_count_resources(od->hwmods[i]);
>
> ? ? ? ?pr_debug("omap_device: %s: counted %d total resources across %d "
> - ? ? ? ? ? ? ? ?"hwmods\n", od->pdev.name, c, od->hwmods_cnt);
> + ? ? ? ? ? ? ? ?"hwmods\n", od->pdev->name, c, od->hwmods_cnt);
>
> ? ? ? ?return c;
> ?}
> @@ -388,6 +388,21 @@ static int omap_device_fill_resources(struct omap_device *od,
> ? ? ? ?return 0;
> ?}
>
> +static void _od_dr_release(struct device *dev, void *res)
> +{
> + ? ? ? kfree(res);
> +}
> +
> +struct omap_device *to_omap_device(struct platform_device *pdev)
> +{
> + ? ? ? void *res;
> +
> + ? ? ? res = devres_find(&pdev->dev, _od_dr_release, NULL, NULL);
> + ? ? ? WARN_ON(!res);
> +
> + ? ? ? return (struct omap_device *)res;
> +}
> +
> ?/**
> ?* omap_device_build - build and register an omap_device with one omap_hwmod
> ?* @pdev_name: name of the platform_device driver to use
> @@ -445,8 +460,8 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int pm_lats_cnt, int is_early_device)
> ?{
> ? ? ? ?int ret = -ENOMEM;
> + ? ? ? struct platform_device *pdev;
> ? ? ? ?struct omap_device *od;
> - ? ? ? char *pdev_name2;
> ? ? ? ?struct resource *res = NULL;
> ? ? ? ?int i, res_count;
> ? ? ? ?struct omap_hwmod **hwmods;
> @@ -460,7 +475,8 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
> ? ? ? ?pr_debug("omap_device: %s: building with %d hwmods\n", pdev_name,
> ? ? ? ? ? ? ? ? oh_cnt);
>
> - ? ? ? od = kzalloc(sizeof(struct omap_device), GFP_KERNEL);
> + ? ? ? od = devres_alloc(_od_dr_release, sizeof(struct omap_device),
> + ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
> ? ? ? ?if (!od)
> ? ? ? ? ? ? ? ?return ERR_PTR(-ENOMEM);
>
> @@ -474,13 +490,9 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
> ? ? ? ?memcpy(hwmods, ohs, sizeof(struct omap_hwmod *) * oh_cnt);
> ? ? ? ?od->hwmods = hwmods;
>
> - ? ? ? pdev_name2 = kzalloc(strlen(pdev_name) + 1, GFP_KERNEL);
> - ? ? ? if (!pdev_name2)
> + ? ? ? pdev = platform_device_alloc(pdev_name, pdev_id);
> + ? ? ? if (!pdev)
> ? ? ? ? ? ? ? ?goto odbs_exit2;
> - ? ? ? strcpy(pdev_name2, pdev_name);
> -
> - ? ? ? od->pdev.name = pdev_name2;
> - ? ? ? od->pdev.id = pdev_id;
>
> ? ? ? ?res_count = omap_device_count_resources(od);
> ? ? ? ?if (res_count > 0) {
> @@ -490,35 +502,37 @@ struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
> ? ? ? ?}
> ? ? ? ?omap_device_fill_resources(od, res);
>
> - ? ? ? od->pdev.num_resources = res_count;
> - ? ? ? od->pdev.resource = res;
> + ? ? ? pdev->num_resources = res_count;
> + ? ? ? pdev->resource = res;
>
> - ? ? ? ret = platform_device_add_data(&od->pdev, pdata, pdata_len);
> + ? ? ? ret = platform_device_add_data(pdev, pdata, pdata_len);
> ? ? ? ?if (ret)
> ? ? ? ? ? ? ? ?goto odbs_exit4;
>
> ? ? ? ?od->pm_lats = pm_lats;
> ? ? ? ?od->pm_lats_cnt = pm_lats_cnt;
>
> - ? ? ? if (is_early_device)
> - ? ? ? ? ? ? ? ret = omap_early_device_register(&od->pdev);
> - ? ? ? else
> - ? ? ? ? ? ? ? ret = omap_device_register(&od->pdev);
> + ? ? ? od->pdev = pdev;
> + ? ? ? devres_add(&pdev->dev, od);
>
> ? ? ? ?for (i = 0; i < oh_cnt; i++) {
> ? ? ? ? ? ? ? ?hwmods[i]->od = od;
> ? ? ? ? ? ? ? ?_add_hwmod_clocks_clkdev(od, hwmods[i]);
> ? ? ? ?}
>
> + ? ? ? if (is_early_device)
> + ? ? ? ? ? ? ? ret = omap_early_device_register(pdev);
> + ? ? ? else
> + ? ? ? ? ? ? ? ret = omap_device_register(pdev);
> +
> ? ? ? ?if (ret)
> ? ? ? ? ? ? ? ?goto odbs_exit4;
>
> - ? ? ? return &od->pdev;
> + ? ? ? return pdev;
>
> ?odbs_exit4:
> ? ? ? ?kfree(res);
> ?odbs_exit3:
> - ? ? ? kfree(pdev_name2);
> ?odbs_exit2:
> ? ? ? ?kfree(hwmods);
> ?odbs_exit1:
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 6658da7..9289dac 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -112,7 +112,7 @@ static int really_probe(struct device *dev, struct device_driver *drv)
> ? ? ? ?atomic_inc(&probe_count);
> ? ? ? ?pr_debug("bus: '%s': %s: probing driver %s with device %s\n",
> ? ? ? ? ? ? ? ? drv->bus->name, __func__, drv->name, dev_name(dev));
> - ? ? ? WARN_ON(!list_empty(&dev->devres_head));
> + ? ? ? /* WARN_ON(!list_empty(&dev->devres_head)); */
>
> ? ? ? ?dev->driver = drv;
> ? ? ? ?if (driver_sysfs_add(dev)) {
> --
> 1.7.6
>
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

[RFC/PATCH 7/7] WIP: HACK/RFC: omap_device: begin to decouple platform_device from omap_device

[Russell King - ARM Linux]

On Thu, Jul 21, 2011 at 04:52:18PM -0700, Kevin Hilman wrote:
> Rather than embedding a struct platform_device inside a struct
> omap_device, decouple them, leaving only a pointer to the
> platform_device inside the omap_device.
> 
> This patch uses devres to allocate and attach the omap_device to the
> struct device, so finding an omap_device from a struct device means
> using devres_find(), and the to_omap_device() helper function was
> modified accordingly.
> 
> RFC/Hack alert:
> 
> Currently the driver core (drivers/base/dd.c) doesn't expect any
> devres resources to exist before the driver's ->probe() is called.  In
> this patch, I just comment out the warning, but we'll need to
> understand why that limitation exists, and if it's a real limitation.
> A first glance suggests that it's not really needed.  If this is a
> true limitation, we'll need to find some way other than devres to
> attach an omap_device to a struct device.
> 
> On OMAP, we will need an omap_device attached to a struct device
> before probe because device HW may be disabled in probe and drivers
> are expected to use runtime PM in ->probe() to activate the hardware
> before access.  Because the runtime PM API calls use omap_device (via
> our PM domain layer), we need omap_device attached to a
> platform_device before probe.

This feels really wrong to overload devres with this.  devres purpose is
to provide the device's _drivers_ with a way to allocate and free resources
in such a way to avoid leaks on .remove or probe failure.  So I think that
overloading it with something that has a different lifetime is completely
wrong.

We could add a new member to struct dev_archdata or pdev_archdata to carry
a pointer to this data, which I think would be a far cleaner (and saner)
way to deal with this.  In much the same way as we already have an of_node
member in struct device.

[RFC/PATCH 7/7] WIP: HACK/RFC: omap_device: begin to decouple platform_device from omap_device

[Grant Likely]

On Sat, Jul 30, 2011 at 01:03:32PM +0100, Russell King - ARM Linux wrote:
> On Thu, Jul 21, 2011 at 04:52:18PM -0700, Kevin Hilman wrote:
> > Rather than embedding a struct platform_device inside a struct
> > omap_device, decouple them, leaving only a pointer to the
> > platform_device inside the omap_device.
> > 
> > This patch uses devres to allocate and attach the omap_device to the
> > struct device, so finding an omap_device from a struct device means
> > using devres_find(), and the to_omap_device() helper function was
> > modified accordingly.
> > 
> > RFC/Hack alert:
> > 
> > Currently the driver core (drivers/base/dd.c) doesn't expect any
> > devres resources to exist before the driver's ->probe() is called.  In
> > this patch, I just comment out the warning, but we'll need to
> > understand why that limitation exists, and if it's a real limitation.
> > A first glance suggests that it's not really needed.  If this is a
> > true limitation, we'll need to find some way other than devres to
> > attach an omap_device to a struct device.
> > 
> > On OMAP, we will need an omap_device attached to a struct device
> > before probe because device HW may be disabled in probe and drivers
> > are expected to use runtime PM in ->probe() to activate the hardware
> > before access.  Because the runtime PM API calls use omap_device (via
> > our PM domain layer), we need omap_device attached to a
> > platform_device before probe.
> 
> This feels really wrong to overload devres with this.  devres purpose is
> to provide the device's _drivers_ with a way to allocate and free resources
> in such a way to avoid leaks on .remove or probe failure.  So I think that
> overloading it with something that has a different lifetime is completely
> wrong.

I disagree; extending devres to also handle device lifetime scoped
resources makes perfect sense. It is a clean extension, and struct device
is really getting rather huge.  I certainly prefer re-scoping devres
to adding more elements to struct device.

> We could add a new member to struct dev_archdata or pdev_archdata to carry
> a pointer to this data, which I think would be a far cleaner (and saner)
> way to deal with this.  In much the same way as we already have an of_node
> member in struct device.

Since this is just for omap_device, which by definition is arm-only, I
don't have any strong objection to using pdev_archdata. If it was
cross-architecture, then I'd argue strongly for the devres approach.

g.

[RFC/PATCH 7/7] WIP: HACK/RFC: omap_device: begin to decouple platform_device from omap_device

[Russell King - ARM Linux]

On Sat, Jul 30, 2011 at 08:58:07PM -0600, Grant Likely wrote:
> On Sat, Jul 30, 2011 at 01:03:32PM +0100, Russell King - ARM Linux wrote:
> > On Thu, Jul 21, 2011 at 04:52:18PM -0700, Kevin Hilman wrote:
> > > Rather than embedding a struct platform_device inside a struct
> > > omap_device, decouple them, leaving only a pointer to the
> > > platform_device inside the omap_device.
> > > 
> > > This patch uses devres to allocate and attach the omap_device to the
> > > struct device, so finding an omap_device from a struct device means
> > > using devres_find(), and the to_omap_device() helper function was
> > > modified accordingly.
> > > 
> > > RFC/Hack alert:
> > > 
> > > Currently the driver core (drivers/base/dd.c) doesn't expect any
> > > devres resources to exist before the driver's ->probe() is called.  In
> > > this patch, I just comment out the warning, but we'll need to
> > > understand why that limitation exists, and if it's a real limitation.
> > > A first glance suggests that it's not really needed.  If this is a
> > > true limitation, we'll need to find some way other than devres to
> > > attach an omap_device to a struct device.
> > > 
> > > On OMAP, we will need an omap_device attached to a struct device
> > > before probe because device HW may be disabled in probe and drivers
> > > are expected to use runtime PM in ->probe() to activate the hardware
> > > before access.  Because the runtime PM API calls use omap_device (via
> > > our PM domain layer), we need omap_device attached to a
> > > platform_device before probe.
> > 
> > This feels really wrong to overload devres with this.  devres purpose is
> > to provide the device's _drivers_ with a way to allocate and free resources
> > in such a way to avoid leaks on .remove or probe failure.  So I think that
> > overloading it with something that has a different lifetime is completely
> > wrong.
> 
> I disagree; extending devres to also handle device lifetime scoped
> resources makes perfect sense. It is a clean extension, and struct device
> is really getting rather huge.  I certainly prefer re-scoping devres
> to adding more elements to struct device.

The point is you're asking something which is designed to have a well
defined lifetime of driver-bind...driver-unbind to do a lot more than
that - extending its lifetime conditional on some flag to the entire
device lifetime instead.

Such extensions tend to be a disaster - and a recipe for confusion as
people will no longer have a clear idea of the lifetime issues.  We
already know that people absolutely struggle to understand lifed
objects - we see it time and time again where people directly kfree
stuff like platform devices without thinking about whether they're
still in use.

So no, extending devres is a _big_ mistake and is far from clean.

Not only that, but if most of the platform devices are omap devices,
(as it would be on OMAP), you'll consume more memory through having to
have the additional management structs for the omap_device stuff than
a simple pointer.

As for struct device getting rather huge, last time I looked it contained
a load of stuff which was there whether or not a platform used it.  Eg,
you get 4 bytes wasted for an of_node whether you have DT support or not.
If sizeof(struct device) really is a problem, then it needs the unused
stuff ifdef'd away.  So I don't buy the "its getting rather huge" argument.

> > We could add a new member to struct dev_archdata or pdev_archdata to carry
> > a pointer to this data, which I think would be a far cleaner (and saner)
> > way to deal with this.  In much the same way as we already have an of_node
> > member in struct device.
> 
> Since this is just for omap_device, which by definition is arm-only, I
> don't have any strong objection to using pdev_archdata. If it was
> cross-architecture, then I'd argue strongly for the devres approach.

Indeed, which is why I think putting it in the platform device archdata
makes total sense, more sense than buggering up the clean devres lifetime
rules that we have today.

[RFC/PATCH 7/7] WIP: HACK/RFC: omap_device: begin to decouple platform_device from omap_device

[Kevin Hilman]

Russell King - ARM Linux <linux@arm.linux.org.uk> writes:

> On Sat, Jul 30, 2011 at 08:58:07PM -0600, Grant Likely wrote:
>> On Sat, Jul 30, 2011 at 01:03:32PM +0100, Russell King - ARM Linux wrote:
>> > On Thu, Jul 21, 2011 at 04:52:18PM -0700, Kevin Hilman wrote:
>> > > Rather than embedding a struct platform_device inside a struct
>> > > omap_device, decouple them, leaving only a pointer to the
>> > > platform_device inside the omap_device.
>> > > 
>> > > This patch uses devres to allocate and attach the omap_device to the
>> > > struct device, so finding an omap_device from a struct device means
>> > > using devres_find(), and the to_omap_device() helper function was
>> > > modified accordingly.
>> > > 
>> > > RFC/Hack alert:
>> > > 
>> > > Currently the driver core (drivers/base/dd.c) doesn't expect any
>> > > devres resources to exist before the driver's ->probe() is called.  In
>> > > this patch, I just comment out the warning, but we'll need to
>> > > understand why that limitation exists, and if it's a real limitation.
>> > > A first glance suggests that it's not really needed.  If this is a
>> > > true limitation, we'll need to find some way other than devres to
>> > > attach an omap_device to a struct device.
>> > > 
>> > > On OMAP, we will need an omap_device attached to a struct device
>> > > before probe because device HW may be disabled in probe and drivers
>> > > are expected to use runtime PM in ->probe() to activate the hardware
>> > > before access.  Because the runtime PM API calls use omap_device (via
>> > > our PM domain layer), we need omap_device attached to a
>> > > platform_device before probe.
>> > 
>> > This feels really wrong to overload devres with this.  devres purpose is
>> > to provide the device's _drivers_ with a way to allocate and free resources
>> > in such a way to avoid leaks on .remove or probe failure.  So I think that
>> > overloading it with something that has a different lifetime is completely
>> > wrong.
>> 
>> I disagree; extending devres to also handle device lifetime scoped
>> resources makes perfect sense. It is a clean extension, and struct device
>> is really getting rather huge.  I certainly prefer re-scoping devres
>> to adding more elements to struct device.
>
> The point is you're asking something which is designed to have a well
> defined lifetime of driver-bind...driver-unbind to do a lot more than
> that - extending its lifetime conditional on some flag to the entire
> device lifetime instead.
>
> Such extensions tend to be a disaster - and a recipe for confusion as
> people will no longer have a clear idea of the lifetime issues.  We
> already know that people absolutely struggle to understand lifed
> objects - we see it time and time again where people directly kfree
> stuff like platform devices without thinking about whether they're
> still in use.
>
> So no, extending devres is a _big_ mistake and is far from clean.
>
> Not only that, but if most of the platform devices are omap devices,
> (as it would be on OMAP), you'll consume more memory through having to
> have the additional management structs for the omap_device stuff than
> a simple pointer.
>
> As for struct device getting rather huge, last time I looked it contained
> a load of stuff which was there whether or not a platform used it.  Eg,
> you get 4 bytes wasted for an of_node whether you have DT support or not.
> If sizeof(struct device) really is a problem, then it needs the unused
> stuff ifdef'd away.  So I don't buy the "its getting rather huge" argument.
>
>> > We could add a new member to struct dev_archdata or pdev_archdata to carry
>> > a pointer to this data, which I think would be a far cleaner (and saner)
>> > way to deal with this.  In much the same way as we already have an of_node
>> > member in struct device.
>> 
>> Since this is just for omap_device, which by definition is arm-only, I
>> don't have any strong objection to using pdev_archdata. If it was
>> cross-architecture, then I'd argue strongly for the devres approach.
>
> Indeed, which is why I think putting it in the platform device archdata
> makes total sense, more sense than buggering up the clean devres lifetime
> rules that we have today.

OK, so I'll continue this approach using pdev_archdata, which would work
fine for me.  It's currently empty, so I'll just add a 'void *' if it's
OK with you Russell:

diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h
index 9f390ce..bb777cd 100644
--- a/arch/arm/include/asm/device.h
+++ b/arch/arm/include/asm/device.h
@@ -13,6 +13,7 @@ struct dev_archdata {
 };
 
 struct pdev_archdata {
+	void *p;
 };
 
 #endif

[RFC/PATCH 7/7] WIP: HACK/RFC: omap_device: begin to decouple platform_device from omap_device

[Grant Likely]

On Mon, Aug 1, 2011 at 4:42 PM, Kevin Hilman <khilman@ti.com> wrote:
> Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
>
>> On Sat, Jul 30, 2011 at 08:58:07PM -0600, Grant Likely wrote:
>>> On Sat, Jul 30, 2011 at 01:03:32PM +0100, Russell King - ARM Linux wrote:
>>> > On Thu, Jul 21, 2011 at 04:52:18PM -0700, Kevin Hilman wrote:
>>> > > Rather than embedding a struct platform_device inside a struct
>>> > > omap_device, decouple them, leaving only a pointer to the
>>> > > platform_device inside the omap_device.
>>> > >
>>> > > This patch uses devres to allocate and attach the omap_device to the
>>> > > struct device, so finding an omap_device from a struct device means
>>> > > using devres_find(), and the to_omap_device() helper function was
>>> > > modified accordingly.
>>> > >
>>> > > RFC/Hack alert:
>>> > >
>>> > > Currently the driver core (drivers/base/dd.c) doesn't expect any
>>> > > devres resources to exist before the driver's ->probe() is called. ?In
>>> > > this patch, I just comment out the warning, but we'll need to
>>> > > understand why that limitation exists, and if it's a real limitation.
>>> > > A first glance suggests that it's not really needed. ?If this is a
>>> > > true limitation, we'll need to find some way other than devres to
>>> > > attach an omap_device to a struct device.
>>> > >
>>> > > On OMAP, we will need an omap_device attached to a struct device
>>> > > before probe because device HW may be disabled in probe and drivers
>>> > > are expected to use runtime PM in ->probe() to activate the hardware
>>> > > before access. ?Because the runtime PM API calls use omap_device (via
>>> > > our PM domain layer), we need omap_device attached to a
>>> > > platform_device before probe.
>>> >
>>> > This feels really wrong to overload devres with this. ?devres purpose is
>>> > to provide the device's _drivers_ with a way to allocate and free resources
>>> > in such a way to avoid leaks on .remove or probe failure. ?So I think that
>>> > overloading it with something that has a different lifetime is completely
>>> > wrong.
>>>
>>> I disagree; extending devres to also handle device lifetime scoped
>>> resources makes perfect sense. It is a clean extension, and struct device
>>> is really getting rather huge. ?I certainly prefer re-scoping devres
>>> to adding more elements to struct device.
>>
>> The point is you're asking something which is designed to have a well
>> defined lifetime of driver-bind...driver-unbind to do a lot more than
>> that - extending its lifetime conditional on some flag to the entire
>> device lifetime instead.
>>
>> Such extensions tend to be a disaster - and a recipe for confusion as
>> people will no longer have a clear idea of the lifetime issues. ?We
>> already know that people absolutely struggle to understand lifed
>> objects - we see it time and time again where people directly kfree
>> stuff like platform devices without thinking about whether they're
>> still in use.
>>
>> So no, extending devres is a _big_ mistake and is far from clean.
>>
>> Not only that, but if most of the platform devices are omap devices,
>> (as it would be on OMAP), you'll consume more memory through having to
>> have the additional management structs for the omap_device stuff than
>> a simple pointer.
>>
>> As for struct device getting rather huge, last time I looked it contained
>> a load of stuff which was there whether or not a platform used it. ?Eg,
>> you get 4 bytes wasted for an of_node whether you have DT support or not.
>> If sizeof(struct device) really is a problem, then it needs the unused
>> stuff ifdef'd away. ?So I don't buy the "its getting rather huge" argument.
>>
>>> > We could add a new member to struct dev_archdata or pdev_archdata to carry
>>> > a pointer to this data, which I think would be a far cleaner (and saner)
>>> > way to deal with this. ?In much the same way as we already have an of_node
>>> > member in struct device.
>>>
>>> Since this is just for omap_device, which by definition is arm-only, I
>>> don't have any strong objection to using pdev_archdata. If it was
>>> cross-architecture, then I'd argue strongly for the devres approach.
>>
>> Indeed, which is why I think putting it in the platform device archdata
>> makes total sense, more sense than buggering up the clean devres lifetime
>> rules that we have today.
>
> OK, so I'll continue this approach using pdev_archdata, which would work
> fine for me. ?It's currently empty, so I'll just add a 'void *' if it's
> OK with you Russell:
>
> diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h
> index 9f390ce..bb777cd 100644
> --- a/arch/arm/include/asm/device.h
> +++ b/arch/arm/include/asm/device.h
> @@ -13,6 +13,7 @@ struct dev_archdata {
> ?};
>
> ?struct pdev_archdata {
> + ? ? ? void *p;
> ?};

struct omap_device *p;

Otherwise it is just asking for type safety problems.

g.

[RFC/PATCH 7/7] WIP: HACK/RFC: omap_device: begin to decouple platform_device from omap_device

[Felipe Balbi]

Hi,

On Mon, Aug 01, 2011 at 04:44:20PM +0100, Grant Likely wrote:
> On Mon, Aug 1, 2011 at 4:42 PM, Kevin Hilman <khilman@ti.com> wrote:
> > Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> >
> >> On Sat, Jul 30, 2011 at 08:58:07PM -0600, Grant Likely wrote:
> >>> On Sat, Jul 30, 2011 at 01:03:32PM +0100, Russell King - ARM Linux wrote:
> >>> > On Thu, Jul 21, 2011 at 04:52:18PM -0700, Kevin Hilman wrote:
> >>> > > Rather than embedding a struct platform_device inside a struct
> >>> > > omap_device, decouple them, leaving only a pointer to the
> >>> > > platform_device inside the omap_device.
> >>> > >
> >>> > > This patch uses devres to allocate and attach the omap_device to the
> >>> > > struct device, so finding an omap_device from a struct device means
> >>> > > using devres_find(), and the to_omap_device() helper function was
> >>> > > modified accordingly.
> >>> > >
> >>> > > RFC/Hack alert:
> >>> > >
> >>> > > Currently the driver core (drivers/base/dd.c) doesn't expect any
> >>> > > devres resources to exist before the driver's ->probe() is called. ?In
> >>> > > this patch, I just comment out the warning, but we'll need to
> >>> > > understand why that limitation exists, and if it's a real limitation.
> >>> > > A first glance suggests that it's not really needed. ?If this is a
> >>> > > true limitation, we'll need to find some way other than devres to
> >>> > > attach an omap_device to a struct device.
> >>> > >
> >>> > > On OMAP, we will need an omap_device attached to a struct device
> >>> > > before probe because device HW may be disabled in probe and drivers
> >>> > > are expected to use runtime PM in ->probe() to activate the hardware
> >>> > > before access. ?Because the runtime PM API calls use omap_device (via
> >>> > > our PM domain layer), we need omap_device attached to a
> >>> > > platform_device before probe.
> >>> >
> >>> > This feels really wrong to overload devres with this. ?devres purpose is
> >>> > to provide the device's _drivers_ with a way to allocate and free resources
> >>> > in such a way to avoid leaks on .remove or probe failure. ?So I think that
> >>> > overloading it with something that has a different lifetime is completely
> >>> > wrong.
> >>>
> >>> I disagree; extending devres to also handle device lifetime scoped
> >>> resources makes perfect sense. It is a clean extension, and struct device
> >>> is really getting rather huge. ?I certainly prefer re-scoping devres
> >>> to adding more elements to struct device.
> >>
> >> The point is you're asking something which is designed to have a well
> >> defined lifetime of driver-bind...driver-unbind to do a lot more than
> >> that - extending its lifetime conditional on some flag to the entire
> >> device lifetime instead.
> >>
> >> Such extensions tend to be a disaster - and a recipe for confusion as
> >> people will no longer have a clear idea of the lifetime issues. ?We
> >> already know that people absolutely struggle to understand lifed
> >> objects - we see it time and time again where people directly kfree
> >> stuff like platform devices without thinking about whether they're
> >> still in use.
> >>
> >> So no, extending devres is a _big_ mistake and is far from clean.
> >>
> >> Not only that, but if most of the platform devices are omap devices,
> >> (as it would be on OMAP), you'll consume more memory through having to
> >> have the additional management structs for the omap_device stuff than
> >> a simple pointer.
> >>
> >> As for struct device getting rather huge, last time I looked it contained
> >> a load of stuff which was there whether or not a platform used it. ?Eg,
> >> you get 4 bytes wasted for an of_node whether you have DT support or not.
> >> If sizeof(struct device) really is a problem, then it needs the unused
> >> stuff ifdef'd away. ?So I don't buy the "its getting rather huge" argument.
> >>
> >>> > We could add a new member to struct dev_archdata or pdev_archdata to carry
> >>> > a pointer to this data, which I think would be a far cleaner (and saner)
> >>> > way to deal with this. ?In much the same way as we already have an of_node
> >>> > member in struct device.
> >>>
> >>> Since this is just for omap_device, which by definition is arm-only, I
> >>> don't have any strong objection to using pdev_archdata. If it was
> >>> cross-architecture, then I'd argue strongly for the devres approach.
> >>
> >> Indeed, which is why I think putting it in the platform device archdata
> >> makes total sense, more sense than buggering up the clean devres lifetime
> >> rules that we have today.
> >
> > OK, so I'll continue this approach using pdev_archdata, which would work
> > fine for me. ?It's currently empty, so I'll just add a 'void *' if it's
> > OK with you Russell:
> >
> > diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h
> > index 9f390ce..bb777cd 100644
> > --- a/arch/arm/include/asm/device.h
> > +++ b/arch/arm/include/asm/device.h
> > @@ -13,6 +13,7 @@ struct dev_archdata {
> > ?};
> >
> > ?struct pdev_archdata {
> > + ? ? ? void *p;
> > ?};
> 
> struct omap_device *p;
> 
> Otherwise it is just asking for type safety problems.

considering that struct omap_device isn't ARM-wide, is it really wise to
to do that ? I guess a void * will do better here.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110801/7c55adce/attachment-0001.sig>

[RFC/PATCH 7/7] WIP: HACK/RFC: omap_device: begin to decouple platform_device from omap_device

[Russell King - ARM Linux]

On Mon, Aug 01, 2011 at 09:50:09PM +0300, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Aug 01, 2011 at 04:44:20PM +0100, Grant Likely wrote:
> > On Mon, Aug 1, 2011 at 4:42 PM, Kevin Hilman <khilman@ti.com> wrote:
> > > diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h
> > > index 9f390ce..bb777cd 100644
> > > --- a/arch/arm/include/asm/device.h
> > > +++ b/arch/arm/include/asm/device.h
> > > @@ -13,6 +13,7 @@ struct dev_archdata {
> > > ?};
> > >
> > > ?struct pdev_archdata {
> > > + ? ? ? void *p;
> > > ?};
> > 
> > struct omap_device *p;
> > 
> > Otherwise it is just asking for type safety problems.
> 
> considering that struct omap_device isn't ARM-wide, is it really wise to
> to do that ? I guess a void * will do better here.

Help the typechecker do its job.  As we have only one (at the moment...)
And make it:

+struct omap_device;

 struct pdev_archdata {
+#ifdef CONFIG_ARCH_OMAP
+	struct omap_device *omap;
+#endif
 };

for bonus points, so we only get the additional pointer for OMAP.

[RFC/PATCH 7/7] WIP: HACK/RFC: omap_device: begin to decouple platform_device from omap_device

[Kevin Hilman]

Russell King - ARM Linux <linux@arm.linux.org.uk> writes:

> Help the typechecker do its job.  As we have only one (at the moment...)
> And make it:
>
> +struct omap_device;
>
>  struct pdev_archdata {
> +#ifdef CONFIG_ARCH_OMAP
> +	struct omap_device *omap;
> +#endif
>  };
>
> for bonus points, so we only get the additional pointer for OMAP.

OK, will do it this way.

Kevin

[RFC/PATCH 7/7] WIP: HACK/RFC: omap_device: begin to decouple platform_device from omap_device

[Felipe Balbi]

Hi,

On Mon, Aug 01, 2011 at 03:11:57PM -0700, Kevin Hilman wrote:
> Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> 
> > Help the typechecker do its job.  As we have only one (at the moment...)
> > And make it:
> >
> > +struct omap_device;
> >
> >  struct pdev_archdata {
> > +#ifdef CONFIG_ARCH_OMAP
> > +	struct omap_device *omap;
> > +#endif
> >  };
> >
> > for bonus points, so we only get the additional pointer for OMAP.
> 
> OK, will do it this way.

this has the tendency to grow larger, no ? What if all other ARMs decide
to add their own pointers there too ? Counting the mach directories we
have:

$ ls arch/arm/ | grep mach | wc -l
64

minus a few duplicates like mach-omap1 and mach-omap2. Still, if we
count 40 different subarchs and each one of them adds their own pointer
here, this will become quite a messy piece of code, no ?

I agree we should try to have type checks, but considering the
possibility of many different pointers, does it really make sense ?
Nothing against it either, though.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110802/934e7289/attachment.sig>

[RFC/PATCH 7/7] WIP: HACK/RFC: omap_device: begin to decouple platform_device from omap_device

[Russell King - ARM Linux]

On Tue, Aug 02, 2011 at 01:55:55AM +0300, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Aug 01, 2011 at 03:11:57PM -0700, Kevin Hilman wrote:
> > Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> > 
> > > Help the typechecker do its job.  As we have only one (at the moment...)
> > > And make it:
> > >
> > > +struct omap_device;
> > >
> > >  struct pdev_archdata {
> > > +#ifdef CONFIG_ARCH_OMAP
> > > +	struct omap_device *omap;
> > > +#endif
> > >  };
> > >
> > > for bonus points, so we only get the additional pointer for OMAP.
> > 
> > OK, will do it this way.
> 
> this has the tendency to grow larger, no ? What if all other ARMs decide
> to add their own pointers there too ?

Their pointers for what?  It's only OMAP which has this special omap_device
thing.  Should that spread, instead of adding more pointers here, the work
should be to consolidate between those various implementations.

[RFC/PATCH 7/7] WIP: HACK/RFC: omap_device: begin to decouple platform_device from omap_device

[Grant Likely]

On Tue, Aug 02, 2011 at 12:09:45AM +0100, Russell King - ARM Linux wrote:
> On Tue, Aug 02, 2011 at 01:55:55AM +0300, Felipe Balbi wrote:
> > Hi,
> > 
> > On Mon, Aug 01, 2011 at 03:11:57PM -0700, Kevin Hilman wrote:
> > > Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> > > 
> > > > Help the typechecker do its job.  As we have only one (at the moment...)
> > > > And make it:
> > > >
> > > > +struct omap_device;
> > > >
> > > >  struct pdev_archdata {
> > > > +#ifdef CONFIG_ARCH_OMAP
> > > > +	struct omap_device *omap;
> > > > +#endif
> > > >  };
> > > >
> > > > for bonus points, so we only get the additional pointer for OMAP.
> > > 
> > > OK, will do it this way.
> > 
> > this has the tendency to grow larger, no ? What if all other ARMs decide
> > to add their own pointers there too ?
> 
> Their pointers for what?  It's only OMAP which has this special omap_device
> thing.  Should that spread, instead of adding more pointers here, the work
> should be to consolidate between those various implementations.

+1

g.

[RFC/PATCH 0/7] decouple platform_device from omap_device

[G, Manjunath Kondaiah]

Kevin,

On Thu, Jul 21, 2011 at 04:52:10PM -0700, Kevin Hilman wrote:
> Here's a first whack, proof-of-concept on how we could start to
> decouple the platform_device from an omap_device.
> 
> The main RFC is in the last patch, and everything leading up to it are
> misc. omap_device cleanups that make the last patch cleaner and
> clearer.  It's really the last patch that does the decoupling.
> 
> This will be necessary if we're going to decouple the platform_device
> creation from the omap_device/omap_hwmod creation etc.  This patch
> leaves them both done in omap_device_build(), but shows that they can
> be decoupled.
Can you pls mention baseline used for these patches? I tried applying on
latest mainline, v3.0 and  git://git.pwsan.com/linux-2.6 prcm-devel-3.1

It will not apply cleanly.

-M

[RFC/PATCH 0/7] decouple platform_device from omap_device

[Hilman, Kevin]

Hi Manjunath,

On Wed, Jul 27, 2011 at 7:04 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote:
> Kevin,
>
> On Thu, Jul 21, 2011 at 04:52:10PM -0700, Kevin Hilman wrote:
>> Here's a first whack, proof-of-concept on how we could start to
>> decouple the platform_device from an omap_device.
>>
>> The main RFC is in the last patch, and everything leading up to it are
>> misc. omap_device cleanups that make the last patch cleaner and
>> clearer. ?It's really the last patch that does the decoupling.
>>
>> This will be necessary if we're going to decouple the platform_device
>> creation from the omap_device/omap_hwmod creation etc. ?This patch
>> leaves them both done in omap_device_build(), but shows that they can
>> be decoupled.
> Can you pls mention baseline used for these patches? I tried applying on
> latest mainline, v3.0 and ?git://git.pwsan.com/linux-2.6 prcm-devel-3.1

Oops, sorry.  I forgot to mention it.

Due to some misc. dependencies (mainly on Beagle board file), I've
temporarily based it on the for-next branch of the arm-soc tree[1]
since that has everything already queued for the next merge window. I
also based it on the omap_device patch I posted which changes the pr_*
prints to dev_*.

For convenience, I've pushed this series to the 'wip/od-devres' branch
of my tree[2].

Sorry for the confusion,

Kevin

[1] git://git.kernel.org/pub/scm/linux/kernel/git/arm/linux-arm-soc.git

[2] git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git

[RFC/PATCH 0/7] decouple platform_device from omap_device

[G, Manjunath Kondaiah]

On Wed, Jul 27, 2011 at 02:45:33PM -0700, Hilman, Kevin wrote:
> Hi Manjunath,
> 
> On Wed, Jul 27, 2011 at 7:04 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote:
> > Kevin,
> >
> > On Thu, Jul 21, 2011 at 04:52:10PM -0700, Kevin Hilman wrote:
> >> Here's a first whack, proof-of-concept on how we could start to
> >> decouple the platform_device from an omap_device.
> >>
> >> The main RFC is in the last patch, and everything leading up to it are
> >> misc. omap_device cleanups that make the last patch cleaner and
> >> clearer. ?It's really the last patch that does the decoupling.
> >>
> >> This will be necessary if we're going to decouple the platform_device
> >> creation from the omap_device/omap_hwmod creation etc. ?This patch
> >> leaves them both done in omap_device_build(), but shows that they can
> >> be decoupled.
> > Can you pls mention baseline used for these patches? I tried applying on
> > latest mainline, v3.0 and ?git://git.pwsan.com/linux-2.6 prcm-devel-3.1
> 
> Oops, sorry.  I forgot to mention it.
> 
> Due to some misc. dependencies (mainly on Beagle board file), I've
> temporarily based it on the for-next branch of the arm-soc tree[1]
> since that has everything already queued for the next merge window. I
> also based it on the omap_device patch I posted which changes the pr_*
> prints to dev_*.
> 
> For convenience, I've pushed this series to the 'wip/od-devres' branch
> of my tree[2].
> 
> Sorry for the confusion,
> 
> Kevin
> 
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/arm/linux-arm-soc.git
> 
> [2] git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git

This series results in boot crash. However, it boots fine without this series. 
I have not tried to debug but it looks like from boot log, the res structure has
invalid entries in mmc probe. As mentioned by Grant in 7/7, the scope of devres 
is not lifetime hence it needs to be fixed.

The bootlog @ http://paste.ubuntu.com/653527/

-M

[RFC/PATCH 0/7] decouple platform_device from omap_device

[Kevin Hilman]

"G, Manjunath Kondaiah" <manjugk@ti.com> writes:

> On Wed, Jul 27, 2011 at 02:45:33PM -0700, Hilman, Kevin wrote:
>> Hi Manjunath,
>> 
>> On Wed, Jul 27, 2011 at 7:04 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote:
>> > Kevin,
>> >
>> > On Thu, Jul 21, 2011 at 04:52:10PM -0700, Kevin Hilman wrote:
>> >> Here's a first whack, proof-of-concept on how we could start to
>> >> decouple the platform_device from an omap_device.
>> >>
>> >> The main RFC is in the last patch, and everything leading up to it are
>> >> misc. omap_device cleanups that make the last patch cleaner and
>> >> clearer. ?It's really the last patch that does the decoupling.
>> >>
>> >> This will be necessary if we're going to decouple the platform_device
>> >> creation from the omap_device/omap_hwmod creation etc. ?This patch
>> >> leaves them both done in omap_device_build(), but shows that they can
>> >> be decoupled.
>> > Can you pls mention baseline used for these patches? I tried applying on
>> > latest mainline, v3.0 and ?git://git.pwsan.com/linux-2.6 prcm-devel-3.1
>> 
>> Oops, sorry.  I forgot to mention it.
>> 
>> Due to some misc. dependencies (mainly on Beagle board file), I've
>> temporarily based it on the for-next branch of the arm-soc tree[1]
>> since that has everything already queued for the next merge window. I
>> also based it on the omap_device patch I posted which changes the pr_*
>> prints to dev_*.
>> 
>> For convenience, I've pushed this series to the 'wip/od-devres' branch
>> of my tree[2].
>> 
>> Sorry for the confusion,
>> 
>> Kevin
>> 
>> [1] git://git.kernel.org/pub/scm/linux/kernel/git/arm/linux-arm-soc.git
>> 
>> [2] git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git
>
> This series results in boot crash. However, it boots fine without this
> series.  I have not tried to debug but it looks like from boot log,
> the res structure has invalid entries in mmc probe. As mentioned by
> Grant in 7/7, the scope of devres is not lifetime hence it needs to be
> fixed.

This limitation was also clearly described in the changelog of patch
7/7.

However, I don't think that problem should cause a problem on boot, only
a from the driver core (which, suprisingly,  I don't see in your bootlog.)

I suspect your crash is because you're not also testing with the MMC
runtime PM series that is merging for v3.1.

Kevin

[RFC/PATCH 3/7] OMAP: McBSP: use existing macros for converting between devices

[Kevin Hilman]

For converting from struct device to platform_device, and from
platform_device to struct omap_device, there are existing macros.  Use
them instead of manual use of container_of().

Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/plat-omap/mcbsp.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index 6c62af1..02199e8 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -260,9 +260,9 @@ EXPORT_SYMBOL(omap_mcbsp_dma_reg_params);
 #ifdef CONFIG_ARCH_OMAP3
 static struct omap_device *find_omap_device_by_dev(struct device *dev)
 {
-	struct platform_device *pdev = container_of(dev,
-					struct platform_device, dev);
-	return container_of(pdev, struct omap_device, pdev);
+	struct platform_device *pdev = to_platform_device(dev);
+
+	return to_omap_device(pdev);
 }
 
 static void omap_st_on(struct omap_mcbsp *mcbsp)
-- 
1.7.6