[PATCH 3/5 v12] arm: omap: usb: register hwmods of usbhs

[PATCH 3/5 v12] arm: omap: usb: register hwmods of usbhs

[Keshava Munegowda]

The hwmod structure of usb_host_hs  and usb_tll are
retrieved and registered with omap device

Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
Reviewed-by: Partha Basak <parthab@india.ti.com>
---
 arch/arm/mach-omap2/usb-host.c |  100 ++++++++++++++--------------------------
 1 files changed, 34 insertions(+), 66 deletions(-)

diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
index 89ae298..771dc78 100644
--- a/arch/arm/mach-omap2/usb-host.c
+++ b/arch/arm/mach-omap2/usb-host.c
@@ -28,51 +28,28 @@
 #include <mach/hardware.h>
 #include <mach/irqs.h>
 #include <plat/usb.h>
+#include <plat/omap_device.h>
 
 #include "mux.h"
 
 #ifdef CONFIG_MFD_OMAP_USB_HOST
 
-#define OMAP_USBHS_DEVICE	"usbhs-omap"
-
-static struct resource usbhs_resources[] = {
-	{
-		.name	= "uhh",
-		.flags	= IORESOURCE_MEM,
-	},
-	{
-		.name	= "tll",
-		.flags	= IORESOURCE_MEM,
-	},
-	{
-		.name	= "ehci",
-		.flags	= IORESOURCE_MEM,
-	},
-	{
-		.name	= "ehci-irq",
-		.flags	= IORESOURCE_IRQ,
-	},
-	{
-		.name	= "ohci",
-		.flags	= IORESOURCE_MEM,
-	},
-	{
-		.name	= "ohci-irq",
-		.flags	= IORESOURCE_IRQ,
-	}
-};
-
-static struct platform_device usbhs_device = {
-	.name		= OMAP_USBHS_DEVICE,
-	.id		= 0,
-	.num_resources	= ARRAY_SIZE(usbhs_resources),
-	.resource	= usbhs_resources,
-};
+#define OMAP_USBHS_DEVICE	"usbhs_omap"
+#define	USBHS_UHH_HWMODNAME	"usb_host_hs"
+#define USBHS_TLL_HWMODNAME	"usb_tll_hs"
 
 static struct usbhs_omap_platform_data		usbhs_data;
 static struct ehci_hcd_omap_platform_data	ehci_data;
 static struct ohci_hcd_omap_platform_data	ohci_data;
 
+static struct omap_device_pm_latency omap_uhhtll_latency[] = {
+	  {
+		.deactivate_func = omap_device_idle_hwmods,
+		.activate_func	 = omap_device_enable_hwmods,
+		.flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
+	  },
+};
+
 /* MUX settings for EHCI pins */
 /*
  * setup_ehci_io_mux - initialize IO pad mux for USBHOST
@@ -508,7 +485,10 @@ static void setup_4430ohci_io_mux(const enum usbhs_omap_port_mode *port_mode)
 
 void __init usbhs_init(const struct usbhs_omap_board_data *pdata)
 {
-	int	i;
+	struct omap_hwmod	*oh[2];
+	struct omap_device	*od;
+	int			bus_id = -1;
+	int			i;
 
 	for (i = 0; i < OMAP3_HS_USB_PORTS; i++) {
 		usbhs_data.port_mode[i] = pdata->port_mode[i];
@@ -523,44 +503,34 @@ void __init usbhs_init(const struct usbhs_omap_board_data *pdata)
 	usbhs_data.ohci_data = &ohci_data;
 
 	if (cpu_is_omap34xx()) {
-		usbhs_resources[0].start = OMAP34XX_UHH_CONFIG_BASE;
-		usbhs_resources[0].end = OMAP34XX_UHH_CONFIG_BASE + SZ_1K - 1;
-		usbhs_resources[1].start = OMAP34XX_USBTLL_BASE;
-		usbhs_resources[1].end = OMAP34XX_USBTLL_BASE + SZ_4K - 1;
-		usbhs_resources[2].start	= OMAP34XX_EHCI_BASE;
-		usbhs_resources[2].end	= OMAP34XX_EHCI_BASE + SZ_1K - 1;
-		usbhs_resources[3].start = INT_34XX_EHCI_IRQ;
-		usbhs_resources[4].start	= OMAP34XX_OHCI_BASE;
-		usbhs_resources[4].end	= OMAP34XX_OHCI_BASE + SZ_1K - 1;
-		usbhs_resources[5].start = INT_34XX_OHCI_IRQ;
 		setup_ehci_io_mux(pdata->port_mode);
 		setup_ohci_io_mux(pdata->port_mode);
 	} else if (cpu_is_omap44xx()) {
-		usbhs_resources[0].start = OMAP44XX_UHH_CONFIG_BASE;
-		usbhs_resources[0].end = OMAP44XX_UHH_CONFIG_BASE + SZ_1K - 1;
-		usbhs_resources[1].start = OMAP44XX_USBTLL_BASE;
-		usbhs_resources[1].end = OMAP44XX_USBTLL_BASE + SZ_4K - 1;
-		usbhs_resources[2].start = OMAP44XX_HSUSB_EHCI_BASE;
-		usbhs_resources[2].end = OMAP44XX_HSUSB_EHCI_BASE + SZ_1K - 1;
-		usbhs_resources[3].start = OMAP44XX_IRQ_EHCI;
-		usbhs_resources[4].start = OMAP44XX_HSUSB_OHCI_BASE;
-		usbhs_resources[4].end = OMAP44XX_HSUSB_OHCI_BASE + SZ_1K - 1;
-		usbhs_resources[5].start = OMAP44XX_IRQ_OHCI;
 		setup_4430ehci_io_mux(pdata->port_mode);
 		setup_4430ohci_io_mux(pdata->port_mode);
 	}
 
-	if (platform_device_add_data(&usbhs_device,
-				&usbhs_data, sizeof(usbhs_data)) < 0) {
-		printk(KERN_ERR "USBHS platform_device_add_data failed\n");
-		goto init_end;
+	oh[0] = omap_hwmod_lookup(USBHS_UHH_HWMODNAME);
+	if (!oh[0]) {
+		pr_err("Could not look up %s\n", USBHS_UHH_HWMODNAME);
+		return;
 	}
 
-	if (platform_device_register(&usbhs_device) < 0)
-		printk(KERN_ERR "USBHS platform_device_register failed\n");
+	oh[1] = omap_hwmod_lookup(USBHS_TLL_HWMODNAME);
+	if (!oh[1]) {
+		pr_err("Could not look up %s\n", USBHS_TLL_HWMODNAME);
+		return;
+	}
 
-init_end:
-	return;
+	od = omap_device_build_ss(OMAP_USBHS_DEVICE, bus_id, oh, 2,
+				(void *)&usbhs_data, sizeof(usbhs_data),
+				omap_uhhtll_latency,
+				ARRAY_SIZE(omap_uhhtll_latency), false);
+	if (IS_ERR(od)) {
+		pr_err("Could not build hwmod devices %s,%s\n",
+			USBHS_UHH_HWMODNAME, USBHS_TLL_HWMODNAME);
+		return;
+	}
 }
 
 #else
@@ -570,5 +540,3 @@ void __init usbhs_init(const struct usbhs_omap_board_data *pdata)
 }
 
 #endif
-
-
-- 
1.6.0.4

[PATCH 3/5 v12] arm: omap: usb: register hwmods of usbhs

[Keshava Munegowda]

The hwmod structure of usb_host_hs  and usb_tll are
retrieved and registered with omap device

Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
Reviewed-by: Partha Basak <parthab@india.ti.com>
---
 arch/arm/mach-omap2/usb-host.c |  100 ++++++++++++++--------------------------
 1 files changed, 34 insertions(+), 66 deletions(-)

diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
index 89ae298..771dc78 100644
--- a/arch/arm/mach-omap2/usb-host.c
+++ b/arch/arm/mach-omap2/usb-host.c
@@ -28,51 +28,28 @@
 #include <mach/hardware.h>
 #include <mach/irqs.h>
 #include <plat/usb.h>
+#include <plat/omap_device.h>
 
 #include "mux.h"
 
 #ifdef CONFIG_MFD_OMAP_USB_HOST
 
-#define OMAP_USBHS_DEVICE	"usbhs-omap"
-
-static struct resource usbhs_resources[] = {
-	{
-		.name	= "uhh",
-		.flags	= IORESOURCE_MEM,
-	},
-	{
-		.name	= "tll",
-		.flags	= IORESOURCE_MEM,
-	},
-	{
-		.name	= "ehci",
-		.flags	= IORESOURCE_MEM,
-	},
-	{
-		.name	= "ehci-irq",
-		.flags	= IORESOURCE_IRQ,
-	},
-	{
-		.name	= "ohci",
-		.flags	= IORESOURCE_MEM,
-	},
-	{
-		.name	= "ohci-irq",
-		.flags	= IORESOURCE_IRQ,
-	}
-};
-
-static struct platform_device usbhs_device = {
-	.name		= OMAP_USBHS_DEVICE,
-	.id		= 0,
-	.num_resources	= ARRAY_SIZE(usbhs_resources),
-	.resource	= usbhs_resources,
-};
+#define OMAP_USBHS_DEVICE	"usbhs_omap"
+#define	USBHS_UHH_HWMODNAME	"usb_host_hs"
+#define USBHS_TLL_HWMODNAME	"usb_tll_hs"
 
 static struct usbhs_omap_platform_data		usbhs_data;
 static struct ehci_hcd_omap_platform_data	ehci_data;
 static struct ohci_hcd_omap_platform_data	ohci_data;
 
+static struct omap_device_pm_latency omap_uhhtll_latency[] = {
+	  {
+		.deactivate_func = omap_device_idle_hwmods,
+		.activate_func	 = omap_device_enable_hwmods,
+		.flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
+	  },
+};
+
 /* MUX settings for EHCI pins */
 /*
  * setup_ehci_io_mux - initialize IO pad mux for USBHOST
@@ -508,7 +485,10 @@ static void setup_4430ohci_io_mux(const enum usbhs_omap_port_mode *port_mode)
 
 void __init usbhs_init(const struct usbhs_omap_board_data *pdata)
 {
-	int	i;
+	struct omap_hwmod	*oh[2];
+	struct omap_device	*od;
+	int			bus_id = -1;
+	int			i;
 
 	for (i = 0; i < OMAP3_HS_USB_PORTS; i++) {
 		usbhs_data.port_mode[i] = pdata->port_mode[i];
@@ -523,44 +503,34 @@ void __init usbhs_init(const struct usbhs_omap_board_data *pdata)
 	usbhs_data.ohci_data = &ohci_data;
 
 	if (cpu_is_omap34xx()) {
-		usbhs_resources[0].start = OMAP34XX_UHH_CONFIG_BASE;
-		usbhs_resources[0].end = OMAP34XX_UHH_CONFIG_BASE + SZ_1K - 1;
-		usbhs_resources[1].start = OMAP34XX_USBTLL_BASE;
-		usbhs_resources[1].end = OMAP34XX_USBTLL_BASE + SZ_4K - 1;
-		usbhs_resources[2].start	= OMAP34XX_EHCI_BASE;
-		usbhs_resources[2].end	= OMAP34XX_EHCI_BASE + SZ_1K - 1;
-		usbhs_resources[3].start = INT_34XX_EHCI_IRQ;
-		usbhs_resources[4].start	= OMAP34XX_OHCI_BASE;
-		usbhs_resources[4].end	= OMAP34XX_OHCI_BASE + SZ_1K - 1;
-		usbhs_resources[5].start = INT_34XX_OHCI_IRQ;
 		setup_ehci_io_mux(pdata->port_mode);
 		setup_ohci_io_mux(pdata->port_mode);
 	} else if (cpu_is_omap44xx()) {
-		usbhs_resources[0].start = OMAP44XX_UHH_CONFIG_BASE;
-		usbhs_resources[0].end = OMAP44XX_UHH_CONFIG_BASE + SZ_1K - 1;
-		usbhs_resources[1].start = OMAP44XX_USBTLL_BASE;
-		usbhs_resources[1].end = OMAP44XX_USBTLL_BASE + SZ_4K - 1;
-		usbhs_resources[2].start = OMAP44XX_HSUSB_EHCI_BASE;
-		usbhs_resources[2].end = OMAP44XX_HSUSB_EHCI_BASE + SZ_1K - 1;
-		usbhs_resources[3].start = OMAP44XX_IRQ_EHCI;
-		usbhs_resources[4].start = OMAP44XX_HSUSB_OHCI_BASE;
-		usbhs_resources[4].end = OMAP44XX_HSUSB_OHCI_BASE + SZ_1K - 1;
-		usbhs_resources[5].start = OMAP44XX_IRQ_OHCI;
 		setup_4430ehci_io_mux(pdata->port_mode);
 		setup_4430ohci_io_mux(pdata->port_mode);
 	}
 
-	if (platform_device_add_data(&usbhs_device,
-				&usbhs_data, sizeof(usbhs_data)) < 0) {
-		printk(KERN_ERR "USBHS platform_device_add_data failed\n");
-		goto init_end;
+	oh[0] = omap_hwmod_lookup(USBHS_UHH_HWMODNAME);
+	if (!oh[0]) {
+		pr_err("Could not look up %s\n", USBHS_UHH_HWMODNAME);
+		return;
 	}
 
-	if (platform_device_register(&usbhs_device) < 0)
-		printk(KERN_ERR "USBHS platform_device_register failed\n");
+	oh[1] = omap_hwmod_lookup(USBHS_TLL_HWMODNAME);
+	if (!oh[1]) {
+		pr_err("Could not look up %s\n", USBHS_TLL_HWMODNAME);
+		return;
+	}
 
-init_end:
-	return;
+	od = omap_device_build_ss(OMAP_USBHS_DEVICE, bus_id, oh, 2,
+				(void *)&usbhs_data, sizeof(usbhs_data),
+				omap_uhhtll_latency,
+				ARRAY_SIZE(omap_uhhtll_latency), false);
+	if (IS_ERR(od)) {
+		pr_err("Could not build hwmod devices %s,%s\n",
+			USBHS_UHH_HWMODNAME, USBHS_TLL_HWMODNAME);
+		return;
+	}
 }
 
 #else
@@ -570,5 +540,3 @@ void __init usbhs_init(const struct usbhs_omap_board_data *pdata)
 }
 
 #endif
-
-
-- 
1.6.0.4

Re: [PATCH 3/5 v12] arm: omap: usb: register hwmods of usbhs

[Paul Walmsley]

Hi

On Fri, 30 Sep 2011, Keshava Munegowda wrote:

> The hwmod structure of usb_host_hs  and usb_tll are
> retrieved and registered with omap device
> 
> Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
> Reviewed-by: Partha Basak <parthab@india.ti.com>
> ---
>  arch/arm/mach-omap2/usb-host.c |  100 ++++++++++++++--------------------------
>  1 files changed, 34 insertions(+), 66 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
> index 89ae298..771dc78 100644
> --- a/arch/arm/mach-omap2/usb-host.c
> +++ b/arch/arm/mach-omap2/usb-host.c
> @@ -28,51 +28,28 @@

> +	oh[0] = omap_hwmod_lookup(USBHS_UHH_HWMODNAME);
> +	if (!oh[0]) {
> +		pr_err("Could not look up %s\n", USBHS_UHH_HWMODNAME);
> +		return;
>  	}
>  
> -	if (platform_device_register(&usbhs_device) < 0)
> -		printk(KERN_ERR "USBHS platform_device_register failed\n");
> +	oh[1] = omap_hwmod_lookup(USBHS_TLL_HWMODNAME);
> +	if (!oh[1]) {
> +		pr_err("Could not look up %s\n", USBHS_TLL_HWMODNAME);
> +		return;
> +	}
>  
> -init_end:
> -	return;
> +	od = omap_device_build_ss(OMAP_USBHS_DEVICE, bus_id, oh, 2,
> +				(void *)&usbhs_data, sizeof(usbhs_data),
> +				omap_uhhtll_latency,
> +				ARRAY_SIZE(omap_uhhtll_latency), false);

Usually there's something wrong with omap_devices that contain 
multiple hwmods.  Is there some reason why there isn't a separate driver 
for the TLL?  Judging by a brief look at drivers/mfd/omap_usb_host.c, the 
TLL handling looks logically distinct?


- Paul

Re: [PATCH 3/5 v12] arm: omap: usb: register hwmods of usbhs

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1867 bytes --]

Hi,

On Fri, Sep 30, 2011 at 01:15:55AM -0600, Paul Walmsley wrote:
> > The hwmod structure of usb_host_hs  and usb_tll are
> > retrieved and registered with omap device
> > 
> > Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
> > Reviewed-by: Partha Basak <parthab@india.ti.com>
> > ---
> >  arch/arm/mach-omap2/usb-host.c |  100 ++++++++++++++--------------------------
> >  1 files changed, 34 insertions(+), 66 deletions(-)
> > 
> > diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
> > index 89ae298..771dc78 100644
> > --- a/arch/arm/mach-omap2/usb-host.c
> > +++ b/arch/arm/mach-omap2/usb-host.c
> > @@ -28,51 +28,28 @@
> 
> > +	oh[0] = omap_hwmod_lookup(USBHS_UHH_HWMODNAME);
> > +	if (!oh[0]) {
> > +		pr_err("Could not look up %s\n", USBHS_UHH_HWMODNAME);
> > +		return;
> >  	}
> >  
> > -	if (platform_device_register(&usbhs_device) < 0)
> > -		printk(KERN_ERR "USBHS platform_device_register failed\n");
> > +	oh[1] = omap_hwmod_lookup(USBHS_TLL_HWMODNAME);
> > +	if (!oh[1]) {
> > +		pr_err("Could not look up %s\n", USBHS_TLL_HWMODNAME);
> > +		return;
> > +	}
> >  
> > -init_end:
> > -	return;
> > +	od = omap_device_build_ss(OMAP_USBHS_DEVICE, bus_id, oh, 2,
> > +				(void *)&usbhs_data, sizeof(usbhs_data),
> > +				omap_uhhtll_latency,
> > +				ARRAY_SIZE(omap_uhhtll_latency), false);
> 
> Usually there's something wrong with omap_devices that contain 
> multiple hwmods.  Is there some reason why there isn't a separate driver 
> for the TLL?  Judging by a brief look at drivers/mfd/omap_usb_host.c, the 
> TLL handling looks logically distinct?

Yes, I have the same feeling. To my understanding, USB Host Subsystem on
OMAP is composed of the Transceiver-less link (TLL) and USB Host (UHH).
Aparently, they could be handled by separate drivers.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 3/5 v12] arm: omap: usb: register hwmods of usbhs

[Munegowda, Keshava]

On Fri, Sep 30, 2011 at 1:04 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Fri, Sep 30, 2011 at 01:15:55AM -0600, Paul Walmsley wrote:
>> > The hwmod structure of usb_host_hs  and usb_tll are
>> > retrieved and registered with omap device
>> >
>> > Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
>> > Reviewed-by: Partha Basak <parthab@india.ti.com>
>> > ---
>> >  arch/arm/mach-omap2/usb-host.c |  100 ++++++++++++++--------------------------
>> >  1 files changed, 34 insertions(+), 66 deletions(-)
>> >
>> > diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
>> > index 89ae298..771dc78 100644
>> > --- a/arch/arm/mach-omap2/usb-host.c
>> > +++ b/arch/arm/mach-omap2/usb-host.c
>> > @@ -28,51 +28,28 @@
>>
>> > +   oh[0] = omap_hwmod_lookup(USBHS_UHH_HWMODNAME);
>> > +   if (!oh[0]) {
>> > +           pr_err("Could not look up %s\n", USBHS_UHH_HWMODNAME);
>> > +           return;
>> >     }
>> >
>> > -   if (platform_device_register(&usbhs_device) < 0)
>> > -           printk(KERN_ERR "USBHS platform_device_register failed\n");
>> > +   oh[1] = omap_hwmod_lookup(USBHS_TLL_HWMODNAME);
>> > +   if (!oh[1]) {
>> > +           pr_err("Could not look up %s\n", USBHS_TLL_HWMODNAME);
>> > +           return;
>> > +   }
>> >
>> > -init_end:
>> > -   return;
>> > +   od = omap_device_build_ss(OMAP_USBHS_DEVICE, bus_id, oh, 2,
>> > +                           (void *)&usbhs_data, sizeof(usbhs_data),
>> > +                           omap_uhhtll_latency,
>> > +                           ARRAY_SIZE(omap_uhhtll_latency), false);
>>
>> Usually there's something wrong with omap_devices that contain
>> multiple hwmods.  Is there some reason why there isn't a separate driver
>> for the TLL?  Judging by a brief look at drivers/mfd/omap_usb_host.c, the
>> TLL handling looks logically distinct?
>
> Yes, I have the same feeling. To my understanding, USB Host Subsystem on
> OMAP is composed of the Transceiver-less link (TLL) and USB Host (UHH).
> Aparently, they could be handled by separate drivers.
>
> --
> balbi

yes, it can be as two separate drivers for uhh and tll;  But i don't
think driver can be used effectively.
Now ehci and ohci gets the clocks , config reg and port settings
through usb host which is sufficient.
If you make them as two different drivers; then ehci and ohci has to
interact with both the drivers separately.
which will be an unnecessary complications. I feel not divided this
driver into two

regards
keshava

Re: [PATCH 3/5 v12] arm: omap: usb: register hwmods of usbhs

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2903 bytes --]

Hi,

On Fri, Sep 30, 2011 at 02:45:32PM +0530, Munegowda, Keshava wrote:
> On Fri, Sep 30, 2011 at 1:04 PM, Felipe Balbi <balbi@ti.com> wrote:
> > Hi,
> >
> > On Fri, Sep 30, 2011 at 01:15:55AM -0600, Paul Walmsley wrote:
> >> > The hwmod structure of usb_host_hs  and usb_tll are
> >> > retrieved and registered with omap device
> >> >
> >> > Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
> >> > Reviewed-by: Partha Basak <parthab@india.ti.com>
> >> > ---
> >> >  arch/arm/mach-omap2/usb-host.c |  100 ++++++++++++++--------------------------
> >> >  1 files changed, 34 insertions(+), 66 deletions(-)
> >> >
> >> > diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
> >> > index 89ae298..771dc78 100644
> >> > --- a/arch/arm/mach-omap2/usb-host.c
> >> > +++ b/arch/arm/mach-omap2/usb-host.c
> >> > @@ -28,51 +28,28 @@
> >>
> >> > +   oh[0] = omap_hwmod_lookup(USBHS_UHH_HWMODNAME);
> >> > +   if (!oh[0]) {
> >> > +           pr_err("Could not look up %s\n", USBHS_UHH_HWMODNAME);
> >> > +           return;
> >> >     }
> >> >
> >> > -   if (platform_device_register(&usbhs_device) < 0)
> >> > -           printk(KERN_ERR "USBHS platform_device_register failed\n");
> >> > +   oh[1] = omap_hwmod_lookup(USBHS_TLL_HWMODNAME);
> >> > +   if (!oh[1]) {
> >> > +           pr_err("Could not look up %s\n", USBHS_TLL_HWMODNAME);
> >> > +           return;
> >> > +   }
> >> >
> >> > -init_end:
> >> > -   return;
> >> > +   od = omap_device_build_ss(OMAP_USBHS_DEVICE, bus_id, oh, 2,
> >> > +                           (void *)&usbhs_data, sizeof(usbhs_data),
> >> > +                           omap_uhhtll_latency,
> >> > +                           ARRAY_SIZE(omap_uhhtll_latency), false);
> >>
> >> Usually there's something wrong with omap_devices that contain
> >> multiple hwmods.  Is there some reason why there isn't a separate driver
> >> for the TLL?  Judging by a brief look at drivers/mfd/omap_usb_host.c, the
> >> TLL handling looks logically distinct?
> >
> > Yes, I have the same feeling. To my understanding, USB Host Subsystem on
> > OMAP is composed of the Transceiver-less link (TLL) and USB Host (UHH).
> > Aparently, they could be handled by separate drivers.
> >
> > --
> > balbi
> 
> yes, it can be as two separate drivers for uhh and tll;  But i don't
> think driver can be used effectively.
> Now ehci and ohci gets the clocks , config reg and port settings
> through usb host which is sufficient.
> If you make them as two different drivers; then ehci and ohci has to
> interact with both the drivers separately.
> which will be an unnecessary complications. I feel not divided this
> driver into two

Come again, EHCI/OHCI need clocks from UHH and TLL ?? If that's the
case, then there's really no easy way to handle this as a device can
have only one parent.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 3/5 v12] arm: omap: usb: register hwmods of usbhs

[Munegowda, Keshava]

On Fri, Sep 30, 2011 at 2:49 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Fri, Sep 30, 2011 at 02:45:32PM +0530, Munegowda, Keshava wrote:
>> On Fri, Sep 30, 2011 at 1:04 PM, Felipe Balbi <balbi@ti.com> wrote:
>> > Hi,
>> >
>> > On Fri, Sep 30, 2011 at 01:15:55AM -0600, Paul Walmsley wrote:
>> >> > The hwmod structure of usb_host_hs  and usb_tll are
>> >> > retrieved and registered with omap device
>> >> >
>> >> > Signed-off-by: Keshava Munegowda <keshava_mgowda@ti.com>
>> >> > Reviewed-by: Partha Basak <parthab@india.ti.com>
>> >> > ---
>> >> >  arch/arm/mach-omap2/usb-host.c |  100 ++++++++++++++--------------------------
>> >> >  1 files changed, 34 insertions(+), 66 deletions(-)
>> >> >
>> >> > diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
>> >> > index 89ae298..771dc78 100644
>> >> > --- a/arch/arm/mach-omap2/usb-host.c
>> >> > +++ b/arch/arm/mach-omap2/usb-host.c
>> >> > @@ -28,51 +28,28 @@
>> >>
>> >> > +   oh[0] = omap_hwmod_lookup(USBHS_UHH_HWMODNAME);
>> >> > +   if (!oh[0]) {
>> >> > +           pr_err("Could not look up %s\n", USBHS_UHH_HWMODNAME);
>> >> > +           return;
>> >> >     }
>> >> >
>> >> > -   if (platform_device_register(&usbhs_device) < 0)
>> >> > -           printk(KERN_ERR "USBHS platform_device_register failed\n");
>> >> > +   oh[1] = omap_hwmod_lookup(USBHS_TLL_HWMODNAME);
>> >> > +   if (!oh[1]) {
>> >> > +           pr_err("Could not look up %s\n", USBHS_TLL_HWMODNAME);
>> >> > +           return;
>> >> > +   }
>> >> >
>> >> > -init_end:
>> >> > -   return;
>> >> > +   od = omap_device_build_ss(OMAP_USBHS_DEVICE, bus_id, oh, 2,
>> >> > +                           (void *)&usbhs_data, sizeof(usbhs_data),
>> >> > +                           omap_uhhtll_latency,
>> >> > +                           ARRAY_SIZE(omap_uhhtll_latency), false);
>> >>
>> >> Usually there's something wrong with omap_devices that contain
>> >> multiple hwmods.  Is there some reason why there isn't a separate driver
>> >> for the TLL?  Judging by a brief look at drivers/mfd/omap_usb_host.c, the
>> >> TLL handling looks logically distinct?
>> >
>> > Yes, I have the same feeling. To my understanding, USB Host Subsystem on
>> > OMAP is composed of the Transceiver-less link (TLL) and USB Host (UHH).
>> > Aparently, they could be handled by separate drivers.
>> >
>> > --
>> > balbi
>>
>> yes, it can be as two separate drivers for uhh and tll;  But i don't
>> think driver can be used effectively.
>> Now ehci and ohci gets the clocks , config reg and port settings
>> through usb host which is sufficient.
>> If you make them as two different drivers; then ehci and ohci has to
>> interact with both the drivers separately.
>> which will be an unnecessary complications. I feel not divided this
>> driver into two
>
> Come again, EHCI/OHCI need clocks from UHH and TLL ?? If that's the
> case, then there's really no easy way to handle this as a device can
> have only one parent.

yes,  if you are using ehci phy mode ( port modes of UHH_HOSTCONFIG register)
then uhh functional ( omap3: interface clocks too) is sufficient;
if you are using ehci in tll mode , then you need tll functional clock too.

regards
keshava

Re: [PATCH 3/5 v12] arm: omap: usb: register hwmods of usbhs

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2255 bytes --]

Hi,

On Fri, Sep 30, 2011 at 02:56:40PM +0530, Munegowda, Keshava wrote:
> >> >> Usually there's something wrong with omap_devices that contain
> >> >> multiple hwmods.  Is there some reason why there isn't a separate driver
> >> >> for the TLL?  Judging by a brief look at drivers/mfd/omap_usb_host.c, the
> >> >> TLL handling looks logically distinct?
> >> >
> >> > Yes, I have the same feeling. To my understanding, USB Host Subsystem on
> >> > OMAP is composed of the Transceiver-less link (TLL) and USB Host (UHH).
> >> > Aparently, they could be handled by separate drivers.
> >> >
> >> > --
> >> > balbi
> >>
> >> yes, it can be as two separate drivers for uhh and tll;  But i don't
> >> think driver can be used effectively.
> >> Now ehci and ohci gets the clocks , config reg and port settings
> >> through usb host which is sufficient.
> >> If you make them as two different drivers; then ehci and ohci has to
> >> interact with both the drivers separately.
> >> which will be an unnecessary complications. I feel not divided this
> >> driver into two
> >
> > Come again, EHCI/OHCI need clocks from UHH and TLL ?? If that's the
> > case, then there's really no easy way to handle this as a device can
> > have only one parent.
> 
> yes,  if you are using ehci phy mode ( port modes of UHH_HOSTCONFIG register)
> then uhh functional ( omap3: interface clocks too) is sufficient;
> if you are using ehci in tll mode , then you need tll functional clock too.

But those clocks are fed into UHH and TLL respectively, not EHCI/OHCI,
right ? IOW, EHCI/OHCI only need those clocks to be stable in order for
it to work.

I'm trying to think of a way to decouple that but nothing comes to mind
right now... Ideally, calling pm_runtime_get_sync() on EHCI/OHCI would
make all clocks enabled and the only way I see for that to happen is to
make TLL parent of UHH which is parent of EHCI/OHCI, but I'm not sure
that makes a lot of sense.

Paul, do you have any tip which we could use ? What we need is to be
able to call pm_runtime_get_sync() or ehci-omap.c and that should enable
UHH clocks and if (port->mode == TLL) enable TLL clocks.

We still want to split UHH and TLL into separate drivers.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]