Re: [PATCH] usb: ehci_omap: fix device detect issue with modules

Previous thread: [PATCH 09/22] USB: s3c2410: deactivate endpoints before gadget unbinding by Greg Kroah-Hartman on Wednesday, June 30, 2010 - 8:24 am. (1 message)

Next thread: Re: [PATCH] HID: expand hid-topseed driver to support TopSeed RF Combo Device by Pieter Hoekstra on Thursday, July 1, 2010 - 2:46 pm. (1 message)
From: Ajay Kumar Gupta
Date: Thursday, July 1, 2010 - 2:49 am

Currently devices don't get detected automatically if the ehci
module is inserted 2nd time onward. We need to disconnect and
reconnect the device for it to get detected and enumerated.

Resetting the USB PHY after the EHCI controller has been initilized
fixes this issue. Tested on OMAP3EVM.

Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
---
 drivers/usb/host/ehci-omap.c |   86 ++++++++++++++++++-----------------------
 1 files changed, 38 insertions(+), 48 deletions(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 5450e62..a2c3d7f 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -236,6 +236,31 @@ static void omap_usb_utmi_init(struct ehci_hcd_omap *omap, u8 tll_channel_mask)
 
 /*-------------------------------------------------------------------------*/
 
+static void ehci_omap_phy_reset(struct ehci_hcd_omap *omap)
+{
+	if ((omap->port_mode[0] == EHCI_HCD_OMAP_MODE_PHY) &&
+			gpio_is_valid(omap->reset_gpio_port[0])) {
+		gpio_request(omap->reset_gpio_port[0], "HSUSB0 reset");
+		gpio_direction_output(omap->reset_gpio_port[0], 0);
+	}
+
+	if ((omap->port_mode[1] == EHCI_HCD_OMAP_MODE_PHY) &&
+			gpio_is_valid(omap->reset_gpio_port[1])) {
+		gpio_request(omap->reset_gpio_port[1], "HSUSB1 reset");
+		gpio_direction_output(omap->reset_gpio_port[1], 0);
+	}
+
+	/* Hold the PHY in RESET for enough time till DIR is high */
+	udelay(10);
+
+	if ((omap->port_mode[0] == EHCI_HCD_OMAP_MODE_PHY) &&
+			gpio_is_valid(omap->reset_gpio_port[0]))
+		gpio_set_value(omap->reset_gpio_port[0], 1);
+	if ((omap->port_mode[1] == EHCI_HCD_OMAP_MODE_PHY) &&
+			gpio_is_valid(omap->reset_gpio_port[1]))
+		gpio_set_value(omap->reset_gpio_port[1], 1);
+}
+
 /* omap_start_ehc
  *	- Start the TI USBHOST controller
  */
@@ -270,24 +295,6 @@ static int omap_start_ehc(struct ehci_hcd_omap *omap, struct usb_hcd *hcd)
 	}
 	clk_enable(omap->usbhost1_48m_fck);
 
-	if (omap->phy_reset) {
-		/* Refer: ISSUE1 */
-		if ...
From: Felipe Balbi
Date: Thursday, July 1, 2010 - 3:49 am

then this means that boards without the reset line routed to a gpio, 
won't work. How about SOFTRESET bit in USBTLL register ?? The only 
problem would be to cope with both OHCI and EHCI.

-- 
balbi

DefectiveByDesign.org
--

From: Felipe Balbi
Date: Thursday, July 1, 2010 - 3:51 am

actualy, should be UHH_SYSCONFIG:SOFTRESET.

-- 
balbi

DefectiveByDesign.org
--

From: Gupta, Ajay Kumar
Date: Thursday, July 1, 2010 - 4:03 am

That would reset the controller and so all the register would get
their default values.

--

From: Felipe Balbi
Date: Thursday, July 1, 2010 - 4:40 am

aa, ok. I mis-read, you want to reset the PHY only. Still, the boards 
without the reset line, won't work, so a better solution (if any) has to 
be found.

Maybe driving port reset signal would make PHY act ?

-- 
balbi

DefectiveByDesign.org
--

From: Gupta, Ajay Kumar
Date: Thursday, July 1, 2010 - 4:47 am

Do you mean to reset PHY using PHY reset command over ULPI ?
Anyways would try this.

--

From: Felipe Balbi
Date: Thursday, July 1, 2010 - 4:48 am

Hi,


yes, correct.

-- 
balbi

DefectiveByDesign.org
--

From: Gupta, Ajay Kumar
Date: Thursday, July 1, 2010 - 8:22 am

Soft resetting the PHY also fixes the issue. Here is the version-2 of
This patch.

-Ajay
--------  cut here -------------

Currently devices don't get detected automatically if the ehci
module is inserted 2nd time onward. We need to disconnect and
reconnect the device for it to get detected and enumerated.

Resetting the USB PHY using PHY reset command over ULPI fixes
this issue. Tested on OMAP3EVM.

Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
---
 drivers/usb/host/ehci-omap.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 5450e62..dec05fb 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -126,6 +126,13 @@
 #define	EHCI_INSNREG05_ULPI_EXTREGADD_SHIFT		8
 #define	EHCI_INSNREG05_ULPI_WRDATA_SHIFT		0
 
+/* ULPI Function Control set register */
+#define	FUNCTION_CTRL_SET_OFFSET			0x5
+#define	ULPI_FUNCTION_CTRL_SET_SUSPENDM			(1 << 6)
+#define	ULPI_FUNCTION_CTRL_SET_RESET			(1 << 5)
+#define	ULPI_FUNCTION_CTRL_SET_OPMODE			(3 << 3)
+#define	ULPI_FUNCTION_CTRL_SET_TERMSELECT		(1 << 2)
+#define	ULPI_FUNCTION_CTRL_SET_XCVRSELECT		(3 << 0)
 /*-------------------------------------------------------------------------*/
 
 static inline void ehci_omap_writel(void __iomem *base, u32 reg, u32 val)
@@ -425,6 +432,26 @@ static int omap_start_ehc(struct ehci_hcd_omap *omap, struct usb_hcd *hcd)
 			gpio_set_value(omap->reset_gpio_port[1], 1);
 	}
 
+	/* Soft reset the PHY using PHY reset command over ULPI */
+	if (omap->port_mode[0] == EHCI_HCD_OMAP_MODE_PHY) {
+		reg = ULPI_FUNCTION_CTRL_SET_RESET
+			| ULPI_FUNCTION_CTRL_SET_SUSPENDM
+			| (0x5 << EHCI_INSNREG05_ULPI_REGADD_SHIFT) /* F_CTRL */
+			| (2 << EHCI_INSNREG05_ULPI_OPSEL_SHIFT) /* WRITE */
+			| (1 << EHCI_INSNREG05_ULPI_PORTSEL_SHIFT) /* PORT1 */
+			| (1 << EHCI_INSNREG05_ULPI_CONTROL_SHIFT); /* START */
+		ehci_omap_writel(omap->ehci_base, ...
From: Gadiyar, Anand
Date: Thursday, July 1, 2010 - 9:37 am

Ajay,

I don't currently understand why this is needed. I see that this
fixes a problem for you, but I do not understand why it works.

I would like to test this on the other PHYs that could interface
with the OMAP3 before acking this. I don't have access to them
right now, and I'll try and dig up those boards on Monday and give
it a spin.

--

From: Felipe Balbi
Date: Monday, July 5, 2010 - 12:14 am

I don't think there's a problem with this, take a look at Section 3.5 
Power on and Reset from ULPI spec:

"After power-up, and when the clock starts toggling, the Link must reset 
the PHY by writing to the Reset bit in the Function Control register. 
When this bit is set, the transceiver will assert dir and reset the 
digital core.
When the reset completes, the PHY de-asserts dir and automatically 
clears the Reset bit in the Function Control register. After 
de-asserting dir, the PHY must immediately re-assert dir and send an RX 
CMD update to the Link."

So, we _do_ need to reset the PHY when probing ehci-hcd, and should 
probably assert SUSPENDM when rmmoding it.

-- 
balbi

DefectiveByDesign.org
--

From: Felipe Balbi
Date: Monday, July 5, 2010 - 12:19 am

Hi,


how about wrapping into a function:

static void omap_ehci_phy_reset(struct ehci_hcd_omap *omap, int port)
{
	unsigned			reg = 0;

	reg = ULPI_FUNCTION_CTRL_SET_RESET
		| ULPI_FUNCTION_CTRL_SET_SUSPENDM
		| (0x5 << EHCI_INSNREG05_ULPI_REGADD_SHIFT) /* F_CTRL */
		| (2 << EHCI_INSNREG05_ULPI_OPSEL_SHIFT) /* WRITE */
		| (port << EHCI_INSNREG05_ULPI_PORTSEL_SHIFT) /* PORT2 */
		| (1 << EHCI_INSNREG05_ULPI_CONTROL_SHIFT); /* START */
	ehci_omap_writel(omap->ehci_base, EHCI_INSNREG05_ULPI, reg);
}

something like that.

-- 
balbi

DefectiveByDesign.org
--

From: Felipe Balbi
Date: Monday, July 5, 2010 - 12:21 am

Hi,


you can use the helpers in <linux/usb/ulpi.h> for these too:

ULPI_SET(ULPI_FUNC_CONTROL);

-- 
balbi

DefectiveByDesign.org
--

From: Gupta, Ajay Kumar
Date: Monday, July 5, 2010 - 4:32 am

Ok fine. I will do the necessary changes and would submit v2 of
this patch.

Thanks,
--

Previous thread: [PATCH 09/22] USB: s3c2410: deactivate endpoints before gadget unbinding by Greg Kroah-Hartman on Wednesday, June 30, 2010 - 8:24 am. (1 message)

Next thread: Re: [PATCH] HID: expand hid-topseed driver to support TopSeed RF Combo Device by Pieter Hoekstra on Thursday, July 1, 2010 - 2:46 pm. (1 message)