[PATCH v6 00/13] Add support for usb on Hikey960

[PATCH v6 00/13] Add support for usb on Hikey960

[Yu Chen]

The patchset adds support for usb functionality of Hikey960, includes:
- usb phy driver for Hisilicon Kirin Soc hi3660
- usb driver for HiKey960 board
- some adjustment in dwc3 and usb role driver
- dts for support usb of HiKey960

---
v5:
* Split modification of usb role into two patches, the first one introduces
  stubs for the exiting functions, and the second adds notifier functions.
v6:
* Add a fix of usb_role_switch_register_notifier initialization from
  John Stultz <john.stultz@linaro.org>
---

Yu Chen (13):
  dt-bindings: phy: Add support for HiSilicon's hi3660 USB PHY
  dt-bindings: misc: Add bindings for HiSilicon usb hub and data role
    switch functionality on HiKey960
  usb: dwc3: dwc3-of-simple: Add support for dwc3 of Hisilicon Soc
    Platform
  usb: dwc3: Add splitdisable quirk for Hisilicon Kirin Soc
  usb: dwc3: Execute GCTL Core Soft Reset while switch mdoe for
    Hisilicon Kirin Soc
  usb: dwc3: Increase timeout for CmdAct cleared by device controller
  phy: Add usb phy support for hi3660 Soc of Hisilicon
  usb: roles: Introduce stubs for the exiting functions in role.h.
  usb: roles: Add usb role switch notifier.
  usb: dwc3: Registering a role switch in the DRD code.
  hikey960: Support usb functionality of Hikey960
  usb: gadget: Add configfs attribuite for controling
    match_existing_only
  dts: hi3660: Add support for usb on Hikey960

 .../bindings/misc/hisilicon-hikey-usb.txt          |  52 +++++
 .../devicetree/bindings/phy/phy-hi3660-usb3.txt    |  26 +++
 MAINTAINERS                                        |   8 +
 arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts  |  53 +++++
 arch/arm64/boot/dts/hisilicon/hi3660.dtsi          |  73 +++++++
 drivers/misc/Kconfig                               |   6 +
 drivers/misc/Makefile                              |   1 +
 drivers/misc/hisi_hikey_usb.c                      | 162 ++++++++++++++
 drivers/phy/hisilicon/Kconfig                      |  10 +
 drivers/phy/hisilicon/Makefile                     |   1 +
 drivers/phy/hisilicon/phy-hi3660-usb3.c            | 233 +++++++++++++++++++++
 drivers/usb/dwc3/Kconfig                           |   1 +
 drivers/usb/dwc3/core.c                            |  45 ++++
 drivers/usb/dwc3/core.h                            |  15 ++
 drivers/usb/dwc3/drd.c                             |  78 ++++++-
 drivers/usb/dwc3/dwc3-of-simple.c                  |   4 +-
 drivers/usb/dwc3/gadget.c                          |   2 +-
 drivers/usb/gadget/configfs.c                      |  32 +++
 drivers/usb/roles/class.c                          |  35 +++-
 include/linux/usb/role.h                           |  46 ++++
 20 files changed, 879 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
 create mode 100644 Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
 create mode 100644 drivers/misc/hisi_hikey_usb.c
 create mode 100644 drivers/phy/hisilicon/phy-hi3660-usb3.c

-- 
2.15.0-rc2

[PATCH v6 01/13] dt-bindings: phy: Add support for HiSilicon's hi3660 USB PHY

[Yu Chen]

This patch adds binding documentation for supporting the hi3660 usb
phy on boards like the HiKey960.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Binghui Wang <wangbinghui@hisilicon.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Yu Chen <chenyu56@huawei.com>
---
v1:
* Fix some format error as suggested by Rob.
v2:
* Change hi3660 usb PHY to hi3660 USB PHY
v3:
* Make device node a subnode of usb3_otg_bc register.
v4:
* Remove "hisilicon,usb3-otg-bc-syscon" property
---
---
 .../devicetree/bindings/phy/phy-hi3660-usb3.txt    | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt

diff --git a/Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt b/Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
new file mode 100644
index 000000000000..e88ba7d92dcb
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
@@ -0,0 +1,26 @@
+Hisilicon hi3660 USB PHY
+-----------------------
+
+Required properties:
+- compatible: should be "hisilicon,hi3660-usb-phy"
+- #phy-cells: must be 0
+- hisilicon,pericrg-syscon: phandle of syscon used to control phy.
+- hisilicon,pctrl-syscon: phandle of syscon used to control phy.
+- hisilicon,eye-diagram-param: parameter set for phy
+Refer to phy/phy-bindings.txt for the generic PHY binding properties
+
+This is a subnode of usb3_otg_bc register node.
+
+Example:
+	usb3_otg_bc: usb3_otg_bc@ff200000 {
+		compatible = "syscon", "simple-mfd";
+		reg = <0x0 0xff200000 0x0 0x1000>;
+
+		usb-phy {
+			compatible = "hisilicon,hi3660-usb-phy";
+			#phy-cells = <0>;
+			hisilicon,pericrg-syscon = <&crg_ctrl>;
+			hisilicon,pctrl-syscon = <&pctrl>;
+			hisilicon,eye-diagram-param = <0x22466e4>;
+		};
+	};
-- 
2.15.0-rc2

[PATCH v6 02/13] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960

[Yu Chen]

This patch adds binding documentation to support usb hub and usb
data role switch of Hisilicon HiKey960 Board.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Binghui Wang <wangbinghui@hisilicon.com>
Signed-off-by: Yu Chen <chenyu56@huawei.com>
---
v1:
* Fix some format errors as suggested by Sergei.
* Modify gpio description to use gpiod API.
v2:
* Remove information about Hikey.
* Fix gpio description.
* Remove device_type of endpoint.
v3:
* Remove property typec-vbus-enable-val.
* Add description of pinctrl-names.
* Add example for "hisilicon,gpio-hubv1"
* Add flag in gpiod properties.
---
---
 .../bindings/misc/hisilicon-hikey-usb.txt          | 52 ++++++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt

diff --git a/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
new file mode 100644
index 000000000000..422e844df719
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
@@ -0,0 +1,52 @@
+Support usb hub and usb data role switch of Hisilicon HiKey960 Board.
+
+-----------------------------
+
+Required properties:
+- compatible: "hisilicon,gpio-hubv1","hisilicon,hikey960-usb"
+- typec-vbus-gpios: gpio to control the vbus of typeC port
+- otg-switch-gpios: gpio to switch DP & DM between the hub and typeC port
+- hub-vdd33-en-gpios: gpio to enable the power of hub
+- pinctrl-names: pin configuration state name ("default")
+- pinctrl-0: pinctrl config
+
+Example
+-----
+
+	hisi_hikey_usb: hisi_hikey_usb {
+		compatible = "hisilicon,hikey960-usb";
+		typec-vbus-gpios = <&gpio25 2 GPIO_ACTIVE_HIGH>;
+		otg-switch-gpios = <&gpio25 6 GPIO_ACTIVE_HIGH>;
+		hub-vdd33-en-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&usbhub5734_pmx_func>;
+
+		port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			hikey_usb_ep: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&dwc3_role_switch_notify>;
+			};
+		};
+	};
+
+	hisi_hikey_usb: hisi_hikey_usb {
+		compatible = "hisilicon,gpio-hubv1";
+		typec-vbus-gpios = <&gpio25 2 GPIO_ACTIVE_LOW>;
+		otg-switch-gpios = <&gpio25 6 GPIO_ACTIVE_HIGH>;
+		hub-vdd33-en-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&usbhub5734_pmx_func>;
+
+		port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			hikey_usb_ep: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&dwc3_role_switch_notify>;
+			};
+		};
+	};
-- 
2.15.0-rc2

[PATCH v6 03/13] usb: dwc3: dwc3-of-simple: Add support for dwc3 of Hisilicon Soc Platform

[Yu Chen]

This patch adds support for the poweron and shutdown of dwc3 core
on Hisilicon Soc Platform.

Cc: Felipe Balbi <balbi@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: John Stultz <john.stultz@linaro.org>
Signed-off-by: Yu Chen <chenyu56@huawei.com>
---
 drivers/usb/dwc3/dwc3-of-simple.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index 4c2771c5e727..0ed09d876542 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -98,7 +98,8 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
 	 * Some controllers need to toggle the usb3-otg reset before trying to
 	 * initialize the PHY, otherwise the PHY times out.
 	 */
-	if (of_device_is_compatible(np, "rockchip,rk3399-dwc3"))
+	if (of_device_is_compatible(np, "rockchip,rk3399-dwc3") ||
+			of_device_is_compatible(np, "hisilicon,hi3660-dwc3"))
 		simple->need_reset = true;
 
 	if (of_device_is_compatible(np, "amlogic,meson-axg-dwc3") ||
@@ -243,6 +244,7 @@ static const struct of_device_id of_dwc3_simple_match[] = {
 	{ .compatible = "amlogic,meson-axg-dwc3" },
 	{ .compatible = "amlogic,meson-gxl-dwc3" },
 	{ .compatible = "allwinner,sun50i-h6-dwc3" },
+	{ .compatible = "hisilicon,hi3660-dwc3" },
 	{ /* Sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, of_dwc3_simple_match);
-- 
2.15.0-rc2

[PATCH v6 04/13] usb: dwc3: Add splitdisable quirk for Hisilicon Kirin Soc

[Yu Chen]

SPLIT_BOUNDARY_DISABLE should be set for DesignWare USB3 DRD Core
of Hisilicon Kirin Soc when dwc3 core act as host.

Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Binghui Wang <wangbinghui@hisilicon.com>
Signed-off-by: Yu Chen <chenyu56@huawei.com>
---
v4:
* Add dwc3_complete definition while CONFIG_PM_SLEEP does not defined.
* Add description for 'dis_split_quirk'.
---
---
 drivers/usb/dwc3/core.c | 26 ++++++++++++++++++++++++++
 drivers/usb/dwc3/core.h |  7 +++++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index a1b126f90261..c3ef6bd2b0d4 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -117,6 +117,7 @@ static void __dwc3_set_mode(struct work_struct *work)
 	struct dwc3 *dwc = work_to_dwc(work);
 	unsigned long flags;
 	int ret;
+	u32 reg;
 
 	if (dwc->dr_mode != USB_DR_MODE_OTG)
 		return;
@@ -169,6 +170,11 @@ static void __dwc3_set_mode(struct work_struct *work)
 			phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
 			phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
 			phy_calibrate(dwc->usb2_generic_phy);
+			if (dwc->dis_split_quirk) {
+				reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
+				reg |= DWC3_GUCTL3_SPLITDISABLE;
+				dwc3_writel(dwc->regs, DWC3_GUCTL3, reg);
+			}
 		}
 		break;
 	case DWC3_GCTL_PRTCAP_DEVICE:
@@ -1306,6 +1312,9 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 	dwc->dis_metastability_quirk = device_property_read_bool(dev,
 				"snps,dis_metastability_quirk");
 
+	dwc->dis_split_quirk = device_property_read_bool(dev,
+				"snps,dis-split-quirk");
+
 	dwc->lpm_nyet_threshold = lpm_nyet_threshold;
 	dwc->tx_de_emphasis = tx_de_emphasis;
 
@@ -1825,10 +1834,27 @@ static int dwc3_resume(struct device *dev)
 
 	return 0;
 }
+
+static void dwc3_complete(struct device *dev)
+{
+	struct dwc3	*dwc = dev_get_drvdata(dev);
+	u32		reg;
+
+	if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST &&
+			dwc->dis_split_quirk) {
+		dev_dbg(dwc->dev, "set DWC3_GUCTL3_SPLITDISABLE\n");
+		reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
+		reg |= DWC3_GUCTL3_SPLITDISABLE;
+		dwc3_writel(dwc->regs, DWC3_GUCTL3, reg);
+	}
+}
+#else
+#define dwc3_complete NULL
 #endif /* CONFIG_PM_SLEEP */
 
 static const struct dev_pm_ops dwc3_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
+	.complete = dwc3_complete,
 	SET_RUNTIME_PM_OPS(dwc3_runtime_suspend, dwc3_runtime_resume,
 			dwc3_runtime_idle)
 };
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 1528d395b156..28475e301ad9 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -136,6 +136,7 @@
 #define DWC3_GEVNTCOUNT(n)	(0xc40c + ((n) * 0x10))
 
 #define DWC3_GHWPARAMS8		0xc600
+#define DWC3_GUCTL3		0xc60c
 #define DWC3_GFLADJ		0xc630
 
 /* Device Registers */
@@ -370,6 +371,9 @@
 /* Global User Control Register 2 */
 #define DWC3_GUCTL2_RST_ACTBITLATER		BIT(14)
 
+/* Global User Control Register 3 */
+#define DWC3_GUCTL3_SPLITDISABLE		BIT(14)
+
 /* Device Configuration Register */
 #define DWC3_DCFG_DEVADDR(addr)	((addr) << 3)
 #define DWC3_DCFG_DEVADDR_MASK	DWC3_DCFG_DEVADDR(0x7f)
@@ -1030,6 +1034,7 @@ struct dwc3_scratchpad_array {
  * 	2	- No de-emphasis
  * 	3	- Reserved
  * @dis_metastability_quirk: set to disable metastability quirk.
+ * @dis_split_quirk: set to disable split boundary.
  * @imod_interval: set the interrupt moderation interval in 250ns
  *                 increments or 0 to disable.
  */
@@ -1216,6 +1221,8 @@ struct dwc3 {
 
 	unsigned		dis_metastability_quirk:1;
 
+	unsigned		dis_split_quirk:1;
+
 	u16			imod_interval;
 };
 
-- 
2.15.0-rc2

[PATCH v6 05/13] usb: dwc3: Execute GCTL Core Soft Reset while switch mdoe for Hisilicon Kirin Soc

[Yu Chen]

A GCTL soft reset should be executed when switch mode for dwc3 core
of Hisilicon Kirin Soc.

Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Binghui Wang <wangbinghui@hisilicon.com>
Signed-off-by: Yu Chen <chenyu56@huawei.com>
---
v4:
* Add description for 'gctl_reset_quirk'.
---
---
 drivers/usb/dwc3/core.c | 19 +++++++++++++++++++
 drivers/usb/dwc3/core.h |  2 ++
 2 files changed, 21 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index c3ef6bd2b0d4..fd581d72794a 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -112,6 +112,19 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode)
 	dwc->current_dr_role = mode;
 }
 
+static void dwc3_gctl_core_soft_reset(struct dwc3 *dwc)
+{
+	u32 reg;
+
+	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
+	reg |= DWC3_GCTL_CORESOFTRESET;
+	dwc3_writel(dwc->regs, DWC3_GCTL, reg);
+
+	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
+	reg &= ~DWC3_GCTL_CORESOFTRESET;
+	dwc3_writel(dwc->regs, DWC3_GCTL, reg);
+}
+
 static void __dwc3_set_mode(struct work_struct *work)
 {
 	struct dwc3 *dwc = work_to_dwc(work);
@@ -157,6 +170,10 @@ static void __dwc3_set_mode(struct work_struct *work)
 
 	dwc3_set_prtcap(dwc, dwc->desired_dr_role);
 
+	/* Execute a GCTL Core Soft Reset when switch mode */
+	if (dwc->gctl_reset_quirk)
+		dwc3_gctl_core_soft_reset(dwc);
+
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
 	switch (dwc->desired_dr_role) {
@@ -1314,6 +1331,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)
 
 	dwc->dis_split_quirk = device_property_read_bool(dev,
 				"snps,dis-split-quirk");
+	dwc->gctl_reset_quirk = device_property_read_bool(dev,
+				"snps,gctl-reset-quirk");
 
 	dwc->lpm_nyet_threshold = lpm_nyet_threshold;
 	dwc->tx_de_emphasis = tx_de_emphasis;
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 28475e301ad9..6a050d663ec7 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1035,6 +1035,7 @@ struct dwc3_scratchpad_array {
  * 	3	- Reserved
  * @dis_metastability_quirk: set to disable metastability quirk.
  * @dis_split_quirk: set to disable split boundary.
+ * @gctl_reset_quirk: set to do a gctl soft-reset while switch operation mode.
  * @imod_interval: set the interrupt moderation interval in 250ns
  *                 increments or 0 to disable.
  */
@@ -1222,6 +1223,7 @@ struct dwc3 {
 	unsigned		dis_metastability_quirk:1;
 
 	unsigned		dis_split_quirk:1;
+	unsigned		gctl_reset_quirk:1;
 
 	u16			imod_interval;
 };
-- 
2.15.0-rc2

[PATCH v6 06/13] usb: dwc3: Increase timeout for CmdAct cleared by device controller

[Yu Chen]

It needs more time for the device controller to clear the CmdAct of
DEPCMD on Hisilicon Kirin Soc.

Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Binghui Wang <wangbinghui@hisilicon.com>
Signed-off-by: Yu Chen <chenyu56@huawei.com>
---
 drivers/usb/dwc3/gadget.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index e293400cc6e9..9cefb579e161 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -270,7 +270,7 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned cmd,
 {
 	const struct usb_endpoint_descriptor *desc = dep->endpoint.desc;
 	struct dwc3		*dwc = dep->dwc;
-	u32			timeout = 1000;
+	u32			timeout = 5000;
 	u32			saved_config = 0;
 	u32			reg;
 
-- 
2.15.0-rc2

[PATCH v6 07/13] phy: Add usb phy support for hi3660 Soc of Hisilicon

[Yu Chen]

This driver handles usb phy power on and shutdown for hi3660 Soc of
Hisilicon.

Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Pengcheng Li <lpc.li@hisilicon.com>
Cc: Jianguo Sun <sunjianguo1@huawei.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Jiancheng Xue <xuejiancheng@hisilicon.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Binghui Wang <wangbinghui@hisilicon.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Yu Chen <chenyu56@huawei.com>
---
v1:
* Remove unused code and add comment for time delay as suggested by
Kishon Vijay Abraham I.
v2:
* Fix license declaration.
* Remove redundant parens.
* Remove unused member variables in struct hi3660_priv.
v4:
* Add comments for HI3660_USB_DEFAULT_PHY_PARAM.
* Add margin for usleep_range.
* Get regmap of otg_bc from parent's of_node.
---
---
 MAINTAINERS                             |   8 ++
 drivers/phy/hisilicon/Kconfig           |  10 ++
 drivers/phy/hisilicon/Makefile          |   1 +
 drivers/phy/hisilicon/phy-hi3660-usb3.c | 233 ++++++++++++++++++++++++++++++++
 4 files changed, 252 insertions(+)
 create mode 100644 drivers/phy/hisilicon/phy-hi3660-usb3.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d18b4d62cc21..d2c161e7a085 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16084,6 +16084,14 @@ L:	linux-usb@vger.kernel.org
 S:	Maintained
 F:	drivers/usb/roles/intel-xhci-usb-role-switch.c
 
+USB IP DRIVER FOR HISILICON KIRIN
+M:	Yu Chen <chenyu56@huawei.com>
+M:	Binghui Wang <wangbinghui@hisilicon.com>
+L:	linux-usb@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
+F:	drivers/phy/hisilicon/phy-hi3660-usb3.c
+
 USB ISP116X DRIVER
 M:	Olav Kongas <ok@artecdesign.ee>
 L:	linux-usb@vger.kernel.org
diff --git a/drivers/phy/hisilicon/Kconfig b/drivers/phy/hisilicon/Kconfig
index b40ee54a1a50..3c142f08987c 100644
--- a/drivers/phy/hisilicon/Kconfig
+++ b/drivers/phy/hisilicon/Kconfig
@@ -12,6 +12,16 @@ config PHY_HI6220_USB
 
 	  To compile this driver as a module, choose M here.
 
+config PHY_HI3660_USB
+	tristate "hi3660 USB PHY support"
+	depends on (ARCH_HISI && ARM64) || COMPILE_TEST
+	select GENERIC_PHY
+	select MFD_SYSCON
+	help
+	  Enable this to support the HISILICON HI3660 USB PHY.
+
+	  To compile this driver as a module, choose M here.
+
 config PHY_HISTB_COMBPHY
 	tristate "HiSilicon STB SoCs COMBPHY support"
 	depends on (ARCH_HISI && ARM64) || COMPILE_TEST
diff --git a/drivers/phy/hisilicon/Makefile b/drivers/phy/hisilicon/Makefile
index f662a4fe18d8..75ba64e2faf8 100644
--- a/drivers/phy/hisilicon/Makefile
+++ b/drivers/phy/hisilicon/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_PHY_HI6220_USB)		+= phy-hi6220-usb.o
+obj-$(CONFIG_PHY_HI3660_USB)		+= phy-hi3660-usb3.o
 obj-$(CONFIG_PHY_HISTB_COMBPHY)		+= phy-histb-combphy.o
 obj-$(CONFIG_PHY_HISI_INNO_USB2)	+= phy-hisi-inno-usb2.o
 obj-$(CONFIG_PHY_HIX5HD2_SATA)		+= phy-hix5hd2-sata.o
diff --git a/drivers/phy/hisilicon/phy-hi3660-usb3.c b/drivers/phy/hisilicon/phy-hi3660-usb3.c
new file mode 100644
index 000000000000..cc0af2c044d0
--- /dev/null
+++ b/drivers/phy/hisilicon/phy-hi3660-usb3.c
@@ -0,0 +1,233 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Phy provider for USB 3.0 controller on HiSilicon 3660 platform
+ *
+ * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd.
+ *		http://www.huawei.com
+ *
+ * Authors: Yu Chen <chenyu56@huawei.com>
+ */
+
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define PERI_CRG_CLK_EN4			0x40
+#define PERI_CRG_CLK_DIS4			0x44
+#define GT_CLK_USB3OTG_REF			BIT(0)
+#define GT_ACLK_USB3OTG				BIT(1)
+
+#define PERI_CRG_RSTEN4				0x90
+#define PERI_CRG_RSTDIS4			0x94
+#define IP_RST_USB3OTGPHY_POR			BIT(3)
+#define IP_RST_USB3OTG				BIT(5)
+
+#define PERI_CRG_ISODIS				0x148
+#define USB_REFCLK_ISO_EN			BIT(25)
+
+#define PCTRL_PERI_CTRL3			0x10
+#define PCTRL_PERI_CTRL3_MSK_START		16
+#define USB_TCXO_EN				BIT(1)
+
+#define PCTRL_PERI_CTRL24			0x64
+#define SC_CLK_USB3PHY_3MUX1_SEL		BIT(25)
+
+#define USBOTG3_CTRL0				0x00
+#define SC_USB3PHY_ABB_GT_EN			BIT(15)
+
+#define USBOTG3_CTRL2				0x08
+#define USBOTG3CTRL2_POWERDOWN_HSP		BIT(0)
+#define USBOTG3CTRL2_POWERDOWN_SSP		BIT(1)
+
+#define USBOTG3_CTRL3				0x0C
+#define USBOTG3_CTRL3_VBUSVLDEXT		BIT(6)
+#define USBOTG3_CTRL3_VBUSVLDEXTSEL		BIT(5)
+
+#define USBOTG3_CTRL4				0x10
+
+#define USBOTG3_CTRL7				0x1c
+#define REF_SSP_EN				BIT(16)
+
+/* This value config the default txtune parameter of the usb 2.0 phy */
+#define HI3660_USB_DEFAULT_PHY_PARAM		0x1c466e3
+
+struct hi3660_priv {
+	struct device *dev;
+	struct regmap *peri_crg;
+	struct regmap *pctrl;
+	struct regmap *otg_bc;
+	u32 eye_diagram_param;
+};
+
+static int hi3660_phy_init(struct phy *phy)
+{
+	struct hi3660_priv *priv = phy_get_drvdata(phy);
+	u32 val, mask;
+	int ret;
+
+	/* usb refclk iso disable */
+	ret = regmap_write(priv->peri_crg, PERI_CRG_ISODIS, USB_REFCLK_ISO_EN);
+	if (ret)
+		goto out;
+
+	/* enable usb_tcxo_en */
+	val = USB_TCXO_EN | (USB_TCXO_EN << PCTRL_PERI_CTRL3_MSK_START);
+	ret = regmap_write(priv->pctrl, PCTRL_PERI_CTRL3, val);
+	if (ret)
+		goto out;
+
+	/* assert phy */
+	val = IP_RST_USB3OTGPHY_POR | IP_RST_USB3OTG;
+	ret = regmap_write(priv->peri_crg, PERI_CRG_RSTEN4, val);
+	if (ret)
+		goto out;
+
+	/* enable phy ref clk */
+	val = SC_USB3PHY_ABB_GT_EN;
+	mask = val;
+	ret = regmap_update_bits(priv->otg_bc, USBOTG3_CTRL0, mask, val);
+	if (ret)
+		goto out;
+
+	val = REF_SSP_EN;
+	mask = val;
+	ret = regmap_update_bits(priv->otg_bc, USBOTG3_CTRL7, mask, val);
+	if (ret)
+		goto out;
+
+	/* exit from IDDQ mode */
+	mask = USBOTG3CTRL2_POWERDOWN_HSP | USBOTG3CTRL2_POWERDOWN_SSP;
+	ret = regmap_update_bits(priv->otg_bc, USBOTG3_CTRL2, mask, 0);
+	if (ret)
+		goto out;
+
+	/* delay for exit from IDDQ mode */
+	usleep_range(100, 120);
+
+	/* deassert phy */
+	val = IP_RST_USB3OTGPHY_POR | IP_RST_USB3OTG;
+	ret = regmap_write(priv->peri_crg, PERI_CRG_RSTDIS4, val);
+	if (ret)
+		goto out;
+
+	/* delay for phy deasserted */
+	usleep_range(10000, 15000);
+
+	/* fake vbus valid signal */
+	val = USBOTG3_CTRL3_VBUSVLDEXT | USBOTG3_CTRL3_VBUSVLDEXTSEL;
+	mask = val;
+	ret = regmap_update_bits(priv->otg_bc, USBOTG3_CTRL3, mask, val);
+	if (ret)
+		goto out;
+
+	/* delay for vbus valid */
+	usleep_range(100, 120);
+
+	ret = regmap_write(priv->otg_bc, USBOTG3_CTRL4,
+			priv->eye_diagram_param);
+	if (ret)
+		goto out;
+
+	return 0;
+out:
+	dev_err(priv->dev, "failed to init phy ret: %d\n", ret);
+	return ret;
+}
+
+static int hi3660_phy_exit(struct phy *phy)
+{
+	struct hi3660_priv *priv = phy_get_drvdata(phy);
+	u32 val;
+	int ret;
+
+	/* assert phy */
+	val = IP_RST_USB3OTGPHY_POR;
+	ret = regmap_write(priv->peri_crg, PERI_CRG_RSTEN4, val);
+	if (ret)
+		goto out;
+
+	/* disable usb_tcxo_en */
+	val = USB_TCXO_EN << PCTRL_PERI_CTRL3_MSK_START;
+	ret = regmap_write(priv->pctrl, PCTRL_PERI_CTRL3, val);
+	if (ret)
+		goto out;
+
+	return 0;
+out:
+	dev_err(priv->dev, "failed to exit phy ret: %d\n", ret);
+	return ret;
+}
+
+static struct phy_ops hi3660_phy_ops = {
+	.init		= hi3660_phy_init,
+	.exit		= hi3660_phy_exit,
+	.owner		= THIS_MODULE,
+};
+
+static int hi3660_phy_probe(struct platform_device *pdev)
+{
+	struct phy_provider *phy_provider;
+	struct device *dev = &pdev->dev;
+	struct phy *phy;
+	struct hi3660_priv *priv;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+	priv->peri_crg = syscon_regmap_lookup_by_phandle(dev->of_node,
+					"hisilicon,pericrg-syscon");
+	if (IS_ERR(priv->peri_crg)) {
+		dev_err(dev, "no hisilicon,pericrg-syscon\n");
+		return PTR_ERR(priv->peri_crg);
+	}
+
+	priv->pctrl = syscon_regmap_lookup_by_phandle(dev->of_node,
+					"hisilicon,pctrl-syscon");
+	if (IS_ERR(priv->pctrl)) {
+		dev_err(dev, "no hisilicon,pctrl-syscon\n");
+		return PTR_ERR(priv->pctrl);
+	}
+
+	/* node of hi3660 phy is a sub-node of usb3_otg_bc */
+	priv->otg_bc = syscon_node_to_regmap(dev->parent->of_node);
+	if (IS_ERR(priv->otg_bc)) {
+		dev_err(dev, "no hisilicon,usb3-otg-bc-syscon\n");
+		return PTR_ERR(priv->otg_bc);
+	}
+
+	if (of_property_read_u32(dev->of_node, "hisilicon,eye-diagram-param",
+		&(priv->eye_diagram_param)))
+		priv->eye_diagram_param = HI3660_USB_DEFAULT_PHY_PARAM;
+
+	phy = devm_phy_create(dev, NULL, &hi3660_phy_ops);
+	if (IS_ERR(phy))
+		return PTR_ERR(phy);
+
+	phy_set_drvdata(phy, priv);
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id hi3660_phy_of_match[] = {
+	{.compatible = "hisilicon,hi3660-usb-phy",},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, hi3660_phy_of_match);
+
+static struct platform_driver hi3660_phy_driver = {
+	.probe	= hi3660_phy_probe,
+	.driver = {
+		.name	= "hi3660-usb-phy",
+		.of_match_table	= hi3660_phy_of_match,
+	}
+};
+module_platform_driver(hi3660_phy_driver);
+
+MODULE_AUTHOR("Yu Chen <chenyu56@huawei.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Hilisicon Hi3660 USB3 PHY Driver");
-- 
2.15.0-rc2

[PATCH v6 08/13] usb: roles: Introduce stubs for the exiting functions in role.h.

[Yu Chen]

This patch adds stubs for the exiting functions while
CONFIG_USB_ROLE_SWITCH does not enabled.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: John Stultz <john.stultz@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Yu Chen <chenyu56@huawei.com>
---
 include/linux/usb/role.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
index c05ffa6abda9..da2b9641b877 100644
--- a/include/linux/usb/role.h
+++ b/include/linux/usb/role.h
@@ -42,6 +42,8 @@ struct usb_role_switch_desc {
 	bool allow_userspace_control;
 };
 
+
+#if IS_ENABLED(CONFIG_USB_ROLE_SWITCH)
 int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role);
 enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw);
 struct usb_role_switch *usb_role_switch_get(struct device *dev);
@@ -51,5 +53,33 @@ struct usb_role_switch *
 usb_role_switch_register(struct device *parent,
 			 const struct usb_role_switch_desc *desc);
 void usb_role_switch_unregister(struct usb_role_switch *sw);
+#else
+static inline int usb_role_switch_set_role(struct usb_role_switch *sw,
+		enum usb_role role)
+{
+	return 0;
+}
+
+static inline enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
+{
+	return USB_ROLE_NONE;
+}
+
+static inline struct usb_role_switch *usb_role_switch_get(struct device *dev)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline void usb_role_switch_put(struct usb_role_switch *sw) { }
+
+static inline struct usb_role_switch *
+usb_role_switch_register(struct device *parent,
+			 const struct usb_role_switch_desc *desc)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline void usb_role_switch_unregister(struct usb_role_switch *sw) { }
+#endif
 
 #endif /* __LINUX_USB_ROLE_H */
-- 
2.15.0-rc2

[PATCH v6 09/13] usb: roles: Add usb role switch notifier.

[Yu Chen]

This patch adds notifier for drivers want to be informed of the usb role
switch.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: John Stultz <john.stultz@linaro.org>
Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Yu Chen <chenyu56@huawei.com>
---
v5:
* Split the patch into two patches, the first one introduces stubs for
* the
exiting functions, and this patch adds notifier functions.
v6:
* Fold a fix in form John Stultz <john.stultz@linaro.org>.
---
---
 drivers/usb/roles/class.c | 35 ++++++++++++++++++++++++++++++++++-
 include/linux/usb/role.h  | 16 ++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index f45d8df5cfb8..5bc60f478725 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -20,6 +20,7 @@ struct usb_role_switch {
 	struct device dev;
 	struct mutex lock; /* device lock*/
 	enum usb_role role;
+	struct blocking_notifier_head nh;
 
 	/* From descriptor */
 	struct device *usb2_port;
@@ -49,8 +50,10 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role)
 	mutex_lock(&sw->lock);
 
 	ret = sw->set(sw->dev.parent, role);
-	if (!ret)
+	if (!ret) {
 		sw->role = role;
+		blocking_notifier_call_chain(&sw->nh, role, NULL);
+	}
 
 	mutex_unlock(&sw->lock);
 
@@ -58,6 +61,35 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role)
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_set_role);
 
+int usb_role_switch_register_notifier(struct usb_role_switch *sw,
+				      struct notifier_block *nb)
+{
+	int ret = blocking_notifier_chain_register(&sw->nh, nb);
+	enum usb_role role;
+
+	if (ret)
+		return ret;
+
+	/* Initialize the notifier that was just registered */
+	mutex_lock(&sw->lock);
+	if (sw->get)
+		role = sw->get(sw->dev.parent);
+	else
+		role = sw->role;
+	blocking_notifier_call_chain(&sw->nh, role, NULL);
+	mutex_unlock(&sw->lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_role_switch_register_notifier);
+
+int usb_role_switch_unregister_notifier(struct usb_role_switch *sw,
+					struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&sw->nh, nb);
+}
+EXPORT_SYMBOL_GPL(usb_role_switch_unregister_notifier);
+
 /**
  * usb_role_switch_get_role - Get the USB role for a switch
  * @sw: USB role switch
@@ -271,6 +303,7 @@ usb_role_switch_register(struct device *parent,
 		return ERR_PTR(-ENOMEM);
 
 	mutex_init(&sw->lock);
+	BLOCKING_INIT_NOTIFIER_HEAD(&sw->nh);
 
 	sw->allow_userspace_control = desc->allow_userspace_control;
 	sw->usb2_port = desc->usb2_port;
diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
index da2b9641b877..99d8b8e4fe61 100644
--- a/include/linux/usb/role.h
+++ b/include/linux/usb/role.h
@@ -53,6 +53,10 @@ struct usb_role_switch *
 usb_role_switch_register(struct device *parent,
 			 const struct usb_role_switch_desc *desc);
 void usb_role_switch_unregister(struct usb_role_switch *sw);
+int usb_role_switch_register_notifier(struct usb_role_switch *sw,
+				      struct notifier_block *nb);
+int usb_role_switch_unregister_notifier(struct usb_role_switch *sw,
+					struct notifier_block *nb);
 #else
 static inline int usb_role_switch_set_role(struct usb_role_switch *sw,
 		enum usb_role role)
@@ -80,6 +84,18 @@ usb_role_switch_register(struct device *parent,
 }
 
 static inline void usb_role_switch_unregister(struct usb_role_switch *sw) { }
+
+static int usb_role_switch_register_notifier(struct usb_role_switch *sw,
+				      struct notifier_block *nb)
+{
+	return -ENODEV;
+}
+
+static int usb_role_switch_unregister_notifier(struct usb_role_switch *sw,
+					struct notifier_block *nb)
+{
+	return -ENODEV;
+}
 #endif
 
 #endif /* __LINUX_USB_ROLE_H */
-- 
2.15.0-rc2

[PATCH v6 10/13] usb: dwc3: Registering a role switch in the DRD code.

[Yu Chen]

The Type-C drivers use USB role switch API to inform the
system about the negotiated data role, so registering a role
switch in the DRD code in order to support platforms with
USB Type-C connectors.

Cc: Jun Li <lijun.kernel@gmail.com>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Yu Chen <chenyu56@huawei.com>
---
v2:
* Assign fwnode in dwc3_role_switch.
v3:
* Add default mode property for usb role switch.
* Add select USB_ROLE_SWITCH for USB_DWC3_DUAL_ROLE in Kconfig.
* Do usb_role_switch_register only if property "usb-role-switch"
* present.
v4:
* Add description for 'role_sw' and 'role_switch_default_mode'.
v5:
* Init dwc3 mode for usb_role_switch branch.
v6:
* Fold a fix of dwc3_usb_role_switch_get in from John Stultz
  <john.stultz@linaro.org>
---
---
 drivers/usb/dwc3/Kconfig |  1 +
 drivers/usb/dwc3/core.h  |  6 ++++
 drivers/usb/dwc3/drd.c   | 78 +++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 2b1494460d0c..661b1d2efc8b 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -44,6 +44,7 @@ config USB_DWC3_DUAL_ROLE
 	bool "Dual Role mode"
 	depends on ((USB=y || USB=USB_DWC3) && (USB_GADGET=y || USB_GADGET=USB_DWC3))
 	depends on (EXTCON=y || EXTCON=USB_DWC3)
+	select USB_ROLE_SWITCH
 	help
 	  This is the default mode of working of DWC3 controller where
 	  both host and gadget features are enabled.
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 6a050d663ec7..e6d40f79faa7 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -25,6 +25,7 @@
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/otg.h>
+#include <linux/usb/role.h>
 #include <linux/ulpi/interface.h>
 
 #include <linux/phy/phy.h>
@@ -958,6 +959,9 @@ struct dwc3_scratchpad_array {
  * @hsphy_mode: UTMI phy mode, one of following:
  *		- USBPHY_INTERFACE_MODE_UTMI
  *		- USBPHY_INTERFACE_MODE_UTMIW
+ * role_sw: usb_role_switch handle
+ * role_switch_default_mode: default operation mode of controller while
+ *			usb role is USB_ROLE_NONE.
  * @usb2_phy: pointer to USB2 PHY
  * @usb3_phy: pointer to USB3 PHY
  * @usb2_generic_phy: pointer to USB2 PHY
@@ -1091,6 +1095,8 @@ struct dwc3 {
 	struct extcon_dev	*edev;
 	struct notifier_block	edev_nb;
 	enum usb_phy_interface	hsphy_mode;
+	struct usb_role_switch	*role_sw;
+	enum usb_dr_mode	role_switch_default_mode;
 
 	u32			fladj;
 	u32			irq_gadget;
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index 726100d1ac0d..4ef3dd9aae66 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -479,6 +479,58 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
 	return edev;
 }
 
+static int dwc3_usb_role_switch_set(struct device *dev, enum usb_role role)
+{
+	struct dwc3 *dwc = dev_get_drvdata(dev);
+	u32 mode;
+
+	switch (role) {
+	case USB_ROLE_HOST:
+		mode = DWC3_GCTL_PRTCAP_HOST;
+		break;
+	case USB_ROLE_DEVICE:
+		mode = DWC3_GCTL_PRTCAP_DEVICE;
+		break;
+	default:
+		if (dwc->role_switch_default_mode == USB_DR_MODE_HOST)
+			mode = DWC3_GCTL_PRTCAP_HOST;
+		else
+			mode = DWC3_GCTL_PRTCAP_DEVICE;
+		break;
+	}
+
+	dwc3_set_mode(dwc, mode);
+	return 0;
+}
+
+static enum usb_role dwc3_usb_role_switch_get(struct device *dev)
+{
+	struct dwc3 *dwc = dev_get_drvdata(dev);
+	unsigned long flags;
+	enum usb_role role;
+
+	spin_lock_irqsave(&dwc->lock, flags);
+	switch (dwc->current_dr_role) {
+	case DWC3_GCTL_PRTCAP_HOST:
+		role = USB_ROLE_HOST;
+		break;
+	case DWC3_GCTL_PRTCAP_DEVICE:
+		role = USB_ROLE_DEVICE;
+		break;
+	case DWC3_GCTL_PRTCAP_OTG:
+		role = dwc->current_otg_role;
+		break;
+	default:
+		if (dwc->role_switch_default_mode == USB_DR_MODE_HOST)
+			role = USB_ROLE_HOST;
+		else
+			role = USB_ROLE_DEVICE;
+		break;
+	}
+	spin_unlock_irqrestore(&dwc->lock, flags);
+	return role;
+}
+
 int dwc3_drd_init(struct dwc3 *dwc)
 {
 	int ret, irq;
@@ -487,7 +539,28 @@ int dwc3_drd_init(struct dwc3 *dwc)
 	if (IS_ERR(dwc->edev))
 		return PTR_ERR(dwc->edev);
 
-	if (dwc->edev) {
+	if (device_property_read_bool(dwc->dev, "usb-role-switch")) {
+		struct usb_role_switch_desc dwc3_role_switch = {0};
+		u32 mode;
+
+		if (device_property_read_bool(dwc->dev,
+				"role-switch-default-host")) {
+			dwc->role_switch_default_mode = USB_DR_MODE_HOST;
+			mode = DWC3_GCTL_PRTCAP_HOST;
+		} else {
+			dwc->role_switch_default_mode = USB_DR_MODE_PERIPHERAL;
+			mode = DWC3_GCTL_PRTCAP_DEVICE;
+		}
+		dwc3_role_switch.fwnode = dev_fwnode(dwc->dev);
+		dwc3_role_switch.set = dwc3_usb_role_switch_set;
+		dwc3_role_switch.get = dwc3_usb_role_switch_get;
+		dwc->role_sw = usb_role_switch_register(dwc->dev,
+							&dwc3_role_switch);
+		if (IS_ERR(dwc->role_sw))
+			return PTR_ERR(dwc->role_sw);
+
+		dwc3_set_mode(dwc, mode);
+	} else if (dwc->edev) {
 		dwc->edev_nb.notifier_call = dwc3_drd_notifier;
 		ret = extcon_register_notifier(dwc->edev, EXTCON_USB_HOST,
 					       &dwc->edev_nb);
@@ -534,6 +607,9 @@ void dwc3_drd_exit(struct dwc3 *dwc)
 {
 	unsigned long flags;
 
+	if (dwc->role_sw)
+		usb_role_switch_unregister(dwc->role_sw);
+
 	if (dwc->edev)
 		extcon_unregister_notifier(dwc->edev, EXTCON_USB_HOST,
 					   &dwc->edev_nb);
-- 
2.15.0-rc2

[PATCH v6 11/13] hikey960: Support usb functionality of Hikey960

[Yu Chen]

This driver handles usb hub power on and typeC port event of HiKey960 board:
1)DP&DM switching between usb hub and typeC port base on typeC port
state
2)Control power of usb hub on Hikey960
3)Control vbus of typeC port

Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Binghui Wang <wangbinghui@hisilicon.com>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Yu Chen <chenyu56@huawei.com>
---
v1:
* Using gpiod API with the gpios.
* Removing registering usb role switch.
* Registering usb role switch notifier.
v2:
* Fix license declaration.
* Add configuration of  gpio direction.
* Remove some log print.
v3:
* Remove property of "typec_vbus_enable_val".
* Remove gpiod_direction_output and set initial value of gpio by
* devm_gpiod_get.
v4:
* Remove 'linux/of.h' and add 'linux/mod_devicetable.h'.
* Remove unused 'root' of_node.
* Remove unuseful NULL check return by 'devm_gpiod_get'.
* Use 'devm_gpiod_get_optional' to get optional gpio.
---
---
 drivers/misc/Kconfig          |   6 ++
 drivers/misc/Makefile         |   1 +
 drivers/misc/hisi_hikey_usb.c | 162 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 169 insertions(+)
 create mode 100644 drivers/misc/hisi_hikey_usb.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 42ab8ec92a04..3b3f610b80c2 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -532,6 +532,12 @@ config PVPANIC
 	  a paravirtualized device provided by QEMU; it lets a virtual machine
 	  (guest) communicate panic events to the host.
 
+config HISI_HIKEY_USB
+	tristate "USB functionality of HiSilicon Hikey Platform"
+	depends on OF && GPIOLIB
+	help
+	  If you say yes here you get support for usb functionality of HiSilicon Hikey Platform.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index d5b7d3404dc7..1c6c108d3a0c 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -61,3 +61,4 @@ obj-$(CONFIG_OCXL)		+= ocxl/
 obj-y				+= cardreader/
 obj-$(CONFIG_PVPANIC)   	+= pvpanic.o
 obj-$(CONFIG_HABANA_AI)		+= habanalabs/
+obj-$(CONFIG_HISI_HIKEY_USB)	+= hisi_hikey_usb.o
diff --git a/drivers/misc/hisi_hikey_usb.c b/drivers/misc/hisi_hikey_usb.c
new file mode 100644
index 000000000000..a3bb46266f04
--- /dev/null
+++ b/drivers/misc/hisi_hikey_usb.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Support for usb functionality of Hikey series boards
+ * based on Hisilicon Kirin Soc.
+ *
+ * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd.
+ *		http://www.huawei.com
+ *
+ * Authors: Yu Chen <chenyu56@huawei.com>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/usb/role.h>
+
+#define DEVICE_DRIVER_NAME "hisi_hikey_usb"
+
+#define HUB_VBUS_POWER_ON 1
+#define HUB_VBUS_POWER_OFF 0
+#define USB_SWITCH_TO_HUB 1
+#define USB_SWITCH_TO_TYPEC 0
+#define TYPEC_VBUS_POWER_ON 1
+#define TYPEC_VBUS_POWER_OFF 0
+
+struct hisi_hikey_usb {
+	struct gpio_desc *otg_switch;
+	struct gpio_desc *typec_vbus;
+	struct gpio_desc *hub_vbus;
+
+	struct usb_role_switch *role_sw;
+	struct notifier_block nb;
+};
+
+static void hub_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb, int value)
+{
+	gpiod_set_value_cansleep(hisi_hikey_usb->hub_vbus, value);
+}
+
+static void usb_switch_ctrl(struct hisi_hikey_usb *hisi_hikey_usb,
+		int switch_to)
+{
+	gpiod_set_value_cansleep(hisi_hikey_usb->otg_switch, switch_to);
+}
+
+static void usb_typec_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb,
+		int value)
+{
+	gpiod_set_value_cansleep(hisi_hikey_usb->typec_vbus, value);
+}
+
+static int hisi_hikey_role_switch(struct notifier_block *nb,
+			unsigned long state, void *data)
+{
+	struct hisi_hikey_usb *hisi_hikey_usb;
+
+	hisi_hikey_usb = container_of(nb, struct hisi_hikey_usb, nb);
+
+	switch (state) {
+	case USB_ROLE_NONE:
+		usb_typec_power_ctrl(hisi_hikey_usb, TYPEC_VBUS_POWER_OFF);
+		usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_HUB);
+		hub_power_ctrl(hisi_hikey_usb, HUB_VBUS_POWER_ON);
+		break;
+	case USB_ROLE_HOST:
+		usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_TYPEC);
+		usb_typec_power_ctrl(hisi_hikey_usb, TYPEC_VBUS_POWER_ON);
+		break;
+	case USB_ROLE_DEVICE:
+		hub_power_ctrl(hisi_hikey_usb, HUB_VBUS_POWER_OFF);
+		usb_typec_power_ctrl(hisi_hikey_usb, TYPEC_VBUS_POWER_OFF);
+		usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_TYPEC);
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int hisi_hikey_usb_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct hisi_hikey_usb *hisi_hikey_usb;
+	int ret;
+
+	hisi_hikey_usb = devm_kzalloc(dev, sizeof(*hisi_hikey_usb), GFP_KERNEL);
+	if (!hisi_hikey_usb)
+		return -ENOMEM;
+
+	hisi_hikey_usb->nb.notifier_call = hisi_hikey_role_switch;
+
+	hisi_hikey_usb->typec_vbus = devm_gpiod_get(dev, "typec-vbus",
+			GPIOD_OUT_LOW);
+	if (IS_ERR(hisi_hikey_usb->typec_vbus))
+		return PTR_ERR(hisi_hikey_usb->typec_vbus);
+
+	hisi_hikey_usb->otg_switch = devm_gpiod_get(dev, "otg-switch",
+			GPIOD_OUT_HIGH);
+	if (IS_ERR(hisi_hikey_usb->otg_switch))
+		return PTR_ERR(hisi_hikey_usb->otg_switch);
+
+	/* hub-vdd33-en is optional */
+	hisi_hikey_usb->hub_vbus = devm_gpiod_get_optional(dev, "hub-vdd33-en",
+			GPIOD_OUT_HIGH);
+	if (IS_ERR(hisi_hikey_usb->hub_vbus))
+		return PTR_ERR(hisi_hikey_usb->hub_vbus);
+
+	hisi_hikey_usb->role_sw = usb_role_switch_get(dev);
+	if (!hisi_hikey_usb->role_sw)
+		return -EPROBE_DEFER;
+	if (IS_ERR(hisi_hikey_usb->role_sw))
+		return PTR_ERR(hisi_hikey_usb->role_sw);
+
+	ret = usb_role_switch_register_notifier(hisi_hikey_usb->role_sw,
+			&hisi_hikey_usb->nb);
+	if (ret) {
+		usb_role_switch_put(hisi_hikey_usb->role_sw);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, hisi_hikey_usb);
+
+	return 0;
+}
+
+static int  hisi_hikey_usb_remove(struct platform_device *pdev)
+{
+	struct hisi_hikey_usb *hisi_hikey_usb = platform_get_drvdata(pdev);
+
+	usb_role_switch_unregister_notifier(hisi_hikey_usb->role_sw,
+			&hisi_hikey_usb->nb);
+
+	usb_role_switch_put(hisi_hikey_usb->role_sw);
+
+	return 0;
+}
+
+static const struct of_device_id id_table_hisi_hikey_usb[] = {
+	{.compatible = "hisilicon,gpio_hubv1"},
+	{.compatible = "hisilicon,hikey960_usb"},
+	{}
+};
+
+static struct platform_driver hisi_hikey_usb_driver = {
+	.probe = hisi_hikey_usb_probe,
+	.remove = hisi_hikey_usb_remove,
+	.driver = {
+		.name = DEVICE_DRIVER_NAME,
+		.of_match_table = id_table_hisi_hikey_usb,
+	},
+};
+
+module_platform_driver(hisi_hikey_usb_driver);
+
+MODULE_AUTHOR("Yu Chen <chenyu56@huawei.com>");
+MODULE_DESCRIPTION("Driver Support for USB functionality of Hikey");
+MODULE_LICENSE("GPL v2");
-- 
2.15.0-rc2

[PATCH v6 12/13] usb: gadget: Add configfs attribuite for controling match_existing_only

[Yu Chen]

Currently the "match_existing_only" of usb_gadget_driver in configfs is
set to one which is not flexible.
Dwc3 udc will be removed when usb core switch to host mode. This causes
failure of writing name of dwc3 udc to configfs's UDC attribuite.
To fix this we need to add a way to change the config of
"match_existing_only".
There are systems like Android do not support udev, so adding
"match_existing_only" attribute to allow configuration by user is cost little.
This patch adds a configfs attribuite for controling match_existing_only
which allow user to config "match_existing_only".

Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Binghui Wang <wangbinghui@hisilicon.com>
Signed-off-by: Yu Chen <chenyu56@huawei.com>
---
 drivers/usb/gadget/configfs.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 025129942894..be85104bfab9 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -291,6 +291,36 @@ static ssize_t gadget_dev_desc_UDC_store(struct config_item *item,
 	return ret;
 }
 
+static ssize_t gadget_driver_match_existing_only_store(struct config_item *item,
+		const char *page, size_t len)
+{
+	struct gadget_info *gi = to_gadget_info(item);
+	struct usb_gadget_driver *gadget_driver = &(gi->composite.gadget_driver);
+	bool match_existing_only;
+	int ret;
+
+	ret = kstrtobool(page, &match_existing_only);
+	if (ret)
+		return ret;
+
+	if (match_existing_only)
+		gadget_driver->match_existing_only = 1;
+	else
+		gadget_driver->match_existing_only = 0;
+
+	return len;
+}
+
+static ssize_t gadget_driver_match_existing_only_show(struct config_item *item,
+		char *page)
+{
+	struct gadget_info *gi = to_gadget_info(item);
+	struct usb_gadget_driver *gadget_driver = &(gi->composite.gadget_driver);
+	bool match_existing_only = !!gadget_driver->match_existing_only;
+
+	return sprintf(page, "%s\n", match_existing_only ? "true" : "false");
+}
+
 CONFIGFS_ATTR(gadget_dev_desc_, bDeviceClass);
 CONFIGFS_ATTR(gadget_dev_desc_, bDeviceSubClass);
 CONFIGFS_ATTR(gadget_dev_desc_, bDeviceProtocol);
@@ -300,6 +330,7 @@ CONFIGFS_ATTR(gadget_dev_desc_, idProduct);
 CONFIGFS_ATTR(gadget_dev_desc_, bcdDevice);
 CONFIGFS_ATTR(gadget_dev_desc_, bcdUSB);
 CONFIGFS_ATTR(gadget_dev_desc_, UDC);
+CONFIGFS_ATTR(gadget_, driver_match_existing_only);
 
 static struct configfs_attribute *gadget_root_attrs[] = {
 	&gadget_dev_desc_attr_bDeviceClass,
@@ -311,6 +342,7 @@ static struct configfs_attribute *gadget_root_attrs[] = {
 	&gadget_dev_desc_attr_bcdDevice,
 	&gadget_dev_desc_attr_bcdUSB,
 	&gadget_dev_desc_attr_UDC,
+	&gadget_attr_driver_match_existing_only,
 	NULL,
 };
 
-- 
2.15.0-rc2

[PATCH v6 13/13] dts: hi3660: Add support for usb on Hikey960

[Yu Chen]

This patch adds support for usb on Hikey960.

Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: John Stultz <john.stultz@linaro.org>
Cc: Binghui Wang <wangbinghui@hisilicon.com>
Signed-off-by: Yu Chen <chenyu56@huawei.com>
---
v2:
* Remove device_type property.
* Add property "usb-role-switch".
v3:
* Make node "usb_phy" a subnode of usb3_otg_bc register.
* Remove property "typec-vbus-enable-val" of hisi_hikey_usb.
v4:
* Remove property "hisilicon,usb3-otg-bc-syscon" of usb-phy.
---
---
 arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 53 ++++++++++++++++
 arch/arm64/boot/dts/hisilicon/hi3660.dtsi         | 73 +++++++++++++++++++++++
 2 files changed, 126 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
index e035cf195b19..d4e11c56b250 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
@@ -13,6 +13,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/usb/pd.h>
 
 / {
 	model = "HiKey960";
@@ -196,6 +197,26 @@
 			method = "smc";
 		};
 	};
+
+	hisi_hikey_usb: hisi_hikey_usb {
+		compatible = "hisilicon,hikey960_usb";
+		typec-vbus-gpios = <&gpio25 2 GPIO_ACTIVE_HIGH>;
+		otg-switch-gpios = <&gpio25 6 GPIO_ACTIVE_HIGH>;
+		hub-vdd33-en-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&usbhub5734_pmx_func>;
+
+		port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			hikey_usb_ep: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&dwc3_role_switch_notify>;
+			};
+		};
+	};
+
 };
 
 /*
@@ -526,6 +547,38 @@
 &i2c1 {
 	status = "okay";
 
+	rt1711h: rt1711h@4e {
+		compatible = "richtek,rt1711h";
+		reg = <0x4e>;
+		status = "ok";
+		interrupt-parent = <&gpio27>;
+		interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&usb_cfg_func>;
+
+		usb_con: connector {
+			compatible = "usb-c-connector";
+			label = "USB-C";
+			data-role = "dual";
+			power-role = "dual";
+			try-power-role = "sink";
+			source-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)>;
+			sink-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)
+				PDO_VAR(5000, 5000, 1000)>;
+			op-sink-microwatt = <10000000>;
+		};
+
+		port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			rt1711h_ep: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&dwc3_role_switch>;
+			};
+		};
+	};
+
 	adv7533: adv7533@39 {
 		status = "ok";
 		compatible = "adi,adv7533";
diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index 2f19e0e5b7cf..173467505ada 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -355,6 +355,12 @@
 			#clock-cells = <1>;
 		};
 
+		pmctrl: pmctrl@fff31000 {
+			compatible = "hisilicon,hi3660-pmctrl", "syscon";
+			reg = <0x0 0xfff31000 0x0 0x1000>;
+			#clock-cells = <1>;
+		};
+
 		pmuctrl: crg_ctrl@fff34000 {
 			compatible = "hisilicon,hi3660-pmuctrl", "syscon";
 			reg = <0x0 0xfff34000 0x0 0x1000>;
@@ -1134,5 +1140,72 @@
 				};
 			};
 		};
+
+		usb3_otg_bc: usb3_otg_bc@ff200000 {
+			compatible = "syscon", "simple-mfd";
+			reg = <0x0 0xff200000 0x0 0x1000>;
+
+			usb_phy: usb-phy {
+				compatible = "hisilicon,hi3660-usb-phy";
+				#phy-cells = <0>;
+				hisilicon,pericrg-syscon = <&crg_ctrl>;
+				hisilicon,pctrl-syscon = <&pctrl>;
+				hisilicon,eye-diagram-param = <0x22466e4>;
+			};
+		};
+
+		usb3: hisi_dwc3 {
+			compatible = "hisilicon,hi3660-dwc3";
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
+
+			clocks = <&crg_ctrl HI3660_CLK_ABB_USB>,
+				 <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
+			clock-names = "clk_usb3phy_ref", "aclk_usb3otg";
+
+			assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
+			assigned-clock-rates = <229000000>;
+			resets = <&crg_rst 0x90 8>,
+				 <&crg_rst 0x90 7>,
+				 <&crg_rst 0x90 6>,
+				 <&crg_rst 0x90 5>;
+
+			dwc3: dwc3@ff100000 {
+				compatible = "snps,dwc3";
+				reg = <0x0 0xff100000 0x0 0x100000>;
+				interrupts = <0 159 4>, <0 161 4>;
+				phys = <&usb_phy>;
+				phy-names = "usb3-phy";
+				dr_mode = "otg";
+				maximum-speed = "super-speed";
+				phy_type = "utmi";
+				snps,dis-del-phy-power-chg-quirk;
+				snps,lfps_filter_quirk;
+				snps,dis_u2_susphy_quirk;
+				snps,dis_u3_susphy_quirk;
+				snps,tx_de_emphasis_quirk;
+				snps,tx_de_emphasis = <1>;
+				snps,dis_enblslpm_quirk;
+				snps,gctl-reset-quirk;
+				usb-role-switch;
+				role-switch-default-host;
+
+				port {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					dwc3_role_switch: endpoint@0 {
+						reg = <0>;
+						remote-endpoint = <&rt1711h_ep>;
+					};
+
+					dwc3_role_switch_notify: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&hikey_usb_ep>;
+					};
+				};
+			};
+		};
 	};
 };
-- 
2.15.0-rc2

Re: [PATCH v6 02/13] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960

[Rob Herring]

On Sat, Apr 20, 2019 at 02:40:08PM +0800, Yu Chen wrote:
> This patch adds binding documentation to support usb hub and usb
> data role switch of Hisilicon HiKey960 Board.

Sorry I've been slow to really review this, but I needed to look at the 
schematics to see what exactly is going on here.

I think this needs some changes to better reflect the h/w and utilize 
existing bindings. It should really be designed ignoring the muxing to 
start with. Define the binding for the TypeC connector and then the host 
hub and make sure they can coexist. Then overlay what you need to switch 
between the 2 modes which AFAICT is just a single GPIO. 

> 
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Binghui Wang <wangbinghui@hisilicon.com>
> Signed-off-by: Yu Chen <chenyu56@huawei.com>
> ---
> v1:
> * Fix some format errors as suggested by Sergei.
> * Modify gpio description to use gpiod API.
> v2:
> * Remove information about Hikey.
> * Fix gpio description.
> * Remove device_type of endpoint.
> v3:
> * Remove property typec-vbus-enable-val.
> * Add description of pinctrl-names.
> * Add example for "hisilicon,gpio-hubv1"
> * Add flag in gpiod properties.
> ---
> ---
>  .../bindings/misc/hisilicon-hikey-usb.txt          | 52 ++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
> 
> diff --git a/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
> new file mode 100644
> index 000000000000..422e844df719
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
> @@ -0,0 +1,52 @@
> +Support usb hub and usb data role switch of Hisilicon HiKey960 Board.
> +
> +-----------------------------
> +
> +Required properties:
> +- compatible: "hisilicon,gpio-hubv1","hisilicon,hikey960-usb"
> +- typec-vbus-gpios: gpio to control the vbus of typeC port

This should be a gpio regulator and then connected to 'vbus-supply' in a 
usb-connector node (see .../bindings/connectors/usb-connector.txt). 

Then you also need the RT1711HWSC TypeC controller in DT. That is 
typically the parent device of the connector node. 

> +- otg-switch-gpios: gpio to switch DP & DM between the hub and typeC port

This probably belongs in USB controller node.

> +- hub-vdd33-en-gpios: gpio to enable the power of hub

This too should be a gpio regulator and then in a hub node. We have 2 
ways to represent hubs. Either as an I2C device or as a child of the 
host controller. The latter is preferred, but I'm not too sure how the 
OF graph connection linking the controller to the TypeC connector will 
work with the usb bus binding.

> +- pinctrl-names: pin configuration state name ("default")
> +- pinctrl-0: pinctrl config
> +
> +Example
> +-----
> +
> +	hisi_hikey_usb: hisi_hikey_usb {
> +		compatible = "hisilicon,hikey960-usb";
> +		typec-vbus-gpios = <&gpio25 2 GPIO_ACTIVE_HIGH>;
> +		otg-switch-gpios = <&gpio25 6 GPIO_ACTIVE_HIGH>;
> +		hub-vdd33-en-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&usbhub5734_pmx_func>;
> +
> +		port {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			hikey_usb_ep: endpoint@0 {
> +				reg = <0>;
> +				remote-endpoint = <&dwc3_role_switch_notify>;
> +			};
> +		};
> +	};
> +
> +	hisi_hikey_usb: hisi_hikey_usb {

Perhaps doesn't matter with my above feedback, but I don't understand 
the point of this 2nd example.

> +		compatible = "hisilicon,gpio-hubv1";
> +		typec-vbus-gpios = <&gpio25 2 GPIO_ACTIVE_LOW>;
> +		otg-switch-gpios = <&gpio25 6 GPIO_ACTIVE_HIGH>;
> +		hub-vdd33-en-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&usbhub5734_pmx_func>;
> +
> +		port {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			hikey_usb_ep: endpoint@0 {
> +				reg = <0>;
> +				remote-endpoint = <&dwc3_role_switch_notify>;
> +			};
> +		};
> +	};
> -- 
> 2.15.0-rc2
> 

Re: [PATCH v6 03/13] usb: dwc3: dwc3-of-simple: Add support for dwc3 of Hisilicon Soc Platform

[Rob Herring]

On Sat, Apr 20, 2019 at 02:40:09PM +0800, Yu Chen wrote:
> This patch adds support for the poweron and shutdown of dwc3 core
> on Hisilicon Soc Platform.
> 
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Yu Chen <chenyu56@huawei.com>
> ---
>  drivers/usb/dwc3/dwc3-of-simple.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
> index 4c2771c5e727..0ed09d876542 100644
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -98,7 +98,8 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
>  	 * Some controllers need to toggle the usb3-otg reset before trying to
>  	 * initialize the PHY, otherwise the PHY times out.
>  	 */
> -	if (of_device_is_compatible(np, "rockchip,rk3399-dwc3"))
> +	if (of_device_is_compatible(np, "rockchip,rk3399-dwc3") ||
> +			of_device_is_compatible(np, "hisilicon,hi3660-dwc3"))
>  		simple->need_reset = true;

Fine, I guess, but really this flag should be moved to the match table 
so we aren't doing compatible matching twice.

>  
>  	if (of_device_is_compatible(np, "amlogic,meson-axg-dwc3") ||
> @@ -243,6 +244,7 @@ static const struct of_device_id of_dwc3_simple_match[] = {
>  	{ .compatible = "amlogic,meson-axg-dwc3" },
>  	{ .compatible = "amlogic,meson-gxl-dwc3" },
>  	{ .compatible = "allwinner,sun50i-h6-dwc3" },
> +	{ .compatible = "hisilicon,hi3660-dwc3" },
>  	{ /* Sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, of_dwc3_simple_match);
> -- 
> 2.15.0-rc2
> 

Re: [PATCH v6 13/13] dts: hi3660: Add support for usb on Hikey960

[Rob Herring]

On Sat, Apr 20, 2019 at 02:40:19PM +0800, Yu Chen wrote:
> This patch adds support for usb on Hikey960.
> 
> Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Binghui Wang <wangbinghui@hisilicon.com>
> Signed-off-by: Yu Chen <chenyu56@huawei.com>
> ---
> v2:
> * Remove device_type property.
> * Add property "usb-role-switch".
> v3:
> * Make node "usb_phy" a subnode of usb3_otg_bc register.
> * Remove property "typec-vbus-enable-val" of hisi_hikey_usb.
> v4:
> * Remove property "hisilicon,usb3-otg-bc-syscon" of usb-phy.
> ---
> ---
>  arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 53 ++++++++++++++++
>  arch/arm64/boot/dts/hisilicon/hi3660.dtsi         | 73 +++++++++++++++++++++++
>  2 files changed, 126 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
> index e035cf195b19..d4e11c56b250 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
> @@ -13,6 +13,7 @@
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/input/input.h>
>  #include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/usb/pd.h>
>  
>  / {
>  	model = "HiKey960";
> @@ -196,6 +197,26 @@
>  			method = "smc";
>  		};
>  	};
> +
> +	hisi_hikey_usb: hisi_hikey_usb {
> +		compatible = "hisilicon,hikey960_usb";
> +		typec-vbus-gpios = <&gpio25 2 GPIO_ACTIVE_HIGH>;
> +		otg-switch-gpios = <&gpio25 6 GPIO_ACTIVE_HIGH>;
> +		hub-vdd33-en-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&usbhub5734_pmx_func>;
> +
> +		port {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			hikey_usb_ep: endpoint@0 {
> +				reg = <0>;
> +				remote-endpoint = <&dwc3_role_switch_notify>;
> +			};
> +		};
> +	};
> +
>  };
>  
>  /*
> @@ -526,6 +547,38 @@
>  &i2c1 {
>  	status = "okay";
>  
> +	rt1711h: rt1711h@4e {
> +		compatible = "richtek,rt1711h";

The binding doc for this should state it should have a 'connector' node.

> +		reg = <0x4e>;
> +		status = "ok";

Can drop this, it is the default.

> +		interrupt-parent = <&gpio27>;
> +		interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&usb_cfg_func>;
> +
> +		usb_con: connector {
> +			compatible = "usb-c-connector";
> +			label = "USB-C";
> +			data-role = "dual";
> +			power-role = "dual";
> +			try-power-role = "sink";
> +			source-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)>;
> +			sink-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)
> +				PDO_VAR(5000, 5000, 1000)>;
> +			op-sink-microwatt = <10000000>;
> +		};
> +
> +		port {

The connector node should have a 'ports' child with 'port@0' being the 
HS connection.

> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			rt1711h_ep: endpoint@0 {
> +				reg = <0>;
> +				remote-endpoint = <&dwc3_role_switch>;
> +			};
> +		};
> +	};
> +
>  	adv7533: adv7533@39 {
>  		status = "ok";
>  		compatible = "adi,adv7533";
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> index 2f19e0e5b7cf..173467505ada 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> @@ -355,6 +355,12 @@
>  			#clock-cells = <1>;
>  		};
>  
> +		pmctrl: pmctrl@fff31000 {
> +			compatible = "hisilicon,hi3660-pmctrl", "syscon";
> +			reg = <0x0 0xfff31000 0x0 0x1000>;
> +			#clock-cells = <1>;
> +		};
> +
>  		pmuctrl: crg_ctrl@fff34000 {
>  			compatible = "hisilicon,hi3660-pmuctrl", "syscon";
>  			reg = <0x0 0xfff34000 0x0 0x1000>;
> @@ -1134,5 +1140,72 @@
>  				};
>  			};
>  		};
> +
> +		usb3_otg_bc: usb3_otg_bc@ff200000 {
> +			compatible = "syscon", "simple-mfd";
> +			reg = <0x0 0xff200000 0x0 0x1000>;
> +
> +			usb_phy: usb-phy {
> +				compatible = "hisilicon,hi3660-usb-phy";
> +				#phy-cells = <0>;
> +				hisilicon,pericrg-syscon = <&crg_ctrl>;
> +				hisilicon,pctrl-syscon = <&pctrl>;
> +				hisilicon,eye-diagram-param = <0x22466e4>;
> +			};
> +		};
> +
> +		usb3: hisi_dwc3 {
> +			compatible = "hisilicon,hi3660-dwc3";
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;

If there are not any wrapper registers, then get rid of the hisi_dwc3 
node. For just clocks and resets we just put everything in one node.

> +
> +			clocks = <&crg_ctrl HI3660_CLK_ABB_USB>,
> +				 <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
> +			clock-names = "clk_usb3phy_ref", "aclk_usb3otg";
> +
> +			assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
> +			assigned-clock-rates = <229000000>;
> +			resets = <&crg_rst 0x90 8>,
> +				 <&crg_rst 0x90 7>,
> +				 <&crg_rst 0x90 6>,
> +				 <&crg_rst 0x90 5>;
> +
> +			dwc3: dwc3@ff100000 {
> +				compatible = "snps,dwc3";
> +				reg = <0x0 0xff100000 0x0 0x100000>;
> +				interrupts = <0 159 4>, <0 161 4>;
> +				phys = <&usb_phy>;
> +				phy-names = "usb3-phy";
> +				dr_mode = "otg";
> +				maximum-speed = "super-speed";
> +				phy_type = "utmi";
> +				snps,dis-del-phy-power-chg-quirk;

> +				snps,lfps_filter_quirk;
> +				snps,dis_u2_susphy_quirk;
> +				snps,dis_u3_susphy_quirk;
> +				snps,tx_de_emphasis_quirk;
> +				snps,tx_de_emphasis = <1>;
> +				snps,dis_enblslpm_quirk;

Pretty sure these aren't documented because we don't use '_' in property 
names.

> +				snps,gctl-reset-quirk;
> +				usb-role-switch;
> +				role-switch-default-host;
> +
> +				port {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					dwc3_role_switch: endpoint@0 {
> +						reg = <0>;
> +						remote-endpoint = <&rt1711h_ep>;
> +					};
> +
> +					dwc3_role_switch_notify: endpoint@1 {
> +						reg = <1>;
> +						remote-endpoint = <&hikey_usb_ep>;
> +					};
> +				};
> +			};
> +		};
>  	};
>  };
> -- 
> 2.15.0-rc2
> 

Re: [PATCH v6 00/13] Add support for usb on Hikey960

[Valentin Schneider]

Hi,

On 20/04/2019 07:40, Yu Chen wrote:
> The patchset adds support for usb functionality of Hikey960, includes:
> - usb phy driver for Hisilicon Kirin Soc hi3660
> - usb driver for HiKey960 board
> - some adjustment in dwc3 and usb role driver
> - dts for support usb of HiKey960
> 

Still works fine on my Debian setup:

Tested-by: Valentin Schneider <valentin.schneider@arm.com>

Thanks,
Valentin

Re: [PATCH v6 02/13] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960

[Chen Yu]

Hi Rob,

On 2019/4/26 5:35, Rob Herring wrote:
> On Sat, Apr 20, 2019 at 02:40:08PM +0800, Yu Chen wrote:
>> This patch adds binding documentation to support usb hub and usb
>> data role switch of Hisilicon HiKey960 Board.
> 
> Sorry I've been slow to really review this, but I needed to look at the 
> schematics to see what exactly is going on here.
> 
> I think this needs some changes to better reflect the h/w and utilize 
> existing bindings. It should really be designed ignoring the muxing to 
> start with. Define the binding for the TypeC connector and then the host 
> hub and make sure they can coexist. Then overlay what you need to switch 
> between the 2 modes which AFAICT is just a single GPIO. 
> 
>>
>> Cc: Kishon Vijay Abraham I <kishon@ti.com>
>> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: John Stultz <john.stultz@linaro.org>
>> Cc: Binghui Wang <wangbinghui@hisilicon.com>
>> Signed-off-by: Yu Chen <chenyu56@huawei.com>
>> ---
>> v1:
>> * Fix some format errors as suggested by Sergei.
>> * Modify gpio description to use gpiod API.
>> v2:
>> * Remove information about Hikey.
>> * Fix gpio description.
>> * Remove device_type of endpoint.
>> v3:
>> * Remove property typec-vbus-enable-val.
>> * Add description of pinctrl-names.
>> * Add example for "hisilicon,gpio-hubv1"
>> * Add flag in gpiod properties.
>> ---
>> ---
>>  .../bindings/misc/hisilicon-hikey-usb.txt          | 52 ++++++++++++++++++++++
>>  1 file changed, 52 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
>>
>> diff --git a/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
>> new file mode 100644
>> index 000000000000..422e844df719
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
>> @@ -0,0 +1,52 @@
>> +Support usb hub and usb data role switch of Hisilicon HiKey960 Board.
>> +
>> +-----------------------------
>> +
>> +Required properties:
>> +- compatible: "hisilicon,gpio-hubv1","hisilicon,hikey960-usb"
>> +- typec-vbus-gpios: gpio to control the vbus of typeC port
> 
> This should be a gpio regulator and then connected to 'vbus-supply' in a 
> usb-connector node (see .../bindings/connectors/usb-connector.txt). 
Currently usb-connector node has no "vbus-supply" property and
I do not find process that handles vbus-supply in RT1711H TypeC driver.

> 
> Then you also need the RT1711HWSC TypeC controller in DT. That is 
> typically the parent device of the connector node. 
> 
>> +- otg-switch-gpios: gpio to switch DP & DM between the hub and typeC port
> 
> This probably belongs in USB controller node.
> 
The otg-switch-gpios controls a mux like fsusb30mux. It is related to
the board design of HiKey960. And the state of the mux is decided by
the typeC port state. So I think it is not so good to make it belongs
in USB controller node.

>> +- hub-vdd33-en-gpios: gpio to enable the power of hub
> 
> This too should be a gpio regulator and then in a hub node. We have 2 
> ways to represent hubs. Either as an I2C device or as a child of the 
> host controller. The latter is preferred, but I'm not too sure how the 
> OF graph connection linking the controller to the TypeC connector will 
> work with the usb bus binding.
> 
There is no particular code except the power control for the hub.
The i2c on the hub is not used. So it can not be an I2C device.
Is there such an example that make the hub as a child of the host controller
and control its power?

>> +- pinctrl-names: pin configuration state name ("default")
>> +- pinctrl-0: pinctrl config
>> +
>> +Example
>> +-----
>> +
>> +	hisi_hikey_usb: hisi_hikey_usb {
>> +		compatible = "hisilicon,hikey960-usb";
>> +		typec-vbus-gpios = <&gpio25 2 GPIO_ACTIVE_HIGH>;
>> +		otg-switch-gpios = <&gpio25 6 GPIO_ACTIVE_HIGH>;
>> +		hub-vdd33-en-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&usbhub5734_pmx_func>;
>> +
>> +		port {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			hikey_usb_ep: endpoint@0 {
>> +				reg = <0>;
>> +				remote-endpoint = <&dwc3_role_switch_notify>;
>> +			};
>> +		};
>> +	};
>> +
>> +	hisi_hikey_usb: hisi_hikey_usb {
> 
> Perhaps doesn't matter with my above feedback, but I don't understand 
> the point of this 2nd example.
> 
OK. I will remove it.
>> +		compatible = "hisilicon,gpio-hubv1";
>> +		typec-vbus-gpios = <&gpio25 2 GPIO_ACTIVE_LOW>;
>> +		otg-switch-gpios = <&gpio25 6 GPIO_ACTIVE_HIGH>;
>> +		hub-vdd33-en-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&usbhub5734_pmx_func>;
>> +
>> +		port {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			hikey_usb_ep: endpoint@0 {
>> +				reg = <0>;
>> +				remote-endpoint = <&dwc3_role_switch_notify>;
>> +			};
>> +		};
>> +	};
>> -- 
>> 2.15.0-rc2
>>
> 
> .
> 

Thanks
- Yu Chen

Re: [PATCH v6 13/13] dts: hi3660: Add support for usb on Hikey960

[Chen Yu]

Hi Rob,

On 2019/4/26 6:00, Rob Herring wrote:
> On Sat, Apr 20, 2019 at 02:40:19PM +0800, Yu Chen wrote:
>> This patch adds support for usb on Hikey960.
>>
>> Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
>> Cc: Wei Xu <xuwei5@hisilicon.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: John Stultz <john.stultz@linaro.org>
>> Cc: Binghui Wang <wangbinghui@hisilicon.com>
>> Signed-off-by: Yu Chen <chenyu56@huawei.com>
>> ---
>> v2:
>> * Remove device_type property.
>> * Add property "usb-role-switch".
>> v3:
>> * Make node "usb_phy" a subnode of usb3_otg_bc register.
>> * Remove property "typec-vbus-enable-val" of hisi_hikey_usb.
>> v4:
>> * Remove property "hisilicon,usb3-otg-bc-syscon" of usb-phy.
>> ---
>> ---
>>  arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 53 ++++++++++++++++
>>  arch/arm64/boot/dts/hisilicon/hi3660.dtsi         | 73 +++++++++++++++++++++++
>>  2 files changed, 126 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>> index e035cf195b19..d4e11c56b250 100644
>> --- a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>> +++ b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>> @@ -13,6 +13,7 @@
>>  #include <dt-bindings/gpio/gpio.h>
>>  #include <dt-bindings/input/input.h>
>>  #include <dt-bindings/interrupt-controller/irq.h>
>> +#include <dt-bindings/usb/pd.h>
>>  
>>  / {
>>  	model = "HiKey960";
>> @@ -196,6 +197,26 @@
>>  			method = "smc";
>>  		};
>>  	};
>> +
>> +	hisi_hikey_usb: hisi_hikey_usb {
>> +		compatible = "hisilicon,hikey960_usb";
>> +		typec-vbus-gpios = <&gpio25 2 GPIO_ACTIVE_HIGH>;
>> +		otg-switch-gpios = <&gpio25 6 GPIO_ACTIVE_HIGH>;
>> +		hub-vdd33-en-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&usbhub5734_pmx_func>;
>> +
>> +		port {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			hikey_usb_ep: endpoint@0 {
>> +				reg = <0>;
>> +				remote-endpoint = <&dwc3_role_switch_notify>;
>> +			};
>> +		};
>> +	};
>> +
>>  };
>>  
>>  /*
>> @@ -526,6 +547,38 @@
>>  &i2c1 {
>>  	status = "okay";
>>  
>> +	rt1711h: rt1711h@4e {
>> +		compatible = "richtek,rt1711h";
> 
> The binding doc for this should state it should have a 'connector' node.
> 
Hi shufan,
Is the 'connector' node an essential node of rt1711h?
Currently it is missing in Documentation/devicetree/bindings/usb/richtek,rt1711h.txt.
Do you think the 'connector' node should add as this patch in the binding doc?

>> +		reg = <0x4e>;
>> +		status = "ok";
> 
> Can drop this, it is the default.
> 
OK.

>> +		interrupt-parent = <&gpio27>;
>> +		interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&usb_cfg_func>;
>> +
>> +		usb_con: connector {
>> +			compatible = "usb-c-connector";
>> +			label = "USB-C";
>> +			data-role = "dual";
>> +			power-role = "dual";
>> +			try-power-role = "sink";
>> +			source-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)>;
>> +			sink-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)
>> +				PDO_VAR(5000, 5000, 1000)>;
>> +			op-sink-microwatt = <10000000>;
>> +		};
>> +
>> +		port {
> 
> The connector node should have a 'ports' child with 'port@0' being the 
> HS connection.
> 
This port and endpoint of the port are used for role_switch matching by
fwnode_graph_devcon_match. I'm not too sure the 'ports' under connector is used in
rt1711h driver？
Hi shufan_lee,
    Can you confirm this?

>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			rt1711h_ep: endpoint@0 {
>> +				reg = <0>;
>> +				remote-endpoint = <&dwc3_role_switch>;
>> +			};
>> +		};
>> +	};
>> +
>>  	adv7533: adv7533@39 {
>>  		status = "ok";
>>  		compatible = "adi,adv7533";
>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> index 2f19e0e5b7cf..173467505ada 100644
>> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> @@ -355,6 +355,12 @@
>>  			#clock-cells = <1>;
>>  		};
>>  
>> +		pmctrl: pmctrl@fff31000 {
>> +			compatible = "hisilicon,hi3660-pmctrl", "syscon";
>> +			reg = <0x0 0xfff31000 0x0 0x1000>;
>> +			#clock-cells = <1>;
>> +		};
>> +
>>  		pmuctrl: crg_ctrl@fff34000 {
>>  			compatible = "hisilicon,hi3660-pmuctrl", "syscon";
>>  			reg = <0x0 0xfff34000 0x0 0x1000>;
>> @@ -1134,5 +1140,72 @@
>>  				};
>>  			};
>>  		};
>> +
>> +		usb3_otg_bc: usb3_otg_bc@ff200000 {
>> +			compatible = "syscon", "simple-mfd";
>> +			reg = <0x0 0xff200000 0x0 0x1000>;
>> +
>> +			usb_phy: usb-phy {
>> +				compatible = "hisilicon,hi3660-usb-phy";
>> +				#phy-cells = <0>;
>> +				hisilicon,pericrg-syscon = <&crg_ctrl>;
>> +				hisilicon,pctrl-syscon = <&pctrl>;
>> +				hisilicon,eye-diagram-param = <0x22466e4>;
>> +			};
>> +		};
>> +
>> +		usb3: hisi_dwc3 {
>> +			compatible = "hisilicon,hi3660-dwc3";
>> +			#address-cells = <2>;
>> +			#size-cells = <2>;
>> +			ranges;
> 
> If there are not any wrapper registers, then get rid of the hisi_dwc3 
> node. For just clocks and resets we just put everything in one node.
> 
I will try to fix this.
>> +
>> +			clocks = <&crg_ctrl HI3660_CLK_ABB_USB>,
>> +				 <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
>> +			clock-names = "clk_usb3phy_ref", "aclk_usb3otg";
>> +
>> +			assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
>> +			assigned-clock-rates = <229000000>;
>> +			resets = <&crg_rst 0x90 8>,
>> +				 <&crg_rst 0x90 7>,
>> +				 <&crg_rst 0x90 6>,
>> +				 <&crg_rst 0x90 5>;
>> +
>> +			dwc3: dwc3@ff100000 {
>> +				compatible = "snps,dwc3";
>> +				reg = <0x0 0xff100000 0x0 0x100000>;
>> +				interrupts = <0 159 4>, <0 161 4>;
>> +				phys = <&usb_phy>;
>> +				phy-names = "usb3-phy";
>> +				dr_mode = "otg";
>> +				maximum-speed = "super-speed";
>> +				phy_type = "utmi";
>> +				snps,dis-del-phy-power-chg-quirk;
> 
>> +				snps,lfps_filter_quirk;
>> +				snps,dis_u2_susphy_quirk;
>> +				snps,dis_u3_susphy_quirk;
>> +				snps,tx_de_emphasis_quirk;
>> +				snps,tx_de_emphasis = <1>;
>> +				snps,dis_enblslpm_quirk;
> 
> Pretty sure these aren't documented because we don't use '_' in property 
> names.
> 
Do you mean property as "snps,dis_enblslpm_quirk"? These properties are
documented in Documentation/devicetree/bindings/usb/dwc3.txt.

>> +				snps,gctl-reset-quirk;
>> +				usb-role-switch;
>> +				role-switch-default-host;
>> +
>> +				port {
>> +					#address-cells = <1>;
>> +					#size-cells = <0>;
>> +
>> +					dwc3_role_switch: endpoint@0 {
>> +						reg = <0>;
>> +						remote-endpoint = <&rt1711h_ep>;
>> +					};
>> +
>> +					dwc3_role_switch_notify: endpoint@1 {
>> +						reg = <1>;
>> +						remote-endpoint = <&hikey_usb_ep>;
>> +					};
>> +				};
>> +			};
>> +		};
>>  	};
>>  };
>> -- 
>> 2.15.0-rc2
>>
> 
> .
> 

Thanks
- Yu Chen

Re: [PATCH v6 02/13] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960

[Rob Herring]

On Tue, Apr 30, 2019 at 1:08 AM Chen Yu <chenyu56@huawei.com> wrote:
>
> Hi Rob,
>
> On 2019/4/26 5:35, Rob Herring wrote:
> > On Sat, Apr 20, 2019 at 02:40:08PM +0800, Yu Chen wrote:
> >> This patch adds binding documentation to support usb hub and usb
> >> data role switch of Hisilicon HiKey960 Board.
> >
> > Sorry I've been slow to really review this, but I needed to look at the
> > schematics to see what exactly is going on here.
> >
> > I think this needs some changes to better reflect the h/w and utilize
> > existing bindings. It should really be designed ignoring the muxing to
> > start with. Define the binding for the TypeC connector and then the host
> > hub and make sure they can coexist. Then overlay what you need to switch
> > between the 2 modes which AFAICT is just a single GPIO.
> >
> >>
> >> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> >> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >> Cc: Rob Herring <robh+dt@kernel.org>
> >> Cc: Mark Rutland <mark.rutland@arm.com>
> >> Cc: John Stultz <john.stultz@linaro.org>
> >> Cc: Binghui Wang <wangbinghui@hisilicon.com>
> >> Signed-off-by: Yu Chen <chenyu56@huawei.com>
> >> ---
> >> v1:
> >> * Fix some format errors as suggested by Sergei.
> >> * Modify gpio description to use gpiod API.
> >> v2:
> >> * Remove information about Hikey.
> >> * Fix gpio description.
> >> * Remove device_type of endpoint.
> >> v3:
> >> * Remove property typec-vbus-enable-val.
> >> * Add description of pinctrl-names.
> >> * Add example for "hisilicon,gpio-hubv1"
> >> * Add flag in gpiod properties.
> >> ---
> >> ---
> >>  .../bindings/misc/hisilicon-hikey-usb.txt          | 52 ++++++++++++++++++++++
> >>  1 file changed, 52 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
> >> new file mode 100644
> >> index 000000000000..422e844df719
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
> >> @@ -0,0 +1,52 @@
> >> +Support usb hub and usb data role switch of Hisilicon HiKey960 Board.
> >> +
> >> +-----------------------------
> >> +
> >> +Required properties:
> >> +- compatible: "hisilicon,gpio-hubv1","hisilicon,hikey960-usb"
> >> +- typec-vbus-gpios: gpio to control the vbus of typeC port
> >
> > This should be a gpio regulator and then connected to 'vbus-supply' in a
> > usb-connector node (see .../bindings/connectors/usb-connector.txt).
> Currently usb-connector node has no "vbus-supply" property and
> I do not find process that handles vbus-supply in RT1711H TypeC driver.

The patch[1] adding that is posted to the list and may not have landed yet.

Whether the RT1711H TypeC driver handles it or not is not a binding problem.

> > Then you also need the RT1711HWSC TypeC controller in DT. That is
> > typically the parent device of the connector node.
> >
> >> +- otg-switch-gpios: gpio to switch DP & DM between the hub and typeC port
> >
> > This probably belongs in USB controller node.
> >
> The otg-switch-gpios controls a mux like fsusb30mux. It is related to
> the board design of HiKey960. And the state of the mux is decided by
> the typeC port state. So I think it is not so good to make it belongs
> in USB controller node.

Let me put it this way. The gpio property belongs wherever the mux is
represented. In this case, I would expect the graph port representing
the HS port to have 2 endpoints representing the 2 mux outputs. We
don't generally put properties in the endpoint or port nodes, but the
parent nodes.

> >> +- hub-vdd33-en-gpios: gpio to enable the power of hub
> >
> > This too should be a gpio regulator and then in a hub node. We have 2
> > ways to represent hubs. Either as an I2C device or as a child of the
> > host controller. The latter is preferred, but I'm not too sure how the
> > OF graph connection linking the controller to the TypeC connector will
> > work with the usb bus binding.
> >
> There is no particular code except the power control for the hub.
> The i2c on the hub is not used. So it can not be an I2C device.
> Is there such an example that make the hub as a child of the host controller
> and control its power?

Yes, bindings/usb/usb-device.txt.

Rob

[1] https://www.spinics.net/lists/kernel/msg3089136.html

RE: [PATCH v6 13/13] dts: hi3660: Add support for usb on Hikey960

[shufan_lee(李書帆)]

Hi Yu Chen,

From: Chen Yu <chenyu56@huawei.com>
Sent: Tuesday, April 30, 2019 3:16 PM
To: Rob Herring <robh@kernel.org>
Cc: Liuyu (R) <liuyu712@hisilicon.com>; linux-usb@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; john.stultz@linaro.org; suzhuangluan@hisilicon.com; kongfei@hisilicon.com; wanghu17@hisilicon.com; butao@hisilicon.com; chenyao11@huawei.com; fangshengzhou@hisilicon.com; lipengcheng8@huawei.com; songxiaowei@hisilicon.com; xuyiping@hisilicon.com; xuyoujun4@huawei.com; yudongbin@hisilicon.com; zangleigang@hisilicon.com; Chunfeng Yun <chunfeng.yun@mediatek.com>; Wei Xu <xuwei5@hisilicon.com>; Mark Rutland <mark.rutland@arm.com>; linux-arm-kernel@lists.infradead.org; Binghui Wang <wangbinghui@hisilicon.com>; shufan_lee(李書帆) <shufan_lee@richtek.com>
Subject: Re: [PATCH v6 13/13] dts: hi3660: Add support for usb on Hikey960

Hi Rob,

On 2019/4/26 6:00, Rob Herring wrote:
>> On Sat, Apr 20, 2019 at 02:40:19PM +0800, Yu Chen wrote:
>>> This patch adds support for usb on Hikey960.
>>>
>>> Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
>>> Cc: Wei Xu <xuwei5@hisilicon.com>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: John Stultz <john.stultz@linaro.org>
>>> Cc: Binghui Wang <wangbinghui@hisilicon.com>
>>> Signed-off-by: Yu Chen <chenyu56@huawei.com>
>>> ---
>>> v2:
>>> * Remove device_type property.
>>> * Add property "usb-role-switch".
>> v3:
>>> * Make node "usb_phy" a subnode of usb3_otg_bc register.
>>> * Remove property "typec-vbus-enable-val" of hisi_hikey_usb.
>>> v4:
>>> * Remove property "hisilicon,usb3-otg-bc-syscon" of usb-phy.
>>> ---
>>> ---
>>>  arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 53 ++++++++++++++++
>>>  arch/arm64/boot/dts/hisilicon/hi3660.dtsi         | 73 +++++++++++++++++++++++
>>>  2 files changed, 126 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>>> b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>>> index e035cf195b19..d4e11c56b250 100644
>>> --- a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>>> +++ b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>>> @@ -13,6 +13,7 @@
>>>  #include <dt-bindings/gpio/gpio.h>
>>>  #include <dt-bindings/input/input.h>  #include
>>> <dt-bindings/interrupt-controller/irq.h>
>>> +#include <dt-bindings/usb/pd.h>
>>>
>>>  / {
>>>  model = "HiKey960";
>>> @@ -196,6 +197,26 @@
>>>  method = "smc";
>>>  };
>>>  };
>>> +
>>> +hisi_hikey_usb: hisi_hikey_usb {
>>> +compatible = "hisilicon,hikey960_usb";
>>> +typec-vbus-gpios = <&gpio25 2 GPIO_ACTIVE_HIGH>;
>>> +otg-switch-gpios = <&gpio25 6 GPIO_ACTIVE_HIGH>;
>>> +hub-vdd33-en-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
>>> +pinctrl-names = "default";
>>> +pinctrl-0 = <&usbhub5734_pmx_func>;
>>> +
>>> +port {
>>> +#address-cells = <1>;
>>> +#size-cells = <0>;
>>> +
>>> +hikey_usb_ep: endpoint@0 {
>>> +reg = <0>;
>>> +remote-endpoint = <&dwc3_role_switch_notify>;
>>> +};
>>> +};
>>> +};
>>> +
>>>  };
>>>
>>>  /*
>>> @@ -526,6 +547,38 @@
>>>  &i2c1 {
>>>  status = "okay";
>>>
>>> +rt1711h: rt1711h@4e {
>>> +compatible = "richtek,rt1711h";
>>
>> The binding doc for this should state it should have a 'connector' node.
>>
> Hi shufan,
> Is the 'connector' node an essential node of rt1711h?
> Currently it is missing in Documentation/devicetree/bindings/usb/richtek,rt1711h.txt.
> Do you think the 'connector' node should add as this patch in the binding doc?

Yes.
For rt1711h, the parsing work is done when calling tcpci_register_port and tcpm_register_port.
The parsing flow is uploaded by Jun Li after rt1711h is uploaded, so richtek,rt1711h.txt hasn't modified yet.
To add rt1711h node and modify the binding doc, richtek,rt1711h.txt, you can refer to Documentation/devicetree/bindings/usb/typec-tcpci.txt

>>> +reg = <0x4e>;
>>> +status = "ok";
>>
>> Can drop this, it is the default.
>>
> OK.

>>> +interrupt-parent = <&gpio27>;
>>> +interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
>>> +pinctrl-names = "default";
>>> +pinctrl-0 = <&usb_cfg_func>;
>>> +
>>> +usb_con: connector {
>>> +compatible = "usb-c-connector";
>>> +label = "USB-C";
>>> +data-role = "dual";
>>> +power-role = "dual";
>>> +try-power-role = "sink";
>>> +source-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)>;
>>> +sink-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)
>>> +PDO_VAR(5000, 5000, 1000)>;
>>> +op-sink-microwatt = <10000000>;
>>> +};
>>> +
>>> +port {
>>
>> The connector node should have a 'ports' child with 'port@0' being the
>> HS connection.
>>
> This port and endpoint of the port are used for role_switch matching by fwnode_graph_devcon_match. I'm not too sure the 'ports' under connector is used in rt1711h driver？
> Hi shufan_lee,
>     Can you confirm this?

The 'ports' is not used in rt1711h's driver.
I'm not sure if this is helpful but the usage of 'ports' may refer to the following patch.
https://lore.kernel.org/patchwork/patch/879945/

>>> +#address-cells = <1>;
>>> +#size-cells = <0>;
>>> +
>>> +rt1711h_ep: endpoint@0 {
>>> +reg = <0>;
>>> +remote-endpoint = <&dwc3_role_switch>;
>>> +};
>>> +};
>>> +};
>>> +
>>>  adv7533: adv7533@39 {
>>>  status = "ok";
>>>  compatible = "adi,adv7533";
>>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>>> b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>>> index 2f19e0e5b7cf..173467505ada 100644
>>> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>>> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>>> @@ -355,6 +355,12 @@
>>>  #clock-cells = <1>;
>>>  };
>>>
>>> +pmctrl: pmctrl@fff31000 {
>>> +compatible = "hisilicon,hi3660-pmctrl", "syscon";
>>> +reg = <0x0 0xfff31000 0x0 0x1000>;
>>> +#clock-cells = <1>;
>>> +};
>>> +
>>>  pmuctrl: crg_ctrl@fff34000 {
>>>  compatible = "hisilicon,hi3660-pmuctrl", "syscon";
>>>  reg = <0x0 0xfff34000 0x0 0x1000>; @@ -1134,5 +1140,72 @@
>>>  };
>>>  };
>>>  };
>>> +
>>> +usb3_otg_bc: usb3_otg_bc@ff200000 {
>>> +compatible = "syscon", "simple-mfd";
>>> +reg = <0x0 0xff200000 0x0 0x1000>;
>>> +
>>> +usb_phy: usb-phy {
>>> +compatible = "hisilicon,hi3660-usb-phy";
>>> +#phy-cells = <0>;
>>> +hisilicon,pericrg-syscon = <&crg_ctrl>;
>>> +hisilicon,pctrl-syscon = <&pctrl>;
>>> +hisilicon,eye-diagram-param = <0x22466e4>;
>>> +};
>>> +};
>>> +
>>> +usb3: hisi_dwc3 {
>>> +compatible = "hisilicon,hi3660-dwc3";
>>> +#address-cells = <2>;
>>> +#size-cells = <2>;
>>> +ranges;
>>
>> If there are not any wrapper registers, then get rid of the hisi_dwc3
>> node. For just clocks and resets we just put everything in one node.
>>
> I will try to fix this.
>>> +
>>> +clocks = <&crg_ctrl HI3660_CLK_ABB_USB>,
>>> + <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
>>> +clock-names = "clk_usb3phy_ref", "aclk_usb3otg";
>>> +
>>> +assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
>>> +assigned-clock-rates = <229000000>;
>>> +resets = <&crg_rst 0x90 8>,
>>> + <&crg_rst 0x90 7>,
>>> + <&crg_rst 0x90 6>,
>>> + <&crg_rst 0x90 5>;
>>> +
>>> +dwc3: dwc3@ff100000 {
>>> +compatible = "snps,dwc3";
>>> +reg = <0x0 0xff100000 0x0 0x100000>;
>>> +interrupts = <0 159 4>, <0 161 4>;
>>> +phys = <&usb_phy>;
>>> +phy-names = "usb3-phy";
>>> +dr_mode = "otg";
>>> +maximum-speed = "super-speed";
>>> +phy_type = "utmi";
>>> +snps,dis-del-phy-power-chg-quirk;
>>
>>> +snps,lfps_filter_quirk;
>>> +snps,dis_u2_susphy_quirk;
>>> +snps,dis_u3_susphy_quirk;
>>> +snps,tx_de_emphasis_quirk;
>>> +snps,tx_de_emphasis = <1>;
>>> +snps,dis_enblslpm_quirk;
>>
>> Pretty sure these aren't documented because we don't use '_' in
>> property names.
>>
> Do you mean property as "snps,dis_enblslpm_quirk"? These properties are documented in Documentation/devicetree/bindings/usb/dwc3.txt.

>>> +snps,gctl-reset-quirk;
>>> +usb-role-switch;
>>> +role-switch-default-host;
>>> +
>>> +port {
>>> +#address-cells = <1>;
>>> +#size-cells = <0>;
>>> +
>>> +dwc3_role_switch: endpoint@0 {
>>> +reg = <0>;
>>> +remote-endpoint = <&rt1711h_ep>;
>>> +};
>>> +
>>> +dwc3_role_switch_notify: endpoint@1 {
>>> +reg = <1>;
>>> +remote-endpoint = <&hikey_usb_ep>;
>>> +};
>>> +};
>>> +};
>>> +};
>>>  };
>>>  };
>>> --
>>> 2.15.0-rc2
>>>
>>
>> .
>>

> Thanks
> - Yu Chen

************* Email Confidentiality Notice ********************

The information contained in this e-mail message (including any attachments) may be confidential, proprietary, privileged, or otherwise exempt from disclosure under applicable laws. It is intended to be conveyed only to the designated recipient(s). Any use, dissemination, distribution, printing, retaining or copying of this e-mail (including its attachments) by unintended recipient(s) is strictly prohibited and may be unlawful. If you are not an intended recipient of this e-mail, or believe that you have received this e-mail in error, please notify the sender immediately (by replying to this e-mail), delete any and all copies of this e-mail (including any attachments) from your system, and do not disclose the content of this e-mail to any other person. Thank you!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6 04/13] usb: dwc3: Add splitdisable quirk for Hisilicon Kirin Soc

[Mauro Carvalho Chehab]

Hi Felipe/Greg,

What's the status of this patch? 

I tested here, together with the Hikey 970 phy RFC patches I sent
last week.

Without this patch, the USB HID driver receives -EPROTO from
submitted URBs, causing it to enter into an endless reset cycle
on every 500 ms, at the hid_io_error() logic.

Tested-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

If you prefer, I can re-submit this one with my SOB.

Thanks,
Mauro

Em Sat, 20 Apr 2019 14:40:10 +0800
Yu Chen <chenyu56@huawei.com> escreveu:

> SPLIT_BOUNDARY_DISABLE should be set for DesignWare USB3 DRD Core
> of Hisilicon Kirin Soc when dwc3 core act as host.
> 
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Binghui Wang <wangbinghui@hisilicon.com>
> Signed-off-by: Yu Chen <chenyu56@huawei.com>
> ---
> v4:
> * Add dwc3_complete definition while CONFIG_PM_SLEEP does not defined.
> * Add description for 'dis_split_quirk'.
> ---
> ---
>  drivers/usb/dwc3/core.c | 26 ++++++++++++++++++++++++++
>  drivers/usb/dwc3/core.h |  7 +++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index a1b126f90261..c3ef6bd2b0d4 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -117,6 +117,7 @@ static void __dwc3_set_mode(struct work_struct *work)
>  	struct dwc3 *dwc = work_to_dwc(work);
>  	unsigned long flags;
>  	int ret;
> +	u32 reg;
>  
>  	if (dwc->dr_mode != USB_DR_MODE_OTG)
>  		return;
> @@ -169,6 +170,11 @@ static void __dwc3_set_mode(struct work_struct *work)
>  			phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
>  			phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
>  			phy_calibrate(dwc->usb2_generic_phy);
> +			if (dwc->dis_split_quirk) {
> +				reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
> +				reg |= DWC3_GUCTL3_SPLITDISABLE;
> +				dwc3_writel(dwc->regs, DWC3_GUCTL3, reg);
> +			}
>  		}
>  		break;
>  	case DWC3_GCTL_PRTCAP_DEVICE:
> @@ -1306,6 +1312,9 @@ static void dwc3_get_properties(struct dwc3 *dwc)
>  	dwc->dis_metastability_quirk = device_property_read_bool(dev,
>  				"snps,dis_metastability_quirk");
>  
> +	dwc->dis_split_quirk = device_property_read_bool(dev,
> +				"snps,dis-split-quirk");
> +
>  	dwc->lpm_nyet_threshold = lpm_nyet_threshold;
>  	dwc->tx_de_emphasis = tx_de_emphasis;
>  
> @@ -1825,10 +1834,27 @@ static int dwc3_resume(struct device *dev)
>  
>  	return 0;
>  }
> +
> +static void dwc3_complete(struct device *dev)
> +{
> +	struct dwc3	*dwc = dev_get_drvdata(dev);
> +	u32		reg;
> +
> +	if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST &&
> +			dwc->dis_split_quirk) {
> +		dev_dbg(dwc->dev, "set DWC3_GUCTL3_SPLITDISABLE\n");
> +		reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
> +		reg |= DWC3_GUCTL3_SPLITDISABLE;
> +		dwc3_writel(dwc->regs, DWC3_GUCTL3, reg);
> +	}
> +}
> +#else
> +#define dwc3_complete NULL
>  #endif /* CONFIG_PM_SLEEP */
>  
>  static const struct dev_pm_ops dwc3_dev_pm_ops = {
>  	SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
> +	.complete = dwc3_complete,
>  	SET_RUNTIME_PM_OPS(dwc3_runtime_suspend, dwc3_runtime_resume,
>  			dwc3_runtime_idle)
>  };
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 1528d395b156..28475e301ad9 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -136,6 +136,7 @@
>  #define DWC3_GEVNTCOUNT(n)	(0xc40c + ((n) * 0x10))
>  
>  #define DWC3_GHWPARAMS8		0xc600
> +#define DWC3_GUCTL3		0xc60c
>  #define DWC3_GFLADJ		0xc630
>  
>  /* Device Registers */
> @@ -370,6 +371,9 @@
>  /* Global User Control Register 2 */
>  #define DWC3_GUCTL2_RST_ACTBITLATER		BIT(14)
>  
> +/* Global User Control Register 3 */
> +#define DWC3_GUCTL3_SPLITDISABLE		BIT(14)
> +
>  /* Device Configuration Register */
>  #define DWC3_DCFG_DEVADDR(addr)	((addr) << 3)
>  #define DWC3_DCFG_DEVADDR_MASK	DWC3_DCFG_DEVADDR(0x7f)
> @@ -1030,6 +1034,7 @@ struct dwc3_scratchpad_array {
>   * 	2	- No de-emphasis
>   * 	3	- Reserved
>   * @dis_metastability_quirk: set to disable metastability quirk.
> + * @dis_split_quirk: set to disable split boundary.
>   * @imod_interval: set the interrupt moderation interval in 250ns
>   *                 increments or 0 to disable.
>   */
> @@ -1216,6 +1221,8 @@ struct dwc3 {
>  
>  	unsigned		dis_metastability_quirk:1;
>  
> +	unsigned		dis_split_quirk:1;
> +
>  	u16			imod_interval;
>  };
>  



Thanks,
Mauro

Re: [PATCH v6 04/13] usb: dwc3: Add splitdisable quirk for Hisilicon Kirin Soc

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2077 bytes --]


Hi Mauro,

Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:

> Hi Felipe/Greg,
>
> What's the status of this patch? 

to be frank, I don't think I have this in my inbox anymore.

> I tested here, together with the Hikey 970 phy RFC patches I sent
> last week.
>
> Without this patch, the USB HID driver receives -EPROTO from
> submitted URBs, causing it to enter into an endless reset cycle
> on every 500 ms, at the hid_io_error() logic.

> Tested-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
>
> If you prefer, I can re-submit this one with my SOB.

Please do, but since you're changing device tree, I need Rob's acked-by.

> Thanks,
> Mauro
>
> Em Sat, 20 Apr 2019 14:40:10 +0800
> Yu Chen <chenyu56@huawei.com> escreveu:
>
>> SPLIT_BOUNDARY_DISABLE should be set for DesignWare USB3 DRD Core
>> of Hisilicon Kirin Soc when dwc3 core act as host.

is this Kirin-specific or is this something that we should do a revision
check? Why does it affect only Hikey kirin? What's the dwc3 revision on
that SoC (grep SNPSID /sys/kernel/debugfs/*dwc3/regdump)?

>> @@ -1825,10 +1834,27 @@ static int dwc3_resume(struct device *dev)
>>  
>>  	return 0;
>>  }
>> +
>> +static void dwc3_complete(struct device *dev)
>> +{
>> +	struct dwc3	*dwc = dev_get_drvdata(dev);
>> +	u32		reg;
>> +
>> +	if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST &&
>> +			dwc->dis_split_quirk) {
>> +		dev_dbg(dwc->dev, "set DWC3_GUCTL3_SPLITDISABLE\n");

no more dev_dbg() should be added. This driver relies exclusively on
tracepoints for debugging.

>> +		reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
>> +		reg |= DWC3_GUCTL3_SPLITDISABLE;
>> +		dwc3_writel(dwc->regs, DWC3_GUCTL3, reg);
>> +	}
>> +}
>> +#else
>> +#define dwc3_complete NULL
>>  #endif /* CONFIG_PM_SLEEP */
>>  
>>  static const struct dev_pm_ops dwc3_dev_pm_ops = {
>>  	SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
>> +	.complete = dwc3_complete,

why is this done on complete? Why can't it be done at the end of
dwc3_resume()?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

Re: [PATCH v6 04/13] usb: dwc3: Add splitdisable quirk for Hisilicon Kirin Soc

[Mauro Carvalho Chehab]

Em Mon, 07 Sep 2020 17:04:31 +0300
Felipe Balbi <balbi@kernel.org> escreveu:

> Hi Mauro,
> 
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:
> 
> > Hi Felipe/Greg,
> >
> > What's the status of this patch?   
> 
> to be frank, I don't think I have this in my inbox anymore.
> 
> > I tested here, together with the Hikey 970 phy RFC patches I sent
> > last week.
> >
> > Without this patch, the USB HID driver receives -EPROTO from
> > submitted URBs, causing it to enter into an endless reset cycle
> > on every 500 ms, at the hid_io_error() logic.  
> 
> > Tested-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> >
> > If you prefer, I can re-submit this one with my SOB.  
> 
> Please do, but since you're changing device tree, I need Rob's acked-by.

Ok, I'll do that.

> > Thanks,
> > Mauro
> >
> > Em Sat, 20 Apr 2019 14:40:10 +0800
> > Yu Chen <chenyu56@huawei.com> escreveu:
> >  
> >> SPLIT_BOUNDARY_DISABLE should be set for DesignWare USB3 DRD Core
> >> of Hisilicon Kirin Soc when dwc3 core act as host.  
> 
> is this Kirin-specific or is this something that we should do a revision
> check? 

I've no idea. I don't have any datasheets from this device.

> Why does it affect only Hikey kirin? 

As John Stultz didn't re-submit this one (and looking at the DT
between Kirin 960 and 970 from the original Kernel 4.9 official
drivers), I suspect that only Kirin 970 requires this quirk.

It could well be due to some Dwc3 revision, but it could also be due
to some differences at the USB part of the SoC, as there are a
few other things different between hikey 960 and 970: it has a
different PHY driver, and there are also some differences at the
USB HUB which is connected into it.

On both devices, the USB physical ports are actually connected
into a HUB. In the case of Hikey 970, the hub seems to be a
TI TUSB8041 4-Port Hub:
	
	$ lsusb
	Bus 002 Device 002: ID 0451:8140 Texas Instruments, Inc. TUSB8041 4-Port Hub
	Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
	Bus 001 Device 004: ID 090c:1000 Silicon Motion, Inc. - Taiwan (formerly Feiya Technology Corp.) Flash Drive
	Bus 001 Device 003: ID 413c:301a Dell Computer Corp. Dell MS116 Optical Mouse
	Bus 001 Device 002: ID 0451:8142 Texas Instruments, Inc. TUSB8041 4-Port Hub
	Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

> What's the dwc3 revision on
> that SoC (grep SNPSID /sys/kernel/debugfs/*dwc3/regdump)?

	GSNPSID = 0x33313130

> 
> >> @@ -1825,10 +1834,27 @@ static int dwc3_resume(struct device *dev)
> >>  
> >>  	return 0;
> >>  }
> >> +
> >> +static void dwc3_complete(struct device *dev)
> >> +{
> >> +	struct dwc3	*dwc = dev_get_drvdata(dev);
> >> +	u32		reg;
> >> +
> >> +	if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST &&
> >> +			dwc->dis_split_quirk) {
> >> +		dev_dbg(dwc->dev, "set DWC3_GUCTL3_SPLITDISABLE\n");  
> 
> no more dev_dbg() should be added. This driver relies exclusively on
> tracepoints for debugging.

Ok. 

> 
> >> +		reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
> >> +		reg |= DWC3_GUCTL3_SPLITDISABLE;
> >> +		dwc3_writel(dwc->regs, DWC3_GUCTL3, reg);
> >> +	}
> >> +}
> >> +#else
> >> +#define dwc3_complete NULL
> >>  #endif /* CONFIG_PM_SLEEP */
> >>  
> >>  static const struct dev_pm_ops dwc3_dev_pm_ops = {
> >>  	SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
> >> +	.complete = dwc3_complete,  
> 
> why is this done on complete? Why can't it be done at the end of
> dwc3_resume()?

Again, no idea. I didn't actually tried to suspend/resume.

Maybe the original author can shed a light on it.

Thanks,
Mauro

Re: [PATCH v6 04/13] usb: dwc3: Add splitdisable quirk for Hisilicon Kirin Soc

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3419 bytes --]


Hi,

Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:
>> > I tested here, together with the Hikey 970 phy RFC patches I sent
>> > last week.
>> >
>> > Without this patch, the USB HID driver receives -EPROTO from
>> > submitted URBs, causing it to enter into an endless reset cycle
>> > on every 500 ms, at the hid_io_error() logic.  
>> 
>> > Tested-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
>> >
>> > If you prefer, I can re-submit this one with my SOB.  
>> 
>> Please do, but since you're changing device tree, I need Rob's acked-by.
>
> Ok, I'll do that.

thanks

>> > Em Sat, 20 Apr 2019 14:40:10 +0800
>> > Yu Chen <chenyu56@huawei.com> escreveu:
>> >  
>> >> SPLIT_BOUNDARY_DISABLE should be set for DesignWare USB3 DRD Core
>> >> of Hisilicon Kirin Soc when dwc3 core act as host.  
>> 
>> is this Kirin-specific or is this something that we should do a revision
>> check? 
>
> I've no idea. I don't have any datasheets from this device.

I see

>> Why does it affect only Hikey kirin? 
>
> As John Stultz didn't re-submit this one (and looking at the DT
> between Kirin 960 and 970 from the original Kernel 4.9 official
> drivers), I suspect that only Kirin 970 requires this quirk.
>
> It could well be due to some Dwc3 revision, but it could also be due
> to some differences at the USB part of the SoC, as there are a

the reason I ask is that if it's caused by dwc3 revision, then we don't
need the extra dt property, we can rely on a revision check. If it's
something that can't be detected in runtime, then we need a property.

> few other things different between hikey 960 and 970: it has a
> different PHY driver, and there are also some differences at the
> USB HUB which is connected into it.
>
> On both devices, the USB physical ports are actually connected
> into a HUB. In the case of Hikey 970, the hub seems to be a
> TI TUSB8041 4-Port Hub:
> 	
> 	$ lsusb
> 	Bus 002 Device 002: ID 0451:8140 Texas Instruments, Inc. TUSB8041 4-Port Hub
> 	Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> 	Bus 001 Device 004: ID 090c:1000 Silicon Motion, Inc. - Taiwan (formerly Feiya Technology Corp.) Flash Drive
> 	Bus 001 Device 003: ID 413c:301a Dell Computer Corp. Dell MS116 Optical Mouse
> 	Bus 001 Device 002: ID 0451:8142 Texas Instruments, Inc. TUSB8041 4-Port Hub
> 	Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
>
>> What's the dwc3 revision on
>> that SoC (grep SNPSID /sys/kernel/debugfs/*dwc3/regdump)?
>
> 	GSNPSID = 0x33313130

This isn't even listed as a known revision in dwc3/core.h. Thinh, could
the issue being described here caused by a known Erratum with this
particular revision?

>> >> +		reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
>> >> +		reg |= DWC3_GUCTL3_SPLITDISABLE;
>> >> +		dwc3_writel(dwc->regs, DWC3_GUCTL3, reg);
>> >> +	}
>> >> +}
>> >> +#else
>> >> +#define dwc3_complete NULL
>> >>  #endif /* CONFIG_PM_SLEEP */
>> >>  
>> >>  static const struct dev_pm_ops dwc3_dev_pm_ops = {
>> >>  	SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
>> >> +	.complete = dwc3_complete,  
>> 
>> why is this done on complete? Why can't it be done at the end of
>> dwc3_resume()?
>
> Again, no idea. I didn't actually tried to suspend/resume.
>
> Maybe the original author can shed a light on it.

yeah, would be nice :-)

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

Re: [PATCH v6 04/13] usb: dwc3: Add splitdisable quirk for Hisilicon Kirin Soc

[Mauro Carvalho Chehab]

Em Mon, 07 Sep 2020 17:04:31 +0300
Felipe Balbi <balbi@kernel.org> escreveu:

> >>  static const struct dev_pm_ops dwc3_dev_pm_ops = {
> >>  	SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
> >> +	.complete = dwc3_complete,  
> 
> why is this done on complete? Why can't it be done at the end of
> dwc3_resume()?

Updating my answer:

No. I just tested moving the code from .complete to the end of
.resume. After moving it, the HID won't be able to properly enable
the device at resume time (logs filtered to show only relevant msgs):

	# echo reboot > /sys/power/disk; echo disk > /sys/power/state

	...
	[   29.051314] Freezing user space processes ... (elapsed 0.001 seconds) done.
	[   29.059811] OOM killer disabled.
	[   30.171526] PM: hibernation: Creating image:
	[   30.171526] PM: hibernation: Need to copy 164192 pages
	[   30.171526] PM: hibernation: Image created (164192 pages copied)
	[   30.171644] Enabling non-boot CPUs ...
	[   30.293953] dwc3 ff100000.dwc3: dwc3_resume
	[   30.424549] usb usb1-port1: status 0101 change 0001
	[   30.428538] usb usb2-port1: status 0203 change 0001
	[   30.536609] usb 1-1: Waited 0ms for CONNECT
	[   30.543822] usb 1-1: finish reset-resume
	[   30.544619] usb 2-1: Waited 0ms for CONNECT
	[   30.558869] usb 2-1: finish reset-resume
	[   30.676706] usb 1-1: reset high-speed USB device number 2 using xhci-hcd
	[   30.882194] hub 1-1:1.0: hub_reset_resume
	[   30.888479] hub 1-1:1.0: enabling power on all ports
	[   30.960767] usb 2-1: reset SuperSpeed Gen 1 USB device number 2 using xhci-hcd
	[   30.994229] hub 2-1:1.0: hub_reset_resume
	[   31.000574] usb 1-1-port1: status 0301 change 0001
	[   31.002088] hub 2-1:1.0: enabling power on all ports
	[   31.009079] usb 1-1-port2: status 0101 change 0001
	[   31.132633] usb 1-1.2: Waited 0ms for CONNECT
	[   31.132665] usb 1-1.1: Waited 0ms for CONNECT
	[   31.140560] usb 1-1.2: finish reset-resume
	[   31.148509] usb 1-1.1: finish reset-resume
	[   31.240760] usb 1-1.2: reset high-speed USB device number 4 using xhci-hcd
	[   32.471492] dwc3 ff100000.dwc3: dwc3_complete
	[   32.480512] PM: Cannot find swap device, try swapon -a
	[   32.489532] PM: Cannot get swap writer
	[   32.575790] Restarting tasks ... 
	[   32.575891] hub 1-0:1.0: state 7 ports 1 chg 0000 evt 0000
	[   32.575894] hub 2-0:1.0: state 7 ports 1 chg 0000 evt 0000
	[   32.575896] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0002
	[   32.576570] hub 2-1:1.0: state 7 ports 4 chg 0000 evt 0000
	[   32.576876] done.
	[   32.600094] hub 2-1:1.0: hub_suspend
	[   32.642090] PM: hibernation: hibernation exit
	[   32.650255] usb 2-1: usb auto-suspend, wakeup 1
	[   32.668581] hub 2-0:1.0: hub_suspend
	[   33.122384] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0002
	[   33.378220] usb 1-1.1: reset low-speed USB device number 3 using xhci-hcd
	[   33.698394] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0000
	[   34.882365] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0002
	[   35.138217] usb 1-1.1: reset low-speed USB device number 3 using xhci-hcd
	[   35.458617] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0000
	[   36.642392] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0002
	[   36.898207] usb 1-1.1: reset low-speed USB device number 3 using xhci-hcd
	[   37.218598] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0000
	[   37.672658] kirin9xx-drm e8600000.dpe: [drm:dss_disable_vblank [kirin9xx_drm]] dss_disable_vblank
	[   38.402368] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0002
	[   38.658174] usb 1-1.1: reset low-speed USB device number 3 using xhci-hcd
	[   38.978594] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0000
	[   40.162361] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0002
	[   40.418148] usb 1-1.1: reset low-speed USB device number 3 using xhci-hcd
	...
	[  397.698132] usb 1-1.1: reset low-speed USB device number 3 using xhci-hcd
	[  398.018565] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0000

Basically, after resume, the code behaves just like the quirk were
not applied.

Explaining with more details, without the splitdisable quirk, what
happens is that the USB URBs return -EPROTO errors, causing
hid-core.c to call hid_io_error(). This function starts a reset
work (usbhid->reset_work), which ends calling (at hid_reset()):

	usb_queue_reset_device(usbhid->intf);

This happens on every HZ/2 intervals, causing an endless reset
loop.

Thanks,
Mauro

Re: [PATCH v6 04/13] usb: dwc3: Add splitdisable quirk for Hisilicon Kirin Soc

[Mauro Carvalho Chehab]

Em Tue, 08 Sep 2020 09:09:46 +0300
Felipe Balbi <balbi@kernel.org> escreveu:

> >> Why does it affect only Hikey kirin?   
> >
> > As John Stultz didn't re-submit this one (and looking at the DT
> > between Kirin 960 and 970 from the original Kernel 4.9 official
> > drivers), I suspect that only Kirin 970 requires this quirk.
> >
> > It could well be due to some Dwc3 revision, but it could also be due
> > to some differences at the USB part of the SoC, as there are a  
> 
> the reason I ask is that if it's caused by dwc3 revision, then we don't
> need the extra dt property, we can rely on a revision check. If it's
> something that can't be detected in runtime, then we need a property.

Yeah, if it would be possible to auto-detect, that would be
better. Yet, hard to say if it would be possible to do that, 
without access to the Dwc3 docs, and without different hardware 
for testing it.

> > few other things different between hikey 960 and 970: it has a
> > different PHY driver, and there are also some differences at the
> > USB HUB which is connected into it.
> >
> > On both devices, the USB physical ports are actually connected
> > into a HUB. In the case of Hikey 970, the hub seems to be a
> > TI TUSB8041 4-Port Hub:
> > 	
> > 	$ lsusb
> > 	Bus 002 Device 002: ID 0451:8140 Texas Instruments, Inc. TUSB8041 4-Port Hub
> > 	Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> > 	Bus 001 Device 004: ID 090c:1000 Silicon Motion, Inc. - Taiwan (formerly Feiya Technology Corp.) Flash Drive
> > 	Bus 001 Device 003: ID 413c:301a Dell Computer Corp. Dell MS116 Optical Mouse
> > 	Bus 001 Device 002: ID 0451:8142 Texas Instruments, Inc. TUSB8041 4-Port Hub
> > 	Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> >  
> >> What's the dwc3 revision on
> >> that SoC (grep SNPSID /sys/kernel/debugfs/*dwc3/regdump)?  
> >
> > 	GSNPSID = 0x33313130  
> 
> This isn't even listed as a known revision in dwc3/core.h. Thinh, could
> the issue being described here caused by a known Erratum with this
> particular revision?

Maybe. Again, hard to tell without any documentation from Synopsys.

Maybe some of the Synopsys folks at USB MK can shed a light.

Thanks,
Mauro

Re: [PATCH v6 04/13] usb: dwc3: Add splitdisable quirk for Hisilicon Kirin Soc

[Thinh Nguyen]

Felipe Balbi wrote:
> Hi,
>
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:
>>>> I tested here, together with the Hikey 970 phy RFC patches I sent
>>>> last week.
>>>>
>>>> Without this patch, the USB HID driver receives -EPROTO from
>>>> submitted URBs, causing it to enter into an endless reset cycle
>>>> on every 500 ms, at the hid_io_error() logic.  
>>>> Tested-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
>>>>
>>>> If you prefer, I can re-submit this one with my SOB.  
>>> Please do, but since you're changing device tree, I need Rob's acked-by.
>> Ok, I'll do that.
> thanks
>
>>>> Em Sat, 20 Apr 2019 14:40:10 +0800
>>>> Yu Chen <chenyu56@huawei.com> escreveu:
>>>>  
>>>>> SPLIT_BOUNDARY_DISABLE should be set for DesignWare USB3 DRD Core
>>>>> of Hisilicon Kirin Soc when dwc3 core act as host.  
>>> is this Kirin-specific or is this something that we should do a revision
>>> check? 
>> I've no idea. I don't have any datasheets from this device.
> I see
>
>>> Why does it affect only Hikey kirin? 
>> As John Stultz didn't re-submit this one (and looking at the DT
>> between Kirin 960 and 970 from the original Kernel 4.9 official
>> drivers), I suspect that only Kirin 970 requires this quirk.
>>
>> It could well be due to some Dwc3 revision, but it could also be due
>> to some differences at the USB part of the SoC, as there are a
> the reason I ask is that if it's caused by dwc3 revision, then we don't
> need the extra dt property, we can rely on a revision check. If it's
> something that can't be detected in runtime, then we need a property.
>
>> few other things different between hikey 960 and 970: it has a
>> different PHY driver, and there are also some differences at the
>> USB HUB which is connected into it.
>>
>> On both devices, the USB physical ports are actually connected
>> into a HUB. In the case of Hikey 970, the hub seems to be a
>> TI TUSB8041 4-Port Hub:
>> 	
>> 	$ lsusb
>> 	Bus 002 Device 002: ID 0451:8140 Texas Instruments, Inc. TUSB8041 4-Port Hub
>> 	Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
>> 	Bus 001 Device 004: ID 090c:1000 Silicon Motion, Inc. - Taiwan (formerly Feiya Technology Corp.) Flash Drive
>> 	Bus 001 Device 003: ID 413c:301a Dell Computer Corp. Dell MS116 Optical Mouse
>> 	Bus 001 Device 002: ID 0451:8142 Texas Instruments, Inc. TUSB8041 4-Port Hub
>> 	Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
>>
>>> What's the dwc3 revision on
>>> that SoC (grep SNPSID /sys/kernel/debugfs/*dwc3/regdump)?
>> 	GSNPSID = 0x33313130
> This isn't even listed as a known revision in dwc3/core.h. Thinh, could
> the issue being described here caused by a known Erratum with this
> particular revision?

If you have a STAR issue number, then I may be able to look it up.

Btw, GNSPSID register is the ID for DWC_usb31 IP. For DWC_usb31 and
DWC_usb32, we need to read the version number and version type registers
(offset 0xc1a0 and 0xc1a4 respectively).

We need to add these registers to the register dump.

BR,
Thinh

>
>>>>> +		reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
>>>>> +		reg |= DWC3_GUCTL3_SPLITDISABLE;
>>>>> +		dwc3_writel(dwc->regs, DWC3_GUCTL3, reg);
>>>>> +	}
>>>>> +}
>>>>> +#else
>>>>> +#define dwc3_complete NULL
>>>>>  #endif /* CONFIG_PM_SLEEP */
>>>>>  
>>>>>  static const struct dev_pm_ops dwc3_dev_pm_ops = {
>>>>>  	SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
>>>>> +	.complete = dwc3_complete,  
>>> why is this done on complete? Why can't it be done at the end of
>>> dwc3_resume()?
>> Again, no idea. I didn't actually tried to suspend/resume.
>>
>> Maybe the original author can shed a light on it.
> yeah, would be nice :-)
>