Exynos5420: Add support for XHCI controller

'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 <[email protected]>

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

Signed-off-by Vivek Gautam <[email protected]>

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 <[email protected]>

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 <[email protected]>

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

Signed-off-by Vivek Gautam <[email protected]>

---
 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(-)

(quoted diff) +0 -0

b/arch/arm/cpu/armv7/exynos/power.c index 563abd7..0f8aa98 100644

arch/arm/cpu/armv7/exynos/power.c +1 -1

@@ -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 =

Quoted 28 lines · Vivek Gautam

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

Signed-off-by Vivek Gautam <[email protected]>

---
 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(-)

(quoted diff) +0 -0

b/arch/arm/cpu/armv7/exynos/power.c index 563abd7..0f8aa98 100644

arch/arm/cpu/armv7/exynos/power.c +1 -1

@@ -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();
 

arch/arm/cpu/armv7/exynos/power.c void set_usbhost_phy_ctrl(unsigned int enable) · line 74

Quoted 8 lines

arch/arm/cpu/armv7/exynos/power.c +8 -0

+     /*
+      * 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 mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

Signed-off-by Vivek Gautam <[email protected]>

---
 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(-)

arch/arm/cpu/armv7/exynos/power.c +13 -5

index 563abd7..0f8aa98 100644
@@ -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)

arch/arm/include/asm/arch-exynos/cpu.h +6 -0

index 718940b..d93cba9 100644
@@ -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

arch/arm/include/asm/arch-exynos/cpu.h +2 -0

 #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)

arch/arm/include/asm/arch-exynos/power.h +1 -1

index c9609a2..c3f2ef0 100644
@@ -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)

drivers/usb/host/xhci-exynos5.c +1 -1

index 1146d10..b5fce40 100644
@@ -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

drivers/usb/host/xhci-exynos5.c +57 -24

+	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);
 }

drivers/usb/host/xhci.c +0 -4

index d1c2e5c..24175e6 100644
@@ -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;

drivers/usb/host/xhci.h +0 -0

index ceb1573..612bf79 100644

Dear Vivek Gautam,

Quoted 86 lines · Vivek Gautam

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

Signed-off-by Vivek Gautam <[email protected]>

---
 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(-)

arch/arm/cpu/armv7/exynos/power.c +13 -5

index 563abd7..0f8aa98 100644
@@ -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)

arch/arm/include/asm/arch-exynos/cpu.h +6 -0

index 718940b..d93cba9 100644
@@ -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
 

arch/arm/include/asm/arch-exynos/cpu.h line 163

Quoted 8 lines

arch/arm/include/asm/arch-exynos/cpu.h +2 -0

 /* 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.

Quoted 39 lines

arch/arm/include/asm/arch-exynos/cpu.h +2 -0

 #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)

arch/arm/include/asm/arch-exynos/power.h +1 -1

index c9609a2..c3f2ef0 100644
@@ -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)

drivers/usb/host/xhci-exynos5.c +1 -1

index 1146d10..b5fce40 100644
@@ -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];
 

drivers/usb/host/xhci-exynos5.c struct exynos_xhci { · line 55

Quoted 11 lines

drivers/usb/host/xhci-exynos5.c +5 -3

 #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.

Quoted 186 lines

drivers/usb/host/xhci-exynos5.c +57 -24

+     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);
 }

drivers/usb/host/xhci.c +0 -4

index d1c2e5c..24175e6 100644
@@ -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;

drivers/usb/host/xhci.h +0 -0

index ceb1573..612bf79 100644
@@ -21,6 +21,10 @@
 #include <asm/io.h>
 #include <linux/list.h>
 

drivers/usb/host/xhci.h line 30

Quoted 8 lines

drivers/usb/host/xhci.h +4 -0

+#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.

Quoted 4 lines

Quoted 4 lines

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-exynos/cpu.h index 718940b..d93cba9 100644

arch/arm/include/asm/arch-exynos/cpu.h +0 -0

Quoted 6 lines

Quoted 6 lines

arch/arm/include/asm/arch-exynos/cpu.h +0 -0

@@ -153,6 +159,8 @@
 
 #define EXYNOS5420_USBPHY_BASE               0x12130000
 #define EXYNOS5420_MMC_BASE          0x12200000
 #define EXYNOS5420_SROMC_BASE                0x12250000
 

Quoted 6 lines

arch/arm/include/asm/arch-exynos/cpu.h line 163

Quoted 2 lines

arch/arm/include/asm/arch-exynos/cpu.h +2 -0

+#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.

Quoted 4 lines

Quoted 4 lines

arch/arm/include/asm/arch-exynos/cpu.h +0 -0

 #define EXYNOS5420_UART_BASE         0x12C00000

#define EXYNOS5420_I2C_BASE 0x12C60000
#define EXYNOS5420_I2C_8910_BASE 0x12E00000

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.

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 <[email protected]>

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 <[email protected]>

---
 board/samsung/smdk5420/smdk5420.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

(quoted diff) +0 -0

b/board/samsung/smdk5420/smdk5420.c index 3ad2ad0..4f23000 100644

board/samsung/smdk5420/smdk5420.c +1 -1

@@ -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)

Quoted 27 lines · 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 <[email protected]>

---
 board/samsung/smdk5420/smdk5420.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

(quoted diff) +0 -0

b/board/samsung/smdk5420/smdk5420.c index 3ad2ad0..4f23000 100644

board/samsung/smdk5420/smdk5420.c +3 -3

@@ -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();
 

board/samsung/smdk5420/smdk5420.c line 29

Quoted 4 lines

board/samsung/smdk5420/smdk5420.c +2 -1

      /* 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 mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Hi Marek,

Quoted 32 lines · Marek Vasut

Quoted 31 lines · 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 <[email protected]>

---

 board/samsung/smdk5420/smdk5420.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

(quoted diff) +0 -0

b/board/samsung/smdk5420/smdk5420.c index 3ad2ad0..4f23000 100644

board/samsung/smdk5420/smdk5420.c +1 -3

@@ -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();

Quoted 7 lines

board/samsung/smdk5420/smdk5420.c line 29

Quoted 5 lines

      /* Enable VBUS power switch */

board/samsung/smdk5420/smdk5420.c +2 -1

-     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]).

Quoted 43 lines · Vivek Gautam

Hi Marek,

Quoted 39 lines · Marek Vasut

Quoted 33 lines · 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 <[email protected]>

---

 board/samsung/smdk5420/smdk5420.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

(quoted diff) +0 -0

b/board/samsung/smdk5420/smdk5420.c index 3ad2ad0..4f23000 100644

board/samsung/smdk5420/smdk5420.c +1 -3

@@ -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 */

board/samsung/smdk5420/smdk5420.c line 29

Quoted 3 lines

board/samsung/smdk5420/smdk5420.c +2 -1

-     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 ?

Quoted 4 lines

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

Quoted 45 lines · Marek Vasut

Quoted 44 lines · Vivek Gautam

Hi Marek,

Quoted 40 lines · Marek Vasut

Quoted 34 lines · 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 <[email protected]>

---

 board/samsung/smdk5420/smdk5420.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

(quoted diff) +0 -0

b/board/samsung/smdk5420/smdk5420.c index 3ad2ad0..4f23000 100644

board/samsung/smdk5420/smdk5420.c +1 -2

@@ -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 */

board/samsung/smdk5420/smdk5420.c line 29

Quoted 3 lines

board/samsung/smdk5420/smdk5420.c +2 -1

-     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 ?

Quoted 7 lines

Quoted 4 lines

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).

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.

Quoted 6 lines

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 mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

HI Marek,

On Mon, Jan 6, 2014 at 9:24 PM, Marek Vasut <[email protected]> wrote:

Thanks for reviewing this. :-)

Quoted 12 lines · Vivek Gautam

Quoted 6 lines

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 ?

Quoted 3 lines

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.

Quoted 20 lines · Vivek Gautam

HI Marek,

On Mon, Jan 6, 2014 at 9:24 PM, Marek Vasut <[email protected]> wrote:

Thanks for reviewing this. :-)

Quoted 12 lines · Vivek Gautam

Quoted 6 lines

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 ?

Quoted 5 lines

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.

Quoted 7 lines

Quoted 4 lines

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

Quoted 4 lines

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.

Quoted 8 lines

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

Quoted 3 lines

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 ?

Quoted 6 lines

3) Implement translation layer, where the common code would still call usb_*()

and the layer would determine whether to call ehci_*() or xhci_*() call.

Quoted 5 lines

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.

Quoted 3 lines

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.

Quoted 8 lines

Quoted 7 lines

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.

Quoted 2 lines

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.