[PATCH 2/2] ehci: Fix EHCI init for OMAP3EVM

[PATCH 2/2] ehci: Fix EHCI init for OMAP3EVM

[Ajay Kumar Gupta]

Multimedia Daughter card on OMAP3EVM uses port2
as EHCI port. Other ports (port1 and port3)are
not used. 

Fixes done includes:
	- Port1 GPIO set to -EINVAL
	- GPIO 135 as port2 phy reset pin
	- Mux setup for GPIO 135
	- Chargepump workaround not required on OMAP3EVM

Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
---
 arch/arm/mach-omap2/board-omap3evm.c |    2 +-
 arch/arm/mach-omap2/usb-ehci.c       |    4 ++++
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
index 595beac..ae32e72 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -308,7 +308,7 @@ static void __init omap3_evm_init(void)
 
 	omap_serial_init();
 	usb_musb_init();
-	usb_ehci_init(EHCI_HCD_OMAP_MODE_PHY, true, true, 57, 61);
+	usb_ehci_init(EHCI_HCD_OMAP_MODE_PHY, false, true, -EINVAL, 135);
 	ads7846_dev_init();
 }
 
diff --git a/arch/arm/mach-omap2/usb-ehci.c b/arch/arm/mach-omap2/usb-ehci.c
index 53e6e85..bf4bf77 100644
--- a/arch/arm/mach-omap2/usb-ehci.c
+++ b/arch/arm/mach-omap2/usb-ehci.c
@@ -173,6 +173,10 @@ void __init usb_ehci_init(enum ehci_hcd_omap_mode phy_mode,
 	if (cpu_is_omap34xx())
 		setup_ehci_io_mux(phy_mode);
 
+#ifdef CONFIG_MACH_OMAP3EVM
+	/* Setup phy reset pin for EHCI on OMAP3EVM */
+	omap_cfg_reg(AF4_34XX_GPIO135_OUT);
+#endif
 	if (platform_device_register(&ehci_device) < 0) {
 		printk(KERN_ERR "Unable to register HS-USB (EHCI) device\n");
 		return;
-- 
1.6.2.4

RE: [PATCH 2/2] ehci: Fix EHCI init for OMAP3EVM

[Gupta, Ajay Kumar]

> >
> > +#ifdef CONFIG_MACH_OMAP3EVM
> > +	/* Setup phy reset pin for EHCI on OMAP3EVM */
> > +	omap_cfg_reg(AF4_34XX_GPIO135_OUT);
> > +#endif
> Why don't put this into board-omap3evm.c?
> 
> For example, on beagle the phy reset signal is routed to GPIO147. I
> don't think we want to see several "#ifdef CONFIG_MACH_..." here.

Looks better but if USB itself is not included in the build then this part of code would be unnecessary in board-omap3evm.c. We again need to add some #ifdef there to avoid this.

Another solution is to add something like board_is_omap3evm() in <mach/board.h> and use them in usb-ehci.c.

Felipe, what do you suggest?

> 
> Moreover, that setup_ehci_io_mux() function is also board-specific since
> different boards may use different pinmux settings  or  different USB
> ports may be enabled/disabled there. Well, at least the function should
> allow to configure only required USB ports.

This is coming very soon.

-Ajay
> 
> 
> Dmitry
> >  	if (platform_device_register(&ehci_device) < 0) {
> >  		printk(KERN_ERR "Unable to register HS-USB (EHCI) device\n");
> >  		return;
> 

Re: [PATCH 2/2] ehci: Fix EHCI init for OMAP3EVM

[Felipe Balbi]

Hi,

On Mon, Jul 06, 2009 at 10:55:05AM +0200, ext Gupta, Ajay Kumar wrote:
> Looks better but if USB itself is not included in the build then this part of code would be unnecessary in board-omap3evm.c. We again need to add some #ifdef there to avoid this.
> 
> Another solution is to add something like board_is_omap3evm() in <mach/board.h> and use them in usb-ehci.c.
> 
> Felipe, what do you suggest?

how about passing setup() and cleanup() function pointers from
board-file to the driver via platform_data ?

the board-specific setup() could handle the mux config then.

-- 
balbi

Re: [PATCH 2/2] ehci: Fix EHCI init for OMAP3EVM

[Felipe Balbi]

On Mon, Jul 06, 2009 at 11:50:59AM +0200, Balbi Felipe (Nokia-D/Helsinki) wrote:
> Hi,
> 
> On Mon, Jul 06, 2009 at 10:55:05AM +0200, ext Gupta, Ajay Kumar wrote:
> > Looks better but if USB itself is not included in the build then this part of code would be unnecessary in board-omap3evm.c. We again need to add some #ifdef there to avoid this.
> > 
> > Another solution is to add something like board_is_omap3evm() in <mach/board.h> and use them in usb-ehci.c.
> > 
> > Felipe, what do you suggest?
> 
> how about passing setup() and cleanup() function pointers from
> board-file to the driver via platform_data ?
> 
> the board-specific setup() could handle the mux config then.

looking more closely, it doesn't make sense what I said...

I need to think this through a little bit more, but I guess, for now the
ifdef is the way to go to, at least, get the board working.

I'll think this through.

-- 
balbi