[U-Boot] [PATCH 0/3] Tegra 2 USB ULPI series

[U-Boot] [PATCH 0/3] Tegra 2 USB ULPI series

[Lucas Stach]

With this series we are able to initialize USB controllers
using an external ULPI phy AKA USB2 on Tegra 2 devices.

This was tested to work on a Toradex Colibri T20 board,
where USB2 is used to access the ASIX ethernet chipset.
Testing was done with "tegra20: usb: rework set_host_mode"
applied. I did not spot any regressions on the UTMI ports.

Patchset is based on top of u-boot-tegra/next

Lucas Stach (3):
  tegra20: complete periph_id enum
  tegra20: add clock_set_pllout function
  tegra20: add USB ULPI init code

 arch/arm/cpu/armv7/tegra20/usb.c            | 131 +++++++++++++++++++++++++---
 arch/arm/cpu/tegra20-common/clock.c         |  26 ++++++
 arch/arm/cpu/tegra20-common/warmboot_avp.c  |   2 +-
 arch/arm/include/asm/arch-tegra20/clk_rst.h |  11 ++-
 arch/arm/include/asm/arch-tegra20/clock.h   |  25 ++++++
 arch/arm/include/asm/arch-tegra20/usb.h     |  29 ++++--
 6 Dateien ge?ndert, 206 Zeilen hinzugef?gt(+), 18 Zeilen entfernt(-)

-- 
1.7.11.2

[U-Boot] [PATCH 1/3] tegra20: complete periph_id enum

[Lucas Stach]

Most Tegra boards output the ULPI reference clock on pad DEV2.

Complete the periph_id enum so that we are able to enable this
clock output circuit.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 arch/arm/cpu/tegra20-common/clock.c       | 1 +
 arch/arm/include/asm/arch-tegra20/clock.h | 6 ++++++
 2 Dateien ge?ndert, 7 Zeilen hinzugef?gt(+)

diff --git a/arch/arm/cpu/tegra20-common/clock.c b/arch/arm/cpu/tegra20-common/clock.c
index 2403874..d9bb851 100644
--- a/arch/arm/cpu/tegra20-common/clock.c
+++ b/arch/arm/cpu/tegra20-common/clock.c
@@ -502,6 +502,7 @@ static int clock_periph_id_isvalid(enum periph_id id)
 		case PERIPH_ID_RESERVED81:
 		case PERIPH_ID_RESERVED82:
 		case PERIPH_ID_RESERVED83:
+		case PERIPH_ID_RESERVED91:
 			printf("Peripheral id %d is reserved\n", id);
 			break;
 		default:
diff --git a/arch/arm/include/asm/arch-tegra20/clock.h b/arch/arm/include/asm/arch-tegra20/clock.h
index ff83bbf..20db9e6 100644
--- a/arch/arm/include/asm/arch-tegra20/clock.h
+++ b/arch/arm/include/asm/arch-tegra20/clock.h
@@ -175,6 +175,12 @@ enum periph_id {
 
 	/* 88 */
 	PERIPH_ID_CRAM2,
+	PERIPH_ID_SYNC_CLK_DOUBLER,
+	PERIPH_ID_CLK_M_DOUBLER,
+	PERIPH_ID_RESERVED91,
+	PERIPH_ID_SUS_OUT,
+	PERIPH_ID_DEV2_OUT,
+	PERIPH_ID_DEV1_OUT,
 
 	PERIPH_ID_COUNT,
 	PERIPH_ID_NONE = -1,
-- 
1.7.11.2

[U-Boot] [PATCH 1/3] tegra20: complete periph_id enum

[Stephen Warren]

On 08/19/2012 10:08 AM, Lucas Stach wrote:
> Most Tegra boards output the ULPI reference clock on pad DEV2.
> 
> Complete the periph_id enum so that we are able to enable this
> clock output circuit.

Acked-by: Stephen Warren <swarren@wwwdotorg.org>

[U-Boot] [PATCH 1/3] tegra20: complete periph_id enum

[Simon Glass]

On Sun, Aug 19, 2012 at 9:08 AM, Lucas Stach <dev@lynxeye.de> wrote:
> Most Tegra boards output the ULPI reference clock on pad DEV2.
>
> Complete the periph_id enum so that we are able to enable this
> clock output circuit.
>
> Signed-off-by: Lucas Stach <dev@lynxeye.de>

Acked-by: Simon Glass <sjg@chromium.org>

> ---
>  arch/arm/cpu/tegra20-common/clock.c       | 1 +
>  arch/arm/include/asm/arch-tegra20/clock.h | 6 ++++++
>  2 Dateien ge?ndert, 7 Zeilen hinzugef?gt(+)
>
> diff --git a/arch/arm/cpu/tegra20-common/clock.c b/arch/arm/cpu/tegra20-common/clock.c
> index 2403874..d9bb851 100644
> --- a/arch/arm/cpu/tegra20-common/clock.c
> +++ b/arch/arm/cpu/tegra20-common/clock.c
> @@ -502,6 +502,7 @@ static int clock_periph_id_isvalid(enum periph_id id)
>                 case PERIPH_ID_RESERVED81:
>                 case PERIPH_ID_RESERVED82:
>                 case PERIPH_ID_RESERVED83:
> +               case PERIPH_ID_RESERVED91:
>                         printf("Peripheral id %d is reserved\n", id);
>                         break;
>                 default:
> diff --git a/arch/arm/include/asm/arch-tegra20/clock.h b/arch/arm/include/asm/arch-tegra20/clock.h
> index ff83bbf..20db9e6 100644
> --- a/arch/arm/include/asm/arch-tegra20/clock.h
> +++ b/arch/arm/include/asm/arch-tegra20/clock.h
> @@ -175,6 +175,12 @@ enum periph_id {
>
>         /* 88 */
>         PERIPH_ID_CRAM2,
> +       PERIPH_ID_SYNC_CLK_DOUBLER,
> +       PERIPH_ID_CLK_M_DOUBLER,
> +       PERIPH_ID_RESERVED91,
> +       PERIPH_ID_SUS_OUT,
> +       PERIPH_ID_DEV2_OUT,
> +       PERIPH_ID_DEV1_OUT,
>
>         PERIPH_ID_COUNT,
>         PERIPH_ID_NONE = -1,
> --
> 1.7.11.2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/3] tegra20: add clock_set_pllout function

[Lucas Stach]

Common practice on Tegra 2 boards is to use the pllp_out4 FO
to generate the ULPI reference clock. For this to work we have
to override the default hardware generated output divider.

This function adds a clean way to do so.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 arch/arm/cpu/tegra20-common/clock.c         | 25 +++++++++++++++++++++++++
 arch/arm/cpu/tegra20-common/warmboot_avp.c  |  2 +-
 arch/arm/include/asm/arch-tegra20/clk_rst.h | 11 +++++++++--
 arch/arm/include/asm/arch-tegra20/clock.h   | 19 +++++++++++++++++++
 4 Dateien ge?ndert, 54 Zeilen hinzugef?gt(+), 3 Zeilen entfernt(-)

diff --git a/arch/arm/cpu/tegra20-common/clock.c b/arch/arm/cpu/tegra20-common/clock.c
index d9bb851..068b552 100644
--- a/arch/arm/cpu/tegra20-common/clock.c
+++ b/arch/arm/cpu/tegra20-common/clock.c
@@ -604,6 +604,31 @@ unsigned long clock_get_periph_rate(enum periph_id periph_id,
 		(readl(reg) & OUT_CLK_DIVISOR_MASK) >> OUT_CLK_DIVISOR_SHIFT);
 }
 
+int clock_set_pllout(enum clock_id clkid, enum pll_out_id pllout, unsigned rate)
+{
+	struct clk_pll *pll = get_pll(clkid);
+	int data = 0, div = 0, offset = 0;
+
+	if (!clock_id_is_pll(clkid))
+		return -1;
+
+	div = clk_get_divider(8, pll_rate[clkid], rate);
+
+	if (div < 0)
+		return -1;
+
+	/* out2 and out4 are in the high part of the register */
+	if (pllout == PLL_OUT2 || pllout == PLL_OUT4)
+		offset = 16;
+
+	data = (div << PLL_OUT_RATIO_SHIFT) |
+	       PLL_OUT_OVRRIDE | PLL_OUT_CLKEN | PLL_OUT_RSTN;
+	clrsetbits_le32(&pll->pll_out[pllout >> 1],
+	                PLL_OUT_RATIO_MASK << offset, data << offset);
+
+	return 0;
+}
+
 /**
  * Find the best available 7.1 format divisor given a parent clock rate and
  * required child clock rate. This function assumes that a second-stage
diff --git a/arch/arm/cpu/tegra20-common/warmboot_avp.c b/arch/arm/cpu/tegra20-common/warmboot_avp.c
index cd01908..f6c71cb 100644
--- a/arch/arm/cpu/tegra20-common/warmboot_avp.c
+++ b/arch/arm/cpu/tegra20-common/warmboot_avp.c
@@ -214,7 +214,7 @@ void wb_start(void)
 
 	reg = PLLM_OUT1_RSTN_RESET_DISABLE | PLLM_OUT1_CLKEN_ENABLE |
 	      PLLM_OUT1_RATIO_VAL_8;
-	writel(reg, &clkrst->crc_pll[CLOCK_ID_MEMORY].pll_out);
+	writel(reg, &clkrst->crc_pll[CLOCK_ID_MEMORY].pll_out[0]);
 
 	reg = SCLK_SWAKE_FIQ_SRC_PLLM_OUT1 | SCLK_SWAKE_IRQ_SRC_PLLM_OUT1 |
 	      SCLK_SWAKE_RUN_SRC_PLLM_OUT1 | SCLK_SWAKE_IDLE_SRC_PLLM_OUT1 |
diff --git a/arch/arm/include/asm/arch-tegra20/clk_rst.h b/arch/arm/include/asm/arch-tegra20/clk_rst.h
index 8c3be91..7b548c2 100644
--- a/arch/arm/include/asm/arch-tegra20/clk_rst.h
+++ b/arch/arm/include/asm/arch-tegra20/clk_rst.h
@@ -27,8 +27,7 @@
 /* PLL registers - there are several PLLs in the clock controller */
 struct clk_pll {
 	uint pll_base;		/* the control register */
-	uint pll_out;		/* output control */
-	uint reserved;
+	uint pll_out[2];	/* output control */
 	uint pll_misc;		/* other misc things */
 };
 
@@ -112,6 +111,14 @@ struct clk_rst_ctlr {
 #define PLL_DIVM_SHIFT		0
 #define PLL_DIVM_MASK		(0x1f << PLL_DIVM_SHIFT)
 
+/* CLK_RST_CONTROLLER_PLLx_OUTx_0 */
+#define PLL_OUT_RSTN		(1 << 0)
+#define PLL_OUT_CLKEN		(1 << 1)
+#define PLL_OUT_OVRRIDE		(1 << 2)
+
+#define PLL_OUT_RATIO_SHIFT	8
+#define PLL_OUT_RATIO_MASK	(0xffU << PLL_OUT_RATIO_SHIFT)
+
 /* CLK_RST_CONTROLLER_PLLx_MISC_0 */
 #define PLL_CPCON_SHIFT		8
 #define PLL_CPCON_MASK		(15U << PLL_CPCON_SHIFT)
diff --git a/arch/arm/include/asm/arch-tegra20/clock.h b/arch/arm/include/asm/arch-tegra20/clock.h
index 20db9e6..dfef51e 100644
--- a/arch/arm/include/asm/arch-tegra20/clock.h
+++ b/arch/arm/include/asm/arch-tegra20/clock.h
@@ -186,6 +186,13 @@ enum periph_id {
 	PERIPH_ID_NONE = -1,
 };
 
+enum pll_out_id {
+	PLL_OUT1,
+	PLL_OUT2,
+	PLL_OUT3,
+	PLL_OUT4
+};
+
 /* Converts a clock number to a clock register: 0=L, 1=H, 2=U */
 #define PERIPH_REG(id) ((id) >> 5)
 
@@ -218,6 +225,18 @@ unsigned long clock_start_pll(enum clock_id id, u32 divm, u32 divn,
 		u32 divp, u32 cpcon, u32 lfcon);
 
 /**
+ * Set PLL output frequency
+ *
+ * @param clkid		clock id
+ * @param pllout	pll output id (
+ * @param rate		desired output rate
+ *
+ * @return 0 if ok, -1 on error (invalid clock id or no suitable divider)
+ */
+int clock_set_pllout(enum clock_id clkid, enum pll_out_id pllout,
+                     unsigned rate);
+
+/**
  * Read low-level parameters of a PLL.
  *
  * @param id	clock id to read (note: USB is not supported)
-- 
1.7.11.2

[U-Boot] [PATCH 2/3] tegra20: add clock_set_pllout function

[Stephen Warren]

On 08/19/2012 10:08 AM, Lucas Stach wrote:
> Common practice on Tegra 2 boards is to use the pllp_out4 FO
> to generate the ULPI reference clock. For this to work we have
> to override the default hardware generated output divider.
> 
> This function adds a clean way to do so.

> diff --git a/arch/arm/cpu/tegra20-common/clock.c b/arch/arm/cpu/tegra20-common/clock.c

> +int clock_set_pllout(enum clock_id clkid, enum pll_out_id pllout, unsigned rate)
> +{
> +	struct clk_pll *pll = get_pll(clkid);
> +	int data = 0, div = 0, offset = 0;
> +
> +	if (!clock_id_is_pll(clkid))
> +		return -1;

If you're going to error-check; you may as well limit the range of
pllout too; not all PLLs have 4 outputs.

Other than that, I think this seems OK.

[U-Boot] [PATCH 3/3] tegra20: add USB ULPI init code

[Lucas Stach]

This adds the required code to set up a ULPI USB port. It is
mostly a port of the Linux ULPI setup code with some tweaks
added for more correctness, discovered along the way of
debugging this.

To use this both CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
have to be set in the board configuration file.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 arch/arm/cpu/armv7/tegra20/usb.c        | 131 +++++++++++++++++++++++++++++---
 arch/arm/include/asm/arch-tegra20/usb.h |  29 +++++--
 2 Dateien ge?ndert, 145 Zeilen hinzugef?gt(+), 15 Zeilen entfernt(-)

diff --git a/arch/arm/cpu/armv7/tegra20/usb.c b/arch/arm/cpu/armv7/tegra20/usb.c
index 77966e5..2ae1244 100644
--- a/arch/arm/cpu/armv7/tegra20/usb.c
+++ b/arch/arm/cpu/armv7/tegra20/usb.c
@@ -32,9 +32,17 @@
 #include <asm/arch/sys_proto.h>
 #include <asm/arch/uart.h>
 #include <asm/arch/usb.h>
+#include <usb/ulpi.h>
 #include <libfdt.h>
 #include <fdtdec.h>
 
+#ifdef CONFIG_USB_ULPI
+	#ifndef CONFIG_USB_ULPI_VIEWPORT
+	#error "To use CONFIG_USB_ULPI on Tegra Boards you have to also \
+	       define CONFIG_USB_ULPI_VIEWPORT"
+	#endif
+#endif
+
 enum {
 	USB_PORTS_MAX	= 4,			/* Maximum ports we allow */
 };
@@ -68,11 +76,13 @@ enum dr_mode {
 struct fdt_usb {
 	struct usb_ctlr *reg;	/* address of registers in physical memory */
 	unsigned utmi:1;	/* 1 if port has external tranceiver, else 0 */
+	unsigned ulpi:1;	/* 1 if port has external ULPI transceiver */
 	unsigned enabled:1;	/* 1 to enable, 0 to disable */
 	unsigned has_legacy_mode:1; /* 1 if this port has legacy mode */
 	enum dr_mode dr_mode;	/* dual role mode */
 	enum periph_id periph_id;/* peripheral id */
 	struct fdt_gpio_state vbus_gpio;	/* GPIO for vbus enable */
+	struct fdt_gpio_state phy_reset_gpio; /* GPIO to reset ULPI phy */
 };
 
 static struct fdt_usb port[USB_PORTS_MAX];	/* List of valid USB ports */
@@ -187,8 +197,8 @@ static void usbf_reset_controller(struct fdt_usb *config,
 	 */
 }
 
-/* set up the USB controller with the parameters provided */
-static int init_usb_controller(struct fdt_usb *config,
+/* set up the UTMI USB controller with the parameters provided */
+static int init_utmi_usb_controller(struct fdt_usb *config,
 				struct usb_ctlr *usbctlr, const u32 timing[])
 {
 	u32 val;
@@ -300,6 +310,83 @@ static int init_usb_controller(struct fdt_usb *config,
 	return 0;
 }
 
+#ifdef CONFIG_USB_ULPI
+/* set up the ULPI USB controller with the parameters provided */
+static int init_ulpi_usb_controller(struct fdt_usb *config,
+                                    struct usb_ctlr *usbctlr)
+{
+	u32 val;
+	int loop_count;
+	struct ulpi_regs *ulpi_reg = (struct ulpi_regs *)0;
+	struct ulpi_viewport ulpi_vp;
+
+	/* reset ULPI phy */
+	if (fdt_gpio_isvalid(&config->phy_reset_gpio)) {
+		fdtdec_setup_gpio(&config->phy_reset_gpio);
+		gpio_direction_output(config->phy_reset_gpio.gpio, 0);
+		mdelay(5);
+		gpio_set_value(config->phy_reset_gpio.gpio, 1);
+	}
+
+	/* Reset the usb controller */
+	clock_enable(config->periph_id);
+	usbf_reset_controller(config, usbctlr);
+
+	/* enable pinmux bypass */
+	setbits_le32(&usbctlr->ulpi_timing_ctrl_0,
+	             ULPI_CLKOUT_PINMUX_BYP | ULPI_OUTPUT_PINMUX_BYP);
+
+	/* Select ULPI parallel interface */
+	clrsetbits_le32(&usbctlr->port_sc1, PTS_MASK, PTS_ULPI << PTS_SHIFT);
+
+	/* enable ULPI transceiver */
+	setbits_le32(&usbctlr->susp_ctrl, ULPI_PHY_ENB);
+
+	/* configure ULPI transceiver timings */
+	val = 0;
+	writel(val, &usbctlr->ulpi_timing_ctrl_1);
+
+	val |= ULPI_DATA_TRIMMER_SEL(4);
+	val |= ULPI_STPDIRNXT_TRIMMER_SEL(4);
+	val |= ULPI_DIR_TRIMMER_SEL(4);
+	writel(val, &usbctlr->ulpi_timing_ctrl_1);
+	udelay(10);
+
+	val |= ULPI_DATA_TRIMMER_LOAD;
+	val |= ULPI_STPDIRNXT_TRIMMER_LOAD;
+	val |= ULPI_DIR_TRIMMER_LOAD;
+	writel(val, &usbctlr->ulpi_timing_ctrl_1);
+
+	/* set up phy for host operation with external vbus supply */
+	ulpi_vp.port_num = 0;
+	ulpi_vp.viewport_addr = (u32)&usbctlr->ulpi_viewport;
+
+	if (ulpi_init(&ulpi_vp)) {
+		debug("Tegra ULPI viewport init failed\n");
+		return -1;
+	}
+
+	ulpi_write(&ulpi_vp, &ulpi_reg->iface_ctrl_set, ULPI_IFACE_PASSTHRU);
+	ulpi_write(&ulpi_vp, &ulpi_reg->otg_ctrl_set, ULPI_OTG_EXTVBUSIND);
+
+	/* enable wakeup events */
+	setbits_le32(&usbctlr->port_sc1, WKCN | WKDS | WKOC);
+
+	setbits_le32(&usbctlr->susp_ctrl, USB_SUSP_CLR);
+	/* Wait for the phy clock to become valid in 100 ms */
+	for (loop_count = 100000; loop_count != 0; loop_count--) {
+		if (readl(&usbctlr->susp_ctrl) & USB_PHY_CLK_VALID)
+			break;
+		udelay(1);
+	}
+	if (!loop_count)
+		return -1;
+	clrbits_le32(&usbctlr->susp_ctrl, USB_SUSP_CLR);
+
+	return 0;
+}
+#endif
+
 static void power_up_port(struct usb_ctlr *usbctlr)
 {
 	/* Deassert power down state */
@@ -331,11 +418,13 @@ static int add_port(struct fdt_usb *config, const u32 timing[])
 		      USB_PORTS_MAX);
 		return -1;
 	}
-	if (init_usb_controller(config, usbctlr, timing)) {
-		debug("tegrausb: Cannot init port\n");
-		return -1;
-	}
+
 	if (config->utmi) {
+		if (init_utmi_usb_controller(config, usbctlr, timing)) {
+			debug("tegrausb: Cannot init port\n");
+			return -1;
+		}
+
 		/* Disable ICUSB FS/LS transceiver */
 		clrbits_le32(&usbctlr->icusb_ctrl, IC_ENB1);
 
@@ -345,6 +434,24 @@ static int add_port(struct fdt_usb *config, const u32 timing[])
 		clrbits_le32(&usbctlr->port_sc1, STS);
 		power_up_port(usbctlr);
 	}
+
+	if (config->ulpi) {
+#ifdef CONFIG_USB_ULPI
+		/* set up 24MHz ULPI reference clock on pllp_out4 */
+		clock_enable(PERIPH_ID_DEV2_OUT);
+		clock_set_pllout(CLOCK_ID_PERIPH, PLL_OUT4, 24000000);
+
+		if (init_ulpi_usb_controller(config, usbctlr)) {
+			debug("tegrausb: Cannot init port\n");
+			return -1;
+		}
+#else
+		debug("No code to set up ULPI controller, please enable"
+		      "CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT");
+		return -1;
+#endif
+	}
+
 	port[port_count++] = *config;
 
 	return 0;
@@ -411,6 +518,7 @@ static int fdt_decode_usb(const void *blob, int node,
 
 	phy = fdt_getprop(blob, node, "phy_type", NULL);
 	config->utmi = phy && 0 == strcmp("utmi", phy);
+	config->ulpi = phy && 0 == strcmp("ulpi", phy);
 	config->enabled = fdtdec_get_is_enabled(blob, node);
 	config->has_legacy_mode = fdtdec_get_bool(blob, node,
 						  "nvidia,has-legacy-mode");
@@ -420,10 +528,13 @@ static int fdt_decode_usb(const void *blob, int node,
 		return -FDT_ERR_NOTFOUND;
 	}
 	fdtdec_decode_gpio(blob, node, "nvidia,vbus-gpio", &config->vbus_gpio);
-	debug("enabled=%d, legacy_mode=%d, utmi=%d, periph_id=%d, vbus=%d, "
-	      "dr_mode=%d\n", config->enabled, config->has_legacy_mode,
-	      config->utmi, config->periph_id, config->vbus_gpio.gpio,
-	      config->dr_mode);
+	fdtdec_decode_gpio(blob, node, "nvidia,phy-reset-gpio",
+	                   &config->phy_reset_gpio);
+	debug("enabled=%d, legacy_mode=%d, utmi=%d, ulpi=%d, periph_id=%d, "
+	      "vbus=%d, phy_reset=%d, dr_mode=%d\n",
+	      config->enabled, config->has_legacy_mode, config->utmi, config->ulpi,
+	      config->periph_id, config->vbus_gpio.gpio,
+	      config->phy_reset_gpio.gpio, config->dr_mode);
 
 	return 0;
 }
diff --git a/arch/arm/include/asm/arch-tegra20/usb.h b/arch/arm/include/asm/arch-tegra20/usb.h
index 638033b..bd89d66 100644
--- a/arch/arm/include/asm/arch-tegra20/usb.h
+++ b/arch/arm/include/asm/arch-tegra20/usb.h
@@ -100,10 +100,12 @@ struct usb_ctlr {
 
 	/* 0x410 */
 	uint usb1_legacy_ctrl;
-	uint reserved12[3];
+	uint reserved12[4];
 
-	/* 0x420 */
-	uint reserved13[56];
+	/* 0x424 */
+	uint ulpi_timing_ctrl_0;
+	uint ulpi_timing_ctrl_1;
+	uint reserved13[53];
 
 	/* 0x500 */
 	uint reserved14[64 * 3];
@@ -144,10 +146,24 @@ struct usb_ctlr {
 #define VBUS_SENSE_CTL_AB_SESS_VLD		2
 #define VBUS_SENSE_CTL_A_SESS_VLD		3
 
+/* USB2_IF_ULPI_TIMING_CTRL_0 */
+#define ULPI_OUTPUT_PINMUX_BYP			(1 << 10)
+#define ULPI_CLKOUT_PINMUX_BYP			(1 << 11)
+
+/* USB2_IF_ULPI_TIMING_CTRL_1 */
+#define ULPI_DATA_TRIMMER_LOAD			(1 << 0)
+#define ULPI_DATA_TRIMMER_SEL(x)		(((x) & 0x7) << 1)
+#define ULPI_STPDIRNXT_TRIMMER_LOAD		(1 << 16)
+#define ULPI_STPDIRNXT_TRIMMER_SEL(x)	(((x) & 0x7) << 17)
+#define ULPI_DIR_TRIMMER_LOAD			(1 << 24)
+#define ULPI_DIR_TRIMMER_SEL(x)			(((x) & 0x7) << 25)
+
 /* USBx_IF_USB_SUSP_CTRL_0 */
+#define ULPI_PHY_ENB				(1 << 13)
 #define UTMIP_PHY_ENB			        (1 << 12)
 #define UTMIP_RESET			        (1 << 11)
 #define USB_PHY_CLK_VALID			(1 << 7)
+#define USB_SUSP_CLR				(1 << 5)
 
 /* USBx_UTMIP_MISC_CFG1 */
 #define UTMIP_PLLU_STABLE_COUNT_SHIFT		6
@@ -203,12 +219,15 @@ struct usb_ctlr {
 /* SB2_CONTROLLER_2_USB2D_PORTSC1_0 */
 #define PTS_SHIFT				30
 #define PTS_MASK				(3U << PTS_SHIFT)
-#define PTS_UTMI	0
+#define PTS_UTMI		0
 #define PTS_RESERVED	1
-#define PTS_ULP		2
+#define PTS_ULPI		2
 #define PTS_ICUSB_SER	3
 
 #define STS					(1 << 29)
+#define WKOC				(1 << 22)
+#define WKDS				(1 << 21)
+#define WKCN				(1 << 20)
 
 /* USBx_UTMIP_XCVR_CFG0_0 */
 #define UTMIP_FORCE_PD_POWERDOWN		(1 << 14)
-- 
1.7.11.2

[U-Boot] [PATCH 3/3] tegra20: add USB ULPI init code

[Igor Grinberg]

Hi Lucas,

On 08/19/12 19:08, Lucas Stach wrote:
> This adds the required code to set up a ULPI USB port. It is
> mostly a port of the Linux ULPI setup code with some tweaks
> added for more correctness, discovered along the way of
> debugging this.

Can you share which tweaks for correctness are there?

> 
> To use this both CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
> have to be set in the board configuration file.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
>  arch/arm/cpu/armv7/tegra20/usb.c        | 131 +++++++++++++++++++++++++++++---
>  arch/arm/include/asm/arch-tegra20/usb.h |  29 +++++--
>  2 Dateien ge?ndert, 145 Zeilen hinzugef?gt(+), 15 Zeilen entfernt(-)
> 
> diff --git a/arch/arm/cpu/armv7/tegra20/usb.c b/arch/arm/cpu/armv7/tegra20/usb.c
> index 77966e5..2ae1244 100644
> --- a/arch/arm/cpu/armv7/tegra20/usb.c
> +++ b/arch/arm/cpu/armv7/tegra20/usb.c
> @@ -32,9 +32,17 @@
>  #include <asm/arch/sys_proto.h>
>  #include <asm/arch/uart.h>
>  #include <asm/arch/usb.h>
> +#include <usb/ulpi.h>
>  #include <libfdt.h>
>  #include <fdtdec.h>
>  
> +#ifdef CONFIG_USB_ULPI
> +	#ifndef CONFIG_USB_ULPI_VIEWPORT
> +	#error "To use CONFIG_USB_ULPI on Tegra Boards you have to also \
> +	       define CONFIG_USB_ULPI_VIEWPORT"

there is a mix of tabs and spaces in the above line, please make it only tabs

> +	#endif
> +#endif
> +
>  enum {
>  	USB_PORTS_MAX	= 4,			/* Maximum ports we allow */
>  };
> @@ -68,11 +76,13 @@ enum dr_mode {
>  struct fdt_usb {
>  	struct usb_ctlr *reg;	/* address of registers in physical memory */
>  	unsigned utmi:1;	/* 1 if port has external tranceiver, else 0 */
> +	unsigned ulpi:1;	/* 1 if port has external ULPI transceiver */
>  	unsigned enabled:1;	/* 1 to enable, 0 to disable */
>  	unsigned has_legacy_mode:1; /* 1 if this port has legacy mode */
>  	enum dr_mode dr_mode;	/* dual role mode */
>  	enum periph_id periph_id;/* peripheral id */
>  	struct fdt_gpio_state vbus_gpio;	/* GPIO for vbus enable */
> +	struct fdt_gpio_state phy_reset_gpio; /* GPIO to reset ULPI phy */
>  };
>  
>  static struct fdt_usb port[USB_PORTS_MAX];	/* List of valid USB ports */
> @@ -187,8 +197,8 @@ static void usbf_reset_controller(struct fdt_usb *config,
>  	 */
>  }
>  
> -/* set up the USB controller with the parameters provided */
> -static int init_usb_controller(struct fdt_usb *config,
> +/* set up the UTMI USB controller with the parameters provided */
> +static int init_utmi_usb_controller(struct fdt_usb *config,
>  				struct usb_ctlr *usbctlr, const u32 timing[])
>  {
>  	u32 val;
> @@ -300,6 +310,83 @@ static int init_usb_controller(struct fdt_usb *config,
>  	return 0;
>  }
>  
> +#ifdef CONFIG_USB_ULPI
> +/* set up the ULPI USB controller with the parameters provided */
> +static int init_ulpi_usb_controller(struct fdt_usb *config,
> +                                    struct usb_ctlr *usbctlr)
> +{
> +	u32 val;
> +	int loop_count;
> +	struct ulpi_regs *ulpi_reg = (struct ulpi_regs *)0;
> +	struct ulpi_viewport ulpi_vp;
> +
> +	/* reset ULPI phy */
> +	if (fdt_gpio_isvalid(&config->phy_reset_gpio)) {
> +		fdtdec_setup_gpio(&config->phy_reset_gpio);
> +		gpio_direction_output(config->phy_reset_gpio.gpio, 0);
> +		mdelay(5);
> +		gpio_set_value(config->phy_reset_gpio.gpio, 1);
> +	}
> +
> +	/* Reset the usb controller */
> +	clock_enable(config->periph_id);
> +	usbf_reset_controller(config, usbctlr);
> +
> +	/* enable pinmux bypass */
> +	setbits_le32(&usbctlr->ulpi_timing_ctrl_0,
> +	             ULPI_CLKOUT_PINMUX_BYP | ULPI_OUTPUT_PINMUX_BYP);
> +
> +	/* Select ULPI parallel interface */
> +	clrsetbits_le32(&usbctlr->port_sc1, PTS_MASK, PTS_ULPI << PTS_SHIFT);
> +
> +	/* enable ULPI transceiver */
> +	setbits_le32(&usbctlr->susp_ctrl, ULPI_PHY_ENB);
> +
> +	/* configure ULPI transceiver timings */
> +	val = 0;
> +	writel(val, &usbctlr->ulpi_timing_ctrl_1);
> +
> +	val |= ULPI_DATA_TRIMMER_SEL(4);
> +	val |= ULPI_STPDIRNXT_TRIMMER_SEL(4);
> +	val |= ULPI_DIR_TRIMMER_SEL(4);
> +	writel(val, &usbctlr->ulpi_timing_ctrl_1);
> +	udelay(10);
> +
> +	val |= ULPI_DATA_TRIMMER_LOAD;
> +	val |= ULPI_STPDIRNXT_TRIMMER_LOAD;
> +	val |= ULPI_DIR_TRIMMER_LOAD;
> +	writel(val, &usbctlr->ulpi_timing_ctrl_1);
> +
> +	/* set up phy for host operation with external vbus supply */
> +	ulpi_vp.port_num = 0;
> +	ulpi_vp.viewport_addr = (u32)&usbctlr->ulpi_viewport;
> +
> +	if (ulpi_init(&ulpi_vp)) {
> +		debug("Tegra ULPI viewport init failed\n");

This looks like an error, right?
So why hide it under debug?

> +		return -1;
> +	}
> +
> +	ulpi_write(&ulpi_vp, &ulpi_reg->iface_ctrl_set, ULPI_IFACE_PASSTHRU);

The implementation for the indicator setup is currently missing
from the generic framework.
Have you thought about adding it?

> +	ulpi_write(&ulpi_vp, &ulpi_reg->otg_ctrl_set, ULPI_OTG_EXTVBUSIND);

Can ulpi_set_vbus(&ulpi_vp, 1, 1, 1) be used here?

> +
> +	/* enable wakeup events */
> +	setbits_le32(&usbctlr->port_sc1, WKCN | WKDS | WKOC);
> +
> +	setbits_le32(&usbctlr->susp_ctrl, USB_SUSP_CLR);
> +	/* Wait for the phy clock to become valid in 100 ms */
> +	for (loop_count = 100000; loop_count != 0; loop_count--) {
> +		if (readl(&usbctlr->susp_ctrl) & USB_PHY_CLK_VALID)
> +			break;
> +		udelay(1);
> +	}
> +	if (!loop_count)
> +		return -1;
> +	clrbits_le32(&usbctlr->susp_ctrl, USB_SUSP_CLR);
> +
> +	return 0;
> +}
> +#endif
> +
>  static void power_up_port(struct usb_ctlr *usbctlr)
>  {
>  	/* Deassert power down state */
> @@ -331,11 +418,13 @@ static int add_port(struct fdt_usb *config, const u32 timing[])
>  		      USB_PORTS_MAX);
>  		return -1;
>  	}
> -	if (init_usb_controller(config, usbctlr, timing)) {
> -		debug("tegrausb: Cannot init port\n");
> -		return -1;
> -	}
> +
>  	if (config->utmi) {
> +		if (init_utmi_usb_controller(config, usbctlr, timing)) {
> +			debug("tegrausb: Cannot init port\n");

This also looks like an error...
So why debug()?

> +			return -1;
> +		}
> +
>  		/* Disable ICUSB FS/LS transceiver */
>  		clrbits_le32(&usbctlr->icusb_ctrl, IC_ENB1);
>  
> @@ -345,6 +434,24 @@ static int add_port(struct fdt_usb *config, const u32 timing[])
>  		clrbits_le32(&usbctlr->port_sc1, STS);
>  		power_up_port(usbctlr);
>  	}
> +
> +	if (config->ulpi) {
> +#ifdef CONFIG_USB_ULPI
> +		/* set up 24MHz ULPI reference clock on pllp_out4 */
> +		clock_enable(PERIPH_ID_DEV2_OUT);
> +		clock_set_pllout(CLOCK_ID_PERIPH, PLL_OUT4, 24000000);

Wouldn't it be clearer if:
1) you put the above inside the init_ulpi_usb_controller() function
2) Provide a !CONFIG_USB_ULPI implementation of the same function
   technically having only the code under #else below inside.

So then here instead of the whole #ifdef, you will have something like:
if (config->ulpi) {
	if (init_ulpi_usb_controller(config, usbctlr)) {
		printf("tegrausb: Cannot init port\n");
		return -1;
	}
}

> +
> +		if (init_ulpi_usb_controller(config, usbctlr)) {
> +			debug("tegrausb: Cannot init port\n");
> +			return -1;
> +		}
> +#else
> +		debug("No code to set up ULPI controller, please enable"
> +		      "CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT");
> +		return -1;
> +#endif
> +	}
> +
>  	port[port_count++] = *config;
>  
>  	return 0;

[...]



-- 
Regards,
Igor.

[U-Boot] [PATCH 3/3] tegra20: add USB ULPI init code

[Lucas Stach]

Hello Igor,

thanks for your review. Comments inline.

Am Montag, den 20.08.2012, 15:07 +0300 schrieb Igor Grinberg:
> Hi Lucas,
> 
> On 08/19/12 19:08, Lucas Stach wrote:
> > This adds the required code to set up a ULPI USB port. It is
> > mostly a port of the Linux ULPI setup code with some tweaks
> > added for more correctness, discovered along the way of
> > debugging this.
> 
> Can you share which tweaks for correctness are there?
> 
Linux code does not actually check if the phy clock comes up correctly,
it just waits 100ms and hopes it is there. Also we explicitly select the
ULPI interface, although it also works without this, as ULPI seems to be
the default state for USB2.

> > 
> > To use this both CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
> > have to be set in the board configuration file.
> > 
> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > ---
> >  arch/arm/cpu/armv7/tegra20/usb.c        | 131 +++++++++++++++++++++++++++++---
> >  arch/arm/include/asm/arch-tegra20/usb.h |  29 +++++--
> >  2 Dateien ge?ndert, 145 Zeilen hinzugef?gt(+), 15 Zeilen entfernt(-)
> > 
> > diff --git a/arch/arm/cpu/armv7/tegra20/usb.c b/arch/arm/cpu/armv7/tegra20/usb.c
> > index 77966e5..2ae1244 100644
> > --- a/arch/arm/cpu/armv7/tegra20/usb.c
> > +++ b/arch/arm/cpu/armv7/tegra20/usb.c
> > @@ -32,9 +32,17 @@
> >  #include <asm/arch/sys_proto.h>
> >  #include <asm/arch/uart.h>
> >  #include <asm/arch/usb.h>
> > +#include <usb/ulpi.h>
> >  #include <libfdt.h>
> >  #include <fdtdec.h>
> >  
> > +#ifdef CONFIG_USB_ULPI
> > +	#ifndef CONFIG_USB_ULPI_VIEWPORT
> > +	#error "To use CONFIG_USB_ULPI on Tegra Boards you have to also \
> > +	       define CONFIG_USB_ULPI_VIEWPORT"
> 
> there is a mix of tabs and spaces in the above line, please make it only tabs

This was actually intentional to align the two lines exactly.

> > +	#endif
> > +#endif
> > +
> >  enum {
> >  	USB_PORTS_MAX	= 4,			/* Maximum ports we allow */
> >  };
> > @@ -68,11 +76,13 @@ enum dr_mode {
> >  struct fdt_usb {
> >  	struct usb_ctlr *reg;	/* address of registers in physical memory */
> >  	unsigned utmi:1;	/* 1 if port has external tranceiver, else 0 */
> > +	unsigned ulpi:1;	/* 1 if port has external ULPI transceiver */
> >  	unsigned enabled:1;	/* 1 to enable, 0 to disable */
> >  	unsigned has_legacy_mode:1; /* 1 if this port has legacy mode */
> >  	enum dr_mode dr_mode;	/* dual role mode */
> >  	enum periph_id periph_id;/* peripheral id */
> >  	struct fdt_gpio_state vbus_gpio;	/* GPIO for vbus enable */
> > +	struct fdt_gpio_state phy_reset_gpio; /* GPIO to reset ULPI phy */
> >  };
> >  
> >  static struct fdt_usb port[USB_PORTS_MAX];	/* List of valid USB ports */
> > @@ -187,8 +197,8 @@ static void usbf_reset_controller(struct fdt_usb *config,
> >  	 */
> >  }
> >  
> > -/* set up the USB controller with the parameters provided */
> > -static int init_usb_controller(struct fdt_usb *config,
> > +/* set up the UTMI USB controller with the parameters provided */
> > +static int init_utmi_usb_controller(struct fdt_usb *config,
> >  				struct usb_ctlr *usbctlr, const u32 timing[])
> >  {
> >  	u32 val;
> > @@ -300,6 +310,83 @@ static int init_usb_controller(struct fdt_usb *config,
> >  	return 0;
> >  }
> >  
> > +#ifdef CONFIG_USB_ULPI
> > +/* set up the ULPI USB controller with the parameters provided */
> > +static int init_ulpi_usb_controller(struct fdt_usb *config,
> > +                                    struct usb_ctlr *usbctlr)
> > +{
> > +	u32 val;
> > +	int loop_count;
> > +	struct ulpi_regs *ulpi_reg = (struct ulpi_regs *)0;
> > +	struct ulpi_viewport ulpi_vp;
> > +
> > +	/* reset ULPI phy */
> > +	if (fdt_gpio_isvalid(&config->phy_reset_gpio)) {
> > +		fdtdec_setup_gpio(&config->phy_reset_gpio);
> > +		gpio_direction_output(config->phy_reset_gpio.gpio, 0);
> > +		mdelay(5);
> > +		gpio_set_value(config->phy_reset_gpio.gpio, 1);
> > +	}
> > +
> > +	/* Reset the usb controller */
> > +	clock_enable(config->periph_id);
> > +	usbf_reset_controller(config, usbctlr);
> > +
> > +	/* enable pinmux bypass */
> > +	setbits_le32(&usbctlr->ulpi_timing_ctrl_0,
> > +	             ULPI_CLKOUT_PINMUX_BYP | ULPI_OUTPUT_PINMUX_BYP);
> > +
> > +	/* Select ULPI parallel interface */
> > +	clrsetbits_le32(&usbctlr->port_sc1, PTS_MASK, PTS_ULPI << PTS_SHIFT);
> > +
> > +	/* enable ULPI transceiver */
> > +	setbits_le32(&usbctlr->susp_ctrl, ULPI_PHY_ENB);
> > +
> > +	/* configure ULPI transceiver timings */
> > +	val = 0;
> > +	writel(val, &usbctlr->ulpi_timing_ctrl_1);
> > +
> > +	val |= ULPI_DATA_TRIMMER_SEL(4);
> > +	val |= ULPI_STPDIRNXT_TRIMMER_SEL(4);
> > +	val |= ULPI_DIR_TRIMMER_SEL(4);
> > +	writel(val, &usbctlr->ulpi_timing_ctrl_1);
> > +	udelay(10);
> > +
> > +	val |= ULPI_DATA_TRIMMER_LOAD;
> > +	val |= ULPI_STPDIRNXT_TRIMMER_LOAD;
> > +	val |= ULPI_DIR_TRIMMER_LOAD;
> > +	writel(val, &usbctlr->ulpi_timing_ctrl_1);
> > +
> > +	/* set up phy for host operation with external vbus supply */
> > +	ulpi_vp.port_num = 0;
> > +	ulpi_vp.viewport_addr = (u32)&usbctlr->ulpi_viewport;
> > +
> > +	if (ulpi_init(&ulpi_vp)) {
> > +		debug("Tegra ULPI viewport init failed\n");
> 
> This looks like an error, right?
> So why hide it under debug?
> 
Yes, you're right. I'll fix that.

> > +		return -1;
> > +	}
> > +
> > +	ulpi_write(&ulpi_vp, &ulpi_reg->iface_ctrl_set, ULPI_IFACE_PASSTHRU);
> 
> The implementation for the indicator setup is currently missing
> from the generic framework.
> Have you thought about adding it?
> 
Yep, only looked into the framework provided functions after writing
this patch. I'll do a patch for this in the next series iteration.

> > +	ulpi_write(&ulpi_vp, &ulpi_reg->otg_ctrl_set, ULPI_OTG_EXTVBUSIND);
> 
> Can ulpi_set_vbus(&ulpi_vp, 1, 1, 1) be used here?
> 
Yes, I already changed this locally.

> > +
> > +	/* enable wakeup events */
> > +	setbits_le32(&usbctlr->port_sc1, WKCN | WKDS | WKOC);
> > +
> > +	setbits_le32(&usbctlr->susp_ctrl, USB_SUSP_CLR);
> > +	/* Wait for the phy clock to become valid in 100 ms */
> > +	for (loop_count = 100000; loop_count != 0; loop_count--) {
> > +		if (readl(&usbctlr->susp_ctrl) & USB_PHY_CLK_VALID)
> > +			break;
> > +		udelay(1);
> > +	}
> > +	if (!loop_count)
> > +		return -1;
> > +	clrbits_le32(&usbctlr->susp_ctrl, USB_SUSP_CLR);
> > +
> > +	return 0;
> > +}
> > +#endif
> > +
> >  static void power_up_port(struct usb_ctlr *usbctlr)
> >  {
> >  	/* Deassert power down state */
> > @@ -331,11 +418,13 @@ static int add_port(struct fdt_usb *config, const u32 timing[])
> >  		      USB_PORTS_MAX);
> >  		return -1;
> >  	}
> > -	if (init_usb_controller(config, usbctlr, timing)) {
> > -		debug("tegrausb: Cannot init port\n");
> > -		return -1;
> > -	}
> > +
> >  	if (config->utmi) {
> > +		if (init_utmi_usb_controller(config, usbctlr, timing)) {
> > +			debug("tegrausb: Cannot init port\n");
> 
> This also looks like an error...
> So why debug()?
> 
> > +			return -1;
> > +		}
> > +
> >  		/* Disable ICUSB FS/LS transceiver */
> >  		clrbits_le32(&usbctlr->icusb_ctrl, IC_ENB1);
> >  
> > @@ -345,6 +434,24 @@ static int add_port(struct fdt_usb *config, const u32 timing[])
> >  		clrbits_le32(&usbctlr->port_sc1, STS);
> >  		power_up_port(usbctlr);
> >  	}
> > +
> > +	if (config->ulpi) {
> > +#ifdef CONFIG_USB_ULPI
> > +		/* set up 24MHz ULPI reference clock on pllp_out4 */
> > +		clock_enable(PERIPH_ID_DEV2_OUT);
> > +		clock_set_pllout(CLOCK_ID_PERIPH, PLL_OUT4, 24000000);
> 
> Wouldn't it be clearer if:
> 1) you put the above inside the init_ulpi_usb_controller() function
> 2) Provide a !CONFIG_USB_ULPI implementation of the same function
>    technically having only the code under #else below inside.
> 
Actually I'm not really sure what to do about this. Although I've not
seen any Tegra boards with a other ULPI reference freq used, maybe we
should just move the clock setup into board code or add a device tree
entry to tell the ref frequency.

Stephen, Tom, any ideas?

> So then here instead of the whole #ifdef, you will have something like:
> if (config->ulpi) {
> 	if (init_ulpi_usb_controller(config, usbctlr)) {
> 		printf("tegrausb: Cannot init port\n");
> 		return -1;
> 	}
> }

> > +
> > +		if (init_ulpi_usb_controller(config, usbctlr)) {
> > +			debug("tegrausb: Cannot init port\n");
> > +			return -1;
> > +		}
> > +#else
> > +		debug("No code to set up ULPI controller, please enable"
> > +		      "CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT");
> > +		return -1;
> > +#endif
> > +	}
> > +
> >  	port[port_count++] = *config;
> >  
> >  	return 0;
> 
> [...]
> 
> 
> 

[U-Boot] [PATCH 3/3] tegra20: add USB ULPI init code

[Stephen Warren]

On 08/20/2012 06:41 AM, Lucas Stach wrote:
> Hello Igor,
> 
> thanks for your review. Comments inline.
> 
> Am Montag, den 20.08.2012, 15:07 +0300 schrieb Igor Grinberg:
>> Hi Lucas,
>>
>> On 08/19/12 19:08, Lucas Stach wrote:
>>> This adds the required code to set up a ULPI USB port. It is
>>> mostly a port of the Linux ULPI setup code with some tweaks
>>> added for more correctness, discovered along the way of
>>> debugging this.

>>>  	if (config->utmi) {
>>> +		if (init_utmi_usb_controller(config, usbctlr, timing)) {
>>> +			debug("tegrausb: Cannot init port\n");
>>
>> This also looks like an error...
>> So why debug()?
>>
>>> +			return -1;
>>> +		}
>>> +
>>>  		/* Disable ICUSB FS/LS transceiver */
>>>  		clrbits_le32(&usbctlr->icusb_ctrl, IC_ENB1);
>>>  
>>> @@ -345,6 +434,24 @@ static int add_port(struct fdt_usb *config, const u32 timing[])
>>>  		clrbits_le32(&usbctlr->port_sc1, STS);
>>>  		power_up_port(usbctlr);
>>>  	}
>>> +
>>> +	if (config->ulpi) {
>>> +#ifdef CONFIG_USB_ULPI
>>> +		/* set up 24MHz ULPI reference clock on pllp_out4 */
>>> +		clock_enable(PERIPH_ID_DEV2_OUT);
>>> +		clock_set_pllout(CLOCK_ID_PERIPH, PLL_OUT4, 24000000);
>>
>> Wouldn't it be clearer if:
>> 1) you put the above inside the init_ulpi_usb_controller() function
>> 2) Provide a !CONFIG_USB_ULPI implementation of the same function
>>    technically having only the code under #else below inside.
>>
> Actually I'm not really sure what to do about this. Although I've not
> seen any Tegra boards with a other ULPI reference freq used, maybe we
> should just move the clock setup into board code or add a device tree
> entry to tell the ref frequency.
> 
> Stephen, Tom, any ideas?

Moving all the initialization into init_utmi_usb_controller() and
init_ulpi_usb_controller() sounds reasonable to me.

I imagine that the reference frequency is somewhat driven by the
requirements of USB itself and/or the ULPI interface. I think it's fine
to just hard-code that in the USB driver for now; we can easily enhance
the driver to make it configurable from either DT or U-Boot config file
in the future if we need.

[U-Boot] [PATCH 3/3] tegra20: add USB ULPI init code

[Igor Grinberg]

On 08/20/12 21:27, Stephen Warren wrote:
> On 08/20/2012 06:41 AM, Lucas Stach wrote:
>> Hello Igor,
>>
>> thanks for your review. Comments inline.
>>
>> Am Montag, den 20.08.2012, 15:07 +0300 schrieb Igor Grinberg:
>>> Hi Lucas,
>>>
>>> On 08/19/12 19:08, Lucas Stach wrote:
>>>> This adds the required code to set up a ULPI USB port. It is
>>>> mostly a port of the Linux ULPI setup code with some tweaks
>>>> added for more correctness, discovered along the way of
>>>> debugging this.
> 
>>>>  	if (config->utmi) {
>>>> +		if (init_utmi_usb_controller(config, usbctlr, timing)) {
>>>> +			debug("tegrausb: Cannot init port\n");
>>>
>>> This also looks like an error...
>>> So why debug()?
>>>
>>>> +			return -1;
>>>> +		}
>>>> +
>>>>  		/* Disable ICUSB FS/LS transceiver */
>>>>  		clrbits_le32(&usbctlr->icusb_ctrl, IC_ENB1);
>>>>  
>>>> @@ -345,6 +434,24 @@ static int add_port(struct fdt_usb *config, const u32 timing[])
>>>>  		clrbits_le32(&usbctlr->port_sc1, STS);
>>>>  		power_up_port(usbctlr);
>>>>  	}
>>>> +
>>>> +	if (config->ulpi) {
>>>> +#ifdef CONFIG_USB_ULPI
>>>> +		/* set up 24MHz ULPI reference clock on pllp_out4 */
>>>> +		clock_enable(PERIPH_ID_DEV2_OUT);
>>>> +		clock_set_pllout(CLOCK_ID_PERIPH, PLL_OUT4, 24000000);
>>>
>>> Wouldn't it be clearer if:
>>> 1) you put the above inside the init_ulpi_usb_controller() function
>>> 2) Provide a !CONFIG_USB_ULPI implementation of the same function
>>>    technically having only the code under #else below inside.
>>>
>> Actually I'm not really sure what to do about this. Although I've not
>> seen any Tegra boards with a other ULPI reference freq used, maybe we
>> should just move the clock setup into board code or add a device tree
>> entry to tell the ref frequency.
>>
>> Stephen, Tom, any ideas?
> 
> Moving all the initialization into init_utmi_usb_controller() and
> init_ulpi_usb_controller() sounds reasonable to me.

Agreed completely.

> 
> I imagine that the reference frequency is somewhat driven by the
> requirements of USB itself and/or the ULPI interface. I think it's fine
> to just hard-code that in the USB driver for now; we can easily enhance
> the driver to make it configurable from either DT or U-Boot config file
> in the future if we need.

Well, it can also be both the controller and the ULPI PHY,
but I agree, you can hard code it for now or make something like:

#define DEFAULT_ULPI_REF_CLK 24000000
#ifndef ULPI_REF_CLK
#define ULPI_REF_CLK DEFAULT_ULPI_REF_CLK
#endif

and use the ULPI_REF_CLK where appropriate.

-- 
Regards,
Igor.

[U-Boot] [PATCH 3/3] tegra20: add USB ULPI init code

[Igor Grinberg]

On 08/20/12 15:41, Lucas Stach wrote:
> Hello Igor,
> 
> thanks for your review. Comments inline.
> 
> Am Montag, den 20.08.2012, 15:07 +0300 schrieb Igor Grinberg:
>> Hi Lucas,
>>
>> On 08/19/12 19:08, Lucas Stach wrote:
>>> This adds the required code to set up a ULPI USB port. It is
>>> mostly a port of the Linux ULPI setup code with some tweaks
>>> added for more correctness, discovered along the way of
>>> debugging this.
>>
>> Can you share which tweaks for correctness are there?
>>
> Linux code does not actually check if the phy clock comes up correctly,
> it just waits 100ms and hopes it is there. Also we explicitly select the
> ULPI interface, although it also works without this, as ULPI seems to be
> the default state for USB2.

Ok. Thanks.

> 
>>>
>>> To use this both CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
>>> have to be set in the board configuration file.
>>>
>>> Signed-off-by: Lucas Stach <dev@lynxeye.de>
>>> ---
>>>  arch/arm/cpu/armv7/tegra20/usb.c        | 131 +++++++++++++++++++++++++++++---
>>>  arch/arm/include/asm/arch-tegra20/usb.h |  29 +++++--
>>>  2 Dateien ge?ndert, 145 Zeilen hinzugef?gt(+), 15 Zeilen entfernt(-)
>>>
>>> diff --git a/arch/arm/cpu/armv7/tegra20/usb.c b/arch/arm/cpu/armv7/tegra20/usb.c
>>> index 77966e5..2ae1244 100644
>>> --- a/arch/arm/cpu/armv7/tegra20/usb.c
>>> +++ b/arch/arm/cpu/armv7/tegra20/usb.c
>>> @@ -32,9 +32,17 @@
>>>  #include <asm/arch/sys_proto.h>
>>>  #include <asm/arch/uart.h>
>>>  #include <asm/arch/usb.h>
>>> +#include <usb/ulpi.h>
>>>  #include <libfdt.h>
>>>  #include <fdtdec.h>
>>>  
>>> +#ifdef CONFIG_USB_ULPI
>>> +	#ifndef CONFIG_USB_ULPI_VIEWPORT
>>> +	#error "To use CONFIG_USB_ULPI on Tegra Boards you have to also \
>>> +	       define CONFIG_USB_ULPI_VIEWPORT"
>>
>> there is a mix of tabs and spaces in the above line, please make it only tabs
> 
> This was actually intentional to align the two lines exactly.

I see. Well, in U-Boot we have had huge threads discussing
how the alignment should be done and the conclusion was that only
tabs should be used (even if it means that the starting letters
will not be one below the other).
Now in the above particular case, you can place a tab between the
"#error" and the message - this will assure the wanted alignment
of both lines.

> 
>>> +	#endif
>>> +#endif
>>> +
>>>  enum {
>>>  	USB_PORTS_MAX	= 4,			/* Maximum ports we allow */
>>>  };
>>> @@ -68,11 +76,13 @@ enum dr_mode {
>>>  struct fdt_usb {
>>>  	struct usb_ctlr *reg;	/* address of registers in physical memory */
>>>  	unsigned utmi:1;	/* 1 if port has external tranceiver, else 0 */
>>> +	unsigned ulpi:1;	/* 1 if port has external ULPI transceiver */
>>>  	unsigned enabled:1;	/* 1 to enable, 0 to disable */
>>>  	unsigned has_legacy_mode:1; /* 1 if this port has legacy mode */
>>>  	enum dr_mode dr_mode;	/* dual role mode */
>>>  	enum periph_id periph_id;/* peripheral id */
>>>  	struct fdt_gpio_state vbus_gpio;	/* GPIO for vbus enable */
>>> +	struct fdt_gpio_state phy_reset_gpio; /* GPIO to reset ULPI phy */
>>>  };
>>>  
>>>  static struct fdt_usb port[USB_PORTS_MAX];	/* List of valid USB ports */
>>> @@ -187,8 +197,8 @@ static void usbf_reset_controller(struct fdt_usb *config,
>>>  	 */
>>>  }
>>>  
>>> -/* set up the USB controller with the parameters provided */
>>> -static int init_usb_controller(struct fdt_usb *config,
>>> +/* set up the UTMI USB controller with the parameters provided */
>>> +static int init_utmi_usb_controller(struct fdt_usb *config,
>>>  				struct usb_ctlr *usbctlr, const u32 timing[])
>>>  {
>>>  	u32 val;
>>> @@ -300,6 +310,83 @@ static int init_usb_controller(struct fdt_usb *config,
>>>  	return 0;
>>>  }
>>>  
>>> +#ifdef CONFIG_USB_ULPI
>>> +/* set up the ULPI USB controller with the parameters provided */
>>> +static int init_ulpi_usb_controller(struct fdt_usb *config,
>>> +                                    struct usb_ctlr *usbctlr)
>>> +{
>>> +	u32 val;
>>> +	int loop_count;
>>> +	struct ulpi_regs *ulpi_reg = (struct ulpi_regs *)0;
>>> +	struct ulpi_viewport ulpi_vp;
>>> +
>>> +	/* reset ULPI phy */
>>> +	if (fdt_gpio_isvalid(&config->phy_reset_gpio)) {
>>> +		fdtdec_setup_gpio(&config->phy_reset_gpio);
>>> +		gpio_direction_output(config->phy_reset_gpio.gpio, 0);
>>> +		mdelay(5);
>>> +		gpio_set_value(config->phy_reset_gpio.gpio, 1);
>>> +	}
>>> +
>>> +	/* Reset the usb controller */
>>> +	clock_enable(config->periph_id);
>>> +	usbf_reset_controller(config, usbctlr);
>>> +
>>> +	/* enable pinmux bypass */
>>> +	setbits_le32(&usbctlr->ulpi_timing_ctrl_0,
>>> +	             ULPI_CLKOUT_PINMUX_BYP | ULPI_OUTPUT_PINMUX_BYP);
>>> +
>>> +	/* Select ULPI parallel interface */
>>> +	clrsetbits_le32(&usbctlr->port_sc1, PTS_MASK, PTS_ULPI << PTS_SHIFT);
>>> +
>>> +	/* enable ULPI transceiver */
>>> +	setbits_le32(&usbctlr->susp_ctrl, ULPI_PHY_ENB);
>>> +
>>> +	/* configure ULPI transceiver timings */
>>> +	val = 0;
>>> +	writel(val, &usbctlr->ulpi_timing_ctrl_1);
>>> +
>>> +	val |= ULPI_DATA_TRIMMER_SEL(4);
>>> +	val |= ULPI_STPDIRNXT_TRIMMER_SEL(4);
>>> +	val |= ULPI_DIR_TRIMMER_SEL(4);
>>> +	writel(val, &usbctlr->ulpi_timing_ctrl_1);
>>> +	udelay(10);
>>> +
>>> +	val |= ULPI_DATA_TRIMMER_LOAD;
>>> +	val |= ULPI_STPDIRNXT_TRIMMER_LOAD;
>>> +	val |= ULPI_DIR_TRIMMER_LOAD;
>>> +	writel(val, &usbctlr->ulpi_timing_ctrl_1);
>>> +
>>> +	/* set up phy for host operation with external vbus supply */
>>> +	ulpi_vp.port_num = 0;
>>> +	ulpi_vp.viewport_addr = (u32)&usbctlr->ulpi_viewport;
>>> +
>>> +	if (ulpi_init(&ulpi_vp)) {
>>> +		debug("Tegra ULPI viewport init failed\n");
>>
>> This looks like an error, right?
>> So why hide it under debug?
>>
> Yes, you're right. I'll fix that.
> 
>>> +		return -1;
>>> +	}
>>> +
>>> +	ulpi_write(&ulpi_vp, &ulpi_reg->iface_ctrl_set, ULPI_IFACE_PASSTHRU);
>>
>> The implementation for the indicator setup is currently missing
>> from the generic framework.
>> Have you thought about adding it?
>>
> Yep, only looked into the framework provided functions after writing
> this patch. I'll do a patch for this in the next series iteration.

Thanks!

> 
>>> +	ulpi_write(&ulpi_vp, &ulpi_reg->otg_ctrl_set, ULPI_OTG_EXTVBUSIND);
>>
>> Can ulpi_set_vbus(&ulpi_vp, 1, 1, 1) be used here?
>>
> Yes, I already changed this locally.
> 
>>> +
>>> +	/* enable wakeup events */
>>> +	setbits_le32(&usbctlr->port_sc1, WKCN | WKDS | WKOC);
>>> +
>>> +	setbits_le32(&usbctlr->susp_ctrl, USB_SUSP_CLR);
>>> +	/* Wait for the phy clock to become valid in 100 ms */
>>> +	for (loop_count = 100000; loop_count != 0; loop_count--) {
>>> +		if (readl(&usbctlr->susp_ctrl) & USB_PHY_CLK_VALID)
>>> +			break;
>>> +		udelay(1);
>>> +	}
>>> +	if (!loop_count)
>>> +		return -1;
>>> +	clrbits_le32(&usbctlr->susp_ctrl, USB_SUSP_CLR);
>>> +
>>> +	return 0;
>>> +}
>>> +#endif
>>> +
>>>  static void power_up_port(struct usb_ctlr *usbctlr)
>>>  {
>>>  	/* Deassert power down state */
>>> @@ -331,11 +418,13 @@ static int add_port(struct fdt_usb *config, const u32 timing[])
>>>  		      USB_PORTS_MAX);
>>>  		return -1;
>>>  	}
>>> -	if (init_usb_controller(config, usbctlr, timing)) {
>>> -		debug("tegrausb: Cannot init port\n");
>>> -		return -1;
>>> -	}
>>> +
>>>  	if (config->utmi) {
>>> +		if (init_utmi_usb_controller(config, usbctlr, timing)) {
>>> +			debug("tegrausb: Cannot init port\n");
>>
>> This also looks like an error...
>> So why debug()?
>>
>>> +			return -1;
>>> +		}
>>> +
>>>  		/* Disable ICUSB FS/LS transceiver */
>>>  		clrbits_le32(&usbctlr->icusb_ctrl, IC_ENB1);
>>>  
>>> @@ -345,6 +434,24 @@ static int add_port(struct fdt_usb *config, const u32 timing[])
>>>  		clrbits_le32(&usbctlr->port_sc1, STS);
>>>  		power_up_port(usbctlr);
>>>  	}
>>> +
>>> +	if (config->ulpi) {
>>> +#ifdef CONFIG_USB_ULPI
>>> +		/* set up 24MHz ULPI reference clock on pllp_out4 */
>>> +		clock_enable(PERIPH_ID_DEV2_OUT);
>>> +		clock_set_pllout(CLOCK_ID_PERIPH, PLL_OUT4, 24000000);
>>
>> Wouldn't it be clearer if:
>> 1) you put the above inside the init_ulpi_usb_controller() function
>> 2) Provide a !CONFIG_USB_ULPI implementation of the same function
>>    technically having only the code under #else below inside.
>>
> Actually I'm not really sure what to do about this. Although I've not
> seen any Tegra boards with a other ULPI reference freq used, maybe we
> should just move the clock setup into board code or add a device tree
> entry to tell the ref frequency.
> 
> Stephen, Tom, any ideas?
> 
>> So then here instead of the whole #ifdef, you will have something like:
>> if (config->ulpi) {
>> 	if (init_ulpi_usb_controller(config, usbctlr)) {
>> 		printf("tegrausb: Cannot init port\n");
>> 		return -1;
>> 	}
>> }
> 
>>> +
>>> +		if (init_ulpi_usb_controller(config, usbctlr)) {
>>> +			debug("tegrausb: Cannot init port\n");
>>> +			return -1;
>>> +		}
>>> +#else
>>> +		debug("No code to set up ULPI controller, please enable"
>>> +		      "CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT");
>>> +		return -1;
>>> +#endif
>>> +	}
>>> +
>>>  	port[port_count++] = *config;
>>>  
>>>  	return 0;
>>
>> [...]
>>
>>
>>
> 
> 
> 

-- 
Regards,
Igor.

[U-Boot] [PATCH 3/3] tegra20: add USB ULPI init code

[Stephen Warren]

On 08/19/2012 10:08 AM, Lucas Stach wrote:
> This adds the required code to set up a ULPI USB port. It is
> mostly a port of the Linux ULPI setup code with some tweaks
> added for more correctness, discovered along the way of
> debugging this.
> 
> To use this both CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
> have to be set in the board configuration file.

> diff --git a/arch/arm/cpu/armv7/tegra20/usb.c b/arch/arm/cpu/armv7/tegra20/usb.c

> -/* set up the USB controller with the parameters provided */
> -static int init_usb_controller(struct fdt_usb *config,
> +/* set up the UTMI USB controller with the parameters provided */
> +static int init_utmi_usb_controller(struct fdt_usb *config,
>  				struct usb_ctlr *usbctlr, const u32 timing[])
...
> +#ifdef CONFIG_USB_ULPI
> +/* set up the ULPI USB controller with the parameters provided */
> +static int init_ulpi_usb_controller(struct fdt_usb *config,
> +                                    struct usb_ctlr *usbctlr)

I'm not sure why there's a CONFIG_USB_ULPI and not a matching
CONFIG_USB_UTMI; it seems rather unbalanced, but anyway...

Aside from that, I don't have any other obvious objections to this.

[U-Boot] [PATCH 0/3] Tegra 2 USB ULPI series

[Igor Grinberg]

On 08/19/12 19:08, Lucas Stach wrote:
> With this series we are able to initialize USB controllers
> using an external ULPI phy AKA USB2 on Tegra 2 devices.
> 
> This was tested to work on a Toradex Colibri T20 board,
> where USB2 is used to access the ASIX ethernet chipset.
> Testing was done with "tegra20: usb: rework set_host_mode"

I'm sorry, I have missed this patch.
Can you please give us a reference to it?

> applied. I did not spot any regressions on the UTMI ports.
> 
> Patchset is based on top of u-boot-tegra/next
> 
> Lucas Stach (3):
>   tegra20: complete periph_id enum
>   tegra20: add clock_set_pllout function
>   tegra20: add USB ULPI init code
> 
>  arch/arm/cpu/armv7/tegra20/usb.c            | 131 +++++++++++++++++++++++++---
>  arch/arm/cpu/tegra20-common/clock.c         |  26 ++++++
>  arch/arm/cpu/tegra20-common/warmboot_avp.c  |   2 +-
>  arch/arm/include/asm/arch-tegra20/clk_rst.h |  11 ++-
>  arch/arm/include/asm/arch-tegra20/clock.h   |  25 ++++++
>  arch/arm/include/asm/arch-tegra20/usb.h     |  29 ++++--
>  6 Dateien ge?ndert, 206 Zeilen hinzugef?gt(+), 18 Zeilen entfernt(-)
> 

-- 
Regards,
Igor.

[U-Boot] [PATCH 0/3] Tegra 2 USB ULPI series

[Lucas Stach]

Am Montag, den 20.08.2012, 15:27 +0300 schrieb Igor Grinberg:
> 
> On 08/19/12 19:08, Lucas Stach wrote:
> > With this series we are able to initialize USB controllers
> > using an external ULPI phy AKA USB2 on Tegra 2 devices.
> > 
> > This was tested to work on a Toradex Colibri T20 board,
> > where USB2 is used to access the ASIX ethernet chipset.
> > Testing was done with "tegra20: usb: rework set_host_mode"
> 
> I'm sorry, I have missed this patch.
> Can you please give us a reference to it?
> 
It has nothing to do with this series per se, it is only needed on the
Colibri board as VBus enable there is not done using the ULPI phy, but a
separate GPIO from the Tegra SoC.

But yes, I'll include a reference when reposting.

> > applied. I did not spot any regressions on the UTMI ports.
> > 
> > Patchset is based on top of u-boot-tegra/next
> > 
> > Lucas Stach (3):
> >   tegra20: complete periph_id enum
> >   tegra20: add clock_set_pllout function
> >   tegra20: add USB ULPI init code
> > 
> >  arch/arm/cpu/armv7/tegra20/usb.c            | 131 +++++++++++++++++++++++++---
> >  arch/arm/cpu/tegra20-common/clock.c         |  26 ++++++
> >  arch/arm/cpu/tegra20-common/warmboot_avp.c  |   2 +-
> >  arch/arm/include/asm/arch-tegra20/clk_rst.h |  11 ++-
> >  arch/arm/include/asm/arch-tegra20/clock.h   |  25 ++++++
> >  arch/arm/include/asm/arch-tegra20/usb.h     |  29 ++++--
> >  6 Dateien ge?ndert, 206 Zeilen hinzugef?gt(+), 18 Zeilen entfernt(-)
> > 
> 

[U-Boot] [PATCH 0/3] Tegra 2 USB ULPI series

[Stephen Warren]

On 08/19/2012 10:08 AM, Lucas Stach wrote:
> With this series we are able to initialize USB controllers
> using an external ULPI phy AKA USB2 on Tegra 2 devices.
> 
> This was tested to work on a Toradex Colibri T20 board,
> where USB2 is used to access the ASIX ethernet chipset.
> Testing was done with "tegra20: usb: rework set_host_mode"
> applied. I did not spot any regressions on the UTMI ports.
> 
> Patchset is based on top of u-boot-tegra/next

Hmmm. I tried testing this on my local branch on Harmony, which has all
the patches necessary for multiple USB ports to work, and indeed they do
on Seaboard. However, this series (plus some Harmony-specific config and
.dts changes) didn't cause the ULPI port to work:-( Any idea why?

[U-Boot] [PATCH 0/3] Tegra 2 USB ULPI series

[Lucas Stach]

Am Montag, den 20.08.2012, 12:09 -0600 schrieb Stephen Warren:
> On 08/19/2012 10:08 AM, Lucas Stach wrote:
> > With this series we are able to initialize USB controllers
> > using an external ULPI phy AKA USB2 on Tegra 2 devices.
> > 
> > This was tested to work on a Toradex Colibri T20 board,
> > where USB2 is used to access the ASIX ethernet chipset.
> > Testing was done with "tegra20: usb: rework set_host_mode"
> > applied. I did not spot any regressions on the UTMI ports.
> > 
> > Patchset is based on top of u-boot-tegra/next
> 
> Hmmm. I tried testing this on my local branch on Harmony, which has all
> the patches necessary for multiple USB ports to work, and indeed they do
> on Seaboard. However, this series (plus some Harmony-specific config and
> .dts changes) didn't cause the ULPI port to work:-( Any idea why?
> 
Could you try the attached hack and report back? If it doesn't help
please build with DEBUG defined in
arch/arm/cpu/armv7/tegra20/usb.c
drivers/usb/host/ehci-hcd.c
and provide me the output of the usb start command.

Thanks,
Lucas

diff --git a/arch/arm/cpu/armv7/tegra20/usb.c b/arch/arm/cpu/armv7/tegra20/usb.c
index 2ae1244..4c85685 100644
--- a/arch/arm/cpu/armv7/tegra20/usb.c
+++ b/arch/arm/cpu/armv7/tegra20/usb.c
@@ -367,7 +367,7 @@ static int init_ulpi_usb_controller(struct fdt_usb *config,
        }
 
        ulpi_write(&ulpi_vp, &ulpi_reg->iface_ctrl_set, ULPI_IFACE_PASSTHRU);
-       ulpi_write(&ulpi_vp, &ulpi_reg->otg_ctrl_set, ULPI_OTG_EXTVBUSIND);
+       ulpi_write(&ulpi_vp, &ulpi_reg->otg_ctrl_set, ULPI_OTG_DRVVBUS|ULPI_OTG_DRVVBUS_EXT|ULPI_OTG_EXTVBUSIND);
 
        /* enable wakeup events */
        setbits_le32(&usbctlr->port_sc1, WKCN | WKDS | WKOC);

[U-Boot] [PATCH 0/3] Tegra 2 USB ULPI series

[Stephen Warren]

On 08/20/2012 01:59 PM, Lucas Stach wrote:
> Am Montag, den 20.08.2012, 12:09 -0600 schrieb Stephen Warren:
>> On 08/19/2012 10:08 AM, Lucas Stach wrote:
>>> With this series we are able to initialize USB controllers
>>> using an external ULPI phy AKA USB2 on Tegra 2 devices.
>>>
>>> This was tested to work on a Toradex Colibri T20 board,
>>> where USB2 is used to access the ASIX ethernet chipset.
>>> Testing was done with "tegra20: usb: rework set_host_mode"
>>> applied. I did not spot any regressions on the UTMI ports.
>>>
>>> Patchset is based on top of u-boot-tegra/next
>>
>> Hmmm. I tried testing this on my local branch on Harmony, which has all
>> the patches necessary for multiple USB ports to work, and indeed they do
>> on Seaboard. However, this series (plus some Harmony-specific config and
>> .dts changes) didn't cause the ULPI port to work:-( Any idea why?
>>
> Could you try the attached hack and report back? If it doesn't help
> please build with DEBUG defined in
> arch/arm/cpu/armv7/tegra20/usb.c
> drivers/usb/host/ehci-hcd.c
> and provide me the output of the usb start command.

Oops. I had forgotten to enable ULPI/ULPI_VIEWPORT config options.

Now that I do that, U-Boot hangs when enabling the ULPI transceiver. I
see this with series v1, v1 plus the patch you included in the email I'm
replying to, and v2.

In particular, if I printf every line, the printf before the following
line appears, but not the one after it:

	setbits_le32(&usbctlr->susp_ctrl, ULPI_PHY_ENB);

If I comment that out, then the code runs farther, eventually hanging
inside ulpi_init(). Do you have any idea what's wrong?