Patchwork [U-Boot,v4] usb: omap: ulpi: fix ulpi transceiver access

login
register
mail settings
Submitter Michael Trimarchi
Date June 10, 2013, 4:18 p.m.
Message ID <20130610161804.GA16619@panicking>
Download mbox | patch
Permalink /patch/250290/
State Accepted
Delegated to: Tom Rini
Headers show

Comments

Michael Trimarchi - June 10, 2013, 4:18 p.m.
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.

Tested on a custom OMAP5430 board with a TUSB1210 ULPI PHY
on USBB1.

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
Acked-by: Igor Grinberg <grinberg@compulab.co.il>
Tested-by: Lubomir Popov <lpopov@mm-sol.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Marek Vasut <marex@denx.de>
Cc: Tom Rini <trini@ti.com>
---
- changes for V4
  * The OMAP INSNREG05_ULPI register expects a value of 1 for Port 1
    and of 2 for Port 2 in the PORTSEL field.
  * BIT31 OMAP_ULPI_START is used in the read function too
- changes for V3
  Fix patch subject
- changes for V2
  Fix commit message
---
 drivers/usb/ulpi/omap-ulpi-viewport.c |   42 +++++++--------------------------
 1 file changed, 9 insertions(+), 33 deletions(-)
Michael Trimarchi - June 20, 2013, 7:53 p.m.
Hi Tom

Should I need to repost?

Michael

On 06/10/2013 06:18 PM, Michael Trimarchi wrote:
> 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.
> 
> Tested on a custom OMAP5430 board with a TUSB1210 ULPI PHY
> on USBB1.
> 
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> Acked-by: Igor Grinberg <grinberg@compulab.co.il>
> Tested-by: Lubomir Popov <lpopov@mm-sol.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Tom Rini <trini@ti.com>
> ---
> - changes for V4
>   * The OMAP INSNREG05_ULPI register expects a value of 1 for Port 1
>     and of 2 for Port 2 in the PORTSEL field.
>   * BIT31 OMAP_ULPI_START is used in the read function too
> - changes for V3
>   Fix patch subject
> - changes for V2
>   Fix commit message
> ---
>  drivers/usb/ulpi/omap-ulpi-viewport.c |   42 +++++++--------------------------
>  1 file changed, 9 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/usb/ulpi/omap-ulpi-viewport.c b/drivers/usb/ulpi/omap-ulpi-viewport.c
> index 3c1ea1a..4db7fa4 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);
> @@ -94,8 +70,8 @@ int ulpi_write(struct ulpi_viewport *ulpi_vp, u8 *reg, u32 value)
>  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);
> +	u32 val = OMAP_ULPI_START | (((ulpi_vp->port_num + 1) & 0xf) << 24) |
> +			 OMAP_ULPI_RD_OPSEL | ((u32)reg << 16);
>  
>  	err = ulpi_request(ulpi_vp, val);
>  	if (err)
>
Tom Rini - June 20, 2013, 8:02 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/20/2013 03:53 PM, Michael Trimarchi wrote:
> Hi Tom
> 
> Should I need to repost?

Nope, we're good.  Just had this assigned to Marek in patchwork when I
did my last pull request, despite telling Marek I'd do it via
u-boot-ti on IRC before.  I'll pick this up soon.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRw1++AAoJENk4IS6UOR1WK+8QAIZTiXwBkK+n9oR62cEYXh2F
NpW7MYLlX8VjwgjQAWuZGYCGq3sJsu4E5xlFp0CWgRUCgvC9qwnGFHizqfWMRraV
Obv1iLrdHFvp0KaRWsHTxSzDTYWex9yjZ5JO81igibVx8/Jgbe/rdjctxQ3vG8yY
1X04VWeEy1mfFiqd9cw/t0Ll7+97XtS2WtgkxCV72JN5m9HKHjcmRoUdcgLarfl4
B3FWzP7Rn6Z1PN6meSIiHeOwlxB0PfxoOnX3c6SUSwYpXYw+j1TcvCntyM8AfOw3
UA3WCUAgcxHjmozB3+SlB00Mfa8G5GCDr8pjhEZ5zZD9ED+2ZZO59zBc5HGrigZb
8CUZmLP+PQYdYNrrorwlu+P3xZiCAtfYuDOCz9Cx4CzkiwGFSAhY0mi1od+c0SpA
7l+v9IgSHTNlIVgLe/doFt0feg968S3DGzfT9kqq29hkc3MqLN0z9uG3vVrUPeg+
2Vrfom3cBnqgNHrYCel+RveFzmD/B5MxO7lgvGFRTW8a/LTwe3t9AtJAot0J4zCZ
UP9X65tkpKppO579auxn7fglH6mdwpKwxW4frG6nPB4Na+fGZoL5CLvkeWAnBmXC
a7YGKhEXlyHmT8tDloT9/0G96Klbqtq/lo2ZFpw7LHlj6MmoDBuouc3jJQj3vsHP
X7srCt0r63B2KPJKlhro
=dKrX
-----END PGP SIGNATURE-----
Marek Vasut - June 20, 2013, 8:14 p.m.
Dear Tom Rini,

> On 06/20/2013 03:53 PM, Michael Trimarchi wrote:
> > Hi Tom
> > 
> > Should I need to repost?
> 
> Nope, we're good.  Just had this assigned to Marek in patchwork when I
> did my last pull request, despite telling Marek I'd do it via
> u-boot-ti on IRC before.  I'll pick this up soon.

Urgh, ok. I'll leave that up to you Tom

Best regards,
Marek Vasut
Tom Rini - July 2, 2013, 8:06 p.m.
On Mon, Jun 10, 2013 at 06:18:04PM +0200, Michael Trimarchi wrote:

> 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.
> 
> Tested on a custom OMAP5430 board with a TUSB1210 ULPI PHY
> on USBB1.
> 
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> Acked-by: Igor Grinberg <grinberg@compulab.co.il>
> Tested-by: Lubomir Popov <lpopov@mm-sol.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Tom Rini <trini@ti.com>

Applied to u-boot-ti/master, thanks!

Patch

diff --git a/drivers/usb/ulpi/omap-ulpi-viewport.c b/drivers/usb/ulpi/omap-ulpi-viewport.c
index 3c1ea1a..4db7fa4 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);
@@ -94,8 +70,8 @@  int ulpi_write(struct ulpi_viewport *ulpi_vp, u8 *reg, u32 value)
 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);
+	u32 val = OMAP_ULPI_START | (((ulpi_vp->port_num + 1) & 0xf) << 24) |
+			 OMAP_ULPI_RD_OPSEL | ((u32)reg << 16);
 
 	err = ulpi_request(ulpi_vp, val);
 	if (err)