[PATCH v3 0/4] platform: Facilitate the creation of pseudo-platform buses

[PATCH v3 0/4] platform: Facilitate the creation of pseudo-platform buses

[Patrick Pannuto]

Most of the interesting stuff is in patch 2/4 and 3/4, inline.

    (lkml.org seems to have lost August 3rd...)
    RFC: http://lkml.indiana.edu/hypermail/linux/kernel/1008.0/01342.html
     v1: http://lkml.indiana.edu/hypermail/linux/kernel/1008.0/01942.html
     v2: http://lkml.indiana.edu/hypermail/linux/kernel/1008.1/00958.html

[PATCH 1/4] platform: Use drv->driver.bus instead of assuming platform_bus_type
    * This patch is already in greg k-h's queue, included only for clarity

[PATCH 2/4] platform: Facilitate the creation of pseudo-platform buses
    * This patch has changed pretty significantly, see its changelog

[PATCH 3/4] msm-bus: Define the msm-bus skeleton
    * This patch is a sample user of the new code, mostly proof-of-concept
      that defines a msm bus_type

[PATCH 4/4] msm: serial: Move msm_uart_driver onto msm bus
    * This patch simply moves a device/driver pair from the platform bus to
      the new msm bus

[PATCH 1/4] platform: Use drv->driver.bus instead of assuming platform_bus_type

[Patrick Pannuto]

In theory (although not *yet* in practice), a driver being passed
to platform_driver_probe might have driver.bus set to something
other than platform_bus_type. Locking drv->driver.bus is always
correct.

Change-Id: Ib015c35237eb5493d17a812576a3a9906e1344d4
Signed-off-by: Patrick Pannuto <ppannuto@codeaurora.org>
---
 drivers/base/platform.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4d99c8b..b69ccb4 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -539,12 +539,12 @@ int __init_or_module platform_driver_probe(struct platform_driver *drv,
 	 * if the probe was successful, and make sure any forced probes of
 	 * new devices fail.
 	 */
-	spin_lock(&platform_bus_type.p->klist_drivers.k_lock);
+	spin_lock(&drv->driver.bus->p->klist_drivers.k_lock);
 	drv->probe = NULL;
 	if (code == 0 && list_empty(&drv->driver.p->klist_devices.k_list))
 		retval = -ENODEV;
 	drv->driver.probe = platform_drv_probe_fail;
-	spin_unlock(&platform_bus_type.p->klist_drivers.k_lock);
+	spin_unlock(&drv->driver.bus->p->klist_drivers.k_lock);
 
 	if (code != retval)
 		platform_driver_unregister(drv);
-- 
1.7.2.1

Re: [PATCH 1/4] platform: Use drv->driver.bus instead of assuming platform_bus_type

[Greg KH]

On Wed, Aug 18, 2010 at 12:15:40PM -0700, Patrick Pannuto wrote:
> In theory (although not *yet* in practice), a driver being passed
> to platform_driver_probe might have driver.bus set to something
> other than platform_bus_type. Locking drv->driver.bus is always
> correct.
> 
> Change-Id: Ib015c35237eb5493d17a812576a3a9906e1344d4

What is that field?  Why did you add it?

confused,

greg k-h

Re: [PATCH 1/4] platform: Use drv->driver.bus instead of assuming platform_bus_type

[Patrick Pannuto]

On 08/18/2010 12:44 PM, Greg KH wrote:
> On Wed, Aug 18, 2010 at 12:15:40PM -0700, Patrick Pannuto wrote:
>> In theory (although not *yet* in practice), a driver being passed
>> to platform_driver_probe might have driver.bus set to something
>> other than platform_bus_type. Locking drv->driver.bus is always
>> correct.
>>
>> Change-Id: Ib015c35237eb5493d17a812576a3a9906e1344d4
> 
> What is that field?  Why did you add it?
> 
> confused,
> 
> greg k-h

Ugh... It's from gerrit; forgot to remove them, sorry.

Do you want a fresh set, or can you just ignore them for now?

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [PATCH 1/4] platform: Use drv->driver.bus instead of assuming platform_bus_type

[Greg KH]

On Wed, Aug 18, 2010 at 12:53:12PM -0700, Patrick Pannuto wrote:
> On 08/18/2010 12:44 PM, Greg KH wrote:
> > On Wed, Aug 18, 2010 at 12:15:40PM -0700, Patrick Pannuto wrote:
> >> In theory (although not *yet* in practice), a driver being passed
> >> to platform_driver_probe might have driver.bus set to something
> >> other than platform_bus_type. Locking drv->driver.bus is always
> >> correct.
> >>
> >> Change-Id: Ib015c35237eb5493d17a812576a3a9906e1344d4
> > 
> > What is that field?  Why did you add it?
> > 
> > confused,
> > 
> > greg k-h
> 
> Ugh... It's from gerrit; forgot to remove them, sorry.
> 
> Do you want a fresh set, or can you just ignore them for now?

I'll ignore them for now.

thanks,

greg k-h

Re: [PATCH 1/4] platform: Use drv->driver.bus instead of assuming platform_bus_type

[Greg KH]

On Wed, Aug 18, 2010 at 12:53:12PM -0700, Patrick Pannuto wrote:
> On 08/18/2010 12:44 PM, Greg KH wrote:
> > On Wed, Aug 18, 2010 at 12:15:40PM -0700, Patrick Pannuto wrote:
> >> In theory (although not *yet* in practice), a driver being passed
> >> to platform_driver_probe might have driver.bus set to something
> >> other than platform_bus_type. Locking drv->driver.bus is always
> >> correct.
> >>
> >> Change-Id: Ib015c35237eb5493d17a812576a3a9906e1344d4
> > 
> > What is that field?  Why did you add it?
> > 
> > confused,
> > 
> > greg k-h
> 
> Ugh... It's from gerrit; forgot to remove them, sorry.
> 
> Do you want a fresh set, or can you just ignore them for now?

Actually, can I get a fresh set now, as others have commented already.

thanks,

greg k-h

Re: [PATCH 1/4] platform: Use drv->driver.bus instead of assuming platform_bus_type

[David Brown]

On Wed, Sep 01, 2010 at 03:34:43PM -0700, Greg KH wrote:
> On Wed, Aug 18, 2010 at 12:53:12PM -0700, Patrick Pannuto wrote:
> > On 08/18/2010 12:44 PM, Greg KH wrote:
> > > On Wed, Aug 18, 2010 at 12:15:40PM -0700, Patrick Pannuto wrote:
> > >> In theory (although not *yet* in practice), a driver being passed
> > >> to platform_driver_probe might have driver.bus set to something
> > >> other than platform_bus_type. Locking drv->driver.bus is always
> > >> correct.
> > >>
> > >> Change-Id: Ib015c35237eb5493d17a812576a3a9906e1344d4
> > > 
> > > What is that field?  Why did you add it?
> > > 
> > > confused,
> > > 
> > > greg k-h
> > 
> > Ugh... It's from gerrit; forgot to remove them, sorry.
> > 
> > Do you want a fresh set, or can you just ignore them for now?
> 
> Actually, can I get a fresh set now, as others have commented already.

I'm going to need to take over these patches for Pat.  Give me a
little while to track down the correct versions and I can send them
out.

Thanks,
David

--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 2/4] platform: Facilitate the creation of pseudo-platform buses

[Patrick Pannuto]

(lkml.org seems to have lost August 3rd...)
RFC: http://lkml.indiana.edu/hypermail/linux/kernel/1008.0/01342.html
 v1: http://lkml.indiana.edu/hypermail/linux/kernel/1008.0/01942.html
 v2: http://lkml.indiana.edu/hypermail/linux/kernel/1008.1/00958.html

Based on the idea and code originally proposed by Kevin Hilman:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html

Changes from v2:
   * Remove the pseudo_platform_bus_[un]register functions
   * Split affected platform functions:
       - Platform functions are split into a wrapper (with the
         original function name) that retains the 'bus-setting'
         and 'parent-setting' duties, and then exports all the
         heavy-lifting to new __platform* counterparts
       - "Pseudo" buses work the same as the platform bus now,
         writing their own wrappers and calling the __platform
         functions
   * Export a *lot* of platform symbols to allow for intialization
     of "pseudo" platform bus types.
       - I personally like this model a lot, I think it is very
         clean from an implementation perspective, even if it
         does export a lot of new symbols
       - Thanks to Michal Miroslaw <mirqus@gmail.com> for the
         suggestion here
   * Add more use-cases justifying the need of platform-ish bus types

Changes from v1:

   * "Pseudo" buses are no longer init'd, they are [un]registered by:
       - pseudo_platform_bus_register(struct bus_type *)
       - pseudo_platform_bus_unregister(struct bus_type *)
   * These registrations [de]allocate a dev_pm_ops structure for the
     pseudo bus_type
   * Do not overwrite the parent if .bus is already set (that is, allow
     pseudo bus devices to be root devices)

   * Split into 2 patches:
       - 1/2: Already sent separately, but included here for clarity
       - 2/2: The real meat of the patch (this patch)

INTRO

As SOCs become more popular, the desire to quickly define a simple,
but functional, bus type with only a few unique properties becomes
desirable. As they become more complicated, the ability to nest these
simple busses and otherwise orchestrate them to match the actual
topology also becomes desirable.

Also, as power management becomes more challenging, grouping devices
on the same SOC under the same /logical bus/ provides a very natural
interface and location for power-management issues for that SOC.

This is a fairly natural extension of the "platform" bus; ultimately,
this patch series allows for the creation of multiple platform buses (SOC
buses), with the correct name and quirks for each of the "platforms" (SOCs)
they are trying to support.

EXAMPLE USAGE

See follow-on patches implementing the MSM bus and moving the
MSM uart device / driver onto it.

Change-Id: I10d2fa4671302e81be8f9cac01a3855cef4eeebf
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Signed-off-by: Patrick Pannuto <ppannuto@codeaurora.org>
---
 drivers/base/platform.c         |  184 ++++++++++++++++++++------------------
 include/linux/platform_device.h |   87 ++++++++++++++++++
 2 files changed, 184 insertions(+), 87 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index b69ccb4..e75640f 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -224,25 +224,10 @@ int platform_device_add_data(struct platform_device *pdev, const void *data,
 }
 EXPORT_SYMBOL_GPL(platform_device_add_data);
 
-/**
- * platform_device_add - add a platform device to device hierarchy
- * @pdev: platform device we're adding
- *
- * This is part 2 of platform_device_register(), though may be called
- * separately _iff_ pdev was allocated by platform_device_alloc().
- */
-int platform_device_add(struct platform_device *pdev)
+int __platform_device_add(struct platform_device *pdev)
 {
 	int i, ret = 0;
 
-	if (!pdev)
-		return -EINVAL;
-
-	if (!pdev->dev.parent)
-		pdev->dev.parent = &platform_bus;
-
-	pdev->dev.bus = &platform_bus_type;
-
 	if (pdev->id != -1)
 		dev_set_name(&pdev->dev, "%s.%d", pdev->name,  pdev->id);
 	else
@@ -289,6 +274,27 @@ int platform_device_add(struct platform_device *pdev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(__platform_device_add);
+
+/**
+ * platform_device_add - add a platform device to device hierarchy
+ * @pdev: platform device we're adding
+ *
+ * This is part 2 of platform_device_register(), though may be called
+ * separately _iff_ pdev was allocated by platform_device_alloc().
+ */
+int platform_device_add(struct platform_device *pdev)
+{
+	if (!pdev)
+		return -EINVAL;
+
+	if (!pdev->dev.parent)
+		pdev->dev.parent = &platform_bus;
+
+	pdev->dev.bus = &platform_bus_type;
+
+	return __platform_device_add(pdev);
+}
 EXPORT_SYMBOL_GPL(platform_device_add);
 
 /**
@@ -476,13 +482,8 @@ static void platform_drv_shutdown(struct device *_dev)
 	drv->shutdown(dev);
 }
 
-/**
- * platform_driver_register - register a driver for platform-level devices
- * @drv: platform driver structure
- */
-int platform_driver_register(struct platform_driver *drv)
+int __platform_driver_register(struct platform_driver *drv)
 {
-	drv->driver.bus = &platform_bus_type;
 	if (drv->probe)
 		drv->driver.probe = platform_drv_probe;
 	if (drv->remove)
@@ -492,6 +493,18 @@ int platform_driver_register(struct platform_driver *drv)
 
 	return driver_register(&drv->driver);
 }
+EXPORT_SYMBOL_GPL(__platform_driver_register);
+
+/**
+ * platform_driver_register - register a driver for platform-level devices
+ * @drv: platform driver structure
+ */
+int platform_driver_register(struct platform_driver *drv)
+{
+	drv->driver.bus = &platform_bus_type;
+
+	return __platform_driver_register(drv);
+}
 EXPORT_SYMBOL_GPL(platform_driver_register);
 
 /**
@@ -504,25 +517,9 @@ void platform_driver_unregister(struct platform_driver *drv)
 }
 EXPORT_SYMBOL_GPL(platform_driver_unregister);
 
-/**
- * platform_driver_probe - register driver for non-hotpluggable device
- * @drv: platform driver structure
- * @probe: the driver probe routine, probably from an __init section
- *
- * Use this instead of platform_driver_register() when you know the device
- * is not hotpluggable and has already been registered, and you want to
- * remove its run-once probe() infrastructure from memory after the driver
- * has bound to the device.
- *
- * One typical use for this would be with drivers for controllers integrated
- * into system-on-chip processors, where the controller devices have been
- * configured as part of board setup.
- *
- * Returns zero if the driver registered and bound to a device, else returns
- * a negative error code and with the driver not registered.
- */
-int __init_or_module platform_driver_probe(struct platform_driver *drv,
-		int (*probe)(struct platform_device *))
+int __init_or_module __platform_driver_probe(struct platform_driver *drv,
+		int (*probe)(struct platform_device *),
+		int (*bustype_driver_register)(struct platform_driver *))
 {
 	int retval, code;
 
@@ -531,7 +528,7 @@ int __init_or_module platform_driver_probe(struct platform_driver *drv,
 
 	/* temporary section violation during probe() */
 	drv->probe = probe;
-	retval = code = platform_driver_register(drv);
+	retval = code = bustype_driver_register(drv);
 
 	/*
 	 * Fixup that section violation, being paranoid about code scanning
@@ -550,6 +547,30 @@ int __init_or_module platform_driver_probe(struct platform_driver *drv,
 		platform_driver_unregister(drv);
 	return retval;
 }
+EXPORT_SYMBOL_GPL(__platform_driver_probe);
+
+/**
+ * platform_driver_probe - register driver for non-hotpluggable device
+ * @drv: platform driver structure
+ * @probe: the driver probe routine, probably from an __init section
+ *
+ * Use this instead of platform_driver_register() when you know the device
+ * is not hotpluggable and has already been registered, and you want to
+ * remove its run-once probe() infrastructure from memory after the driver
+ * has bound to the device.
+ *
+ * One typical use for this would be with drivers for controllers integrated
+ * into system-on-chip processors, where the controller devices have been
+ * configured as part of board setup.
+ *
+ * Returns zero if the driver registered and bound to a device, else returns
+ * a negative error code and with the driver not registered.
+ */
+int __init_or_module platform_driver_probe(struct platform_driver *drv,
+		int (*probe)(struct platform_device *))
+{
+	return __platform_driver_probe(drv, probe, &platform_driver_register);
+}
 EXPORT_SYMBOL_GPL(platform_driver_probe);
 
 /**
@@ -627,12 +648,13 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
 	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
 }
 
-static struct device_attribute platform_dev_attrs[] = {
+struct device_attribute platform_dev_attrs[] = {
 	__ATTR_RO(modalias),
 	__ATTR_NULL,
 };
+EXPORT_SYMBOL_GPL(platform_dev_attrs);
 
-static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
+int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
 	struct platform_device	*pdev = to_platform_device(dev);
 
@@ -640,6 +662,7 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
 		(pdev->id_entry) ? pdev->id_entry->name : pdev->name);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(platform_uevent);
 
 static const struct platform_device_id *platform_match_id(
 			const struct platform_device_id *id,
@@ -668,7 +691,7 @@ static const struct platform_device_id *platform_match_id(
  * and compare it against the name of the driver. Return whether they match
  * or not.
  */
-static int platform_match(struct device *dev, struct device_driver *drv)
+int platform_match(struct device *dev, struct device_driver *drv)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct platform_driver *pdrv = to_platform_driver(drv);
@@ -680,10 +703,11 @@ static int platform_match(struct device *dev, struct device_driver *drv)
 	/* fall-back to driver name match */
 	return (strcmp(pdev->name, drv->name) == 0);
 }
+EXPORT_SYMBOL_GPL(platform_match);
 
 #ifdef CONFIG_PM_SLEEP
 
-static int platform_legacy_suspend(struct device *dev, pm_message_t mesg)
+int platform_legacy_suspend(struct device *dev, pm_message_t mesg)
 {
 	struct platform_driver *pdrv = to_platform_driver(dev->driver);
 	struct platform_device *pdev = to_platform_device(dev);
@@ -695,7 +719,7 @@ static int platform_legacy_suspend(struct device *dev, pm_message_t mesg)
 	return ret;
 }
 
-static int platform_legacy_resume(struct device *dev)
+int platform_legacy_resume(struct device *dev)
 {
 	struct platform_driver *pdrv = to_platform_driver(dev->driver);
 	struct platform_device *pdev = to_platform_device(dev);
@@ -707,7 +731,7 @@ static int platform_legacy_resume(struct device *dev)
 	return ret;
 }
 
-static int platform_pm_prepare(struct device *dev)
+int platform_pm_prepare(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -717,19 +741,16 @@ static int platform_pm_prepare(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_prepare);
 
-static void platform_pm_complete(struct device *dev)
+void platform_pm_complete(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 
 	if (drv && drv->pm && drv->pm->complete)
 		drv->pm->complete(dev);
 }
-
-#else /* !CONFIG_PM_SLEEP */
-
-#define platform_pm_prepare		NULL
-#define platform_pm_complete		NULL
+EXPORT_SYMBOL_GPL(platform_pm_complete);
 
 #endif /* !CONFIG_PM_SLEEP */
 
@@ -752,6 +773,7 @@ int __weak platform_pm_suspend(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_suspend);
 
 int __weak platform_pm_suspend_noirq(struct device *dev)
 {
@@ -768,6 +790,7 @@ int __weak platform_pm_suspend_noirq(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_suspend_noirq);
 
 int __weak platform_pm_resume(struct device *dev)
 {
@@ -786,6 +809,7 @@ int __weak platform_pm_resume(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_resume);
 
 int __weak platform_pm_resume_noirq(struct device *dev)
 {
@@ -802,19 +826,13 @@ int __weak platform_pm_resume_noirq(struct device *dev)
 
 	return ret;
 }
-
-#else /* !CONFIG_SUSPEND */
-
-#define platform_pm_suspend		NULL
-#define platform_pm_resume		NULL
-#define platform_pm_suspend_noirq	NULL
-#define platform_pm_resume_noirq	NULL
+EXPORT_SYMBOL_GPL(platform_pm_resume_noirq);
 
 #endif /* !CONFIG_SUSPEND */
 
 #ifdef CONFIG_HIBERNATION
 
-static int platform_pm_freeze(struct device *dev)
+int platform_pm_freeze(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -831,8 +849,9 @@ static int platform_pm_freeze(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_freeze);
 
-static int platform_pm_freeze_noirq(struct device *dev)
+int platform_pm_freeze_noirq(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -847,8 +866,9 @@ static int platform_pm_freeze_noirq(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_freeze_noirq);
 
-static int platform_pm_thaw(struct device *dev)
+int platform_pm_thaw(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -865,8 +885,9 @@ static int platform_pm_thaw(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_thaw);
 
-static int platform_pm_thaw_noirq(struct device *dev)
+int platform_pm_thaw_noirq(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -881,8 +902,9 @@ static int platform_pm_thaw_noirq(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_thaw_noirq);
 
-static int platform_pm_poweroff(struct device *dev)
+int platform_pm_poweroff(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -899,8 +921,9 @@ static int platform_pm_poweroff(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_poweroff);
 
-static int platform_pm_poweroff_noirq(struct device *dev)
+int platform_pm_poweroff_noirq(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -915,8 +938,9 @@ static int platform_pm_poweroff_noirq(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_poweroff_noirq);
 
-static int platform_pm_restore(struct device *dev)
+int platform_pm_restore(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -933,8 +957,9 @@ static int platform_pm_restore(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_restore);
 
-static int platform_pm_restore_noirq(struct device *dev)
+int platform_pm_restore_noirq(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -949,17 +974,7 @@ static int platform_pm_restore_noirq(struct device *dev)
 
 	return ret;
 }
-
-#else /* !CONFIG_HIBERNATION */
-
-#define platform_pm_freeze		NULL
-#define platform_pm_thaw		NULL
-#define platform_pm_poweroff		NULL
-#define platform_pm_restore		NULL
-#define platform_pm_freeze_noirq	NULL
-#define platform_pm_thaw_noirq		NULL
-#define platform_pm_poweroff_noirq	NULL
-#define platform_pm_restore_noirq	NULL
+EXPORT_SYMBOL_GPL(platform_pm_restore_noirq);
 
 #endif /* !CONFIG_HIBERNATION */
 
@@ -980,15 +995,9 @@ int __weak platform_pm_runtime_idle(struct device *dev)
 	return pm_generic_runtime_idle(dev);
 };
 
-#else /* !CONFIG_PM_RUNTIME */
-
-#define platform_pm_runtime_suspend NULL
-#define platform_pm_runtime_resume NULL
-#define platform_pm_runtime_idle NULL
-
 #endif /* !CONFIG_PM_RUNTIME */
 
-static const struct dev_pm_ops platform_dev_pm_ops = {
+const struct dev_pm_ops platform_dev_pm_ops = {
 	.prepare = platform_pm_prepare,
 	.complete = platform_pm_complete,
 	.suspend = platform_pm_suspend,
@@ -1007,6 +1016,7 @@ static const struct dev_pm_ops platform_dev_pm_ops = {
 	.runtime_resume = platform_pm_runtime_resume,
 	.runtime_idle = platform_pm_runtime_idle,
 };
+EXPORT_SYMBOL_GPL(platform_dev_pm_ops);
 
 struct bus_type platform_bus_type = {
 	.name		= "platform",
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 5417944..6d7d399 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -53,6 +53,7 @@ extern int platform_device_add_resources(struct platform_device *pdev,
 					 const struct resource *res,
 					 unsigned int num);
 extern int platform_device_add_data(struct platform_device *pdev, const void *data, size_t size);
+extern int __platform_device_add(struct platform_device *pdev);
 extern int platform_device_add(struct platform_device *pdev);
 extern void platform_device_del(struct platform_device *pdev);
 extern void platform_device_put(struct platform_device *pdev);
@@ -67,12 +68,16 @@ struct platform_driver {
 	const struct platform_device_id *id_table;
 };
 
+extern int __platform_driver_register(struct platform_driver *);
 extern int platform_driver_register(struct platform_driver *);
 extern void platform_driver_unregister(struct platform_driver *);
 
 /* non-hotpluggable platform devices may use this so that probe() and
  * its support may live in __init sections, conserving runtime memory.
  */
+extern int __platform_driver_probe(struct platform_driver *driver,
+		int (*probe)(struct platform_device *),
+		int (*bustype_driver_register)(struct platform_driver *));
 extern int platform_driver_probe(struct platform_driver *driver,
 		int (*probe)(struct platform_device *));
 
@@ -84,6 +89,88 @@ extern struct platform_device *platform_create_bundle(struct platform_driver *dr
 					struct resource *res, unsigned int n_res,
 					const void *data, size_t size);
 
+extern struct device_attribute platform_dev_attrs[];
+extern int platform_uevent(struct device *dev, struct kobj_uevent_env *env);
+extern int platform_match(struct device *dev, struct device_driver *drv);
+
+#ifdef CONFIG_PM_SLEEP
+extern int platform_pm_prepare(struct device *dev);
+extern void platform_pm_complete(struct device *dev);
+#else  /* !CONFIG_PM_SLEEP */
+#define platform_pm_prepare		NULL
+#define platform_pm_complete		NULL
+#endif /* !CONFIG_PM_SLEEP */
+
+#ifdef CONFIG_SUSPEND
+extern int __weak platform_pm_suspend(struct device *dev);
+extern int __weak platform_pm_suspend_noirq(struct device *dev);
+extern int __weak platform_pm_resume(struct device *dev);
+extern int __weak platform_pm_resume_noirq(struct device *dev);
+#else  /* !CONFIG_SUSPEND */
+#define platform_pm_suspend		NULL
+#define platform_pm_resume		NULL
+#define platform_pm_suspend_noirq	NULL
+#define platform_pm_resume_noirq	NULL
+#endif /* !CONFIG_SUSPEND */
+
+#ifdef CONFIG_HIBERNATION
+extern int platform_pm_freeze(struct device *dev);
+extern int platform_pm_freeze_noirq(struct device *dev);
+extern int platform_pm_thaw(struct device *dev);
+extern int platform_pm_thaw_noirq(struct device *dev);
+extern int platform_pm_poweroff(struct device *dev);
+extern int platform_pm_poweroff_noirq(struct device *dev);
+extern int platform_pm_restore(struct device *dev);
+extern int platform_pm_restore_noirq(struct device *dev);
+#else  /* !CONFIG_HIBERNATION */
+#define platform_pm_freeze		NULL
+#define platform_pm_thaw		NULL
+#define platform_pm_poweroff		NULL
+#define platform_pm_restore		NULL
+#define platform_pm_freeze_noirq	NULL
+#define platform_pm_thaw_noirq		NULL
+#define platform_pm_poweroff_noirq	NULL
+#define platform_pm_restore_noirq	NULL
+#endif /* !CONFIG_HIBERNATION */
+
+#ifdef CONFIG_PM_RUNTIME
+extern int __weak platform_pm_runtime_suspend(struct device *dev);
+extern int __weak platform_pm_runtime_resume(struct device *dev);
+extern int __weak platform_pm_runtime_idle(struct device *dev);
+#else  /* !CONFIG_PM_RUNTIME */
+#define platform_pm_runtime_suspend NULL
+#define platform_pm_runtime_resume NULL
+#define platform_pm_runtime_idle NULL
+#endif /* !CONFIG_PM_RUNTIME */
+
+extern const struct dev_pm_ops platform_dev_pm_ops;
+
+#define PLATFORM_BUS_TEMPLATE \
+	.name		= "XXX_OVERRIDEME_XXX",	\
+	.dev_attrs	= platform_dev_attrs,	\
+	.match		= platform_match,	\
+	.uevent		= platform_uevent,	\
+	.pm		= &platform_dev_pm_ops
+
+#define PLATFORM_PM_OPS_TEMPLATE \
+	.prepare = platform_pm_prepare,			\
+	.complete = platform_pm_complete,		\
+	.suspend = platform_pm_suspend,			\
+	.resume = platform_pm_resume,			\
+	.freeze = platform_pm_freeze,			\
+	.thaw = platform_pm_thaw,			\
+	.poweroff = platform_pm_poweroff,		\
+	.restore = platform_pm_restore,			\
+	.suspend_noirq = platform_pm_suspend_noirq,	\
+	.resume_noirq = platform_pm_resume_noirq,	\
+	.freeze_noirq = platform_pm_freeze_noirq,	\
+	.thaw_noirq = platform_pm_thaw_noirq,		\
+	.poweroff_noirq = platform_pm_poweroff_noirq,	\
+	.restore_noirq = platform_pm_restore_noirq,	\
+	.runtime_suspend = platform_pm_runtime_suspend,	\
+	.runtime_resume = platform_pm_runtime_resume,	\
+	.runtime_idle = platform_pm_runtime_idle
+
 /* early platform driver interface */
 struct early_platform_driver {
 	const char *class_str;
-- 
1.7.2.1

Re: [PATCH 2/4] platform: Facilitate the creation of pseudo-platform buses

[Grant Likely]

Hi Patrick,

On Wed, Aug 18, 2010 at 1:15 PM, Patrick Pannuto
<ppannuto@codeaurora.org> wrote:
> (lkml.org seems to have lost August 3rd...)
> RFC: http://lkml.indiana.edu/hypermail/linux/kernel/1008.0/01342.html
>  v1: http://lkml.indiana.edu/hypermail/linux/kernel/1008.0/01942.html
>  v2: http://lkml.indiana.edu/hypermail/linux/kernel/1008.1/00958.html
>
> Based on the idea and code originally proposed by Kevin Hilman:
> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html
>
> Changes from v2:
>   * Remove the pseudo_platform_bus_[un]register functions
>   * Split affected platform functions:
>       - Platform functions are split into a wrapper (with the
>         original function name) that retains the 'bus-setting'
>         and 'parent-setting' duties, and then exports all the
>         heavy-lifting to new __platform* counterparts
>       - "Pseudo" buses work the same as the platform bus now,
>         writing their own wrappers and calling the __platform
>         functions
>   * Export a *lot* of platform symbols to allow for intialization
>     of "pseudo" platform bus types.
>       - I personally like this model a lot, I think it is very
>         clean from an implementation perspective, even if it
>         does export a lot of new symbols
>       - Thanks to Michal Miroslaw <mirqus@gmail.com> for the
>         suggestion here

I'm not quite as hot on this approach, mostly because it is a lot more
invasive to the namespace than just exposing an initialization
routine.  However, I leave that as a point of 'taste' and Greg can
decide.  :-)

Otherwise, the patch looks good.  As I commented on the other patch, I
still want to see a real use case (ie, a bus with different behaviour)
before casting my vote.

One more comment below...

>   * Add more use-cases justifying the need of platform-ish bus types
>
> Changes from v1:
>
>   * "Pseudo" buses are no longer init'd, they are [un]registered by:
>       - pseudo_platform_bus_register(struct bus_type *)
>       - pseudo_platform_bus_unregister(struct bus_type *)
>   * These registrations [de]allocate a dev_pm_ops structure for the
>     pseudo bus_type
>   * Do not overwrite the parent if .bus is already set (that is, allow
>     pseudo bus devices to be root devices)
>
>   * Split into 2 patches:
>       - 1/2: Already sent separately, but included here for clarity
>       - 2/2: The real meat of the patch (this patch)
>
> INTRO
>
> As SOCs become more popular, the desire to quickly define a simple,
> but functional, bus type with only a few unique properties becomes
> desirable. As they become more complicated, the ability to nest these
> simple busses and otherwise orchestrate them to match the actual
> topology also becomes desirable.
>
> Also, as power management becomes more challenging, grouping devices
> on the same SOC under the same /logical bus/ provides a very natural
> interface and location for power-management issues for that SOC.
>
> This is a fairly natural extension of the "platform" bus; ultimately,
> this patch series allows for the creation of multiple platform buses (SOC
> buses), with the correct name and quirks for each of the "platforms" (SOCs)
> they are trying to support.
>
> EXAMPLE USAGE
>
> See follow-on patches implementing the MSM bus and moving the
> MSM uart device / driver onto it.
>
> Change-Id: I10d2fa4671302e81be8f9cac01a3855cef4eeebf
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> Signed-off-by: Patrick Pannuto <ppannuto@codeaurora.org>
> ---
>  drivers/base/platform.c         |  184 ++++++++++++++++++++------------------
>  include/linux/platform_device.h |   87 ++++++++++++++++++
>  2 files changed, 184 insertions(+), 87 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index b69ccb4..e75640f 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -224,25 +224,10 @@ int platform_device_add_data(struct platform_device *pdev, const void *data,
>  }
>  EXPORT_SYMBOL_GPL(platform_device_add_data);
>
> -/**
> - * platform_device_add - add a platform device to device hierarchy
> - * @pdev: platform device we're adding
> - *
> - * This is part 2 of platform_device_register(), though may be called
> - * separately _iff_ pdev was allocated by platform_device_alloc().
> - */
> -int platform_device_add(struct platform_device *pdev)
> +int __platform_device_add(struct platform_device *pdev)
>  {
>        int i, ret = 0;
>
> -       if (!pdev)
> -               return -EINVAL;
> -
> -       if (!pdev->dev.parent)
> -               pdev->dev.parent = &platform_bus;
> -
> -       pdev->dev.bus = &platform_bus_type;
> -
>        if (pdev->id != -1)
>                dev_set_name(&pdev->dev, "%s.%d", pdev->name,  pdev->id);
>        else
> @@ -289,6 +274,27 @@ int platform_device_add(struct platform_device *pdev)
>
>        return ret;
>  }
> +EXPORT_SYMBOL_GPL(__platform_device_add);
> +
> +/**
> + * platform_device_add - add a platform device to device hierarchy
> + * @pdev: platform device we're adding
> + *
> + * This is part 2 of platform_device_register(), though may be called
> + * separately _iff_ pdev was allocated by platform_device_alloc().
> + */
> +int platform_device_add(struct platform_device *pdev)
> +{
> +       if (!pdev)
> +               return -EINVAL;
> +
> +       if (!pdev->dev.parent)
> +               pdev->dev.parent = &platform_bus;
> +
> +       pdev->dev.bus = &platform_bus_type;
> +
> +       return __platform_device_add(pdev);
> +}
>  EXPORT_SYMBOL_GPL(platform_device_add);
>
>  /**
> @@ -476,13 +482,8 @@ static void platform_drv_shutdown(struct device *_dev)
>        drv->shutdown(dev);
>  }
>
> -/**
> - * platform_driver_register - register a driver for platform-level devices
> - * @drv: platform driver structure
> - */
> -int platform_driver_register(struct platform_driver *drv)
> +int __platform_driver_register(struct platform_driver *drv)
>  {
> -       drv->driver.bus = &platform_bus_type;
>        if (drv->probe)
>                drv->driver.probe = platform_drv_probe;
>        if (drv->remove)
> @@ -492,6 +493,18 @@ int platform_driver_register(struct platform_driver *drv)
>
>        return driver_register(&drv->driver);
>  }
> +EXPORT_SYMBOL_GPL(__platform_driver_register);
> +
> +/**
> + * platform_driver_register - register a driver for platform-level devices
> + * @drv: platform driver structure
> + */
> +int platform_driver_register(struct platform_driver *drv)
> +{
> +       drv->driver.bus = &platform_bus_type;
> +
> +       return __platform_driver_register(drv);
> +}
>  EXPORT_SYMBOL_GPL(platform_driver_register);
>
>  /**
> @@ -504,25 +517,9 @@ void platform_driver_unregister(struct platform_driver *drv)
>  }
>  EXPORT_SYMBOL_GPL(platform_driver_unregister);
>
> -/**
> - * platform_driver_probe - register driver for non-hotpluggable device
> - * @drv: platform driver structure
> - * @probe: the driver probe routine, probably from an __init section
> - *
> - * Use this instead of platform_driver_register() when you know the device
> - * is not hotpluggable and has already been registered, and you want to
> - * remove its run-once probe() infrastructure from memory after the driver
> - * has bound to the device.
> - *
> - * One typical use for this would be with drivers for controllers integrated
> - * into system-on-chip processors, where the controller devices have been
> - * configured as part of board setup.
> - *
> - * Returns zero if the driver registered and bound to a device, else returns
> - * a negative error code and with the driver not registered.
> - */
> -int __init_or_module platform_driver_probe(struct platform_driver *drv,
> -               int (*probe)(struct platform_device *))
> +int __init_or_module __platform_driver_probe(struct platform_driver *drv,
> +               int (*probe)(struct platform_device *),
> +               int (*bustype_driver_register)(struct platform_driver *))

Personally, unless you already have a use-case for using the
platform_driver_probe() hook on the custom busses, I would just leave
this one unimplemented.  It isn't critical (and I don't like it, but
that's a rant for another email).

>  {
>        int retval, code;
>
> @@ -531,7 +528,7 @@ int __init_or_module platform_driver_probe(struct platform_driver *drv,
>
>        /* temporary section violation during probe() */
>        drv->probe = probe;
> -       retval = code = platform_driver_register(drv);
> +       retval = code = bustype_driver_register(drv);
>
>        /*
>         * Fixup that section violation, being paranoid about code scanning
> @@ -550,6 +547,30 @@ int __init_or_module platform_driver_probe(struct platform_driver *drv,
>                platform_driver_unregister(drv);
>        return retval;
>  }
> +EXPORT_SYMBOL_GPL(__platform_driver_probe);
> +
> +/**
> + * platform_driver_probe - register driver for non-hotpluggable device
> + * @drv: platform driver structure
> + * @probe: the driver probe routine, probably from an __init section
> + *
> + * Use this instead of platform_driver_register() when you know the device
> + * is not hotpluggable and has already been registered, and you want to
> + * remove its run-once probe() infrastructure from memory after the driver
> + * has bound to the device.
> + *
> + * One typical use for this would be with drivers for controllers integrated
> + * into system-on-chip processors, where the controller devices have been
> + * configured as part of board setup.
> + *
> + * Returns zero if the driver registered and bound to a device, else returns
> + * a negative error code and with the driver not registered.
> + */
> +int __init_or_module platform_driver_probe(struct platform_driver *drv,
> +               int (*probe)(struct platform_device *))
> +{
> +       return __platform_driver_probe(drv, probe, &platform_driver_register);
> +}
>  EXPORT_SYMBOL_GPL(platform_driver_probe);
>
>  /**
> @@ -627,12 +648,13 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
>        return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
>  }
>
> -static struct device_attribute platform_dev_attrs[] = {
> +struct device_attribute platform_dev_attrs[] = {
>        __ATTR_RO(modalias),
>        __ATTR_NULL,
>  };
> +EXPORT_SYMBOL_GPL(platform_dev_attrs);
>
> -static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
> +int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
>  {
>        struct platform_device  *pdev = to_platform_device(dev);
>
> @@ -640,6 +662,7 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
>                (pdev->id_entry) ? pdev->id_entry->name : pdev->name);
>        return 0;
>  }
> +EXPORT_SYMBOL_GPL(platform_uevent);
>
>  static const struct platform_device_id *platform_match_id(
>                        const struct platform_device_id *id,
> @@ -668,7 +691,7 @@ static const struct platform_device_id *platform_match_id(
>  * and compare it against the name of the driver. Return whether they match
>  * or not.
>  */
> -static int platform_match(struct device *dev, struct device_driver *drv)
> +int platform_match(struct device *dev, struct device_driver *drv)
>  {
>        struct platform_device *pdev = to_platform_device(dev);
>        struct platform_driver *pdrv = to_platform_driver(drv);
> @@ -680,10 +703,11 @@ static int platform_match(struct device *dev, struct device_driver *drv)
>        /* fall-back to driver name match */
>        return (strcmp(pdev->name, drv->name) == 0);
>  }
> +EXPORT_SYMBOL_GPL(platform_match);
>
>  #ifdef CONFIG_PM_SLEEP
>
> -static int platform_legacy_suspend(struct device *dev, pm_message_t mesg)
> +int platform_legacy_suspend(struct device *dev, pm_message_t mesg)
>  {
>        struct platform_driver *pdrv = to_platform_driver(dev->driver);
>        struct platform_device *pdev = to_platform_device(dev);
> @@ -695,7 +719,7 @@ static int platform_legacy_suspend(struct device *dev, pm_message_t mesg)
>        return ret;
>  }
>
> -static int platform_legacy_resume(struct device *dev)
> +int platform_legacy_resume(struct device *dev)
>  {
>        struct platform_driver *pdrv = to_platform_driver(dev->driver);
>        struct platform_device *pdev = to_platform_device(dev);
> @@ -707,7 +731,7 @@ static int platform_legacy_resume(struct device *dev)
>        return ret;
>  }
>
> -static int platform_pm_prepare(struct device *dev)
> +int platform_pm_prepare(struct device *dev)
>  {
>        struct device_driver *drv = dev->driver;
>        int ret = 0;
> @@ -717,19 +741,16 @@ static int platform_pm_prepare(struct device *dev)
>
>        return ret;
>  }
> +EXPORT_SYMBOL_GPL(platform_pm_prepare);
>
> -static void platform_pm_complete(struct device *dev)
> +void platform_pm_complete(struct device *dev)
>  {
>        struct device_driver *drv = dev->driver;
>
>        if (drv && drv->pm && drv->pm->complete)
>                drv->pm->complete(dev);
>  }
> -
> -#else /* !CONFIG_PM_SLEEP */
> -
> -#define platform_pm_prepare            NULL
> -#define platform_pm_complete           NULL
> +EXPORT_SYMBOL_GPL(platform_pm_complete);
>
>  #endif /* !CONFIG_PM_SLEEP */
>
> @@ -752,6 +773,7 @@ int __weak platform_pm_suspend(struct device *dev)
>
>        return ret;
>  }
> +EXPORT_SYMBOL_GPL(platform_pm_suspend);
>
>  int __weak platform_pm_suspend_noirq(struct device *dev)
>  {
> @@ -768,6 +790,7 @@ int __weak platform_pm_suspend_noirq(struct device *dev)
>
>        return ret;
>  }
> +EXPORT_SYMBOL_GPL(platform_pm_suspend_noirq);
>
>  int __weak platform_pm_resume(struct device *dev)
>  {
> @@ -786,6 +809,7 @@ int __weak platform_pm_resume(struct device *dev)
>
>        return ret;
>  }
> +EXPORT_SYMBOL_GPL(platform_pm_resume);
>
>  int __weak platform_pm_resume_noirq(struct device *dev)
>  {
> @@ -802,19 +826,13 @@ int __weak platform_pm_resume_noirq(struct device *dev)
>
>        return ret;
>  }
> -
> -#else /* !CONFIG_SUSPEND */
> -
> -#define platform_pm_suspend            NULL
> -#define platform_pm_resume             NULL
> -#define platform_pm_suspend_noirq      NULL
> -#define platform_pm_resume_noirq       NULL
> +EXPORT_SYMBOL_GPL(platform_pm_resume_noirq);
>
>  #endif /* !CONFIG_SUSPEND */
>
>  #ifdef CONFIG_HIBERNATION
>
> -static int platform_pm_freeze(struct device *dev)
> +int platform_pm_freeze(struct device *dev)
>  {
>        struct device_driver *drv = dev->driver;
>        int ret = 0;
> @@ -831,8 +849,9 @@ static int platform_pm_freeze(struct device *dev)
>
>        return ret;
>  }
> +EXPORT_SYMBOL_GPL(platform_pm_freeze);
>
> -static int platform_pm_freeze_noirq(struct device *dev)
> +int platform_pm_freeze_noirq(struct device *dev)
>  {
>        struct device_driver *drv = dev->driver;
>        int ret = 0;
> @@ -847,8 +866,9 @@ static int platform_pm_freeze_noirq(struct device *dev)
>
>        return ret;
>  }
> +EXPORT_SYMBOL_GPL(platform_pm_freeze_noirq);
>
> -static int platform_pm_thaw(struct device *dev)
> +int platform_pm_thaw(struct device *dev)
>  {
>        struct device_driver *drv = dev->driver;
>        int ret = 0;
> @@ -865,8 +885,9 @@ static int platform_pm_thaw(struct device *dev)
>
>        return ret;
>  }
> +EXPORT_SYMBOL_GPL(platform_pm_thaw);
>
> -static int platform_pm_thaw_noirq(struct device *dev)
> +int platform_pm_thaw_noirq(struct device *dev)
>  {
>        struct device_driver *drv = dev->driver;
>        int ret = 0;
> @@ -881,8 +902,9 @@ static int platform_pm_thaw_noirq(struct device *dev)
>
>        return ret;
>  }
> +EXPORT_SYMBOL_GPL(platform_pm_thaw_noirq);
>
> -static int platform_pm_poweroff(struct device *dev)
> +int platform_pm_poweroff(struct device *dev)
>  {
>        struct device_driver *drv = dev->driver;
>        int ret = 0;
> @@ -899,8 +921,9 @@ static int platform_pm_poweroff(struct device *dev)
>
>        return ret;
>  }
> +EXPORT_SYMBOL_GPL(platform_pm_poweroff);
>
> -static int platform_pm_poweroff_noirq(struct device *dev)
> +int platform_pm_poweroff_noirq(struct device *dev)
>  {
>        struct device_driver *drv = dev->driver;
>        int ret = 0;
> @@ -915,8 +938,9 @@ static int platform_pm_poweroff_noirq(struct device *dev)
>
>        return ret;
>  }
> +EXPORT_SYMBOL_GPL(platform_pm_poweroff_noirq);
>
> -static int platform_pm_restore(struct device *dev)
> +int platform_pm_restore(struct device *dev)
>  {
>        struct device_driver *drv = dev->driver;
>        int ret = 0;
> @@ -933,8 +957,9 @@ static int platform_pm_restore(struct device *dev)
>
>        return ret;
>  }
> +EXPORT_SYMBOL_GPL(platform_pm_restore);
>
> -static int platform_pm_restore_noirq(struct device *dev)
> +int platform_pm_restore_noirq(struct device *dev)
>  {
>        struct device_driver *drv = dev->driver;
>        int ret = 0;
> @@ -949,17 +974,7 @@ static int platform_pm_restore_noirq(struct device *dev)
>
>        return ret;
>  }
> -
> -#else /* !CONFIG_HIBERNATION */
> -
> -#define platform_pm_freeze             NULL
> -#define platform_pm_thaw               NULL
> -#define platform_pm_poweroff           NULL
> -#define platform_pm_restore            NULL
> -#define platform_pm_freeze_noirq       NULL
> -#define platform_pm_thaw_noirq         NULL
> -#define platform_pm_poweroff_noirq     NULL
> -#define platform_pm_restore_noirq      NULL
> +EXPORT_SYMBOL_GPL(platform_pm_restore_noirq);
>
>  #endif /* !CONFIG_HIBERNATION */
>
> @@ -980,15 +995,9 @@ int __weak platform_pm_runtime_idle(struct device *dev)
>        return pm_generic_runtime_idle(dev);
>  };
>
> -#else /* !CONFIG_PM_RUNTIME */
> -
> -#define platform_pm_runtime_suspend NULL
> -#define platform_pm_runtime_resume NULL
> -#define platform_pm_runtime_idle NULL
> -
>  #endif /* !CONFIG_PM_RUNTIME */
>
> -static const struct dev_pm_ops platform_dev_pm_ops = {
> +const struct dev_pm_ops platform_dev_pm_ops = {
>        .prepare = platform_pm_prepare,
>        .complete = platform_pm_complete,
>        .suspend = platform_pm_suspend,
> @@ -1007,6 +1016,7 @@ static const struct dev_pm_ops platform_dev_pm_ops = {
>        .runtime_resume = platform_pm_runtime_resume,
>        .runtime_idle = platform_pm_runtime_idle,
>  };
> +EXPORT_SYMBOL_GPL(platform_dev_pm_ops);
>
>  struct bus_type platform_bus_type = {
>        .name           = "platform",
> diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
> index 5417944..6d7d399 100644
> --- a/include/linux/platform_device.h
> +++ b/include/linux/platform_device.h
> @@ -53,6 +53,7 @@ extern int platform_device_add_resources(struct platform_device *pdev,
>                                         const struct resource *res,
>                                         unsigned int num);
>  extern int platform_device_add_data(struct platform_device *pdev, const void *data, size_t size);
> +extern int __platform_device_add(struct platform_device *pdev);
>  extern int platform_device_add(struct platform_device *pdev);
>  extern void platform_device_del(struct platform_device *pdev);
>  extern void platform_device_put(struct platform_device *pdev);
> @@ -67,12 +68,16 @@ struct platform_driver {
>        const struct platform_device_id *id_table;
>  };
>
> +extern int __platform_driver_register(struct platform_driver *);
>  extern int platform_driver_register(struct platform_driver *);
>  extern void platform_driver_unregister(struct platform_driver *);
>
>  /* non-hotpluggable platform devices may use this so that probe() and
>  * its support may live in __init sections, conserving runtime memory.
>  */
> +extern int __platform_driver_probe(struct platform_driver *driver,
> +               int (*probe)(struct platform_device *),
> +               int (*bustype_driver_register)(struct platform_driver *));
>  extern int platform_driver_probe(struct platform_driver *driver,
>                int (*probe)(struct platform_device *));
>
> @@ -84,6 +89,88 @@ extern struct platform_device *platform_create_bundle(struct platform_driver *dr
>                                        struct resource *res, unsigned int n_res,
>                                        const void *data, size_t size);
>
> +extern struct device_attribute platform_dev_attrs[];
> +extern int platform_uevent(struct device *dev, struct kobj_uevent_env *env);
> +extern int platform_match(struct device *dev, struct device_driver *drv);
> +
> +#ifdef CONFIG_PM_SLEEP
> +extern int platform_pm_prepare(struct device *dev);
> +extern void platform_pm_complete(struct device *dev);
> +#else  /* !CONFIG_PM_SLEEP */
> +#define platform_pm_prepare            NULL
> +#define platform_pm_complete           NULL
> +#endif /* !CONFIG_PM_SLEEP */
> +
> +#ifdef CONFIG_SUSPEND
> +extern int __weak platform_pm_suspend(struct device *dev);
> +extern int __weak platform_pm_suspend_noirq(struct device *dev);
> +extern int __weak platform_pm_resume(struct device *dev);
> +extern int __weak platform_pm_resume_noirq(struct device *dev);
> +#else  /* !CONFIG_SUSPEND */
> +#define platform_pm_suspend            NULL
> +#define platform_pm_resume             NULL
> +#define platform_pm_suspend_noirq      NULL
> +#define platform_pm_resume_noirq       NULL
> +#endif /* !CONFIG_SUSPEND */
> +
> +#ifdef CONFIG_HIBERNATION
> +extern int platform_pm_freeze(struct device *dev);
> +extern int platform_pm_freeze_noirq(struct device *dev);
> +extern int platform_pm_thaw(struct device *dev);
> +extern int platform_pm_thaw_noirq(struct device *dev);
> +extern int platform_pm_poweroff(struct device *dev);
> +extern int platform_pm_poweroff_noirq(struct device *dev);
> +extern int platform_pm_restore(struct device *dev);
> +extern int platform_pm_restore_noirq(struct device *dev);
> +#else  /* !CONFIG_HIBERNATION */
> +#define platform_pm_freeze             NULL
> +#define platform_pm_thaw               NULL
> +#define platform_pm_poweroff           NULL
> +#define platform_pm_restore            NULL
> +#define platform_pm_freeze_noirq       NULL
> +#define platform_pm_thaw_noirq         NULL
> +#define platform_pm_poweroff_noirq     NULL
> +#define platform_pm_restore_noirq      NULL
> +#endif /* !CONFIG_HIBERNATION */
> +
> +#ifdef CONFIG_PM_RUNTIME
> +extern int __weak platform_pm_runtime_suspend(struct device *dev);
> +extern int __weak platform_pm_runtime_resume(struct device *dev);
> +extern int __weak platform_pm_runtime_idle(struct device *dev);
> +#else  /* !CONFIG_PM_RUNTIME */
> +#define platform_pm_runtime_suspend NULL
> +#define platform_pm_runtime_resume NULL
> +#define platform_pm_runtime_idle NULL
> +#endif /* !CONFIG_PM_RUNTIME */
> +
> +extern const struct dev_pm_ops platform_dev_pm_ops;
> +
> +#define PLATFORM_BUS_TEMPLATE \
> +       .name           = "XXX_OVERRIDEME_XXX", \
> +       .dev_attrs      = platform_dev_attrs,   \
> +       .match          = platform_match,       \
> +       .uevent         = platform_uevent,      \
> +       .pm             = &platform_dev_pm_ops
> +
> +#define PLATFORM_PM_OPS_TEMPLATE \
> +       .prepare = platform_pm_prepare,                 \
> +       .complete = platform_pm_complete,               \
> +       .suspend = platform_pm_suspend,                 \
> +       .resume = platform_pm_resume,                   \
> +       .freeze = platform_pm_freeze,                   \
> +       .thaw = platform_pm_thaw,                       \
> +       .poweroff = platform_pm_poweroff,               \
> +       .restore = platform_pm_restore,                 \
> +       .suspend_noirq = platform_pm_suspend_noirq,     \
> +       .resume_noirq = platform_pm_resume_noirq,       \
> +       .freeze_noirq = platform_pm_freeze_noirq,       \
> +       .thaw_noirq = platform_pm_thaw_noirq,           \
> +       .poweroff_noirq = platform_pm_poweroff_noirq,   \
> +       .restore_noirq = platform_pm_restore_noirq,     \
> +       .runtime_suspend = platform_pm_runtime_suspend, \
> +       .runtime_resume = platform_pm_runtime_resume,   \
> +       .runtime_idle = platform_pm_runtime_idle
> +
>  /* early platform driver interface */
>  struct early_platform_driver {
>        const char *class_str;
> --
> 1.7.2.1
>
>



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

[PATCH 3/4] msm-bus: Define the msm-bus skeleton

[Patrick Pannuto]

This defines the msm_bus_type and adds 3 bus
devices (msm-apps, msm-system, msm-mmss) to it.

With this new model it is trivial to move devices and
drivers onto the msm_bus, simply

	s/platform_device_register/msm_device_register
	s/platform_driver_register/msm_driver_register

This is not the final architecture of msm_bus /devices/,
rather a demonstration of the API usage to define and
utilize the msm_bus_type. Architecture will likely be
specified in board files or perhaps OF.

The resulting bus structure is (snipped):

/sys/bus
|-- msm-bus-type
|   |-- devices
|   |   |-- msm-apps -> ../../../devices/msm-apps
|   |   |-- msm-mmss -> ../../../devices/msm-mmss
|   |   |-- msm-system -> ../../../devices/msm-system
|   |   |-- some-msm-dev -> ../../../devices/msm-system/some-msm-dev
|   |-- drivers
|   |   |-- some-msm-drv

/sys/devices
|-- msm-apps
|-- msm-mmss
|-- msm-system
|   |-- some-msm-dev

Which maps the desired topology

QUICK COMMENT

It is worth noting that this patch is a fairly minimal implementation,
that is, it does not yet have any functionality that makes it differ
from the platform bus - it just shows how it would be done.

Also, it only implements the part of the API it needs to, which could
be confusing - you register devices with msm_device_register, yet
unregister them with platform_device_unregister; although it would
be perfectly valid to add

   #define msm_device_unregister platform_device_unregister
   (...etc)

to msm_device.h to "complete the API".

This patch and the following are a /proof of concept/, not the acutal
patches for MSM; physical bus devices vary, and will not be defined as
statically as shown here

Change-Id: I0f4cd8eb515726ef1945d8ea972f0f8a5e145a7b
Signed-off-by: Patrick Pannuto <ppannuto@codeaurora.org>
---
 arch/arm/mach-msm/Makefile                  |    1 +
 arch/arm/mach-msm/include/mach/msm_device.h |   28 +++++++
 arch/arm/mach-msm/msm_bus.c                 |  105 +++++++++++++++++++++++++++
 3 files changed, 134 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-msm/include/mach/msm_device.h
 create mode 100644 arch/arm/mach-msm/msm_bus.c

diff --git a/arch/arm/mach-msm/Makefile b/arch/arm/mach-msm/Makefile
index 7046106..977ba89 100644
--- a/arch/arm/mach-msm/Makefile
+++ b/arch/arm/mach-msm/Makefile
@@ -4,6 +4,7 @@ obj-y += vreg.o
 obj-y += acpuclock-arm11.o
 obj-y += clock.o clock-pcom.o
 obj-y += gpio.o
+obj-y += msm_bus.o
 
 ifdef CONFIG_MSM_VIC
 obj-y += irq-vic.o
diff --git a/arch/arm/mach-msm/include/mach/msm_device.h b/arch/arm/mach-msm/include/mach/msm_device.h
new file mode 100644
index 0000000..f46a2d0
--- /dev/null
+++ b/arch/arm/mach-msm/include/mach/msm_device.h
@@ -0,0 +1,28 @@
+/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA.
+ */
+
+#ifndef _MSM_DEVICE_H
+#define _MSM_DEVICE_H
+
+#include <linux/platform_device.h>
+
+extern int msm_device_register(struct platform_device *pdev);
+extern int msm_driver_register(struct platform_driver *pdrv);
+extern int msm_driver_probe(struct platform_driver *pdrv,
+		int (*probe)(struct platform_device *));
+
+#endif
diff --git a/arch/arm/mach-msm/msm_bus.c b/arch/arm/mach-msm/msm_bus.c
new file mode 100644
index 0000000..f32942c
--- /dev/null
+++ b/arch/arm/mach-msm/msm_bus.c
@@ -0,0 +1,105 @@
+/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA.
+ */
+
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include <mach/msm_device.h>
+
+const struct dev_pm_ops msm_pm_ops = {
+	PLATFORM_PM_OPS_TEMPLATE,
+	.prepare	= NULL,
+};
+
+struct bus_type msm_bus_type = {
+	PLATFORM_BUS_TEMPLATE,
+	.name		= "msm-bus-type",
+	.dev_attrs	= NULL,
+	.pm		= &msm_pm_ops,
+};
+
+static struct platform_device msm_apps_bus = {
+	.name			= "msm-apps",
+	.id			= -1,
+};
+
+static struct platform_device msm_system_bus = {
+	.name			= "msm-system",
+	.id			= -1,
+};
+
+static struct platform_device msm_mmss_bus = {
+	.name			= "msm-mmss",
+	.id			= -1,
+};
+
+int msm_device_register(struct platform_device *pdev)
+{
+	pdev->dev.bus = &msm_bus_type;
+	/* XXX: Use platform_data to assign pdev->dev.parent */
+
+	device_initialize(&pdev->dev);
+	return __platform_device_add(pdev);
+}
+
+int msm_driver_register(struct platform_driver *pdrv)
+{
+	pdrv->driver.bus = &msm_bus_type;
+
+	return __platform_driver_register(pdrv);
+}
+
+int msm_driver_probe(struct platform_driver *pdrv,
+		int (*probe)(struct platform_device *))
+{
+	return __platform_driver_probe(pdrv, probe, &msm_driver_register);
+}
+
+static int __init msm_bus_init(void)
+{
+	int error;
+
+	error = bus_register(&msm_bus_type);
+	if (error)
+		return error;
+
+	error = msm_device_register(&msm_apps_bus);
+	if (error)
+		goto fail_apps_bus;
+
+	error = msm_device_register(&msm_system_bus);
+	if (error)
+		goto fail_system_bus;
+
+	error = msm_device_register(&msm_mmss_bus);
+	if (error)
+		goto fail_mmss_bus;
+
+	return error;
+
+	/* platform_device_unregister(&msm_mmss_bus); */
+fail_mmss_bus:
+	platform_device_unregister(&msm_system_bus);
+fail_system_bus:
+	platform_device_unregister(&msm_apps_bus);
+fail_apps_bus:
+	bus_unregister(&msm_bus_type);
+
+	return error;
+}
+postcore_initcall(msm_bus_init);
-- 
1.7.2.1

[PATCH 3/4] msm-bus: Define the msm-bus skeleton

[Patrick Pannuto]

This defines the msm_bus_type and adds 3 bus
devices (msm-apps, msm-system, msm-mmss) to it.

With this new model it is trivial to move devices and
drivers onto the msm_bus, simply

	s/platform_device_register/msm_device_register
	s/platform_driver_register/msm_driver_register

This is not the final architecture of msm_bus /devices/,
rather a demonstration of the API usage to define and
utilize the msm_bus_type. Architecture will likely be
specified in board files or perhaps OF.

The resulting bus structure is (snipped):

/sys/bus
|-- msm-bus-type
|   |-- devices
|   |   |-- msm-apps -> ../../../devices/msm-apps
|   |   |-- msm-mmss -> ../../../devices/msm-mmss
|   |   |-- msm-system -> ../../../devices/msm-system
|   |   |-- some-msm-dev -> ../../../devices/msm-system/some-msm-dev
|   |-- drivers
|   |   |-- some-msm-drv

/sys/devices
|-- msm-apps
|-- msm-mmss
|-- msm-system
|   |-- some-msm-dev

Which maps the desired topology

QUICK COMMENT

It is worth noting that this patch is a fairly minimal implementation,
that is, it does not yet have any functionality that makes it differ
from the platform bus - it just shows how it would be done.

Also, it only implements the part of the API it needs to, which could
be confusing - you register devices with msm_device_register, yet
unregister them with platform_device_unregister; although it would
be perfectly valid to add

   #define msm_device_unregister platform_device_unregister
   (...etc)

to msm_device.h to "complete the API".

This patch and the following are a /proof of concept/, not the acutal
patches for MSM; physical bus devices vary, and will not be defined as
statically as shown here

Change-Id: I0f4cd8eb515726ef1945d8ea972f0f8a5e145a7b
Signed-off-by: Patrick Pannuto <ppannuto@codeaurora.org>
---
 arch/arm/mach-msm/Makefile                  |    1 +
 arch/arm/mach-msm/include/mach/msm_device.h |   28 +++++++
 arch/arm/mach-msm/msm_bus.c                 |  105 +++++++++++++++++++++++++++
 3 files changed, 134 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-msm/include/mach/msm_device.h
 create mode 100644 arch/arm/mach-msm/msm_bus.c

diff --git a/arch/arm/mach-msm/Makefile b/arch/arm/mach-msm/Makefile
index 7046106..977ba89 100644
--- a/arch/arm/mach-msm/Makefile
+++ b/arch/arm/mach-msm/Makefile
@@ -4,6 +4,7 @@ obj-y += vreg.o
 obj-y += acpuclock-arm11.o
 obj-y += clock.o clock-pcom.o
 obj-y += gpio.o
+obj-y += msm_bus.o
 
 ifdef CONFIG_MSM_VIC
 obj-y += irq-vic.o
diff --git a/arch/arm/mach-msm/include/mach/msm_device.h b/arch/arm/mach-msm/include/mach/msm_device.h
new file mode 100644
index 0000000..f46a2d0
--- /dev/null
+++ b/arch/arm/mach-msm/include/mach/msm_device.h
@@ -0,0 +1,28 @@
+/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA.
+ */
+
+#ifndef _MSM_DEVICE_H
+#define _MSM_DEVICE_H
+
+#include <linux/platform_device.h>
+
+extern int msm_device_register(struct platform_device *pdev);
+extern int msm_driver_register(struct platform_driver *pdrv);
+extern int msm_driver_probe(struct platform_driver *pdrv,
+		int (*probe)(struct platform_device *));
+
+#endif
diff --git a/arch/arm/mach-msm/msm_bus.c b/arch/arm/mach-msm/msm_bus.c
new file mode 100644
index 0000000..f32942c
--- /dev/null
+++ b/arch/arm/mach-msm/msm_bus.c
@@ -0,0 +1,105 @@
+/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA.
+ */
+
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include <mach/msm_device.h>
+
+const struct dev_pm_ops msm_pm_ops = {
+	PLATFORM_PM_OPS_TEMPLATE,
+	.prepare	= NULL,
+};
+
+struct bus_type msm_bus_type = {
+	PLATFORM_BUS_TEMPLATE,
+	.name		= "msm-bus-type",
+	.dev_attrs	= NULL,
+	.pm		= &msm_pm_ops,
+};
+
+static struct platform_device msm_apps_bus = {
+	.name			= "msm-apps",
+	.id			= -1,
+};
+
+static struct platform_device msm_system_bus = {
+	.name			= "msm-system",
+	.id			= -1,
+};
+
+static struct platform_device msm_mmss_bus = {
+	.name			= "msm-mmss",
+	.id			= -1,
+};
+
+int msm_device_register(struct platform_device *pdev)
+{
+	pdev->dev.bus = &msm_bus_type;
+	/* XXX: Use platform_data to assign pdev->dev.parent */
+
+	device_initialize(&pdev->dev);
+	return __platform_device_add(pdev);
+}
+
+int msm_driver_register(struct platform_driver *pdrv)
+{
+	pdrv->driver.bus = &msm_bus_type;
+
+	return __platform_driver_register(pdrv);
+}
+
+int msm_driver_probe(struct platform_driver *pdrv,
+		int (*probe)(struct platform_device *))
+{
+	return __platform_driver_probe(pdrv, probe, &msm_driver_register);
+}
+
+static int __init msm_bus_init(void)
+{
+	int error;
+
+	error = bus_register(&msm_bus_type);
+	if (error)
+		return error;
+
+	error = msm_device_register(&msm_apps_bus);
+	if (error)
+		goto fail_apps_bus;
+
+	error = msm_device_register(&msm_system_bus);
+	if (error)
+		goto fail_system_bus;
+
+	error = msm_device_register(&msm_mmss_bus);
+	if (error)
+		goto fail_mmss_bus;
+
+	return error;
+
+	/* platform_device_unregister(&msm_mmss_bus); */
+fail_mmss_bus:
+	platform_device_unregister(&msm_system_bus);
+fail_system_bus:
+	platform_device_unregister(&msm_apps_bus);
+fail_apps_bus:
+	bus_unregister(&msm_bus_type);
+
+	return error;
+}
+postcore_initcall(msm_bus_init);
-- 
1.7.2.1

Re: [PATCH 3/4] msm-bus: Define the msm-bus skeleton

[Grant Likely]

On Wed, Aug 18, 2010 at 1:15 PM, Patrick Pannuto
<ppannuto@codeaurora.org> wrote:
> This defines the msm_bus_type and adds 3 bus
> devices (msm-apps, msm-system, msm-mmss) to it.
>
> With this new model it is trivial to move devices and
> drivers onto the msm_bus, simply
>
>        s/platform_device_register/msm_device_register
>        s/platform_driver_register/msm_driver_register
>
> This is not the final architecture of msm_bus /devices/,
> rather a demonstration of the API usage to define and
> utilize the msm_bus_type. Architecture will likely be
> specified in board files or perhaps OF.
>
> The resulting bus structure is (snipped):
>
> /sys/bus
> |-- msm-bus-type
> |   |-- devices
> |   |   |-- msm-apps -> ../../../devices/msm-apps
> |   |   |-- msm-mmss -> ../../../devices/msm-mmss
> |   |   |-- msm-system -> ../../../devices/msm-system
> |   |   |-- some-msm-dev -> ../../../devices/msm-system/some-msm-dev
> |   |-- drivers
> |   |   |-- some-msm-drv
>
> /sys/devices
> |-- msm-apps
> |-- msm-mmss
> |-- msm-system
> |   |-- some-msm-dev
>
> Which maps the desired topology
>
> QUICK COMMENT
>
> It is worth noting that this patch is a fairly minimal implementation,
> that is, it does not yet have any functionality that makes it differ
> from the platform bus - it just shows how it would be done.

Okay, so that's the core point.  without the differentiated behaviour,
you can do exactly the same thing, with the same topology, without the
new bus types.

So, the question remains, what is the new functionality that needs to
be supported that platform_bus_type doesn't implement?  That is the
example that I'm really keen to see!  :-)

Cheers,
g.

>
> Also, it only implements the part of the API it needs to, which could
> be confusing - you register devices with msm_device_register, yet
> unregister them with platform_device_unregister; although it would
> be perfectly valid to add
>
>   #define msm_device_unregister platform_device_unregister
>   (...etc)
>
> to msm_device.h to "complete the API".
>
> This patch and the following are a /proof of concept/, not the acutal
> patches for MSM; physical bus devices vary, and will not be defined as
> statically as shown here
>
> Change-Id: I0f4cd8eb515726ef1945d8ea972f0f8a5e145a7b
> Signed-off-by: Patrick Pannuto <ppannuto@codeaurora.org>
> ---
>  arch/arm/mach-msm/Makefile                  |    1 +
>  arch/arm/mach-msm/include/mach/msm_device.h |   28 +++++++
>  arch/arm/mach-msm/msm_bus.c                 |  105 +++++++++++++++++++++++++++
>  3 files changed, 134 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-msm/include/mach/msm_device.h
>  create mode 100644 arch/arm/mach-msm/msm_bus.c
>
> diff --git a/arch/arm/mach-msm/Makefile b/arch/arm/mach-msm/Makefile
> index 7046106..977ba89 100644
> --- a/arch/arm/mach-msm/Makefile
> +++ b/arch/arm/mach-msm/Makefile
> @@ -4,6 +4,7 @@ obj-y += vreg.o
>  obj-y += acpuclock-arm11.o
>  obj-y += clock.o clock-pcom.o
>  obj-y += gpio.o
> +obj-y += msm_bus.o
>
>  ifdef CONFIG_MSM_VIC
>  obj-y += irq-vic.o
> diff --git a/arch/arm/mach-msm/include/mach/msm_device.h b/arch/arm/mach-msm/include/mach/msm_device.h
> new file mode 100644
> index 0000000..f46a2d0
> --- /dev/null
> +++ b/arch/arm/mach-msm/include/mach/msm_device.h
> @@ -0,0 +1,28 @@
> +/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> + * 02110-1301, USA.
> + */
> +
> +#ifndef _MSM_DEVICE_H
> +#define _MSM_DEVICE_H
> +
> +#include <linux/platform_device.h>
> +
> +extern int msm_device_register(struct platform_device *pdev);
> +extern int msm_driver_register(struct platform_driver *pdrv);
> +extern int msm_driver_probe(struct platform_driver *pdrv,
> +               int (*probe)(struct platform_device *));
> +
> +#endif
> diff --git a/arch/arm/mach-msm/msm_bus.c b/arch/arm/mach-msm/msm_bus.c
> new file mode 100644
> index 0000000..f32942c
> --- /dev/null
> +++ b/arch/arm/mach-msm/msm_bus.c
> @@ -0,0 +1,105 @@
> +/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> + * 02110-1301, USA.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#include <mach/msm_device.h>
> +
> +const struct dev_pm_ops msm_pm_ops = {
> +       PLATFORM_PM_OPS_TEMPLATE,
> +       .prepare        = NULL,
> +};
> +
> +struct bus_type msm_bus_type = {
> +       PLATFORM_BUS_TEMPLATE,
> +       .name           = "msm-bus-type",
> +       .dev_attrs      = NULL,
> +       .pm             = &msm_pm_ops,
> +};
> +
> +static struct platform_device msm_apps_bus = {
> +       .name                   = "msm-apps",
> +       .id                     = -1,
> +};
> +
> +static struct platform_device msm_system_bus = {
> +       .name                   = "msm-system",
> +       .id                     = -1,
> +};
> +
> +static struct platform_device msm_mmss_bus = {
> +       .name                   = "msm-mmss",
> +       .id                     = -1,
> +};
> +
> +int msm_device_register(struct platform_device *pdev)
> +{
> +       pdev->dev.bus = &msm_bus_type;
> +       /* XXX: Use platform_data to assign pdev->dev.parent */
> +
> +       device_initialize(&pdev->dev);
> +       return __platform_device_add(pdev);
> +}
> +
> +int msm_driver_register(struct platform_driver *pdrv)
> +{
> +       pdrv->driver.bus = &msm_bus_type;
> +
> +       return __platform_driver_register(pdrv);
> +}
> +
> +int msm_driver_probe(struct platform_driver *pdrv,
> +               int (*probe)(struct platform_device *))
> +{
> +       return __platform_driver_probe(pdrv, probe, &msm_driver_register);
> +}
> +
> +static int __init msm_bus_init(void)
> +{
> +       int error;
> +
> +       error = bus_register(&msm_bus_type);
> +       if (error)
> +               return error;
> +
> +       error = msm_device_register(&msm_apps_bus);
> +       if (error)
> +               goto fail_apps_bus;
> +
> +       error = msm_device_register(&msm_system_bus);
> +       if (error)
> +               goto fail_system_bus;
> +
> +       error = msm_device_register(&msm_mmss_bus);
> +       if (error)
> +               goto fail_mmss_bus;
> +
> +       return error;
> +
> +       /* platform_device_unregister(&msm_mmss_bus); */
> +fail_mmss_bus:
> +       platform_device_unregister(&msm_system_bus);
> +fail_system_bus:
> +       platform_device_unregister(&msm_apps_bus);
> +fail_apps_bus:
> +       bus_unregister(&msm_bus_type);
> +
> +       return error;
> +}
> +postcore_initcall(msm_bus_init);
> --
> 1.7.2.1
>
>



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

[PATCH 3/4] msm-bus: Define the msm-bus skeleton

[Grant Likely]

On Wed, Aug 18, 2010 at 1:15 PM, Patrick Pannuto
<ppannuto@codeaurora.org> wrote:
> This defines the msm_bus_type and adds 3 bus
> devices (msm-apps, msm-system, msm-mmss) to it.
>
> With this new model it is trivial to move devices and
> drivers onto the msm_bus, simply
>
> ? ? ? ?s/platform_device_register/msm_device_register
> ? ? ? ?s/platform_driver_register/msm_driver_register
>
> This is not the final architecture of msm_bus /devices/,
> rather a demonstration of the API usage to define and
> utilize the msm_bus_type. Architecture will likely be
> specified in board files or perhaps OF.
>
> The resulting bus structure is (snipped):
>
> /sys/bus
> |-- msm-bus-type
> | ? |-- devices
> | ? | ? |-- msm-apps -> ../../../devices/msm-apps
> | ? | ? |-- msm-mmss -> ../../../devices/msm-mmss
> | ? | ? |-- msm-system -> ../../../devices/msm-system
> | ? | ? |-- some-msm-dev -> ../../../devices/msm-system/some-msm-dev
> | ? |-- drivers
> | ? | ? |-- some-msm-drv
>
> /sys/devices
> |-- msm-apps
> |-- msm-mmss
> |-- msm-system
> | ? |-- some-msm-dev
>
> Which maps the desired topology
>
> QUICK COMMENT
>
> It is worth noting that this patch is a fairly minimal implementation,
> that is, it does not yet have any functionality that makes it differ
> from the platform bus - it just shows how it would be done.

Okay, so that's the core point.  without the differentiated behaviour,
you can do exactly the same thing, with the same topology, without the
new bus types.

So, the question remains, what is the new functionality that needs to
be supported that platform_bus_type doesn't implement?  That is the
example that I'm really keen to see!  :-)

Cheers,
g.

>
> Also, it only implements the part of the API it needs to, which could
> be confusing - you register devices with msm_device_register, yet
> unregister them with platform_device_unregister; although it would
> be perfectly valid to add
>
> ? #define msm_device_unregister platform_device_unregister
> ? (...etc)
>
> to msm_device.h to "complete the API".
>
> This patch and the following are a /proof of concept/, not the acutal
> patches for MSM; physical bus devices vary, and will not be defined as
> statically as shown here
>
> Change-Id: I0f4cd8eb515726ef1945d8ea972f0f8a5e145a7b
> Signed-off-by: Patrick Pannuto <ppannuto@codeaurora.org>
> ---
> ?arch/arm/mach-msm/Makefile ? ? ? ? ? ? ? ? ?| ? ?1 +
> ?arch/arm/mach-msm/include/mach/msm_device.h | ? 28 +++++++
> ?arch/arm/mach-msm/msm_bus.c ? ? ? ? ? ? ? ? | ?105 +++++++++++++++++++++++++++
> ?3 files changed, 134 insertions(+), 0 deletions(-)
> ?create mode 100644 arch/arm/mach-msm/include/mach/msm_device.h
> ?create mode 100644 arch/arm/mach-msm/msm_bus.c
>
> diff --git a/arch/arm/mach-msm/Makefile b/arch/arm/mach-msm/Makefile
> index 7046106..977ba89 100644
> --- a/arch/arm/mach-msm/Makefile
> +++ b/arch/arm/mach-msm/Makefile
> @@ -4,6 +4,7 @@ obj-y += vreg.o
> ?obj-y += acpuclock-arm11.o
> ?obj-y += clock.o clock-pcom.o
> ?obj-y += gpio.o
> +obj-y += msm_bus.o
>
> ?ifdef CONFIG_MSM_VIC
> ?obj-y += irq-vic.o
> diff --git a/arch/arm/mach-msm/include/mach/msm_device.h b/arch/arm/mach-msm/include/mach/msm_device.h
> new file mode 100644
> index 0000000..f46a2d0
> --- /dev/null
> +++ b/arch/arm/mach-msm/include/mach/msm_device.h
> @@ -0,0 +1,28 @@
> +/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> + * 02110-1301, USA.
> + */
> +
> +#ifndef _MSM_DEVICE_H
> +#define _MSM_DEVICE_H
> +
> +#include <linux/platform_device.h>
> +
> +extern int msm_device_register(struct platform_device *pdev);
> +extern int msm_driver_register(struct platform_driver *pdrv);
> +extern int msm_driver_probe(struct platform_driver *pdrv,
> + ? ? ? ? ? ? ? int (*probe)(struct platform_device *));
> +
> +#endif
> diff --git a/arch/arm/mach-msm/msm_bus.c b/arch/arm/mach-msm/msm_bus.c
> new file mode 100644
> index 0000000..f32942c
> --- /dev/null
> +++ b/arch/arm/mach-msm/msm_bus.c
> @@ -0,0 +1,105 @@
> +/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> + * 02110-1301, USA.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#include <mach/msm_device.h>
> +
> +const struct dev_pm_ops msm_pm_ops = {
> + ? ? ? PLATFORM_PM_OPS_TEMPLATE,
> + ? ? ? .prepare ? ? ? ?= NULL,
> +};
> +
> +struct bus_type msm_bus_type = {
> + ? ? ? PLATFORM_BUS_TEMPLATE,
> + ? ? ? .name ? ? ? ? ? = "msm-bus-type",
> + ? ? ? .dev_attrs ? ? ?= NULL,
> + ? ? ? .pm ? ? ? ? ? ? = &msm_pm_ops,
> +};
> +
> +static struct platform_device msm_apps_bus = {
> + ? ? ? .name ? ? ? ? ? ? ? ? ? = "msm-apps",
> + ? ? ? .id ? ? ? ? ? ? ? ? ? ? = -1,
> +};
> +
> +static struct platform_device msm_system_bus = {
> + ? ? ? .name ? ? ? ? ? ? ? ? ? = "msm-system",
> + ? ? ? .id ? ? ? ? ? ? ? ? ? ? = -1,
> +};
> +
> +static struct platform_device msm_mmss_bus = {
> + ? ? ? .name ? ? ? ? ? ? ? ? ? = "msm-mmss",
> + ? ? ? .id ? ? ? ? ? ? ? ? ? ? = -1,
> +};
> +
> +int msm_device_register(struct platform_device *pdev)
> +{
> + ? ? ? pdev->dev.bus = &msm_bus_type;
> + ? ? ? /* XXX: Use platform_data to assign pdev->dev.parent */
> +
> + ? ? ? device_initialize(&pdev->dev);
> + ? ? ? return __platform_device_add(pdev);
> +}
> +
> +int msm_driver_register(struct platform_driver *pdrv)
> +{
> + ? ? ? pdrv->driver.bus = &msm_bus_type;
> +
> + ? ? ? return __platform_driver_register(pdrv);
> +}
> +
> +int msm_driver_probe(struct platform_driver *pdrv,
> + ? ? ? ? ? ? ? int (*probe)(struct platform_device *))
> +{
> + ? ? ? return __platform_driver_probe(pdrv, probe, &msm_driver_register);
> +}
> +
> +static int __init msm_bus_init(void)
> +{
> + ? ? ? int error;
> +
> + ? ? ? error = bus_register(&msm_bus_type);
> + ? ? ? if (error)
> + ? ? ? ? ? ? ? return error;
> +
> + ? ? ? error = msm_device_register(&msm_apps_bus);
> + ? ? ? if (error)
> + ? ? ? ? ? ? ? goto fail_apps_bus;
> +
> + ? ? ? error = msm_device_register(&msm_system_bus);
> + ? ? ? if (error)
> + ? ? ? ? ? ? ? goto fail_system_bus;
> +
> + ? ? ? error = msm_device_register(&msm_mmss_bus);
> + ? ? ? if (error)
> + ? ? ? ? ? ? ? goto fail_mmss_bus;
> +
> + ? ? ? return error;
> +
> + ? ? ? /* platform_device_unregister(&msm_mmss_bus); */
> +fail_mmss_bus:
> + ? ? ? platform_device_unregister(&msm_system_bus);
> +fail_system_bus:
> + ? ? ? platform_device_unregister(&msm_apps_bus);
> +fail_apps_bus:
> + ? ? ? bus_unregister(&msm_bus_type);
> +
> + ? ? ? return error;
> +}
> +postcore_initcall(msm_bus_init);
> --
> 1.7.2.1
>
>



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

Re: [PATCH 3/4] msm-bus: Define the msm-bus skeleton

[Patrick Pannuto]

>> QUICK COMMENT
>>
>> It is worth noting that this patch is a fairly minimal implementation,
>> that is, it does not yet have any functionality that makes it differ
>> from the platform bus - it just shows how it would be done.
> 
> Okay, so that's the core point.  without the differentiated behaviour,
> you can do exactly the same thing, with the same topology, without the
> new bus types.
> 
> So, the question remains, what is the new functionality that needs to
> be supported that platform_bus_type doesn't implement?  That is the
> example that I'm really keen to see!  :-)
> 
> Cheers,
> g.
> 

Ahhh... I was afraid you'd say that.


>> +static struct platform_device msm_apps_bus = {
>> +       .name                   = "root-fabric-apps",
>> +       .id                     = -1,
>> +};
>> +
>> +static struct platform_device msm_system_bus = {
>> +       .name                   = "root-fabric-system",
>> +       .id                     = -1,
>> +};
>> +
>> +static struct platform_device msm_mmss_bus = {
>> +       .name                   = "fabric-mmss",
>> +       .id                     = -1,
>> +};
>> +
>> +int msm_device_register(struct platform_device *pdev)
>> +{
>> +       pdev->dev.bus = &msm_bus_type;
>> +       /* XXX: Use platform_data to assign pdev->dev.parent */
>> +
>> +       device_initialize(&pdev->dev);
>> +       return __platform_device_add(pdev);
>> +}
>> +

With the understanding that I do not own the code that would be actually
doing this, hopefully some pseduo-code can help?

int msm_device_register(struct platform_device *pdev)
{
	pdev->dev.bus = &msm_bus_type;

	if (!strncmp(pdev->name, "root", strlen("root"))) {
		/* We are a root fabric (physical bus), no parent */
		pdev->dev.parent = NULL;
	}
	else {
		struct msm_dev_info* info = pdev->dev.platform_data;
		switch(info->magic) {
		case APPS_MAGIC:
			pdev->dev.parent = &msm_apps_bus;
			break;
		case SYSTEM_MAGIC:
			pdev->dev.parent = &msm_system_bus;
			break;
		case MMSS_MAGIC:
			pdev->dev.parent = &msm_mmss_bus;
			break;
		}
	}

	device_initailize(&pdev->dev);
	return __platform_device_add(pdev);
}

int msm_bus_probe(struct device* dev)
{
	int error;
	struct msm_dev_info* info = dev->platform_data;
	struct sibling_device* sib;

	list_for_each_entry(sib, &info->sib_list, list) {
		error = build_path(dev, sib->dev, sib->requirements);
		if (error)
			return error;
	}

	if (dev->driver->probe)
		error = dev->driver->probe(dev);

	return error;
}

What you see here is handling the fact that "fabrics" are actually separate
buses, so we must be intelligent in adding devices so they attach to the
right parent.  When probing devices, they may have other devices they need
to talk to, which may be on a different fabric (physical bus), so we need
to prove that we can build a path from the first device to its sibling,
meeting all of the requirements (clock speed, bandwidth, etc) along all
the physical buses along the way.


AND NOW, for a completely different argument:

If nothing else, this would greatly help to clean up the current state of
/sys on embedded devices. From my understanding, the platform bus is a
"legacy" bus, where things that have no other home go to roost. Let us
look at my dev box:

	$ ppannuto@ppannuto-linux:~$ ls /sys/bus
	acpi  i2c       mmc  pci_express  pnp   sdio   spi
	hid   mdio_bus  pci  platform     scsi  serio  usb

	ppannuto@ppannuto-linux:~$ ls /sys/bus/platform/devices/
	Fixed MDIO bus.0  i8042  pcspkr  serial8250

And now my droid:

	$ ls /sys/bus
	platform omapdss i2c spi usb mmc sdio usb-serial w1 hid

	$ ls /sys/bus/platform/devices
	power.0 ram_console.0 omap_mdm_ctrl omap2-nand.0 omapdss sfh7743
	bu52014hfv vib-gpio musb_hdrc ehci-omap.0 ohci.0 sholes_bpwake
	omap_hdq.0 wl127x-rfkill.0 wl127x-test.0 mmci-omap-hs.0 TIWLAN_SDIO.1
	omapvout pvrsrvkm omaplfb android_usb usb_mass_storage omap34xxcam
	omap2_mcspi.1 omap2_mcspi.2  omap2_mcspi.3 omap2_mcspi.4 omap-uart.1
	omap-uart.2 omap-uart.3 omap-mcbsp.1 omap-mcbsp.2 omap-mcbsp.3 omap-mcbsp.4
	omap-mcbsp.5 i2c_omap.1 i2c_omap.2 i2c_omap.3 omap_wdt omapfb cpcap-regltr.0
	cpcap-regltr.17 cpcap-regltr.1 cpcap-regltr.2 cpcap-regltr.3 cpcap-regltr.4
	cpcap-regltr.5 cpcap-regltr.6 cpcap-regltr.7 cpcap-regltr.8 cpcap-regltr.9
	cpcap-regltr.10 cpcap-regltr.11 cpcap-regltr.12 cpcap-regltr.13 cpcap-regltr.14
	cpcap-regltr.15 cpcap-regltr.16 cpcap-regltr.18 cpcap_adc cpcap_key cpcap_battery
	button-backlight notification-led keyboard-backlight flash-torch cpcap_usb
	cpcap_usb_det cpcap_audio cpcap_3mm5 cpcap_rtc cpcap_uc C6410 keyreset.0
	gpio-event.0 device_wifi.1 hp3a omap-previewer omap-resizer.2 alarm
	cpcap_usb_charger cpcap_usb_connected

	$ ls /sys/bus/omapdss/devices
	display0

	$ ls /sys/bus/w1/devices
	89-000000000000

This is a fairly ugly mess.  IMHO, it would reflect the view of the world much
better if most of that lived under /sys/bus/omap (or something similar); such a
scheme also gives omap (and msm, and others) a home for all of the custom power
code that is sure to go with their SOCs.


-Pat

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[PATCH 3/4] msm-bus: Define the msm-bus skeleton

[Patrick Pannuto]

>> QUICK COMMENT
>>
>> It is worth noting that this patch is a fairly minimal implementation,
>> that is, it does not yet have any functionality that makes it differ
>> from the platform bus - it just shows how it would be done.
> 
> Okay, so that's the core point.  without the differentiated behaviour,
> you can do exactly the same thing, with the same topology, without the
> new bus types.
> 
> So, the question remains, what is the new functionality that needs to
> be supported that platform_bus_type doesn't implement?  That is the
> example that I'm really keen to see!  :-)
> 
> Cheers,
> g.
> 

Ahhh... I was afraid you'd say that.


>> +static struct platform_device msm_apps_bus = {
>> +       .name                   = "root-fabric-apps",
>> +       .id                     = -1,
>> +};
>> +
>> +static struct platform_device msm_system_bus = {
>> +       .name                   = "root-fabric-system",
>> +       .id                     = -1,
>> +};
>> +
>> +static struct platform_device msm_mmss_bus = {
>> +       .name                   = "fabric-mmss",
>> +       .id                     = -1,
>> +};
>> +
>> +int msm_device_register(struct platform_device *pdev)
>> +{
>> +       pdev->dev.bus = &msm_bus_type;
>> +       /* XXX: Use platform_data to assign pdev->dev.parent */
>> +
>> +       device_initialize(&pdev->dev);
>> +       return __platform_device_add(pdev);
>> +}
>> +

With the understanding that I do not own the code that would be actually
doing this, hopefully some pseduo-code can help?

int msm_device_register(struct platform_device *pdev)
{
	pdev->dev.bus = &msm_bus_type;

	if (!strncmp(pdev->name, "root", strlen("root"))) {
		/* We are a root fabric (physical bus), no parent */
		pdev->dev.parent = NULL;
	}
	else {
		struct msm_dev_info* info = pdev->dev.platform_data;
		switch(info->magic) {
		case APPS_MAGIC:
			pdev->dev.parent = &msm_apps_bus;
			break;
		case SYSTEM_MAGIC:
			pdev->dev.parent = &msm_system_bus;
			break;
		case MMSS_MAGIC:
			pdev->dev.parent = &msm_mmss_bus;
			break;
		}
	}

	device_initailize(&pdev->dev);
	return __platform_device_add(pdev);
}

int msm_bus_probe(struct device* dev)
{
	int error;
	struct msm_dev_info* info = dev->platform_data;
	struct sibling_device* sib;

	list_for_each_entry(sib, &info->sib_list, list) {
		error = build_path(dev, sib->dev, sib->requirements);
		if (error)
			return error;
	}

	if (dev->driver->probe)
		error = dev->driver->probe(dev);

	return error;
}

What you see here is handling the fact that "fabrics" are actually separate
buses, so we must be intelligent in adding devices so they attach to the
right parent.  When probing devices, they may have other devices they need
to talk to, which may be on a different fabric (physical bus), so we need
to prove that we can build a path from the first device to its sibling,
meeting all of the requirements (clock speed, bandwidth, etc) along all
the physical buses along the way.


AND NOW, for a completely different argument:

If nothing else, this would greatly help to clean up the current state of
/sys on embedded devices. From my understanding, the platform bus is a
"legacy" bus, where things that have no other home go to roost. Let us
look at my dev box:

	$ ppannuto at ppannuto-linux:~$ ls /sys/bus
	acpi  i2c       mmc  pci_express  pnp   sdio   spi
	hid   mdio_bus  pci  platform     scsi  serio  usb

	ppannuto at ppannuto-linux:~$ ls /sys/bus/platform/devices/
	Fixed MDIO bus.0  i8042  pcspkr  serial8250

And now my droid:

	$ ls /sys/bus
	platform omapdss i2c spi usb mmc sdio usb-serial w1 hid

	$ ls /sys/bus/platform/devices
	power.0 ram_console.0 omap_mdm_ctrl omap2-nand.0 omapdss sfh7743
	bu52014hfv vib-gpio musb_hdrc ehci-omap.0 ohci.0 sholes_bpwake
	omap_hdq.0 wl127x-rfkill.0 wl127x-test.0 mmci-omap-hs.0 TIWLAN_SDIO.1
	omapvout pvrsrvkm omaplfb android_usb usb_mass_storage omap34xxcam
	omap2_mcspi.1 omap2_mcspi.2  omap2_mcspi.3 omap2_mcspi.4 omap-uart.1
	omap-uart.2 omap-uart.3 omap-mcbsp.1 omap-mcbsp.2 omap-mcbsp.3 omap-mcbsp.4
	omap-mcbsp.5 i2c_omap.1 i2c_omap.2 i2c_omap.3 omap_wdt omapfb cpcap-regltr.0
	cpcap-regltr.17 cpcap-regltr.1 cpcap-regltr.2 cpcap-regltr.3 cpcap-regltr.4
	cpcap-regltr.5 cpcap-regltr.6 cpcap-regltr.7 cpcap-regltr.8 cpcap-regltr.9
	cpcap-regltr.10 cpcap-regltr.11 cpcap-regltr.12 cpcap-regltr.13 cpcap-regltr.14
	cpcap-regltr.15 cpcap-regltr.16 cpcap-regltr.18 cpcap_adc cpcap_key cpcap_battery
	button-backlight notification-led keyboard-backlight flash-torch cpcap_usb
	cpcap_usb_det cpcap_audio cpcap_3mm5 cpcap_rtc cpcap_uc C6410 keyreset.0
	gpio-event.0 device_wifi.1 hp3a omap-previewer omap-resizer.2 alarm
	cpcap_usb_charger cpcap_usb_connected

	$ ls /sys/bus/omapdss/devices
	display0

	$ ls /sys/bus/w1/devices
	89-000000000000

This is a fairly ugly mess.  IMHO, it would reflect the view of the world much
better if most of that lived under /sys/bus/omap (or something similar); such a
scheme also gives omap (and msm, and others) a home for all of the custom power
code that is sure to go with their SOCs.


-Pat

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [PATCH 3/4] msm-bus: Define the msm-bus skeleton

[Grant Likely]

On Thu, Aug 19, 2010 at 12:12 PM, Patrick Pannuto
<ppannuto@codeaurora.org> wrote:
>>> QUICK COMMENT
>>>
>>> It is worth noting that this patch is a fairly minimal implementation,
>>> that is, it does not yet have any functionality that makes it differ
>>> from the platform bus - it just shows how it would be done.
>>
>> Okay, so that's the core point.  without the differentiated behaviour,
>> you can do exactly the same thing, with the same topology, without the
>> new bus types.
>>
>> So, the question remains, what is the new functionality that needs to
>> be supported that platform_bus_type doesn't implement?  That is the
>> example that I'm really keen to see!  :-)
>>
>> Cheers,
>> g.
>>
>
> Ahhh... I was afraid you'd say that.

:-)  Hi Patrick.

>>> +static struct platform_device msm_apps_bus = {
>>> +       .name                   = "root-fabric-apps",
>>> +       .id                     = -1,
>>> +};
>>> +
>>> +static struct platform_device msm_system_bus = {
>>> +       .name                   = "root-fabric-system",
>>> +       .id                     = -1,
>>> +};
>>> +
>>> +static struct platform_device msm_mmss_bus = {
>>> +       .name                   = "fabric-mmss",
>>> +       .id                     = -1,
>>> +};
>>> +
>>> +int msm_device_register(struct platform_device *pdev)
>>> +{
>>> +       pdev->dev.bus = &msm_bus_type;
>>> +       /* XXX: Use platform_data to assign pdev->dev.parent */
>>> +
>>> +       device_initialize(&pdev->dev);
>>> +       return __platform_device_add(pdev);
>>> +}
>>> +
>
> With the understanding that I do not own the code that would be actually
> doing this, hopefully some pseduo-code can help?
>
> int msm_device_register(struct platform_device *pdev)
> {
>        pdev->dev.bus = &msm_bus_type;
>
>        if (!strncmp(pdev->name, "root", strlen("root"))) {
>                /* We are a root fabric (physical bus), no parent */
>                pdev->dev.parent = NULL;

Parent is null by default.

>        }
>        else {
>                struct msm_dev_info* info = pdev->dev.platform_data;
>                switch(info->magic) {
>                case APPS_MAGIC:
>                        pdev->dev.parent = &msm_apps_bus;
>                        break;
>                case SYSTEM_MAGIC:
>                        pdev->dev.parent = &msm_system_bus;
>                        break;
>                case MMSS_MAGIC:
>                        pdev->dev.parent = &msm_mmss_bus;
>                        break;
>                }
>        }
>
>        device_initailize(&pdev->dev);
>        return __platform_device_add(pdev);
> }

Yikes, don't do this.  The parent bus information can be statically
assigned to the devices definitions themselves (see example at end of
email).  Doing it in code is opaque and too complex.

> int msm_bus_probe(struct device* dev)
> {
>        int error;
>        struct msm_dev_info* info = dev->platform_data;
>        struct sibling_device* sib;
>
>        list_for_each_entry(sib, &info->sib_list, list) {
>                error = build_path(dev, sib->dev, sib->requirements);
>                if (error)
>                        return error;
>        }
>
>        if (dev->driver->probe)
>                error = dev->driver->probe(dev);
>
>        return error;
> }
>
> What you see here is handling the fact that "fabrics" are actually separate
> buses, so we must be intelligent in adding devices so they attach to the
> right parent.  When probing devices, they may have other devices they need
> to talk to, which may be on a different fabric (physical bus), so we need
> to prove that we can build a path from the first device to its sibling,
> meeting all of the requirements (clock speed, bandwidth, etc) along all
> the physical buses along the way.

Proving a path is a valid concern, but from what I see it should
already be supported with the existing platform_bus_type.

On the probing point, I agree.  There are many cases of device
initialization order dependencies which the kernel does not currently
explicitly enforce.  In a lot of cases it happens to work "just by
luck" (but not entirely luck, because the order things get initialized
in rarely changes).  I've been playing with using bus notifiers to
postpone initialization when a dependant device hasn't yet been
initialized.  But that is a problem regardless of bus type (for
example, I had an Ethernet platform_device that needed to wait for an
mdio_device to show up).

> AND NOW, for a completely different argument:
>
> If nothing else, this would greatly help to clean up the current state of
> /sys on embedded devices. From my understanding, the platform bus is a
> "legacy" bus, where things that have no other home go to roost. Let us
> look at my dev box:

Not entirely true.  That was the case originally, but now it also
means (especially in embedded) simple memory mapped devices that must
be explicitly instantiated because the hardware is not able to probe
for them.

>
>        $ ppannuto@ppannuto-linux:~$ ls /sys/bus
>        acpi  i2c       mmc  pci_express  pnp   sdio   spi
>        hid   mdio_bus  pci  platform     scsi  serio  usb
>
>        ppannuto@ppannuto-linux:~$ ls /sys/bus/platform/devices/
>        Fixed MDIO bus.0  i8042  pcspkr  serial8250
>
> And now my droid:
>
>        $ ls /sys/bus
>        platform omapdss i2c spi usb mmc sdio usb-serial w1 hid
>
>        $ ls /sys/bus/platform/devices
>        power.0 ram_console.0 omap_mdm_ctrl omap2-nand.0 omapdss sfh7743
>        bu52014hfv vib-gpio musb_hdrc ehci-omap.0 ohci.0 sholes_bpwake
>        omap_hdq.0 wl127x-rfkill.0 wl127x-test.0 mmci-omap-hs.0 TIWLAN_SDIO.1
>        omapvout pvrsrvkm omaplfb android_usb usb_mass_storage omap34xxcam
>        omap2_mcspi.1 omap2_mcspi.2  omap2_mcspi.3 omap2_mcspi.4 omap-uart.1
>        omap-uart.2 omap-uart.3 omap-mcbsp.1 omap-mcbsp.2 omap-mcbsp.3 omap-mcbsp.4
>        omap-mcbsp.5 i2c_omap.1 i2c_omap.2 i2c_omap.3 omap_wdt omapfb cpcap-regltr.0
>        cpcap-regltr.17 cpcap-regltr.1 cpcap-regltr.2 cpcap-regltr.3 cpcap-regltr.4
>        cpcap-regltr.5 cpcap-regltr.6 cpcap-regltr.7 cpcap-regltr.8 cpcap-regltr.9
>        cpcap-regltr.10 cpcap-regltr.11 cpcap-regltr.12 cpcap-regltr.13 cpcap-regltr.14
>        cpcap-regltr.15 cpcap-regltr.16 cpcap-regltr.18 cpcap_adc cpcap_key cpcap_battery
>        button-backlight notification-led keyboard-backlight flash-torch cpcap_usb
>        cpcap_usb_det cpcap_audio cpcap_3mm5 cpcap_rtc cpcap_uc C6410 keyreset.0
>        gpio-event.0 device_wifi.1 hp3a omap-previewer omap-resizer.2 alarm
>        cpcap_usb_charger cpcap_usb_connected

You're looking in the wrong place.  Look in /sys/devices/platform
instead.  /sys/bus/*/devices shows the bus /type/ attachment, not the
physical bus attachement.

To rehash:

/sys/bus - shows the bus types and the devices registered against them
*as symlinks* to the device's sysfs directory
/sys/devices - shows the actual topology from the Linux point of view.

To illustrate, look at /sys/bus/pci_express/devices on a PC:

$ ls -l /sys/bus/pci_express/devices
0000:00:1c.0:pcie01 ->
../../../devices/pci0000:00/0000:00:1c.0/0000:00:1c.0:pcie01
0000:00:1c.0:pcie04 ->
../../../devices/pci0000:00/0000:00:1c.0/0000:00:1c.0:pcie04
0000:00:1c.0:pcie08 ->
../../../devices/pci0000:00/0000:00:1c.0/0000:00:1c.0:pcie08
0000:00:1c.1:pcie01 ->
../../../devices/pci0000:00/0000:00:1c.1/0000:00:1c.1:pcie01
0000:00:1c.1:pcie04 ->
../../../devices/pci0000:00/0000:00:1c.1/0000:00:1c.1:pcie04
0000:00:1c.1:pcie08 ->
../../../devices/pci0000:00/0000:00:1c.1/0000:00:1c.1:pcie08

See?  The pci_express devices actually live in /sys/devices, and on 2
separate physical busses; 0000:00:1c.0 and 0000:00:1c.1

That being said, /sys/devices/platform on your phone probably looks
identical because everything is registered without setting the parent
pointer.  However, it isn't always that way... (see below).

> This is a fairly ugly mess.  IMHO, it would reflect the view of the world much
> better if most of that lived under /sys/bus/omap (or something similar); such a
> scheme also gives omap (and msm, and others) a home for all of the custom power
> code that is sure to go with their SOCs.

This is the topology argument, which I agree is desirable to represent
accurately for several reasons, but in this case it has nothing to do
with the bus_type.  For example, take a look at my mpc5200 board:

$ ls /sys/bus/platform
f0000000.soc5200	       f0000670.timer		f0002000.serial
f0000200.cdm		       f0000800.rtc		f0003000.ethernet
f0000500.interrupt-controller  f0000900.can		f0003000.mdio
f0000600.timer		       f0000980.can		f0003a00.ata
f0000610.timer		       f0000b00.gpio		f0003d00.i2c
f0000620.timer		       f0000c00.gpio		f0003d40.i2c
f0000630.timer		       f0000f00.spi		f0008000.sram
f0000640.timer		       f0001000.usb		fe000000.flash
f0000650.timer		       f0001200.dma-controller	localbus.0
f0000660.timer		       f0001f00.xlb

$ ls /sys/devices/platform
power  uevent

Lots of platform devices, but none of them live in
/sys/devices/platform.  So, what's going on here?  Well, on powerpc
the platform device topology matches the device initialization data
which already organizes the devices based on the physical bus
attachment.  You can see this by looking where the symlinks in
/sys/bus/platform/devices point to:

$ ls -l /sys/bus/platform/devices
f0000000.soc5200 -> ../../../devices/f0000000.soc5200
f0000200.cdm -> ../../../devices/f0000000.soc5200/f0000200.cdm
[... trimmed for brevity ...]
f0003d40.i2c -> ../../../devices/f0000000.soc5200/f0003d40.i2c
f0008000.sram -> ../../../devices/f0000000.soc5200/f0008000.sram
fe000000.flash -> ../../../devices/localbus.0/fe000000.flash
localbus.0 -> ../../../devices/localbus.0

Most devices are children of the "f0000000.soc5200" physical bus, and
the flash device is a child of the external "localbus.0".  You'll also
notice that both f0000000.soc5200 and localbus.0 are also platform
devices on their own.

So, the topology issue can be solved right now without any regard to
the bus_type.  Also, the converse is true.  New bus_types can be added
to handle different behaviour without changing the existing topology.
Following the topology argument muddies the issue of whether or not
inheriting new bus types from plaform_bus_type is a good idea.

Try this: on one of you msm boards create a new static struct device
or struct platform_device, make sure it gets registered before the
other devices, set the .dev.parent pointer in all the msm_device_uart
platform_devices to point to it.  For example:

struct platform_device msm_bus = {
	.name = "msm_sample_bus",
};

struct platform_device msm_device_uart1 = {
	.name = "msm_serial",
	.id = 1,
	.num_resources = ARRAY_SIZE(resource_uart3),
	.resource = resources_uart3,
	.dev = {
		.parent = &msm_bus.dev,
	},
};

Then look at the effect on the system.  Data like topology can and
should be defined in a static manor; either like the above, or as I
prefer, in a flattened device tree file.

Hmmm, I've written a lot.  I should summarize.  On the topology front,
it is a separate issue, and can be solved right now without a new bus
type.

On the new bus_type front, I'm still not convinced.  However,
supporting PM operations is the most likely line of argument that
would sway me.  I've got to reply to Kevin's email and I'll elaborate
there.  I'm particularly concerned about the concept of sharing device
driver instances between bus_types.

Cheers,
g.

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

[PATCH 3/4] msm-bus: Define the msm-bus skeleton

[Grant Likely]

On Thu, Aug 19, 2010 at 12:12 PM, Patrick Pannuto
<ppannuto@codeaurora.org> wrote:
>>> QUICK COMMENT
>>>
>>> It is worth noting that this patch is a fairly minimal implementation,
>>> that is, it does not yet have any functionality that makes it differ
>>> from the platform bus - it just shows how it would be done.
>>
>> Okay, so that's the core point. ?without the differentiated behaviour,
>> you can do exactly the same thing, with the same topology, without the
>> new bus types.
>>
>> So, the question remains, what is the new functionality that needs to
>> be supported that platform_bus_type doesn't implement? ?That is the
>> example that I'm really keen to see! ?:-)
>>
>> Cheers,
>> g.
>>
>
> Ahhh... I was afraid you'd say that.

:-)  Hi Patrick.

>>> +static struct platform_device msm_apps_bus = {
>>> + ? ? ? .name ? ? ? ? ? ? ? ? ? = "root-fabric-apps",
>>> + ? ? ? .id ? ? ? ? ? ? ? ? ? ? = -1,
>>> +};
>>> +
>>> +static struct platform_device msm_system_bus = {
>>> + ? ? ? .name ? ? ? ? ? ? ? ? ? = "root-fabric-system",
>>> + ? ? ? .id ? ? ? ? ? ? ? ? ? ? = -1,
>>> +};
>>> +
>>> +static struct platform_device msm_mmss_bus = {
>>> + ? ? ? .name ? ? ? ? ? ? ? ? ? = "fabric-mmss",
>>> + ? ? ? .id ? ? ? ? ? ? ? ? ? ? = -1,
>>> +};
>>> +
>>> +int msm_device_register(struct platform_device *pdev)
>>> +{
>>> + ? ? ? pdev->dev.bus = &msm_bus_type;
>>> + ? ? ? /* XXX: Use platform_data to assign pdev->dev.parent */
>>> +
>>> + ? ? ? device_initialize(&pdev->dev);
>>> + ? ? ? return __platform_device_add(pdev);
>>> +}
>>> +
>
> With the understanding that I do not own the code that would be actually
> doing this, hopefully some pseduo-code can help?
>
> int msm_device_register(struct platform_device *pdev)
> {
> ? ? ? ?pdev->dev.bus = &msm_bus_type;
>
> ? ? ? ?if (!strncmp(pdev->name, "root", strlen("root"))) {
> ? ? ? ? ? ? ? ?/* We are a root fabric (physical bus), no parent */
> ? ? ? ? ? ? ? ?pdev->dev.parent = NULL;

Parent is null by default.

> ? ? ? ?}
> ? ? ? ?else {
> ? ? ? ? ? ? ? ?struct msm_dev_info* info = pdev->dev.platform_data;
> ? ? ? ? ? ? ? ?switch(info->magic) {
> ? ? ? ? ? ? ? ?case APPS_MAGIC:
> ? ? ? ? ? ? ? ? ? ? ? ?pdev->dev.parent = &msm_apps_bus;
> ? ? ? ? ? ? ? ? ? ? ? ?break;
> ? ? ? ? ? ? ? ?case SYSTEM_MAGIC:
> ? ? ? ? ? ? ? ? ? ? ? ?pdev->dev.parent = &msm_system_bus;
> ? ? ? ? ? ? ? ? ? ? ? ?break;
> ? ? ? ? ? ? ? ?case MMSS_MAGIC:
> ? ? ? ? ? ? ? ? ? ? ? ?pdev->dev.parent = &msm_mmss_bus;
> ? ? ? ? ? ? ? ? ? ? ? ?break;
> ? ? ? ? ? ? ? ?}
> ? ? ? ?}
>
> ? ? ? ?device_initailize(&pdev->dev);
> ? ? ? ?return __platform_device_add(pdev);
> }

Yikes, don't do this.  The parent bus information can be statically
assigned to the devices definitions themselves (see example at end of
email).  Doing it in code is opaque and too complex.

> int msm_bus_probe(struct device* dev)
> {
> ? ? ? ?int error;
> ? ? ? ?struct msm_dev_info* info = dev->platform_data;
> ? ? ? ?struct sibling_device* sib;
>
> ? ? ? ?list_for_each_entry(sib, &info->sib_list, list) {
> ? ? ? ? ? ? ? ?error = build_path(dev, sib->dev, sib->requirements);
> ? ? ? ? ? ? ? ?if (error)
> ? ? ? ? ? ? ? ? ? ? ? ?return error;
> ? ? ? ?}
>
> ? ? ? ?if (dev->driver->probe)
> ? ? ? ? ? ? ? ?error = dev->driver->probe(dev);
>
> ? ? ? ?return error;
> }
>
> What you see here is handling the fact that "fabrics" are actually separate
> buses, so we must be intelligent in adding devices so they attach to the
> right parent. ?When probing devices, they may have other devices they need
> to talk to, which may be on a different fabric (physical bus), so we need
> to prove that we can build a path from the first device to its sibling,
> meeting all of the requirements (clock speed, bandwidth, etc) along all
> the physical buses along the way.

Proving a path is a valid concern, but from what I see it should
already be supported with the existing platform_bus_type.

On the probing point, I agree.  There are many cases of device
initialization order dependencies which the kernel does not currently
explicitly enforce.  In a lot of cases it happens to work "just by
luck" (but not entirely luck, because the order things get initialized
in rarely changes).  I've been playing with using bus notifiers to
postpone initialization when a dependant device hasn't yet been
initialized.  But that is a problem regardless of bus type (for
example, I had an Ethernet platform_device that needed to wait for an
mdio_device to show up).

> AND NOW, for a completely different argument:
>
> If nothing else, this would greatly help to clean up the current state of
> /sys on embedded devices. From my understanding, the platform bus is a
> "legacy" bus, where things that have no other home go to roost. Let us
> look at my dev box:

Not entirely true.  That was the case originally, but now it also
means (especially in embedded) simple memory mapped devices that must
be explicitly instantiated because the hardware is not able to probe
for them.

>
> ? ? ? ?$ ppannuto at ppannuto-linux:~$ ls /sys/bus
> ? ? ? ?acpi ?i2c ? ? ? mmc ?pci_express ?pnp ? sdio ? spi
> ? ? ? ?hid ? mdio_bus ?pci ?platform ? ? scsi ?serio ?usb
>
> ? ? ? ?ppannuto at ppannuto-linux:~$ ls /sys/bus/platform/devices/
> ? ? ? ?Fixed MDIO bus.0 ?i8042 ?pcspkr ?serial8250
>
> And now my droid:
>
> ? ? ? ?$ ls /sys/bus
> ? ? ? ?platform omapdss i2c spi usb mmc sdio usb-serial w1 hid
>
> ? ? ? ?$ ls /sys/bus/platform/devices
> ? ? ? ?power.0 ram_console.0 omap_mdm_ctrl omap2-nand.0 omapdss sfh7743
> ? ? ? ?bu52014hfv vib-gpio musb_hdrc ehci-omap.0 ohci.0 sholes_bpwake
> ? ? ? ?omap_hdq.0 wl127x-rfkill.0 wl127x-test.0 mmci-omap-hs.0 TIWLAN_SDIO.1
> ? ? ? ?omapvout pvrsrvkm omaplfb android_usb usb_mass_storage omap34xxcam
> ? ? ? ?omap2_mcspi.1 omap2_mcspi.2 ?omap2_mcspi.3 omap2_mcspi.4 omap-uart.1
> ? ? ? ?omap-uart.2 omap-uart.3 omap-mcbsp.1 omap-mcbsp.2 omap-mcbsp.3 omap-mcbsp.4
> ? ? ? ?omap-mcbsp.5 i2c_omap.1 i2c_omap.2 i2c_omap.3 omap_wdt omapfb cpcap-regltr.0
> ? ? ? ?cpcap-regltr.17 cpcap-regltr.1 cpcap-regltr.2 cpcap-regltr.3 cpcap-regltr.4
> ? ? ? ?cpcap-regltr.5 cpcap-regltr.6 cpcap-regltr.7 cpcap-regltr.8 cpcap-regltr.9
> ? ? ? ?cpcap-regltr.10 cpcap-regltr.11 cpcap-regltr.12 cpcap-regltr.13 cpcap-regltr.14
> ? ? ? ?cpcap-regltr.15 cpcap-regltr.16 cpcap-regltr.18 cpcap_adc cpcap_key cpcap_battery
> ? ? ? ?button-backlight notification-led keyboard-backlight flash-torch cpcap_usb
> ? ? ? ?cpcap_usb_det cpcap_audio cpcap_3mm5 cpcap_rtc cpcap_uc C6410 keyreset.0
> ? ? ? ?gpio-event.0 device_wifi.1 hp3a omap-previewer omap-resizer.2 alarm
> ? ? ? ?cpcap_usb_charger cpcap_usb_connected

You're looking in the wrong place.  Look in /sys/devices/platform
instead.  /sys/bus/*/devices shows the bus /type/ attachment, not the
physical bus attachement.

To rehash:

/sys/bus - shows the bus types and the devices registered against them
*as symlinks* to the device's sysfs directory
/sys/devices - shows the actual topology from the Linux point of view.

To illustrate, look at /sys/bus/pci_express/devices on a PC:

$ ls -l /sys/bus/pci_express/devices
0000:00:1c.0:pcie01 ->
../../../devices/pci0000:00/0000:00:1c.0/0000:00:1c.0:pcie01
0000:00:1c.0:pcie04 ->
../../../devices/pci0000:00/0000:00:1c.0/0000:00:1c.0:pcie04
0000:00:1c.0:pcie08 ->
../../../devices/pci0000:00/0000:00:1c.0/0000:00:1c.0:pcie08
0000:00:1c.1:pcie01 ->
../../../devices/pci0000:00/0000:00:1c.1/0000:00:1c.1:pcie01
0000:00:1c.1:pcie04 ->
../../../devices/pci0000:00/0000:00:1c.1/0000:00:1c.1:pcie04
0000:00:1c.1:pcie08 ->
../../../devices/pci0000:00/0000:00:1c.1/0000:00:1c.1:pcie08

See?  The pci_express devices actually live in /sys/devices, and on 2
separate physical busses; 0000:00:1c.0 and 0000:00:1c.1

That being said, /sys/devices/platform on your phone probably looks
identical because everything is registered without setting the parent
pointer.  However, it isn't always that way... (see below).

> This is a fairly ugly mess. ?IMHO, it would reflect the view of the world much
> better if most of that lived under /sys/bus/omap (or something similar); such a
> scheme also gives omap (and msm, and others) a home for all of the custom power
> code that is sure to go with their SOCs.

This is the topology argument, which I agree is desirable to represent
accurately for several reasons, but in this case it has nothing to do
with the bus_type.  For example, take a look at my mpc5200 board:

$ ls /sys/bus/platform
f0000000.soc5200	       f0000670.timer		f0002000.serial
f0000200.cdm		       f0000800.rtc		f0003000.ethernet
f0000500.interrupt-controller  f0000900.can		f0003000.mdio
f0000600.timer		       f0000980.can		f0003a00.ata
f0000610.timer		       f0000b00.gpio		f0003d00.i2c
f0000620.timer		       f0000c00.gpio		f0003d40.i2c
f0000630.timer		       f0000f00.spi		f0008000.sram
f0000640.timer		       f0001000.usb		fe000000.flash
f0000650.timer		       f0001200.dma-controller	localbus.0
f0000660.timer		       f0001f00.xlb

$ ls /sys/devices/platform
power  uevent

Lots of platform devices, but none of them live in
/sys/devices/platform.  So, what's going on here?  Well, on powerpc
the platform device topology matches the device initialization data
which already organizes the devices based on the physical bus
attachment.  You can see this by looking where the symlinks in
/sys/bus/platform/devices point to:

$ ls -l /sys/bus/platform/devices
f0000000.soc5200 -> ../../../devices/f0000000.soc5200
f0000200.cdm -> ../../../devices/f0000000.soc5200/f0000200.cdm
[... trimmed for brevity ...]
f0003d40.i2c -> ../../../devices/f0000000.soc5200/f0003d40.i2c
f0008000.sram -> ../../../devices/f0000000.soc5200/f0008000.sram
fe000000.flash -> ../../../devices/localbus.0/fe000000.flash
localbus.0 -> ../../../devices/localbus.0

Most devices are children of the "f0000000.soc5200" physical bus, and
the flash device is a child of the external "localbus.0".  You'll also
notice that both f0000000.soc5200 and localbus.0 are also platform
devices on their own.

So, the topology issue can be solved right now without any regard to
the bus_type.  Also, the converse is true.  New bus_types can be added
to handle different behaviour without changing the existing topology.
Following the topology argument muddies the issue of whether or not
inheriting new bus types from plaform_bus_type is a good idea.

Try this: on one of you msm boards create a new static struct device
or struct platform_device, make sure it gets registered before the
other devices, set the .dev.parent pointer in all the msm_device_uart
platform_devices to point to it.  For example:

struct platform_device msm_bus = {
	.name = "msm_sample_bus",
};

struct platform_device msm_device_uart1 = {
	.name = "msm_serial",
	.id = 1,
	.num_resources = ARRAY_SIZE(resource_uart3),
	.resource = resources_uart3,
	.dev = {
		.parent = &msm_bus.dev,
	},
};

Then look at the effect on the system.  Data like topology can and
should be defined in a static manor; either like the above, or as I
prefer, in a flattened device tree file.

Hmmm, I've written a lot.  I should summarize.  On the topology front,
it is a separate issue, and can be solved right now without a new bus
type.

On the new bus_type front, I'm still not convinced.  However,
supporting PM operations is the most likely line of argument that
would sway me.  I've got to reply to Kevin's email and I'll elaborate
there.  I'm particularly concerned about the concept of sharing device
driver instances between bus_types.

Cheers,
g.

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

[PATCH 4/4] msm: serial: Move msm_uart_driver onto msm bus

[Patrick Pannuto]

Proof of concept; move one device / driver pair

Change-Id: I1afb6f54e6574057699db5b8f9fb7f4456a52010
Signed-off-by: Patrick Pannuto <ppannuto@codeaurora.org>
---
 arch/arm/mach-msm/board-qsd8x50.c |    3 ++-
 drivers/serial/msm_serial.c       |    4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-msm/board-qsd8x50.c b/arch/arm/mach-msm/board-qsd8x50.c
index e3cc807..0deb369 100644
--- a/arch/arm/mach-msm/board-qsd8x50.c
+++ b/arch/arm/mach-msm/board-qsd8x50.c
@@ -27,6 +27,7 @@
 #include <asm/setup.h>
 
 #include <mach/board.h>
+#include <mach/msm_device.h>
 #include <mach/irqs.h>
 #include <mach/sirc.h>
 #include <mach/gpio.h>
@@ -65,7 +66,7 @@ static void __init qsd8x50_init_irq(void)
 static void __init qsd8x50_init(void)
 {
 	msm8x50_init_uart3();
-	platform_add_devices(devices, ARRAY_SIZE(devices));
+	msm_device_register(&msm_device_uart3);
 }
 
 MACHINE_START(QSD8X50_SURF, "QCT QSD8X50 SURF")
diff --git a/drivers/serial/msm_serial.c b/drivers/serial/msm_serial.c
index f8c816e..3332fe7 100644
--- a/drivers/serial/msm_serial.c
+++ b/drivers/serial/msm_serial.c
@@ -32,6 +32,8 @@
 #include <linux/clk.h>
 #include <linux/platform_device.h>
 
+#include <mach/msm_device.h>
+
 #include "msm_serial.h"
 
 struct msm_port {
@@ -732,7 +734,7 @@ static int __init msm_serial_init(void)
 	if (unlikely(ret))
 		return ret;
 
-	ret = platform_driver_probe(&msm_platform_driver, msm_serial_probe);
+	ret = msm_driver_probe(&msm_platform_driver, msm_serial_probe);
 	if (unlikely(ret))
 		uart_unregister_driver(&msm_uart_driver);
 
-- 
1.7.2.1

[PATCH 4/4] msm: serial: Move msm_uart_driver onto msm bus

[Patrick Pannuto]

Proof of concept; move one device / driver pair

Change-Id: I1afb6f54e6574057699db5b8f9fb7f4456a52010
Signed-off-by: Patrick Pannuto <ppannuto@codeaurora.org>
---
 arch/arm/mach-msm/board-qsd8x50.c |    3 ++-
 drivers/serial/msm_serial.c       |    4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-msm/board-qsd8x50.c b/arch/arm/mach-msm/board-qsd8x50.c
index e3cc807..0deb369 100644
--- a/arch/arm/mach-msm/board-qsd8x50.c
+++ b/arch/arm/mach-msm/board-qsd8x50.c
@@ -27,6 +27,7 @@
 #include <asm/setup.h>
 
 #include <mach/board.h>
+#include <mach/msm_device.h>
 #include <mach/irqs.h>
 #include <mach/sirc.h>
 #include <mach/gpio.h>
@@ -65,7 +66,7 @@ static void __init qsd8x50_init_irq(void)
 static void __init qsd8x50_init(void)
 {
 	msm8x50_init_uart3();
-	platform_add_devices(devices, ARRAY_SIZE(devices));
+	msm_device_register(&msm_device_uart3);
 }
 
 MACHINE_START(QSD8X50_SURF, "QCT QSD8X50 SURF")
diff --git a/drivers/serial/msm_serial.c b/drivers/serial/msm_serial.c
index f8c816e..3332fe7 100644
--- a/drivers/serial/msm_serial.c
+++ b/drivers/serial/msm_serial.c
@@ -32,6 +32,8 @@
 #include <linux/clk.h>
 #include <linux/platform_device.h>
 
+#include <mach/msm_device.h>
+
 #include "msm_serial.h"
 
 struct msm_port {
@@ -732,7 +734,7 @@ static int __init msm_serial_init(void)
 	if (unlikely(ret))
 		return ret;
 
-	ret = platform_driver_probe(&msm_platform_driver, msm_serial_probe);
+	ret = msm_driver_probe(&msm_platform_driver, msm_serial_probe);
 	if (unlikely(ret))
 		uart_unregister_driver(&msm_uart_driver);
 
-- 
1.7.2.1