[U-Boot] [PATCH v2 0/4] Migrate Vybrid USB driver to driver model

[U-Boot] [PATCH v2 0/4] Migrate Vybrid USB driver to driver model

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

-- 
2.9.2

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

[Sanchayan Maity]

Add driver model support for Vybrid USB driver.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 drivers/usb/host/ehci-vf.c | 208 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 201 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/ehci-vf.c b/drivers/usb/host/ehci-vf.c
index 61789dd..f6f9efb 100644
--- a/drivers/usb/host/ehci-vf.c
+++ b/drivers/usb/host/ehci-vf.c
@@ -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
-- 
2.9.2

[U-Boot] [PATCH v2 2/4] ARM: dts: vf: Add device tree node for USB on Vybrid

[Sanchayan Maity]

Add device tree node for USB peripheral on Vybrid.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 arch/arm/dts/vf.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/dts/vf.dtsi b/arch/arm/dts/vf.dtsi
index 1530d2f..d7d21a3 100644
--- a/arch/arm/dts/vf.dtsi
+++ b/arch/arm/dts/vf.dtsi
@@ -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";
+			};
 		};
 	};
 };
-- 
2.9.2

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

[Sanchayan Maity]

Enable USB device tree node for Toradex Colibri Vybrid module.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 arch/arm/dts/vf-colibri.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/dts/vf-colibri.dtsi b/arch/arm/dts/vf-colibri.dtsi
index dc52748..c2f104a 100644
--- a/arch/arm/dts/vf-colibri.dtsi
+++ b/arch/arm/dts/vf-colibri.dtsi
@@ -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";
 };
-- 
2.9.2

[U-Boot] [PATCH v2 4/4] configs: colibri_vf_defconfig: Enable USB driver model for Colibri Vybrid

[Sanchayan Maity]

Enable USB driver model for Toradex Colibri Vybrid modules.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 configs/colibri_vf_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/colibri_vf_defconfig b/configs/colibri_vf_defconfig
index 986cec4..5017c7d 100644
--- a/configs/colibri_vf_defconfig
+++ b/configs/colibri_vf_defconfig
@@ -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
-- 
2.9.2

[U-Boot] [PATCH v2 0/4] Migrate Vybrid USB driver to driver model

[maitysanchayan at gmail.com]

Hello,

Ping?

Regards,
Sanchayan.

On 16-08-09 23:44:58, Sanchayan Maity wrote:
> 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(-)
> 
> -- 
> 2.9.2
> 

[U-Boot] [PATCH v2 0/4] Migrate Vybrid USB driver to driver model

[Marek Vasut]

On 08/09/2016 08:14 PM, Sanchayan Maity wrote:
> 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(-)
> 

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

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 0/4] Migrate Vybrid USB driver to driver model

[maitysanchayan at gmail.com]

On 16-08-23 15:17:12, Marek Vasut wrote:
> On 08/09/2016 08:14 PM, Sanchayan Maity wrote:
> > 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(-)
> > 
> 
> 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.

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

[Simon Glass]

Hi,

On 9 August 2016 at 12:14, Sanchayan Maity <maitysanchayan@gmail.com> wrote:
>
> Add driver model support for Vybrid USB driver.
>
> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> ---
>  drivers/usb/host/ehci-vf.c | 208 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 201 insertions(+), 7 deletions(-)

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

>
> diff --git a/drivers/usb/host/ehci-vf.c b/drivers/usb/host/ehci-vf.c
> index 61789dd..f6f9efb 100644
> --- a/drivers/usb/host/ehci-vf.c
> +++ b/drivers/usb/host/ehci-vf.c
> @@ -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,

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

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

> +       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")) {

You can say

if (!strcmp...

if you like. Up to you.

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

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

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

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

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

> +
> +       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
> --
> 2.9.2

Regards,
Simon

[U-Boot] [PATCH v2 2/4] ARM: dts: vf: Add device tree node for USB on Vybrid

[Simon Glass]

On 9 August 2016 at 12:15, Sanchayan Maity <maitysanchayan@gmail.com> wrote:
> Add device tree node for USB peripheral on Vybrid.
>
> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> ---
>  arch/arm/dts/vf.dtsi | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

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

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

[Simon Glass]

On 9 August 2016 at 12:15, Sanchayan Maity <maitysanchayan@gmail.com> wrote:
> Enable USB device tree node for Toradex Colibri Vybrid module.
>
> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> ---
>  arch/arm/dts/vf-colibri.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)

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

[U-Boot] [PATCH v2 4/4] configs: colibri_vf_defconfig: Enable USB driver model for Colibri Vybrid

[Simon Glass]

On 9 August 2016 at 12:15, Sanchayan Maity <maitysanchayan@gmail.com> wrote:
> Enable USB driver model for Toradex Colibri Vybrid modules.
>
> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> ---
>  configs/colibri_vf_defconfig | 1 +
>  1 file changed, 1 insertion(+)

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

[U-Boot] [PATCH v2 0/4] Migrate Vybrid USB driver to driver model

[Simon Glass]

Hi,

On 1 September 2016 at 03:49,  <maitysanchayan@gmail.com> wrote:
> On 16-08-23 15:17:12, Marek Vasut wrote:
>> On 08/09/2016 08:14 PM, Sanchayan Maity wrote:
>> > 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(-)
>> >
>>
>> 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

[U-Boot] [PATCH v2 0/4] Migrate Vybrid USB driver to driver model

[Marek Vasut]

On 09/03/2016 12:53 AM, Simon Glass wrote:
> Hi,
> 
> On 1 September 2016 at 03:49,  <maitysanchayan@gmail.com> wrote:
>> On 16-08-23 15:17:12, Marek Vasut wrote:
>>> On 08/09/2016 08:14 PM, Sanchayan Maity wrote:
>>>> 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(-)
>>>>
>>>
>>> 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 .


-- 
Best regards,
Marek Vasut