diff mbox

[U-Boot] usb: tegra: move Tegra EHCI implementation to correct place

Message ID 1351206943-29358-1-git-send-email-dev@lynxeye.de
State Changes Requested
Delegated to: Tom Warren
Headers show

Commit Message

Lucas Stach Oct. 25, 2012, 11:15 p.m. UTC 
Move the Tegra EHCI implementation to the correct directory in the tree.
This code is specific to the Tegra EHCI controller, not to the Tegra SoC
in general.

This is not just a move of the code, but also some small changes squashed in.
Most notable:
- removed some unneeded parameters from function calls, to make functions
  more self contained
- decrease max controller count to 3, both Tegra 2 and 3 have at most 3
  EHCI controllers, we can aleays increase this later if the need arises
- controllers only get activated at ehci_hcd_init time, not at board_usb_init,
  which is the more obvious init point and saves time if you are not going
  to use usb in your boot process at all

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
This patch is based on the u-boot-usb tree

I've tested this on the Colibri T20 platform with no functional regressions.
All 3 USB controllers (both UTMI and ULPI) work as before the change.
---
 arch/arm/cpu/armv7/tegra20/Makefile                |   2 -
 .../tegra20/usb.c => drivers/usb/host/ehci-tegra.c | 210 +++++++++++----------
 2 Dateien geändert, 109 Zeilen hinzugefügt(+), 103 Zeilen entfernt(-)
 rename arch/arm/cpu/armv7/tegra20/usb.c => drivers/usb/host/ehci-tegra.c (87%)

Comments

Marek Vasut Oct. 26, 2012, 7:09 a.m. UTC | #1 
Dear Lucas Stach,

> Move the Tegra EHCI implementation to the correct directory in the tree.
> This code is specific to the Tegra EHCI controller, not to the Tegra SoC
> in general.
> 
> This is not just a move of the code, but also some small changes squashed

Unsquash them please, it's really hard to review. Also, use git format-patch -M 
-C to generate the patchset and to detect renames.

> in. Most notable:
> - removed some unneeded parameters from function calls, to make functions
>   more self contained
> - decrease max controller count to 3, both Tegra 2 and 3 have at most 3
>   EHCI controllers, we can aleays increase this later if the need arises
> - controllers only get activated at ehci_hcd_init time, not at
> board_usb_init, which is the more obvious init point and saves time if you
> are not going to use usb in your boot process at all
[...]
Best regards,
Marek Vasut
Lucas Stach Oct. 26, 2012, 7:29 a.m. UTC | #2 
Hi Marek,

Am Freitag, den 26.10.2012, 09:09 +0200 schrieb Marek Vasut:
> Dear Lucas Stach,
> 
> > Move the Tegra EHCI implementation to the correct directory in the tree.
> > This code is specific to the Tegra EHCI controller, not to the Tegra SoC
> > in general.
> > 
> > This is not just a move of the code, but also some small changes squashed
> 
> Unsquash them please, it's really hard to review. Also, use git format-patch -M 
> -C to generate the patchset and to detect renames.
> 
I've generated the patch with format-patch -B -M -C, so what you see is
basically the diff of the original file with the new implementation. So
the diff shows all the small changes I blather about.

The changes are squashed in because I did them while moving the
implementation over, and I would like to avoid doing them in single
patches as it's notably more work and IMHO with the current diff you can
clearly see what changed. So please let me know if you are really that
keen to have them as separate patches, but it may take me some while to
respin this as I'm occupied with other things right now.

Thanks,
Lucas
> > in. Most notable:
> > - removed some unneeded parameters from function calls, to make functions
> >   more self contained
> > - decrease max controller count to 3, both Tegra 2 and 3 have at most 3
> >   EHCI controllers, we can aleays increase this later if the need arises
> > - controllers only get activated at ehci_hcd_init time, not at
> > board_usb_init, which is the more obvious init point and saves time if you
> > are not going to use usb in your boot process at all
> [...]
> Best regards,
> Marek Vasut
>
Marek Vasut Oct. 26, 2012, 10:13 a.m. UTC | #3 
Dear Lucas Stach,

> Hi Marek,
> 
> Am Freitag, den 26.10.2012, 09:09 +0200 schrieb Marek Vasut:
> > Dear Lucas Stach,
> > 
> > > Move the Tegra EHCI implementation to the correct directory in the
> > > tree. This code is specific to the Tegra EHCI controller, not to the
> > > Tegra SoC in general.
> > > 
> > > This is not just a move of the code, but also some small changes
> > > squashed
> > 
> > Unsquash them please, it's really hard to review. Also, use git
> > format-patch -M -C to generate the patchset and to detect renames.
> 
> I've generated the patch with format-patch -B -M -C, so what you see is
> basically the diff of the original file with the new implementation. So
> the diff shows all the small changes I blather about.
> 
> The changes are squashed in because I did them while moving the
> implementation over, and I would like to avoid doing them in single
> patches as it's notably more work and IMHO with the current diff you can
> clearly see what changed. So please let me know if you are really that
> keen to have them as separate patches, but it may take me some while to
> respin this as I'm occupied with other things right now.
[...]

Let's do it properly please.

Best regards,
Marek Vasut
Simon Glass Oct. 26, 2012, 4:11 p.m. UTC | #4 
Hi Lucas,

On Fri, Oct 26, 2012 at 3:13 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Lucas Stach,
>
>> Hi Marek,
>>
>> Am Freitag, den 26.10.2012, 09:09 +0200 schrieb Marek Vasut:
>> > Dear Lucas Stach,
>> >
>> > > Move the Tegra EHCI implementation to the correct directory in the
>> > > tree. This code is specific to the Tegra EHCI controller, not to the
>> > > Tegra SoC in general.
>> > >
>> > > This is not just a move of the code, but also some small changes
>> > > squashed
>> >
>> > Unsquash them please, it's really hard to review. Also, use git
>> > format-patch -M -C to generate the patchset and to detect renames.
>>
>> I've generated the patch with format-patch -B -M -C, so what you see is
>> basically the diff of the original file with the new implementation. So
>> the diff shows all the small changes I blather about.
>>
>> The changes are squashed in because I did them while moving the
>> implementation over, and I would like to avoid doing them in single
>> patches as it's notably more work and IMHO with the current diff you can
>> clearly see what changed. So please let me know if you are really that
>> keen to have them as separate patches, but it may take me some while to
>> respin this as I'm occupied with other things right now.
> [...]
>
> Let's do it properly please.

Sorry, but I agree. I did start reviewing it, but stopped because
really we shouldn't have so many things in one patch.

Regards,
Simon

>
> Best regards,
> Marek Vasut
diff mbox

Patch

diff --git a/arch/arm/cpu/armv7/tegra20/Makefile b/arch/arm/cpu/armv7/tegra20/Makefile
index 09a0314..2c4d5c9 100644
--- a/arch/arm/cpu/armv7/tegra20/Makefile
+++ b/arch/arm/cpu/armv7/tegra20/Makefile
@@ -27,8 +27,6 @@  include $(TOPDIR)/config.mk
 
 LIB	=  $(obj)lib$(SOC).o
 
-COBJS-$(CONFIG_USB_EHCI_TEGRA) += usb.o
-
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
 OBJS	:= $(addprefix $(obj),$(COBJS))
diff --git a/arch/arm/cpu/armv7/tegra20/usb.c b/drivers/usb/host/ehci-tegra.c
similarity index 87%
rename from arch/arm/cpu/armv7/tegra20/usb.c
rename to drivers/usb/host/ehci-tegra.c
index 1bccf2b..0646028 100644
--- a/arch/arm/cpu/armv7/tegra20/usb.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -1,6 +1,7 @@ 
 /*
  * Copyright (c) 2011 The Chromium OS Authors.
- * (C) Copyright 2010,2011 NVIDIA Corporation <www.nvidia.com>
+ * Copyright (c) 2009-2012 NVIDIA Corporation
+ * Copyright (c) 2012 Lucas Stach
  *
  * See file CREDITS for list of people who contributed to this
  * project.
@@ -22,6 +23,12 @@ 
  */
 
 #include <common.h>
+#include <fdtdec.h>
+#include <libfdt.h>
+#include <usb.h>
+#include <usb/ulpi.h>
+
+#include <asm/errno.h>
 #include <asm/io.h>
 #include <asm-generic/gpio.h>
 #include <asm/arch/clock.h>
@@ -29,12 +36,11 @@ 
 #include <asm/arch/pinmux.h>
 #include <asm/arch/tegra.h>
 #include <asm/arch/usb.h>
-#include <usb/ulpi.h>
 #include <asm/arch-tegra/clk_rst.h>
 #include <asm/arch-tegra/sys_proto.h>
 #include <asm/arch-tegra/uart.h>
-#include <libfdt.h>
-#include <fdtdec.h>
+
+#include "ehci.h"
 
 #ifdef CONFIG_USB_ULPI
 	#ifndef CONFIG_USB_ULPI_VIEWPORT
@@ -43,9 +49,7 @@ 
 	#endif
 #endif
 
-enum {
-	USB_PORTS_MAX	= 4,			/* Maximum ports we allow */
-};
+#define USB_PORTS_MAX	3		/* maximum number of ports we allow */
 
 /* Parameters we need for USB */
 enum {
@@ -146,6 +150,23 @@  static const u8 utmip_elastic_limit = 16;
 /* UTMIP High Speed Sync Start Delay */
 static const u8 utmip_hs_sync_start_delay = 9;
 
+/*
+ * A known hardware issue where Connect Status Change bit of PORTSC register
+ * of USB1 controller will be set after Port Reset.
+ * We have to clear it in order for later device enumeration to proceed.
+ * This ehci_powerup_fixup overrides the weak function ehci_powerup_fixup
+ * in "ehci-hcd.c".
+ */
+void ehci_powerup_fixup(uint32_t *status_reg, uint32_t *reg)
+{
+	mdelay(50);
+	if (((u32) status_reg & TEGRA_USB_ADDR_MASK) != TEGRA_USB1_BASE)
+		return;
+	/* For EHCI_PS_CSC to be cleared in ehci_hcd.c */
+	if (ehci_readl(status_reg) & EHCI_PS_CSC)
+		*reg |= EHCI_PS_CSC;
+}
+
 /* Put the port into host mode */
 static void set_host_mode(struct fdt_usb *config)
 {
@@ -190,19 +211,15 @@  void usbf_reset_controller(struct fdt_usb *config, struct usb_ctlr *usbctlr)
 	/* Enable the UTMIP PHY */
 	if (config->utmi)
 		setbits_le32(&usbctlr->susp_ctrl, UTMIP_PHY_ENB);
-
-	/*
-	 * TODO: where do we take the USB1 out of reset? The old code would
-	 * take USB3 out of reset, but not USB1. This code doesn't do either.
-	 */
 }
 
 /* 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[])
+static int init_utmi_usb_controller(struct fdt_usb *config)
 {
 	u32 val;
 	int loop_count;
+	u32 *timing;
+	struct usb_ctlr *usbctlr = config->reg;
 
 	clock_enable(config->periph_id);
 
@@ -229,6 +246,8 @@  static int init_utmi_usb_controller(struct fdt_usb *config,
 	 * PLL Delay CONFIGURATION settings. The following parameters control
 	 * the bring up of the plls.
 	 */
+	timing = usb_pll[clock_get_osc_freq()];
+
 	val = readl(&usbctlr->utmip_misc_cfg1);
 	clrsetbits_le32(&val, UTMIP_PLLU_STABLE_COUNT_MASK,
 		timing[PARAM_STABLE_COUNT] << UTMIP_PLLU_STABLE_COUNT_SHIFT);
@@ -331,12 +350,12 @@  static int init_utmi_usb_controller(struct fdt_usb *config,
 #endif
 
 /* set up the ULPI USB controller with the parameters provided */
-static int init_ulpi_usb_controller(struct fdt_usb *config,
-				struct usb_ctlr *usbctlr)
+static int init_ulpi_usb_controller(struct fdt_usb *config)
 {
 	u32 val;
 	int loop_count;
 	struct ulpi_viewport ulpi_vp;
+	struct usb_ctlr *usbctlr = config->reg;
 
 	/* set up ULPI reference clock on pllp_out4 */
 	clock_enable(PERIPH_ID_DEV2_OUT);
@@ -408,86 +427,15 @@  static int init_ulpi_usb_controller(struct fdt_usb *config,
 	return 0;
 }
 #else
-static int init_ulpi_usb_controller(struct fdt_usb *config,
-				struct usb_ctlr *usbctlr)
+static int init_ulpi_usb_controller(struct fdt_usb *config)
 {
 	printf("No code to set up ULPI controller, please enable"
-			"CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT");
+		"CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT");
 	return -1;
 }
 #endif
 
-static void config_clock(const u32 timing[])
-{
-	clock_start_pll(CLOCK_ID_USB,
-		timing[PARAM_DIVM], timing[PARAM_DIVN], timing[PARAM_DIVP],
-		timing[PARAM_CPCON], timing[PARAM_LFCON]);
-}
-
-/**
- * Add a new USB port to the list of available ports.
- *
- * @param config	USB port configuration
- * @return 0 if ok, -1 if error (too many ports)
- */
-static int add_port(struct fdt_usb *config, const u32 timing[])
-{
-	struct usb_ctlr *usbctlr = config->reg;
-
-	if (port_count == USB_PORTS_MAX) {
-		printf("tegrausb: Cannot register more than %d ports\n",
-		      USB_PORTS_MAX);
-		return -1;
-	}
-
-	if (config->utmi && init_utmi_usb_controller(config, usbctlr, timing)) {
-		printf("tegrausb: Cannot init port\n");
-		return -1;
-	}
-
-	if (config->ulpi && init_ulpi_usb_controller(config, usbctlr)) {
-		printf("tegrausb: Cannot init port\n");
-		return -1;
-	}
-
-	port[port_count++] = *config;
-
-	return 0;
-}
-
-int tegrausb_start_port(int portnum, u32 *hccr, u32 *hcor)
-{
-	struct usb_ctlr *usbctlr;
-
-	if (portnum >= port_count)
-		return -1;
-	set_host_mode(&port[portnum]);
-
-	usbctlr = port[portnum].reg;
-	*hccr = (u32)&usbctlr->cap_length;
-	*hcor = (u32)&usbctlr->usb_cmd;
-	return 0;
-}
-
-int tegrausb_stop_port(int portnum)
-{
-	struct usb_ctlr *usbctlr;
-
-	usbctlr = port[portnum].reg;
-
-	/* Stop controller */
-	writel(0, &usbctlr->usb_cmd);
-	udelay(1000);
-
-	/* Initiate controller reset */
-	writel(2, &usbctlr->usb_cmd);
-	udelay(1000);
-
-	return 0;
-}
-
-int fdt_decode_usb(const void *blob, int node, unsigned osc_frequency_mhz,
-		   struct fdt_usb *config)
+int fdt_decode_usb(const void *blob, int node, struct fdt_usb *config)
 {
 	const char *phy, *mode;
 
@@ -502,7 +450,7 @@  int fdt_decode_usb(const void *blob, int node, unsigned osc_frequency_mhz,
 			config->dr_mode = DR_MODE_OTG;
 		else {
 			debug("%s: Cannot decode dr_mode '%s'\n", __func__,
-			      mode);
+				mode);
 			return -FDT_ERR_NOTFOUND;
 		}
 	} else {
@@ -534,15 +482,16 @@  int fdt_decode_usb(const void *blob, int node, unsigned osc_frequency_mhz,
 
 int board_usb_init(const void *blob)
 {
-	struct fdt_usb config;
-	unsigned osc_freq = clock_get_rate(CLOCK_ID_OSC);
-	enum clock_osc_freq freq;
+	struct fdt_usb *config;
 	int node_list[USB_PORTS_MAX];
 	int node, count, i;
+	u32 *timing;
 
 	/* Set up the USB clocks correctly based on our oscillator frequency */
-	freq = clock_get_osc_freq();
-	config_clock(usb_pll[freq]);
+	timing = usb_pll[clock_get_osc_freq()];
+	clock_start_pll(CLOCK_ID_USB,
+		timing[PARAM_DIVM], timing[PARAM_DIVN], timing[PARAM_DIVP],
+		timing[PARAM_CPCON], timing[PARAM_LFCON]);
 
 	/* count may return <0 on error */
 	count = fdtdec_find_aliases_for_id(blob, "usb",
@@ -552,16 +501,75 @@  int board_usb_init(const void *blob)
 		node = node_list[i];
 		if (!node)
 			continue;
-		if (fdt_decode_usb(blob, node, osc_freq, &config)) {
-			debug("Cannot decode USB node %s\n",
-			      fdt_get_name(blob, node, NULL));
+
+		if (port_count == USB_PORTS_MAX) {
+			printf("tegrausb: Cannot register more than %d ports\n",
+				USB_PORTS_MAX);
 			return -1;
 		}
 
-		if (add_port(&config, usb_pll[freq]))
+		config = &port[port_count++];
+
+		if (fdt_decode_usb(blob, node, config)) {
+			debug("Cannot decode USB node %s\n",
+				fdt_get_name(blob, node, NULL));
 			return -1;
-		set_host_mode(&config);
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * Create the appropriate control structures to manage
+ * a new EHCI host controller.
+ */
+int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor)
+{
+	struct usb_ctlr *usbctlr;
+	struct fdt_usb *config;
+
+	if (index >= port_count)
+		return -1;
+
+	/* init appropriate controller type */
+	config = &port[index];
+
+	if (config->utmi && init_utmi_usb_controller(config)) {
+		puts("tegrausb: Cannot init port\n");
+		return -1;
 	}
 
+	if (config->ulpi && init_ulpi_usb_controller(config)) {
+		puts("tegrausb: Cannot init port\n");
+		return -1;
+	}
+
+	set_host_mode(config);
+
+	usbctlr = config->reg;
+	*hccr = (struct ehci_hccr *)&usbctlr->cap_length;
+	*hcor = (struct ehci_hcor *)&usbctlr->usb_cmd;
+	return 0;
+}
+
+/*
+ * Destroy the appropriate control structures corresponding
+ * the the EHCI host controller.
+ */
+int ehci_hcd_stop(int index)
+{
+	struct usb_ctlr *usbctlr;
+
+	usbctlr = port[index].reg;
+
+	/* Stop controller */
+	writel(0, &usbctlr->usb_cmd);
+	udelay(1000);
+
+	/* Initiate controller reset */
+	writel(2, &usbctlr->usb_cmd);
+	udelay(1000);
+
 	return 0;
 }