[U-Boot] [PATCH v3] 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 michael@amarulasolutions.com Reviewed-by: Igor Grinberg grinberg@compulab.co.il --- - 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 & 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)
Dear Michael Trimarchi,
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 Reviewed-by: Igor Grinberg grinberg@compulab.co.il
Tom, can you ACK/NAK this ? I have no omap board.
Best regards, Marek Vasut
Dear Marek
On 06/09/2013 10:05 PM, Marek Vasut wrote:
Dear Michael Trimarchi,
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 Reviewed-by: Igor Grinberg grinberg@compulab.co.il
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=2a18e1248588c326f0...
that is used to fix this errata
http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=a0dd0ee69578e32f14...
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
Best regards, Marek Vasut
Michael
Dear Michael Trimarchi,
Dear Marek
On 06/09/2013 10:05 PM, Marek Vasut wrote:
Dear Michael Trimarchi,
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 Reviewed-by: Igor Grinberg grinberg@compulab.co.il
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
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.
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
Hi Marek
On 06/09/2013 11:09 PM, Marek Vasut wrote:
Dear Michael Trimarchi,
Dear Marek
On 06/09/2013 10:05 PM, Marek Vasut wrote:
Dear Michael Trimarchi,
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 Reviewed-by: Igor Grinberg grinberg@compulab.co.il
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
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.
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);
Michael
Best regards, Marek Vasut
Dear Michael Trimarchi,
Hi Marek
On 06/09/2013 11:09 PM, Marek Vasut wrote:
Dear Michael Trimarchi,
Dear Marek
On 06/09/2013 10:05 PM, Marek Vasut wrote:
Dear Michael Trimarchi,
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 Reviewed-by: Igor Grinberg grinberg@compulab.co.il
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
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.
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);
Michael
Make sure you base this stuff on u-boot-usb/master too.
Best regards, Marek Vasut
On 06/10/13 00:49, Marek Vasut wrote:
Dear Michael Trimarchi,
Hi Marek
On 06/09/2013 11:09 PM, Marek Vasut wrote:
Dear Michael Trimarchi,
Dear Marek
On 06/09/2013 10:05 PM, Marek Vasut wrote:
Dear Michael Trimarchi,
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 Reviewed-by: Igor Grinberg grinberg@compulab.co.il
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
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.
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);
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?
Hi
On 06/10/2013 03:31 PM, Igor Grinberg wrote:
On 06/10/13 00:49, Marek Vasut wrote:
Dear Michael Trimarchi,
Hi Marek
On 06/09/2013 11:09 PM, Marek Vasut wrote:
Dear Michael Trimarchi,
Dear Marek
On 06/09/2013 10:05 PM, Marek Vasut wrote:
Dear Michael Trimarchi,
> 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 > Reviewed-by: Igor Grinberg grinberg@compulab.co.il
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
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.
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);
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
participants (3)
-
Igor Grinberg -
Marek Vasut -
Michael Trimarchi