[U-Boot] [PATCH v2 0/7] rk3399: enable host controllers

[U-Boot] [PATCH v2 0/7] rk3399: enable host controllers

[Kever Yang]

rk3399 evb has two typec port(dwc3 controller) which support dual role
device with separate GPIO for vbus control and two USB 2.0 host port
(generic EHCI controller) with one GPIO for vbus control.

This patch set enable all these host controllers and have test with usb
disk and usb ethernet devices on rk3399 evb.

Note: type-C port only support usb 2.0 currently because the PD driver
and USB 3.0 phy driver not enabled.


Changes in v2:
- update for comments from Marek
- use regulator_get_by_platname instead of uclass_get_device_by_name
- add Acked-by Tag from Simon

Kever Yang (3):
  dts: rk3399-evb: add regulator-fixed for usb host vbus
  board: evb-rk3399: enable usb 2.0 host vbus power on board_init
  config: evb-rk3399: enable fixed regulator

MengDongyang (4):
  usb: xhci-rockchip: add rockchip dwc3 controller driver
  rockchip: select DM_USB for rockchip SoC
  config: rk3399: add usb related configs
  dts: rk3399: add dwc3_typec node for rk3399

 arch/arm/Kconfig                       |   1 +
 arch/arm/dts/rk3399-evb.dts            |  14 ++
 arch/arm/dts/rk3399.dtsi               |  45 +++++++
 board/rockchip/evb_rk3399/evb-rk3399.c |  15 ++-
 configs/evb-rk3399_defconfig           |   9 ++
 drivers/usb/host/Makefile              |   1 +
 drivers/usb/host/xhci-rockchip.c       | 227 +++++++++++++++++++++++++++++++++
 include/configs/rk3399_common.h        |  13 ++
 include/linux/usb/dwc3.h               |   9 ++
 9 files changed, 333 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/host/xhci-rockchip.c

-- 
1.9.1

[U-Boot] [PATCH v2 1/7] usb: xhci-rockchip: add rockchip dwc3 controller driver

[Kever Yang]

From: MengDongyang <daniel.meng@rock-chips.com>

This patch add support for rockchip dwc3 controller, which corresponding
to the two type-C port on rk3399 evb.
Only support usb2.0 currently for we have not enable the usb3.0 phy
driver and PD(fusb302) driver.

Signed-off-by: MengDongyang <daniel.meng@rock-chips.com>
Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

Changes in v2:
- update for comments from Marek

 drivers/usb/host/Makefile        |   1 +
 drivers/usb/host/xhci-rockchip.c | 227 +++++++++++++++++++++++++++++++++++++++
 include/linux/usb/dwc3.h         |   9 ++
 3 files changed, 237 insertions(+)
 create mode 100644 drivers/usb/host/xhci-rockchip.c

diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 620d114..fdefcf6 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_USB_EHCI_ZYNQ) += ehci-zynq.o
 # xhci
 obj-$(CONFIG_USB_XHCI_HCD) += xhci.o xhci-mem.o xhci-ring.o
 obj-$(CONFIG_USB_XHCI_DWC3) += xhci-dwc3.o
+obj-$(CONFIG_USB_XHCI_ROCKCHIP) += xhci-rockchip.o
 obj-$(CONFIG_USB_XHCI_ZYNQMP) += xhci-zynqmp.o
 obj-$(CONFIG_USB_XHCI_KEYSTONE) += xhci-keystone.o
 obj-$(CONFIG_USB_XHCI_EXYNOS) += xhci-exynos5.o
diff --git a/drivers/usb/host/xhci-rockchip.c b/drivers/usb/host/xhci-rockchip.c
new file mode 100644
index 0000000..baf6b6e
--- /dev/null
+++ b/drivers/usb/host/xhci-rockchip.c
@@ -0,0 +1,227 @@
+/*
+ * Copyright (c) 2016 Rockchip, Inc.
+ * Authors: Daniel Meng <daniel.meng@rock-chips.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+#include <common.h>
+#include <dm.h>
+#include <fdtdec.h>
+#include <libfdt.h>
+#include <malloc.h>
+#include <usb.h>
+#include <watchdog.h>
+#include <asm/gpio.h>
+#include <asm-generic/errno.h>
+#include <linux/compat.h>
+#include <linux/usb/dwc3.h>
+
+#include "xhci.h"
+
+DECLARE_GLOBAL_DATA_PTR;
+
+struct rockchip_xhci_platdata {
+	fdt_addr_t hcd_base;
+	fdt_addr_t phy_base;
+	struct gpio_desc vbus_gpio;
+};
+
+/*
+ * Contains pointers to register base addresses
+ * for the usb controller.
+ */
+struct rockchip_xhci {
+	struct usb_platdata usb_plat;
+	struct xhci_ctrl ctrl;
+	struct xhci_hccr *hcd;
+	struct dwc3 *dwc3_reg;
+	struct udevice *dev;
+};
+
+static int xhci_usb_ofdata_to_platdata(struct udevice *dev)
+{
+	struct rockchip_xhci_platdata *plat = dev_get_platdata(dev);
+	struct udevice *child;
+	int ret = 0;
+
+	/*
+	 * Get the base address for XHCI controller from the device node
+	 */
+	plat->hcd_base = dev_get_addr(dev);
+	if (plat->hcd_base == FDT_ADDR_T_NONE) {
+		debug("Can't get the XHCI register base address\n");
+		return -ENXIO;
+	}
+
+	/*
+	 * Get the base address for usbphy from the device node
+	 */
+	for (device_find_first_child(dev, &child); child;
+	     device_find_next_child(&child)) {
+		if (!of_device_is_compatible(child, "rockchip,rk3399-usb3-phy"))
+			continue;
+		plat->phy_base = dev_get_addr(child);
+		break;
+	}
+
+	if (plat->phy_base == FDT_ADDR_T_NONE) {
+		debug("Can't get the usbphy register address\n");
+		return -ENXIO;
+	}
+
+	/* Vbus gpio */
+	ret = gpio_request_by_name(dev, "rockchip,vbus-gpio", 0,
+				   &plat->vbus_gpio, GPIOD_IS_OUT);
+	if (ret)
+		debug("rockchip,vbus-gpio node missing!");
+
+	return 0;
+}
+
+/*
+ * rockchip_dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
+ * @dwc: Pointer to our controller context structure
+ * @rockchip_xhci: Pointer to dev private data
+ */
+static void rockchip_dwc3_phy_setup(struct dwc3 *dwc3_reg,
+				    struct rockchip_xhci *rockchip)
+{
+	u32 reg;
+	const void *blob = gd->fdt_blob;
+	struct udevice *dev = rockchip->dev;
+	const struct fdt_property *prop;
+	const u32 *data;
+
+	reg = readl(&dwc3_reg->g_usb3pipectl[0]);
+
+	/* To do set dwc3 usb3 pipe control */
+
+	writel(reg, &dwc3_reg->g_usb3pipectl[0]);
+
+	/* Set dwc3 usb2 phy config */
+
+	reg = readl(&dwc3_reg->g_usb2phycfg[0]);
+
+	prop = fdt_get_property(blob, dev->of_offset,
+				"snps,dis-enblslpm-quirk", NULL);
+	if (prop)
+		reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
+
+	prop = fdt_get_property(blob, dev->of_offset,
+				"snps,phyif-utmi-bits", NULL);
+	data = (u32 *)prop->data;
+	if (fdt32_to_cpu(*data) == 16) {
+		reg |= DWC3_GUSB2PHYCFG_PHYIF;
+		reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
+		reg |= 5 << DWC3_GUSB2PHYCFG_USBTRDTIM_OFFSET;
+	} else if (fdt32_to_cpu(*data) == 8) {
+		reg &= ~DWC3_GUSB2PHYCFG_PHYIF;
+		reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
+		reg |= 9 << DWC3_GUSB2PHYCFG_USBTRDTIM_OFFSET;
+	}
+
+	prop = fdt_get_property(blob, dev->of_offset,
+				"snps,dis-u2-freeclk-exists-quirk", NULL);
+	if (prop)
+		reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS;
+
+	prop = fdt_get_property(blob, dev->of_offset,
+				"snps,dis-u2-susphy-quirk", NULL);
+	if (prop)
+		reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
+
+	writel(reg, &dwc3_reg->g_usb2phycfg[0]);
+}
+
+static int rockchip_xhci_core_init(struct rockchip_xhci *rockchip)
+{
+	int ret;
+
+	ret = dwc3_core_init(rockchip->dwc3_reg);
+	if (ret) {
+		debug("failed to initialize core\n");
+		return -EINVAL;
+	}
+
+	rockchip_dwc3_phy_setup(rockchip->dwc3_reg, rockchip);
+
+	/* We are hard-coding DWC3 core to Host Mode */
+	dwc3_set_mode(rockchip->dwc3_reg, DWC3_GCTL_PRTCAP_HOST);
+
+	return 0;
+}
+
+static int rockchip_xhci_core_exit(struct rockchip_xhci *rockchip)
+{
+	return 0;
+}
+
+static int xhci_usb_probe(struct udevice *dev)
+{
+	struct rockchip_xhci_platdata *plat = dev_get_platdata(dev);
+	struct rockchip_xhci *ctx = dev_get_priv(dev);
+	struct xhci_hcor *hcor;
+	int ret;
+
+	ctx->hcd = (struct xhci_hccr *)plat->hcd_base;
+	ctx->dwc3_reg = (struct dwc3 *)((char *)(ctx->hcd) + DWC3_REG_OFFSET);
+	ctx->dev = dev;
+	hcor = (struct xhci_hcor *)((uint64_t)ctx->hcd +
+			HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase)));
+
+	/* setup the Vbus gpio here */
+	if (dm_gpio_is_valid(&plat->vbus_gpio))
+		dm_gpio_set_value(&plat->vbus_gpio, 1);
+
+	ret = rockchip_xhci_core_init(ctx);
+	if (ret) {
+		puts("XHCI: failed to initialize controller\n");
+		return -EINVAL;
+	}
+
+	return xhci_register(dev, ctx->hcd, hcor);
+}
+
+static int xhci_usb_remove(struct udevice *dev)
+{
+	struct rockchip_xhci *ctx = dev_get_priv(dev);
+	int ret;
+
+	ret = xhci_deregister(dev);
+	if (ret)
+		return ret;
+	ret = rockchip_xhci_core_exit(ctx);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct udevice_id xhci_usb_ids[] = {
+	{ .compatible = "rockchip,rk3399-xhci" },
+	{ }
+};
+
+U_BOOT_DRIVER(usb_xhci) = {
+	.name	= "xhci_rockchip",
+	.id	= UCLASS_USB,
+	.of_match = xhci_usb_ids,
+	.ofdata_to_platdata = xhci_usb_ofdata_to_platdata,
+	.probe = xhci_usb_probe,
+	.remove = xhci_usb_remove,
+	.ops	= &xhci_usb_ops,
+	.bind	= dm_scan_fdt_dev,
+	.platdata_auto_alloc_size = sizeof(struct rockchip_xhci_platdata),
+	.priv_auto_alloc_size = sizeof(struct rockchip_xhci),
+	.flags	= DM_FLAG_ALLOC_PRIV_DMA,
+};
+
+static const struct udevice_id usb_phy_ids[] = {
+	{ .compatible = "rockchip,rk3399-usb3-phy" },
+	{ }
+};
+
+U_BOOT_DRIVER(usb_phy) = {
+	.name = "usb_phy_rockchip",
+	.of_match = usb_phy_ids,
+};
diff --git a/include/linux/usb/dwc3.h b/include/linux/usb/dwc3.h
index 6d1e365..b1e3d0a 100644
--- a/include/linux/usb/dwc3.h
+++ b/include/linux/usb/dwc3.h
@@ -180,7 +180,16 @@ struct dwc3 {					/* offset: 0xC100 */
 
 /* Global USB2 PHY Configuration Register */
 #define DWC3_GUSB2PHYCFG_PHYSOFTRST		(1 << 31)
+#define DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS	(1 << 30)
+#define DWC3_GUSB2PHYCFG_ENBLSLPM		(1 << 8)
 #define DWC3_GUSB2PHYCFG_SUSPHY			(1 << 6)
+#define DWC3_GUSB2PHYCFG_PHYIF			(1 << 3)
+
+/* Global USB2 PHY Configuration Mask */
+#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASK		(0xf << 10)
+
+/* Global USB2 PHY Configuration Offset */
+#define DWC3_GUSB2PHYCFG_USBTRDTIM_OFFSET	10
 
 /* Global USB3 PIPE Control Register */
 #define DWC3_GUSB3PIPECTL_PHYSOFTRST		(1 << 31)
-- 
1.9.1

[U-Boot] [PATCH v2 2/7] rockchip: select DM_USB for rockchip SoC

[Kever Yang]

From: MengDongyang <daniel.meng@rock-chips.com>

Select DM_USB to compatible with USB DM driver model.

Signed-off-by: MengDongyang <daniel.meng@rock-chips.com>
Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
Acked-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 arch/arm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 5ac9401..4dfd4b2 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -861,6 +861,7 @@ config ARCH_ROCKCHIP
 	select DM_SERIAL
 	select DM_SPI
 	select DM_SPI_FLASH
+	select DM_USB if USB
 
 config TARGET_THUNDERX_88XX
 	bool "Support ThunderX 88xx"
-- 
1.9.1

[U-Boot] [PATCH v2 3/7] config: rk3399: add usb related configs

[Kever Yang]

From: MengDongyang <daniel.meng@rock-chips.com>

This patch to enable configs for usb module
- xhci
- ehci
- usb storage
- usb net

Signed-off-by: MengDongyang <daniel.meng@rock-chips.com>
Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
Acked-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 configs/evb-rk3399_defconfig    |  7 +++++++
 include/configs/rk3399_common.h | 13 +++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/configs/evb-rk3399_defconfig b/configs/evb-rk3399_defconfig
index 2951678..378111f 100644
--- a/configs/evb-rk3399_defconfig
+++ b/configs/evb-rk3399_defconfig
@@ -15,6 +15,7 @@ CONFIG_CMD_EXT2=y
 CONFIG_CMD_EXT4=y
 CONFIG_CMD_FAT=y
 CONFIG_CMD_FS_GENERIC=y
+CONFIG_CMD_USB=y
 CONFIG_REGMAP=y
 CONFIG_SYSCON=y
 CONFIG_CLK=y
@@ -36,3 +37,9 @@ CONFIG_DEBUG_UART_CLOCK=24000000
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_USE_TINY_PRINTF=y
 CONFIG_ERRNO_STR=y
+CONFIG_USB=y
+CONFIG_USB_STORAGE=y
+CONFIG_USB_XHCI_HCD=y
+CONFIG_USB_XHCI_DWC3=y
+CONFIG_USB_EHCI_HCD=y
+CONFIG_USB_EHCI_GENERIC=y
diff --git a/include/configs/rk3399_common.h b/include/configs/rk3399_common.h
index bc85e2f..4938165 100644
--- a/include/configs/rk3399_common.h
+++ b/include/configs/rk3399_common.h
@@ -106,4 +106,17 @@
 
 #endif
 
+/* enable usb config for usb ether */
+#define CONFIG_USB_HOST_ETHER
+
+#define CONFIG_USB_ETHER_ASIX
+#define CONFIG_USB_ETHER_ASIX88179
+#define CONFIG_USB_ETHER_MCS7830
+#define CONFIG_USB_ETHER_SMSC95XX
+#define CONFIG_USB_ETHER_RTL8152
+
+/* rockchip xhci host driver */
+#define CONFIG_SYS_USB_XHCI_MAX_ROOT_PORTS	2
+#define CONFIG_USB_XHCI_ROCKCHIP
+
 #endif
-- 
1.9.1

[U-Boot] [PATCH v2 4/7] dts: rk3399: add dwc3_typec node for rk3399

[Kever Yang]

From: MengDongyang <daniel.meng@rock-chips.com>

rk3399 has two dwc3 controller for type-C port, add the dts node
and enable them.

Signed-off-by: MengDongyang <daniel.meng@rock-chips.com>
Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
Acked-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 arch/arm/dts/rk3399-evb.dts |  8 ++++++++
 arch/arm/dts/rk3399.dtsi    | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/arch/arm/dts/rk3399-evb.dts b/arch/arm/dts/rk3399-evb.dts
index e92a492..7b49f6f 100644
--- a/arch/arm/dts/rk3399-evb.dts
+++ b/arch/arm/dts/rk3399-evb.dts
@@ -85,6 +85,10 @@
 	status = "okay";
 };
 
+&dwc3_typec0 {
+	status = "okay";
+};
+
 &usb_host1_ehci {
 	status = "okay";
 };
@@ -93,6 +97,10 @@
 	status = "okay";
 };
 
+&dwc3_typec1 {
+	status = "okay";
+};
+
 &pinctrl {
 	pmic {
 		pmic_int_l: pmic-int-l {
diff --git a/arch/arm/dts/rk3399.dtsi b/arch/arm/dts/rk3399.dtsi
index 6d82078..179860c 100644
--- a/arch/arm/dts/rk3399.dtsi
+++ b/arch/arm/dts/rk3399.dtsi
@@ -9,6 +9,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/pinctrl/rockchip.h>
+#define USB_CLASS_HUB			9
 
 / {
 	compatible = "rockchip,rk3399";
@@ -230,6 +231,50 @@
 		status = "disabled";
 	};
 
+	dwc3_typec0: usb at fe800000 {
+		compatible = "rockchip,rk3399-xhci";
+		reg = <0x0 0xfe800000 0x0 0x100000>;
+		status = "disabled";
+		rockchip,vbus-gpio = <&gpio1 3 GPIO_ACTIVE_HIGH>;
+		snps,dis-enblslpm-quirk;
+		snps,phyif-utmi-bits = <16>;
+		snps,dis-u2-freeclk-exists-quirk;
+		snps,dis-u2-susphy-quirk;
+
+		#address-cells = <2>;
+		#size-cells = <2>;
+		hub {
+			compatible = "usb-hub";
+			usb,device-class = <USB_CLASS_HUB>;
+		};
+		typec_phy0 {
+			compatible = "rockchip,rk3399-usb3-phy";
+			reg = <0x0 0xff7c0000 0x0 0x40000>;
+		};
+	};
+
+	dwc3_typec1: usb at fe900000 {
+		compatible = "rockchip,rk3399-xhci";
+		reg = <0x0 0xfe900000 0x0 0x100000>;
+		status = "disabled";
+		rockchip,vbus-gpio = <&gpio1 4 GPIO_ACTIVE_HIGH>;
+		snps,dis-enblslpm-quirk;
+		snps,phyif-utmi-bits = <16>;
+		snps,dis-u2-freeclk-exists-quirk;
+		snps,dis-u2-susphy-quirk;
+
+		#address-cells = <2>;
+		#size-cells = <2>;
+		hub {
+			compatible = "usb-hub";
+			usb,device-class = <USB_CLASS_HUB>;
+		};
+		typec_phy1 {
+			compatible = "rockchip,rk3399-usb3-phy";
+			reg = <0x0 0xff800000 0x0 0x40000>;
+		};
+	};
+
 	gic: interrupt-controller at fee00000 {
 		compatible = "arm,gic-v3";
 		#interrupt-cells = <3>;
-- 
1.9.1

[U-Boot] [PATCH v2 5/7] dts: rk3399-evb: add regulator-fixed for usb host vbus

[Kever Yang]

rk3399 evb using one gpio to enable 5V output for both USB 2.0
host port, let's use fixed regulator for them.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
Acked-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 arch/arm/dts/rk3399-evb.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/dts/rk3399-evb.dts b/arch/arm/dts/rk3399-evb.dts
index 7b49f6f..bd7801b 100644
--- a/arch/arm/dts/rk3399-evb.dts
+++ b/arch/arm/dts/rk3399-evb.dts
@@ -43,6 +43,12 @@
 		regulator-always-on;
 		regulator-boot-on;
 	};
+
+	vcc5v0_host: vcc5v0-host-en {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc5v0_host";
+		gpio = <&gpio4 25 GPIO_ACTIVE_HIGH>;
+	};
 };
 
 &emmc_phy {
-- 
1.9.1

[U-Boot] [PATCH v2 6/7] board: evb-rk3399: enable usb 2.0 host vbus power on board_init

[Kever Yang]

rk3399 using one gpio control signal for two usb 2.0 host port,
it's better to enable the power in board file instead of in usb driver.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

Changes in v2:
- use regulator_get_by_platname instead of uclass_get_device_by_name

 board/rockchip/evb_rk3399/evb-rk3399.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c b/board/rockchip/evb_rk3399/evb-rk3399.c
index cd61f59..d1f1e89 100644
--- a/board/rockchip/evb_rk3399/evb-rk3399.c
+++ b/board/rockchip/evb_rk3399/evb-rk3399.c
@@ -9,12 +9,13 @@
 #include <asm/arch/periph.h>
 #include <usb.h>
 #include <dwc3-uboot.h>
+#include <power/regulator.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
 int board_init(void)
 {
-	struct udevice *pinctrl;
+	struct udevice *pinctrl, *regulator;
 	int ret;
 
 	/*
@@ -40,6 +41,18 @@ int board_init(void)
 		goto out;
 	}
 
+	ret = regulator_get_by_platname("vcc5v0_host", &regulator);
+	if (ret) {
+		debug("%s vcc5v0_host init fail! ret %d\n", __func__, ret);
+		goto out;
+	}
+
+	ret = regulator_set_enable(regulator, true);
+	if (ret) {
+		debug("%s vcc5v0-host-en set fail!\n", __func__);
+		goto out;
+	}
+
 out:
 	return 0;
 }
-- 
1.9.1

[U-Boot] [PATCH v2 7/7] config: evb-rk3399: enable fixed regulator

[Kever Yang]

This patch enable fixed regulator driver for rk3399 evb.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
Acked-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- add Acked-by Tag from Simon

 configs/evb-rk3399_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/evb-rk3399_defconfig b/configs/evb-rk3399_defconfig
index 378111f..9a6d422 100644
--- a/configs/evb-rk3399_defconfig
+++ b/configs/evb-rk3399_defconfig
@@ -43,3 +43,5 @@ CONFIG_USB_XHCI_HCD=y
 CONFIG_USB_XHCI_DWC3=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_GENERIC=y
+CONFIG_DM_REGULATOR=y
+CONFIG_DM_REGULATOR_FIXED=y
-- 
1.9.1

[U-Boot] [PATCH v2 1/7] usb: xhci-rockchip: add rockchip dwc3 controller driver

[Marek Vasut]

On 08/19/2016 09:19 AM, Kever Yang wrote:
> From: MengDongyang <daniel.meng@rock-chips.com>
> 
> This patch add support for rockchip dwc3 controller, which corresponding
> to the two type-C port on rk3399 evb.
> Only support usb2.0 currently for we have not enable the usb3.0 phy
> driver and PD(fusb302) driver.
> 
> Signed-off-by: MengDongyang <daniel.meng@rock-chips.com>
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>

Hi,

[...]

> +static void rockchip_dwc3_phy_setup(struct dwc3 *dwc3_reg,
> +				    struct rockchip_xhci *rockchip)
> +{
> +	u32 reg;
> +	const void *blob = gd->fdt_blob;
> +	struct udevice *dev = rockchip->dev;
> +	const struct fdt_property *prop;
> +	const u32 *data;
> +
> +	reg = readl(&dwc3_reg->g_usb3pipectl[0]);
> +
> +	/* To do set dwc3 usb3 pipe control */
> +
> +	writel(reg, &dwc3_reg->g_usb3pipectl[0]);

This read - do nothing - write looks suspicious. Is this needed ?

Also, drop the newline after the comment, it's useless, both above and
below here.

> +	/* Set dwc3 usb2 phy config */
> +
> +	reg = readl(&dwc3_reg->g_usb2phycfg[0]);
> +
> +	prop = fdt_get_property(blob, dev->of_offset,
> +				"snps,dis-enblslpm-quirk", NULL);
> +	if (prop)
> +		reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
> +
> +	prop = fdt_get_property(blob, dev->of_offset,
> +				"snps,phyif-utmi-bits", NULL);
> +	data = (u32 *)prop->data;
> +	if (fdt32_to_cpu(*data) == 16) {
> +		reg |= DWC3_GUSB2PHYCFG_PHYIF;
> +		reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
> +		reg |= 5 << DWC3_GUSB2PHYCFG_USBTRDTIM_OFFSET;
> +	} else if (fdt32_to_cpu(*data) == 8) {
> +		reg &= ~DWC3_GUSB2PHYCFG_PHYIF;
> +		reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
> +		reg |= 9 << DWC3_GUSB2PHYCFG_USBTRDTIM_OFFSET;
> +	}
> +
> +	prop = fdt_get_property(blob, dev->of_offset,
> +				"snps,dis-u2-freeclk-exists-quirk", NULL);
> +	if (prop)
> +		reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS;
> +
> +	prop = fdt_get_property(blob, dev->of_offset,
> +				"snps,dis-u2-susphy-quirk", NULL);
> +	if (prop)
> +		reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
> +
> +	writel(reg, &dwc3_reg->g_usb2phycfg[0]);
> +}

[...]

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 6/7] board: evb-rk3399: enable usb 2.0 host vbus power on board_init

[Simon Glass]

On 19 August 2016 at 01:19, Kever Yang <kever.yang@rock-chips.com> wrote:
>
> rk3399 using one gpio control signal for two usb 2.0 host port,
> it's better to enable the power in board file instead of in usb driver.
>
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
>
> Changes in v2:
> - use regulator_get_by_platname instead of uclass_get_device_by_name
>
>  board/rockchip/evb_rk3399/evb-rk3399.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)


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

[U-Boot] [PATCH v2 1/7] usb: xhci-rockchip: add rockchip dwc3 controller driver

[Simon Glass]

Hi Kever,

On 19 August 2016 at 01:19, Kever Yang <kever.yang@rock-chips.com> wrote:
> From: MengDongyang <daniel.meng@rock-chips.com>
>
> This patch add support for rockchip dwc3 controller, which corresponding
> to the two type-C port on rk3399 evb.
> Only support usb2.0 currently for we have not enable the usb3.0 phy
> driver and PD(fusb302) driver.
>
> Signed-off-by: MengDongyang <daniel.meng@rock-chips.com>
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
>
> Changes in v2:
> - update for comments from Marek
>
>  drivers/usb/host/Makefile        |   1 +
>  drivers/usb/host/xhci-rockchip.c | 227 +++++++++++++++++++++++++++++++++++++++
>  include/linux/usb/dwc3.h         |   9 ++
>  3 files changed, 237 insertions(+)
>  create mode 100644 drivers/usb/host/xhci-rockchip.c
>
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index 620d114..fdefcf6 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_USB_EHCI_ZYNQ) += ehci-zynq.o
>  # xhci
>  obj-$(CONFIG_USB_XHCI_HCD) += xhci.o xhci-mem.o xhci-ring.o
>  obj-$(CONFIG_USB_XHCI_DWC3) += xhci-dwc3.o
> +obj-$(CONFIG_USB_XHCI_ROCKCHIP) += xhci-rockchip.o
>  obj-$(CONFIG_USB_XHCI_ZYNQMP) += xhci-zynqmp.o
>  obj-$(CONFIG_USB_XHCI_KEYSTONE) += xhci-keystone.o
>  obj-$(CONFIG_USB_XHCI_EXYNOS) += xhci-exynos5.o
> diff --git a/drivers/usb/host/xhci-rockchip.c b/drivers/usb/host/xhci-rockchip.c
> new file mode 100644
> index 0000000..baf6b6e
> --- /dev/null
> +++ b/drivers/usb/host/xhci-rockchip.c
> @@ -0,0 +1,227 @@
> +/*
> + * Copyright (c) 2016 Rockchip, Inc.
> + * Authors: Daniel Meng <daniel.meng@rock-chips.com>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +#include <common.h>
> +#include <dm.h>
> +#include <fdtdec.h>
> +#include <libfdt.h>
> +#include <malloc.h>
> +#include <usb.h>
> +#include <watchdog.h>
> +#include <asm/gpio.h>
> +#include <asm-generic/errno.h>
> +#include <linux/compat.h>
> +#include <linux/usb/dwc3.h>
> +
> +#include "xhci.h"
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +struct rockchip_xhci_platdata {
> +       fdt_addr_t hcd_base;
> +       fdt_addr_t phy_base;
> +       struct gpio_desc vbus_gpio;
> +};
> +
> +/*
> + * Contains pointers to register base addresses
> + * for the usb controller.
> + */
> +struct rockchip_xhci {
> +       struct usb_platdata usb_plat;
> +       struct xhci_ctrl ctrl;
> +       struct xhci_hccr *hcd;
> +       struct dwc3 *dwc3_reg;
> +       struct udevice *dev;

What is dev used for? Can you please add a comment?

> +};
> +
> +static int xhci_usb_ofdata_to_platdata(struct udevice *dev)
> +{
> +       struct rockchip_xhci_platdata *plat = dev_get_platdata(dev);
> +       struct udevice *child;
> +       int ret = 0;
> +
> +       /*
> +        * Get the base address for XHCI controller from the device node
> +        */
> +       plat->hcd_base = dev_get_addr(dev);
> +       if (plat->hcd_base == FDT_ADDR_T_NONE) {
> +               debug("Can't get the XHCI register base address\n");
> +               return -ENXIO;
> +       }
> +
> +       /*
> +        * Get the base address for usbphy from the device node
> +        */

/* Get the base address ... */

> +       for (device_find_first_child(dev, &child); child;
> +            device_find_next_child(&child)) {
> +               if (!of_device_is_compatible(child, "rockchip,rk3399-usb3-phy"))
> +                       continue;
> +               plat->phy_base = dev_get_addr(child);
> +               break;
> +       }
> +
> +       if (plat->phy_base == FDT_ADDR_T_NONE) {
> +               debug("Can't get the usbphy register address\n");
> +               return -ENXIO;
> +       }
> +
> +       /* Vbus gpio */
> +       ret = gpio_request_by_name(dev, "rockchip,vbus-gpio", 0,
> +                                  &plat->vbus_gpio, GPIOD_IS_OUT);
> +       if (ret)
> +               debug("rockchip,vbus-gpio node missing!");
> +
> +       return 0;
> +}
> +
> +/*
> + * rockchip_dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core

* rockchip_dwc3_phy_setup() - Configure

> + * @dwc: Pointer to our controller context structure
> + * @rockchip_xhci: Pointer to dev private data
> + */
> +static void rockchip_dwc3_phy_setup(struct dwc3 *dwc3_reg,
> +                                   struct rockchip_xhci *rockchip)
> +{
> +       u32 reg;
> +       const void *blob = gd->fdt_blob;
> +       struct udevice *dev = rockchip->dev;
> +       const struct fdt_property *prop;
> +       const u32 *data;
> +
> +       reg = readl(&dwc3_reg->g_usb3pipectl[0]);
> +
> +       /* To do set dwc3 usb3 pipe control */

Is this a TODO? If so:

   TODO(email): ...

> +
> +       writel(reg, &dwc3_reg->g_usb3pipectl[0]);
> +
> +       /* Set dwc3 usb2 phy config */

please drop this blank line

> +
> +       reg = readl(&dwc3_reg->g_usb2phycfg[0]);
> +
> +       prop = fdt_get_property(blob, dev->of_offset,
> +                               "snps,dis-enblslpm-quirk", NULL);

Can you do:

if (fdtdec_get_bool(blob, dev->of_offset, "snps,dis-enblslpm-quirk")
               reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;

> +       if (prop)
> +               reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
> +
> +       prop = fdt_get_property(blob, dev->of_offset,
> +                               "snps,phyif-utmi-bits", NULL);

I think you want fdtdec_get_int() here. It does the endianness
conversion for you.

> +       data = (u32 *)prop->data;
> +       if (fdt32_to_cpu(*data) == 16) {
> +               reg |= DWC3_GUSB2PHYCFG_PHYIF;
> +               reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
> +               reg |= 5 << DWC3_GUSB2PHYCFG_USBTRDTIM_OFFSET;

What is 5 for? Please add a comment.

> +       } else if (fdt32_to_cpu(*data) == 8) {
> +               reg &= ~DWC3_GUSB2PHYCFG_PHYIF;
> +               reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
> +               reg |= 9 << DWC3_GUSB2PHYCFG_USBTRDTIM_OFFSET;

What is 9 for?

> +       }
> +
> +       prop = fdt_get_property(blob, dev->of_offset,
> +                               "snps,dis-u2-freeclk-exists-quirk", NULL);

fdtdec_get_bool

> +       if (prop)
> +               reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS;
> +
> +       prop = fdt_get_property(blob, dev->of_offset,
> +                               "snps,dis-u2-susphy-quirk", NULL);

fdtdec_get_bool

> +       if (prop)
> +               reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
> +
> +       writel(reg, &dwc3_reg->g_usb2phycfg[0]);
> +}
> +
> +static int rockchip_xhci_core_init(struct rockchip_xhci *rockchip)
> +{
> +       int ret;
> +
> +       ret = dwc3_core_init(rockchip->dwc3_reg);
> +       if (ret) {
> +               debug("failed to initialize core\n");
> +               return -EINVAL;

return ret

> +       }
> +
> +       rockchip_dwc3_phy_setup(rockchip->dwc3_reg, rockchip);
> +
> +       /* We are hard-coding DWC3 core to Host Mode */
> +       dwc3_set_mode(rockchip->dwc3_reg, DWC3_GCTL_PRTCAP_HOST);
> +
> +       return 0;
> +}
> +
> +static int rockchip_xhci_core_exit(struct rockchip_xhci *rockchip)
> +{
> +       return 0;
> +}
> +
> +static int xhci_usb_probe(struct udevice *dev)
> +{
> +       struct rockchip_xhci_platdata *plat = dev_get_platdata(dev);
> +       struct rockchip_xhci *ctx = dev_get_priv(dev);
> +       struct xhci_hcor *hcor;
> +       int ret;
> +
> +       ctx->hcd = (struct xhci_hccr *)plat->hcd_base;
> +       ctx->dwc3_reg = (struct dwc3 *)((char *)(ctx->hcd) + DWC3_REG_OFFSET);
> +       ctx->dev = dev;
> +       hcor = (struct xhci_hcor *)((uint64_t)ctx->hcd +
> +                       HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase)));
> +
> +       /* setup the Vbus gpio here */
> +       if (dm_gpio_is_valid(&plat->vbus_gpio))
> +               dm_gpio_set_value(&plat->vbus_gpio, 1);
> +
> +       ret = rockchip_xhci_core_init(ctx);
> +       if (ret) {
> +               puts("XHCI: failed to initialize controller\n");

debug()

> +               return -EINVAL;

return ret

> +       }
> +
> +       return xhci_register(dev, ctx->hcd, hcor);
> +}
> +
> +static int xhci_usb_remove(struct udevice *dev)
> +{
> +       struct rockchip_xhci *ctx = dev_get_priv(dev);
> +       int ret;
> +
> +       ret = xhci_deregister(dev);
> +       if (ret)
> +               return ret;
> +       ret = rockchip_xhci_core_exit(ctx);
> +       if (ret)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +static const struct udevice_id xhci_usb_ids[] = {
> +       { .compatible = "rockchip,rk3399-xhci" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(usb_xhci) = {
> +       .name   = "xhci_rockchip",
> +       .id     = UCLASS_USB,
> +       .of_match = xhci_usb_ids,
> +       .ofdata_to_platdata = xhci_usb_ofdata_to_platdata,
> +       .probe = xhci_usb_probe,
> +       .remove = xhci_usb_remove,
> +       .ops    = &xhci_usb_ops,
> +       .bind   = dm_scan_fdt_dev,
> +       .platdata_auto_alloc_size = sizeof(struct rockchip_xhci_platdata),
> +       .priv_auto_alloc_size = sizeof(struct rockchip_xhci),
> +       .flags  = DM_FLAG_ALLOC_PRIV_DMA,
> +};
> +
> +static const struct udevice_id usb_phy_ids[] = {
> +       { .compatible = "rockchip,rk3399-usb3-phy" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(usb_phy) = {
> +       .name = "usb_phy_rockchip",
> +       .of_match = usb_phy_ids,
> +};
> diff --git a/include/linux/usb/dwc3.h b/include/linux/usb/dwc3.h
> index 6d1e365..b1e3d0a 100644
> --- a/include/linux/usb/dwc3.h
> +++ b/include/linux/usb/dwc3.h
> @@ -180,7 +180,16 @@ struct dwc3 {                                      /* offset: 0xC100 */
>
>  /* Global USB2 PHY Configuration Register */
>  #define DWC3_GUSB2PHYCFG_PHYSOFTRST            (1 << 31)
> +#define DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS     (1 << 30)
> +#define DWC3_GUSB2PHYCFG_ENBLSLPM              (1 << 8)
>  #define DWC3_GUSB2PHYCFG_SUSPHY                        (1 << 6)
> +#define DWC3_GUSB2PHYCFG_PHYIF                 (1 << 3)
> +
> +/* Global USB2 PHY Configuration Mask */
> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASK                (0xf << 10)
> +
> +/* Global USB2 PHY Configuration Offset */
> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_OFFSET      10
>
>  /* Global USB3 PIPE Control Register */
>  #define DWC3_GUSB3PIPECTL_PHYSOFTRST           (1 << 31)
> --
> 1.9.1
>

Regards,
Simon

[U-Boot] [PATCH v2 1/7] usb: xhci-rockchip: add rockchip dwc3 controller driver

[Kever Yang]

Hi Marek,

On 08/19/2016 06:52 PM, Marek Vasut wrote:
> On 08/19/2016 09:19 AM, Kever Yang wrote:
>> From: MengDongyang <daniel.meng@rock-chips.com>
>>
>> This patch add support for rockchip dwc3 controller, which corresponding
>> to the two type-C port on rk3399 evb.
>> Only support usb2.0 currently for we have not enable the usb3.0 phy
>> driver and PD(fusb302) driver.
>>
>> Signed-off-by: MengDongyang <daniel.meng@rock-chips.com>
>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> Hi,
>
> [...]
>
>> +static void rockchip_dwc3_phy_setup(struct dwc3 *dwc3_reg,
>> +				    struct rockchip_xhci *rockchip)
>> +{
>> +	u32 reg;
>> +	const void *blob = gd->fdt_blob;
>> +	struct udevice *dev = rockchip->dev;
>> +	const struct fdt_property *prop;
>> +	const u32 *data;
>> +
>> +	reg = readl(&dwc3_reg->g_usb3pipectl[0]);
>> +
>> +	/* To do set dwc3 usb3 pipe control */
>> +
>> +	writel(reg, &dwc3_reg->g_usb3pipectl[0]);
> This read - do nothing - write looks suspicious. Is this needed ?

This is for usb3.0 phy, will remove in next version for we not enable 
usb3.0 phy  now.
>
> Also, drop the newline after the comment, it's useless, both above and
> below here.

Will fix in next version.

Thanks,
- Kever
>
>> +	/* Set dwc3 usb2 phy config */
>> +
>> +	reg = readl(&dwc3_reg->g_usb2phycfg[0]);
>> +
>> +	prop = fdt_get_property(blob, dev->of_offset,
>> +				"snps,dis-enblslpm-quirk", NULL);
>> +	if (prop)
>> +		reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
>> +
>> +	prop = fdt_get_property(blob, dev->of_offset,
>> +				"snps,phyif-utmi-bits", NULL);
>> +	data = (u32 *)prop->data;
>> +	if (fdt32_to_cpu(*data) == 16) {
>> +		reg |= DWC3_GUSB2PHYCFG_PHYIF;
>> +		reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
>> +		reg |= 5 << DWC3_GUSB2PHYCFG_USBTRDTIM_OFFSET;
>> +	} else if (fdt32_to_cpu(*data) == 8) {
>> +		reg &= ~DWC3_GUSB2PHYCFG_PHYIF;
>> +		reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
>> +		reg |= 9 << DWC3_GUSB2PHYCFG_USBTRDTIM_OFFSET;
>> +	}
>> +
>> +	prop = fdt_get_property(blob, dev->of_offset,
>> +				"snps,dis-u2-freeclk-exists-quirk", NULL);
>> +	if (prop)
>> +		reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS;
>> +
>> +	prop = fdt_get_property(blob, dev->of_offset,
>> +				"snps,dis-u2-susphy-quirk", NULL);
>> +	if (prop)
>> +		reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>> +
>> +	writel(reg, &dwc3_reg->g_usb2phycfg[0]);
>> +}
> [...]
>

[U-Boot] [PATCH v2 1/7] usb: xhci-rockchip: add rockchip dwc3 controller driver

[Kever Yang]

Hi Simon,

On 08/22/2016 12:20 PM, Simon Glass wrote:
> Hi Kever,
>
> On 19 August 2016 at 01:19, Kever Yang <kever.yang@rock-chips.com> wrote:
>> From: MengDongyang <daniel.meng@rock-chips.com>
>>
>> This patch add support for rockchip dwc3 controller, which corresponding
>> to the two type-C port on rk3399 evb.
>> Only support usb2.0 currently for we have not enable the usb3.0 phy
>> driver and PD(fusb302) driver.
>>
>> Signed-off-by: MengDongyang <daniel.meng@rock-chips.com>
>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>> ---
>>
>> Changes in v2:
>> - update for comments from Marek
>>
>>   drivers/usb/host/Makefile        |   1 +
>>   drivers/usb/host/xhci-rockchip.c | 227 +++++++++++++++++++++++++++++++++++++++
>>   include/linux/usb/dwc3.h         |   9 ++
>>   3 files changed, 237 insertions(+)
>>   create mode 100644 drivers/usb/host/xhci-rockchip.c
>>
>> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
>> index 620d114..fdefcf6 100644
>> --- a/drivers/usb/host/Makefile
>> +++ b/drivers/usb/host/Makefile
>> @@ -56,6 +56,7 @@ obj-$(CONFIG_USB_EHCI_ZYNQ) += ehci-zynq.o
>>   # xhci
>>   obj-$(CONFIG_USB_XHCI_HCD) += xhci.o xhci-mem.o xhci-ring.o
>>   obj-$(CONFIG_USB_XHCI_DWC3) += xhci-dwc3.o
>> +obj-$(CONFIG_USB_XHCI_ROCKCHIP) += xhci-rockchip.o
>>   obj-$(CONFIG_USB_XHCI_ZYNQMP) += xhci-zynqmp.o
>>   obj-$(CONFIG_USB_XHCI_KEYSTONE) += xhci-keystone.o
>>   obj-$(CONFIG_USB_XHCI_EXYNOS) += xhci-exynos5.o
>> diff --git a/drivers/usb/host/xhci-rockchip.c b/drivers/usb/host/xhci-rockchip.c
>> new file mode 100644
>> index 0000000..baf6b6e
>> --- /dev/null
>> +++ b/drivers/usb/host/xhci-rockchip.c
>> @@ -0,0 +1,227 @@
>> +/*
>> + * Copyright (c) 2016 Rockchip, Inc.
>> + * Authors: Daniel Meng <daniel.meng@rock-chips.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <fdtdec.h>
>> +#include <libfdt.h>
>> +#include <malloc.h>
>> +#include <usb.h>
>> +#include <watchdog.h>
>> +#include <asm/gpio.h>
>> +#include <asm-generic/errno.h>
>> +#include <linux/compat.h>
>> +#include <linux/usb/dwc3.h>
>> +
>> +#include "xhci.h"
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +struct rockchip_xhci_platdata {
>> +       fdt_addr_t hcd_base;
>> +       fdt_addr_t phy_base;
>> +       struct gpio_desc vbus_gpio;
>> +};
>> +
>> +/*
>> + * Contains pointers to register base addresses
>> + * for the usb controller.
>> + */
>> +struct rockchip_xhci {
>> +       struct usb_platdata usb_plat;
>> +       struct xhci_ctrl ctrl;
>> +       struct xhci_hccr *hcd;
>> +       struct dwc3 *dwc3_reg;
>> +       struct udevice *dev;
> What is dev used for? Can you please add a comment?

This dev is used for get the fdt from dev when do phy setup, but I think 
this
can be removed.

>
>> +};
>> +
>> +static int xhci_usb_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +       struct rockchip_xhci_platdata *plat = dev_get_platdata(dev);
>> +       struct udevice *child;
>> +       int ret = 0;
>> +
>> +       /*
>> +        * Get the base address for XHCI controller from the device node
>> +        */
>> +       plat->hcd_base = dev_get_addr(dev);
>> +       if (plat->hcd_base == FDT_ADDR_T_NONE) {
>> +               debug("Can't get the XHCI register base address\n");
>> +               return -ENXIO;
>> +       }
>> +
>> +       /*
>> +        * Get the base address for usbphy from the device node
>> +        */
> /* Get the base address ... */

Will fix in next version.
>
>> +       for (device_find_first_child(dev, &child); child;
>> +            device_find_next_child(&child)) {
>> +               if (!of_device_is_compatible(child, "rockchip,rk3399-usb3-phy"))
>> +                       continue;
>> +               plat->phy_base = dev_get_addr(child);
>> +               break;
>> +       }
>> +
>> +       if (plat->phy_base == FDT_ADDR_T_NONE) {
>> +               debug("Can't get the usbphy register address\n");
>> +               return -ENXIO;
>> +       }
>> +
>> +       /* Vbus gpio */
>> +       ret = gpio_request_by_name(dev, "rockchip,vbus-gpio", 0,
>> +                                  &plat->vbus_gpio, GPIOD_IS_OUT);
>> +       if (ret)
>> +               debug("rockchip,vbus-gpio node missing!");
>> +
>> +       return 0;
>> +}
>> +
>> +/*
>> + * rockchip_dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
> * rockchip_dwc3_phy_setup() - Configure

Will fix in next version.
>
>> + * @dwc: Pointer to our controller context structure
>> + * @rockchip_xhci: Pointer to dev private data
>> + */
>> +static void rockchip_dwc3_phy_setup(struct dwc3 *dwc3_reg,
>> +                                   struct rockchip_xhci *rockchip)
>> +{
>> +       u32 reg;
>> +       const void *blob = gd->fdt_blob;
>> +       struct udevice *dev = rockchip->dev;
>> +       const struct fdt_property *prop;
>> +       const u32 *data;
>> +
>> +       reg = readl(&dwc3_reg->g_usb3pipectl[0]);
>> +
>> +       /* To do set dwc3 usb3 pipe control */
> Is this a TODO? If so:
>
>     TODO(email): ...

Will remove in next version.

>
>> +
>> +       writel(reg, &dwc3_reg->g_usb3pipectl[0]);
>> +
>> +       /* Set dwc3 usb2 phy config */
> please drop this blank line

Will fix in next version.
>> +
>> +       reg = readl(&dwc3_reg->g_usb2phycfg[0]);
>> +
>> +       prop = fdt_get_property(blob, dev->of_offset,
>> +                               "snps,dis-enblslpm-quirk", NULL);
> Can you do:
>
> if (fdtdec_get_bool(blob, dev->of_offset, "snps,dis-enblslpm-quirk")
>                 reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;

Will follow you suggestion in next version.

>
>> +       if (prop)
>> +               reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
>> +
>> +       prop = fdt_get_property(blob, dev->of_offset,
>> +                               "snps,phyif-utmi-bits", NULL);
> I think you want fdtdec_get_int() here. It does the endianness
> conversion for you.

Will fix in next version.
>
>> +       data = (u32 *)prop->data;
>> +       if (fdt32_to_cpu(*data) == 16) {
>> +               reg |= DWC3_GUSB2PHYCFG_PHYIF;
>> +               reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
>> +               reg |= 5 << DWC3_GUSB2PHYCFG_USBTRDTIM_OFFSET;
> What is 5 for? Please add a comment.

Will fix in next version.
>
>> +       } else if (fdt32_to_cpu(*data) == 8) {
>> +               reg &= ~DWC3_GUSB2PHYCFG_PHYIF;
>> +               reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
>> +               reg |= 9 << DWC3_GUSB2PHYCFG_USBTRDTIM_OFFSET;
> What is 9 for?

Will fix in next version.
>
>> +       }
>> +
>> +       prop = fdt_get_property(blob, dev->of_offset,
>> +                               "snps,dis-u2-freeclk-exists-quirk", NULL);
> fdtdec_get_bool

Will fix in next version.
>
>> +       if (prop)
>> +               reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS;
>> +
>> +       prop = fdt_get_property(blob, dev->of_offset,
>> +                               "snps,dis-u2-susphy-quirk", NULL);
> fdtdec_get_bool

Will fix in next version.
>
>> +       if (prop)
>> +               reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>> +
>> +       writel(reg, &dwc3_reg->g_usb2phycfg[0]);
>> +}
>> +
>> +static int rockchip_xhci_core_init(struct rockchip_xhci *rockchip)
>> +{
>> +       int ret;
>> +
>> +       ret = dwc3_core_init(rockchip->dwc3_reg);
>> +       if (ret) {
>> +               debug("failed to initialize core\n");
>> +               return -EINVAL;
> return ret

Will fix in next version.
>
>> +       }
>> +
>> +       rockchip_dwc3_phy_setup(rockchip->dwc3_reg, rockchip);
>> +
>> +       /* We are hard-coding DWC3 core to Host Mode */
>> +       dwc3_set_mode(rockchip->dwc3_reg, DWC3_GCTL_PRTCAP_HOST);
>> +
>> +       return 0;
>> +}
>> +
>> +static int rockchip_xhci_core_exit(struct rockchip_xhci *rockchip)
>> +{
>> +       return 0;
>> +}
>> +
>> +static int xhci_usb_probe(struct udevice *dev)
>> +{
>> +       struct rockchip_xhci_platdata *plat = dev_get_platdata(dev);
>> +       struct rockchip_xhci *ctx = dev_get_priv(dev);
>> +       struct xhci_hcor *hcor;
>> +       int ret;
>> +
>> +       ctx->hcd = (struct xhci_hccr *)plat->hcd_base;
>> +       ctx->dwc3_reg = (struct dwc3 *)((char *)(ctx->hcd) + DWC3_REG_OFFSET);
>> +       ctx->dev = dev;
>> +       hcor = (struct xhci_hcor *)((uint64_t)ctx->hcd +
>> +                       HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase)));
>> +
>> +       /* setup the Vbus gpio here */
>> +       if (dm_gpio_is_valid(&plat->vbus_gpio))
>> +               dm_gpio_set_value(&plat->vbus_gpio, 1);
>> +
>> +       ret = rockchip_xhci_core_init(ctx);
>> +       if (ret) {
>> +               puts("XHCI: failed to initialize controller\n");
> debug()

Will fix in next version.
>
>> +               return -EINVAL;
> return ret

Will fix in next version.

Thanks,
- Kever
>
>> +       }
>> +
>> +       return xhci_register(dev, ctx->hcd, hcor);
>> +}
>> +
>> +static int xhci_usb_remove(struct udevice *dev)
>> +{
>> +       struct rockchip_xhci *ctx = dev_get_priv(dev);
>> +       int ret;
>> +
>> +       ret = xhci_deregister(dev);
>> +       if (ret)
>> +               return ret;
>> +       ret = rockchip_xhci_core_exit(ctx);
>> +       if (ret)
>> +               return ret;
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct udevice_id xhci_usb_ids[] = {
>> +       { .compatible = "rockchip,rk3399-xhci" },
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(usb_xhci) = {
>> +       .name   = "xhci_rockchip",
>> +       .id     = UCLASS_USB,
>> +       .of_match = xhci_usb_ids,
>> +       .ofdata_to_platdata = xhci_usb_ofdata_to_platdata,
>> +       .probe = xhci_usb_probe,
>> +       .remove = xhci_usb_remove,
>> +       .ops    = &xhci_usb_ops,
>> +       .bind   = dm_scan_fdt_dev,
>> +       .platdata_auto_alloc_size = sizeof(struct rockchip_xhci_platdata),
>> +       .priv_auto_alloc_size = sizeof(struct rockchip_xhci),
>> +       .flags  = DM_FLAG_ALLOC_PRIV_DMA,
>> +};
>> +
>> +static const struct udevice_id usb_phy_ids[] = {
>> +       { .compatible = "rockchip,rk3399-usb3-phy" },
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(usb_phy) = {
>> +       .name = "usb_phy_rockchip",
>> +       .of_match = usb_phy_ids,
>> +};
>> diff --git a/include/linux/usb/dwc3.h b/include/linux/usb/dwc3.h
>> index 6d1e365..b1e3d0a 100644
>> --- a/include/linux/usb/dwc3.h
>> +++ b/include/linux/usb/dwc3.h
>> @@ -180,7 +180,16 @@ struct dwc3 {                                      /* offset: 0xC100 */
>>
>>   /* Global USB2 PHY Configuration Register */
>>   #define DWC3_GUSB2PHYCFG_PHYSOFTRST            (1 << 31)
>> +#define DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS     (1 << 30)
>> +#define DWC3_GUSB2PHYCFG_ENBLSLPM              (1 << 8)
>>   #define DWC3_GUSB2PHYCFG_SUSPHY                        (1 << 6)
>> +#define DWC3_GUSB2PHYCFG_PHYIF                 (1 << 3)
>> +
>> +/* Global USB2 PHY Configuration Mask */
>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASK                (0xf << 10)
>> +
>> +/* Global USB2 PHY Configuration Offset */
>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_OFFSET      10
>>
>>   /* Global USB3 PIPE Control Register */
>>   #define DWC3_GUSB3PIPECTL_PHYSOFTRST           (1 << 31)
>> --
>> 1.9.1
>>
> Regards,
> Simon
>
>
>