usb: omap: ulpi: fix ulpi transceiver access

PATCH Superseded Michael Trimarchi 3 participants 8 messages Jun 6, 2013 – Jun 10, 2013

Subsystems (2)

USB OMAP USB support

lore

Share & tools

lore

Michael Trimarchi

author

2013-06-06 14:48 UTC #0 lore raw

PATCH v3 1/1 usb: omap: ulpi: fix ulpi transceiver access

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 <[email protected]>

Reviewed-by Igor Grinberg <[email protected]>

---
- 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(-)

drivers/usb/ulpi/omap-ulpi-viewport.c +8 ~~-32~~

index 3c1ea1a..2a42033 100644
@@ -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 & 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 & 0xf) << 24) |
-			 OMAP_ULPI_WR_OPSEL | ((u32)reg << 16);
+			 OMAP_ULPI_RD_OPSEL | ((u32)reg << 16);
 
 	err = ulpi_request(ulpi_vp, val);
 	if (err)

Signature · 1 line

1.7.9.5

Marek Vasut

2013-06-09 20:05 UTC #1 lore raw

PATCH v3 1/1 usb: omap: ulpi: fix ulpi transceiver access

Dear Michael Trimarchi,

Quoted 10 lines

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.

Quoted 2 lines

Signed-off-by Michael Trimarchi <[email protected]>
Reviewed-by Igor Grinberg <[email protected]>

Tom, can you ACK/NAK this ? I have no omap board.

Best regards,
Marek Vasut

Michael Trimarchi

author

2013-06-09 21:01 UTC #2 lore raw

PATCH v3 1/1 usb: omap: ulpi: fix ulpi transceiver access

Dear Marek

Quoted 12 lines · Marek Vasut

Dear Michael Trimarchi,
Quoted 10 lines
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.

Quoted 5 lines

Quoted 2 lines
Signed-off-by Michael Trimarchi <[email protected]>
Reviewed-by Igor Grinberg <[email protected]>
Tom, can you ACK/NAK this ? I have no omap board.

I don't understand the point, the old code was wrong and you can check omap3/omap4 documentation. If you revert it you still have a wrong code so it's better to drop omap3/4 viewport.

You can take a look at this patch

http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=2a18e1248588c326f0a63c5bce4a611d709130a8

that is used to fix this errata

http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=a0dd0ee69578e32f1469596b8fd3a6c8ef172d42

I'm using this ulpi code in one of our device. I have fixed the u-boot viewport code because I have seen it wrong. Sorry for the late response but I was busy for a Wedding ;), I can try to test it tomorrow on an omap3 device but I think that is more easy for Stefano because he has already a platform with a recent uboot

Quoted 3 lines

Best regards,
Marek Vasut

Michael

Marek Vasut

2013-06-09 21:09 UTC #3 lore raw

PATCH v3 1/1 usb: omap: ulpi: fix ulpi transceiver access

Dear Michael Trimarchi,

Quoted 29 lines

Dear Marek
Quoted 12 lines · Marek Vasut
Dear Michael Trimarchi,
Quoted 10 lines
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.
Quoted 4 lines
Quoted 2 lines
Signed-off-by Michael Trimarchi <[email protected]>
Reviewed-by Igor Grinberg <[email protected]>
Tom, can you ACK/NAK this ? I have no omap board.
I don't understand the point, the old code was wrong and you can check omap3/omap4 documentation. If you revert it you still have a wrong code so it's better to drop omap3/4 viewport.
You can take a look at this patch
http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=2a18e1248588c326f
0a63c5bce4a611d709130a8

Quoted 8 lines

that is used to fix this errata
http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=a0dd0ee69578e32f1
469596b8fd3a6c8ef172d42
I'm using this ulpi code in one of our device. I have fixed the u-boot viewport code because I have seen it wrong. Sorry for the late response but I was busy for a Wedding ;)

Fear not! I'm busy having no life, it's really hard task! I will actually be busy with that until sometimes mid-next-week.

Quoted 3 lines

I can try to test it tomorrow on an omap3
device but I think that is more easy for Stefano because he has already a platform with a recent uboot

I don't care who tests it, I'd just like to make sure it's tested on more devices than one ;-)

Best regards,
Marek Vasut

Michael Trimarchi

author

2013-06-09 21:17 UTC #4 lore raw

PATCH v3 1/1 usb: omap: ulpi: fix ulpi transceiver access

Hi Marek

Quoted 43 lines · Marek Vasut

Dear Michael Trimarchi,
Quoted 29 lines
Dear Marek
Quoted 12 lines · Marek Vasut
Dear Michael Trimarchi,
Quoted 10 lines
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.
Quoted 4 lines
Quoted 2 lines
Signed-off-by Michael Trimarchi <[email protected]>
Reviewed-by Igor Grinberg <[email protected]>
Tom, can you ACK/NAK this ? I have no omap board.
I don't understand the point, the old code was wrong and you can check omap3/omap4 documentation. If you revert it you still have a wrong code so it's better to drop omap3/4 viewport.
You can take a look at this patch
http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=2a18e1248588c326f
0a63c5bce4a611d709130a8
Quoted 8 lines
that is used to fix this errata
http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=a0dd0ee69578e32f1
469596b8fd3a6c8ef172d42
I'm using this ulpi code in one of our device. I have fixed the u-boot viewport code because I have seen it wrong. Sorry for the late response but I was busy for a Wedding ;)
Fear not! I'm busy having no life, it's really hard task! I will actually be busy with that until sometimes mid-next-week.

Quoted 6 lines

Quoted 3 lines
I can try to test it tomorrow on an omap3
device but I think that is more easy for Stefano because he has already a platform with a recent uboot
I don't care who tests it, I'd just like to make sure it's tested on more devices than one ;-)

I think that I have already understand the problem.
The port_num is used starting from 0 in omap-ehci, so if this is correct my patch need a fix

from

u32 val = (OMAP_ULPI_START | (ulpi_vp->port_num & 0xf) << 24) |
OMAP_ULPI_WR_OPSEL | ((u32)reg << 16) | (value & 0xff);

u32 val = (OMAP_ULPI_START | ((ulpi_vp->port_num + 1) & 0xf) << 24) |
OMAP_ULPI_WR_OPSEL | ((u32)reg << 16) | (value & 0xff);

Michael

Quoted 4 lines

Best regards,
Marek Vasut

Marek Vasut

2013-06-09 21:49 UTC #5 lore raw

PATCH v3 1/1 usb: omap: ulpi: fix ulpi transceiver access

Dear Michael Trimarchi,

Quoted 62 lines

Hi Marek
Quoted 43 lines · Marek Vasut
Dear Michael Trimarchi,
Quoted 29 lines
Dear Marek
Quoted 12 lines · Marek Vasut
Dear Michael Trimarchi,
Quoted 10 lines
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.
Quoted 4 lines
Quoted 2 lines
Signed-off-by Michael Trimarchi <[email protected]>
Reviewed-by Igor Grinberg <[email protected]>
Tom, can you ACK/NAK this ? I have no omap board.
I don't understand the point, the old code was wrong and you can check omap3/omap4 documentation. If you revert it you still have a wrong code so it's better to drop omap3/4 viewport.
You can take a look at this patch
http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=2a18e1248588c3
26f 0a63c5bce4a611d709130a8
Quoted 8 lines
that is used to fix this errata
http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=a0dd0ee69578e3
2f1 469596b8fd3a6c8ef172d42
I'm using this ulpi code in one of our device. I have fixed the u-boot viewport code because I have seen it wrong. Sorry for the late response but I was busy for a Wedding ;)
Fear not! I'm busy having no life, it's really hard task! I will actually be busy with that until sometimes mid-next-week.
Quoted 6 lines
Quoted 3 lines
I can try to test it tomorrow on an omap3
device but I think that is more easy for Stefano because he has already a platform with a recent uboot
I don't care who tests it, I'd just like to make sure it's tested on more devices than one ;-)
I think that I have already understand the problem.
The port_num is used starting from 0 in omap-ehci, so if this is correct my patch need a fix
from
u32 val = (OMAP_ULPI_START | (ulpi_vp->port_num & 0xf) << 24) |
OMAP_ULPI_WR_OPSEL | ((u32)reg << 16) | (value & 0xff);

Quoted 6 lines

to
u32 val = (OMAP_ULPI_START | ((ulpi_vp->port_num + 1) & 0xf) << 24) |
OMAP_ULPI_WR_OPSEL | ((u32)reg << 16) | (value & 0xff);
Michael

Make sure you base this stuff on u-boot-usb/master too.

Best regards,
Marek Vasut

Igor Grinberg

2013-06-10 13:31 UTC #6 lore raw

PATCH v3 1/1 usb: omap: ulpi: fix ulpi transceiver access

Quoted 64 lines · Marek Vasut

Dear Michael Trimarchi,
Quoted 62 lines
Hi Marek
Quoted 43 lines · Marek Vasut
Dear Michael Trimarchi,
Quoted 29 lines
Dear Marek
Quoted 12 lines · Marek Vasut
Dear Michael Trimarchi,
Quoted 10 lines
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.
Quoted 4 lines
Quoted 2 lines
Signed-off-by Michael Trimarchi <[email protected]>
Reviewed-by Igor Grinberg <[email protected]>
Tom, can you ACK/NAK this ? I have no omap board.
I don't understand the point, the old code was wrong and you can check omap3/omap4 documentation. If you revert it you still have a wrong code so it's better to drop omap3/4 viewport.
You can take a look at this patch
http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=2a18e1248588c3
26f 0a63c5bce4a611d709130a8
Quoted 8 lines
that is used to fix this errata
http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=a0dd0ee69578e3
2f1 469596b8fd3a6c8ef172d42
I'm using this ulpi code in one of our device. I have fixed the u-boot viewport code because I have seen it wrong. Sorry for the late response but I was busy for a Wedding ;)
Fear not! I'm busy having no life, it's really hard task! I will actually be busy with that until sometimes mid-next-week.
Quoted 6 lines
Quoted 3 lines
I can try to test it tomorrow on an omap3
device but I think that is more easy for Stefano because he has already a platform with a recent uboot
I don't care who tests it, I'd just like to make sure it's tested on more devices than one ;-)
I think that I have already understand the problem.
The port_num is used starting from 0 in omap-ehci, so if this is correct my patch need a fix
from
u32 val = (OMAP_ULPI_START | (ulpi_vp->port_num & 0xf) << 24) |
OMAP_ULPI_WR_OPSEL | ((u32)reg << 16) | (value & 0xff);

Quoted 8 lines

Quoted 6 lines
to
u32 val = (OMAP_ULPI_START | ((ulpi_vp->port_num + 1) & 0xf) << 24) |
OMAP_ULPI_WR_OPSEL | ((u32)reg << 16) | (value & 0xff);
Michael
Make sure you base this stuff on u-boot-usb/master too.

I can see the v3 of this patch is already in Tom's repo and he has sent a pull request with it.
I think currently it would be better to just do an incremental patch fixing the port number to avoid any collisions, no? Tom?

Signature · 2 lines

Regards,
Igor.

Michael Trimarchi

author

2013-06-10 13:37 UTC #7 lore raw

PATCH v3 1/1 usb: omap: ulpi: fix ulpi transceiver access

Quoted 70 lines · Igor Grinberg

Quoted 69 lines · Marek Vasut
Dear Michael Trimarchi,
Quoted 67 lines
Hi Marek
Quoted 43 lines · Marek Vasut
Dear Michael Trimarchi,
Quoted 29 lines
Dear Marek
Quoted 12 lines · Marek Vasut
Dear Michael Trimarchi,
Quoted 10 lines
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.
Quoted 4 lines
Quoted 2 lines
Signed-off-by Michael Trimarchi <[email protected]>
Reviewed-by Igor Grinberg <[email protected]>
Tom, can you ACK/NAK this ? I have no omap board.
I don't understand the point, the old code was wrong and you can check omap3/omap4 documentation. If you revert it you still have a wrong code so it's better to drop omap3/4 viewport.
You can take a look at this patch
http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=2a18e1248588c3
26f 0a63c5bce4a611d709130a8
Quoted 8 lines
that is used to fix this errata
http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=a0dd0ee69578e3
2f1 469596b8fd3a6c8ef172d42
I'm using this ulpi code in one of our device. I have fixed the u-boot viewport code because I have seen it wrong. Sorry for the late response but I was busy for a Wedding ;)
Fear not! I'm busy having no life, it's really hard task! I will actually be busy with that until sometimes mid-next-week.
Quoted 6 lines
Quoted 3 lines
I can try to test it tomorrow on an omap3
device but I think that is more easy for Stefano because he has already a platform with a recent uboot
I don't care who tests it, I'd just like to make sure it's tested on more devices than one ;-)
I think that I have already understand the problem.
The port_num is used starting from 0 in omap-ehci, so if this is correct my patch need a fix
from
u32 val = (OMAP_ULPI_START | (ulpi_vp->port_num & 0xf) << 24) |
OMAP_ULPI_WR_OPSEL | ((u32)reg << 16) | (value & 0xff);
to
u32 val = (OMAP_ULPI_START | ((ulpi_vp->port_num + 1) & 0xf) << 24) |
OMAP_ULPI_WR_OPSEL | ((u32)reg << 16) | (value & 0xff);

Quoted 10 lines

Quoted 3 lines
Quoted 1 line
Michael
Make sure you base this stuff on u-boot-usb/master too.
I can see the v3 of this patch is already in Tom's repo and he has sent a pull request with it.
I think currently it would be better to just do an incremental patch fixing the port number to avoid any collisions, no? Tom?

Ok, I will clone Tom's repo and prepare a patch

Michael