"Gopinath, Thara" <th...@ti.com> writes:
>>>-----Original Message-----
>>>From: linux-omap-ow...@vger.kernel.org
>>>[mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Kevin
>>>Hilman
>>>Sent: Wednesday, September 01, 2010 5:33 AM
>>>To: linux-omap@vger.kernel.org
>>>Cc: p...@pwsan.com
>>>Subject: [PATCH 2/2] OMAP: omap_device: make all devices a child of a new
>>>omap_bus device
>>>
>>>From: Kevin Hilman <khil...@ti.com>
>>>
>>>In order to help differentiate omap_devices from normal
>>>platform_devices, make them all a parent of a new omap_bus device.
>>>
>>>Then, in order to determine if a platform_device is also an
>>>omap_device, checking the parent is all that is needed.
>>>
>>>Users of this feature are the runtime PM core for OMAP, where we need
>>>to know if a device being passed in is an omap_device or not in order
>>>to know whether to call the omap_device API with it.
>>>
>>>In addition, all omap_devices will now show up under /sys/devices/omap
>>>instead of /sys/devices/platform
>
> Hello Kevin,
>
> Couple of minor queries/comments below.
Hi Thara,
Thanks for the review.
>>>
>>>Signed-off-by: Kevin Hilman <khil...@ti.com>
>>>---
>>> arch/arm/plat-omap/include/plat/omap_device.h | 2 ++
>>> arch/arm/plat-omap/omap_device.c | 18 ++++++++++++++++++
>>> 2 files changed, 20 insertions(+), 0 deletions(-)
>>>
>>>diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-
>>>omap/include/plat/omap_device.h
>>>index bad4c3d..26d0c10 100644
>>>--- a/arch/arm/plat-omap/include/plat/omap_device.h
>>>+++ b/arch/arm/plat-omap/include/plat/omap_device.h
>>>@@ -36,6 +36,8 @@
>>>
>>> #include <plat/omap_hwmod.h>
>>>
>>>+extern struct device omap_bus;
>>>+
>
> Why is this extern declaration needed? Is it later on
> to check in runtime pm code pdev->dev.parent == omap_bus??
Yes.
>>> /* omap_device._state values */
>>> #define OMAP_DEVICE_STATE_UNKNOWN 0
>>> #define OMAP_DEVICE_STATE_ENABLED 1
>>>diff --git a/arch/arm/plat-omap/omap_device.c
>>>b/arch/arm/plat-omap/omap_device.c
>>>index 7f05f49..3e215fa 100644
>>>--- a/arch/arm/plat-omap/omap_device.c
>>>+++ b/arch/arm/plat-omap/omap_device.c
>>>@@ -463,8 +463,11 @@ int omap_early_device_register(struct omap_device *od)
>>> */
>>> int omap_device_register(struct omap_device *od)
>>> {
>>>+ struct platform_device *pdev = &od->pdev;
>>>+
>>> pr_debug("omap_device: %s: registering\n", od->pdev.name);
>>>
>>>+ pdev->dev.parent = &omap_bus;
>
> What if device_register(&omap_bus) has returned an
> error? Do we still go ahead with assigning omap_bus
> as parent?
Good point. I followed the lead of the platform_bus here which does the
same thing, and bascially assumes that device_register() does not fail.
I'll take a look to see if this could be handled cleaner. Thanks.
>>> return platform_device_register(&od->pdev);
>>> }
>>>
>>>@@ -737,3 +740,18 @@ int omap_device_enable_clocks(struct omap_device *od)
>>> /* XXX pass along return value here? */
>>> return 0;
>>> }
>>>+
>>>+struct device omap_bus = {
>>>+ .init_name = "omap",
>>>+};
>>>+
>>>+static int __init omap_device_init(void)
>>>+{
>>>+ int error = 0;
>
> Is the initialization to 0 needed?
Nope
>>>+
>>>+ printk("%s:\n", __func__);
>
> Spurious printk???
Yup.
Oops, I cleaned those two up locally but sent the wrong version.
Thanks for catching,
Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
