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

[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

[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

[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

[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