[PATCH v10 0/5] support rockchip dwc3 driver

[PATCH v10 0/5] support rockchip dwc3 driver

[William Wu]

This series add support for rockchip DWC3 driver,
and add additional optional properties for specific
platforms (e.g., rockchip rk3399 platform).

And because rockchip DWC3 need additional handling of
cable events and mode switch to support DRD mode, so
we add a new dwc3-rockchip driver, rather than use the
generic of glue layer which merely enable some clocks
and populate its children.

William Wu (5):
  usb: dwc3: add dis_u2_freeclk_exists_quirk
  usb: dwc3: make usb2 phy utmi interface configurable
  usb: dwc3: add dis_del_phy_power_chg_quirk
  usb: dwc3: rockchip: add devicetree bindings documentation
  usb: dwc3: add rockchip specific glue layer

 Documentation/devicetree/bindings/usb/dwc3.txt     |   5 +
 Documentation/devicetree/bindings/usb/generic.txt  |   6 +
 .../devicetree/bindings/usb/rockchip,dwc3.txt      |  71 ++++
 drivers/usb/dwc3/Kconfig                           |   9 +
 drivers/usb/dwc3/Makefile                          |   1 +
 drivers/usb/dwc3/core.c                            |  30 +-
 drivers/usb/dwc3/core.h                            |  21 +
 drivers/usb/dwc3/dwc3-rockchip.c                   | 441 +++++++++++++++++++++
 8 files changed, 583 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
 create mode 100644 drivers/usb/dwc3/dwc3-rockchip.c

-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v10 1/5] usb: dwc3: add dis_u2_freeclk_exists_quirk

[William Wu]

Add a quirk to clear the GUSB2PHYCFG.U2_FREECLK_EXISTS bit,
which specifies whether the USB2.0 PHY provides a free-running
PHY clock, which is active when the clock control input is active.

Signed-off-by: William Wu <william.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
Changes in v10:
- None

Changes in v9:
- None

Changes in v8:
- add Acked-by (Rob Herring)

Changes in v7:
- None

Changes in v6:
- use '-' instead of '_' in dts (Rob Herring)

Changes in v5:
- None

Changes in v4:
- rebase on top of balbi testing/next, remove pdata (balbi)

Changes in v3:
- None

Changes in v2:
- None

 Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++
 drivers/usb/dwc3/core.c                        | 5 +++++
 drivers/usb/dwc3/core.h                        | 5 +++++
 3 files changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
index 7d7ce08..020b0e9 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -39,6 +39,9 @@ Optional properties:
 			disabling the suspend signal to the PHY.
  - snps,dis_rxdet_inp3_quirk: when set core will disable receiver detection
 			in PHY P3 power state.
+ - snps,dis-u2-freeclk-exists-quirk: when set, clear the u2_freeclk_exists
+			in GUSB2PHYCFG, specify that USB2 PHY doesn't provide
+			a free-running PHY clock.
  - snps,is-utmi-l1-suspend: true when DWC3 asserts output signal
 			utmi_l1_suspend_n, false when asserts utmi_sleep_n
  - snps,hird-threshold: HIRD threshold
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 35d0924..14316e5 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -500,6 +500,9 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
 	if (dwc->dis_enblslpm_quirk)
 		reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
 
+	if (dwc->dis_u2_freeclk_exists_quirk)
+		reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS;
+
 	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
 
 	return 0;
@@ -924,6 +927,8 @@ static int dwc3_probe(struct platform_device *pdev)
 				"snps,dis_enblslpm_quirk");
 	dwc->dis_rxdet_inp3_quirk = device_property_read_bool(dev,
 				"snps,dis_rxdet_inp3_quirk");
+	dwc->dis_u2_freeclk_exists_quirk = device_property_read_bool(dev,
+				"snps,dis-u2-freeclk-exists-quirk");
 
 	dwc->tx_de_emphasis_quirk = device_property_read_bool(dev,
 				"snps,tx_de_emphasis_quirk");
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 1a6cc48..08ed9e0 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -199,6 +199,7 @@
 
 /* Global USB2 PHY Configuration Register */
 #define DWC3_GUSB2PHYCFG_PHYSOFTRST	(1 << 31)
+#define DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS	(1 << 30)
 #define DWC3_GUSB2PHYCFG_SUSPHY		(1 << 6)
 #define DWC3_GUSB2PHYCFG_ULPI_UTMI	(1 << 4)
 #define DWC3_GUSB2PHYCFG_ENBLSLPM	(1 << 8)
@@ -803,6 +804,9 @@ struct dwc3_scratchpad_array {
  * @dis_u2_susphy_quirk: set if we disable usb2 suspend phy
  * @dis_enblslpm_quirk: set if we clear enblslpm in GUSB2PHYCFG,
  *                      disabling the suspend signal to the PHY.
+ * @dis_u2_freeclk_exists_quirk : set if we clear u2_freeclk_exists
+ *			in GUSB2PHYCFG, specify that USB2 PHY doesn't
+ *			provide a free-running PHY clock.
  * @tx_de_emphasis_quirk: set if we enable Tx de-emphasis quirk
  * @tx_de_emphasis: Tx de-emphasis value
  * 	0	- -6dB de-emphasis
@@ -946,6 +950,7 @@ struct dwc3 {
 	unsigned		dis_u2_susphy_quirk:1;
 	unsigned		dis_enblslpm_quirk:1;
 	unsigned		dis_rxdet_inp3_quirk:1;
+	unsigned		dis_u2_freeclk_exists_quirk:1;
 
 	unsigned		tx_de_emphasis_quirk:1;
 	unsigned		tx_de_emphasis:2;
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v10 2/5] usb: dwc3: make usb2 phy utmi interface configurable

[William Wu]

Support to configure the UTMI+ PHY with an 8- or 16-bit
interface via DT. The UTMI+ PHY interface is a hardware
capability, and it's platform dependent. Normally, the
PHYIF can be configured during coreconsultant.

But for some specific USB cores(e.g. rk3399 SoC DWC3),
the default PHYIF configuration value is false, so we
need to reconfigure it by software.

Signed-off-by: William Wu <william.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
Changes in v10:
- None

Changes in v9:
- None

Changes in v8:
- configure utmi interface via phy_type property in DT (Heiko, Rob Herring)
- add Acked-by (Rob Herring)
- modify commit message (Rob Herring)

Changes in v7:
- remove quirk and use only one property to configure utmi (Heiko, Rob Herring)

Changes in v6:
- use '-' instead of '_' in dts (Rob Herring)

Changes in v5:
- None

Changes in v4:
- rebase on top of balbi testing/next, remove pdata (balbi)

Changes in v3:
- None

Changes in v2:
- add a quirk for phyif_utmi (balbi)

 Documentation/devicetree/bindings/usb/generic.txt |  6 ++++++
 drivers/usb/dwc3/core.c                           | 18 ++++++++++++++++++
 drivers/usb/dwc3/core.h                           | 12 ++++++++++++
 3 files changed, 36 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/generic.txt b/Documentation/devicetree/bindings/usb/generic.txt
index bba8257..bfadeb1 100644
--- a/Documentation/devicetree/bindings/usb/generic.txt
+++ b/Documentation/devicetree/bindings/usb/generic.txt
@@ -11,6 +11,11 @@ Optional properties:
 			"peripheral" and "otg". In case this attribute isn't
 			passed via DT, USB DRD controllers should default to
 			OTG.
+ - phy_type: tells USB controllers that we want to configure the core to support
+			a UTMI+ PHY with an 8- or 16-bit interface if UTMI+ is
+			selected. Valid arguments are "utmi" and "utmi_wide".
+			In case this isn't passed via DT, USB controllers should
+			default to HW capability.
  - otg-rev: tells usb driver the release number of the OTG and EH supplement
 			with which the device and its descriptors are compliant,
 			in binary-coded decimal (i.e. 2.0 is 0200H). This
@@ -34,6 +39,7 @@ dwc3@4a030000 {
 	usb-phy = <&usb2_phy>, <&usb3,phy>;
 	maximum-speed = "super-speed";
 	dr_mode = "otg";
+	phy_type = "utmi_wide";
 	otg-rev = <0x0200>;
 	adp-disable;
 };
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 14316e5..cdac019 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -485,6 +485,23 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
 		break;
 	}
 
+	switch (dwc->hsphy_mode) {
+	case USBPHY_INTERFACE_MODE_UTMI:
+		reg &= ~(DWC3_GUSB2PHYCFG_PHYIF_MASK |
+		       DWC3_GUSB2PHYCFG_USBTRDTIM_MASK);
+		reg |= DWC3_GUSB2PHYCFG_PHYIF(UTMI_PHYIF_8_BIT) |
+		       DWC3_GUSB2PHYCFG_USBTRDTIM(USBTRDTIM_UTMI_8_BIT);
+		break;
+	case USBPHY_INTERFACE_MODE_UTMIW:
+		reg &= ~(DWC3_GUSB2PHYCFG_PHYIF_MASK |
+		       DWC3_GUSB2PHYCFG_USBTRDTIM_MASK);
+		reg |= DWC3_GUSB2PHYCFG_PHYIF(UTMI_PHYIF_16_BIT) |
+		       DWC3_GUSB2PHYCFG_USBTRDTIM(USBTRDTIM_UTMI_16_BIT);
+		break;
+	default:
+		break;
+	}
+
 	/*
 	 * Above 1.94a, it is recommended to set DWC3_GUSB2PHYCFG_SUSPHY to
 	 * '0' during coreConsultant configuration. So default value will
@@ -891,6 +908,7 @@ static int dwc3_probe(struct platform_device *pdev)
 
 	dwc->maximum_speed = usb_get_maximum_speed(dev);
 	dwc->dr_mode = usb_get_dr_mode(dev);
+	dwc->hsphy_mode = of_usb_get_phy_mode(dev->of_node);
 
 	dwc->has_lpm_erratum = device_property_read_bool(dev,
 				"snps,has-lpm-erratum");
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 08ed9e0..cc4f551 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -203,6 +203,14 @@
 #define DWC3_GUSB2PHYCFG_SUSPHY		(1 << 6)
 #define DWC3_GUSB2PHYCFG_ULPI_UTMI	(1 << 4)
 #define DWC3_GUSB2PHYCFG_ENBLSLPM	(1 << 8)
+#define DWC3_GUSB2PHYCFG_PHYIF(n)	(n << 3)
+#define DWC3_GUSB2PHYCFG_PHYIF_MASK	DWC3_GUSB2PHYCFG_PHYIF(1)
+#define DWC3_GUSB2PHYCFG_USBTRDTIM(n)	(n << 10)
+#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASK	DWC3_GUSB2PHYCFG_USBTRDTIM(0xf)
+#define USBTRDTIM_UTMI_8_BIT		9
+#define USBTRDTIM_UTMI_16_BIT		5
+#define UTMI_PHYIF_16_BIT		1
+#define UTMI_PHYIF_8_BIT		0
 
 /* Global USB2 PHY Vendor Control Register */
 #define DWC3_GUSB2PHYACC_NEWREGREQ	(1 << 25)
@@ -748,6 +756,9 @@ struct dwc3_scratchpad_array {
  * @maximum_speed: maximum speed requested (mainly for testing purposes)
  * @revision: revision register contents
  * @dr_mode: requested mode of operation
+ * @hsphy_mode: UTMI phy mode, one of following:
+ *		- USBPHY_INTERFACE_MODE_UTMI
+ *		- USBPHY_INTERFACE_MODE_UTMIW
  * @usb2_phy: pointer to USB2 PHY
  * @usb3_phy: pointer to USB3 PHY
  * @usb2_generic_phy: pointer to USB2 PHY
@@ -853,6 +864,7 @@ struct dwc3 {
 	size_t			regs_size;
 
 	enum usb_dr_mode	dr_mode;
+	enum usb_phy_interface	hsphy_mode;
 
 	u32			fladj;
 	u32			irq_gadget;
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v10 3/5] usb: dwc3: add dis_del_phy_power_chg_quirk

[William Wu]

Add a quirk to clear the GUSB3PIPECTL.DELAYP1TRANS bit,
which specifies whether disable delay PHY power change
from P0 to P1/P2/P3 when link state changing from U0
to U1/U2/U3 respectively.

Signed-off-by: William Wu <william.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
Changes in v10:
- None

Changes in v9:
- None

Changes in v8:
- add Acked-by (Rob Herring)

Changes in v7:
- None

Changes in v6:
- use '-' instead of '_' in dts (Rob Herring)

Changes in v5:
- None

Changes in v4:
- rebase on top of balbi testing/next, remove pdata (balbi)

Changes in v3:
- None

Changes in v2:
- None

 Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
 drivers/usb/dwc3/core.c                        | 5 +++++
 drivers/usb/dwc3/core.h                        | 3 +++
 3 files changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
index 020b0e9..e96bfc2 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -42,6 +42,8 @@ Optional properties:
  - snps,dis-u2-freeclk-exists-quirk: when set, clear the u2_freeclk_exists
 			in GUSB2PHYCFG, specify that USB2 PHY doesn't provide
 			a free-running PHY clock.
+ - snps,dis-del-phy-power-chg-quirk: when set core will change PHY power
+			from P0 to P1/P2/P3 without delay.
  - snps,is-utmi-l1-suspend: true when DWC3 asserts output signal
 			utmi_l1_suspend_n, false when asserts utmi_sleep_n
  - snps,hird-threshold: HIRD threshold
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index cdac019..e887b38 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -448,6 +448,9 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
 	if (dwc->dis_u3_susphy_quirk)
 		reg &= ~DWC3_GUSB3PIPECTL_SUSPHY;
 
+	if (dwc->dis_del_phy_power_chg_quirk)
+		reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE;
+
 	dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
 
 	reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
@@ -947,6 +950,8 @@ static int dwc3_probe(struct platform_device *pdev)
 				"snps,dis_rxdet_inp3_quirk");
 	dwc->dis_u2_freeclk_exists_quirk = device_property_read_bool(dev,
 				"snps,dis-u2-freeclk-exists-quirk");
+	dwc->dis_del_phy_power_chg_quirk = device_property_read_bool(dev,
+				"snps,dis-del-phy-power-chg-quirk");
 
 	dwc->tx_de_emphasis_quirk = device_property_read_bool(dev,
 				"snps,tx_de_emphasis_quirk");
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index cc4f551..3d94acd 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -818,6 +818,8 @@ struct dwc3_scratchpad_array {
  * @dis_u2_freeclk_exists_quirk : set if we clear u2_freeclk_exists
  *			in GUSB2PHYCFG, specify that USB2 PHY doesn't
  *			provide a free-running PHY clock.
+ * @dis_del_phy_power_chg_quirk: set if we disable delay phy power
+ *			change quirk.
  * @tx_de_emphasis_quirk: set if we enable Tx de-emphasis quirk
  * @tx_de_emphasis: Tx de-emphasis value
  * 	0	- -6dB de-emphasis
@@ -963,6 +965,7 @@ struct dwc3 {
 	unsigned		dis_enblslpm_quirk:1;
 	unsigned		dis_rxdet_inp3_quirk:1;
 	unsigned		dis_u2_freeclk_exists_quirk:1;
+	unsigned		dis_del_phy_power_chg_quirk:1;
 
 	unsigned		tx_de_emphasis_quirk:1;
 	unsigned		tx_de_emphasis:2;
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v10 4/5] usb: dwc3: rockchip: add devicetree bindings documentation

[William Wu]

This patch adds the devicetree documentation required for Rockchip
USB3.0 core wrapper consisting of USB3.0 IP from Synopsys.

It supports DRD mode, and could operate in device mode (SS, HS, FS)
and host mode (SS, HS, FS, LS).

Signed-off-by: William Wu <william.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
Changes in v10:
- None

Changes in v9:
- add required properties "resets" and "reset-names"
- add optional property "extcon"

Changes in v8:
- None

Changes in v7:
- add Acked-by (Rob Herring)

Changes in v6:
- rename bus_clk, and add usbdrd3_1 node as an example (Heiko)

Changes in v5:
- rename clock-names, and remove unnecessary clocks (Heiko)

Changes in v4:
- modify commit log, and add phy documentation location (Sergei)

Changes in v3:
- add dwc3 address (balbi)

Changes in v2:
- add rockchip,dwc3.txt to Documentation/devicetree/bindings/ (balbi, Brian)

 .../devicetree/bindings/usb/rockchip,dwc3.txt      | 71 ++++++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/rockchip,dwc3.txt

diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt b/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
new file mode 100644
index 0000000..3a79be8
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
@@ -0,0 +1,71 @@
+Rockchip SuperSpeed DWC3 USB SoC controller
+
+Required properties:
+- compatible:	should contain "rockchip,rk3399-dwc3" for rk3399 SoC
+- clocks:	A list of phandle + clock-specifier pairs for the
+		clocks listed in clock-names
+- clock-names:	Should contain the following:
+  "ref_clk"	Controller reference clk, have to be 24 MHz
+  "suspend_clk"	Controller suspend clk, have to be 24 MHz or 32 KHz
+  "bus_clk"	Master/Core clock, have to be >= 62.5 MHz for SS
+		operation and >= 30MHz for HS operation
+  "grf_clk"	Controller grf clk
+- resets:	List of phandle and reset specifier pairs. Should contain
+		softreset line of the DWC3 controller
+- reset-names:	List of reset signal names. Names should contain "usb3-otg"
+		for DWC3 controller reset.
+
+Optional properties:
+- extcon:	Phandles to external connector devices, which provide
+		"EXTCON_USB" and "EXTCON_USB_HOST" cable events.
+
+Required child node:
+A child node must exist to represent the core DWC3 IP block. The name of
+the node is not important. The content of the node is defined in dwc3.txt.
+
+Phy documentation is provided in the following places:
+Documentation/devicetree/bindings/phy/rockchip,dwc3-usb-phy.txt
+
+Example device nodes:
+
+	usbdrd3_0: usb@fe800000 {
+		compatible = "rockchip,rk3399-dwc3";
+		clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
+			 <&cru ACLK_USB3OTG0>, <&cru ACLK_USB3_GRF>;
+		clock-names = "ref_clk", "suspend_clk",
+			      "bus_clk", "grf_clk";
+		resets = <&cru SRST_A_USB3_OTG0>;
+		reset-names = "usb3-otg";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+		status = "disabled";
+		usbdrd_dwc3_0: dwc3@fe800000 {
+			compatible = "snps,dwc3";
+			reg = <0x0 0xfe800000 0x0 0x100000>;
+			interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
+			dr_mode = "otg";
+			status = "disabled";
+		};
+	};
+
+	usbdrd3_1: usb@fe900000 {
+		compatible = "rockchip,rk3399-dwc3";
+		clocks = <&cru SCLK_USB3OTG1_REF>, <&cru SCLK_USB3OTG1_SUSPEND>,
+			 <&cru ACLK_USB3OTG1>, <&cru ACLK_USB3_GRF>;
+		clock-names = "ref_clk", "suspend_clk",
+			      "bus_clk", "grf_clk";
+		resets = <&cru SRST_A_USB3_OTG1>;
+		reset-names = "usb3-otg";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+		status = "disabled";
+		usbdrd_dwc3_1: dwc3@fe900000 {
+			compatible = "snps,dwc3";
+			reg = <0x0 0xfe900000 0x0 0x100000>;
+			interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+			dr_mode = "otg";
+			status = "disabled";
+		};
+	};
-- 
1.9.1

[PATCH v10 5/5] usb: dwc3: add rockchip specific glue layer

[William Wu]

Add rockchip specific glue layer to support USB3 Peripheral mode
and Host mode on rockchip platforms (e.g. rk3399).

The DesignWare USB3 integrated in rockchip SoCs is a configurable
IP Core which can be instantiated as Dual-Role Device (DRD), Host
Only (XHCI) and Peripheral Only configurations.

We use extcon notifier to manage usb cable detection and mode switch.
Enable DWC3 PM runtime auto suspend to allow core enter runtime_suspend
if USB cable is dettached. For host mode, it requires to keep whole
DWC3 controller in reset state to hold pipe power state in P2 before
initializing PHY. And it need to reconfigure USB PHY interface of DWC3
core after deassert DWC3 controller reset.

The current driver supports Host only and Peripheral Only well, for
now, we will add support for OTG after we have it all stabilized.

Signed-off-by: William Wu <william.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---
Changes in v10:
- fix building error

Changes in v9:
- Add a new dwc3-rockchip.c driver for rk3399, rather than use dwc3-of-simple.c

 drivers/usb/dwc3/Kconfig         |   9 +
 drivers/usb/dwc3/Makefile        |   1 +
 drivers/usb/dwc3/core.c          |   2 +-
 drivers/usb/dwc3/core.h          |   1 +
 drivers/usb/dwc3/dwc3-rockchip.c | 441 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 453 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/dwc3/dwc3-rockchip.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index a64ce1c..3d5ec30 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -96,6 +96,15 @@ config USB_DWC3_OF_SIMPLE
 	 Currently supports Xilinx and Qualcomm DWC USB3 IP.
 	 Say 'Y' or 'M' if you have one such device.
 
+config USB_DWC3_ROCKCHIP
+	tristate "Rockchip Platforms"
+	depends on EXTCON && (ARCH_ROCKCHIP || COMPILE_TEST)
+	depends on OF
+	default USB_DWC3
+	help
+	  Support of USB2/3 functionality in Rockchip platforms.
+	  say 'Y' or 'M' if you have one such device.
+
 config USB_DWC3_ST
 	tristate "STMicroelectronics Platforms"
 	depends on ARCH_STI && OF
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 22420e1..86fc4fd 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -38,4 +38,5 @@ obj-$(CONFIG_USB_DWC3_EXYNOS)		+= dwc3-exynos.o
 obj-$(CONFIG_USB_DWC3_PCI)		+= dwc3-pci.o
 obj-$(CONFIG_USB_DWC3_KEYSTONE)		+= dwc3-keystone.o
 obj-$(CONFIG_USB_DWC3_OF_SIMPLE)	+= dwc3-of-simple.o
+obj-$(CONFIG_USB_DWC3_ROCKCHIP)		+= dwc3-rockchip.o
 obj-$(CONFIG_USB_DWC3_ST)		+= dwc3-st.o
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index e887b38..3da6215 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -405,7 +405,7 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc)
  * initialized. The PHY interfaces and the PHYs get initialized together with
  * the core in dwc3_core_init.
  */
-static int dwc3_phy_setup(struct dwc3 *dwc)
+int dwc3_phy_setup(struct dwc3 *dwc)
 {
 	u32 reg;
 	int ret;
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 3d94acd..79403ff 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1123,6 +1123,7 @@ struct dwc3_gadget_ep_cmd_params {
 /* prototypes */
 void dwc3_set_mode(struct dwc3 *dwc, u32 mode);
 u32 dwc3_core_fifo_space(struct dwc3_ep *dep, u8 type);
+int dwc3_phy_setup(struct dwc3 *dwc);
 
 /* check whether we are on the DWC_usb31 core */
 static inline bool dwc3_is_usb31(struct dwc3 *dwc)
diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
new file mode 100644
index 0000000..eeae1a9
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-rockchip.c
@@ -0,0 +1,441 @@
+/**
+ * dwc3-rockchip.c - Rockchip Specific Glue layer
+ *
+ * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
+ *
+ * Authors: William Wu <william.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/pm_runtime.h>
+#include <linux/extcon.h>
+#include <linux/reset.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+
+#include "core.h"
+#include "io.h"
+
+#define DWC3_ROCKCHIP_AUTOSUSPEND_DELAY  500 /* ms */
+
+struct dwc3_rockchip {
+	int			num_clocks;
+	struct device		*dev;
+	struct clk		**clks;
+	struct dwc3		*dwc;
+	struct reset_control	*otg_rst;
+	struct extcon_dev	*edev;
+	struct notifier_block	device_nb;
+	struct notifier_block	host_nb;
+	struct work_struct	otg_work;
+};
+
+static int dwc3_rockchip_device_notifier(struct notifier_block *nb,
+					 unsigned long event, void *ptr)
+{
+	struct dwc3_rockchip *rockchip =
+		container_of(nb, struct dwc3_rockchip, device_nb);
+
+	schedule_work(&rockchip->otg_work);
+
+	return NOTIFY_DONE;
+}
+
+static int dwc3_rockchip_host_notifier(struct notifier_block *nb,
+				       unsigned long event, void *ptr)
+{
+	struct dwc3_rockchip *rockchip =
+		container_of(nb, struct dwc3_rockchip, host_nb);
+
+	schedule_work(&rockchip->otg_work);
+
+	return NOTIFY_DONE;
+}
+
+static void dwc3_rockchip_otg_extcon_evt_work(struct work_struct *work)
+{
+	struct dwc3_rockchip	*rockchip =
+		container_of(work, struct dwc3_rockchip, otg_work);
+	struct dwc3		*dwc = rockchip->dwc;
+	struct extcon_dev	*edev = rockchip->edev;
+	struct usb_hcd		*hcd;
+	unsigned long		flags;
+	int			ret;
+	u32			reg;
+
+	if (extcon_get_cable_state_(edev, EXTCON_USB) > 0) {
+		if (dwc->connected)
+			return;
+
+		/*
+		 * If dr_mode is host only, never to set
+		 * the mode to the peripheral mode.
+		 */
+		if (WARN_ON(dwc->dr_mode == USB_DR_MODE_HOST))
+			return;
+
+		pm_runtime_get_sync(dwc->dev);
+
+		spin_lock_irqsave(&dwc->lock, flags);
+		dwc->connected = true;
+		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
+		spin_unlock_irqrestore(&dwc->lock, flags);
+
+		pm_runtime_put_sync(dwc->dev);
+
+		dev_info(rockchip->dev, "USB peripheral connected\n");
+	} else if (extcon_get_cable_state_(edev, EXTCON_USB_HOST) > 0) {
+		if (dwc->connected)
+			return;
+
+		/*
+		 * If dr_mode is device only, never to
+		 * set the mode to the host mode.
+		 */
+		if (WARN_ON(dwc->dr_mode == USB_DR_MODE_PERIPHERAL))
+			return;
+
+		reset_control_assert(rockchip->otg_rst);
+
+		ret = phy_power_on(dwc->usb2_generic_phy);
+		if (ret < 0)
+			return;
+
+		ret = phy_power_on(dwc->usb3_generic_phy);
+		if (ret < 0)
+			return;
+
+		reset_control_deassert(rockchip->otg_rst);
+
+		dwc3_phy_setup(dwc);
+
+		hcd = dev_get_drvdata(&dwc->xhci->dev);
+
+		spin_lock_irqsave(&dwc->lock, flags);
+		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
+		dwc->connected = true;
+		spin_unlock_irqrestore(&dwc->lock, flags);
+
+		if (hcd->state == HC_STATE_HALT) {
+			usb_add_hcd(hcd, hcd->irq, IRQF_SHARED);
+			usb_add_hcd(hcd->shared_hcd, hcd->irq, IRQF_SHARED);
+		}
+
+		dev_info(rockchip->dev, "USB HOST connected\n");
+	} else {
+		if (!dwc->connected)
+			return;
+
+		reg = dwc3_readl(dwc->regs, DWC3_GCTL);
+
+		if (DWC3_GCTL_PRTCAP(reg) == DWC3_GCTL_PRTCAP_HOST ||
+		    DWC3_GCTL_PRTCAP(reg) == DWC3_GCTL_PRTCAP_OTG) {
+			hcd = dev_get_drvdata(&dwc->xhci->dev);
+
+			if (hcd->state != HC_STATE_HALT) {
+				usb_remove_hcd(hcd->shared_hcd);
+				usb_remove_hcd(hcd);
+			}
+
+			phy_power_off(dwc->usb2_generic_phy);
+			phy_power_off(dwc->usb3_generic_phy);
+		}
+
+		spin_lock_irqsave(&dwc->lock, flags);
+		dwc->connected = false;
+		spin_unlock_irqrestore(&dwc->lock, flags);
+
+		dev_info(rockchip->dev, "USB unconnected\n");
+	}
+}
+
+static int dwc3_rockchip_extcon_register(struct dwc3_rockchip *rockchip)
+{
+	int			ret;
+	struct device		*dev = rockchip->dev;
+	struct extcon_dev	*edev;
+
+	if (device_property_read_bool(dev, "extcon")) {
+		edev = extcon_get_edev_by_phandle(dev, 0);
+		if (IS_ERR(edev)) {
+			if (PTR_ERR(edev) != -EPROBE_DEFER)
+				dev_err(dev, "couldn't get extcon device\n");
+			return PTR_ERR(edev);
+		}
+
+		INIT_WORK(&rockchip->otg_work,
+			  dwc3_rockchip_otg_extcon_evt_work);
+
+		rockchip->device_nb.notifier_call =
+				dwc3_rockchip_device_notifier;
+		ret = extcon_register_notifier(edev, EXTCON_USB,
+					       &rockchip->device_nb);
+		if (ret < 0) {
+			dev_err(dev, "failed to register notifier for USB\n");
+			return ret;
+		}
+
+		rockchip->host_nb.notifier_call =
+				dwc3_rockchip_host_notifier;
+		ret = extcon_register_notifier(edev, EXTCON_USB_HOST,
+					       &rockchip->host_nb);
+		if (ret < 0) {
+			dev_err(dev, "failed to register notifier for USB HOST\n");
+			extcon_unregister_notifier(edev, EXTCON_USB,
+						   &rockchip->device_nb);
+			return ret;
+		}
+
+		rockchip->edev = edev;
+	}
+
+	return 0;
+}
+
+static void dwc3_rockchip_extcon_unregister(struct dwc3_rockchip *rockchip)
+{
+	if (!rockchip->edev)
+		return;
+
+	extcon_unregister_notifier(rockchip->edev, EXTCON_USB,
+				   &rockchip->device_nb);
+	extcon_unregister_notifier(rockchip->edev, EXTCON_USB_HOST,
+				   &rockchip->host_nb);
+}
+
+static int dwc3_rockchip_probe(struct platform_device *pdev)
+{
+	struct dwc3_rockchip	*rockchip;
+	struct device		*dev = &pdev->dev;
+	struct device_node	*np = dev->of_node, *child;
+	struct platform_device	*child_pdev;
+
+	unsigned int		count;
+	int			ret;
+	int			i;
+
+	rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL);
+
+	if (!rockchip)
+		return -ENOMEM;
+
+	count = of_clk_get_parent_count(np);
+	if (!count)
+		return -ENOENT;
+
+	rockchip->num_clocks = count;
+
+	rockchip->clks = devm_kcalloc(dev, rockchip->num_clocks,
+				      sizeof(struct clk *), GFP_KERNEL);
+	if (!rockchip->clks)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, rockchip);
+
+	rockchip->dev = dev;
+	rockchip->edev = NULL;
+
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		dev_err(dev, "get_sync failed with err %d\n", ret);
+		goto err1;
+	}
+
+	for (i = 0; i < rockchip->num_clocks; i++) {
+		struct clk	*clk;
+
+		clk = of_clk_get(np, i);
+		if (IS_ERR(clk)) {
+			while (--i >= 0)
+				clk_put(rockchip->clks[i]);
+			ret = PTR_ERR(clk);
+
+			goto err1;
+		}
+
+		ret = clk_prepare_enable(clk);
+		if (ret < 0) {
+			while (--i >= 0) {
+				clk_disable_unprepare(rockchip->clks[i]);
+				clk_put(rockchip->clks[i]);
+			}
+			clk_put(clk);
+
+			goto err1;
+		}
+
+		rockchip->clks[i] = clk;
+	}
+
+	rockchip->otg_rst = devm_reset_control_get(dev, "usb3-otg");
+	if (IS_ERR(rockchip->otg_rst)) {
+		dev_err(dev, "could not get reset controller\n");
+		ret = PTR_ERR(rockchip->otg_rst);
+		goto err2;
+	}
+
+	ret = dwc3_rockchip_extcon_register(rockchip);
+	if (ret < 0)
+		goto err2;
+
+	child = of_get_child_by_name(np, "dwc3");
+	if (!child) {
+		dev_err(dev, "failed to find dwc3 core node\n");
+		ret = -ENODEV;
+		goto err3;
+	}
+
+	/* Allocate and initialize the core */
+	ret = of_platform_populate(np, NULL, NULL, dev);
+	if (ret) {
+		dev_err(dev, "failed to create dwc3 core\n");
+		goto err3;
+	}
+
+	child_pdev = of_find_device_by_node(child);
+	if (!child_pdev) {
+		dev_err(dev, "failed to find dwc3 core device\n");
+		ret = -ENODEV;
+		goto err4;
+	}
+
+	rockchip->dwc = platform_get_drvdata(child_pdev);
+
+	if (rockchip->edev) {
+		pm_runtime_set_autosuspend_delay(&child_pdev->dev,
+						 DWC3_ROCKCHIP_AUTOSUSPEND_DELAY);
+		pm_runtime_allow(&child_pdev->dev);
+
+		if (rockchip->dwc->dr_mode == USB_DR_MODE_HOST ||
+		    rockchip->dwc->dr_mode == USB_DR_MODE_OTG) {
+			struct usb_hcd *hcd =
+				dev_get_drvdata(&rockchip->dwc->xhci->dev);
+
+			if (hcd->state != HC_STATE_HALT) {
+				usb_remove_hcd(hcd->shared_hcd);
+				usb_remove_hcd(hcd);
+			}
+		}
+	}
+
+	return ret;
+
+err4:
+	of_platform_depopulate(dev);
+
+err3:
+	dwc3_rockchip_extcon_unregister(rockchip);
+
+err2:
+	for (i = 0; i < rockchip->num_clocks; i++) {
+		clk_disable_unprepare(rockchip->clks[i]);
+		clk_put(rockchip->clks[i]);
+	}
+
+err1:
+	pm_runtime_put_sync(dev);
+	pm_runtime_disable(dev);
+
+	return ret;
+}
+
+static int dwc3_rockchip_remove(struct platform_device *pdev)
+{
+	struct dwc3_rockchip	*rockchip = platform_get_drvdata(pdev);
+	struct device		*dev = &pdev->dev;
+	int			i;
+
+	for (i = 0; i < rockchip->num_clocks; i++) {
+		clk_disable_unprepare(rockchip->clks[i]);
+		clk_put(rockchip->clks[i]);
+	}
+
+	dwc3_rockchip_extcon_unregister(rockchip);
+
+	of_platform_depopulate(dev);
+
+	pm_runtime_put_sync(dev);
+	pm_runtime_disable(dev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int dwc3_rockchip_suspend(struct device *dev)
+{
+	struct dwc3_rockchip	*rockchip = dev_get_drvdata(dev);
+	int			i;
+
+	for (i = 0; i < rockchip->num_clocks; i++)
+		clk_disable(rockchip->clks[i]);
+
+	return 0;
+}
+
+static int dwc3_rockchip_resume(struct device *dev)
+{
+	struct dwc3_rockchip	*rockchip = dev_get_drvdata(dev);
+	int			i;
+
+	for (i = 0; i < rockchip->num_clocks; i++)
+		clk_enable(rockchip->clks[i]);
+
+	/* runtime set active to reflect active state. */
+	pm_runtime_disable(dev);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+
+	return 0;
+}
+
+static const struct dev_pm_ops dwc3_rockchip_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(dwc3_rockchip_suspend, dwc3_rockchip_resume)
+};
+
+#define DEV_PM_OPS	(&dwc3_rockchip_dev_pm_ops)
+#else
+#define DEV_PM_OPS	NULL
+#endif /* CONFIG_PM_SLEEP */
+
+static const struct of_device_id rockchip_dwc3_match[] = {
+	{ .compatible = "rockchip,rk3399-dwc3" },
+	{ /* Sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, rockchip_dwc3_match);
+
+static struct platform_driver dwc3_rockchip_driver = {
+	.probe		= dwc3_rockchip_probe,
+	.remove		= dwc3_rockchip_remove,
+	.driver		= {
+		.name	= "rockchip-dwc3",
+		.of_match_table = rockchip_dwc3_match,
+		.pm	= DEV_PM_OPS,
+	},
+};
+
+module_platform_driver(dwc3_rockchip_driver);
+
+MODULE_ALIAS("platform:rockchip-dwc3");
+MODULE_AUTHOR("William Wu <william.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("DesignWare USB3 ROCKCHIP Glue Layer");
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 5/5] usb: dwc3: add rockchip specific glue layer

[kbuild test robot]

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

Hi William,

[auto build test ERROR on balbi-usb/next]
[also build test ERROR on v4.8-rc2 next-20160815]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/William-Wu/support-rockchip-dwc3-driver/20160815-165927
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: m68k-allyesconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=m68k 

All errors (new ones prefixed by >>):

   drivers/usb/dwc3/dwc3-rockchip.c: In function 'dwc3_rockchip_probe':
>> drivers/usb/dwc3/dwc3-rockchip.c:239:2: error: implicit declaration of function 'of_clk_get_parent_count' [-Werror=implicit-function-declaration]
     count = of_clk_get_parent_count(np);
     ^
   cc1: some warnings being treated as errors

vim +/of_clk_get_parent_count +239 drivers/usb/dwc3/dwc3-rockchip.c

   233	
   234		rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL);
   235	
   236		if (!rockchip)
   237			return -ENOMEM;
   238	
 > 239		count = of_clk_get_parent_count(np);
   240		if (!count)
   241			return -ENOENT;
   242	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 37282 bytes --]

Re: [PATCH v10 5/5] usb: dwc3: add rockchip specific glue layer

[kbuild test robot]

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

Hi William,

[auto build test ERROR on balbi-usb/next]
[also build test ERROR on v4.8-rc2 next-20160815]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/William-Wu/support-rockchip-dwc3-driver/20160815-165927
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: arm-arm67 (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> ERROR: "dwc3_trace" [drivers/usb/dwc3/dwc3-rockchip.ko] undefined!
>> ERROR: "dwc3_phy_setup" [drivers/usb/dwc3/dwc3-rockchip.ko] undefined!
>> ERROR: "dwc3_set_mode" [drivers/usb/dwc3/dwc3-rockchip.ko] undefined!
>> ERROR: "__tracepoint_dwc3_readl" [drivers/usb/dwc3/dwc3-rockchip.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 29458 bytes --]

Re: [PATCH v10 5/5] usb: dwc3: add rockchip specific glue layer

[Felipe Balbi]

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


Hi,

William Wu <william.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org> writes:
> Add rockchip specific glue layer to support USB3 Peripheral mode
> and Host mode on rockchip platforms (e.g. rk3399).
>
> The DesignWare USB3 integrated in rockchip SoCs is a configurable
> IP Core which can be instantiated as Dual-Role Device (DRD), Host
> Only (XHCI) and Peripheral Only configurations.
>
> We use extcon notifier to manage usb cable detection and mode switch.
> Enable DWC3 PM runtime auto suspend to allow core enter runtime_suspend
> if USB cable is dettached. For host mode, it requires to keep whole
> DWC3 controller in reset state to hold pipe power state in P2 before
> initializing PHY. And it need to reconfigure USB PHY interface of DWC3
> core after deassert DWC3 controller reset.
>
> The current driver supports Host only and Peripheral Only well, for
> now, we will add support for OTG after we have it all stabilized.
>
> Signed-off-by: William Wu <william.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
> Changes in v10:
> - fix building error
>
> Changes in v9:
> - Add a new dwc3-rockchip.c driver for rk3399, rather than use dwc3-of-simple.c
>
>  drivers/usb/dwc3/Kconfig         |   9 +
>  drivers/usb/dwc3/Makefile        |   1 +
>  drivers/usb/dwc3/core.c          |   2 +-
>  drivers/usb/dwc3/core.h          |   1 +
>  drivers/usb/dwc3/dwc3-rockchip.c | 441 +++++++++++++++++++++++++++++++++++++++

William, if you need to touch core dwc3 to introduce a glue layer,
you're doing it wrong.

> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index e887b38..3da6215 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -405,7 +405,7 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc)
>   * initialized. The PHY interfaces and the PHYs get initialized together with
>   * the core in dwc3_core_init.
>   */
> -static int dwc3_phy_setup(struct dwc3 *dwc)
> +int dwc3_phy_setup(struct dwc3 *dwc)

there's no way I'll let this slip through the cracks :-)

> diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
> new file mode 100644
> index 0000000..eeae1a9
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-rockchip.c
> @@ -0,0 +1,441 @@

[...]

> +static int dwc3_rockchip_probe(struct platform_device *pdev)
> +{
> +	struct dwc3_rockchip	*rockchip;
> +	struct device		*dev = &pdev->dev;
> +	struct device_node	*np = dev->of_node, *child;
> +	struct platform_device	*child_pdev;
> +
> +	unsigned int		count;
> +	int			ret;
> +	int			i;
> +
> +	rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL);
> +
> +	if (!rockchip)
> +		return -ENOMEM;
> +
> +	count = of_clk_get_parent_count(np);
> +	if (!count)
> +		return -ENOENT;
> +
> +	rockchip->num_clocks = count;
> +
> +	rockchip->clks = devm_kcalloc(dev, rockchip->num_clocks,
> +				      sizeof(struct clk *), GFP_KERNEL);
> +	if (!rockchip->clks)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, rockchip);
> +
> +	rockchip->dev = dev;
> +	rockchip->edev = NULL;
> +
> +	pm_runtime_set_active(dev);
> +	pm_runtime_enable(dev);
> +	ret = pm_runtime_get_sync(dev);
> +	if (ret < 0) {
> +		dev_err(dev, "get_sync failed with err %d\n", ret);
> +		goto err1;
> +	}
> +
> +	for (i = 0; i < rockchip->num_clocks; i++) {
> +		struct clk	*clk;
> +
> +		clk = of_clk_get(np, i);
> +		if (IS_ERR(clk)) {
> +			while (--i >= 0)
> +				clk_put(rockchip->clks[i]);
> +			ret = PTR_ERR(clk);
> +
> +			goto err1;
> +		}
> +
> +		ret = clk_prepare_enable(clk);
> +		if (ret < 0) {
> +			while (--i >= 0) {
> +				clk_disable_unprepare(rockchip->clks[i]);
> +				clk_put(rockchip->clks[i]);
> +			}
> +			clk_put(clk);
> +
> +			goto err1;
> +		}
> +
> +		rockchip->clks[i] = clk;
> +	}
> +
> +	rockchip->otg_rst = devm_reset_control_get(dev, "usb3-otg");
> +	if (IS_ERR(rockchip->otg_rst)) {
> +		dev_err(dev, "could not get reset controller\n");
> +		ret = PTR_ERR(rockchip->otg_rst);
> +		goto err2;
> +	}
> +
> +	ret = dwc3_rockchip_extcon_register(rockchip);
> +	if (ret < 0)
> +		goto err2;
> +
> +	child = of_get_child_by_name(np, "dwc3");
> +	if (!child) {
> +		dev_err(dev, "failed to find dwc3 core node\n");
> +		ret = -ENODEV;
> +		goto err3;
> +	}
> +
> +	/* Allocate and initialize the core */
> +	ret = of_platform_populate(np, NULL, NULL, dev);
> +	if (ret) {
> +		dev_err(dev, "failed to create dwc3 core\n");
> +		goto err3;
> +	}
> +
> +	child_pdev = of_find_device_by_node(child);
> +	if (!child_pdev) {
> +		dev_err(dev, "failed to find dwc3 core device\n");
> +		ret = -ENODEV;
> +		goto err4;
> +	}
> +
> +	rockchip->dwc = platform_get_drvdata(child_pdev);

No! You will *NOT* the core struct device. Don't even try to come up
with tricks like this.

Let's do this: introduce a glue layer that gets peripheral-only
working. Remove PM for now, too. Start with something simple, get the
bare minimum working upstream and add more stuff as you go.

Trying to do everything in one patch just makes it much more likely for
your patch to be NAKed. What you're doing here is bypassing all the
layering we've built. That won't work very well. The only thing you'll
get is for your patches to continue to be NAKed.

Avoid the tricks and abuses. Just because you _can_ do it somehow, it
doesn't mean you _should_ do it :-)

Your best option right now, is to remove PM and dual-role support and a
minimal glue layer supported.

In fact, all you *really* need is to add a compatible to
dwc3-of-simple.c. That should be enough to get your dwc3 working. Don't
do anything more than that. For dual-role and PM, we can add it
generically to dwc3-of-simple.c when all pieces fall into place.

-- 
balbi

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

Re: [PATCH v10 5/5] usb: dwc3: add rockchip specific glue layer

[William.wu]

Dear Balbi，


On 2016/8/16 15:19, Felipe Balbi wrote:
> Hi,
>
> William Wu <william.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org> writes:
>> Add rockchip specific glue layer to support USB3 Peripheral mode
>> and Host mode on rockchip platforms (e.g. rk3399).
>>
>> The DesignWare USB3 integrated in rockchip SoCs is a configurable
>> IP Core which can be instantiated as Dual-Role Device (DRD), Host
>> Only (XHCI) and Peripheral Only configurations.
>>
>> We use extcon notifier to manage usb cable detection and mode switch.
>> Enable DWC3 PM runtime auto suspend to allow core enter runtime_suspend
>> if USB cable is dettached. For host mode, it requires to keep whole
>> DWC3 controller in reset state to hold pipe power state in P2 before
>> initializing PHY. And it need to reconfigure USB PHY interface of DWC3
>> core after deassert DWC3 controller reset.
>>
>> The current driver supports Host only and Peripheral Only well, for
>> now, we will add support for OTG after we have it all stabilized.
>>
>> Signed-off-by: William Wu <william.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>> ---
>> Changes in v10:
>> - fix building error
>>
>> Changes in v9:
>> - Add a new dwc3-rockchip.c driver for rk3399, rather than use dwc3-of-simple.c
>>
>>   drivers/usb/dwc3/Kconfig         |   9 +
>>   drivers/usb/dwc3/Makefile        |   1 +
>>   drivers/usb/dwc3/core.c          |   2 +-
>>   drivers/usb/dwc3/core.h          |   1 +
>>   drivers/usb/dwc3/dwc3-rockchip.c | 441 +++++++++++++++++++++++++++++++++++++++
> William, if you need to touch core dwc3 to introduce a glue layer,
> you're doing it wrong.

Sorry， I realized that it's not better to touch core dwc3 in a specific 
glue layer.
I touched dwc3 in glue layer, because I want to support dual-role mode， and
according to our dwc3 IP and usb3 PHY IP design， it need to reinit dwc3 
core
whenever  usb cable attached.

Anyway, it's wrong to do that.:-[

>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index e887b38..3da6215 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -405,7 +405,7 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc)
>>    * initialized. The PHY interfaces and the PHYs get initialized together with
>>    * the core in dwc3_core_init.
>>    */
>> -static int dwc3_phy_setup(struct dwc3 *dwc)
>> +int dwc3_phy_setup(struct dwc3 *dwc)
> there's no way I'll let this slip through the cracks :-)

Why I need  dwc3_phy_setup in  glue layer is because usb3 IP design
in rockchip platform. If dwc3 works on host mode, it requires to put
dwc3 controller in reset state before usb3 phy initializing，and after
deassert reset,  we need to reconfigure UTMI+ PHY interface because
our dwc3 core can't configure PHY interface correctly.

Thank you for give me a chance to explain it.:-)

>
>> diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
>> new file mode 100644
>> index 0000000..eeae1a9
>> --- /dev/null
>> +++ b/drivers/usb/dwc3/dwc3-rockchip.c
>> @@ -0,0 +1,441 @@
> [...]
>
>> +static int dwc3_rockchip_probe(struct platform_device *pdev)
>> +{
>> +	struct dwc3_rockchip	*rockchip;
>> +	struct device		*dev = &pdev->dev;
>> +	struct device_node	*np = dev->of_node, *child;
>> +	struct platform_device	*child_pdev;
>> +
>> +	unsigned int		count;
>> +	int			ret;
>> +	int			i;
>> +
>> +	rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL);
>> +
>> +	if (!rockchip)
>> +		return -ENOMEM;
>> +
>> +	count = of_clk_get_parent_count(np);
>> +	if (!count)
>> +		return -ENOENT;
>> +
>> +	rockchip->num_clocks = count;
>> +
>> +	rockchip->clks = devm_kcalloc(dev, rockchip->num_clocks,
>> +				      sizeof(struct clk *), GFP_KERNEL);
>> +	if (!rockchip->clks)
>> +		return -ENOMEM;
>> +
>> +	platform_set_drvdata(pdev, rockchip);
>> +
>> +	rockchip->dev = dev;
>> +	rockchip->edev = NULL;
>> +
>> +	pm_runtime_set_active(dev);
>> +	pm_runtime_enable(dev);
>> +	ret = pm_runtime_get_sync(dev);
>> +	if (ret < 0) {
>> +		dev_err(dev, "get_sync failed with err %d\n", ret);
>> +		goto err1;
>> +	}
>> +
>> +	for (i = 0; i < rockchip->num_clocks; i++) {
>> +		struct clk	*clk;
>> +
>> +		clk = of_clk_get(np, i);
>> +		if (IS_ERR(clk)) {
>> +			while (--i >= 0)
>> +				clk_put(rockchip->clks[i]);
>> +			ret = PTR_ERR(clk);
>> +
>> +			goto err1;
>> +		}
>> +
>> +		ret = clk_prepare_enable(clk);
>> +		if (ret < 0) {
>> +			while (--i >= 0) {
>> +				clk_disable_unprepare(rockchip->clks[i]);
>> +				clk_put(rockchip->clks[i]);
>> +			}
>> +			clk_put(clk);
>> +
>> +			goto err1;
>> +		}
>> +
>> +		rockchip->clks[i] = clk;
>> +	}
>> +
>> +	rockchip->otg_rst = devm_reset_control_get(dev, "usb3-otg");
>> +	if (IS_ERR(rockchip->otg_rst)) {
>> +		dev_err(dev, "could not get reset controller\n");
>> +		ret = PTR_ERR(rockchip->otg_rst);
>> +		goto err2;
>> +	}
>> +
>> +	ret = dwc3_rockchip_extcon_register(rockchip);
>> +	if (ret < 0)
>> +		goto err2;
>> +
>> +	child = of_get_child_by_name(np, "dwc3");
>> +	if (!child) {
>> +		dev_err(dev, "failed to find dwc3 core node\n");
>> +		ret = -ENODEV;
>> +		goto err3;
>> +	}
>> +
>> +	/* Allocate and initialize the core */
>> +	ret = of_platform_populate(np, NULL, NULL, dev);
>> +	if (ret) {
>> +		dev_err(dev, "failed to create dwc3 core\n");
>> +		goto err3;
>> +	}
>> +
>> +	child_pdev = of_find_device_by_node(child);
>> +	if (!child_pdev) {
>> +		dev_err(dev, "failed to find dwc3 core device\n");
>> +		ret = -ENODEV;
>> +		goto err4;
>> +	}
>> +
>> +	rockchip->dwc = platform_get_drvdata(child_pdev);
> No! You will *NOT* the core struct device. Don't even try to come up
> with tricks like this.
>
> Let's do this: introduce a glue layer that gets peripheral-only
> working. Remove PM for now, too. Start with something simple, get the
> bare minimum working upstream and add more stuff as you go.
>
> Trying to do everything in one patch just makes it much more likely for
> your patch to be NAKed. What you're doing here is bypassing all the
> layering we've built. That won't work very well. The only thing you'll
> get is for your patches to continue to be NAKed.
>
> Avoid the tricks and abuses. Just because you _can_ do it somehow, it
> doesn't mean you _should_ do it :-)
>
> Your best option right now, is to remove PM and dual-role support and a
> minimal glue layer supported.
>
> In fact, all you *really* need is to add a compatible to
> dwc3-of-simple.c. That should be enough to get your dwc3 working. Don't
> do anything more than that. For dual-role and PM, we can add it
> generically to dwc3-of-simple.c when all pieces fall into place.
>
Ah, thanks very much for your kind explanation. I think I just only need
to add a compatible to dwc3-of-simple.c，for now， and I have tested
my dwc3， it worked well on peripheral only mode and host only mode
without PM. Further, if dwc3-of-simple.c adds generic handling of dual-role
and PM, I can improve our dwc3 feature.:-)

Best regards，
         William.wu




--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 5/5] usb: dwc3: add rockchip specific glue layer

[Felipe Balbi]

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


Hi,

"William.wu" <William.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org> writes:
>> William Wu <william.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org> writes:
>>> Add rockchip specific glue layer to support USB3 Peripheral mode
>>> and Host mode on rockchip platforms (e.g. rk3399).
>>>
>>> The DesignWare USB3 integrated in rockchip SoCs is a configurable
>>> IP Core which can be instantiated as Dual-Role Device (DRD), Host
>>> Only (XHCI) and Peripheral Only configurations.
>>>
>>> We use extcon notifier to manage usb cable detection and mode switch.
>>> Enable DWC3 PM runtime auto suspend to allow core enter runtime_suspend
>>> if USB cable is dettached. For host mode, it requires to keep whole
>>> DWC3 controller in reset state to hold pipe power state in P2 before
>>> initializing PHY. And it need to reconfigure USB PHY interface of DWC3
>>> core after deassert DWC3 controller reset.
>>>
>>> The current driver supports Host only and Peripheral Only well, for
>>> now, we will add support for OTG after we have it all stabilized.
>>>
>>> Signed-off-by: William Wu <william.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>>> ---
>>> Changes in v10:
>>> - fix building error
>>>
>>> Changes in v9:
>>> - Add a new dwc3-rockchip.c driver for rk3399, rather than use dwc3-of-simple.c
>>>
>>>   drivers/usb/dwc3/Kconfig         |   9 +
>>>   drivers/usb/dwc3/Makefile        |   1 +
>>>   drivers/usb/dwc3/core.c          |   2 +-
>>>   drivers/usb/dwc3/core.h          |   1 +
>>>   drivers/usb/dwc3/dwc3-rockchip.c | 441 +++++++++++++++++++++++++++++++++++++++
>> William, if you need to touch core dwc3 to introduce a glue layer,
>> you're doing it wrong.
>
> Sorry， I realized that it's not better to touch core dwc3 in a specific 
> glue layer.
> I touched dwc3 in glue layer, because I want to support dual-role mode， and
> according to our dwc3 IP and usb3 PHY IP design， it need to reinit dwc3 
> core
> whenever  usb cable attached.
>
> Anyway, it's wrong to do that.:-[
>
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index e887b38..3da6215 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -405,7 +405,7 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc)
>>>    * initialized. The PHY interfaces and the PHYs get initialized together with
>>>    * the core in dwc3_core_init.
>>>    */
>>> -static int dwc3_phy_setup(struct dwc3 *dwc)
>>> +int dwc3_phy_setup(struct dwc3 *dwc)
>> there's no way I'll let this slip through the cracks :-)
>
> Why I need  dwc3_phy_setup in  glue layer is because usb3 IP design
> in rockchip platform. If dwc3 works on host mode, it requires to put
> dwc3 controller in reset state before usb3 phy initializing，and after
> deassert reset,  we need to reconfigure UTMI+ PHY interface because
> our dwc3 core can't configure PHY interface correctly.
>
> Thank you for give me a chance to explain it.:-)
>
>>
>>> diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
>>> new file mode 100644
>>> index 0000000..eeae1a9
>>> --- /dev/null
>>> +++ b/drivers/usb/dwc3/dwc3-rockchip.c
>>> @@ -0,0 +1,441 @@
>> [...]
>>
>>> +static int dwc3_rockchip_probe(struct platform_device *pdev)
>>> +{
>>> +	struct dwc3_rockchip	*rockchip;
>>> +	struct device		*dev = &pdev->dev;
>>> +	struct device_node	*np = dev->of_node, *child;
>>> +	struct platform_device	*child_pdev;
>>> +
>>> +	unsigned int		count;
>>> +	int			ret;
>>> +	int			i;
>>> +
>>> +	rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL);
>>> +
>>> +	if (!rockchip)
>>> +		return -ENOMEM;
>>> +
>>> +	count = of_clk_get_parent_count(np);
>>> +	if (!count)
>>> +		return -ENOENT;
>>> +
>>> +	rockchip->num_clocks = count;
>>> +
>>> +	rockchip->clks = devm_kcalloc(dev, rockchip->num_clocks,
>>> +				      sizeof(struct clk *), GFP_KERNEL);
>>> +	if (!rockchip->clks)
>>> +		return -ENOMEM;
>>> +
>>> +	platform_set_drvdata(pdev, rockchip);
>>> +
>>> +	rockchip->dev = dev;
>>> +	rockchip->edev = NULL;
>>> +
>>> +	pm_runtime_set_active(dev);
>>> +	pm_runtime_enable(dev);
>>> +	ret = pm_runtime_get_sync(dev);
>>> +	if (ret < 0) {
>>> +		dev_err(dev, "get_sync failed with err %d\n", ret);
>>> +		goto err1;
>>> +	}
>>> +
>>> +	for (i = 0; i < rockchip->num_clocks; i++) {
>>> +		struct clk	*clk;
>>> +
>>> +		clk = of_clk_get(np, i);
>>> +		if (IS_ERR(clk)) {
>>> +			while (--i >= 0)
>>> +				clk_put(rockchip->clks[i]);
>>> +			ret = PTR_ERR(clk);
>>> +
>>> +			goto err1;
>>> +		}
>>> +
>>> +		ret = clk_prepare_enable(clk);
>>> +		if (ret < 0) {
>>> +			while (--i >= 0) {
>>> +				clk_disable_unprepare(rockchip->clks[i]);
>>> +				clk_put(rockchip->clks[i]);
>>> +			}
>>> +			clk_put(clk);
>>> +
>>> +			goto err1;
>>> +		}
>>> +
>>> +		rockchip->clks[i] = clk;
>>> +	}
>>> +
>>> +	rockchip->otg_rst = devm_reset_control_get(dev, "usb3-otg");
>>> +	if (IS_ERR(rockchip->otg_rst)) {
>>> +		dev_err(dev, "could not get reset controller\n");
>>> +		ret = PTR_ERR(rockchip->otg_rst);
>>> +		goto err2;
>>> +	}
>>> +
>>> +	ret = dwc3_rockchip_extcon_register(rockchip);
>>> +	if (ret < 0)
>>> +		goto err2;
>>> +
>>> +	child = of_get_child_by_name(np, "dwc3");
>>> +	if (!child) {
>>> +		dev_err(dev, "failed to find dwc3 core node\n");
>>> +		ret = -ENODEV;
>>> +		goto err3;
>>> +	}
>>> +
>>> +	/* Allocate and initialize the core */
>>> +	ret = of_platform_populate(np, NULL, NULL, dev);
>>> +	if (ret) {
>>> +		dev_err(dev, "failed to create dwc3 core\n");
>>> +		goto err3;
>>> +	}
>>> +
>>> +	child_pdev = of_find_device_by_node(child);
>>> +	if (!child_pdev) {
>>> +		dev_err(dev, "failed to find dwc3 core device\n");
>>> +		ret = -ENODEV;
>>> +		goto err4;
>>> +	}
>>> +
>>> +	rockchip->dwc = platform_get_drvdata(child_pdev);
>> No! You will *NOT* the core struct device. Don't even try to come up
>> with tricks like this.
>>
>> Let's do this: introduce a glue layer that gets peripheral-only
>> working. Remove PM for now, too. Start with something simple, get the
>> bare minimum working upstream and add more stuff as you go.
>>
>> Trying to do everything in one patch just makes it much more likely for
>> your patch to be NAKed. What you're doing here is bypassing all the
>> layering we've built. That won't work very well. The only thing you'll
>> get is for your patches to continue to be NAKed.
>>
>> Avoid the tricks and abuses. Just because you _can_ do it somehow, it
>> doesn't mean you _should_ do it :-)
>>
>> Your best option right now, is to remove PM and dual-role support and a
>> minimal glue layer supported.
>>
>> In fact, all you *really* need is to add a compatible to
>> dwc3-of-simple.c. That should be enough to get your dwc3 working. Don't
>> do anything more than that. For dual-role and PM, we can add it
>> generically to dwc3-of-simple.c when all pieces fall into place.
>>
> Ah, thanks very much for your kind explanation. I think I just only need
> to add a compatible to dwc3-of-simple.c，for now， and I have tested
> my dwc3， it worked well on peripheral only mode and host only mode
> without PM. Further, if dwc3-of-simple.c adds generic handling of dual-role
> and PM, I can improve our dwc3 feature.:-)

that's my point exactly. We can add more support generically so that
other platforms can benefit from the work. PM should be simple for
dwc3-of-simple.c. Dual-role will take a little more effort. In almost
there actually. There are a few missing pieces but it should be doable
(hopefully) within the next two major releases.

Your integration is no different than other companies' using DWC3 in
dual-role setup. For example TI's DWC3 have the same requirements as you
do, so it makes sense to add it straight to dwc3-core. Roger Quadros
(now in Cc) has been working on dual-role for TI's platforms and we've
been discussing about how to add missing pieces generically. Perhaps
you'd want to join the discussion.

-- 
balbi

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

Re: [PATCH v10 5/5] usb: dwc3: add rockchip specific glue layer

[William.wu]

Dear Balbi,


On 2016/8/16 18:43, Felipe Balbi wrote:
> Hi,
>
> "William.wu" <William.wu@rock-chips.com> writes:
>>> William Wu <william.wu@rock-chips.com> writes:
>>>> Add rockchip specific glue layer to support USB3 Peripheral mode
>>>> and Host mode on rockchip platforms (e.g. rk3399).
>>>>
>>>> The DesignWare USB3 integrated in rockchip SoCs is a configurable
>>>> IP Core which can be instantiated as Dual-Role Device (DRD), Host
>>>> Only (XHCI) and Peripheral Only configurations.
>>>>
>>>> We use extcon notifier to manage usb cable detection and mode switch.
>>>> Enable DWC3 PM runtime auto suspend to allow core enter runtime_suspend
>>>> if USB cable is dettached. For host mode, it requires to keep whole
>>>> DWC3 controller in reset state to hold pipe power state in P2 before
>>>> initializing PHY. And it need to reconfigure USB PHY interface of DWC3
>>>> core after deassert DWC3 controller reset.
>>>>
>>>> The current driver supports Host only and Peripheral Only well, for
>>>> now, we will add support for OTG after we have it all stabilized.
>>>>
>>>> Signed-off-by: William Wu <william.wu@rock-chips.com>
>>>> ---
>>>> Changes in v10:
>>>> - fix building error
>>>>
>>>> Changes in v9:
>>>> - Add a new dwc3-rockchip.c driver for rk3399, rather than use dwc3-of-simple.c
>>>>
>>>>    drivers/usb/dwc3/Kconfig         |   9 +
>>>>    drivers/usb/dwc3/Makefile        |   1 +
>>>>    drivers/usb/dwc3/core.c          |   2 +-
>>>>    drivers/usb/dwc3/core.h          |   1 +
>>>>    drivers/usb/dwc3/dwc3-rockchip.c | 441 +++++++++++++++++++++++++++++++++++++++
>>> William, if you need to touch core dwc3 to introduce a glue layer,
>>> you're doing it wrong.
>> Sorry， I realized that it's not better to touch core dwc3 in a specific
>> glue layer.
>> I touched dwc3 in glue layer, because I want to support dual-role mode， and
>> according to our dwc3 IP and usb3 PHY IP design， it need to reinit dwc3
>> core
>> whenever  usb cable attached.
>>
>> Anyway, it's wrong to do that.:-[
>>
>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>> index e887b38..3da6215 100644
>>>> --- a/drivers/usb/dwc3/core.c
>>>> +++ b/drivers/usb/dwc3/core.c
>>>> @@ -405,7 +405,7 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc)
>>>>     * initialized. The PHY interfaces and the PHYs get initialized together with
>>>>     * the core in dwc3_core_init.
>>>>     */
>>>> -static int dwc3_phy_setup(struct dwc3 *dwc)
>>>> +int dwc3_phy_setup(struct dwc3 *dwc)
>>> there's no way I'll let this slip through the cracks :-)
>> Why I need  dwc3_phy_setup in  glue layer is because usb3 IP design
>> in rockchip platform. If dwc3 works on host mode, it requires to put
>> dwc3 controller in reset state before usb3 phy initializing，and after
>> deassert reset,  we need to reconfigure UTMI+ PHY interface because
>> our dwc3 core can't configure PHY interface correctly.
>>
>> Thank you for give me a chance to explain it.:-)
>>
>>>> diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
>>>> new file mode 100644
>>>> index 0000000..eeae1a9
>>>> --- /dev/null
>>>> +++ b/drivers/usb/dwc3/dwc3-rockchip.c
>>>> @@ -0,0 +1,441 @@
>>> [...]
>>>
>>>> +static int dwc3_rockchip_probe(struct platform_device *pdev)
>>>> +{
>>>> +	struct dwc3_rockchip	*rockchip;
>>>> +	struct device		*dev = &pdev->dev;
>>>> +	struct device_node	*np = dev->of_node, *child;
>>>> +	struct platform_device	*child_pdev;
>>>> +
>>>> +	unsigned int		count;
>>>> +	int			ret;
>>>> +	int			i;
>>>> +
>>>> +	rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL);
>>>> +
>>>> +	if (!rockchip)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	count = of_clk_get_parent_count(np);
>>>> +	if (!count)
>>>> +		return -ENOENT;
>>>> +
>>>> +	rockchip->num_clocks = count;
>>>> +
>>>> +	rockchip->clks = devm_kcalloc(dev, rockchip->num_clocks,
>>>> +				      sizeof(struct clk *), GFP_KERNEL);
>>>> +	if (!rockchip->clks)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	platform_set_drvdata(pdev, rockchip);
>>>> +
>>>> +	rockchip->dev = dev;
>>>> +	rockchip->edev = NULL;
>>>> +
>>>> +	pm_runtime_set_active(dev);
>>>> +	pm_runtime_enable(dev);
>>>> +	ret = pm_runtime_get_sync(dev);
>>>> +	if (ret < 0) {
>>>> +		dev_err(dev, "get_sync failed with err %d\n", ret);
>>>> +		goto err1;
>>>> +	}
>>>> +
>>>> +	for (i = 0; i < rockchip->num_clocks; i++) {
>>>> +		struct clk	*clk;
>>>> +
>>>> +		clk = of_clk_get(np, i);
>>>> +		if (IS_ERR(clk)) {
>>>> +			while (--i >= 0)
>>>> +				clk_put(rockchip->clks[i]);
>>>> +			ret = PTR_ERR(clk);
>>>> +
>>>> +			goto err1;
>>>> +		}
>>>> +
>>>> +		ret = clk_prepare_enable(clk);
>>>> +		if (ret < 0) {
>>>> +			while (--i >= 0) {
>>>> +				clk_disable_unprepare(rockchip->clks[i]);
>>>> +				clk_put(rockchip->clks[i]);
>>>> +			}
>>>> +			clk_put(clk);
>>>> +
>>>> +			goto err1;
>>>> +		}
>>>> +
>>>> +		rockchip->clks[i] = clk;
>>>> +	}
>>>> +
>>>> +	rockchip->otg_rst = devm_reset_control_get(dev, "usb3-otg");
>>>> +	if (IS_ERR(rockchip->otg_rst)) {
>>>> +		dev_err(dev, "could not get reset controller\n");
>>>> +		ret = PTR_ERR(rockchip->otg_rst);
>>>> +		goto err2;
>>>> +	}
>>>> +
>>>> +	ret = dwc3_rockchip_extcon_register(rockchip);
>>>> +	if (ret < 0)
>>>> +		goto err2;
>>>> +
>>>> +	child = of_get_child_by_name(np, "dwc3");
>>>> +	if (!child) {
>>>> +		dev_err(dev, "failed to find dwc3 core node\n");
>>>> +		ret = -ENODEV;
>>>> +		goto err3;
>>>> +	}
>>>> +
>>>> +	/* Allocate and initialize the core */
>>>> +	ret = of_platform_populate(np, NULL, NULL, dev);
>>>> +	if (ret) {
>>>> +		dev_err(dev, "failed to create dwc3 core\n");
>>>> +		goto err3;
>>>> +	}
>>>> +
>>>> +	child_pdev = of_find_device_by_node(child);
>>>> +	if (!child_pdev) {
>>>> +		dev_err(dev, "failed to find dwc3 core device\n");
>>>> +		ret = -ENODEV;
>>>> +		goto err4;
>>>> +	}
>>>> +
>>>> +	rockchip->dwc = platform_get_drvdata(child_pdev);
>>> No! You will *NOT* the core struct device. Don't even try to come up
>>> with tricks like this.
>>>
>>> Let's do this: introduce a glue layer that gets peripheral-only
>>> working. Remove PM for now, too. Start with something simple, get the
>>> bare minimum working upstream and add more stuff as you go.
>>>
>>> Trying to do everything in one patch just makes it much more likely for
>>> your patch to be NAKed. What you're doing here is bypassing all the
>>> layering we've built. That won't work very well. The only thing you'll
>>> get is for your patches to continue to be NAKed.
>>>
>>> Avoid the tricks and abuses. Just because you _can_ do it somehow, it
>>> doesn't mean you _should_ do it :-)
>>>
>>> Your best option right now, is to remove PM and dual-role support and a
>>> minimal glue layer supported.
>>>
>>> In fact, all you *really* need is to add a compatible to
>>> dwc3-of-simple.c. That should be enough to get your dwc3 working. Don't
>>> do anything more than that. For dual-role and PM, we can add it
>>> generically to dwc3-of-simple.c when all pieces fall into place.
>>>
>> Ah, thanks very much for your kind explanation. I think I just only need
>> to add a compatible to dwc3-of-simple.c，for now， and I have tested
>> my dwc3， it worked well on peripheral only mode and host only mode
>> without PM. Further, if dwc3-of-simple.c adds generic handling of dual-role
>> and PM, I can improve our dwc3 feature.:-)
> that's my point exactly. We can add more support generically so that
> other platforms can benefit from the work. PM should be simple for
> dwc3-of-simple.c. Dual-role will take a little more effort. In almost
> there actually. There are a few missing pieces but it should be doable
> (hopefully) within the next two major releases.
>
> Your integration is no different than other companies' using DWC3 in
> dual-role setup. For example TI's DWC3 have the same requirements as you
> do, so it makes sense to add it straight to dwc3-core. Roger Quadros
> (now in Cc) has been working on dual-role for TI's platforms and we've
> been discussing about how to add missing pieces generically. Perhaps
> you'd want to join the discussion.
Thanks！ I'll upload a new patch later. And I have seen the dual-role patch
uploaded by Roger Quadros, it's helpful for me. I'm interested in the 
patch，
but I need to understand the patch first， hope to be able to join the 
discussion.:-)



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v10 5/5] usb: dwc3: add rockchip specific glue layer

[Felipe Balbi]

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


Hi,

"William.wu" <William.wu@rock-chips.com> writes:
>> "William.wu" <William.wu@rock-chips.com> writes:
>>>> William Wu <william.wu@rock-chips.com> writes:
>>>>> Add rockchip specific glue layer to support USB3 Peripheral mode
>>>>> and Host mode on rockchip platforms (e.g. rk3399).
>>>>>
>>>>> The DesignWare USB3 integrated in rockchip SoCs is a configurable
>>>>> IP Core which can be instantiated as Dual-Role Device (DRD), Host
>>>>> Only (XHCI) and Peripheral Only configurations.
>>>>>
>>>>> We use extcon notifier to manage usb cable detection and mode switch.
>>>>> Enable DWC3 PM runtime auto suspend to allow core enter runtime_suspend
>>>>> if USB cable is dettached. For host mode, it requires to keep whole
>>>>> DWC3 controller in reset state to hold pipe power state in P2 before
>>>>> initializing PHY. And it need to reconfigure USB PHY interface of DWC3
>>>>> core after deassert DWC3 controller reset.
>>>>>
>>>>> The current driver supports Host only and Peripheral Only well, for
>>>>> now, we will add support for OTG after we have it all stabilized.
>>>>>
>>>>> Signed-off-by: William Wu <william.wu@rock-chips.com>
>>>>> ---
>>>>> Changes in v10:
>>>>> - fix building error
>>>>>
>>>>> Changes in v9:
>>>>> - Add a new dwc3-rockchip.c driver for rk3399, rather than use dwc3-of-simple.c
>>>>>
>>>>>    drivers/usb/dwc3/Kconfig         |   9 +
>>>>>    drivers/usb/dwc3/Makefile        |   1 +
>>>>>    drivers/usb/dwc3/core.c          |   2 +-
>>>>>    drivers/usb/dwc3/core.h          |   1 +
>>>>>    drivers/usb/dwc3/dwc3-rockchip.c | 441 +++++++++++++++++++++++++++++++++++++++
>>>> William, if you need to touch core dwc3 to introduce a glue layer,
>>>> you're doing it wrong.
>>> Sorry， I realized that it's not better to touch core dwc3 in a specific
>>> glue layer.
>>> I touched dwc3 in glue layer, because I want to support dual-role mode， and
>>> according to our dwc3 IP and usb3 PHY IP design， it need to reinit dwc3
>>> core
>>> whenever  usb cable attached.
>>>
>>> Anyway, it's wrong to do that.:-[
>>>
>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>>> index e887b38..3da6215 100644
>>>>> --- a/drivers/usb/dwc3/core.c
>>>>> +++ b/drivers/usb/dwc3/core.c
>>>>> @@ -405,7 +405,7 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc)
>>>>>     * initialized. The PHY interfaces and the PHYs get initialized together with
>>>>>     * the core in dwc3_core_init.
>>>>>     */
>>>>> -static int dwc3_phy_setup(struct dwc3 *dwc)
>>>>> +int dwc3_phy_setup(struct dwc3 *dwc)
>>>> there's no way I'll let this slip through the cracks :-)
>>> Why I need  dwc3_phy_setup in  glue layer is because usb3 IP design
>>> in rockchip platform. If dwc3 works on host mode, it requires to put
>>> dwc3 controller in reset state before usb3 phy initializing，and after
>>> deassert reset,  we need to reconfigure UTMI+ PHY interface because
>>> our dwc3 core can't configure PHY interface correctly.
>>>
>>> Thank you for give me a chance to explain it.:-)
>>>
>>>>> diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
>>>>> new file mode 100644
>>>>> index 0000000..eeae1a9
>>>>> --- /dev/null
>>>>> +++ b/drivers/usb/dwc3/dwc3-rockchip.c
>>>>> @@ -0,0 +1,441 @@
>>>> [...]
>>>>
>>>>> +static int dwc3_rockchip_probe(struct platform_device *pdev)
>>>>> +{
>>>>> +	struct dwc3_rockchip	*rockchip;
>>>>> +	struct device		*dev = &pdev->dev;
>>>>> +	struct device_node	*np = dev->of_node, *child;
>>>>> +	struct platform_device	*child_pdev;
>>>>> +
>>>>> +	unsigned int		count;
>>>>> +	int			ret;
>>>>> +	int			i;
>>>>> +
>>>>> +	rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL);
>>>>> +
>>>>> +	if (!rockchip)
>>>>> +		return -ENOMEM;
>>>>> +
>>>>> +	count = of_clk_get_parent_count(np);
>>>>> +	if (!count)
>>>>> +		return -ENOENT;
>>>>> +
>>>>> +	rockchip->num_clocks = count;
>>>>> +
>>>>> +	rockchip->clks = devm_kcalloc(dev, rockchip->num_clocks,
>>>>> +				      sizeof(struct clk *), GFP_KERNEL);
>>>>> +	if (!rockchip->clks)
>>>>> +		return -ENOMEM;
>>>>> +
>>>>> +	platform_set_drvdata(pdev, rockchip);
>>>>> +
>>>>> +	rockchip->dev = dev;
>>>>> +	rockchip->edev = NULL;
>>>>> +
>>>>> +	pm_runtime_set_active(dev);
>>>>> +	pm_runtime_enable(dev);
>>>>> +	ret = pm_runtime_get_sync(dev);
>>>>> +	if (ret < 0) {
>>>>> +		dev_err(dev, "get_sync failed with err %d\n", ret);
>>>>> +		goto err1;
>>>>> +	}
>>>>> +
>>>>> +	for (i = 0; i < rockchip->num_clocks; i++) {
>>>>> +		struct clk	*clk;
>>>>> +
>>>>> +		clk = of_clk_get(np, i);
>>>>> +		if (IS_ERR(clk)) {
>>>>> +			while (--i >= 0)
>>>>> +				clk_put(rockchip->clks[i]);
>>>>> +			ret = PTR_ERR(clk);
>>>>> +
>>>>> +			goto err1;
>>>>> +		}
>>>>> +
>>>>> +		ret = clk_prepare_enable(clk);
>>>>> +		if (ret < 0) {
>>>>> +			while (--i >= 0) {
>>>>> +				clk_disable_unprepare(rockchip->clks[i]);
>>>>> +				clk_put(rockchip->clks[i]);
>>>>> +			}
>>>>> +			clk_put(clk);
>>>>> +
>>>>> +			goto err1;
>>>>> +		}
>>>>> +
>>>>> +		rockchip->clks[i] = clk;
>>>>> +	}
>>>>> +
>>>>> +	rockchip->otg_rst = devm_reset_control_get(dev, "usb3-otg");
>>>>> +	if (IS_ERR(rockchip->otg_rst)) {
>>>>> +		dev_err(dev, "could not get reset controller\n");
>>>>> +		ret = PTR_ERR(rockchip->otg_rst);
>>>>> +		goto err2;
>>>>> +	}
>>>>> +
>>>>> +	ret = dwc3_rockchip_extcon_register(rockchip);
>>>>> +	if (ret < 0)
>>>>> +		goto err2;
>>>>> +
>>>>> +	child = of_get_child_by_name(np, "dwc3");
>>>>> +	if (!child) {
>>>>> +		dev_err(dev, "failed to find dwc3 core node\n");
>>>>> +		ret = -ENODEV;
>>>>> +		goto err3;
>>>>> +	}
>>>>> +
>>>>> +	/* Allocate and initialize the core */
>>>>> +	ret = of_platform_populate(np, NULL, NULL, dev);
>>>>> +	if (ret) {
>>>>> +		dev_err(dev, "failed to create dwc3 core\n");
>>>>> +		goto err3;
>>>>> +	}
>>>>> +
>>>>> +	child_pdev = of_find_device_by_node(child);
>>>>> +	if (!child_pdev) {
>>>>> +		dev_err(dev, "failed to find dwc3 core device\n");
>>>>> +		ret = -ENODEV;
>>>>> +		goto err4;
>>>>> +	}
>>>>> +
>>>>> +	rockchip->dwc = platform_get_drvdata(child_pdev);
>>>> No! You will *NOT* the core struct device. Don't even try to come up
>>>> with tricks like this.
>>>>
>>>> Let's do this: introduce a glue layer that gets peripheral-only
>>>> working. Remove PM for now, too. Start with something simple, get the
>>>> bare minimum working upstream and add more stuff as you go.
>>>>
>>>> Trying to do everything in one patch just makes it much more likely for
>>>> your patch to be NAKed. What you're doing here is bypassing all the
>>>> layering we've built. That won't work very well. The only thing you'll
>>>> get is for your patches to continue to be NAKed.
>>>>
>>>> Avoid the tricks and abuses. Just because you _can_ do it somehow, it
>>>> doesn't mean you _should_ do it :-)
>>>>
>>>> Your best option right now, is to remove PM and dual-role support and a
>>>> minimal glue layer supported.
>>>>
>>>> In fact, all you *really* need is to add a compatible to
>>>> dwc3-of-simple.c. That should be enough to get your dwc3 working. Don't
>>>> do anything more than that. For dual-role and PM, we can add it
>>>> generically to dwc3-of-simple.c when all pieces fall into place.
>>>>
>>> Ah, thanks very much for your kind explanation. I think I just only need
>>> to add a compatible to dwc3-of-simple.c，for now， and I have tested
>>> my dwc3， it worked well on peripheral only mode and host only mode
>>> without PM. Further, if dwc3-of-simple.c adds generic handling of dual-role
>>> and PM, I can improve our dwc3 feature.:-)
>> that's my point exactly. We can add more support generically so that
>> other platforms can benefit from the work. PM should be simple for
>> dwc3-of-simple.c. Dual-role will take a little more effort. In almost
>> there actually. There are a few missing pieces but it should be doable
>> (hopefully) within the next two major releases.
>>
>> Your integration is no different than other companies' using DWC3 in
>> dual-role setup. For example TI's DWC3 have the same requirements as you
>> do, so it makes sense to add it straight to dwc3-core. Roger Quadros
>> (now in Cc) has been working on dual-role for TI's platforms and we've
>> been discussing about how to add missing pieces generically. Perhaps
>> you'd want to join the discussion.
> Thanks！ I'll upload a new patch later. And I have seen the dual-role patch
> uploaded by Roger Quadros, it's helpful for me. I'm interested in the 
> patch，
> but I need to understand the patch first， hope to be able to join the 
> discussion.:-)

cool, thanks. More users means we're more likely to make a trully
generic layer. We're discussing some simplification of that layer,
however, so that it doesn't take as much code to get DRD working.

-- 
balbi

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