Add generic ULPI layer

From: Jana Rapava <[email protected]>

Add partial ULPI specification implementation that should be enough to interface the ULPI PHYs in the boot loader context.
Add a viewport implementation for Chipidea/ARC based controllers.

Signed-off-by Jana Rapava <[email protected]>

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

Cc Remy Bohmer <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Wolfgang Grandegger <[email protected]>

Cc Simon Glass <[email protected]>

From: Jana Rapava <[email protected]>

Add partial ULPI specification implementation that should be enough to interface the ULPI PHYs in the boot loader context.
Add a viewport implementation for Chipidea/ARC based controllers.

Signed-off-by Jana Rapava <[email protected]>

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

Cc Remy Bohmer <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Wolfgang Grandegger <[email protected]>

Cc Simon Glass <[email protected]>

---

?Makefile ? ? ? ? ? ? ? ? ? ? ? ? | ? ?1 +
?drivers/usb/ulpi/Makefile ? ? ? ?| ? 44 ++++++
?drivers/usb/ulpi/ulpi-viewport.c | ?118 +++++++++++++++
?drivers/usb/ulpi/ulpi.c ? ? ? ? ?| ?227 +++++++++++++++++++++++++++++
?include/usb/ulpi.h ? ? ? ? ? ? ? | ?298 ++++++++++++++++++++++++++++++++++++++

?5 files changed, 688 insertions(+), 0 deletions(-) ?create mode 100644 drivers/usb/ulpi/Makefile ?create mode 100644 drivers/usb/ulpi/ulpi-viewport.c ?create mode 100644 drivers/usb/ulpi/ulpi.c ?create mode 100644 include/usb/ulpi.h

drivers/usb/ulpi/ulpi.c new file +72 -0

new file mode 100644
index 0000000..805e29d
@@ -0,0 +1,227 @@
+/*
+ * Copyright (C) 2011 Jana Rapava <[email protected]>
+ * Copyright (C) 2011 CompuLab, Ltd. <www.compulab.co.il>
+ *
+ * Authors: Jana Rapava <[email protected]>
+ * ? ? ? ? Igor Grinberg <[email protected]>
+ *
+ * Based on:
+ * linux/drivers/usb/otg/ulpi.c
+ * Generic ULPI USB transceiver support
+ *
+ * Original Copyright follow:
+ * Copyright (C) 2009 Daniel Mack <[email protected]>
+ *
+ * Based on sources from
+ *
+ * ? Sascha Hauer <[email protected]>
+ * ? Freescale Semiconductors
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the
+ * GNU General Public License for more details.
+ */
+
+#include <common.h>
+#include <exports.h>
+#include <usb/ulpi.h>
+
+#define ULPI_ID_REGS_COUNT ? ? 4
+#define ULPI_TEST_VALUE ? ? ? ? ? ? ? ?0x55 ? ?/* 0x55 == 0b01010101 */
+
+static struct ulpi_regs *ulpi = (struct ulpi_regs *)0;
+
+static int ulpi_integrity_check(u32 ulpi_viewport)
+{
+ ? ? ? u32 err, val, tval = ULPI_TEST_VALUE;
+ ? ? ? int i;
+
+ ? ? ? /* Use the 'special' test value to check all bits */
+ ? ? ? for (i = 0; i < 2; i++, tval <<= 1) {
+ ? ? ? ? ? ? ? err = ulpi_write(ulpi_viewport, &ulpi->scratch, tval);
+ ? ? ? ? ? ? ? if (err)
+ ? ? ? ? ? ? ? ? ? ? ? return err;
+
+ ? ? ? ? ? ? ? val = ulpi_read(ulpi_viewport, &ulpi->scratch);
+ ? ? ? ? ? ? ? if (val != tval) {
+ ? ? ? ? ? ? ? ? ? ? ? printf("ULPI integrity check failed\n");
+ ? ? ? ? ? ? ? ? ? ? ? return val;
+ ? ? ? ? ? ? ? }
+ ? ? ? }
+
+ ? ? ? return 0;
+}
+
+int ulpi_init(u32 ulpi_viewport)
+{
+ ? ? ? u32 val, id = 0;
+ ? ? ? u8 *reg = &ulpi->product_id_high;
+ ? ? ? int i;
+
+ ? ? ? /* Assemble ID from four ULPI ID registers (8 bits each). */
+ ? ? ? for (i = 0; i < ULPI_ID_REGS_COUNT; i++) {
+ ? ? ? ? ? ? ? val = ulpi_read(ulpi_viewport, reg - i);
+ ? ? ? ? ? ? ? if (val == ULPI_ERROR)
+ ? ? ? ? ? ? ? ? ? ? ? return val;
+

drivers/usb/ulpi/ulpi.c new file +38 -0

+{
+ ? ? ? u8 tspeed = ULPI_FC_FULL_SPEED;
+ ? ? ? u32 val;
+
+ ? ? ? switch (speed) {
+ ? ? ? case ULPI_FC_HIGH_SPEED:
+ ? ? ? case ULPI_FC_FULL_SPEED:
+ ? ? ? case ULPI_FC_LOW_SPEED:
+ ? ? ? case ULPI_FC_FS4LS:
+ ? ? ? ? ? ? ? tspeed = speed;
+ ? ? ? ? ? ? ? break;
+ ? ? ? default:
+ ? ? ? ? ? ? ? printf("ULPI: %s: wrong transceiver speed specified, "
+ ? ? ? ? ? ? ? ? ? ? ? "falling back to full speed\n", __func__);
+ ? ? ? }
+
+ ? ? ? val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);
+ ? ? ? if (val == ULPI_ERROR)
+ ? ? ? ? ? ? ? return val;
+
+ ? ? ? /* clear the previous speed setting */
+ ? ? ? val = (val & ~ULPI_FC_XCVRSEL_MASK) | tspeed;
+
+ ? ? ? return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);
+}
+
+int ulpi_set_vbus(u32 ulpi_viewport, int on, int ext_power, int ext_ind)
+{
+ ? ? ? u32 flags = ULPI_OTG_DRVVBUS;
+ ? ? ? u8 *reg = on ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;
+
+ ? ? ? if (ext_power)
+ ? ? ? ? ? ? ? flags |= ULPI_OTG_DRVVBUS_EXT;
+ ? ? ? if (ext_ind)
+ ? ? ? ? ? ? ? flags |= ULPI_OTG_EXTVBUSIND;
+
+ ? ? ? return ulpi_write(ulpi_viewport, reg, flags);
+}

drivers/usb/ulpi/ulpi.c new file +17 -0

+{
+ ? ? ? u8 topmode = ULPI_FC_OPMODE_NORMAL;
+ ? ? ? u32 val;
+
+ ? ? ? switch (opmode) {
+ ? ? ? case ULPI_FC_OPMODE_NORMAL:
+ ? ? ? case ULPI_FC_OPMODE_NONDRIVING:
+ ? ? ? case ULPI_FC_OPMODE_DISABLE_NRZI:
+ ? ? ? case ULPI_FC_OPMODE_NOSYNC_NOEOP:
+ ? ? ? ? ? ? ? topmode = opmode;
+ ? ? ? ? ? ? ? break;
+ ? ? ? default:
+ ? ? ? ? ? ? ? printf("ULPI: %s: wrong OpMode specified, "
+ ? ? ? ? ? ? ? ? ? ? ? "falling back to OpMode Normal\n", __func__);
+ ? ? ? }
+
+ ? ? ? val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);

drivers/usb/ulpi/ulpi.c new file +7 -0

+{
+ ? ? ? switch (smode) {
+ ? ? ? case ULPI_IFACE_6_PIN_SERIAL_MODE:
+ ? ? ? case ULPI_IFACE_3_PIN_SERIAL_MODE:
+ ? ? ? ? ? ? ? break;
+ ? ? ? default:
+ ? ? ? ? ? ? ? printf("ULPI: %s: unrecognized Serial Mode specified\n",

drivers/usb/ulpi/ulpi.c new file +33 -0

+
+ ? ? ? err = ulpi_write(ulpi_viewport, &ulpi->function_ctrl_clear,
+ ? ? ? ? ? ? ? ? ? ? ? ULPI_FC_SUSPENDM);
+ ? ? ? if (err)
+ ? ? ? ? ? ? ? printf("ULPI: %s: failed writing the suspend bit\n", __func__);
+
+ ? ? ? return err;
+}
+
+/*
+ * Wait for ULPI PHY reset to complete.
+ * Actual wait for reset must be done in a view port specific way,
+ * because it involves checking the DIR line.
+ */
+static int __ulpi_reset_wait(u32 ulpi_viewport)
+{
+ ? ? ? u32 val;
+ ? ? ? int timeout = CONFIG_USB_ULPI_TIMEOUT;
+
+ ? ? ? /* Wait for the RESET bit to become zero */
+ ? ? ? while (--timeout) {
+ ? ? ? ? ? ? ? /*
+ ? ? ? ? ? ? ? ?* This function is generic and suppose to work
+ ? ? ? ? ? ? ? ?* with any viewport, so we cheat here and don't check
+ ? ? ? ? ? ? ? ?* for the error of ulpi_read(), if there is one, then
+ ? ? ? ? ? ? ? ?* there will be a timeout.
+ ? ? ? ? ? ? ? ?*/
+ ? ? ? ? ? ? ? val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);
+ ? ? ? ? ? ? ? if (!(val & ULPI_FC_RESET))
+ ? ? ? ? ? ? ? ? ? ? ? return 0;
+
+ ? ? ? ? ? ? ? udelay(1);
+ ? ? ? }

Hi Igor,

Looks good - a few comments from me.

Quoted 12 lines · Igor Grinberg

From: Jana Rapava <[email protected]>

Add partial ULPI specification implementation that should be enough to interface the ULPI PHYs in the boot loader context.
Add a viewport implementation for Chipidea/ARC based controllers.

Signed-off-by Jana Rapava <[email protected]>

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

Cc Remy Bohmer <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Wolfgang Grandegger <[email protected]>

Cc Simon Glass <[email protected]>

Quoted 6 lines

---
 Makefile                         |    1 +
 drivers/usb/ulpi/Makefile        |   44 ++++++
 drivers/usb/ulpi/ulpi-viewport.c |  118 +++++++++++++++
 drivers/usb/ulpi/ulpi.c          |  227 +++++++++++++++++++++++++++++
 include/usb/ulpi.h               |  298 ++++++++++++++++++++++++++++++++++++++

This would benefit from additions to the README describing the two new CONFIGs you add.

Quoted 89 lines

 5 files changed, 688 insertions(+), 0 deletions(-)
 create mode 100644 drivers/usb/ulpi/Makefile
 create mode 100644 drivers/usb/ulpi/ulpi-viewport.c
 create mode 100644 drivers/usb/ulpi/ulpi.c
 create mode 100644 include/usb/ulpi.h

drivers/usb/ulpi/ulpi.c new file +76 -0

new file mode 100644
index 0000000..805e29d
@@ -0,0 +1,227 @@
+/*
+ * Copyright (C) 2011 Jana Rapava <[email protected]>
+ * Copyright (C) 2011 CompuLab, Ltd. <www.compulab.co.il>
+ *
+ * Authors: Jana Rapava <[email protected]>
+ *         Igor Grinberg <[email protected]>
+ *
+ * Based on:
+ * linux/drivers/usb/otg/ulpi.c
+ * Generic ULPI USB transceiver support
+ *
+ * Original Copyright follow:
+ * Copyright (C) 2009 Daniel Mack <[email protected]>
+ *
+ * Based on sources from
+ *
+ *   Sascha Hauer <[email protected]>
+ *   Freescale Semiconductors
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <common.h>
+#include <exports.h>
+#include <usb/ulpi.h>
+
+#define ULPI_ID_REGS_COUNT     4
+#define ULPI_TEST_VALUE                0x55    /* 0x55 == 0b01010101 */
+
+static struct ulpi_regs *ulpi = (struct ulpi_regs *)0;
+
+static int ulpi_integrity_check(u32 ulpi_viewport)
+{
+       u32 err, val, tval = ULPI_TEST_VALUE;
+       int i;
+
+       /* Use the 'special' test value to check all bits */
+       for (i = 0; i < 2; i++, tval <<= 1) {
+               err = ulpi_write(ulpi_viewport, &ulpi->scratch, tval);
+               if (err)
+                       return err;
+
+               val = ulpi_read(ulpi_viewport, &ulpi->scratch);
+               if (val != tval) {
+                       printf("ULPI integrity check failed\n");
+                       return val;
+               }
+       }
+
+       return 0;
+}
+
+int ulpi_init(u32 ulpi_viewport)
+{
+       u32 val, id = 0;
+       u8 *reg = &ulpi->product_id_high;
+       int i;
+
+       /* Assemble ID from four ULPI ID registers (8 bits each). */
+       for (i = 0; i < ULPI_ID_REGS_COUNT; i++) {
+               val = ulpi_read(ulpi_viewport, reg - i);
+               if (val == ULPI_ERROR)
+                       return val;
+
+               id = (id << 8) | val;
+       }
+
+       /* Split ID into vendor and product ID. */

drivers/usb/ulpi/ulpi.c line 82

Quoted 6 lines

drivers/usb/ulpi/ulpi.c new file +6 -0

+       debug("ULPI transceiver ID 0x%04x:0x%04x\n", id >> 16, id & 0xffff);
+
+       return ulpi_integrity_check(ulpi_viewport);
+}
+
+int ulpi_select_transceiver(u32 ulpi_viewport, u8 speed)

Is there any point in making the argument u8?

How about just unsigned?

I think this adds a mask to the call = larger code size. You check the arg with the switch() below anyway.

Quoted 43 lines

drivers/usb/ulpi/ulpi.c new file +42 -0

+{
+       u8 tspeed = ULPI_FC_FULL_SPEED;
+       u32 val;
+
+       switch (speed) {
+       case ULPI_FC_HIGH_SPEED:
+       case ULPI_FC_FULL_SPEED:
+       case ULPI_FC_LOW_SPEED:
+       case ULPI_FC_FS4LS:
+               tspeed = speed;
+               break;
+       default:
+               printf("ULPI: %s: wrong transceiver speed specified, "
+                       "falling back to full speed\n", __func__);
+       }
+
+       val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);
+       if (val == ULPI_ERROR)
+               return val;
+
+       /* clear the previous speed setting */
+       val = (val & ~ULPI_FC_XCVRSEL_MASK) | tspeed;
+
+       return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);
+}
+
+int ulpi_set_vbus(u32 ulpi_viewport, int on, int ext_power, int ext_ind)
+{
+       u32 flags = ULPI_OTG_DRVVBUS;
+       u8 *reg = on ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;
+
+       if (ext_power)
+               flags |= ULPI_OTG_DRVVBUS_EXT;
+       if (ext_ind)
+               flags |= ULPI_OTG_EXTVBUSIND;
+
+       return ulpi_write(ulpi_viewport, reg, flags);
+}
+
+int ulpi_set_pd(u32 ulpi_viewport, int enable)
+{
+       u32 val = ULPI_OTG_DP_PULLDOWN | ULPI_OTG_DM_PULLDOWN;

drivers/usb/ulpi/ulpi.c line 130

Quoted 6 lines

drivers/usb/ulpi/ulpi.c new file +6 -0

+       u8 *reg = enable ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;
+
+       return ulpi_write(ulpi_viewport, reg, val);
+}
+
+int ulpi_opmode_sel(u32 ulpi_viewport, u8 opmode)

suggest unsigned for arg instead of u8

Quoted 18 lines

drivers/usb/ulpi/ulpi.c new file +17 -0

+{
+       u8 topmode = ULPI_FC_OPMODE_NORMAL;
+       u32 val;
+
+       switch (opmode) {
+       case ULPI_FC_OPMODE_NORMAL:
+       case ULPI_FC_OPMODE_NONDRIVING:
+       case ULPI_FC_OPMODE_DISABLE_NRZI:
+       case ULPI_FC_OPMODE_NOSYNC_NOEOP:
+               topmode = opmode;
+               break;
+       default:
+               printf("ULPI: %s: wrong OpMode specified, "
+                       "falling back to OpMode Normal\n", __func__);
+       }
+
+       val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);

drivers/usb/ulpi/ulpi.c line 157

Quoted 10 lines

drivers/usb/ulpi/ulpi.c new file +10 -0

+       if (val == ULPI_ERROR)
+               return val;
+
+       /* clear the previous opmode setting */
+       val = (val & ~ULPI_FC_OPMODE_MASK) | topmode;
+
+       return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);
+}
+
+int ulpi_serial_mode_enable(u32 ulpi_viewport, u8 smode)

and again

Quoted 13 lines

drivers/usb/ulpi/ulpi.c new file +13 -0

+{
+       switch (smode) {
+       case ULPI_IFACE_6_PIN_SERIAL_MODE:
+       case ULPI_IFACE_3_PIN_SERIAL_MODE:
+               break;
+       default:
+               printf("ULPI: %s: unrecognized Serial Mode specified\n",
+                       __func__);
+               return ULPI_ERROR;
+       }
+
+       return ulpi_write(ulpi_viewport, &ulpi->iface_ctrl_set, smode);
+}

drivers/usb/ulpi/ulpi.c line 174

Quoted 4 lines

drivers/usb/ulpi/ulpi.c new file +4 -0

+
+int ulpi_suspend(u32 ulpi_viewport)
+{
+       u32 err;

This function returns int but err is u32. Which do you want? I think function return values should be native types, int or unsigned in this case.

Quoted 34 lines

drivers/usb/ulpi/ulpi.c new file +33 -0

+
+       err = ulpi_write(ulpi_viewport, &ulpi->function_ctrl_clear,
+                       ULPI_FC_SUSPENDM);
+       if (err)
+               printf("ULPI: %s: failed writing the suspend bit\n", __func__);
+
+       return err;
+}
+
+/*
+ * Wait for ULPI PHY reset to complete.
+ * Actual wait for reset must be done in a view port specific way,
+ * because it involves checking the DIR line.
+ */
+static int __ulpi_reset_wait(u32 ulpi_viewport)
+{
+       u32 val;
+       int timeout = CONFIG_USB_ULPI_TIMEOUT;
+
+       /* Wait for the RESET bit to become zero */
+       while (--timeout) {
+               /*
+                * This function is generic and suppose to work
+                * with any viewport, so we cheat here and don't check
+                * for the error of ulpi_read(), if there is one, then
+                * there will be a timeout.
+                */
+               val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);
+               if (!(val & ULPI_FC_RESET))
+                       return 0;
+
+               udelay(1);
+       }

drivers/usb/ulpi/ulpi.c line 217

Quoted 10 lines

drivers/usb/ulpi/ulpi.c new file +10 -0

+
+       printf("ULPI: %s: reset timed out\n", __func__);
+
+       return ULPI_ERROR;
+}
+int ulpi_reset_wait(u32) __attribute__((weak, alias("__ulpi_reset_wait")));
+
+int ulpi_reset(u32 ulpi_viewport)
+{
+       u32 err;

same as above function

drivers/usb/ulpi/ulpi.c line 227

Quoted 10 lines

drivers/usb/ulpi/ulpi.c new file +10 -0

+
+       err = ulpi_write(ulpi_viewport,
+                       &ulpi->function_ctrl_set, ULPI_FC_RESET);
+       if (err) {
+               printf("ULPI: %s: failed writing reset bit\n", __func__);
+               return err;
+       }
+
+       return ulpi_reset_wait(ulpi_viewport);
+}

In general if you have time you could check that you document all the args in your comments rather than just some. But I think in all cases it is pretty clear anyway.

Just out of interest, is it possible to test this? How would I plumb it in?

Quoted 5 lines

Hi Igor,

Looks good - a few comments from me.

On Mon, Dec 5, 2011 at 1:07 AM, Igor Grinberg <[email protected]>

Quoted 112 lines

Quoted 12 lines

From: Jana Rapava <[email protected]>

Add partial ULPI specification implementation that should be enough to interface the ULPI PHYs in the boot loader context.
Add a viewport implementation for Chipidea/ARC based controllers.

Signed-off-by Jana Rapava <[email protected]>

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

Cc Remy Bohmer <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Wolfgang Grandegger <[email protected]>

Cc Simon Glass <[email protected]>

Quoted 8 lines

---

 Makefile                         |    1 +
 drivers/usb/ulpi/Makefile        |   44 ++++++
 drivers/usb/ulpi/ulpi-viewport.c |  118 +++++++++++++++
 drivers/usb/ulpi/ulpi.c          |  227 +++++++++++++++++++++++++++++
 include/usb/ulpi.h               |  298

++++++++++++++++++++++++++++++++++++++

This would benefit from additions to the README describing the two new CONFIGs you add.

Quoted 88 lines

 5 files changed, 688 insertions(+), 0 deletions(-)
 create mode 100644 drivers/usb/ulpi/Makefile
 create mode 100644 drivers/usb/ulpi/ulpi-viewport.c
 create mode 100644 drivers/usb/ulpi/ulpi.c
 create mode 100644 include/usb/ulpi.h

drivers/usb/ulpi/ulpi.c new file +76 -0

new file mode 100644
index 0000000..805e29d
@@ -0,0 +1,227 @@
+/*
+ * Copyright (C) 2011 Jana Rapava <[email protected]>
+ * Copyright (C) 2011 CompuLab, Ltd. <www.compulab.co.il>
+ *
+ * Authors: Jana Rapava <[email protected]>
+ *         Igor Grinberg <[email protected]>
+ *
+ * Based on:
+ * linux/drivers/usb/otg/ulpi.c
+ * Generic ULPI USB transceiver support
+ *
+ * Original Copyright follow:
+ * Copyright (C) 2009 Daniel Mack <[email protected]>
+ *
+ * Based on sources from
+ *
+ *   Sascha Hauer <[email protected]>
+ *   Freescale Semiconductors
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <common.h>
+#include <exports.h>
+#include <usb/ulpi.h>
+
+#define ULPI_ID_REGS_COUNT     4
+#define ULPI_TEST_VALUE                0x55    /* 0x55 == 0b01010101 */
+
+static struct ulpi_regs *ulpi = (struct ulpi_regs *)0;
+
+static int ulpi_integrity_check(u32 ulpi_viewport)
+{
+       u32 err, val, tval = ULPI_TEST_VALUE;
+       int i;
+
+       /* Use the 'special' test value to check all bits */
+       for (i = 0; i < 2; i++, tval <<= 1) {
+               err = ulpi_write(ulpi_viewport, &ulpi->scratch, tval);
+               if (err)
+                       return err;
+
+               val = ulpi_read(ulpi_viewport, &ulpi->scratch);
+               if (val != tval) {
+                       printf("ULPI integrity check failed\n");
+                       return val;
+               }
+       }
+
+       return 0;
+}
+
+int ulpi_init(u32 ulpi_viewport)
+{
+       u32 val, id = 0;
+       u8 *reg = &ulpi->product_id_high;
+       int i;
+
+       /* Assemble ID from four ULPI ID registers (8 bits each). */
+       for (i = 0; i < ULPI_ID_REGS_COUNT; i++) {
+               val = ulpi_read(ulpi_viewport, reg - i);
+               if (val == ULPI_ERROR)
+                       return val;
+
+               id = (id << 8) | val;
+       }
+
+       /* Split ID into vendor and product ID. */

Quoted 8 lines

drivers/usb/ulpi/ulpi.c line 82

Quoted 6 lines

drivers/usb/ulpi/ulpi.c new file +1 -0

+       debug("ULPI transceiver ID 0x%04x:0x%04x\n", id >> 16, id &

0xffff); +

drivers/usb/ulpi/ulpi.c new file +4 -0

+       return ulpi_integrity_check(ulpi_viewport);
+}
+
+int ulpi_select_transceiver(u32 ulpi_viewport, u8 speed)

Is there any point in making the argument u8?

Not really...

Quoted 1 line

How about just unsigned?

Sounds sane

Quoted 2 lines

I think this adds a mask to the call = larger code size. You check the arg with the switch() below anyway.

Haven't thought about that...
I don't know how is this exactly works, but I don't see any reason why shouldn't I use unsigned here.

Quoted 42 lines

Quoted 42 lines

drivers/usb/ulpi/ulpi.c new file +27 -0

+{
+       u8 tspeed = ULPI_FC_FULL_SPEED;
+       u32 val;
+
+       switch (speed) {
+       case ULPI_FC_HIGH_SPEED:
+       case ULPI_FC_FULL_SPEED:
+       case ULPI_FC_LOW_SPEED:
+       case ULPI_FC_FS4LS:
+               tspeed = speed;
+               break;
+       default:
+               printf("ULPI: %s: wrong transceiver speed specified, "
+                       "falling back to full speed\n", __func__);
+       }
+
+       val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);
+       if (val == ULPI_ERROR)
+               return val;
+
+       /* clear the previous speed setting */
+       val = (val & ~ULPI_FC_XCVRSEL_MASK) | tspeed;
+
+       return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);
+}
+
+int ulpi_set_vbus(u32 ulpi_viewport, int on, int ext_power, int

ext_ind) +{

drivers/usb/ulpi/ulpi.c new file +14 -0

+       u32 flags = ULPI_OTG_DRVVBUS;
+       u8 *reg = on ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;
+
+       if (ext_power)
+               flags |= ULPI_OTG_DRVVBUS_EXT;
+       if (ext_ind)
+               flags |= ULPI_OTG_EXTVBUSIND;
+
+       return ulpi_write(ulpi_viewport, reg, flags);
+}
+
+int ulpi_set_pd(u32 ulpi_viewport, int enable)
+{
+       u32 val = ULPI_OTG_DP_PULLDOWN | ULPI_OTG_DM_PULLDOWN;

Quoted 8 lines

drivers/usb/ulpi/ulpi.c line 130

Quoted 6 lines

drivers/usb/ulpi/ulpi.c new file +6 -0

+       u8 *reg = enable ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;
+
+       return ulpi_write(ulpi_viewport, reg, val);
+}
+
+int ulpi_opmode_sel(u32 ulpi_viewport, u8 opmode)

suggest unsigned for arg instead of u8

Ok

Quoted 43 lines

Quoted 17 lines

drivers/usb/ulpi/ulpi.c new file +17 -0

+{
+       u8 topmode = ULPI_FC_OPMODE_NORMAL;
+       u32 val;
+
+       switch (opmode) {
+       case ULPI_FC_OPMODE_NORMAL:
+       case ULPI_FC_OPMODE_NONDRIVING:
+       case ULPI_FC_OPMODE_DISABLE_NRZI:
+       case ULPI_FC_OPMODE_NOSYNC_NOEOP:
+               topmode = opmode;
+               break;
+       default:
+               printf("ULPI: %s: wrong OpMode specified, "
+                       "falling back to OpMode Normal\n", __func__);
+       }
+
+       val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);

drivers/usb/ulpi/ulpi.c line 157

Quoted 10 lines

drivers/usb/ulpi/ulpi.c new file +10 -0

+       if (val == ULPI_ERROR)
+               return val;
+
+       /* clear the previous opmode setting */
+       val = (val & ~ULPI_FC_OPMODE_MASK) | topmode;
+
+       return ulpi_write(ulpi_viewport, &ulpi->function_ctrl, val);
+}
+
+int ulpi_serial_mode_enable(u32 ulpi_viewport, u8 smode)

and again

Quoted 13 lines

drivers/usb/ulpi/ulpi.c new file +13 -0

+{
+       switch (smode) {
+       case ULPI_IFACE_6_PIN_SERIAL_MODE:
+       case ULPI_IFACE_3_PIN_SERIAL_MODE:
+               break;
+       default:
+               printf("ULPI: %s: unrecognized Serial Mode specified\n",
+                       __func__);
+               return ULPI_ERROR;
+       }
+
+       return ulpi_write(ulpi_viewport, &ulpi->iface_ctrl_set, smode);
+}

Quoted 8 lines

drivers/usb/ulpi/ulpi.c line 174

Quoted 4 lines

drivers/usb/ulpi/ulpi.c new file +4 -0

+
+int ulpi_suspend(u32 ulpi_viewport)
+{
+       u32 err;

This function returns int but err is u32. Which do you want? I think function return values should be native types, int or unsigned in this case.

Yep, missed this... it also applies in some other places.

Quoted 46 lines

Quoted 40 lines

drivers/usb/ulpi/ulpi.c new file +5 -0

+
+       err = ulpi_write(ulpi_viewport, &ulpi->function_ctrl_clear,
+                       ULPI_FC_SUSPENDM);
+       if (err)
+               printf("ULPI: %s: failed writing the suspend bit\n",

__func__); +

drivers/usb/ulpi/ulpi.c new file +33 -0

+       return err;
+}
+
+/*
+ * Wait for ULPI PHY reset to complete.
+ * Actual wait for reset must be done in a view port specific way,
+ * because it involves checking the DIR line.
+ */
+static int __ulpi_reset_wait(u32 ulpi_viewport)
+{
+       u32 val;
+       int timeout = CONFIG_USB_ULPI_TIMEOUT;
+
+       /* Wait for the RESET bit to become zero */
+       while (--timeout) {
+               /*
+                * This function is generic and suppose to work
+                * with any viewport, so we cheat here and don't check
+                * for the error of ulpi_read(), if there is one, then
+                * there will be a timeout.
+                */
+               val = ulpi_read(ulpi_viewport, &ulpi->function_ctrl);
+               if (!(val & ULPI_FC_RESET))
+                       return 0;
+
+               udelay(1);
+       }
+
+       printf("ULPI: %s: reset timed out\n", __func__);
+
+       return ULPI_ERROR;
+}
+int ulpi_reset_wait(u32) __attribute__((weak,

alias("__ulpi_reset_wait"))); +

drivers/usb/ulpi/ulpi.c line 217

Quoted 3 lines

drivers/usb/ulpi/ulpi.c new file +3 -0

+int ulpi_reset(u32 ulpi_viewport)
+{
+       u32 err;

same as above function

Quoted 14 lines

drivers/usb/ulpi/ulpi.c line 227

Quoted 10 lines

drivers/usb/ulpi/ulpi.c new file +5 -0

+
+       err = ulpi_write(ulpi_viewport,
+                       &ulpi->function_ctrl_set, ULPI_FC_RESET);
+       if (err) {
+               printf("ULPI: %s: failed writing reset bit\n",

__func__); + return err;

drivers/usb/ulpi/ulpi.c new file +4 -0

+       }
+
+       return ulpi_reset_wait(ulpi_viewport);
+}

In general if you have time you could check that you document all the args in your comments rather than just some. But I think in all cases it is pretty clear anyway.

Me too, that's exactly the reason I haven't documented them all around - I don't like the stupid copy/paste work...

Quoted 2 lines

Just out of interest, is it possible to test this? How would I plumb it in?

Well, from my experience with ULPI hardware,
I think the controller specific glue looks like the right place for putting the ULPI layer calls in.

In general, the controller code knows which PHYs it supports and board code knows which PHY is assembled on the board, so it is not that straight simple to find the right place.

I think, Marek has patches that supposed to use this framework on efikamx board.

Quoted 6 lines · Simon Glass

Quoted 5 lines

Hi Igor,

Looks good - a few comments from me.

On Mon, Dec 5, 2011 at 1:07 AM, Igor Grinberg <[email protected]>

Quoted 13 lines · wrote:

Quoted 13 lines

Quoted 13 lines

From: Jana Rapava <[email protected]>

Add partial ULPI specification implementation that should be enough to interface the ULPI PHYs in the boot loader context.
Add a viewport implementation for Chipidea/ARC based controllers.

Signed-off-by Jana Rapava <[email protected]>

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

Cc Remy Bohmer <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Wolfgang Grandegger <[email protected]>

Cc Simon Glass <[email protected]>

---

Quoted 11 lines

Quoted 2 lines

Just out of interest, is it possible to test this? How would I plumb it in?

Well, from my experience with ULPI hardware,
I think the controller specific glue looks like the right place for putting the ULPI layer calls in.

In general, the controller code knows which PHYs it supports and board code knows which PHY is assembled on the board, so it is not that straight simple to find the right place.

Quoted 2 lines

I think, Marek has patches that supposed to use this framework on efikamx board.

I tried using the interface, but the design seems completely wrong :-( Jana was supposed to design it mainly for the efikamx board, but this interface is unusable there.

I had to fall back to basic ulpi_read()/ulpi_write() calls :-(

I'm afraid we won't make it for .12 release window with this patches ... very bad :-( I'll try talking to WD if he can push the release window to allow this

(or redesigned version) in, but I don't know if that's a good idea.

Hi Marek,

Quoted 21 lines · Marek Vasut

Quoted 6 lines · Simon Glass

Quoted 5 lines

Hi Igor,

Looks good - a few comments from me.

On Mon, Dec 5, 2011 at 1:07 AM, Igor Grinberg <[email protected]>

Quoted 13 lines · wrote:

Quoted 13 lines

Quoted 13 lines

From: Jana Rapava <[email protected]>

Add partial ULPI specification implementation that should be enough to interface the ULPI PHYs in the boot loader context.
Add a viewport implementation for Chipidea/ARC based controllers.

Signed-off-by Jana Rapava <[email protected]>

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

Cc Remy Bohmer <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Wolfgang Grandegger <[email protected]>

Cc Simon Glass <[email protected]>

---

[...]

Quoted 14 lines

Quoted 13 lines

Quoted 2 lines

Just out of interest, is it possible to test this? How would I plumb it in?

Well, from my experience with ULPI hardware,
I think the controller specific glue looks like the right place for putting the ULPI layer calls in.

In general, the controller code knows which PHYs it supports and board code knows which PHY is assembled on the board, so it is not that straight simple to find the right place.

I think, Marek has patches that supposed to use this framework on efikamx board.

Quoted 3 lines

I tried using the interface, but the design seems completely wrong :-( Jana was supposed to design it mainly for the efikamx board, but this interface is unusable there.

May I ask you why?
Isn't it because of that nasty VBUS bug efikamx has?
You can't say the design is wrong if it is more generic then you want...

Quoted 2 lines

I had to fall back to basic ulpi_read()/ulpi_write() calls :-(

That's too bad.
Because ulpi_{read|write}() is only a viewport implementation and it is not following the ULPI spec.

Quoted 4 lines

I'm afraid we won't make it for .12 release window with this patches ... very bad :-( I'll try talking to WD if he can push the release window to allow this

Good.

Quoted 1 line

(or redesigned version) in, but I don't know if that's a good idea.

I don't think it should be redesigned.
Currently, it is generic and abstracts the ULPI specification nicely.

It can be used on any architecture.
I have already stated in the cover letter,
what IMO is missing to improve usability, but that will not be a problem.

Do you have the efikamx patches somewhere I can look at?

Quoted 45 lines

Hi Marek,

Quoted 21 lines · Marek Vasut

Quoted 6 lines · Simon Glass

Quoted 5 lines

Hi Igor,

Looks good - a few comments from me.

On Mon, Dec 5, 2011 at 1:07 AM, Igor Grinberg <[email protected]>

Quoted 13 lines · wrote:

Quoted 13 lines

Quoted 13 lines

From: Jana Rapava <[email protected]>

Add partial ULPI specification implementation that should be enough to interface the ULPI PHYs in the boot loader context.
Add a viewport implementation for Chipidea/ARC based controllers.

Signed-off-by Jana Rapava <[email protected]>

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

Cc Remy Bohmer <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Wolfgang Grandegger <[email protected]>

Cc Simon Glass <[email protected]>

---

[...]

Quoted 17 lines

Quoted 7 lines

Quoted 2 lines

Just out of interest, is it possible to test this? How would I plumb it in?

Well, from my experience with ULPI hardware,
I think the controller specific glue looks like the right place for putting the ULPI layer calls in.

Quoted 6 lines

In general, the controller code knows which PHYs it supports and board code knows which PHY is assembled on the board, so it is not that straight simple to find the right place.

I think, Marek has patches that supposed to use this framework on efikamx board.

I tried using the interface, but the design seems completely wrong :-( Jana was supposed to design it mainly for the efikamx board, but this interface is unusable there.

Quoted 3 lines

May I ask you why?
Isn't it because of that nasty VBUS bug efikamx has?
You can't say the design is wrong if it is more generic then you want...

I think it's overengineered. Basically, to achieve what I needed, I either didn't find the right function or I had to use multiple functions. Therefore I had to fall back to plain simple ulpi_read/write().

Quoted 6 lines

Quoted 2 lines

I had to fall back to basic ulpi_read()/ulpi_write() calls :-(

That's too bad.
Because ulpi_{read|write}() is only a viewport implementation and it is not following the ULPI spec.

Well ... we'll need to rethink this.

Quoted 5 lines

Quoted 4 lines

I'm afraid we won't make it for .12 release window with this patches ... very bad :-( I'll try talking to WD if he can push the release window to allow this

Quoted 6 lines

Good.

Quoted 1 line

(or redesigned version) in, but I don't know if that's a good idea.

I don't think it should be redesigned.
Currently, it is generic and abstracts the ULPI specification nicely.

Nicely maybe, but try using it on top of some hardware that has ULPI chip.

Quoted 6 lines

It can be used on any architecture.
I have already stated in the cover letter,
what IMO is missing to improve usability, but that will not be a problem.

Do you have the efikamx patches somewhere I can look at?

Submitted to ML just a while ago.

M

Although it is a single patch, I felt that a cover letter will definetly not hurt here, also the patch version history is so big, so I decided to move it here.

The ULPI (UTMI Low Pin (count) Interface) PHYs are widely used on variety of boards. This requires a generic architecture independant implementation which can be reused and will eliminate the need for direct write of arbitrary values to the ULPI transciever. Although, the generic implementation can be reused on any architecture, the access to ULPI PHY must be done in a platform specific way. The platform specific way is in majority of case called a viewport. Also, the ULPI specification defines a hybrid aproach for managing the ULPI PHY. That is, the PHY must be managed through both the PHY registers and control lines.

The proposed patch provides a partial implementation of the ULPI specification, which should be enough for boot loader use cases, and a viewport implementation for Chipidea/ARC based controllers, which, AFAIK, are used on imx and tegra SoCs.

It is based on the Wolfgang's master branch (4 Dec 2012), compile tested and checkpatch clean.

What is still missing, IMO:
- documentation for the CONFIG_* macros (I can add it in a separate

- a way to make most of the initialization in one ulpi_init() call
- viewport extension to be able to implement resume, reset and disabling the serial mode

The change log:
Changes for v2:
- make code EHCI-independent
- use udelay() in waiting loop
- mark static functions as static
- naming changes
Changes for v3:
- merge with patch ulpi: add generic ULPI support header file
- rewrite ULPI interface in more functionality-oriented way
Changes for v4:
- add error-checking
- add waiting for completion into ulpi_reset() function
Changes for v5:
- CodingStyle changes
- add comments
- simplify implemenation of the ULPI interface functions
Changes for v6:
- cleanup function ulpi_drive_vbus()
Changes for v7:
- ulpi-viewport.c:
- reorder bit definitions
- split ulpi_request() to two functions
- reuse ulpi_wakeup() from ulpi_request()
to remove duplicated calls from ulpi_{read|write}()
- inline ulpi_*_mask as it is simple and used only once
- ulpi.c:
- move several defines into c file
- rework all the functions to propagate error values
- move function description comments into ulpi.h along with declarations
- check arguments validity (as suggested by Simon)
- fix cases when using the *_set register, bits cannot be cleared
- shorten several arguments names (e.g. ulpi_set_vbus())
- add ability to disable VBUS
- clean up ulpi_set_pd()
- add ability to enter the serial mode
- add verbosity in error cases
- remove ulpi_resume() as it were wrong and must be implemented in a viewport specific way
- rework ulpi_reset() as it must be implemented in a viewport specific way, but provide kind of generic implementation which should work in most of the cases
- ulpi.h:
- add default timeout value
- remove unused defines
- move several defines inside c files
- add description for each function
- move the API declaration to the top of the header file

Jana Rapava (1):
USB: Add generic ULPI layer and a viewport

 Makefile                         |    1 +
 drivers/usb/ulpi/Makefile        |   44 ++++++
 drivers/usb/ulpi/ulpi-viewport.c |  118 +++++++++++++++
 drivers/usb/ulpi/ulpi.c          |  227 +++++++++++++++++++++++++++++
 include/usb/ulpi.h               |  298
++++++++++++++++++++++++++++++++++++++ 5 files changed, 688 insertions(+),
0 deletions(-)
 create mode 100644 drivers/usb/ulpi/Makefile
 create mode 100644 drivers/usb/ulpi/ulpi-viewport.c
 create mode 100644 drivers/usb/ulpi/ulpi.c
 create mode 100644 include/usb/ulpi.h

Quoted 20 lines

Although it is a single patch, I felt that a cover letter will definetly not hurt here, also the patch version history is so big, so I decided to move it here.

The ULPI (UTMI Low Pin (count) Interface) PHYs are widely used on variety of boards. This requires a generic architecture independant implementation which can be reused and will eliminate the need for direct write of arbitrary values to the ULPI transciever. Although, the generic implementation can be reused on any architecture, the access to ULPI PHY must be done in a platform specific way. The platform specific way is in majority of case called a viewport. Also, the ULPI specification defines a hybrid aproach for managing the ULPI PHY. That is, the PHY must be managed through both the PHY registers and control lines.

The proposed patch provides a partial implementation of the ULPI specification, which should be enough for boot loader use cases, and a viewport implementation for Chipidea/ARC based controllers, which, AFAIK, are used on imx and tegra SoCs.

Quoted 5 lines

It is based on the Wolfgang's master branch (4 Dec 2012), compile tested and checkpatch clean.

What is still missing, IMO:
- documentation for the CONFIG_* macros (I can add it in a separate

patch)

Quoted 57 lines

- a way to make most of the initialization in one ulpi_init() call
- viewport extension to be able to implement resume, reset and disabling the serial mode

The change log:
Changes for v2:
- make code EHCI-independent
- use udelay() in waiting loop
- mark static functions as static
- naming changes
Changes for v3:
- merge with patch ulpi: add generic ULPI support header file
- rewrite ULPI interface in more functionality-oriented way
Changes for v4:
- add error-checking
- add waiting for completion into ulpi_reset() function
Changes for v5:
- CodingStyle changes
- add comments
- simplify implemenation of the ULPI interface functions
Changes for v6:
- cleanup function ulpi_drive_vbus()
Changes for v7:
- ulpi-viewport.c:
- reorder bit definitions
- split ulpi_request() to two functions
- reuse ulpi_wakeup() from ulpi_request()
to remove duplicated calls from ulpi_{read|write}()
- inline ulpi_*_mask as it is simple and used only once
- ulpi.c:
- move several defines into c file
- rework all the functions to propagate error values
- move function description comments into ulpi.h along with declarations
- check arguments validity (as suggested by Simon)
- fix cases when using the *_set register, bits cannot be cleared
- shorten several arguments names (e.g. ulpi_set_vbus())
- add ability to disable VBUS
- clean up ulpi_set_pd()
- add ability to enter the serial mode
- add verbosity in error cases
- remove ulpi_resume() as it were wrong and must be implemented in a viewport specific way
- rework ulpi_reset() as it must be implemented in a viewport specific way, but provide kind of generic implementation which should work in most of the cases
- ulpi.h:
- add default timeout value
- remove unused defines
- move several defines inside c files
- add description for each function
- move the API declaration to the top of the header file

Jana Rapava (1):
USB: Add generic ULPI layer and a viewport

Quoted 11 lines

 Makefile                         |    1 +
 drivers/usb/ulpi/Makefile        |   44 ++++++
 drivers/usb/ulpi/ulpi-viewport.c |  118 +++++++++++++++
 drivers/usb/ulpi/ulpi.c          |  227 +++++++++++++++++++++++++++++
 include/usb/ulpi.h               |  298
++++++++++++++++++++++++++++++++++++++ 5 files changed, 688 insertions(+),
0 deletions(-)
 create mode 100644 drivers/usb/ulpi/Makefile
 create mode 100644 drivers/usb/ulpi/ulpi-viewport.c
 create mode 100644 drivers/usb/ulpi/ulpi.c
 create mode 100644 include/usb/ulpi.h

Igor, please add Cc annotations to this patch too.

Hi Marek,

Quoted 95 lines · Marek Vasut

Quoted 20 lines

Although it is a single patch, I felt that a cover letter will definetly not hurt here, also the patch version history is so big, so I decided to move it here.

The ULPI (UTMI Low Pin (count) Interface) PHYs are widely used on variety of boards. This requires a generic architecture independant implementation which can be reused and will eliminate the need for direct write of arbitrary values to the ULPI transciever. Although, the generic implementation can be reused on any architecture, the access to ULPI PHY must be done in a platform specific way. The platform specific way is in majority of case called a viewport. Also, the ULPI specification defines a hybrid aproach for managing the ULPI PHY. That is, the PHY must be managed through both the PHY registers and control lines.

The proposed patch provides a partial implementation of the ULPI specification, which should be enough for boot loader use cases, and a viewport implementation for Chipidea/ARC based controllers, which, AFAIK, are used on imx and tegra SoCs.

Quoted 5 lines

It is based on the Wolfgang's master branch (4 Dec 2012), compile tested and checkpatch clean.

What is still missing, IMO:
- documentation for the CONFIG_* macros (I can add it in a separate

patch)

Quoted 68 lines

- a way to make most of the initialization in one ulpi_init() call
- viewport extension to be able to implement resume, reset and disabling the serial mode

The change log:
Changes for v2:
- make code EHCI-independent
- use udelay() in waiting loop
- mark static functions as static
- naming changes
Changes for v3:
- merge with patch ulpi: add generic ULPI support header file
- rewrite ULPI interface in more functionality-oriented way
Changes for v4:
- add error-checking
- add waiting for completion into ulpi_reset() function
Changes for v5:
- CodingStyle changes
- add comments
- simplify implemenation of the ULPI interface functions
Changes for v6:
- cleanup function ulpi_drive_vbus()
Changes for v7:
- ulpi-viewport.c:
- reorder bit definitions
- split ulpi_request() to two functions
- reuse ulpi_wakeup() from ulpi_request()
to remove duplicated calls from ulpi_{read|write}()
- inline ulpi_*_mask as it is simple and used only once
- ulpi.c:
- move several defines into c file
- rework all the functions to propagate error values
- move function description comments into ulpi.h along with declarations
- check arguments validity (as suggested by Simon)
- fix cases when using the *_set register, bits cannot be cleared
- shorten several arguments names (e.g. ulpi_set_vbus())
- add ability to disable VBUS
- clean up ulpi_set_pd()
- add ability to enter the serial mode
- add verbosity in error cases
- remove ulpi_resume() as it were wrong and must be implemented in a viewport specific way
- rework ulpi_reset() as it must be implemented in a viewport specific way, but provide kind of generic implementation which should work in most of the cases
- ulpi.h:
- add default timeout value
- remove unused defines
- move several defines inside c files
- add description for each function
- move the API declaration to the top of the header file

Jana Rapava (1):
USB: Add generic ULPI layer and a viewport

 Makefile                         |    1 +
 drivers/usb/ulpi/Makefile        |   44 ++++++
 drivers/usb/ulpi/ulpi-viewport.c |  118 +++++++++++++++
 drivers/usb/ulpi/ulpi.c          |  227 +++++++++++++++++++++++++++++
 include/usb/ulpi.h               |  298
++++++++++++++++++++++++++++++++++++++ 5 files changed, 688 insertions(+),
0 deletions(-)
 create mode 100644 drivers/usb/ulpi/Makefile
 create mode 100644 drivers/usb/ulpi/ulpi-viewport.c
 create mode 100644 drivers/usb/ulpi/ulpi.c
 create mode 100644 include/usb/ulpi.h

Quoted 1 line

Igor, please add Cc annotations to this patch too.

Well, I indeed forgot this, but I've send a forward of this email to everybody supposed to be in Cc. Adding Cc once again...

Quoted 120 lines

Hi Marek,

Quoted 117 lines · Marek Vasut

Quoted 20 lines

Although it is a single patch, I felt that a cover letter will definetly not hurt here, also the patch version history is so big, so I decided to move it here.

The ULPI (UTMI Low Pin (count) Interface) PHYs are widely used on variety of boards. This requires a generic architecture independant implementation which can be reused and will eliminate the need for direct write of arbitrary values to the ULPI transciever. Although, the generic implementation can be reused on any architecture, the access to ULPI PHY must be done in a platform specific way. The platform specific way is in majority of case called a viewport. Also, the ULPI specification defines a hybrid aproach for managing the ULPI PHY. That is, the PHY must be managed through both the PHY registers and control lines.

The proposed patch provides a partial implementation of the ULPI specification, which should be enough for boot loader use cases, and a viewport implementation for Chipidea/ARC based controllers, which, AFAIK, are used on imx and tegra SoCs.

Quoted 5 lines

It is based on the Wolfgang's master branch (4 Dec 2012), compile tested and checkpatch clean.

What is still missing, IMO:
- documentation for the CONFIG_* macros (I can add it in a separate

patch)

Quoted 88 lines

- a way to make most of the initialization in one ulpi_init() call
- viewport extension to be able to implement resume,

reset and disabling the serial mode

The change log:

Changes for v2:
- make code EHCI-independent
- use udelay() in waiting loop
- mark static functions as static
- naming changes

Changes for v3:
- merge with patch ulpi: add generic ULPI support header file
- rewrite ULPI interface in more functionality-oriented way

Changes for v4:
- add error-checking
- add waiting for completion into ulpi_reset() function

Changes for v5:
- CodingStyle changes
- add comments
- simplify implemenation of the ULPI interface functions

Changes for v6:
- cleanup function ulpi_drive_vbus()

Changes for v7:
- ulpi-viewport.c:
- reorder bit definitions
- split ulpi_request() to two functions
- reuse ulpi_wakeup() from ulpi_request()

to remove duplicated calls from ulpi_{read|write}()

- inline ulpi_*_mask as it is simple and used only once

- ulpi.c:
- move several defines into c file
- rework all the functions to propagate error values
- move function description comments into ulpi.h

along with declarations

- check arguments validity (as suggested by Simon)
- fix cases when using the *_set register,

bits cannot be cleared

- shorten several arguments names (e.g. ulpi_set_vbus())
- add ability to disable VBUS
- clean up ulpi_set_pd()
- add ability to enter the serial mode
- add verbosity in error cases
- remove ulpi_resume() as it were wrong and

must be implemented in a viewport specific way

- rework ulpi_reset() as it must be implemented in a

viewport specific way, but provide kind of generic implementation which should work in most of the cases

- ulpi.h:
- add default timeout value
- remove unused defines
- move several defines inside c files
- add description for each function
- move the API declaration to the top of the header file

Jana Rapava (1):
USB: Add generic ULPI layer and a viewport

 Makefile                         |    1 +
 drivers/usb/ulpi/Makefile        |   44 ++++++
 drivers/usb/ulpi/ulpi-viewport.c |  118 +++++++++++++++
 drivers/usb/ulpi/ulpi.c          |  227 +++++++++++++++++++++++++++++
 include/usb/ulpi.h               |  298

++++++++++++++++++++++++++++++++++++++ 5 files changed, 688 insertions(+), 0 deletions(-)

 create mode 100644 drivers/usb/ulpi/Makefile
 create mode 100644 drivers/usb/ulpi/ulpi-viewport.c
 create mode 100644 drivers/usb/ulpi/ulpi.c
 create mode 100644 include/usb/ulpi.h

Quoted 4 lines

Quoted 1 line

Igor, please add Cc annotations to this patch too.

Well, I indeed forgot this, but I've send a forward of this email to everybody supposed to be in Cc. Adding Cc once again...

That's interesting...
I've added much more people to Cc, than those listed currently... It looks like the bounce from the list "eats" the Cc partially...