dwc3 dependencies for HiKey960 USB

Just wanted to send these out again to try to make some progress on these patches originally by Yu Chen to get HiKey960 dev-board's USB functionality working.

For now I've dropped the hub switching functionality from the series, as I've not yet been able to come up with a solution to Rob's objections. So this set focuses just on the dwc3 changes needed.

The full patchset (including hub switching and dts changes not submitted here) can be found here:
https://git.linaro.org/people/john.stultz/android-dev.git/log/?id=dcb542a207ac5c5f94729601b2390ca5fbf6f6cd

I'd greatly appreciate any feedback or thoughts!

thanks
-john

New in v8:
* Rob objected to the custom hub switching logic in the previous series, so I've dropped it for now.

John Stultz (5):
dt-bindings: usb: generic: Add role-switch-default-mode binding
usb: dwc3: Add support for role-switch-default-mode binding
dt-bindings: usb: dwc3: Allow clock list & resets to be more flexible
usb: dwc3: Rework clock initialization to be more flexible
usb: dwc3: Rework resets initialization to be more flexible

Yu Chen (1):
usb: dwc3: Registering a role switch in the DRD code.

 .../devicetree/bindings/usb/dwc3.txt          |  5 +-
 .../devicetree/bindings/usb/generic.txt       |  6 ++
 drivers/usb/dwc3/core.c                       | 22 ++---
 drivers/usb/dwc3/core.h                       |  6 ++
 drivers/usb/dwc3/drd.c                        | 96 ++++++++++++++++++-
 5 files changed, 116 insertions(+), 19 deletions(-)

2.17.1

From: Yu Chen <[email protected]>

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.

---
v2: Fix role_sw and role_switch_default_mode descriptions as
reported by kbuild test robot <[email protected]>

v3: Split out the role-switch-default-host logic into its own
patch

v5: Drop selecting CONFIG_USB_ROLE_SWITCH & ifdef dependent code

v6: Fix build issue Reported-by: kbuild test robot <[email protected]>

v7: Minor fix for CONFIG_USB_ROLE_SWITCH=m case not building
the role switch handling code, reported by Guillaume Gardet <[email protected]>

---
 drivers/usb/dwc3/core.h |  3 ++
 drivers/usb/dwc3/drd.c  | 77 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 79 insertions(+), 1 deletion(-)

index 77c4a9abe365..a99e57636172 100644
@@ -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>

@@ -953,6 +954,7 @@ 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
  * @usb2_phy: pointer to USB2 PHY
  * @usb3_phy: pointer to USB3 PHY
  * @usb2_generic_phy: pointer to USB2 PHY

@@ -1086,6 +1088,7 @@ struct dwc3 {
 	struct extcon_dev	*edev;
 	struct notifier_block	edev_nb;
 	enum usb_phy_interface	hsphy_mode;
+	struct usb_role_switch	*role_sw;
 
 	u32			fladj;
 	u32			irq_gadget;

2.17.1

@bjorn found an API change that explodes this one.

Fixed here: https://lkml.org/lkml/2020/3/11/1034

Huh. I wonder why I don't see that causing trouble in my testing. :/

Oddly, trying to use your patch results in build failures for me... Is the api change something in -next?

thanks
-john

Ah. Ok, I found it: "usb: roles: Provide the switch drivers handle to the switch in the API"

I'll add that to my patch series, retest and resubmit. Thanks for the heads up! -john

Add binding to configure the default role the controller assumes is host mode when the usb role is USB_ROLE_NONE.

---
v5: Switch to string rather then a bool

---
 Documentation/devicetree/bindings/usb/generic.txt | 6 ++++++
 1 file changed, 6 insertions(+)

index e6790d2a4da9..67c51759a642 100644
@@ -35,6 +35,12 @@ Optional properties:
 			the USB data role (USB host or USB device) for a given
 			USB connector, such as Type-C, Type-B(micro).
 			see connector/usb-connector.txt.
+ - role-switch-default-mode: indicating if usb-role-switch is enabled, the
+			device default operation mode of controller while usb
+			role is USB_ROLE_NONE. Valid arguments are "host" and
+			"peripheral". Defaults to "peripheral" if not
+			specified.
+
 
 This is an attribute to a USB controller such as:
 

2.17.1

Support the new role-switch-default-mode binding for configuring the default role the controller assumes as when the usb role is USB_ROLE_NONE

This patch was split out from a larger patch originally by Yu Chen <[email protected]>

---
v3: Split this patch out from addition of usb-role-switch
handling
v5: Reworked to use string based role-switch-default-mode

---
 drivers/usb/dwc3/core.h |  3 +++
 drivers/usb/dwc3/drd.c  | 25 ++++++++++++++++++++++---
 2 files changed, 25 insertions(+), 3 deletions(-)

index a99e57636172..57d549a1ad0b 100644
@@ -955,6 +955,8 @@ struct dwc3_scratchpad_array {
  *		- 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

@@ -1089,6 +1091,7 @@ struct dwc3 {
 	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;

index 331c6e997f0c..db68d48c2267 100644
@@ -491,7 +491,10 @@ static int dwc3_usb_role_switch_set(struct device *dev, enum usb_role role)
 		mode = DWC3_GCTL_PRTCAP_DEVICE;
 		break;
 	default:
-		mode = DWC3_GCTL_PRTCAP_DEVICE;
+		if (dwc->role_switch_default_mode == USB_DR_MODE_HOST)
+			mode = DWC3_GCTL_PRTCAP_HOST;
+		else
+			mode = DWC3_GCTL_PRTCAP_DEVICE;
 		break;
 	}
 

@@ -517,7 +520,10 @@ static enum usb_role dwc3_usb_role_switch_get(struct device *dev)
 		role = dwc->current_otg_role;
 		break;
 	default:
-		role = USB_ROLE_DEVICE;
+		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);

@@ -527,6 +533,19 @@ static enum usb_role dwc3_usb_role_switch_get(struct device *dev)
 static int dwc3_setup_role_switch(struct dwc3 *dwc)
 {
 	struct usb_role_switch_desc dwc3_role_switch = {NULL};
+	const char *str;
+	u32 mode;
+	int ret;
+
+	ret = device_property_read_string(dwc->dev, "role-switch-default-mode",
+					  &str);
+	if (ret >= 0  && !strncmp(str, "host", strlen("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;

@@ -535,7 +554,7 @@ static int dwc3_setup_role_switch(struct dwc3 *dwc)
 	if (IS_ERR(dwc->role_sw))
 		return PTR_ERR(dwc->role_sw);
 
-	dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
+	dwc3_set_mode(dwc, mode);
 	return 0;
 }
 #else

2.17.1

Rather then adding another device specific binding to support hikey960, Rob Herring suggested we expand the current dwc3 binding to allow for variable numbers of clocks and resets.

---
 Documentation/devicetree/bindings/usb/dwc3.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

index 66780a47ad85..29768b0ca923 100644
@@ -7,7 +7,8 @@ Required properties:
  - compatible: must be "snps,dwc3"
  - reg : Address and length of the register set for the device
  - interrupts: Interrupts used by the dwc3 controller.
- - clock-names: should contain "ref", "bus_early", "suspend"
+ - clock-names: list of clock names. Ideally should be "ref",
+                "bus_early", "suspend" but may be less or more.
  - clocks: list of phandle and clock specifier pairs corresponding to
            entries in the clock-names property.
 

@@ -36,7 +37,7 @@ Optional properties:
  - phys: from the *Generic PHY* bindings
  - phy-names: from the *Generic PHY* bindings; supported names are "usb2-phy"
 	or "usb3-phy".
- - resets: a single pair of phandle and reset specifier
+ - resets: set of phandle and reset specifier pairs
  - snps,usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
  - snps,usb3_lpm_capable: determines if platform is USB3 LPM capable
  - snps,dis-start-transfer-quirk: when set, disable isoc START TRANSFER command

2.17.1

The dwc3 core binding specifies three clocks:
ref, bus_early, and suspend

which are all controlled in the driver together.

However some variants of the hardware my not have all three clks, or some may have more. Usually this was handled by using the dwc3-of-simple glue driver, but that resulted in a proliferation of bindings for for every variant, when the only difference was the clocks and resets lists.

So this patch reworks the reading of the clks from the dts to use devm_clk_bulk_get_all() will will fetch all the clocks specified in the dts together.

This patch was recommended by Rob Herring <[email protected]> as an alternative to creating multiple bindings for each variant of hardware.

---
v3: Rework dwc3 core rather then adding another dwc-of-simple
binding.
v6: Re-introduce this patch, on Rob's suggestion

---
 drivers/usb/dwc3/core.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

index 1d85c42b9c67..ba21af5c1204 100644
@@ -289,12 +289,6 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
 	return 0;
 }
 
-static const struct clk_bulk_data dwc3_core_clks[] = {
-	{ .id = "ref" },
-	{ .id = "bus_early" },
-	{ .id = "suspend" },
-};
-
 /*
  * dwc3_frame_length_adjustment - Adjusts frame length if required
  * @dwc3: Pointer to our controller context structure

@@ -1441,11 +1435,6 @@ static int dwc3_probe(struct platform_device *pdev)
 	if (!dwc)
 		return -ENOMEM;
 
-	dwc->clks = devm_kmemdup(dev, dwc3_core_clks, sizeof(dwc3_core_clks),
-				 GFP_KERNEL);
-	if (!dwc->clks)
-		return -ENOMEM;
-
 	dwc->dev = dev;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

@@ -1481,17 +1470,18 @@ static int dwc3_probe(struct platform_device *pdev)
 		return PTR_ERR(dwc->reset);
 
 	if (dev->of_node) {
-		dwc->num_clks = ARRAY_SIZE(dwc3_core_clks);
-
-		ret = devm_clk_bulk_get(dev, dwc->num_clks, dwc->clks);
+		ret = devm_clk_bulk_get_all(dev, &dwc->clks);
 		if (ret == -EPROBE_DEFER)
 			return ret;
 		/*
 		 * Clocks are optional, but new DT platforms should support all
 		 * clocks as required by the DT-binding.
 		 */
-		if (ret)
+		if (ret < 0)
 			dwc->num_clks = 0;
+		else
+			dwc->num_clks = ret;
+
 	}
 
 	ret = reset_control_deassert(dwc->reset);

2.17.1

The dwc3 core binding specifies one reset.

However some variants of the hardware may have more. Previously this was handled by using the dwc3-of-simple glue driver, but that resulted in a proliferation of bindings for for every variant, when the only difference was the clocks and resets lists.

So this patch reworks the reading of the resets to fetch all the resets specified in the dts together.

This patch was recommended by Rob Herring <[email protected]> as an alternative to creating multiple bindings for each variant of hardware.

---
v3: Rework dwc3 core rather then adding another dwc-of-simple
binding.
v6: Re-introduce this patch, on Rob's suggestion

---
 drivers/usb/dwc3/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index ba21af5c1204..2afcc04da338 100644
@@ -1465,7 +1465,7 @@ static int dwc3_probe(struct platform_device *pdev)
 
 	dwc3_get_properties(dwc);
 
-	dwc->reset = devm_reset_control_get_optional_shared(dev, NULL);
+	dwc->reset = devm_reset_control_array_get(dev, true, true);
 	if (IS_ERR(dwc->reset))
 		return PTR_ERR(dwc->reset);
 

2.17.1