Patchwork [U-Boot] Please pull u-boot-ti/master

login
register
mail settings
Submitter Michael Trimarchi
Date June 9, 2013, 9:37 p.m.
Message ID <51B4F599.7030205@amarulasolutions.com>
Download mbox | patch
Permalink /patch/250150/
State Not Applicable
Delegated to: Tom Rini
Headers show

Comments

Michael Trimarchi - June 9, 2013, 9:37 p.m.
Hi

On 06/08/2013 10:43 PM, Lubomir Popov wrote:
> Hi Tom, Michael,
> 
>> Hello,
>>
>> The following changes since commit 3da0e5750b24a9491058df6126c7be577a276c09:
>>
>>   arm: factorize relocate_code routine (2013-05-30 20:24:38 +0200)
>>
>> are available in the git repository at:
>>
>>   git://git.denx.de/u-boot-ti.git master
>>
>> for you to fetch changes up to 80dd596d1b442ff53dbeb33eccdb8efd2283be79:
>>
> 
> [snip]
> 
>> Michael Trimarchi (1):
>>       usb: omap: ulpi: fix ulpi transceiver access
> 
> [snip]
> 
>>  drivers/usb/ulpi/omap-ulpi-viewport.c              |   40 +-
> 
> [snip]
> 
> I just made a clean clone of u-boot-ti/master, manually applied the
> changes to the ehci files, added my board files and made a build.
> Everything seems to work fine, but I see an error message regarding
> ULPI reset that was not present before, and obviously it is due to
> Michael's changes:
> 
> SOM5_EVB # usb start
> (Re)start USB...
> USB0:   ULPI: ulpi_reset: failed writing reset bit

Let me understand. The patch is wrong because you have a problem now.
The old code was not sending any write command so any ulpi_reset and the
test condition was wrong.

> USB EHCI 1.00
> scanning bus 0 for devices... 6 USB Device(s) found
>        scanning usb for storage devices... 3 Storage Device(s) found
>        scanning usb for ethernet devices... 1 Ethernet Device(s) found
> SOM5_EVB # usb tree
> USB device tree:
>   1  Hub (480 Mb/s, 0mA)
>   |  u-boot EHCI Host Controller
>   |
>   +-2  Mass Storage (480 Mb/s, 200mA)
>   |    FSC                  MEMORYBIRD USB2      C157040817120315AA
>   |
>   +-3  Hub (480 Mb/s, 2mA)
>   | |
>   | +-4  Mass Storage (480 Mb/s, 96mA)
>   | |    Generic Ultra Fast Media Reader 000000264001
>   | |
>   | +-5  Mass Storage (480 Mb/s, 100mA)
>   |      USB Flash Drive 531C43B21928F11F
>   |
>   +-6  Vendor specific (480 Mb/s, 500mA)
> 
> SOM5_EVB #
> 
> Otherwise everything is OK, the device on the ULPI port is working
> (it is #2 above). It is now late and I shall investigate in detail
> tomorrow, this is just an early warning ;)

Can you test the attach patch? Yes I know it is not inline but I will
send inline tomorrow and I have done changing the old one. 
I was thinking port number was starting from 1 but I have done a quick check
on soft_reset of omap and it is starting from 0

Michael

> 
> Best regards,
> Lubo
>
Lubomir Popov - June 10, 2013, 8:54 a.m.
Hi Michael,

On 10/06/13 00:37, Michael Trimarchi wrote:
> Hi
> 
> On 06/08/2013 10:43 PM, Lubomir Popov wrote:
>> Hi Tom, Michael,
>>
>>> Hello,
>>>
>>> The following changes since commit 3da0e5750b24a9491058df6126c7be577a276c09:
>>>
>>>   arm: factorize relocate_code routine (2013-05-30 20:24:38 +0200)
>>>
>>> are available in the git repository at:
>>>
>>>   git://git.denx.de/u-boot-ti.git master
>>>
>>> for you to fetch changes up to 80dd596d1b442ff53dbeb33eccdb8efd2283be79:
>>>
>>
>> [snip]
>>
>>> Michael Trimarchi (1):
>>>       usb: omap: ulpi: fix ulpi transceiver access
>>
>> [snip]
>>
>>>  drivers/usb/ulpi/omap-ulpi-viewport.c              |   40 +-
>>
>> [snip]
>>
>> I just made a clean clone of u-boot-ti/master, manually applied the
>> changes to the ehci files, added my board files and made a build.
>> Everything seems to work fine, but I see an error message regarding
>> ULPI reset that was not present before, and obviously it is due to
>> Michael's changes:
>>
>> SOM5_EVB # usb start
>> (Re)start USB...
>> USB0:   ULPI: ulpi_reset: failed writing reset bit
> 
> Let me understand. The patch is wrong because you have a problem now.
> The old code was not sending any write command so any ulpi_reset and the
> test condition was wrong.
Right.

> 
>> USB EHCI 1.00
>> scanning bus 0 for devices... 6 USB Device(s) found
>>        scanning usb for storage devices... 3 Storage Device(s) found
>>        scanning usb for ethernet devices... 1 Ethernet Device(s) found
>> SOM5_EVB # usb tree
>> USB device tree:
>>   1  Hub (480 Mb/s, 0mA)
>>   |  u-boot EHCI Host Controller
>>   |
>>   +-2  Mass Storage (480 Mb/s, 200mA)
>>   |    FSC                  MEMORYBIRD USB2      C157040817120315AA
>>   |
>>   +-3  Hub (480 Mb/s, 2mA)
>>   | |
>>   | +-4  Mass Storage (480 Mb/s, 96mA)
>>   | |    Generic Ultra Fast Media Reader 000000264001
>>   | |
>>   | +-5  Mass Storage (480 Mb/s, 100mA)
>>   |      USB Flash Drive 531C43B21928F11F
>>   |
>>   +-6  Vendor specific (480 Mb/s, 500mA)
>>
>> SOM5_EVB #
>>
>> Otherwise everything is OK, the device on the ULPI port is working
>> (it is #2 above). It is now late and I shall investigate in detail
>> tomorrow, this is just an early warning ;)
> 
> Can you test the attach patch? Yes I know it is not inline but I will
> send inline tomorrow and I have done changing the old one. 
> I was thinking port number was starting from 1 but I have done a quick check
> on soft_reset of omap and it is starting from 0

Just tested on a OMAP5430 custom board. Everything is OK, PHY soft reset
passes. My ULPI PHY is on port 0, and with the previous version the insreg05
bit 31 remained stuck at 1, producing this error. Now when we write 1 to the
port field instead of 0, everything is fine. So

Tested-by: Lubomir Popov <lpopov@mm-sol.com>

While testing, I however encountered another issue (not related to this patch,
take it easy ;-) ): one particular USB flash stick, connected to the ULPI port,
does systematically (100%) not get detected upon the first 'usb start' command
after power-on; subsequent usb start/reset commands detect it alright. I have
experimented a bit with some delays (assuming that it is some sort of timing
problem), but without success. Other sticks are OK. Removing the call to
omap_ehci_soft_phy_reset() (which calls ulpi_reset() and the viewport stuff)
does not change anything.

I'm currently at work, where I have other obligations and cannot spend much
time for U-Boot. Therefore, if somebody could share any experience on this
subject, please let me know.

One thing that perhaps I should clarify: my ULPI PHY is the TI part TUSB1210;
on the other hand, TI themselves recommend PHYs by SMSC, at least for the OMAP4.
This was not the case for OMAP5, but who knows...

Thanks,
Lubo

> 
> Michael
> 
>>
>> Best regards,
>> Lubo
>>

Patch

This patch fix the omap access to the transceiver
configuration registers using the ulpi bus. As reported by
the documentation the bit31 is used only to check if the
transaction is done or still running and the reading and
writing operation have different offset and have different
values. What we need to do at the end of a transaction is
leave the bus in done state. Anyway an error using the ulpi
omap register is not recoverable so any error give out the
usage of this interface.

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
- changes for V4
  port_num start from 0 so we need to take in account when
  we use the omap access function
- changes for V3
  Fix patch subject
- changes for V2
  Fix commit message
---
 drivers/usb/ulpi/omap-ulpi-viewport.c |   40 +++++++--------------------------
 1 file changed, 8 insertions(+), 32 deletions(-)

diff --git a/drivers/usb/ulpi/omap-ulpi-viewport.c b/drivers/usb/ulpi/omap-ulpi-viewport.c
index 3c1ea1a..2a42033 100644
--- a/drivers/usb/ulpi/omap-ulpi-viewport.c
+++ b/drivers/usb/ulpi/omap-ulpi-viewport.c
@@ -22,18 +22,19 @@ 
 #include <asm/io.h>
 #include <usb/ulpi.h>
 
-#define OMAP_ULPI_WR_OPSEL	(3 << 21)
-#define OMAP_ULPI_ACCESS	(1 << 31)
+#define OMAP_ULPI_WR_OPSEL	(2 << 22)
+#define OMAP_ULPI_RD_OPSEL	(3 << 22)
+#define OMAP_ULPI_START		(1 << 31)
 
 /*
- * Wait for the ULPI Access to complete
+ * Wait for having ulpi in done state
  */
 static int ulpi_wait(struct ulpi_viewport *ulpi_vp, u32 mask)
 {
 	int timeout = CONFIG_USB_ULPI_TIMEOUT;
 
 	while (--timeout) {
-		if ((readl(ulpi_vp->viewport_addr) & mask))
+		if (!(readl(ulpi_vp->viewport_addr) & mask))
 			return 0;
 
 		udelay(1);
@@ -43,40 +44,15 @@  static int ulpi_wait(struct ulpi_viewport *ulpi_vp, u32 mask)
 }
 
 /*
- * Wake the ULPI PHY up for communication
- *
- * returns 0 on success.
- */
-static int ulpi_wakeup(struct ulpi_viewport *ulpi_vp)
-{
-	int err;
-
-	if (readl(ulpi_vp->viewport_addr) & OMAP_ULPI_ACCESS)
-		return 0; /* already awake */
-
-	writel(OMAP_ULPI_ACCESS, ulpi_vp->viewport_addr);
-
-	err = ulpi_wait(ulpi_vp, OMAP_ULPI_ACCESS);
-	if (err)
-		debug("ULPI wakeup timed out\n");
-
-	return err;
-}
-
-/*
  * Issue a ULPI read/write request
  */
 static int ulpi_request(struct ulpi_viewport *ulpi_vp, u32 value)
 {
 	int err;
 
-	err = ulpi_wakeup(ulpi_vp);
-	if (err)
-		return err;
-
 	writel(value, ulpi_vp->viewport_addr);
 
-	err = ulpi_wait(ulpi_vp, OMAP_ULPI_ACCESS);
+	err = ulpi_wait(ulpi_vp, OMAP_ULPI_START);
 	if (err)
 		debug("ULPI request timed out\n");
 
@@ -85,7 +61,7 @@  static int ulpi_request(struct ulpi_viewport *ulpi_vp, u32 value)
 
 int ulpi_write(struct ulpi_viewport *ulpi_vp, u8 *reg, u32 value)
 {
-	u32 val = ((ulpi_vp->port_num & 0xf) << 24) |
+	u32 val = (OMAP_ULPI_START | ((ulpi_vp->port_num + 1) & 0xf) << 24) |
 			OMAP_ULPI_WR_OPSEL | ((u32)reg << 16) | (value & 0xff);
 
 	return ulpi_request(ulpi_vp, val);
@@ -95,7 +71,7 @@  u32 ulpi_read(struct ulpi_viewport *ulpi_vp, u8 *reg)
 {
 	int err;
 	u32 val = (((ulpi_vp->port_num + 1) & 0xf) << 24) |
-			 OMAP_ULPI_WR_OPSEL | ((u32)reg << 16);
+			 OMAP_ULPI_RD_OPSEL | ((u32)reg << 16);
 
 	err = ulpi_request(ulpi_vp, val);
 	if (err)
-- 
1.7.9.5