Skip to contentSkip to conversation

Migrate Vybrid USB driver to driver model

PATCH Superseded Sanchayan Maity 3 participants 14 messages Aug 9, 2016 – Sep 4, 2016

Subsystems (3)
Arm (32-bit) USB USB ehci driver
lore
Share & tools
lore

Sanchayan Maity

author
#0 lore raw

v2 cover Migrate Vybrid USB driver to driver model

Hello,

This is the second version of the patchset for migrating Vybrid USB driver to driver model.

Compare to the first version, this version takes care of dr_mode property and correctly handles OTG as well when gpio is specified for use as ID detection pin. This is an essential requirement for OTG as Vybrid USB controller is not a true OTG though it can be configured as either host or device. The ID pin which is unique for OTG operation is not present on Vybrid.

The problem with client that I was observing was related to sequence numbers. While trying to implement the OTG functionality I observed that if during probe of USB0 if it returns ENODEV, the probe of USB1 provides a sequence number of 0 while we expect 1. The code relies on sequence numbers for initialising the appropriate peripherals. I use the bind operation to force a sequence number. This also seems to solve the problems I was having with USB client and mentioned in the previous version of the patchset.

Host and client functionality are both functional with this patch. Patch series is based on top of latest u-boot master at the moment of this writing. Tested on Toradex Colibri Vybrid VF61 module.

Thanks to Lukasz and Stefan for their comments.

V1 Patches:
[1].
https://patchwork.ozlabs.org/patch/655370/
[2].
https://patchwork.ozlabs.org/patch/655371/
[3].
https://patchwork.ozlabs.org/patch/655372/
[4].
https://patchwork.ozlabs.org/patch/655373/

Sanchayan Maity (4):
usb: host: ehci-vf: Migrate Vybrid USB to driver model
ARM: dts: vf: Add device tree node for USB on Vybrid
ARM: dts: vf-colibri: Enable USB device tree node for Colibri Vybrid
configs: colibri_vf_defconfig: Enable USB driver model for Colibri Vybrid

 arch/arm/dts/vf-colibri.dtsi |  11 +++
 arch/arm/dts/vf.dtsi         |  14 +++
 configs/colibri_vf_defconfig |   1 +
 drivers/usb/host/ehci-vf.c   | 208 +++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 227 insertions(+), 7 deletions(-)
Signature · 1 line
2.9.2
author
#1 lore raw

PATCH v2 1/4 usb: host: ehci-vf: Migrate Vybrid USB to driver model

Add driver model support for Vybrid USB driver.

Signed-off-by Sanchayan Maity <[email protected]>
---
 drivers/usb/host/ehci-vf.c | 208 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 201 insertions(+), 7 deletions(-)
drivers/usb/host/ehci-vf.c +201 -7
index 61789dd..f6f9efb 100644
@@ -8,16 +8,20 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <usb.h>
 #include <errno.h>
 #include <linux/compiler.h>
 #include <asm/io.h>
+#include <asm-generic/gpio.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/crm_regs.h>
 #include <asm/imx-common/iomux-v3.h>
 #include <asm/imx-common/regs-usbphy.h>
 #include <usb/ehci-ci.h>
+#include <libfdt.h>
+#include <fdtdec.h>
 
 #include "ehci.h"
 

@@ -32,6 +36,8 @@
 #define UCMD_RUN_STOP		(1 << 0) /* controller run/stop */
 #define UCMD_RESET			(1 << 1) /* controller reset */
 
+DECLARE_GLOBAL_DATA_PTR;
+
 static const unsigned phy_bases[] = {
 	USB_PHY0_BASE_ADDR,
 	USB_PHY1_BASE_ADDR,

@@ -131,24 +137,39 @@ int __weak board_ehci_hcd_init(int port)
 	return 0;
 }
 
+int ehci_vf_common_init(struct usb_ehci *ehci, int index)
+{
+	int ret;
+
+	/* Do board specific initialisation */
+	ret = board_ehci_hcd_init(index);
+	if (ret)
+		return ret;
+
+	usb_power_config(index);
+	usb_oc_config(index);
+	usb_internal_phy_clock_gate(index);
+	usb_phy_enable(index, ehci);
+
+	return 0;
+}
+
+#ifndef CONFIG_DM_USB
 int ehci_hcd_init(int index, enum usb_init_type init,
 		struct ehci_hccr **hccr, struct ehci_hcor **hcor)
 {
 	struct usb_ehci *ehci;
 	enum usb_init_type type;
+	int ret;
 
 	if (index >= ARRAY_SIZE(nc_reg_bases))
 		return -EINVAL;
 
 	ehci = (struct usb_ehci *)nc_reg_bases[index];
 
-	/* Do board specific initialisation */
-	board_ehci_hcd_init(index);
-
-	usb_power_config(index);
-	usb_oc_config(index);
-	usb_internal_phy_clock_gate(index);
-	usb_phy_enable(index, ehci);
+	ret = ehci_vf_common_init(index);
+	if (ret)
+		return ret;
 
 	*hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength);
 	*hcor = (struct ehci_hcor *)((uint32_t)*hccr +

@@ -175,3 +196,176 @@ int ehci_hcd_stop(int index)
 {
 	return 0;
 }
+#else
+/* Possible port types (dual role mode) */
+enum dr_mode {
+	DR_MODE_NONE = 0,
+	DR_MODE_HOST,		/* supports host operation */
+	DR_MODE_DEVICE,		/* supports device operation */
+	DR_MODE_OTG,		/* supports both */
+};
+
+struct ehci_vf_priv_data {
+	struct ehci_ctrl ctrl;
+	struct usb_ehci *ehci;
+	struct gpio_desc cdet_gpio;
+	enum usb_init_type init_type;
+	enum dr_mode dr_mode;
+	u32 portnr;
+};
+
+static int vf_usb_ofdata_to_platdata(struct udevice *dev)
+{
+	struct ehci_vf_priv_data *priv = dev_get_priv(dev);
+	const void *dt_blob = gd->fdt_blob;
+	int node = dev->of_offset;
+	const char *mode;
+
+	priv->portnr = dev->seq;
+
+	priv->ehci = (struct usb_ehci *)dev_get_addr(dev);
+	mode = fdt_getprop(dt_blob, node, "dr_mode", NULL);
+	if (mode) {
+		if (0 == strcmp(mode, "host")) {
+			priv->dr_mode = DR_MODE_HOST;
+			priv->init_type = USB_INIT_HOST;
+		} else if (0 == strcmp(mode, "peripheral")) {
+			priv->dr_mode = DR_MODE_DEVICE;
+			priv->init_type = USB_INIT_DEVICE;
+		} else if (0 == strcmp(mode, "otg")) {
+			priv->dr_mode = DR_MODE_OTG;
+			/*
+			 * We set init_type to device by default when OTG
+			 * mode is requested. If a valid gpio is provided
+			 * we will switch the init_type based on the state
+			 * of the gpio pin.
+			 */
+			priv->init_type = USB_INIT_DEVICE;
+		} else {
+			debug("%s: Cannot decode dr_mode '%s'\n",
+			      __func__, mode);
+			return -EINVAL;
+		}
+	} else {
+		priv->dr_mode = DR_MODE_HOST;
+		priv->init_type = USB_INIT_HOST;
+	}
+
+	if (priv->dr_mode == DR_MODE_OTG) {
+		gpio_request_by_name_nodev(dt_blob, node, "fsl,cdet-gpio", 0,
+					   &priv->cdet_gpio, GPIOD_IS_IN);
+		if (dm_gpio_is_valid(&priv->cdet_gpio)) {
+			if (dm_gpio_get_value(&priv->cdet_gpio))
+				priv->init_type = USB_INIT_DEVICE;
+			else
+				priv->init_type = USB_INIT_HOST;
+		}
+	}
+
+	return 0;
+}
+
+static int vf_init_after_reset(struct ehci_ctrl *dev)
+{
+	struct ehci_vf_priv_data *priv = dev->priv;
+	enum usb_init_type type = priv->init_type;
+	struct usb_ehci *ehci = priv->ehci;
+	int ret;
+
+	ret = ehci_vf_common_init(priv->ehci, priv->portnr);
+	if (ret)
+		return ret;
+
+	if (type == USB_INIT_DEVICE)
+		return 0;
+
+	setbits_le32(&ehci->usbmode, CM_HOST);
+	writel((PORT_PTS_UTMI | PORT_PTS_PTW), &ehci->portsc);
+	setbits_le32(&ehci->portsc, USB_EN);
+
+	mdelay(10);
+
+	return 0;
+}
+
+static const struct ehci_ops vf_ehci_ops = {
+	.init_after_reset = vf_init_after_reset
+};
+
+static int vf_usb_bind(struct udevice *dev)
+{
+	static int num_controllers;
+
+	/*
+	 * Without this hack, if we return ENODEV for USB Controller 0, on
+	 * probe for the next controller, USB Controller 1 will be given a
+	 * sequence number of 0. This conflicts with our requirement of
+	 * sequence numbers while initialising the peripherals.
+	 */
+	dev->req_seq = num_controllers;
+	num_controllers++;
+
+	return 0;
+}
+
+static int ehci_usb_probe(struct udevice *dev)
+{
+	struct usb_platdata *plat = dev_get_platdata(dev);
+	struct ehci_vf_priv_data *priv = dev_get_priv(dev);
+	struct usb_ehci *ehci = priv->ehci;
+	struct ehci_hccr *hccr;
+	struct ehci_hcor *hcor;
+	int ret;
+
+	ret = ehci_vf_common_init(ehci, priv->portnr);
+	if (ret)
+		return ret;
+
+	if (priv->init_type != plat->init_type)
+		return -ENODEV;
+
+	if (priv->init_type == USB_INIT_HOST) {
+		setbits_le32(&ehci->usbmode, CM_HOST);
+		writel((PORT_PTS_UTMI | PORT_PTS_PTW), &ehci->portsc);
+		setbits_le32(&ehci->portsc, USB_EN);
+	}
+
+	mdelay(10);
+
+	hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength);
+	hcor = (struct ehci_hcor *)((uint32_t)hccr +
+				HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
+
+	return ehci_register(dev, hccr, hcor, &vf_ehci_ops, 0, priv->init_type);
+}
+
+static int ehci_usb_remove(struct udevice *dev)
+{
+	int ret;
+
+	ret = ehci_deregister(dev);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct udevice_id vf_usb_ids[] = {
+	{ .compatible = "fsl,vf610-usb" },
+	{ }
+};
+
+U_BOOT_DRIVER(usb_ehci) = {
+	.name = "ehci_vf",
+	.id = UCLASS_USB,
+	.of_match = vf_usb_ids,
+	.bind = vf_usb_bind,
+	.probe = ehci_usb_probe,
+	.remove = ehci_usb_remove,
+	.ops = &ehci_usb_ops,
+	.ofdata_to_platdata = vf_usb_ofdata_to_platdata,
+	.platdata_auto_alloc_size = sizeof(struct usb_platdata),
+	.priv_auto_alloc_size = sizeof(struct ehci_vf_priv_data),
+	.flags = DM_FLAG_ALLOC_PRIV_DMA,
+};
+#endif
Signature · 1 line
2.9.2
author
#3 lore raw

PATCH v2 2/4 ARM: dts: vf: Add device tree node for USB on Vybrid

In reply to Sanchayan Maity

Add device tree node for USB peripheral on Vybrid.

Signed-off-by Sanchayan Maity <[email protected]>
---
 arch/arm/dts/vf.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
arch/arm/dts/vf.dtsi +14 -0
index 1530d2f..d7d21a3 100644
@@ -4,6 +4,7 @@
  * SPDX-License-Identifier:     GPL-2.0+ or X11
  */
 /include/ "skeleton.dtsi"
+#include <dt-bindings/gpio/gpio.h>
 
 / {
 	aliases {

@@ -20,6 +21,8 @@
 		serial5 = &uart5;
 		spi0 = &dspi0;
 		spi1 = &dspi1;
+		ehci0 = &ehci0;
+		ehci1 = &ehci1;
 	};
 
 	soc {

@@ -113,6 +116,12 @@
 				reg = <0x400ff100 0x40>;
 				#gpio-cells = <2>;
 			};
+
+			ehci0: ehci at 40034000 {
+				compatible = "fsl,vf610-usb";
+				reg = <0x40034000 0x800>;
+				status = "disabled";
+			};
 		};
 
 		aips1: aips-bus at 40080000 {

@@ -133,6 +142,11 @@
 				status = "disabled";
 			};
 
+			ehci1: ehci at 400b4000 {
+				compatible = "fsl,vf610-usb";
+				reg = <0x400b4000 0x800>;
+				status = "disabled";
+			};
 		};
 	};
 };
Signature · 1 line
2.9.2
author
#5 lore raw

PATCH v2 3/4 ARM: dts: vf-colibri: Enable USB device tree node for Colibri Vybrid

In reply to Sanchayan Maity

Enable USB device tree node for Toradex Colibri Vybrid module.

Signed-off-by Sanchayan Maity <[email protected]>
---
 arch/arm/dts/vf-colibri.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)
arch/arm/dts/vf-colibri.dtsi +11 -0
index dc52748..c2f104a 100644
@@ -21,6 +21,17 @@
 	};
 };
 
+&ehci0 {
+	dr_mode = "otg";
+	fsl,cdet-gpio = <&gpio3 6 GPIO_ACTIVE_HIGH>;
+	status = "okay";
+};
+
+&ehci1 {
+	dr_mode = "host";
+	status = "okay";
+};
+
 &uart0 {
 	status = "okay";
 };
Signature · 1 line
2.9.2
author
#7 lore raw

PATCH v2 4/4 configs: colibri_vf_defconfig: Enable USB driver model for Colibri Vybrid

In reply to Sanchayan Maity

Enable USB driver model for Toradex Colibri Vybrid modules.

Signed-off-by Sanchayan Maity <[email protected]>
---
 configs/colibri_vf_defconfig | 1 +
 1 file changed, 1 insertion(+)
configs/colibri_vf_defconfig +1 -0
index 986cec4..5017c7d 100644
@@ -33,6 +33,7 @@ CONFIG_SYS_NAND_VF610_NFC_60_ECC_BYTES=y
 CONFIG_FSL_LPUART=y
 CONFIG_FSL_DSPI=y
 CONFIG_USB=y
+CONFIG_DM_USB=y
 CONFIG_USB_GADGET=y
 CONFIG_CI_UDC=y
 CONFIG_USB_GADGET_DOWNLOAD=y
Signature · 1 line
2.9.2
author
#9 lore raw

v2 cover Migrate Vybrid USB driver to driver model

In reply to Sanchayan Maity

Hello,

Ping?

Regards,
Sanchayan.

Quoted 52 lines · Sanchayan Maity

Hello,

This is the second version of the patchset for migrating Vybrid USB driver to driver model.

Compare to the first version, this version takes care of dr_mode property and correctly handles OTG as well when gpio is specified for use as ID detection pin. This is an essential requirement for OTG as Vybrid USB controller is not a true OTG though it can be configured as either host or device. The ID pin which is unique for OTG operation is not present on Vybrid.

The problem with client that I was observing was related to sequence numbers. While trying to implement the OTG functionality I observed that if during probe of USB0 if it returns ENODEV, the probe of USB1 provides a sequence number of 0 while we expect 1. The code relies on sequence numbers for initialising the appropriate peripherals. I use the bind operation to force a sequence number. This also seems to solve the problems I was having with USB client and mentioned in the previous version of the patchset.

Host and client functionality are both functional with this patch. Patch series is based on top of latest u-boot master at the moment of this writing. Tested on Toradex Colibri Vybrid VF61 module.

Thanks to Lukasz and Stefan for their comments.

V1 Patches:
[1].
https://patchwork.ozlabs.org/patch/655370/
[2].
https://patchwork.ozlabs.org/patch/655371/
[3].
https://patchwork.ozlabs.org/patch/655372/
[4].
https://patchwork.ozlabs.org/patch/655373/

Sanchayan Maity (4):
usb: host: ehci-vf: Migrate Vybrid USB to driver model
ARM: dts: vf: Add device tree node for USB on Vybrid
ARM: dts: vf-colibri: Enable USB device tree node for Colibri Vybrid
configs: colibri_vf_defconfig: Enable USB driver model for Colibri Vybrid

 arch/arm/dts/vf-colibri.dtsi |  11 +++
 arch/arm/dts/vf.dtsi         |  14 +++
 configs/colibri_vf_defconfig |   1 +
 drivers/usb/host/ehci-vf.c   | 208 +++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 227 insertions(+), 7 deletions(-)
Signature · 1 line
2.9.2
#10 lore raw

v2 cover Migrate Vybrid USB driver to driver model

In reply to Sanchayan Maity
Quoted 43 lines · Sanchayan Maity

Hello,

This is the second version of the patchset for migrating Vybrid USB driver to driver model.

Compare to the first version, this version takes care of dr_mode property and correctly handles OTG as well when gpio is specified for use as ID detection pin. This is an essential requirement for OTG as Vybrid USB controller is not a true OTG though it can be configured as either host or device. The ID pin which is unique for OTG operation is not present on Vybrid.

The problem with client that I was observing was related to sequence numbers. While trying to implement the OTG functionality I observed that if during probe of USB0 if it returns ENODEV, the probe of USB1 provides a sequence number of 0 while we expect 1. The code relies on sequence numbers for initialising the appropriate peripherals. I use the bind operation to force a sequence number. This also seems to solve the problems I was having with USB client and mentioned in the previous version of the patchset.

Host and client functionality are both functional with this patch. Patch series is based on top of latest u-boot master at the moment of this writing. Tested on Toradex Colibri Vybrid VF61 module.

Thanks to Lukasz and Stefan for their comments.

V1 Patches:
[1].
https://patchwork.ozlabs.org/patch/655370/
[2].
https://patchwork.ozlabs.org/patch/655371/
[3].
https://patchwork.ozlabs.org/patch/655372/
[4].
https://patchwork.ozlabs.org/patch/655373/

Sanchayan Maity (4):
usb: host: ehci-vf: Migrate Vybrid USB to driver model
ARM: dts: vf: Add device tree node for USB on Vybrid
ARM: dts: vf-colibri: Enable USB device tree node for Colibri Vybrid
configs: colibri_vf_defconfig: Enable USB driver model for Colibri Vybrid

Quoted 6 lines
 arch/arm/dts/vf-colibri.dtsi |  11 +++
 arch/arm/dts/vf.dtsi         |  14 +++
 configs/colibri_vf_defconfig |   1 +
 drivers/usb/host/ehci-vf.c   | 208 +++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 227 insertions(+), 7 deletions(-)

Looks OK to me, Simon, since this is DM, can you review it ?

Signature · 2 lines
Best regards,
Marek Vasut
author
#11 lore raw

v2 cover Migrate Vybrid USB driver to driver model

Quoted 44 lines · Marek Vasut
Quoted 43 lines · Sanchayan Maity

Hello,

This is the second version of the patchset for migrating Vybrid USB driver to driver model.

Compare to the first version, this version takes care of dr_mode property and correctly handles OTG as well when gpio is specified for use as ID detection pin. This is an essential requirement for OTG as Vybrid USB controller is not a true OTG though it can be configured as either host or device. The ID pin which is unique for OTG operation is not present on Vybrid.

The problem with client that I was observing was related to sequence numbers. While trying to implement the OTG functionality I observed that if during probe of USB0 if it returns ENODEV, the probe of USB1 provides a sequence number of 0 while we expect 1. The code relies on sequence numbers for initialising the appropriate peripherals. I use the bind operation to force a sequence number. This also seems to solve the problems I was having with USB client and mentioned in the previous version of the patchset.

Host and client functionality are both functional with this patch. Patch series is based on top of latest u-boot master at the moment of this writing. Tested on Toradex Colibri Vybrid VF61 module.

Thanks to Lukasz and Stefan for their comments.

V1 Patches:
[1].
https://patchwork.ozlabs.org/patch/655370/
[2].
https://patchwork.ozlabs.org/patch/655371/
[3].
https://patchwork.ozlabs.org/patch/655372/
[4].
https://patchwork.ozlabs.org/patch/655373/

Sanchayan Maity (4):
usb: host: ehci-vf: Migrate Vybrid USB to driver model
ARM: dts: vf: Add device tree node for USB on Vybrid
ARM: dts: vf-colibri: Enable USB device tree node for Colibri Vybrid
configs: colibri_vf_defconfig: Enable USB driver model for Colibri Vybrid

Quoted 9 lines
Quoted 6 lines
 arch/arm/dts/vf-colibri.dtsi |  11 +++
 arch/arm/dts/vf.dtsi         |  14 +++
 configs/colibri_vf_defconfig |   1 +
 drivers/usb/host/ehci-vf.c   | 208 +++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 227 insertions(+), 7 deletions(-)

Looks OK to me, Simon, since this is DM, can you review it ?

Hello Simon,

Do you think the patchset is ok?

Thanks.

Regards,
Sanchayan.

#2 lore raw

PATCH v2 1/4 usb: host: ehci-vf: Migrate Vybrid USB to driver model

In reply to Sanchayan Maity

Hi,

Quoted 7 lines · Sanchayan Maity

Add driver model support for Vybrid USB driver.

Signed-off-by Sanchayan Maity <[email protected]>
---
 drivers/usb/host/ehci-vf.c | 208 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 201 insertions(+), 7 deletions(-)
Reviewed-by : Simon Glass <[email protected]>
Quoted 27 lines
drivers/usb/host/ehci-vf.c +4 -0
index 61789dd..f6f9efb 100644
@@ -8,16 +8,20 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <usb.h>
 #include <errno.h>
 #include <linux/compiler.h>
 #include <asm/io.h>
+#include <asm-generic/gpio.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/crm_regs.h>
 #include <asm/imx-common/iomux-v3.h>
 #include <asm/imx-common/regs-usbphy.h>
 #include <usb/ehci-ci.h>
+#include <libfdt.h>
+#include <fdtdec.h>
 
 #include "ehci.h"

drivers/usb/host/ehci-vf.c line 43

Quoted 8 lines
drivers/usb/host/ehci-vf.c +2 -0
@@ -32,6 +36,8 @@
 #define UCMD_RUN_STOP          (1 << 0) /* controller run/stop */
 #define UCMD_RESET                     (1 << 1) /* controller reset */
 
+DECLARE_GLOBAL_DATA_PTR;
+
 static const unsigned phy_bases[] = {
        USB_PHY0_BASE_ADDR,
        USB_PHY1_BASE_ADDR,

This should be converted to DT, perhaps in a follow-up patch.

drivers/usb/host/ehci-vf.c int __weak board_ehci_hcd_init(int port) · line 144

Quoted 9 lines
drivers/usb/host/ehci-vf.c +5 -0
@@ -131,24 +137,39 @@ int __weak board_ehci_hcd_init(int port)
        return 0;
 }
 
+int ehci_vf_common_init(struct usb_ehci *ehci, int index)
+{
+       int ret;
+
+       /* Do board specific initialisation */

nit: board-specific

Quoted 63 lines
drivers/usb/host/ehci-vf.c +38 -7
+       ret = board_ehci_hcd_init(index);
+       if (ret)
+               return ret;
+
+       usb_power_config(index);
+       usb_oc_config(index);
+       usb_internal_phy_clock_gate(index);
+       usb_phy_enable(index, ehci);
+
+       return 0;
+}
+
+#ifndef CONFIG_DM_USB
 int ehci_hcd_init(int index, enum usb_init_type init,
                struct ehci_hccr **hccr, struct ehci_hcor **hcor)
 {
        struct usb_ehci *ehci;
        enum usb_init_type type;
+       int ret;
 
        if (index >= ARRAY_SIZE(nc_reg_bases))
                return -EINVAL;
 
        ehci = (struct usb_ehci *)nc_reg_bases[index];
 
-       /* Do board specific initialisation */
-       board_ehci_hcd_init(index);
-
-       usb_power_config(index);
-       usb_oc_config(index);
-       usb_internal_phy_clock_gate(index);
-       usb_phy_enable(index, ehci);
+       ret = ehci_vf_common_init(index);
+       if (ret)
+               return ret;
 
        *hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength);
        *hcor = (struct ehci_hcor *)((uint32_t)*hccr +

@@ -175,3 +196,176 @@ int ehci_hcd_stop(int index)
 {
        return 0;
 }
+#else
+/* Possible port types (dual role mode) */
+enum dr_mode {
+       DR_MODE_NONE = 0,
+       DR_MODE_HOST,           /* supports host operation */
+       DR_MODE_DEVICE,         /* supports device operation */
+       DR_MODE_OTG,            /* supports both */
+};
+
+struct ehci_vf_priv_data {
+       struct ehci_ctrl ctrl;
+       struct usb_ehci *ehci;
+       struct gpio_desc cdet_gpio;
+       enum usb_init_type init_type;
+       enum dr_mode dr_mode;
+       u32 portnr;
+};
+
+static int vf_usb_ofdata_to_platdata(struct udevice *dev)
+{
+       struct ehci_vf_priv_data *priv = dev_get_priv(dev);

drivers/usb/host/ehci-vf.c int ehci_hcd_stop(int index) · line 229

Quoted 10 lines
drivers/usb/host/ehci-vf.c +10 -0
+       const void *dt_blob = gd->fdt_blob;
+       int node = dev->of_offset;
+       const char *mode;
+
+       priv->portnr = dev->seq;
+
+       priv->ehci = (struct usb_ehci *)dev_get_addr(dev);
+       mode = fdt_getprop(dt_blob, node, "dr_mode", NULL);
+       if (mode) {
+               if (0 == strcmp(mode, "host")) {

You can say

if (!strcmp...

if you like. Up to you.

Quoted 67 lines
drivers/usb/host/ehci-vf.c +67 -0
+                       priv->dr_mode = DR_MODE_HOST;
+                       priv->init_type = USB_INIT_HOST;
+               } else if (0 == strcmp(mode, "peripheral")) {
+                       priv->dr_mode = DR_MODE_DEVICE;
+                       priv->init_type = USB_INIT_DEVICE;
+               } else if (0 == strcmp(mode, "otg")) {
+                       priv->dr_mode = DR_MODE_OTG;
+                       /*
+                        * We set init_type to device by default when OTG
+                        * mode is requested. If a valid gpio is provided
+                        * we will switch the init_type based on the state
+                        * of the gpio pin.
+                        */
+                       priv->init_type = USB_INIT_DEVICE;
+               } else {
+                       debug("%s: Cannot decode dr_mode '%s'\n",
+                             __func__, mode);
+                       return -EINVAL;
+               }
+       } else {
+               priv->dr_mode = DR_MODE_HOST;
+               priv->init_type = USB_INIT_HOST;
+       }
+
+       if (priv->dr_mode == DR_MODE_OTG) {
+               gpio_request_by_name_nodev(dt_blob, node, "fsl,cdet-gpio", 0,
+                                          &priv->cdet_gpio, GPIOD_IS_IN);
+               if (dm_gpio_is_valid(&priv->cdet_gpio)) {
+                       if (dm_gpio_get_value(&priv->cdet_gpio))
+                               priv->init_type = USB_INIT_DEVICE;
+                       else
+                               priv->init_type = USB_INIT_HOST;
+               }
+       }
+
+       return 0;
+}
+
+static int vf_init_after_reset(struct ehci_ctrl *dev)
+{
+       struct ehci_vf_priv_data *priv = dev->priv;
+       enum usb_init_type type = priv->init_type;
+       struct usb_ehci *ehci = priv->ehci;
+       int ret;
+
+       ret = ehci_vf_common_init(priv->ehci, priv->portnr);
+       if (ret)
+               return ret;
+
+       if (type == USB_INIT_DEVICE)
+               return 0;
+
+       setbits_le32(&ehci->usbmode, CM_HOST);
+       writel((PORT_PTS_UTMI | PORT_PTS_PTW), &ehci->portsc);
+       setbits_le32(&ehci->portsc, USB_EN);
+
+       mdelay(10);
+
+       return 0;
+}
+
+static const struct ehci_ops vf_ehci_ops = {
+       .init_after_reset = vf_init_after_reset
+};
+
+static int vf_usb_bind(struct udevice *dev)
+{

drivers/usb/host/ehci-vf.c int ehci_hcd_stop(int index) · line 306

Quoted 10 lines
drivers/usb/host/ehci-vf.c +10 -0
+       static int num_controllers;
+
+       /*
+        * Without this hack, if we return ENODEV for USB Controller 0, on
+        * probe for the next controller, USB Controller 1 will be given a
+        * sequence number of 0. This conflicts with our requirement of
+        * sequence numbers while initialising the peripherals.
+        */
+       dev->req_seq = num_controllers;
+       num_controllers++;

Yes, but if you use DT you can define the number using an alias.

Quoted 9 lines
drivers/usb/host/ehci-vf.c +9 -0
+
+       return 0;
+}
+
+static int ehci_usb_probe(struct udevice *dev)
+{
+       struct usb_platdata *plat = dev_get_platdata(dev);
+       struct ehci_vf_priv_data *priv = dev_get_priv(dev);
+       struct usb_ehci *ehci = priv->ehci;

drivers/usb/host/ehci-vf.c int ehci_hcd_stop(int index) · line 325

Quoted 10 lines
drivers/usb/host/ehci-vf.c +10 -0
+       struct ehci_hccr *hccr;
+       struct ehci_hcor *hcor;
+       int ret;
+
+       ret = ehci_vf_common_init(ehci, priv->portnr);
+       if (ret)
+               return ret;
+
+       if (priv->init_type != plat->init_type)
+               return -ENODEV;

-ENODEV means that there is no device. I think -EINVAL or -EPERM or something else would be better.

drivers/usb/host/ehci-vf.c int ehci_hcd_stop(int index) · line 333

Quoted 8 lines
drivers/usb/host/ehci-vf.c +8 -0
+
+       if (priv->init_type == USB_INIT_HOST) {
+               setbits_le32(&ehci->usbmode, CM_HOST);
+               writel((PORT_PTS_UTMI | PORT_PTS_PTW), &ehci->portsc);
+               setbits_le32(&ehci->portsc, USB_EN);
+       }
+
+       mdelay(10);

Why? Please add a comment. That's a really long time.

Quoted 39 lines
drivers/usb/host/ehci-vf.c +38 -1
+
+       hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength);
+       hcor = (struct ehci_hcor *)((uint32_t)hccr +
+                               HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
+
+       return ehci_register(dev, hccr, hcor, &vf_ehci_ops, 0, priv->init_type);
+}
+
+static int ehci_usb_remove(struct udevice *dev)
+{
+       int ret;
+
+       ret = ehci_deregister(dev);
+       if (ret)
+               return ret;
+
+       return 0;
+}
+
+static const struct udevice_id vf_usb_ids[] = {
+       { .compatible = "fsl,vf610-usb" },
+       { }
+};
+
+U_BOOT_DRIVER(usb_ehci) = {
+       .name = "ehci_vf",
+       .id = UCLASS_USB,
+       .of_match = vf_usb_ids,
+       .bind = vf_usb_bind,
+       .probe = ehci_usb_probe,
+       .remove = ehci_usb_remove,
+       .ops = &ehci_usb_ops,
+       .ofdata_to_platdata = vf_usb_ofdata_to_platdata,
+       .platdata_auto_alloc_size = sizeof(struct usb_platdata),
+       .priv_auto_alloc_size = sizeof(struct ehci_vf_priv_data),
+       .flags = DM_FLAG_ALLOC_PRIV_DMA,
+};
+#endif
--
Quoted 1 line

2.9.2

Regards,
Simon

#12 lore raw

v2 cover Migrate Vybrid USB driver to driver model

In reply to maitysanchayan at gmail.com

Hi,

Quoted 52 lines · On 1 September 2016 at 03:49, <[email protected]> wrote:
Quoted 51 lines · Marek Vasut
Quoted 49 lines · Sanchayan Maity

Hello,

This is the second version of the patchset for migrating Vybrid USB driver to driver model.

Compare to the first version, this version takes care of dr_mode property and correctly handles OTG as well when gpio is specified for use as ID detection pin. This is an essential requirement for OTG as Vybrid USB controller is not a true OTG though it can be configured as either host or device. The ID pin which is unique for OTG operation is not present on Vybrid.

The problem with client that I was observing was related to sequence numbers. While trying to implement the OTG functionality I observed that if during probe of USB0 if it returns ENODEV, the probe of USB1 provides a sequence number of 0 while we expect 1. The code relies on sequence numbers for initialising the appropriate peripherals. I use the bind operation to force a sequence number. This also seems to solve the problems I was having with USB client and mentioned in the previous version of the patchset.

Host and client functionality are both functional with this patch. Patch series is based on top of latest u-boot master at the moment of this writing. Tested on Toradex Colibri Vybrid VF61 module.

Thanks to Lukasz and Stefan for their comments.

V1 Patches:
[1].
https://patchwork.ozlabs.org/patch/655370/
[2].
https://patchwork.ozlabs.org/patch/655371/
[3].
https://patchwork.ozlabs.org/patch/655372/
[4].
https://patchwork.ozlabs.org/patch/655373/

Sanchayan Maity (4):
usb: host: ehci-vf: Migrate Vybrid USB to driver model
ARM: dts: vf: Add device tree node for USB on Vybrid
ARM: dts: vf-colibri: Enable USB device tree node for Colibri Vybrid
configs: colibri_vf_defconfig: Enable USB driver model for Colibri Vybrid

 arch/arm/dts/vf-colibri.dtsi |  11 +++
 arch/arm/dts/vf.dtsi         |  14 +++
 configs/colibri_vf_defconfig |   1 +
 drivers/usb/host/ehci-vf.c   | 208 +++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 227 insertions(+), 7 deletions(-)
Quoted 6 lines
Quoted 2 lines

Looks OK to me, Simon, since this is DM, can you review it ?

Hello Simon,

Do you think the patchset is ok?

Yes I think so.

Regards,
Simon

#13 lore raw

v2 cover Migrate Vybrid USB driver to driver model

Quoted 55 lines · Simon Glass

Hi,

Quoted 52 lines · On 1 September 2016 at 03:49, <[email protected]> wrote:
Quoted 51 lines · Marek Vasut
Quoted 49 lines · Sanchayan Maity

Hello,

This is the second version of the patchset for migrating Vybrid USB driver to driver model.

Compare to the first version, this version takes care of dr_mode property and correctly handles OTG as well when gpio is specified for use as ID detection pin. This is an essential requirement for OTG as Vybrid USB controller is not a true OTG though it can be configured as either host or device. The ID pin which is unique for OTG operation is not present on Vybrid.

The problem with client that I was observing was related to sequence numbers. While trying to implement the OTG functionality I observed that if during probe of USB0 if it returns ENODEV, the probe of USB1 provides a sequence number of 0 while we expect 1. The code relies on sequence numbers for initialising the appropriate peripherals. I use the bind operation to force a sequence number. This also seems to solve the problems I was having with USB client and mentioned in the previous version of the patchset.

Host and client functionality are both functional with this patch. Patch series is based on top of latest u-boot master at the moment of this writing. Tested on Toradex Colibri Vybrid VF61 module.

Thanks to Lukasz and Stefan for their comments.

V1 Patches:
[1].
https://patchwork.ozlabs.org/patch/655370/
[2].
https://patchwork.ozlabs.org/patch/655371/
[3].
https://patchwork.ozlabs.org/patch/655372/
[4].
https://patchwork.ozlabs.org/patch/655373/

Sanchayan Maity (4):
usb: host: ehci-vf: Migrate Vybrid USB to driver model
ARM: dts: vf: Add device tree node for USB on Vybrid
ARM: dts: vf-colibri: Enable USB device tree node for Colibri Vybrid
configs: colibri_vf_defconfig: Enable USB driver model for Colibri Vybrid

 arch/arm/dts/vf-colibri.dtsi |  11 +++
 arch/arm/dts/vf.dtsi         |  14 +++
 configs/colibri_vf_defconfig |   1 +
 drivers/usb/host/ehci-vf.c   | 208 +++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 227 insertions(+), 7 deletions(-)
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

Looks OK to me, Simon, since this is DM, can you review it ?

Hello Simon,

Do you think the patchset is ok?

Yes I think so.

Applied to u-boot-usb/next .

Signature · 2 lines
Best regards,
Marek Vasut