[U-Boot] [PATCH 0/8] Exynos5420: Add support for XHCI controller

[U-Boot] [PATCH 0/8] Exynos5420: Add support for XHCI controller

[Vivek Gautam]

This patch series is based on u-boot-samsung 'master' branch.
The series is tested for both XHCI controller ports as well
as EHCI controller ports present on smdk5420 and smdk5250.

The series inclues few cleanup patches (cpu address fix, device
tree cleanup), plus multiple controller support for 'xhci' on
exynos5420 based systems.
The Vbus GPIO patch in the series is using the non-dt style of
configuring the GPIOs (using board_usb_vbus_init helper function).

Vivek Gautam (8):
  arm: exynos5420: Fix base address for USBPHY
  arm: exynos5420: Add base addresses for USB 3.0
  dts: exynos5250: Remove explicit device node of 'xhci'
  config: exynos5: Enable XHCI on all Exynos5 systems
  usb: xhci-exynos5: Add support for multiple USB 3.0 controllers
  dts: exynos5420: Move device node for xhci 2nd controller
  smdk5420: Correct the vbus gpio configuration for USB controller
  config: smdk5420: Enable support for multiple XHCI controllers

 arch/arm/cpu/armv7/exynos/power.c         |   18 ++++--
 arch/arm/dts/exynos5250.dtsi              |   14 -----
 arch/arm/dts/exynos5420.dtsi              |   12 ++++
 arch/arm/include/asm/arch-exynos/cpu.h    |   16 ++++-
 arch/arm/include/asm/arch-exynos/power.h  |    2 +-
 board/samsung/dts/exynos5420-smdk5420.dts |   12 ----
 board/samsung/smdk5420/smdk5420.c         |   11 ++--
 drivers/usb/host/xhci-exynos5.c           |   91 ++++++++++++++++++++---------
 drivers/usb/host/xhci.c                   |    4 --
 drivers/usb/host/xhci.h                   |    4 ++
 include/configs/exynos5-dt.h              |    7 +++
 include/configs/exynos5250-dt.h           |    7 ---
 include/configs/smdk5420.h                |    3 +
 13 files changed, 122 insertions(+), 79 deletions(-)

-- 
1.7.10.4

[U-Boot] [PATCH 1/8] arm: exynos5420: Fix base address for USBPHY

[Vivek Gautam]

'USBPHY' which denotes USB 2.0 PHY is available on exynos5420
Add it's base address rather than making it 'DEVICE_NOT_AVAILABLE'

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 arch/arm/include/asm/arch-exynos/cpu.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/arch-exynos/cpu.h b/arch/arm/include/asm/arch-exynos/cpu.h
index 573f755..6f9b1c5 100644
--- a/arch/arm/include/asm/arch-exynos/cpu.h
+++ b/arch/arm/include/asm/arch-exynos/cpu.h
@@ -148,6 +148,7 @@
 #define EXYNOS5420_DMC_CTRL_BASE	0x10C20000
 #define EXYNOS5420_DMC_TZASC_BASE	0x10D40000
 #define EXYNOS5420_USB_HOST_EHCI_BASE	0x12110000
+#define EXYNOS5420_USBPHY_BASE		0x12130000
 #define EXYNOS5420_MMC_BASE		0x12200000
 #define EXYNOS5420_SROMC_BASE		0x12250000
 #define EXYNOS5420_UART_BASE		0x12C00000
@@ -164,7 +165,6 @@
 #define EXYNOS5420_MIPI_DSIM_BASE	0x14500000
 #define EXYNOS5420_DP_BASE		0x145B0000
 
-#define EXYNOS5420_USBPHY_BASE		DEVICE_NOT_AVAILABLE
 #define EXYNOS5420_USBOTG_BASE		DEVICE_NOT_AVAILABLE
 #define EXYNOS5420_FIMD_BASE		DEVICE_NOT_AVAILABLE
 #define EXYNOS5420_ADC_BASE		DEVICE_NOT_AVAILABLE
-- 
1.7.10.4

[U-Boot] [PATCH 1/8] arm: exynos5420: Fix base address for USBPHY

[Marek Vasut]

On Monday, January 06, 2014 at 10:29:36 AM, Vivek Gautam wrote:
> 'USBPHY' which denotes USB 2.0 PHY is available on exynos5420
> Add it's base address rather than making it 'DEVICE_NOT_AVAILABLE'
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>

Reviewed-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

[U-Boot] [PATCH 2/8] arm: exynos5420: Add base addresses for USB 3.0

[Vivek Gautam]

Adding missing base addresses for XHCI and USB3-PHY,
to enable support for XHCI on exynos5420 systems.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 arch/arm/include/asm/arch-exynos/cpu.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/arch-exynos/cpu.h b/arch/arm/include/asm/arch-exynos/cpu.h
index 6f9b1c5..718940b 100644
--- a/arch/arm/include/asm/arch-exynos/cpu.h
+++ b/arch/arm/include/asm/arch-exynos/cpu.h
@@ -147,6 +147,8 @@
 #define EXYNOS5420_DMC_PHY_BASE		0x10C00000
 #define EXYNOS5420_DMC_CTRL_BASE	0x10C20000
 #define EXYNOS5420_DMC_TZASC_BASE	0x10D40000
+#define EXYNOS5420_USB_HOST_XHCI_BASE	0x12000000
+#define EXYNOS5420_USB3PHY_BASE		0x12100000
 #define EXYNOS5420_USB_HOST_EHCI_BASE	0x12110000
 #define EXYNOS5420_USBPHY_BASE		0x12130000
 #define EXYNOS5420_MMC_BASE		0x12200000
@@ -169,8 +171,6 @@
 #define EXYNOS5420_FIMD_BASE		DEVICE_NOT_AVAILABLE
 #define EXYNOS5420_ADC_BASE		DEVICE_NOT_AVAILABLE
 #define EXYNOS5420_MODEM_BASE		DEVICE_NOT_AVAILABLE
-#define EXYNOS5420_USB3PHY_BASE		DEVICE_NOT_AVAILABLE
-#define EXYNOS5420_USB_HOST_XHCI_BASE	DEVICE_NOT_AVAILABLE
 
 #ifndef __ASSEMBLY__
 #include <asm/io.h>
-- 
1.7.10.4

[U-Boot] [PATCH 2/8] arm: exynos5420: Add base addresses for USB 3.0

[Marek Vasut]

On Monday, January 06, 2014 at 10:29:37 AM, Vivek Gautam wrote:
> Adding missing base addresses for XHCI and USB3-PHY,
> to enable support for XHCI on exynos5420 systems.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>

Reviewed-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

[U-Boot] [PATCH 3/8] dts: exynos5250: Remove explicit device node of 'xhci'

[Vivek Gautam]

Device node for 'xhci' already exists in 'exynos5' dtsi file,
so we don't need an explicit mention about it again in
'exynos5250' dtsi file.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 arch/arm/dts/exynos5250.dtsi |   14 --------------
 1 file changed, 14 deletions(-)

diff --git a/arch/arm/dts/exynos5250.dtsi b/arch/arm/dts/exynos5250.dtsi
index 0c644e7..bb3f361 100644
--- a/arch/arm/dts/exynos5250.dtsi
+++ b/arch/arm/dts/exynos5250.dtsi
@@ -63,18 +63,4 @@
 		samsung,i2s-bit-clk-framesize = <32>;
 		samsung,i2s-id = <1>;
 	};
-
-
-	xhci at 12000000 {
-		compatible = "samsung,exynos5250-xhci";
-		reg = <0x12000000 0x10000>;
-		#address-cells = <1>;
-		#size-cells = <1>;
-
-		phy {
-			compatible = "samsung,exynos5250-usb3-phy";
-			reg = <0x12100000 0x100>;
-		};
-	};
-
 };
-- 
1.7.10.4

[U-Boot] [PATCH 3/8] dts: exynos5250: Remove explicit device node of 'xhci'

[Marek Vasut]

On Monday, January 06, 2014 at 10:29:38 AM, Vivek Gautam wrote:
> Device node for 'xhci' already exists in 'exynos5' dtsi file,
> so we don't need an explicit mention about it again in
> 'exynos5250' dtsi file.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>

Reviewed-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

[U-Boot] [PATCH 4/8] config: exynos5: Enable XHCI on all Exynos5 systems

[Vivek Gautam]

XHCI host controller is present on Exynos5250 as well as
Exynos5420, so enabling the correseponding config for
all Exynos5 systems.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 include/configs/exynos5-dt.h    |    7 +++++++
 include/configs/exynos5250-dt.h |    7 -------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/configs/exynos5-dt.h b/include/configs/exynos5-dt.h
index d684790..b2068a5 100644
--- a/include/configs/exynos5-dt.h
+++ b/include/configs/exynos5-dt.h
@@ -231,6 +231,13 @@
 #define CONFIG_SYS_I2C_S3C24X0_SLAVE    0x0
 #define CONFIG_I2C_EDID
 
+/* USB */
+#define CONFIG_CMD_USB
+#define CONFIG_USB_XHCI
+#define CONFIG_USB_XHCI_EXYNOS
+#define CONFIG_SYS_USB_XHCI_MAX_ROOT_PORTS	2
+#define CONFIG_USB_STORAGE
+
 /* SPI */
 #define CONFIG_ENV_IS_IN_SPI_FLASH
 #define CONFIG_SPI_FLASH
diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h
index 615df64..ebf5d88 100644
--- a/include/configs/exynos5250-dt.h
+++ b/include/configs/exynos5250-dt.h
@@ -20,13 +20,6 @@
 #define MACH_TYPE_SMDK5250		3774
 #define CONFIG_MACH_TYPE		MACH_TYPE_SMDK5250
 
-/* USB */
-#define CONFIG_CMD_USB
-#define CONFIG_USB_XHCI
-#define CONFIG_USB_XHCI_EXYNOS
-#define CONFIG_SYS_USB_XHCI_MAX_ROOT_PORTS	2
-#define CONFIG_USB_STORAGE
-
 /* USB boot mode */
 #define CONFIG_USB_BOOTING
 #define EXYNOS_COPY_USB_FNPTR_ADDR	0x02020070
-- 
1.7.10.4

[U-Boot] [PATCH 4/8] config: exynos5: Enable XHCI on all Exynos5 systems

[Marek Vasut]

On Monday, January 06, 2014 at 10:29:39 AM, Vivek Gautam wrote:
> XHCI host controller is present on Exynos5250 as well as
> Exynos5420, so enabling the correseponding config for
> all Exynos5 systems.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>

Reviewed-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

[U-Boot] [PATCH 5/8] usb: xhci-exynos5: Add support for multiple USB 3.0 controllers

[Vivek Gautam]

Add required support to use multiple USB 3.0 controllers available
on exynos5420 SoC.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 arch/arm/cpu/armv7/exynos/power.c        |   18 ++++--
 arch/arm/include/asm/arch-exynos/cpu.h   |   10 ++++
 arch/arm/include/asm/arch-exynos/power.h |    2 +-
 drivers/usb/host/xhci-exynos5.c          |   91 +++++++++++++++++++++---------
 drivers/usb/host/xhci.c                  |    4 --
 drivers/usb/host/xhci.h                  |    4 ++
 6 files changed, 91 insertions(+), 38 deletions(-)

diff --git a/arch/arm/cpu/armv7/exynos/power.c b/arch/arm/cpu/armv7/exynos/power.c
index 563abd7..0f8aa98 100644
--- a/arch/arm/cpu/armv7/exynos/power.c
+++ b/arch/arm/cpu/armv7/exynos/power.c
@@ -59,26 +59,34 @@ void set_usbhost_phy_ctrl(unsigned int enable)
 		exynos5_set_usbhost_phy_ctrl(enable);
 }
 
-static void exynos5_set_usbdrd_phy_ctrl(unsigned int enable)
+static void exynos5_set_usbdrd_phy_ctrl(unsigned int enable, int dev_index)
 {
 	struct exynos5_power *power =
 		(struct exynos5_power *)samsung_get_base_power();
 
+	/*
+	 * Assuming here that the DRD_PHY_CONTROL registers
+	 * are contiguous, so that :
+	 * addressof(DRD_PHY1_CONTROL) = addressof(DRD_PHY_CONTROL) + 0x4;
+	 * which is the case with exynos5420.
+	 * For exynos5250 this should work out of box, since dev_index will
+	 * always be '0' in that case
+	 */
 	if (enable) {
 		/* Enabling USBDRD_PHY */
-		setbits_le32(&power->usbdrd_phy_control,
+		setbits_le32(&power->usbdrd_phy_control + dev_index,
 				POWER_USB_DRD_PHY_CTRL_EN);
 	} else {
 		/* Disabling USBDRD_PHY */
-		clrbits_le32(&power->usbdrd_phy_control,
+		clrbits_le32(&power->usbdrd_phy_control + dev_index,
 				POWER_USB_DRD_PHY_CTRL_EN);
 	}
 }
 
-void set_usbdrd_phy_ctrl(unsigned int enable)
+void set_usbdrd_phy_ctrl(unsigned int enable, int dev_index)
 {
 	if (cpu_is_exynos5())
-		exynos5_set_usbdrd_phy_ctrl(enable);
+		exynos5_set_usbdrd_phy_ctrl(enable, dev_index);
 }
 
 static void exynos5_dp_phy_control(unsigned int enable)
diff --git a/arch/arm/include/asm/arch-exynos/cpu.h b/arch/arm/include/asm/arch-exynos/cpu.h
index 718940b..d93cba9 100644
--- a/arch/arm/include/asm/arch-exynos/cpu.h
+++ b/arch/arm/include/asm/arch-exynos/cpu.h
@@ -54,6 +54,8 @@
 #define EXYNOS4_USB_HOST_XHCI_BASE	DEVICE_NOT_AVAILABLE
 #define EXYNOS4_USB3PHY_BASE		DEVICE_NOT_AVAILABLE
 #define EXYNOS4_DMC_TZASC_BASE		DEVICE_NOT_AVAILABLE
+#define EXYNOS4_USB_HOST_XHCI_1_BASE	DEVICE_NOT_AVAILABLE
+#define EXYNOS4_USB3PHY_1_BASE		DEVICE_NOT_AVAILABLE
 
 /* EXYNOS4X12 */
 #define EXYNOS4X12_GPIO_PART3_BASE	0x03860000
@@ -93,6 +95,8 @@
 #define EXYNOS4X12_USB_HOST_XHCI_BASE	DEVICE_NOT_AVAILABLE
 #define EXYNOS4X12_USB3PHY_BASE		DEVICE_NOT_AVAILABLE
 #define EXYNOS4X12_DMC_TZASC_BASE	DEVICE_NOT_AVAILABLE
+#define EXYNOS4X12_USB_HOST_XHCI_1_BASE	DEVICE_NOT_AVAILABLE
+#define EXYNOS4X12_USB3PHY_1_BASE	DEVICE_NOT_AVAILABLE
 
 /* EXYNOS5 */
 #define EXYNOS5_I2C_SPACING		0x10000
@@ -132,6 +136,8 @@
 #define EXYNOS5_ADC_BASE		DEVICE_NOT_AVAILABLE
 #define EXYNOS5_MODEM_BASE		DEVICE_NOT_AVAILABLE
 #define EXYNOS5_DMC_TZASC_BASE		DEVICE_NOT_AVAILABLE
+#define EXYNOS5_USB_HOST_XHCI_1_BASE	DEVICE_NOT_AVAILABLE
+#define EXYNOS5_USB3PHY_1_BASE		DEVICE_NOT_AVAILABLE
 
 /* EXYNOS5420 */
 #define EXYNOS5420_AUDIOSS_BASE		0x03810000
@@ -153,6 +159,8 @@
 #define EXYNOS5420_USBPHY_BASE		0x12130000
 #define EXYNOS5420_MMC_BASE		0x12200000
 #define EXYNOS5420_SROMC_BASE		0x12250000
+#define EXYNOS5420_USB_HOST_XHCI_1_BASE	0x12400000
+#define EXYNOS5420_USB3PHY_1_BASE	0x12500000
 #define EXYNOS5420_UART_BASE		0x12C00000
 #define EXYNOS5420_I2C_BASE		0x12C60000
 #define EXYNOS5420_I2C_8910_BASE	0x12E00000
@@ -276,8 +284,10 @@ SAMSUNG_BASE(timer, PWMTIMER_BASE)
 SAMSUNG_BASE(uart, UART_BASE)
 SAMSUNG_BASE(usb_phy, USBPHY_BASE)
 SAMSUNG_BASE(usb3_phy, USB3PHY_BASE)
+SAMSUNG_BASE(usb3_phy_1, USB3PHY_1_BASE)
 SAMSUNG_BASE(usb_ehci, USB_HOST_EHCI_BASE)
 SAMSUNG_BASE(usb_xhci, USB_HOST_XHCI_BASE)
+SAMSUNG_BASE(usb_xhci_1, USB_HOST_XHCI_1_BASE)
 SAMSUNG_BASE(usb_otg, USBOTG_BASE)
 SAMSUNG_BASE(watchdog, WATCHDOG_BASE)
 SAMSUNG_BASE(power, POWER_BASE)
diff --git a/arch/arm/include/asm/arch-exynos/power.h b/arch/arm/include/asm/arch-exynos/power.h
index c9609a2..c3f2ef0 100644
--- a/arch/arm/include/asm/arch-exynos/power.h
+++ b/arch/arm/include/asm/arch-exynos/power.h
@@ -1684,7 +1684,7 @@ void set_hw_thermal_trip(void);
 #define POWER_USB_HOST_PHY_CTRL_EN		(1 << 0)
 #define POWER_USB_HOST_PHY_CTRL_DISABLE		(0 << 0)
 
-void set_usbdrd_phy_ctrl(unsigned int enable);
+void set_usbdrd_phy_ctrl(unsigned int enable, int dev_index);
 
 #define POWER_USB_DRD_PHY_CTRL_EN		(1 << 0)
 #define POWER_USB_DRD_PHY_CTRL_DISABLE		(0 << 0)
diff --git a/drivers/usb/host/xhci-exynos5.c b/drivers/usb/host/xhci-exynos5.c
index 1146d10..b5fce40 100644
--- a/drivers/usb/host/xhci-exynos5.c
+++ b/drivers/usb/host/xhci-exynos5.c
@@ -43,18 +43,30 @@ struct exynos_xhci {
 	struct fdt_gpio_state vbus_gpio;
 };
 
-static struct exynos_xhci exynos;
+static struct exynos_xhci reg_bases[CONFIG_USB_MAX_CONTROLLER_COUNT];
 
 #ifdef CONFIG_OF_CONTROL
-static int exynos_usb3_parse_dt(const void *blob, struct exynos_xhci *exynos)
+static int exynos_usb3_parse_dt(const void *blob,
+				struct exynos_xhci *base,
+				int index)
 {
 	fdt_addr_t addr;
-	unsigned int node;
-	int depth;
+	int depth, count;
+	unsigned int node = 0;
+	int nodes[CONFIG_USB_MAX_CONTROLLER_COUNT];
+
+	/* First find all the compatible nodes */
+	count = fdtdec_find_aliases_for_id(blob, "xhci",
+			COMPAT_SAMSUNG_EXYNOS5_XHCI, nodes,
+			CONFIG_USB_MAX_CONTROLLER_COUNT);
+	if (count < 0) {
+		printf("XHCI: Can't get device node for xhci\n");
+		return -ENODEV;
+	}
 
-	node = fdtdec_next_compatible(blob, 0, COMPAT_SAMSUNG_EXYNOS5_XHCI);
+	node = nodes[index];
 	if (node <= 0) {
-		debug("XHCI: Can't get device node for xhci\n");
+		printf("XHCI: Can't get device node for xhci\n");
 		return -ENODEV;
 	}
 
@@ -66,10 +78,10 @@ static int exynos_usb3_parse_dt(const void *blob, struct exynos_xhci *exynos)
 		debug("Can't get the XHCI register base address\n");
 		return -ENXIO;
 	}
-	exynos->hcd = (struct xhci_hccr *)addr;
+	base->hcd = (struct xhci_hccr *)addr;
 
 	/* Vbus gpio */
-	fdtdec_decode_gpio(blob, node, "samsung,vbus-gpio", &exynos->vbus_gpio);
+	fdtdec_decode_gpio(blob, node, "samsung,vbus-gpio", &base->vbus_gpio);
 
 	depth = 0;
 	node = fdtdec_next_compatible_subnode(blob, node,
@@ -82,9 +94,9 @@ static int exynos_usb3_parse_dt(const void *blob, struct exynos_xhci *exynos)
 	/*
 	 * Get the base address for usbphy from the device node
 	 */
-	exynos->usb3_phy = (struct exynos_usb3_phy *)fdtdec_get_addr(blob, node,
+	base->usb3_phy = (struct exynos_usb3_phy *)fdtdec_get_addr(blob, node,
 								"reg");
-	if (exynos->usb3_phy == NULL) {
+	if (base->usb3_phy == NULL) {
 		debug("Can't get the usbphy register address\n");
 		return -ENXIO;
 	}
@@ -97,9 +109,6 @@ static void exynos5_usb3_phy_init(struct exynos_usb3_phy *phy)
 {
 	u32 reg;
 
-	/* enabling usb_drd phy */
-	set_usbdrd_phy_ctrl(POWER_USB_DRD_PHY_CTRL_EN);
-
 	/* Reset USB 3.0 PHY */
 	writel(0x0, &phy->phy_reg0);
 
@@ -176,9 +185,6 @@ static void exynos5_usb3_phy_exit(struct exynos_usb3_phy *phy)
 	setbits_le32(&phy->phy_test,
 			PHYTEST_POWERDOWN_SSP |
 			PHYTEST_POWERDOWN_HSP);
-
-	/* disabling usb_drd phy */
-	set_usbdrd_phy_ctrl(POWER_USB_DRD_PHY_CTRL_DISABLE);
 }
 
 void dwc3_set_mode(struct dwc3 *dwc3_reg, u32 mode)
@@ -259,41 +265,64 @@ static int dwc3_core_init(struct dwc3 *dwc3_reg)
 	return 0;
 }
 
-static int exynos_xhci_core_init(struct exynos_xhci *exynos)
+static int exynos_xhci_core_init(struct exynos_xhci *base)
 {
 	int ret;
 
-	exynos5_usb3_phy_init(exynos->usb3_phy);
+	exynos5_usb3_phy_init(base->usb3_phy);
 
-	ret = dwc3_core_init(exynos->dwc3_reg);
+	ret = dwc3_core_init(base->dwc3_reg);
 	if (ret) {
 		debug("failed to initialize core\n");
 		return -EINVAL;
 	}
 
-	/* We are hard-coding DWC3 core to Host Mode */
-	dwc3_set_mode(exynos->dwc3_reg, DWC3_GCTL_PRTCAP_HOST);
+	/*
+	 * TODO: We are hard-coding DWC3 core to Host Mode;
+	 * when we have complete DWC3 support we may want to
+	 * have both device as well as host mode, so this will
+	 * vanish off then.
+	 */
+	dwc3_set_mode(base->dwc3_reg, DWC3_GCTL_PRTCAP_HOST);
 
 	return 0;
 }
 
-static void exynos_xhci_core_exit(struct exynos_xhci *exynos)
+static void exynos_xhci_core_exit(struct exynos_xhci *base)
 {
-	exynos5_usb3_phy_exit(exynos->usb3_phy);
+	exynos5_usb3_phy_exit(base->usb3_phy);
 }
 
 int xhci_hcd_init(int index, struct xhci_hccr **hccr, struct xhci_hcor **hcor)
 {
-	struct exynos_xhci *ctx = &exynos;
+	struct exynos_xhci *ctx = &reg_bases[index];
 	int ret;
 
 #ifdef CONFIG_OF_CONTROL
-	exynos_usb3_parse_dt(gd->fdt_blob, ctx);
+	exynos_usb3_parse_dt(gd->fdt_blob, ctx, index);
 #else
-	ctx->usb3_phy = (struct exynos_usb3_phy *)samsung_get_base_usb3_phy();
-	ctx->hcd = (struct xhci_hccr *)samsung_get_base_usb_xhci();
+	/*
+	 * right now we only have h/w with 2 controllers, so limiting the
+	 * index to two here: either 0 or 1.
+	 */
+	if (index == 0) {
+		ctx->usb3_phy = (struct exynos_usb3_phy *)
+					samsung_get_base_usb3_phy();
+		ctx->hcd = (struct xhci_hccr *)
+					samsung_get_base_usb_xhci();
+	} else if (index == 1) {
+		ctx->usb3_phy = (struct exynos_usb3_phy *)
+					samsung_get_base_usb3_phy_1();
+		ctx->hcd = (struct xhci_hccr *)
+					samsung_get_base_usb_xhci_1();
+	}
 #endif
 
+	if (!ctx->hcd || !ctx->usb3_phy) {
+		printf("XHCI: Unable to find Host controller\n");
+		return -ENODEV;
+	}
+
 	ctx->dwc3_reg = (struct dwc3 *)((char *)(ctx->hcd) + DWC3_REG_OFFSET);
 
 #ifdef CONFIG_OF_CONTROL
@@ -302,6 +331,9 @@ int xhci_hcd_init(int index, struct xhci_hccr **hccr, struct xhci_hcor **hcor)
 		gpio_direction_output(ctx->vbus_gpio.gpio, 1);
 #endif
 
+	/* Power-on usb_drd phy */
+	set_usbdrd_phy_ctrl(POWER_USB_DRD_PHY_CTRL_EN, index);
+
 	ret = exynos_xhci_core_init(ctx);
 	if (ret) {
 		puts("XHCI: failed to initialize controller\n");
@@ -321,7 +353,10 @@ int xhci_hcd_init(int index, struct xhci_hccr **hccr, struct xhci_hcor **hcor)
 
 void xhci_hcd_stop(int index)
 {
-	struct exynos_xhci *ctx = &exynos;
+	struct exynos_xhci *ctx = &reg_bases[index];
 
 	exynos_xhci_core_exit(ctx);
+
+	/* Power-off usb_drd phy */
+	set_usbdrd_phy_ctrl(POWER_USB_DRD_PHY_CTRL_DISABLE, index);
 }
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d1c2e5c..24175e6 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -30,10 +30,6 @@
 #include <asm-generic/errno.h>
 #include "xhci.h"
 
-#ifndef CONFIG_USB_MAX_CONTROLLER_COUNT
-#define CONFIG_USB_MAX_CONTROLLER_COUNT 1
-#endif
-
 static struct descriptor {
 	struct usb_hub_descriptor hub;
 	struct usb_device_descriptor device;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index ceb1573..612bf79 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -21,6 +21,10 @@
 #include <asm/io.h>
 #include <linux/list.h>
 
+#ifndef CONFIG_USB_MAX_CONTROLLER_COUNT
+#define CONFIG_USB_MAX_CONTROLLER_COUNT 1
+#endif
+
 #define upper_32_bits(n) (u32)((n) >> 32)
 #define lower_32_bits(n) (u32)(n)
 
-- 
1.7.10.4

[U-Boot] [PATCH 5/8] usb: xhci-exynos5: Add support for multiple USB 3.0 controllers

[Marek Vasut]

On Monday, January 06, 2014 at 10:29:40 AM, Vivek Gautam wrote:
> Add required support to use multiple USB 3.0 controllers available
> on exynos5420 SoC.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> ---
>  arch/arm/cpu/armv7/exynos/power.c        |   18 ++++--
>  arch/arm/include/asm/arch-exynos/cpu.h   |   10 ++++
>  arch/arm/include/asm/arch-exynos/power.h |    2 +-
>  drivers/usb/host/xhci-exynos5.c          |   91
> +++++++++++++++++++++--------- drivers/usb/host/xhci.c                  | 
>   4 --
>  drivers/usb/host/xhci.h                  |    4 ++
>  6 files changed, 91 insertions(+), 38 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/exynos/power.c
> b/arch/arm/cpu/armv7/exynos/power.c index 563abd7..0f8aa98 100644
> --- a/arch/arm/cpu/armv7/exynos/power.c
> +++ b/arch/arm/cpu/armv7/exynos/power.c
> @@ -59,26 +59,34 @@ void set_usbhost_phy_ctrl(unsigned int enable)
>  		exynos5_set_usbhost_phy_ctrl(enable);
>  }
> 
> -static void exynos5_set_usbdrd_phy_ctrl(unsigned int enable)
> +static void exynos5_set_usbdrd_phy_ctrl(unsigned int enable, int
> dev_index) {
>  	struct exynos5_power *power =
>  		(struct exynos5_power *)samsung_get_base_power();
> 
> +	/*
> +	 * Assuming here that the DRD_PHY_CONTROL registers
> +	 * are contiguous, so that :
> +	 * addressof(DRD_PHY1_CONTROL) = addressof(DRD_PHY_CONTROL) + 0x4;
> +	 * which is the case with exynos5420.
> +	 * For exynos5250 this should work out of box, since dev_index will
> +	 * always be '0' in that case
> +	 */

Why don't you convert the entry for usbdrd_phy_control into an array in the 
'struct exynos5_power' structure instead ? Then you could index it with 
dev_index.

[...]

Do you think it'd be possible to split this patch into drivers/usb/ specific 
part and CPU-specific part ? I'd like to see that to prevent merge conflicts.

[...]

Best regards,
Marek Vasut

[U-Boot] [PATCH 5/8] usb: xhci-exynos5: Add support for multiple USB 3.0 controllers

[Vivek Gautam]

On Mon, Jan 6, 2014 at 9:21 PM, Marek Vasut <marex@denx.de> wrote:
> On Monday, January 06, 2014 at 10:29:40 AM, Vivek Gautam wrote:
>> Add required support to use multiple USB 3.0 controllers available
>> on exynos5420 SoC.
>>
>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> ---
>>  arch/arm/cpu/armv7/exynos/power.c        |   18 ++++--
>>  arch/arm/include/asm/arch-exynos/cpu.h   |   10 ++++
>>  arch/arm/include/asm/arch-exynos/power.h |    2 +-
>>  drivers/usb/host/xhci-exynos5.c          |   91
>> +++++++++++++++++++++--------- drivers/usb/host/xhci.c                  |
>>   4 --
>>  drivers/usb/host/xhci.h                  |    4 ++
>>  6 files changed, 91 insertions(+), 38 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv7/exynos/power.c
>> b/arch/arm/cpu/armv7/exynos/power.c index 563abd7..0f8aa98 100644
>> --- a/arch/arm/cpu/armv7/exynos/power.c
>> +++ b/arch/arm/cpu/armv7/exynos/power.c
>> @@ -59,26 +59,34 @@ void set_usbhost_phy_ctrl(unsigned int enable)
>>               exynos5_set_usbhost_phy_ctrl(enable);
>>  }
>>
>> -static void exynos5_set_usbdrd_phy_ctrl(unsigned int enable)
>> +static void exynos5_set_usbdrd_phy_ctrl(unsigned int enable, int
>> dev_index) {
>>       struct exynos5_power *power =
>>               (struct exynos5_power *)samsung_get_base_power();
>>
>> +     /*
>> +      * Assuming here that the DRD_PHY_CONTROL registers
>> +      * are contiguous, so that :
>> +      * addressof(DRD_PHY1_CONTROL) = addressof(DRD_PHY_CONTROL) + 0x4;
>> +      * which is the case with exynos5420.
>> +      * For exynos5250 this should work out of box, since dev_index will
>> +      * always be '0' in that case
>> +      */
>
> Why don't you convert the entry for usbdrd_phy_control into an array in the
> 'struct exynos5_power' structure instead ? Then you could index it with
> dev_index.

Aah, i should have been more careful here. There's already a structure
available for exynos5420 "exynos5420_power",
which i can use. I just would need to use api "proid_is_exynos5420()"
or "proid_is_exynos5250()" to differentiate between
the two.

>
> [...]
>
> Do you think it'd be possible to split this patch into drivers/usb/ specific
> part and CPU-specific part ? I'd like to see that to prevent merge conflicts.

Ok, i'll separate out the two parts.

>
> [...]
>
> Best regards,
> Marek Vasut
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot



-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

[U-Boot] [PATCH 5/8] usb: xhci-exynos5: Add support for multiple USB 3.0 controllers

[Minkyu Kang]

Dear Vivek Gautam,

On 06/01/14 18:29, Vivek Gautam wrote:
> Add required support to use multiple USB 3.0 controllers available
> on exynos5420 SoC.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> ---
>  arch/arm/cpu/armv7/exynos/power.c        |   18 ++++--
>  arch/arm/include/asm/arch-exynos/cpu.h   |   10 ++++
>  arch/arm/include/asm/arch-exynos/power.h |    2 +-
>  drivers/usb/host/xhci-exynos5.c          |   91 +++++++++++++++++++++---------
>  drivers/usb/host/xhci.c                  |    4 --
>  drivers/usb/host/xhci.h                  |    4 ++
>  6 files changed, 91 insertions(+), 38 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/exynos/power.c b/arch/arm/cpu/armv7/exynos/power.c
> index 563abd7..0f8aa98 100644
> --- a/arch/arm/cpu/armv7/exynos/power.c
> +++ b/arch/arm/cpu/armv7/exynos/power.c
> @@ -59,26 +59,34 @@ void set_usbhost_phy_ctrl(unsigned int enable)
>  		exynos5_set_usbhost_phy_ctrl(enable);
>  }
>  
> -static void exynos5_set_usbdrd_phy_ctrl(unsigned int enable)
> +static void exynos5_set_usbdrd_phy_ctrl(unsigned int enable, int dev_index)
>  {
>  	struct exynos5_power *power =
>  		(struct exynos5_power *)samsung_get_base_power();
>  
> +	/*
> +	 * Assuming here that the DRD_PHY_CONTROL registers
> +	 * are contiguous, so that :
> +	 * addressof(DRD_PHY1_CONTROL) = addressof(DRD_PHY_CONTROL) + 0x4;
> +	 * which is the case with exynos5420.
> +	 * For exynos5250 this should work out of box, since dev_index will
> +	 * always be '0' in that case
> +	 */
>  	if (enable) {
>  		/* Enabling USBDRD_PHY */
> -		setbits_le32(&power->usbdrd_phy_control,
> +		setbits_le32(&power->usbdrd_phy_control + dev_index,
>  				POWER_USB_DRD_PHY_CTRL_EN);
>  	} else {
>  		/* Disabling USBDRD_PHY */
> -		clrbits_le32(&power->usbdrd_phy_control,
> +		clrbits_le32(&power->usbdrd_phy_control + dev_index,
>  				POWER_USB_DRD_PHY_CTRL_EN);
>  	}
>  }
>  
> -void set_usbdrd_phy_ctrl(unsigned int enable)
> +void set_usbdrd_phy_ctrl(unsigned int enable, int dev_index)
>  {
>  	if (cpu_is_exynos5())
> -		exynos5_set_usbdrd_phy_ctrl(enable);
> +		exynos5_set_usbdrd_phy_ctrl(enable, dev_index);
>  }
>  
>  static void exynos5_dp_phy_control(unsigned int enable)
> diff --git a/arch/arm/include/asm/arch-exynos/cpu.h b/arch/arm/include/asm/arch-exynos/cpu.h
> index 718940b..d93cba9 100644
> --- a/arch/arm/include/asm/arch-exynos/cpu.h
> +++ b/arch/arm/include/asm/arch-exynos/cpu.h
> @@ -54,6 +54,8 @@
>  #define EXYNOS4_USB_HOST_XHCI_BASE	DEVICE_NOT_AVAILABLE
>  #define EXYNOS4_USB3PHY_BASE		DEVICE_NOT_AVAILABLE
>  #define EXYNOS4_DMC_TZASC_BASE		DEVICE_NOT_AVAILABLE
> +#define EXYNOS4_USB_HOST_XHCI_1_BASE	DEVICE_NOT_AVAILABLE
> +#define EXYNOS4_USB3PHY_1_BASE		DEVICE_NOT_AVAILABLE
>  
>  /* EXYNOS4X12 */
>  #define EXYNOS4X12_GPIO_PART3_BASE	0x03860000
> @@ -93,6 +95,8 @@
>  #define EXYNOS4X12_USB_HOST_XHCI_BASE	DEVICE_NOT_AVAILABLE
>  #define EXYNOS4X12_USB3PHY_BASE		DEVICE_NOT_AVAILABLE
>  #define EXYNOS4X12_DMC_TZASC_BASE	DEVICE_NOT_AVAILABLE
> +#define EXYNOS4X12_USB_HOST_XHCI_1_BASE	DEVICE_NOT_AVAILABLE
> +#define EXYNOS4X12_USB3PHY_1_BASE	DEVICE_NOT_AVAILABLE
>  
>  /* EXYNOS5 */
>  #define EXYNOS5_I2C_SPACING		0x10000
> @@ -132,6 +136,8 @@
>  #define EXYNOS5_ADC_BASE		DEVICE_NOT_AVAILABLE
>  #define EXYNOS5_MODEM_BASE		DEVICE_NOT_AVAILABLE
>  #define EXYNOS5_DMC_TZASC_BASE		DEVICE_NOT_AVAILABLE
> +#define EXYNOS5_USB_HOST_XHCI_1_BASE	DEVICE_NOT_AVAILABLE
> +#define EXYNOS5_USB3PHY_1_BASE		DEVICE_NOT_AVAILABLE
>  
>  /* EXYNOS5420 */
>  #define EXYNOS5420_AUDIOSS_BASE		0x03810000
> @@ -153,6 +159,8 @@
>  #define EXYNOS5420_USBPHY_BASE		0x12130000
>  #define EXYNOS5420_MMC_BASE		0x12200000
>  #define EXYNOS5420_SROMC_BASE		0x12250000
> +#define EXYNOS5420_USB_HOST_XHCI_1_BASE	0x12400000
> +#define EXYNOS5420_USB3PHY_1_BASE	0x12500000

No. we don't have to add phy_1 and xhci_1.
It looks weird.
please access phy1 and xhci1 by offset.

>  #define EXYNOS5420_UART_BASE		0x12C00000
>  #define EXYNOS5420_I2C_BASE		0x12C60000
>  #define EXYNOS5420_I2C_8910_BASE	0x12E00000
> @@ -276,8 +284,10 @@ SAMSUNG_BASE(timer, PWMTIMER_BASE)
>  SAMSUNG_BASE(uart, UART_BASE)
>  SAMSUNG_BASE(usb_phy, USBPHY_BASE)
>  SAMSUNG_BASE(usb3_phy, USB3PHY_BASE)
> +SAMSUNG_BASE(usb3_phy_1, USB3PHY_1_BASE)
>  SAMSUNG_BASE(usb_ehci, USB_HOST_EHCI_BASE)
>  SAMSUNG_BASE(usb_xhci, USB_HOST_XHCI_BASE)
> +SAMSUNG_BASE(usb_xhci_1, USB_HOST_XHCI_1_BASE)
>  SAMSUNG_BASE(usb_otg, USBOTG_BASE)
>  SAMSUNG_BASE(watchdog, WATCHDOG_BASE)
>  SAMSUNG_BASE(power, POWER_BASE)
> diff --git a/arch/arm/include/asm/arch-exynos/power.h b/arch/arm/include/asm/arch-exynos/power.h
> index c9609a2..c3f2ef0 100644
> --- a/arch/arm/include/asm/arch-exynos/power.h
> +++ b/arch/arm/include/asm/arch-exynos/power.h
> @@ -1684,7 +1684,7 @@ void set_hw_thermal_trip(void);
>  #define POWER_USB_HOST_PHY_CTRL_EN		(1 << 0)
>  #define POWER_USB_HOST_PHY_CTRL_DISABLE		(0 << 0)
>  
> -void set_usbdrd_phy_ctrl(unsigned int enable);
> +void set_usbdrd_phy_ctrl(unsigned int enable, int dev_index);
>  
>  #define POWER_USB_DRD_PHY_CTRL_EN		(1 << 0)
>  #define POWER_USB_DRD_PHY_CTRL_DISABLE		(0 << 0)
> diff --git a/drivers/usb/host/xhci-exynos5.c b/drivers/usb/host/xhci-exynos5.c
> index 1146d10..b5fce40 100644
> --- a/drivers/usb/host/xhci-exynos5.c
> +++ b/drivers/usb/host/xhci-exynos5.c
> @@ -43,18 +43,30 @@ struct exynos_xhci {
>  	struct fdt_gpio_state vbus_gpio;
>  };
>  
> -static struct exynos_xhci exynos;
> +static struct exynos_xhci reg_bases[CONFIG_USB_MAX_CONTROLLER_COUNT];
>  
>  #ifdef CONFIG_OF_CONTROL
> -static int exynos_usb3_parse_dt(const void *blob, struct exynos_xhci *exynos)
> +static int exynos_usb3_parse_dt(const void *blob,
> +				struct exynos_xhci *base,
> +				int index)
>  {
>  	fdt_addr_t addr;
> -	unsigned int node;
> -	int depth;
> +	int depth, count;
> +	unsigned int node = 0;

= 0; unnecessary.

> +	int nodes[CONFIG_USB_MAX_CONTROLLER_COUNT];
> +
> +	/* First find all the compatible nodes */
> +	count = fdtdec_find_aliases_for_id(blob, "xhci",
> +			COMPAT_SAMSUNG_EXYNOS5_XHCI, nodes,
> +			CONFIG_USB_MAX_CONTROLLER_COUNT);
> +	if (count < 0) {
> +		printf("XHCI: Can't get device node for xhci\n");
> +		return -ENODEV;
> +	}
>  
> -	node = fdtdec_next_compatible(blob, 0, COMPAT_SAMSUNG_EXYNOS5_XHCI);
> +	node = nodes[index];
>  	if (node <= 0) {
> -		debug("XHCI: Can't get device node for xhci\n");
> +		printf("XHCI: Can't get device node for xhci\n");
>  		return -ENODEV;
>  	}
>  
> @@ -66,10 +78,10 @@ static int exynos_usb3_parse_dt(const void *blob, struct exynos_xhci *exynos)
>  		debug("Can't get the XHCI register base address\n");
>  		return -ENXIO;
>  	}
> -	exynos->hcd = (struct xhci_hccr *)addr;
> +	base->hcd = (struct xhci_hccr *)addr;
>  
>  	/* Vbus gpio */
> -	fdtdec_decode_gpio(blob, node, "samsung,vbus-gpio", &exynos->vbus_gpio);
> +	fdtdec_decode_gpio(blob, node, "samsung,vbus-gpio", &base->vbus_gpio);
>  
>  	depth = 0;
>  	node = fdtdec_next_compatible_subnode(blob, node,
> @@ -82,9 +94,9 @@ static int exynos_usb3_parse_dt(const void *blob, struct exynos_xhci *exynos)
>  	/*
>  	 * Get the base address for usbphy from the device node
>  	 */
> -	exynos->usb3_phy = (struct exynos_usb3_phy *)fdtdec_get_addr(blob, node,
> +	base->usb3_phy = (struct exynos_usb3_phy *)fdtdec_get_addr(blob, node,
>  								"reg");
> -	if (exynos->usb3_phy == NULL) {
> +	if (base->usb3_phy == NULL) {
>  		debug("Can't get the usbphy register address\n");
>  		return -ENXIO;
>  	}
> @@ -97,9 +109,6 @@ static void exynos5_usb3_phy_init(struct exynos_usb3_phy *phy)
>  {
>  	u32 reg;
>  
> -	/* enabling usb_drd phy */
> -	set_usbdrd_phy_ctrl(POWER_USB_DRD_PHY_CTRL_EN);
> -
>  	/* Reset USB 3.0 PHY */
>  	writel(0x0, &phy->phy_reg0);
>  
> @@ -176,9 +185,6 @@ static void exynos5_usb3_phy_exit(struct exynos_usb3_phy *phy)
>  	setbits_le32(&phy->phy_test,
>  			PHYTEST_POWERDOWN_SSP |
>  			PHYTEST_POWERDOWN_HSP);
> -
> -	/* disabling usb_drd phy */
> -	set_usbdrd_phy_ctrl(POWER_USB_DRD_PHY_CTRL_DISABLE);
>  }
>  
>  void dwc3_set_mode(struct dwc3 *dwc3_reg, u32 mode)
> @@ -259,41 +265,64 @@ static int dwc3_core_init(struct dwc3 *dwc3_reg)
>  	return 0;
>  }
>  
> -static int exynos_xhci_core_init(struct exynos_xhci *exynos)
> +static int exynos_xhci_core_init(struct exynos_xhci *base)
>  {
>  	int ret;
>  
> -	exynos5_usb3_phy_init(exynos->usb3_phy);
> +	exynos5_usb3_phy_init(base->usb3_phy);
>  
> -	ret = dwc3_core_init(exynos->dwc3_reg);
> +	ret = dwc3_core_init(base->dwc3_reg);
>  	if (ret) {
>  		debug("failed to initialize core\n");
>  		return -EINVAL;
>  	}
>  
> -	/* We are hard-coding DWC3 core to Host Mode */
> -	dwc3_set_mode(exynos->dwc3_reg, DWC3_GCTL_PRTCAP_HOST);
> +	/*
> +	 * TODO: We are hard-coding DWC3 core to Host Mode;
> +	 * when we have complete DWC3 support we may want to
> +	 * have both device as well as host mode, so this will
> +	 * vanish off then.
> +	 */
> +	dwc3_set_mode(base->dwc3_reg, DWC3_GCTL_PRTCAP_HOST);
>  
>  	return 0;
>  }
>  
> -static void exynos_xhci_core_exit(struct exynos_xhci *exynos)
> +static void exynos_xhci_core_exit(struct exynos_xhci *base)
>  {
> -	exynos5_usb3_phy_exit(exynos->usb3_phy);
> +	exynos5_usb3_phy_exit(base->usb3_phy);
>  }
>  
>  int xhci_hcd_init(int index, struct xhci_hccr **hccr, struct xhci_hcor **hcor)
>  {
> -	struct exynos_xhci *ctx = &exynos;
> +	struct exynos_xhci *ctx = &reg_bases[index];
>  	int ret;
>  
>  #ifdef CONFIG_OF_CONTROL
> -	exynos_usb3_parse_dt(gd->fdt_blob, ctx);
> +	exynos_usb3_parse_dt(gd->fdt_blob, ctx, index);
>  #else
> -	ctx->usb3_phy = (struct exynos_usb3_phy *)samsung_get_base_usb3_phy();
> -	ctx->hcd = (struct xhci_hccr *)samsung_get_base_usb_xhci();
> +	/*
> +	 * right now we only have h/w with 2 controllers, so limiting the
> +	 * index to two here: either 0 or 1.
> +	 */
> +	if (index == 0) {
> +		ctx->usb3_phy = (struct exynos_usb3_phy *)
> +					samsung_get_base_usb3_phy();
> +		ctx->hcd = (struct xhci_hccr *)
> +					samsung_get_base_usb_xhci();
> +	} else if (index == 1) {
> +		ctx->usb3_phy = (struct exynos_usb3_phy *)
> +					samsung_get_base_usb3_phy_1();
> +		ctx->hcd = (struct xhci_hccr *)
> +					samsung_get_base_usb_xhci_1();
> +	}
>  #endif
>  
> +	if (!ctx->hcd || !ctx->usb3_phy) {
> +		printf("XHCI: Unable to find Host controller\n");
> +		return -ENODEV;
> +	}
> +
>  	ctx->dwc3_reg = (struct dwc3 *)((char *)(ctx->hcd) + DWC3_REG_OFFSET);
>  
>  #ifdef CONFIG_OF_CONTROL
> @@ -302,6 +331,9 @@ int xhci_hcd_init(int index, struct xhci_hccr **hccr, struct xhci_hcor **hcor)
>  		gpio_direction_output(ctx->vbus_gpio.gpio, 1);
>  #endif
>  
> +	/* Power-on usb_drd phy */
> +	set_usbdrd_phy_ctrl(POWER_USB_DRD_PHY_CTRL_EN, index);
> +
>  	ret = exynos_xhci_core_init(ctx);
>  	if (ret) {
>  		puts("XHCI: failed to initialize controller\n");
> @@ -321,7 +353,10 @@ int xhci_hcd_init(int index, struct xhci_hccr **hccr, struct xhci_hcor **hcor)
>  
>  void xhci_hcd_stop(int index)
>  {
> -	struct exynos_xhci *ctx = &exynos;
> +	struct exynos_xhci *ctx = &reg_bases[index];
>  
>  	exynos_xhci_core_exit(ctx);
> +
> +	/* Power-off usb_drd phy */
> +	set_usbdrd_phy_ctrl(POWER_USB_DRD_PHY_CTRL_DISABLE, index);
>  }
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index d1c2e5c..24175e6 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -30,10 +30,6 @@
>  #include <asm-generic/errno.h>
>  #include "xhci.h"
>  
> -#ifndef CONFIG_USB_MAX_CONTROLLER_COUNT
> -#define CONFIG_USB_MAX_CONTROLLER_COUNT 1
> -#endif
> -
>  static struct descriptor {
>  	struct usb_hub_descriptor hub;
>  	struct usb_device_descriptor device;
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index ceb1573..612bf79 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -21,6 +21,10 @@
>  #include <asm/io.h>
>  #include <linux/list.h>
>  
> +#ifndef CONFIG_USB_MAX_CONTROLLER_COUNT
> +#define CONFIG_USB_MAX_CONTROLLER_COUNT 1
> +#endif
> +
>  #define upper_32_bits(n) (u32)((n) >> 32)
>  #define lower_32_bits(n) (u32)(n)
>  
> 

Thanks,
Minkyu Kang.

[U-Boot] [PATCH 5/8] usb: xhci-exynos5: Add support for multiple USB 3.0 controllers

[Vivek Gautam]

Hi Minkyu Kang,


On Tue, Jan 7, 2014 at 11:00 AM, Minkyu Kang <mk7.kang@samsung.com> wrote:
> Dear Vivek Gautam,
>
> On 06/01/14 18:29, Vivek Gautam wrote:
>> Add required support to use multiple USB 3.0 controllers available
>> on exynos5420 SoC.
>>
>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> ---
>>  arch/arm/cpu/armv7/exynos/power.c        |   18 ++++--
>>  arch/arm/include/asm/arch-exynos/cpu.h   |   10 ++++
>>  arch/arm/include/asm/arch-exynos/power.h |    2 +-
>>  drivers/usb/host/xhci-exynos5.c          |   91 +++++++++++++++++++++---------
>>  drivers/usb/host/xhci.c                  |    4 --
>>  drivers/usb/host/xhci.h                  |    4 ++
>>  6 files changed, 91 insertions(+), 38 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv7/exynos/power.c b/arch/arm/cpu/armv7/exynos/power.c
>> index 563abd7..0f8aa98 100644
>> --- a/arch/arm/cpu/armv7/exynos/power.c
>> +++ b/arch/arm/cpu/armv7/exynos/power.c
>> @@ -59,26 +59,34 @@ void set_usbhost_phy_ctrl(unsigned int enable)
>>               exynos5_set_usbhost_phy_ctrl(enable);
>>  }
>>
>> -static void exynos5_set_usbdrd_phy_ctrl(unsigned int enable)
>> +static void exynos5_set_usbdrd_phy_ctrl(unsigned int enable, int dev_index)
>>  {
>>       struct exynos5_power *power =
>>               (struct exynos5_power *)samsung_get_base_power();
>>
>> +     /*
>> +      * Assuming here that the DRD_PHY_CONTROL registers
>> +      * are contiguous, so that :
>> +      * addressof(DRD_PHY1_CONTROL) = addressof(DRD_PHY_CONTROL) + 0x4;
>> +      * which is the case with exynos5420.
>> +      * For exynos5250 this should work out of box, since dev_index will
>> +      * always be '0' in that case
>> +      */
>>       if (enable) {
>>               /* Enabling USBDRD_PHY */
>> -             setbits_le32(&power->usbdrd_phy_control,
>> +             setbits_le32(&power->usbdrd_phy_control + dev_index,
>>                               POWER_USB_DRD_PHY_CTRL_EN);
>>       } else {
>>               /* Disabling USBDRD_PHY */
>> -             clrbits_le32(&power->usbdrd_phy_control,
>> +             clrbits_le32(&power->usbdrd_phy_control + dev_index,
>>                               POWER_USB_DRD_PHY_CTRL_EN);
>>       }
>>  }
>>
>> -void set_usbdrd_phy_ctrl(unsigned int enable)
>> +void set_usbdrd_phy_ctrl(unsigned int enable, int dev_index)
>>  {
>>       if (cpu_is_exynos5())
>> -             exynos5_set_usbdrd_phy_ctrl(enable);
>> +             exynos5_set_usbdrd_phy_ctrl(enable, dev_index);
>>  }
>>
>>  static void exynos5_dp_phy_control(unsigned int enable)
>> diff --git a/arch/arm/include/asm/arch-exynos/cpu.h b/arch/arm/include/asm/arch-exynos/cpu.h
>> index 718940b..d93cba9 100644
>> --- a/arch/arm/include/asm/arch-exynos/cpu.h
>> +++ b/arch/arm/include/asm/arch-exynos/cpu.h
>> @@ -54,6 +54,8 @@
>>  #define EXYNOS4_USB_HOST_XHCI_BASE   DEVICE_NOT_AVAILABLE
>>  #define EXYNOS4_USB3PHY_BASE         DEVICE_NOT_AVAILABLE
>>  #define EXYNOS4_DMC_TZASC_BASE               DEVICE_NOT_AVAILABLE
>> +#define EXYNOS4_USB_HOST_XHCI_1_BASE DEVICE_NOT_AVAILABLE
>> +#define EXYNOS4_USB3PHY_1_BASE               DEVICE_NOT_AVAILABLE
>>
>>  /* EXYNOS4X12 */
>>  #define EXYNOS4X12_GPIO_PART3_BASE   0x03860000
>> @@ -93,6 +95,8 @@
>>  #define EXYNOS4X12_USB_HOST_XHCI_BASE        DEVICE_NOT_AVAILABLE
>>  #define EXYNOS4X12_USB3PHY_BASE              DEVICE_NOT_AVAILABLE
>>  #define EXYNOS4X12_DMC_TZASC_BASE    DEVICE_NOT_AVAILABLE
>> +#define EXYNOS4X12_USB_HOST_XHCI_1_BASE      DEVICE_NOT_AVAILABLE
>> +#define EXYNOS4X12_USB3PHY_1_BASE    DEVICE_NOT_AVAILABLE
>>
>>  /* EXYNOS5 */
>>  #define EXYNOS5_I2C_SPACING          0x10000
>> @@ -132,6 +136,8 @@
>>  #define EXYNOS5_ADC_BASE             DEVICE_NOT_AVAILABLE
>>  #define EXYNOS5_MODEM_BASE           DEVICE_NOT_AVAILABLE
>>  #define EXYNOS5_DMC_TZASC_BASE               DEVICE_NOT_AVAILABLE
>> +#define EXYNOS5_USB_HOST_XHCI_1_BASE DEVICE_NOT_AVAILABLE
>> +#define EXYNOS5_USB3PHY_1_BASE               DEVICE_NOT_AVAILABLE
>>
>>  /* EXYNOS5420 */
>>  #define EXYNOS5420_AUDIOSS_BASE              0x03810000
>> @@ -153,6 +159,8 @@
>>  #define EXYNOS5420_USBPHY_BASE               0x12130000
>>  #define EXYNOS5420_MMC_BASE          0x12200000
>>  #define EXYNOS5420_SROMC_BASE                0x12250000
>> +#define EXYNOS5420_USB_HOST_XHCI_1_BASE      0x12400000
>> +#define EXYNOS5420_USB3PHY_1_BASE    0x12500000
>
> No. we don't have to add phy_1 and xhci_1.
> It looks weird.
> please access phy1 and xhci1 by offset.

Ok, so you mean i should add offsets definitions for phy1 and xhci1
may be in the driver file or in
"arch/arm/include/asm/arch-exynos/xhci-exynos.h" file, and add these
offsets to xhci_base and
phy_base addresses ?
That should be fine, i will modify this accordingly.

>
>>  #define EXYNOS5420_UART_BASE         0x12C00000
>>  #define EXYNOS5420_I2C_BASE          0x12C60000
>>  #define EXYNOS5420_I2C_8910_BASE     0x12E00000
>> @@ -276,8 +284,10 @@ SAMSUNG_BASE(timer, PWMTIMER_BASE)
>>  SAMSUNG_BASE(uart, UART_BASE)
>>  SAMSUNG_BASE(usb_phy, USBPHY_BASE)
>>  SAMSUNG_BASE(usb3_phy, USB3PHY_BASE)
>> +SAMSUNG_BASE(usb3_phy_1, USB3PHY_1_BASE)
>>  SAMSUNG_BASE(usb_ehci, USB_HOST_EHCI_BASE)
>>  SAMSUNG_BASE(usb_xhci, USB_HOST_XHCI_BASE)
>> +SAMSUNG_BASE(usb_xhci_1, USB_HOST_XHCI_1_BASE)
>>  SAMSUNG_BASE(usb_otg, USBOTG_BASE)
>>  SAMSUNG_BASE(watchdog, WATCHDOG_BASE)
>>  SAMSUNG_BASE(power, POWER_BASE)
>> diff --git a/arch/arm/include/asm/arch-exynos/power.h b/arch/arm/include/asm/arch-exynos/power.h
>> index c9609a2..c3f2ef0 100644
>> --- a/arch/arm/include/asm/arch-exynos/power.h
>> +++ b/arch/arm/include/asm/arch-exynos/power.h
>> @@ -1684,7 +1684,7 @@ void set_hw_thermal_trip(void);
>>  #define POWER_USB_HOST_PHY_CTRL_EN           (1 << 0)
>>  #define POWER_USB_HOST_PHY_CTRL_DISABLE              (0 << 0)
>>
>> -void set_usbdrd_phy_ctrl(unsigned int enable);
>> +void set_usbdrd_phy_ctrl(unsigned int enable, int dev_index);
>>
>>  #define POWER_USB_DRD_PHY_CTRL_EN            (1 << 0)
>>  #define POWER_USB_DRD_PHY_CTRL_DISABLE               (0 << 0)
>> diff --git a/drivers/usb/host/xhci-exynos5.c b/drivers/usb/host/xhci-exynos5.c
>> index 1146d10..b5fce40 100644
>> --- a/drivers/usb/host/xhci-exynos5.c
>> +++ b/drivers/usb/host/xhci-exynos5.c
>> @@ -43,18 +43,30 @@ struct exynos_xhci {
>>       struct fdt_gpio_state vbus_gpio;
>>  };
>>
>> -static struct exynos_xhci exynos;
>> +static struct exynos_xhci reg_bases[CONFIG_USB_MAX_CONTROLLER_COUNT];
>>
>>  #ifdef CONFIG_OF_CONTROL
>> -static int exynos_usb3_parse_dt(const void *blob, struct exynos_xhci *exynos)
>> +static int exynos_usb3_parse_dt(const void *blob,
>> +                             struct exynos_xhci *base,
>> +                             int index)
>>  {
>>       fdt_addr_t addr;
>> -     unsigned int node;
>> -     int depth;
>> +     int depth, count;
>> +     unsigned int node = 0;
>
> = 0; unnecessary.
Ok, will remove this.

>
>> +     int nodes[CONFIG_USB_MAX_CONTROLLER_COUNT];
>> +
>> +     /* First find all the compatible nodes */
>> +     count = fdtdec_find_aliases_for_id(blob, "xhci",
>> +                     COMPAT_SAMSUNG_EXYNOS5_XHCI, nodes,
>> +                     CONFIG_USB_MAX_CONTROLLER_COUNT);
>> +     if (count < 0) {
>> +             printf("XHCI: Can't get device node for xhci\n");
>> +             return -ENODEV;
>> +     }
>>
>> -     node = fdtdec_next_compatible(blob, 0, COMPAT_SAMSUNG_EXYNOS5_XHCI);
>> +     node = nodes[index];
>>       if (node <= 0) {
>> -             debug("XHCI: Can't get device node for xhci\n");
>> +             printf("XHCI: Can't get device node for xhci\n");
>>               return -ENODEV;
>>       }
>>
>> @@ -66,10 +78,10 @@ static int exynos_usb3_parse_dt(const void *blob, struct exynos_xhci *exynos)
>>               debug("Can't get the XHCI register base address\n");
>>               return -ENXIO;
>>       }
>> -     exynos->hcd = (struct xhci_hccr *)addr;
>> +     base->hcd = (struct xhci_hccr *)addr;
>>
>>       /* Vbus gpio */
>> -     fdtdec_decode_gpio(blob, node, "samsung,vbus-gpio", &exynos->vbus_gpio);
>> +     fdtdec_decode_gpio(blob, node, "samsung,vbus-gpio", &base->vbus_gpio);
>>
>>       depth = 0;
>>       node = fdtdec_next_compatible_subnode(blob, node,
>> @@ -82,9 +94,9 @@ static int exynos_usb3_parse_dt(const void *blob, struct exynos_xhci *exynos)
>>       /*
>>        * Get the base address for usbphy from the device node
>>        */
>> -     exynos->usb3_phy = (struct exynos_usb3_phy *)fdtdec_get_addr(blob, node,
>> +     base->usb3_phy = (struct exynos_usb3_phy *)fdtdec_get_addr(blob, node,
>>                                                               "reg");
>> -     if (exynos->usb3_phy == NULL) {
>> +     if (base->usb3_phy == NULL) {
>>               debug("Can't get the usbphy register address\n");
>>               return -ENXIO;
>>       }
>> @@ -97,9 +109,6 @@ static void exynos5_usb3_phy_init(struct exynos_usb3_phy *phy)
>>  {
>>       u32 reg;
>>
>> -     /* enabling usb_drd phy */
>> -     set_usbdrd_phy_ctrl(POWER_USB_DRD_PHY_CTRL_EN);
>> -
>>       /* Reset USB 3.0 PHY */
>>       writel(0x0, &phy->phy_reg0);
>>
>> @@ -176,9 +185,6 @@ static void exynos5_usb3_phy_exit(struct exynos_usb3_phy *phy)
>>       setbits_le32(&phy->phy_test,
>>                       PHYTEST_POWERDOWN_SSP |
>>                       PHYTEST_POWERDOWN_HSP);
>> -
>> -     /* disabling usb_drd phy */
>> -     set_usbdrd_phy_ctrl(POWER_USB_DRD_PHY_CTRL_DISABLE);
>>  }
>>
>>  void dwc3_set_mode(struct dwc3 *dwc3_reg, u32 mode)
>> @@ -259,41 +265,64 @@ static int dwc3_core_init(struct dwc3 *dwc3_reg)
>>       return 0;
>>  }
>>
>> -static int exynos_xhci_core_init(struct exynos_xhci *exynos)
>> +static int exynos_xhci_core_init(struct exynos_xhci *base)
>>  {
>>       int ret;
>>
>> -     exynos5_usb3_phy_init(exynos->usb3_phy);
>> +     exynos5_usb3_phy_init(base->usb3_phy);
>>
>> -     ret = dwc3_core_init(exynos->dwc3_reg);
>> +     ret = dwc3_core_init(base->dwc3_reg);
>>       if (ret) {
>>               debug("failed to initialize core\n");
>>               return -EINVAL;
>>       }
>>
>> -     /* We are hard-coding DWC3 core to Host Mode */
>> -     dwc3_set_mode(exynos->dwc3_reg, DWC3_GCTL_PRTCAP_HOST);
>> +     /*
>> +      * TODO: We are hard-coding DWC3 core to Host Mode;
>> +      * when we have complete DWC3 support we may want to
>> +      * have both device as well as host mode, so this will
>> +      * vanish off then.
>> +      */
>> +     dwc3_set_mode(base->dwc3_reg, DWC3_GCTL_PRTCAP_HOST);
>>
>>       return 0;
>>  }
>>
>> -static void exynos_xhci_core_exit(struct exynos_xhci *exynos)
>> +static void exynos_xhci_core_exit(struct exynos_xhci *base)
>>  {
>> -     exynos5_usb3_phy_exit(exynos->usb3_phy);
>> +     exynos5_usb3_phy_exit(base->usb3_phy);
>>  }
>>
>>  int xhci_hcd_init(int index, struct xhci_hccr **hccr, struct xhci_hcor **hcor)
>>  {
>> -     struct exynos_xhci *ctx = &exynos;
>> +     struct exynos_xhci *ctx = &reg_bases[index];
>>       int ret;
>>
>>  #ifdef CONFIG_OF_CONTROL
>> -     exynos_usb3_parse_dt(gd->fdt_blob, ctx);
>> +     exynos_usb3_parse_dt(gd->fdt_blob, ctx, index);
>>  #else
>> -     ctx->usb3_phy = (struct exynos_usb3_phy *)samsung_get_base_usb3_phy();
>> -     ctx->hcd = (struct xhci_hccr *)samsung_get_base_usb_xhci();
>> +     /*
>> +      * right now we only have h/w with 2 controllers, so limiting the
>> +      * index to two here: either 0 or 1.
>> +      */
>> +     if (index == 0) {
>> +             ctx->usb3_phy = (struct exynos_usb3_phy *)
>> +                                     samsung_get_base_usb3_phy();
>> +             ctx->hcd = (struct xhci_hccr *)
>> +                                     samsung_get_base_usb_xhci();
>> +     } else if (index == 1) {
>> +             ctx->usb3_phy = (struct exynos_usb3_phy *)
>> +                                     samsung_get_base_usb3_phy_1();
>> +             ctx->hcd = (struct xhci_hccr *)
>> +                                     samsung_get_base_usb_xhci_1();
>> +     }
>>  #endif
>>
>> +     if (!ctx->hcd || !ctx->usb3_phy) {
>> +             printf("XHCI: Unable to find Host controller\n");
>> +             return -ENODEV;
>> +     }
>> +
>>       ctx->dwc3_reg = (struct dwc3 *)((char *)(ctx->hcd) + DWC3_REG_OFFSET);
>>
>>  #ifdef CONFIG_OF_CONTROL
>> @@ -302,6 +331,9 @@ int xhci_hcd_init(int index, struct xhci_hccr **hccr, struct xhci_hcor **hcor)
>>               gpio_direction_output(ctx->vbus_gpio.gpio, 1);
>>  #endif
>>
>> +     /* Power-on usb_drd phy */
>> +     set_usbdrd_phy_ctrl(POWER_USB_DRD_PHY_CTRL_EN, index);
>> +
>>       ret = exynos_xhci_core_init(ctx);
>>       if (ret) {
>>               puts("XHCI: failed to initialize controller\n");
>> @@ -321,7 +353,10 @@ int xhci_hcd_init(int index, struct xhci_hccr **hccr, struct xhci_hcor **hcor)
>>
>>  void xhci_hcd_stop(int index)
>>  {
>> -     struct exynos_xhci *ctx = &exynos;
>> +     struct exynos_xhci *ctx = &reg_bases[index];
>>
>>       exynos_xhci_core_exit(ctx);
>> +
>> +     /* Power-off usb_drd phy */
>> +     set_usbdrd_phy_ctrl(POWER_USB_DRD_PHY_CTRL_DISABLE, index);
>>  }
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index d1c2e5c..24175e6 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -30,10 +30,6 @@
>>  #include <asm-generic/errno.h>
>>  #include "xhci.h"
>>
>> -#ifndef CONFIG_USB_MAX_CONTROLLER_COUNT
>> -#define CONFIG_USB_MAX_CONTROLLER_COUNT 1
>> -#endif
>> -
>>  static struct descriptor {
>>       struct usb_hub_descriptor hub;
>>       struct usb_device_descriptor device;
>> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
>> index ceb1573..612bf79 100644
>> --- a/drivers/usb/host/xhci.h
>> +++ b/drivers/usb/host/xhci.h
>> @@ -21,6 +21,10 @@
>>  #include <asm/io.h>
>>  #include <linux/list.h>
>>
>> +#ifndef CONFIG_USB_MAX_CONTROLLER_COUNT
>> +#define CONFIG_USB_MAX_CONTROLLER_COUNT 1
>> +#endif
>> +
>>  #define upper_32_bits(n) (u32)((n) >> 32)
>>  #define lower_32_bits(n) (u32)(n)
>>
>>
>
> Thanks,
> Minkyu Kang.
Thanks for reviewing  :-)



-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

[U-Boot] [PATCH 5/8] usb: xhci-exynos5: Add support for multiple USB 3.0 controllers

[Marek Vasut]

On Tuesday, January 07, 2014 at 10:15:20 AM, Vivek Gautam wrote:

[...]

> >> diff --git a/arch/arm/include/asm/arch-exynos/cpu.h
> >> b/arch/arm/include/asm/arch-exynos/cpu.h index 718940b..d93cba9 100644
> >> --- a/arch/arm/include/asm/arch-exynos/cpu.h
> >> +++ b/arch/arm/include/asm/arch-exynos/cpu.h

[...]

> >> @@ -153,6 +159,8 @@
> >> 
> >>  #define EXYNOS5420_USBPHY_BASE               0x12130000
> >>  #define EXYNOS5420_MMC_BASE          0x12200000
> >>  #define EXYNOS5420_SROMC_BASE                0x12250000
> >> 
> >> +#define EXYNOS5420_USB_HOST_XHCI_1_BASE      0x12400000
> >> +#define EXYNOS5420_USB3PHY_1_BASE    0x12500000
> > 
> > No. we don't have to add phy_1 and xhci_1.
> > It looks weird.
> > please access phy1 and xhci1 by offset.
> 
> Ok, so you mean i should add offsets definitions for phy1 and xhci1
> may be in the driver file or in
> "arch/arm/include/asm/arch-exynos/xhci-exynos.h" file, and add these
> offsets to xhci_base and
> phy_base addresses ?
> That should be fine, i will modify this accordingly.
> 
> >>  #define EXYNOS5420_UART_BASE         0x12C00000
> >>  #define EXYNOS5420_I2C_BASE          0x12C60000
> >>  #define EXYNOS5420_I2C_8910_BASE     0x12E00000
> >> 
[...]

Minor rant, please trim the email when replying so it contains only the relevant 
parts. Otherwise, the irrelevant noise might cause some of the relevant replies 
to get lost.

Best regards,
Marek Vasut

[U-Boot] [PATCH 5/8] usb: xhci-exynos5: Add support for multiple USB 3.0 controllers

[Vivek Gautam]

On Tue, Jan 7, 2014 at 4:29 PM, Marek Vasut <marex@denx.de> wrote:
> On Tuesday, January 07, 2014 at 10:15:20 AM, Vivek Gautam wrote:
>
> [...]
>
> Minor rant, please trim the email when replying so it contains only the relevant
> parts. Otherwise, the irrelevant noise might cause some of the relevant replies
> to get lost.

Sure, will keep that in mind.

-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

[U-Boot] [PATCH 6/8] dts: exynos5420: Move device node for xhci 2nd controller

[Vivek Gautam]

XHCI's 2nd controller is present on all Exynos5420 machines,
so moving the corresponding node from board dts to SoC dts
for exynos5420.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 arch/arm/dts/exynos5420.dtsi              |   12 ++++++++++++
 board/samsung/dts/exynos5420-smdk5420.dts |   12 ------------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm/dts/exynos5420.dtsi b/arch/arm/dts/exynos5420.dtsi
index 02ead61..edcb46d 100644
--- a/arch/arm/dts/exynos5420.dtsi
+++ b/arch/arm/dts/exynos5420.dtsi
@@ -67,4 +67,16 @@
 		reg = <0x12E20000 0x100>;
 		interrupts = <0 203 0>;
 	};
+
+	xhci1: xhci at 12400000 {
+		compatible = "samsung,exynos5250-xhci";
+		reg = <0x12400000 0x10000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		phy {
+			compatible = "samsung,exynos5250-usb3-phy";
+			reg = <0x12500000 0x100>;
+		};
+	};
 };
diff --git a/board/samsung/dts/exynos5420-smdk5420.dts b/board/samsung/dts/exynos5420-smdk5420.dts
index d739763..958ff79 100644
--- a/board/samsung/dts/exynos5420-smdk5420.dts
+++ b/board/samsung/dts/exynos5420-smdk5420.dts
@@ -154,16 +154,4 @@
 	dmc {
 		mem-type = "ddr3";
 	};
-
-	xhci1: xhci at 12400000 {
-		compatible = "samsung,exynos5250-xhci";
-		reg = <0x12400000 0x10000>;
-		#address-cells = <1>;
-		#size-cells = <1>;
-
-		phy {
-			compatible = "samsung,exynos5250-usb3-phy";
-			reg = <0x12500000 0x100>;
-		};
-	};
 };
-- 
1.7.10.4

[U-Boot] [PATCH 6/8] dts: exynos5420: Move device node for xhci 2nd controller

[Marek Vasut]

On Monday, January 06, 2014 at 10:29:41 AM, Vivek Gautam wrote:
> XHCI's 2nd controller is present on all Exynos5420 machines,
> so moving the corresponding node from board dts to SoC dts
> for exynos5420.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>

Reviewed-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

[U-Boot] [PATCH 7/8] smdk5420: Correct the vbus gpio configuration for USB controller

[Vivek Gautam]

On smdk5420 two availbale XHCI controllers require VBUS GPIO
to be configured, the EHCI however doesn't have any such VBUS
GPIO. So correcting the available board_usb_vbus_init() function
to the needs.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 board/samsung/smdk5420/smdk5420.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/board/samsung/smdk5420/smdk5420.c b/board/samsung/smdk5420/smdk5420.c
index 3ad2ad0..4f23000 100644
--- a/board/samsung/smdk5420/smdk5420.c
+++ b/board/samsung/smdk5420/smdk5420.c
@@ -18,14 +18,15 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#ifdef CONFIG_USB_EHCI_EXYNOS
+#ifdef CONFIG_USB_XHCI_EXYNOS
 static int board_usb_vbus_init(void)
 {
-	struct exynos5_gpio_part1 *gpio1 = (struct exynos5_gpio_part1 *)
-						samsung_get_base_gpio_part1();
+	struct exynos5420_gpio_part4 *gpio4 = (struct exynos5420_gpio_part4 *)
+						samsung_get_base_gpio_part4();
 
 	/* Enable VBUS power switch */
-	s5p_gpio_direction_output(&gpio1->x2, 6, 1);
+	s5p_gpio_direction_output(&gpio4->g0, 5, 1);
+	s5p_gpio_direction_output(&gpio4->g1, 4, 1);
 
 	/* VBUS turn ON time */
 	mdelay(3);
@@ -36,7 +37,7 @@ static int board_usb_vbus_init(void)
 
 int exynos_init(void)
 {
-#ifdef CONFIG_USB_EHCI_EXYNOS
+#ifdef CONFIG_USB_XHCI_EXYNOS
 	board_usb_vbus_init();
 #endif
 	return 0;
-- 
1.7.10.4

[U-Boot] [PATCH 7/8] smdk5420: Correct the vbus gpio configuration for USB controller

[Marek Vasut]

On Monday, January 06, 2014 at 10:29:42 AM, Vivek Gautam wrote:
> On smdk5420 two availbale XHCI controllers require VBUS GPIO
> to be configured, the EHCI however doesn't have any such VBUS
> GPIO. So correcting the available board_usb_vbus_init() function
> to the needs.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> ---
>  board/samsung/smdk5420/smdk5420.c |   11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/board/samsung/smdk5420/smdk5420.c
> b/board/samsung/smdk5420/smdk5420.c index 3ad2ad0..4f23000 100644
> --- a/board/samsung/smdk5420/smdk5420.c
> +++ b/board/samsung/smdk5420/smdk5420.c
> @@ -18,14 +18,15 @@
> 
>  DECLARE_GLOBAL_DATA_PTR;
> 
> -#ifdef CONFIG_USB_EHCI_EXYNOS
> +#ifdef CONFIG_USB_XHCI_EXYNOS
>  static int board_usb_vbus_init(void)
>  {
> -	struct exynos5_gpio_part1 *gpio1 = (struct exynos5_gpio_part1 *)
> -						samsung_get_base_gpio_part1();
> +	struct exynos5420_gpio_part4 *gpio4 = (struct exynos5420_gpio_part4 *)
> +						samsung_get_base_gpio_part4();
> 
>  	/* Enable VBUS power switch */
> -	s5p_gpio_direction_output(&gpio1->x2, 6, 1);
> +	s5p_gpio_direction_output(&gpio4->g0, 5, 1);
> +	s5p_gpio_direction_output(&gpio4->g1, 4, 1);

This VBUS toggling should happen on per-port basis, right ?

Best regards,
Marek Vasut

[U-Boot] [PATCH 7/8] smdk5420: Correct the vbus gpio configuration for USB controller

[Vivek Gautam]

Hi Marek,


On Mon, Jan 6, 2014 at 9:22 PM, Marek Vasut <marex@denx.de> wrote:
> On Monday, January 06, 2014 at 10:29:42 AM, Vivek Gautam wrote:
>> On smdk5420 two availbale XHCI controllers require VBUS GPIO
>> to be configured, the EHCI however doesn't have any such VBUS
>> GPIO. So correcting the available board_usb_vbus_init() function
>> to the needs.
>>
>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> ---
>>  board/samsung/smdk5420/smdk5420.c |   11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/board/samsung/smdk5420/smdk5420.c
>> b/board/samsung/smdk5420/smdk5420.c index 3ad2ad0..4f23000 100644
>> --- a/board/samsung/smdk5420/smdk5420.c
>> +++ b/board/samsung/smdk5420/smdk5420.c
>> @@ -18,14 +18,15 @@
>>
>>  DECLARE_GLOBAL_DATA_PTR;
>>
>> -#ifdef CONFIG_USB_EHCI_EXYNOS
>> +#ifdef CONFIG_USB_XHCI_EXYNOS
>>  static int board_usb_vbus_init(void)
>>  {
>> -     struct exynos5_gpio_part1 *gpio1 = (struct exynos5_gpio_part1 *)
>> -                                             samsung_get_base_gpio_part1();
>> +     struct exynos5420_gpio_part4 *gpio4 = (struct exynos5420_gpio_part4 *)
>> +                                             samsung_get_base_gpio_part4();
>>
>>       /* Enable VBUS power switch */
>> -     s5p_gpio_direction_output(&gpio1->x2, 6, 1);
>> +     s5p_gpio_direction_output(&gpio4->g0, 5, 1);
>> +     s5p_gpio_direction_output(&gpio4->g1, 4, 1);
>
> This VBUS toggling should happen on per-port basis, right ?

No this VBUS gpio toggling is actually enabling the VBUS line of the controller.
And since we have 2 controllers on exynos5420, so added the required 2
gpios for the purpose
(GPG0[5], and GPG1[4]).

>
> Best regards,
> Marek Vasut
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot



-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

[U-Boot] [PATCH 7/8] smdk5420: Correct the vbus gpio configuration for USB controller

[Marek Vasut]

On Tuesday, January 07, 2014 at 07:37:46 AM, Vivek Gautam wrote:
> Hi Marek,
> 
> On Mon, Jan 6, 2014 at 9:22 PM, Marek Vasut <marex@denx.de> wrote:
> > On Monday, January 06, 2014 at 10:29:42 AM, Vivek Gautam wrote:
> >> On smdk5420 two availbale XHCI controllers require VBUS GPIO
> >> to be configured, the EHCI however doesn't have any such VBUS
> >> GPIO. So correcting the available board_usb_vbus_init() function
> >> to the needs.
> >> 
> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> >> ---
> >> 
> >>  board/samsung/smdk5420/smdk5420.c |   11 ++++++-----
> >>  1 file changed, 6 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/board/samsung/smdk5420/smdk5420.c
> >> b/board/samsung/smdk5420/smdk5420.c index 3ad2ad0..4f23000 100644
> >> --- a/board/samsung/smdk5420/smdk5420.c
> >> +++ b/board/samsung/smdk5420/smdk5420.c
> >> @@ -18,14 +18,15 @@
> >> 
> >>  DECLARE_GLOBAL_DATA_PTR;
> >> 
> >> -#ifdef CONFIG_USB_EHCI_EXYNOS
> >> +#ifdef CONFIG_USB_XHCI_EXYNOS
> >> 
> >>  static int board_usb_vbus_init(void)
> >>  {
> >> 
> >> -     struct exynos5_gpio_part1 *gpio1 = (struct exynos5_gpio_part1 *)
> >> -                                            
> >> samsung_get_base_gpio_part1(); +     struct exynos5420_gpio_part4
> >> *gpio4 = (struct exynos5420_gpio_part4 *) +                            
> >>                 samsung_get_base_gpio_part4();
> >> 
> >>       /* Enable VBUS power switch */
> >> 
> >> -     s5p_gpio_direction_output(&gpio1->x2, 6, 1);
> >> +     s5p_gpio_direction_output(&gpio4->g0, 5, 1);
> >> +     s5p_gpio_direction_output(&gpio4->g1, 4, 1);
> > 
> > This VBUS toggling should happen on per-port basis, right ?
> 
> No this VBUS gpio toggling is actually enabling the VBUS line of the
> controller. And since we have 2 controllers on exynos5420, so added the
> required 2 gpios for the purpose
> (GPG0[5], and GPG1[4]).

So if I understand this correctly, it should happen on per-controller basis 
then?

Best regards,
Marek Vasut

[U-Boot] [PATCH 7/8] smdk5420: Correct the vbus gpio configuration for USB controller

[Vivek Gautam]

On Tue, Jan 7, 2014 at 1:19 PM, Marek Vasut <marex@denx.de> wrote:
> On Tuesday, January 07, 2014 at 07:37:46 AM, Vivek Gautam wrote:
>> Hi Marek,
>>
>> On Mon, Jan 6, 2014 at 9:22 PM, Marek Vasut <marex@denx.de> wrote:
>> > On Monday, January 06, 2014 at 10:29:42 AM, Vivek Gautam wrote:
>> >> On smdk5420 two availbale XHCI controllers require VBUS GPIO
>> >> to be configured, the EHCI however doesn't have any such VBUS
>> >> GPIO. So correcting the available board_usb_vbus_init() function
>> >> to the needs.
>> >>
>> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> >> ---
>> >>
>> >>  board/samsung/smdk5420/smdk5420.c |   11 ++++++-----
>> >>  1 file changed, 6 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/board/samsung/smdk5420/smdk5420.c
>> >> b/board/samsung/smdk5420/smdk5420.c index 3ad2ad0..4f23000 100644
>> >> --- a/board/samsung/smdk5420/smdk5420.c
>> >> +++ b/board/samsung/smdk5420/smdk5420.c
>> >> @@ -18,14 +18,15 @@
>> >>
>> >>  DECLARE_GLOBAL_DATA_PTR;
>> >>
>> >> -#ifdef CONFIG_USB_EHCI_EXYNOS
>> >> +#ifdef CONFIG_USB_XHCI_EXYNOS
>> >>
>> >>  static int board_usb_vbus_init(void)
>> >>  {
>> >>
>> >> -     struct exynos5_gpio_part1 *gpio1 = (struct exynos5_gpio_part1 *)
>> >> -
>> >> samsung_get_base_gpio_part1(); +     struct exynos5420_gpio_part4
>> >> *gpio4 = (struct exynos5420_gpio_part4 *) +
>> >>                 samsung_get_base_gpio_part4();
>> >>
>> >>       /* Enable VBUS power switch */
>> >>
>> >> -     s5p_gpio_direction_output(&gpio1->x2, 6, 1);
>> >> +     s5p_gpio_direction_output(&gpio4->g0, 5, 1);
>> >> +     s5p_gpio_direction_output(&gpio4->g1, 4, 1);
>> >
>> > This VBUS toggling should happen on per-port basis, right ?
>>
>> No this VBUS gpio toggling is actually enabling the VBUS line of the
>> controller. And since we have 2 controllers on exynos5420, so added the
>> required 2 gpios for the purpose
>> (GPG0[5], and GPG1[4]).
>
> So if I understand this correctly, it should happen on per-controller basis
> then?

True, i thought of putting this in DT actually, similar to what we do
for exynos5250 as also available in the patch :
[PATCH 1/2] exynos5250: usb: Fix VBus gpio numbers for ehci and xhci
controllers  (http://patchwork.ozlabs.org/patch/306553/)

But then the mathematics involved in gpio_direction_output() api
doesn't yield me the correct gpio pin (which actually gave in case of
Exynos5250).
So i thought of putting it in the board file only. I should have put
proper comment here although :-(

>
> Best regards,
> Marek Vasut



-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

[U-Boot] [PATCH 7/8] smdk5420: Correct the vbus gpio configuration for USB controller

[Marek Vasut]

On Tuesday, January 07, 2014 at 10:03:01 AM, Vivek Gautam wrote:
> On Tue, Jan 7, 2014 at 1:19 PM, Marek Vasut <marex@denx.de> wrote:
> > On Tuesday, January 07, 2014 at 07:37:46 AM, Vivek Gautam wrote:
> >> Hi Marek,
> >> 
> >> On Mon, Jan 6, 2014 at 9:22 PM, Marek Vasut <marex@denx.de> wrote:
> >> > On Monday, January 06, 2014 at 10:29:42 AM, Vivek Gautam wrote:
> >> >> On smdk5420 two availbale XHCI controllers require VBUS GPIO
> >> >> to be configured, the EHCI however doesn't have any such VBUS
> >> >> GPIO. So correcting the available board_usb_vbus_init() function
> >> >> to the needs.
> >> >> 
> >> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> >> >> ---
> >> >> 
> >> >>  board/samsung/smdk5420/smdk5420.c |   11 ++++++-----
> >> >>  1 file changed, 6 insertions(+), 5 deletions(-)
> >> >> 
> >> >> diff --git a/board/samsung/smdk5420/smdk5420.c
> >> >> b/board/samsung/smdk5420/smdk5420.c index 3ad2ad0..4f23000 100644
> >> >> --- a/board/samsung/smdk5420/smdk5420.c
> >> >> +++ b/board/samsung/smdk5420/smdk5420.c
> >> >> @@ -18,14 +18,15 @@
> >> >> 
> >> >>  DECLARE_GLOBAL_DATA_PTR;
> >> >> 
> >> >> -#ifdef CONFIG_USB_EHCI_EXYNOS
> >> >> +#ifdef CONFIG_USB_XHCI_EXYNOS
> >> >> 
> >> >>  static int board_usb_vbus_init(void)
> >> >>  {
> >> >> 
> >> >> -     struct exynos5_gpio_part1 *gpio1 = (struct exynos5_gpio_part1
> >> >> *) -
> >> >> samsung_get_base_gpio_part1(); +     struct exynos5420_gpio_part4
> >> >> *gpio4 = (struct exynos5420_gpio_part4 *) +
> >> >> 
> >> >>                 samsung_get_base_gpio_part4();
> >> >>       
> >> >>       /* Enable VBUS power switch */
> >> >> 
> >> >> -     s5p_gpio_direction_output(&gpio1->x2, 6, 1);
> >> >> +     s5p_gpio_direction_output(&gpio4->g0, 5, 1);
> >> >> +     s5p_gpio_direction_output(&gpio4->g1, 4, 1);
> >> > 
> >> > This VBUS toggling should happen on per-port basis, right ?
> >> 
> >> No this VBUS gpio toggling is actually enabling the VBUS line of the
> >> controller. And since we have 2 controllers on exynos5420, so added the
> >> required 2 gpios for the purpose
> >> (GPG0[5], and GPG1[4]).
> > 
> > So if I understand this correctly, it should happen on per-controller
> > basis then?
> 
> True, i thought of putting this in DT actually, similar to what we do
> for exynos5250 as also available in the patch :
> [PATCH 1/2] exynos5250: usb: Fix VBus gpio numbers for ehci and xhci
> controllers  (http://patchwork.ozlabs.org/patch/306553/)
> 
> But then the mathematics involved in gpio_direction_output() api
> doesn't yield me the correct gpio pin (which actually gave in case of
> Exynos5250).

Ugh, does that mean we have a breakage in the GPIO or DT stuff ?
[...]

Best regards,
Marek Vasut

[U-Boot] [PATCH 8/8] config: smdk5420: Enable support for multiple XHCI controllers

[Vivek Gautam]

Update USB_MAX_CONTROLLER_COUNT to be '2' to enable support
for both usb 3.0 controllers on exynos5420.

NOTE: To use EHCI on exynos5420, this change will need to be
reverted, since there's only one EHCI controller available on
exynos5420.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 include/configs/smdk5420.h |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/configs/smdk5420.h b/include/configs/smdk5420.h
index 447f8e5..57d70bf 100644
--- a/include/configs/smdk5420.h
+++ b/include/configs/smdk5420.h
@@ -53,4 +53,7 @@
 
 #define CONFIG_MAX_I2C_NUM	11
 
+/* Multiple USB controller support */
+#define CONFIG_USB_MAX_CONTROLLER_COUNT	2
+
 #endif	/* __CONFIG_5420_H */
-- 
1.7.10.4

[U-Boot] [PATCH 8/8] config: smdk5420: Enable support for multiple XHCI controllers

[Marek Vasut]

On Monday, January 06, 2014 at 10:29:43 AM, Vivek Gautam wrote:
> Update USB_MAX_CONTROLLER_COUNT to be '2' to enable support
> for both usb 3.0 controllers on exynos5420.
> 
> NOTE: To use EHCI on exynos5420, this change will need to be
> reverted, since there's only one EHCI controller available on
> exynos5420.

OK, this I seriously dislike. Hence a question ... my feeling is the XHCI and 
EHCI code for handling multiple controllers and multiple ports is almost the 
same. Can this code be adjusted to handle both EHCI and XHCI controllers 
indiscriminately ?

This would result in having CONFIG_USB_MAX_CONTROLLER_COUNT 3 on this board and 
being able to use both EHCI and XHCI . Would this be possible for you to 
implement please ? Thanks !

Best regards,
Marek Vasut

[U-Boot] [PATCH 8/8] config: smdk5420: Enable support for multiple XHCI controllers

[Vivek Gautam]

HI Marek,


On Mon, Jan 6, 2014 at 9:24 PM, Marek Vasut <marex@denx.de> wrote:

Thanks for reviewing this. :-)

> On Monday, January 06, 2014 at 10:29:43 AM, Vivek Gautam wrote:
>> Update USB_MAX_CONTROLLER_COUNT to be '2' to enable support
>> for both usb 3.0 controllers on exynos5420.
>>
>> NOTE: To use EHCI on exynos5420, this change will need to be
>> reverted, since there's only one EHCI controller available on
>> exynos5420.
>
> OK, this I seriously dislike. Hence a question ... my feeling is the XHCI and
> EHCI code for handling multiple controllers and multiple ports is almost the
> same. Can this code be adjusted to handle both EHCI and XHCI controllers
> indiscriminately ?

The controller count is actually used by the usb core (common/usb*)
framework, right ?
There's a need to add support for multiple controller types actually
(so that EHCI
and XHCI can be enabled simultaneously). Is that what you mean here ?

>
> This would result in having CONFIG_USB_MAX_CONTROLLER_COUNT 3 on this board and
> being able to use both EHCI and XHCI . Would this be possible for you to
> implement please ? Thanks !

Ok, so you mean get all the controllers present on the board (EHCI and
XHCI) setup.
I think right now, the way USB core handles the controller count is
quite different, so
that 'CONFIG_USB_MAX_CONTROLLER_COUNT' helps in probing multiple controllers
of the same type.
I had plans to add support to use controllers of different types (EHCI
and XHCI etc) simultaneously.
But haven't really put a code for that yet.
Can you suggest some pointers about how would you like to see EHCI and
XHCI or even OHCI work
together :-)
I can take up this.

>
> Best regards,
> Marek Vasut
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot



-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

[U-Boot] [PATCH 8/8] config: smdk5420: Enable support for multiple XHCI controllers

[Marek Vasut]

On Tuesday, January 07, 2014 at 09:55:37 AM, Vivek Gautam wrote:
> HI Marek,
> 
> 
> On Mon, Jan 6, 2014 at 9:24 PM, Marek Vasut <marex@denx.de> wrote:
> 
> Thanks for reviewing this. :-)
> 
> > On Monday, January 06, 2014 at 10:29:43 AM, Vivek Gautam wrote:
> >> Update USB_MAX_CONTROLLER_COUNT to be '2' to enable support
> >> for both usb 3.0 controllers on exynos5420.
> >> 
> >> NOTE: To use EHCI on exynos5420, this change will need to be
> >> reverted, since there's only one EHCI controller available on
> >> exynos5420.
> > 
> > OK, this I seriously dislike. Hence a question ... my feeling is the XHCI
> > and EHCI code for handling multiple controllers and multiple ports is
> > almost the same. Can this code be adjusted to handle both EHCI and XHCI
> > controllers indiscriminately ?
> 
> The controller count is actually used by the usb core (common/usb*)
> framework, right ?
> There's a need to add support for multiple controller types actually
> (so that EHCI
> and XHCI can be enabled simultaneously). Is that what you mean here ?

Yes. That should not be too much of a problem though.

> > This would result in having CONFIG_USB_MAX_CONTROLLER_COUNT 3 on this
> > board and being able to use both EHCI and XHCI . Would this be possible
> > for you to implement please ? Thanks !
> 
> Ok, so you mean get all the controllers present on the board (EHCI and
> XHCI) setup.

Yes

> I think right now, the way USB core handles the controller count is
> quite different, so
> that 'CONFIG_USB_MAX_CONTROLLER_COUNT' helps in probing multiple
> controllers of the same type.

The only problem is, that both EHCI and XHCI core implement:

submit_control_msg()
submit_bulk_msg()
submit_int_msg()
usb_lowlevel_init()
usb_lowlevel_stop()

How I'd do this:
1) Rename all these submit_*() functions to usb_submit_*()
2) Rename all five usb_*() functions to ehci_*() resp. xhci_*()
3) Implement translation layer, where the common code would still call usb_*()
   and the layer would determine whether to call ehci_*() or xhci_*() call.

Surely, you'd need to change the probe() logic a bit, the controller drivers 
would have to call some "usb_register_driver()" function for each controller. 
This function would generate a new entry in a linked list of all controllers 
registered and would add a flag whether this controller is EHCI or XHCI.

Effectively, this would even remove the need for CONFIG_USB_MAX_CONTROLLER_COUNT 
at all, since you'd be tracking all the USB controllers in the USB core code and 
in a linked list.

> I had plans to add support to use controllers of different types (EHCI
> and XHCI etc) simultaneously.
> But haven't really put a code for that yet.
> Can you suggest some pointers about how would you like to see EHCI and
> XHCI or even OHCI work
> together :-)
> I can take up this.

Thank you, see above please. OHCI might be nastier, but should happen along 
these lines above as well.

Best regards,
Marek Vasut

[U-Boot] [PATCH 8/8] config: smdk5420: Enable support for multiple XHCI controllers

[Vivek Gautam]

On Tue, Jan 7, 2014 at 2:44 PM, Marek Vasut <marex@denx.de> wrote:
> On Tuesday, January 07, 2014 at 09:55:37 AM, Vivek Gautam wrote:
>> HI Marek,
>>
>>
>> On Mon, Jan 6, 2014 at 9:24 PM, Marek Vasut <marex@denx.de> wrote:
>>
>> Thanks for reviewing this. :-)
>>
>> > On Monday, January 06, 2014 at 10:29:43 AM, Vivek Gautam wrote:
>> >> Update USB_MAX_CONTROLLER_COUNT to be '2' to enable support
>> >> for both usb 3.0 controllers on exynos5420.
>> >>
>> >> NOTE: To use EHCI on exynos5420, this change will need to be
>> >> reverted, since there's only one EHCI controller available on
>> >> exynos5420.
>> >
>> > OK, this I seriously dislike. Hence a question ... my feeling is the XHCI
>> > and EHCI code for handling multiple controllers and multiple ports is
>> > almost the same. Can this code be adjusted to handle both EHCI and XHCI
>> > controllers indiscriminately ?
>>
>> The controller count is actually used by the usb core (common/usb*)
>> framework, right ?
>> There's a need to add support for multiple controller types actually
>> (so that EHCI
>> and XHCI can be enabled simultaneously). Is that what you mean here ?
>
> Yes. That should not be too much of a problem though.

True :-)

>
>> > This would result in having CONFIG_USB_MAX_CONTROLLER_COUNT 3 on this
>> > board and being able to use both EHCI and XHCI . Would this be possible
>> > for you to implement please ? Thanks !
>>
>> Ok, so you mean get all the controllers present on the board (EHCI and
>> XHCI) setup.
>
> Yes
>
>> I think right now, the way USB core handles the controller count is
>> quite different, so
>> that 'CONFIG_USB_MAX_CONTROLLER_COUNT' helps in probing multiple
>> controllers of the same type.
>
> The only problem is, that both EHCI and XHCI core implement:
>
> submit_control_msg()
> submit_bulk_msg()
> submit_int_msg()
> usb_lowlevel_init()
> usb_lowlevel_stop()

exactly.

>
> How I'd do this:
> 1) Rename all these submit_*() functions to usb_submit_*()
> 2) Rename all five usb_*() functions to ehci_*() resp. xhci_*()
This will happen in each controller drivers (ehci/xhci) which would
eventually be
called fro upper translation layer you mentioned below, right ?

> 3) Implement translation layer, where the common code would still call usb_*()
>    and the layer would determine whether to call ehci_*() or xhci_*() call.
>
> Surely, you'd need to change the probe() logic a bit, the controller drivers
> would have to call some "usb_register_driver()" function for each controller.
> This function would generate a new entry in a linked list of all controllers
> registered and would add a flag whether this controller is EHCI or XHCI.

Ok, i can understand this. Will try to put up code for this.

>
> Effectively, this would even remove the need for CONFIG_USB_MAX_CONTROLLER_COUNT
> at all, since you'd be tracking all the USB controllers in the USB core code and
> in a linked list.

Yeah, right. We won't need the controller count in this case.

>
>> I had plans to add support to use controllers of different types (EHCI
>> and XHCI etc) simultaneously.
>> But haven't really put a code for that yet.
>> Can you suggest some pointers about how would you like to see EHCI and
>> XHCI or even OHCI work
>> together :-)
>> I can take up this.
>
> Thank you, see above please. OHCI might be nastier, but should happen along
> these lines above as well.
Ok, so first i will take up EHCI and XHCI then, and later 'll see the OHCI part.

>
> Best regards,
> Marek Vasut



-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

[U-Boot] [PATCH 8/8] config: smdk5420: Enable support for multiple XHCI controllers

[Marek Vasut]

On Tuesday, January 07, 2014 at 10:37:24 AM, Vivek Gautam wrote:

[...]

> > How I'd do this:
> > 1) Rename all these submit_*() functions to usb_submit_*()
> > 2) Rename all five usb_*() functions to ehci_*() resp. xhci_*()
> 
> This will happen in each controller drivers (ehci/xhci) which would
> eventually be
> called fro upper translation layer you mentioned below, right ?

Yes. Actually, thinking of this some more, to make the transition more fine-
grained, we might go about it like this even:

1) Rename all these submit_*() functions to usb_submit_*()
 -> This one stay the same, just rename them all so they're consistent.
2) Rename all five usb_*() functions to ehci_*() resp. xhci_*()
 -> In here, we should already have a fetus of the translation layer.
    My idea here would be to implement drivers/usb/host/usb_trans.c, which
    would implement all five usb_*() functions only as a wrappers. Each of
    the five function would contain #ifdef and call the respective xhci_*()
    or ehci_*() or [ou]hci_*() implementation depending on the configuration.
    This would preserve the current behavior, which would be nice.
3) Implement real translation layer
  -> Here, the usb_trans.c would need to be extended to track all the 
     controllers in a linked list. The usb core code would not change much,
     all of the logic would live in this usb_trans.c .
  -> Here, the usb_trans.c will no longer contain any of the ifdefs from
     step 2), but it will likely turn out you would need to have ifdefs to
     prevent triggering GCC complaint about missing definition of a functions.

I don't want to impose my ideas on you too hard, so please push back if you see 
some obvious flaws or have a much better idea!

> > 3) Implement translation layer, where the common code would still call
> > usb_*()
> > 
> >    and the layer would determine whether to call ehci_*() or xhci_*()
> >    call.
> > 
> > Surely, you'd need to change the probe() logic a bit, the controller
> > drivers would have to call some "usb_register_driver()" function for
> > each controller. This function would generate a new entry in a linked
> > list of all controllers registered and would add a flag whether this
> > controller is EHCI or XHCI.
> 
> Ok, i can understand this. Will try to put up code for this.

Thank you, this is much appreciated !

> > Effectively, this would even remove the need for
> > CONFIG_USB_MAX_CONTROLLER_COUNT at all, since you'd be tracking all the
> > USB controllers in the USB core code and in a linked list.
> 
> Yeah, right. We won't need the controller count in this case.

Yes, nonexistent preprocessor symbol is a good preprocessor symbol :)

> >> I had plans to add support to use controllers of different types (EHCI
> >> and XHCI etc) simultaneously.
> >> But haven't really put a code for that yet.
> >> Can you suggest some pointers about how would you like to see EHCI and
> >> XHCI or even OHCI work
> >> together :-)
> >> I can take up this.
> > 
> > Thank you, see above please. OHCI might be nastier, but should happen
> > along these lines above as well.
> 
> Ok, so first i will take up EHCI and XHCI then, and later 'll see the OHCI
> part.

Don't forget UHCI too. I think you will need to tackle all four at once, but it 
should be mostly manual work, nothing too frightening. I think your predecessors 
rectified the API nicely already :)

Best regards,
Marek Vasut