[PATCH 5/9 v3] usb: musb: Using omap_device_build for musb device registration

[PATCH 5/9 v3] usb: musb: Using omap_device_build for musb device registration

[Hema HK]

Using omap_device_build api instead of platform_device_register for musb
device registration.The device specific resources defined in centralized
database will be used. So removed the resource definitions from the musb
platform file.

Signed-off-by: Hema HK <hemahk-l0cyMroinI0@public.gmane.org>
Cc: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
Cc: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Cc: Kevin Hilman <khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org>
Cc: Cousson, Benoit <b-cousson-l0cyMroinI0@public.gmane.org>
Cc: Paul Walmsley <paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org>

---
 arch/arm/mach-omap2/usb-musb.c |   75 ++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 37 deletions(-)

Index: linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
===================================================================
--- linux-omap-pm.orig/arch/arm/mach-omap2/usb-musb.c
+++ linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
@@ -29,24 +29,11 @@
 #include <mach/hardware.h>
 #include <mach/irqs.h>
 #include <plat/usb.h>
+#include <plat/omap_device.h>
 
 #ifdef CONFIG_USB_MUSB_SOC
-
-static struct resource musb_resources[] = {
-	[0] = { /* start and end set dynamically */
-		.flags	= IORESOURCE_MEM,
-	},
-	[1] = {	/* general IRQ */
-		.start	= INT_243X_HS_USB_MC,
-		.flags	= IORESOURCE_IRQ,
-		.name	= "mc",
-	},
-	[2] = {	/* DMA IRQ */
-		.start	= INT_243X_HS_USB_DMA,
-		.flags	= IORESOURCE_IRQ,
-		.name	= "dma",
-	},
-};
+static const char name[] = "musb_hdrc";
+#define MAX_OMAP_MUSB_HWMOD_NAME_LEN	16
 
 static struct musb_hdrc_config musb_config = {
 	.multipoint	= 1,
@@ -75,31 +62,30 @@ static struct musb_hdrc_platform_data mu
 
 static u64 musb_dmamask = DMA_BIT_MASK(32);
 
-static struct platform_device musb_device = {
-	.name		= "musb_hdrc",
-	.id		= -1,
-	.dev = {
-		.dma_mask		= &musb_dmamask,
-		.coherent_dma_mask	= DMA_BIT_MASK(32),
-		.platform_data		= &musb_plat,
+static struct omap_device_pm_latency omap_musb_latency[] = {
+	{
+		.deactivate_func = omap_device_idle_hwmods,
+		.activate_func   = omap_device_enable_hwmods,
+		.flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
 	},
-	.num_resources	= ARRAY_SIZE(musb_resources),
-	.resource	= musb_resources,
 };
 
 void __init usb_musb_init(struct omap_musb_board_data *board_data)
 {
-	if (cpu_is_omap243x()) {
-		musb_resources[0].start = OMAP243X_HS_BASE;
-	} else if (cpu_is_omap34xx()) {
-		musb_resources[0].start = OMAP34XX_HSUSB_OTG_BASE;
-	} else if (cpu_is_omap44xx()) {
-		musb_resources[0].start = OMAP44XX_HSUSB_OTG_BASE;
-		musb_resources[1].start = OMAP44XX_IRQ_HS_USB_MC_N;
-		musb_resources[2].start = OMAP44XX_IRQ_HS_USB_DMA_N;
+	struct omap_hwmod *oh;
+	struct omap_device *od;
+	struct platform_device *pdev;
+	struct device	*dev;
+	int bus_id = -1;
+	const char *oh_name = "usb_otg_hs";
+	struct musb_hdrc_platform_data *pdata;
+
+	oh = omap_hwmod_lookup(oh_name);
+
+	if (!oh) {
+		pr_err("Could not look up %s\n", oh_name);
+		return;
 	}
-	musb_resources[0].end = musb_resources[0].start + SZ_4K - 1;
-
 	/*
 	 * REVISIT: This line can be removed once all the platforms using
 	 * musb_core.c have been converted to use use clkdev.
@@ -110,8 +96,23 @@ void __init usb_musb_init(struct omap_mu
 	musb_plat.mode = board_data->mode;
 	musb_plat.extvbus = board_data->extvbus;
 
-	if (platform_device_register(&musb_device) < 0)
-		printk(KERN_ERR "Unable to register HS-USB (MUSB) device\n");
+	pdata = &musb_plat;
+
+	od = omap_device_build(name, bus_id, oh, pdata,
+			       sizeof(struct musb_hdrc_platform_data),
+						omap_musb_latency,
+			       ARRAY_SIZE(omap_musb_latency), false);
+	if (IS_ERR(od)) {
+		pr_err("Could not build omap_device for %s %s\n",
+						name, oh_name);
+		return;
+	}
+	pdev = &od->pdev;
+	dev = &pdev->dev;
+	get_device(dev);
+	dev->dma_mask = &musb_dmamask;
+	dev->coherent_dma_mask = musb_dmamask;
+	put_device(dev);
 }
 
 #else
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/9 v3] usb: musb: Using omap_device_build for musb device registration

[Felipe Balbi]

Hi,

On Wed, Sep 22, 2010 at 07:29:10PM -0500, Kalliguddi, Hema wrote:
>+#define MAX_OMAP_MUSB_HWMOD_NAME_LEN	16

this isn't used anywhere.

>@@ -75,31 +62,30 @@ static struct musb_hdrc_platform_data mu
>
> static u64 musb_dmamask = DMA_BIT_MASK(32);
>
>-static struct platform_device musb_device = {
>-	.name		= "musb_hdrc",
>-	.id		= -1,
>-	.dev = {
>-		.dma_mask		= &musb_dmamask,
>-		.coherent_dma_mask	= DMA_BIT_MASK(32),
>-		.platform_data		= &musb_plat,
>+static struct omap_device_pm_latency omap_musb_latency[] = {
>+	{
>+		.deactivate_func = omap_device_idle_hwmods,
>+		.activate_func   = omap_device_enable_hwmods,
>+		.flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
> 	},
>-	.num_resources	= ARRAY_SIZE(musb_resources),
>-	.resource	= musb_resources,
> };
>
> void __init usb_musb_init(struct omap_musb_board_data *board_data)
> {
>-	if (cpu_is_omap243x()) {
>-		musb_resources[0].start = OMAP243X_HS_BASE;
>-	} else if (cpu_is_omap34xx()) {
>-		musb_resources[0].start = OMAP34XX_HSUSB_OTG_BASE;
>-	} else if (cpu_is_omap44xx()) {
>-		musb_resources[0].start = OMAP44XX_HSUSB_OTG_BASE;
>-		musb_resources[1].start = OMAP44XX_IRQ_HS_USB_MC_N;
>-		musb_resources[2].start = OMAP44XX_IRQ_HS_USB_DMA_N;
>+	struct omap_hwmod *oh;
>+	struct omap_device *od;
>+	struct platform_device *pdev;
>+	struct device	*dev;
>+	int bus_id = -1;
>+	const char *oh_name = "usb_otg_hs";
>+	struct musb_hdrc_platform_data *pdata;
>+
>+	oh = omap_hwmod_lookup(oh_name);
>+
>+	if (!oh) {
>+		pr_err("Could not look up %s\n", oh_name);
>+		return;
> 	}

Paul, Kevin, to me it looks like a duplication that all devices will
have to:

oh = omap_hwmod_lookup("my_hwmod_name");
omap_device_build("my_device_name", bus_id, oh, pdata, sizeof(*pdata));

could the omap_hwmod_lookup() part be moved to omap_device_build ? Or
maybe create a omap_hwmod_lookup_and_build(oh_name, dev_name, bus_id,
pdata, sizeof(*pdata)) ??

>@@ -110,8 +96,23 @@ void __init usb_musb_init(struct omap_mu
> 	musb_plat.mode = board_data->mode;
> 	musb_plat.extvbus = board_data->extvbus;
>
>-	if (platform_device_register(&musb_device) < 0)
>-		printk(KERN_ERR "Unable to register HS-USB (MUSB) device\n");
>+	pdata = &musb_plat;
>+
>+	od = omap_device_build(name, bus_id, oh, pdata,
>+			       sizeof(struct musb_hdrc_platform_data),

use sizeof(*pdata), if we change the name of that structure (very
unlikely, but still) it'll avoid unwanted compile breakage.

>+	pdev = &od->pdev;
>+	dev = &pdev->dev;
>+	get_device(dev);
>+	dev->dma_mask = &musb_dmamask;
>+	dev->coherent_dma_mask = musb_dmamask;
>+	put_device(dev);

I think this is also a duplication, it's gonna on all hwmod device
registration, no ?

-- 
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 5/9 v3] usb: musb: Using omap_device_build for musb device registration

[Kalliguddi, Hema]

Hi, 

>-----Original Message-----
>From: Balbi, Felipe 
>Sent: Thursday, September 23, 2010 11:51 AM
>To: Kalliguddi, Hema
>Cc: linux-omap@vger.kernel.org; linux-usb@vger.kernel.org; 
>Balbi, Felipe; Tony Lindgren; Kevin Hilman; Cousson, Benoit; 
>Paul Walmsley
>Subject: Re: [PATCH 5/9 v3] usb: musb: Using omap_device_build 
>for musb device registration
>
>Hi,
>
>On Wed, Sep 22, 2010 at 07:29:10PM -0500, Kalliguddi, Hema wrote:
>>+#define MAX_OMAP_MUSB_HWMOD_NAME_LEN	16
>
>this isn't used anywhere.
>
>>@@ -75,31 +62,30 @@ static struct musb_hdrc_platform_data mu
>>
>> static u64 musb_dmamask = DMA_BIT_MASK(32);
>>
>>-static struct platform_device musb_device = {
>>-	.name		= "musb_hdrc",
>>-	.id		= -1,
>>-	.dev = {
>>-		.dma_mask		= &musb_dmamask,
>>-		.coherent_dma_mask	= DMA_BIT_MASK(32),
>>-		.platform_data		= &musb_plat,
>>+static struct omap_device_pm_latency omap_musb_latency[] = {
>>+	{
>>+		.deactivate_func = omap_device_idle_hwmods,
>>+		.activate_func   = omap_device_enable_hwmods,
>>+		.flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
>> 	},
>>-	.num_resources	= ARRAY_SIZE(musb_resources),
>>-	.resource	= musb_resources,
>> };
>>
>> void __init usb_musb_init(struct omap_musb_board_data *board_data)
>> {
>>-	if (cpu_is_omap243x()) {
>>-		musb_resources[0].start = OMAP243X_HS_BASE;
>>-	} else if (cpu_is_omap34xx()) {
>>-		musb_resources[0].start = OMAP34XX_HSUSB_OTG_BASE;
>>-	} else if (cpu_is_omap44xx()) {
>>-		musb_resources[0].start = OMAP44XX_HSUSB_OTG_BASE;
>>-		musb_resources[1].start = OMAP44XX_IRQ_HS_USB_MC_N;
>>-		musb_resources[2].start = OMAP44XX_IRQ_HS_USB_DMA_N;
>>+	struct omap_hwmod *oh;
>>+	struct omap_device *od;
>>+	struct platform_device *pdev;
>>+	struct device	*dev;
>>+	int bus_id = -1;
>>+	const char *oh_name = "usb_otg_hs";
>>+	struct musb_hdrc_platform_data *pdata;
>>+
>>+	oh = omap_hwmod_lookup(oh_name);
>>+
>>+	if (!oh) {
>>+		pr_err("Could not look up %s\n", oh_name);
>>+		return;
>> 	}
>
>Paul, Kevin, to me it looks like a duplication that all devices will
>have to:
>
>oh = omap_hwmod_lookup("my_hwmod_name");
>omap_device_build("my_device_name", bus_id, oh, pdata, sizeof(*pdata));
>
>could the omap_hwmod_lookup() part be moved to omap_device_build ? Or
>maybe create a omap_hwmod_lookup_and_build(oh_name, dev_name, bus_id,
>pdata, sizeof(*pdata)) ??

>>@@ -110,8 +96,23 @@ void __init usb_musb_init(struct omap_mu
>> 	musb_plat.mode = board_data->mode;
>> 	musb_plat.extvbus = board_data->extvbus;
>>
>>-	if (platform_device_register(&musb_device) < 0)
>>-		printk(KERN_ERR "Unable to register HS-USB 
>(MUSB) device\n");
>>+	pdata = &musb_plat;
>>+
>>+	od = omap_device_build(name, bus_id, oh, pdata,
>>+			       sizeof(struct musb_hdrc_platform_data),
>
>use sizeof(*pdata), if we change the name of that structure (very
>unlikely, but still) it'll avoid unwanted compile breakage.
>
Ok. Make sense.

>>+	pdev = &od->pdev;
>>+	dev = &pdev->dev;
>>+	get_device(dev);
>>+	dev->dma_mask = &musb_dmamask;
>>+	dev->coherent_dma_mask = musb_dmamask;
>>+	put_device(dev);
>
>I think this is also a duplication, it's gonna on all hwmod device
>registration, no ?

Any driver which uses the device->dma_mask will have to it after the device build.
~Hema
>
>-- 
>balbi
>

Re: [PATCH 5/9 v3] usb: musb: Using omap_device_build for musb device registration

[Felipe Balbi]

Hi,

On Thu, Sep 23, 2010 at 01:46:51AM -0500, Kalliguddi, Hema wrote:
>Any driver which uses the device->dma_mask will have to it after the
>device build.

yeah, let's see what Kevin, Paul and Tony say about it.

-- 
balbi

Re: [PATCH 5/9 v3] usb: musb: Using omap_device_build for musb device registration

[Kevin Hilman]

Felipe Balbi <balbi@ti.com> writes:

> Hi,
>
> On Wed, Sep 22, 2010 at 07:29:10PM -0500, Kalliguddi, Hema wrote:
>>+#define MAX_OMAP_MUSB_HWMOD_NAME_LEN	16
>
> this isn't used anywhere.
>
>>@@ -75,31 +62,30 @@ static struct musb_hdrc_platform_data mu
>>
>> static u64 musb_dmamask = DMA_BIT_MASK(32);
>>
>>-static struct platform_device musb_device = {
>>-	.name		= "musb_hdrc",
>>-	.id		= -1,
>>-	.dev = {
>>-		.dma_mask		= &musb_dmamask,
>>-		.coherent_dma_mask	= DMA_BIT_MASK(32),
>>-		.platform_data		= &musb_plat,
>>+static struct omap_device_pm_latency omap_musb_latency[] = {
>>+	{
>>+		.deactivate_func = omap_device_idle_hwmods,
>>+		.activate_func   = omap_device_enable_hwmods,
>>+		.flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
>> 	},
>>-	.num_resources	= ARRAY_SIZE(musb_resources),
>>-	.resource	= musb_resources,
>> };
>>
>> void __init usb_musb_init(struct omap_musb_board_data *board_data)
>> {
>>-	if (cpu_is_omap243x()) {
>>-		musb_resources[0].start = OMAP243X_HS_BASE;
>>-	} else if (cpu_is_omap34xx()) {
>>-		musb_resources[0].start = OMAP34XX_HSUSB_OTG_BASE;
>>-	} else if (cpu_is_omap44xx()) {
>>-		musb_resources[0].start = OMAP44XX_HSUSB_OTG_BASE;
>>-		musb_resources[1].start = OMAP44XX_IRQ_HS_USB_MC_N;
>>-		musb_resources[2].start = OMAP44XX_IRQ_HS_USB_DMA_N;
>>+	struct omap_hwmod *oh;
>>+	struct omap_device *od;
>>+	struct platform_device *pdev;
>>+	struct device	*dev;
>>+	int bus_id = -1;
>>+	const char *oh_name = "usb_otg_hs";
>>+	struct musb_hdrc_platform_data *pdata;
>>+
>>+	oh = omap_hwmod_lookup(oh_name);
>>+
>>+	if (!oh) {
>>+		pr_err("Could not look up %s\n", oh_name);
>>+		return;
>> 	}
>
> Paul, Kevin, to me it looks like a duplication that all devices will
> have to:
>
> oh = omap_hwmod_lookup("my_hwmod_name");
> omap_device_build("my_device_name", bus_id, oh, pdata, sizeof(*pdata));
>
> could the omap_hwmod_lookup() part be moved to omap_device_build ? Or
> maybe create a omap_hwmod_lookup_and_build(oh_name, dev_name, bus_id,
> pdata, sizeof(*pdata)) ??

I don't think that this is too much extra work.

Also, many drivers are not doing a single hwmod lookup, they are using
an iterator to iterate over a bunch of hwmods in a given class (c.f.
omap_hwmod_for_each_by_class)

So, IMO, keeping the lookup and build separate is better.

>>@@ -110,8 +96,23 @@ void __init usb_musb_init(struct omap_mu
>> 	musb_plat.mode = board_data->mode;
>> 	musb_plat.extvbus = board_data->extvbus;
>>
>>-	if (platform_device_register(&musb_device) < 0)
>>-		printk(KERN_ERR "Unable to register HS-USB (MUSB) device\n");
>>+	pdata = &musb_plat;
>>+
>>+	od = omap_device_build(name, bus_id, oh, pdata,
>>+			       sizeof(struct musb_hdrc_platform_data),
>
> use sizeof(*pdata), if we change the name of that structure (very
> unlikely, but still) it'll avoid unwanted compile breakage.
>
>>+	pdev = &od->pdev;
>>+	dev = &pdev->dev;
>>+	get_device(dev);
>>+	dev->dma_mask = &musb_dmamask;
>>+	dev->coherent_dma_mask = musb_dmamask;
>>+	put_device(dev);
>
> I think this is also a duplication, it's gonna on all hwmod device
> registration, no ?

Only for devices setting DMA masks.

This is no more duplication than we have today for all platform_devices.

Kevin

Re: [PATCH 5/9 v3] usb: musb: Using omap_device_build for musb device registration

[Felipe Balbi]

Hi,

On Thu, Sep 23, 2010 at 10:23:17AM -0500, Kevin Hilman wrote:
>> could the omap_hwmod_lookup() part be moved to omap_device_build ? Or
>> maybe create a omap_hwmod_lookup_and_build(oh_name, dev_name, bus_id,
>> pdata, sizeof(*pdata)) ??
>
>I don't think that this is too much extra work.
>
>Also, many drivers are not doing a single hwmod lookup, they are using
>an iterator to iterate over a bunch of hwmods in a given class (c.f.
>omap_hwmod_for_each_by_class)
>
>So, IMO, keeping the lookup and build separate is better.
>
>> I think this is also a duplication, it's gonna on all hwmod device
>> registration, no ?
>
>Only for devices setting DMA masks.
>
>This is no more duplication than we have today for all platform_devices.

ok, agreed.

-- 
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/9 v3] usb: musb: Using omap_device_build for musb device registration

[Paul Walmsley]

Hi Felipe,

On Thu, 23 Sep 2010, Felipe Balbi wrote:

> Paul, Kevin, to me it looks like a duplication that all devices will
> have to:
> 
> oh = omap_hwmod_lookup("my_hwmod_name");
> omap_device_build("my_device_name", bus_id, oh, pdata, sizeof(*pdata));
> 
> could the omap_hwmod_lookup() part be moved to omap_device_build ? Or
> maybe create a omap_hwmod_lookup_and_build(oh_name, dev_name, bus_id,
> pdata, sizeof(*pdata)) ??

... (re dma mask) ...

> I think this is also a duplication, it's gonna on all hwmod device
> registration, no ?

Probably at some point in the future, once a larger set of hwmod 
conversions are done, we can look at the way that most of the integration 
code initializes.  Ideally, we can combine as much common code together as 
possible into some data-driven approach.  But right now I think it's a 
little early to do that, since I personally am not sure what most 
drivers/devices will need to do in their integration code.  On the other 
hand, it's not too early to start thinking about it :-)


- Paul