decouple platform_device from omap_device

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.

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 <[email protected]>

---
 arch/arm/mach-omap2/board-omap3beagle.c |   23 ++++++++++-------------
 1 files changed, 10 insertions(+), 13 deletions(-)

arch/arm/mach-omap2/board-omap3beagle.c +0 -0

index 32f5f89..3ae16b4 100644
@@ -491,23 +491,22 @@ static void __init beagle_opp_init(void)
 

arch/arm/mach-omap2/board-omap3beagle.c static void __init beagle_opp_init(void) · line 497

Quoted 10 lines

arch/arm/mach-omap2/board-omap3beagle.c +3 -4

 	/* 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.

(quoted diff) +4 -0

+struct device *omap_hwmod_to_device(const char *oh_name)
+{
+	struct omap_hwmod *oh;
+

On 11:57-20110722, Felipe Balbi wrote:
[...]

Quoted 13 lines

arch/arm/mach-omap2/board-omap3beagle.c static void __init beagle_opp_init(void) · line 497

Quoted 10 lines

/* Custom OPP enabled for all xM versions */ if (cpu_is_omap3630()) {

arch/arm/mach-omap2/board-omap3beagle.c +3 -4

-		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.

Quoted 6 lines

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<[email protected]>
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<[email protected]>

---
  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(-)

arch/arm/mach-omap2/omap_hwmod.c +0 -0

index 293fa6c..77d01a2 100644

Hi Nishanth,

Quoted 47 lines · Nishanth

On 11:57-20110722, Felipe Balbi wrote:
[...]

Quoted 13 lines

arch/arm/mach-omap2/board-omap3beagle.c static void __init beagle_opp_init(void) · line 497

Quoted 10 lines

arch/arm/mach-omap2/board-omap3beagle.c +3 -4

 	/* 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.

Quoted 6 lines

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<[email protected]>
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<[email protected]>

---
 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(-)

arch/arm/mach-omap2/omap_hwmod.c +0 -0

index 293fa6c..77d01a2 100644

arch/arm/mach-omap2/omap_hwmod.c line 145

Quoted 4 lines

arch/arm/mach-omap2/omap_hwmod.c +1 -0

@@ -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.

Quoted 8 lines

arch/arm/mach-omap2/omap_hwmod.c +0 -0

index 293fa6c..77d01a2 100644
@@ -142,6 +142,7 @@

?#include "powerdomain.h"
?#include<plat/clock.h>

Quoted 2 lines

?#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.

On Thu, Jul 28, 2011 at 07:57, Cousson, Benoit <[email protected]> wrote:
[...]

Quoted 8 lines

Quoted 8 lines

arch/arm/mach-omap2/omap_hwmod.c +0 -0

index 293fa6c..77d01a2 100644
@@ -142,6 +142,7 @@

?#include "powerdomain.h"
?#include<plat/clock.h>

Quoted 8 lines

Quoted 2 lines

?#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.

Quoted 4 lines

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.

From f03490456e24f72ca5272303c95a6f0b212494d5 Mon Sep 17 00:00:00 2001
From: Nishanth Menon <[email protected]>
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 <[email protected]>

---
 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(-)

arch/arm/plat-omap/include/plat/omap_device.h +1 -0

index 70d31d0..7a3c046 100644
@@ -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,

arch/arm/plat-omap/omap_device.c +21 -0

index 92b4496..21df532 100644
@@ -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);

Review

Quoted 3 lines

(quoted diff) +2 -0

+}
+EXPORT_SYMBOL(omap_hwmod_name_get_odev);

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

Quoted 6 lines · Felipe Balbi

Review

Quoted 3 lines

(quoted diff) +2 -0

+}
+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.

Hi,

Quoted 21 lines · 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 <[email protected]>

---
?arch/arm/mach-omap2/board-omap3beagle.c | ? 23 ++++++++++------------- ?1 files changed, 10 insertions(+), 13 deletions(-)

arch/arm/mach-omap2/board-omap3beagle.c +0 -0

index 32f5f89..3ae16b4 100644
@@ -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;

Quoted 3 lines

- ? ? ? ? ? ? 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

Hi,

Quoted 21 lines · 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 <[email protected]>

---
?arch/arm/mach-omap2/board-omap3beagle.c | ? 23 ++++++++++------------- ?1 files changed, 10 insertions(+), 13 deletions(-)

arch/arm/mach-omap2/board-omap3beagle.c +0 -0

index 32f5f89..3ae16b4 100644
@@ -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;

Quoted 3 lines

- ? ? ? ? ? ? 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

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 <[email protected]>

---
 arch/arm/plat-omap/mcbsp.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

arch/arm/plat-omap/mcbsp.c +0 -0

index 6c62af1..02199e8 100644

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<[email protected]>

---
  arch/arm/plat-omap/mcbsp.c |    6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)

arch/arm/plat-omap/mcbsp.c +0 -0

index 6c62af1..02199e8 100644
@@ -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)

Quoted 2 lines · Kevin Hilman

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

You mean from platform_device to omap_device?

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 <[email protected]>

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(-)

arch/arm/mach-omap2/opp.c +1 -1

index ab8b35b..9262a6b 100644
@@ -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) {

arch/arm/plat-omap/include/plat/omap_device.h +0 -0

index 7a3ec4f..6bd4f6f 100644
@@ -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;

arch/arm/plat-omap/include/plat/omap_device.h +0 -0

@@ -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 */

arch/arm/plat-omap/include/plat/omap_device.h +1 -1

-#define to_omap_device(x) container_of((x), struct omap_device, pdev)
+struct omap_device *to_omap_device(struct platform_device *pdev);

?#endif

arch/arm/plat-omap/omap_device.c +2 -2

index c420b94..52ce013 100644
@@ -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);

arch/arm/plat-omap/omap_device.c +1 -1

@@ -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,

arch/arm/plat-omap/omap_device.c +2 -2

@@ -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);

arch/arm/plat-omap/omap_device.c +1 -1

@@ -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,

arch/arm/plat-omap/omap_device.c +6 -6

@@ -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;
?}

arch/arm/plat-omap/omap_device.c +15 -0

@@ -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

arch/arm/plat-omap/omap_device.c +16 -16

@@ -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:

drivers/base/dd.c +1 -2

index 6658da7..9289dac 100644
@@ -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

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.

Quoted 19 lines · 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.

Quoted 6 lines

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.

Quoted 27 lines · Russell King - ARM Linux

Quoted 19 lines · 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.

Quoted 6 lines

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.

Quoted 5 lines

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.

Quoted 4 lines

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.

Quoted 34 lines · Grant Likely

Quoted 27 lines · Russell King - ARM Linux

Quoted 19 lines · 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.

Quoted 6 lines

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.

Quoted 5 lines

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.

Quoted 4 lines

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.

Quoted 5 lines

Quoted 4 lines

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.

Quoted 3 lines

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.

Quoted 70 lines · Russell King - ARM Linux

Quoted 34 lines · Grant Likely

Quoted 27 lines · Russell King - ARM Linux

Quoted 19 lines · 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.

Quoted 6 lines

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.

Quoted 5 lines

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.

Quoted 4 lines

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.

Quoted 5 lines

Quoted 4 lines

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.

Quoted 7 lines

Quoted 3 lines

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:

arch/arm/include/asm/device.h +0 -0

index 9f390ce..bb777cd 100644

Quoted 91 lines · Kevin Hilman

Quoted 70 lines · Russell King - ARM Linux

Quoted 34 lines · Grant Likely

Quoted 27 lines · Russell King - ARM Linux

Quoted 19 lines · 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.

Quoted 6 lines

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.

Quoted 5 lines

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.

Quoted 4 lines

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.

Quoted 5 lines

Quoted 4 lines

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.

Quoted 7 lines

Quoted 3 lines

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:

arch/arm/include/asm/device.h +0 -0

index 9f390ce..bb777cd 100644
@@ -13,6 +13,7 @@ struct dev_archdata {
 ?};
 

arch/arm/include/asm/device.h struct dev_archdata { · line 17

Quoted 3 lines

arch/arm/include/asm/device.h +1 -0

 ?struct pdev_archdata {
+ ? ? ? void *p;
 ?};

struct omap_device *p;

Otherwise it is just asking for type safety problems.

Hi,

Quoted 14 lines · Grant Likely

arch/arm/include/asm/device.h struct dev_archdata { · line 17

Quoted 10 lines · Kevin Hilman

arch/arm/include/asm/device.h +1 -0

index 9f390ce..bb777cd 100644
@@ -13,6 +13,7 @@ struct dev_archdata {
 ?};
 
 ?struct pdev_archdata {
+ ? ? ? void *p;
 ?};

struct omap_device *p;

Quoted 1 line

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:

(quoted diff) +4 -0

+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.

Quoted 11 lines · Russell King - ARM Linux

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

(quoted diff) +4 -0

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

Quoted 1 line

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

OK, will do it this way.

Hi,

Quoted 13 lines · Kevin Hilman

Quoted 11 lines · Russell King - ARM Linux

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

(quoted diff) +4 -0

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

Quoted 3 lines

Quoted 1 line

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 ?

Quoted 20 lines · Felipe Balbi

Hi,

Quoted 16 lines · Kevin Hilman

Quoted 11 lines · Russell King - ARM Linux

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

(quoted diff) +4 -0

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

Quoted 1 line

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

OK, will do it this way.

Quoted 2 lines

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.

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,

Quoted 7 lines · 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.

Quoted 4 lines

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

Hi Manjunath,

Quoted 10 lines · Manjunath Kondaiah

Kevin,

Quoted 7 lines · 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.

Quoted 6 lines

Quoted 4 lines

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

Quoted 37 lines · Kevin

Hi Manjunath,

Quoted 10 lines · Manjunath Kondaiah

Kevin,

Quoted 7 lines · 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.

Quoted 6 lines

Quoted 4 lines

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

Quoted 1 line

[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.