[PATCH v2 1/2] dt-bindings: usb: Add Apple dwc3 bindings

[PATCH v2 1/2] dt-bindings: usb: Add Apple dwc3 bindings

[Sven Peter]

Apple Silicon SoCs such as the M1 have multiple USB controllers based on
the Synopsys DesignWare USB3 controller.
References to the ATC PHY required for SuperSpeed are left out for now
until support has been upstreamed as well.

Signed-off-by: Sven Peter <sven@svenpeter.dev>
---
v1 -> v2:
 - added apple,dwc3 bindings instead of a property for the reset quirk
   as requested by robh

I think I have to use GPL-2.0 for this binding since it's based
on and references snps,dwc3.yaml which is also only GPL-2.0.
Otherwise I'd be fine with the usual GPL/BSD dual license as well.

 .../devicetree/bindings/usb/apple,dwc3.yaml   | 64 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/apple,dwc3.yaml

diff --git a/Documentation/devicetree/bindings/usb/apple,dwc3.yaml b/Documentation/devicetree/bindings/usb/apple,dwc3.yaml
new file mode 100644
index 000000000000..fb3b3489e6b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/apple,dwc3.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/apple,dwc3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Apple Silicon DWC3 USB controller
+
+maintainers:
+  - Sven Peter <sven@svenpeter.dev>
+
+description:
+  On Apple Silicon SoCs such as the M1 each Type-C port has a corresponding
+  USB controller based on the Synopsys DesignWare USB3 controller.
+
+  The common content of this binding is defined in snps,dwc3.yaml.
+
+allOf:
+  - $ref: snps,dwc3.yaml#
+
+select:
+  properties:
+    compatible:
+      contains:
+        const: apple,dwc3
+  required:
+    - compatible
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - apple,t8103-dwc3
+          - apple,t6000-dwc3
+      - const: apple,dwc3
+      - const: snps,dwc3
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/apple-aic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    usb@82280000 {
+      compatible = "apple,t8103-dwc3", "apple,dwc3", "snps,dwc3";
+      reg = <0x82280000 0x10000>;
+      interrupts = <AIC_IRQ 777 IRQ_TYPE_LEVEL_HIGH>;
+
+      dr_mode = "otg";
+      usb-role-switch;
+      role-switch-default-mode = "host";
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 3b79fd441dde..03e7cc48877a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1724,6 +1724,7 @@ T:	git https://github.com/AsahiLinux/linux.git
 F:	Documentation/devicetree/bindings/arm/apple.yaml
 F:	Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
 F:	Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
+F:	Documentation/devicetree/bindings/usb/apple,dwc3.yaml
 F:	arch/arm64/boot/dts/apple/
 F:	drivers/irqchip/irq-apple-aic.c
 F:	include/dt-bindings/interrupt-controller/apple-aic.h
-- 
2.25.1

[PATCH v2 2/2] usb: dwc3: Add role switch reset quirk for Apple DWC3

[Sven Peter]

As mad as it sounds, the dwc3 controller present on Apple SoCs must be
reset and reinitialized whenever a device is unplugged from the root port
and triggers a role switch notification from the USB PD controller.

This is required for at least two reasons:

  - The USB2 D+/D- lines are connected through a stateful eUSB2 repeater
    which in turn is controlled by a variant of the TI TPS6598x USB PD
    chip. When the USB PD controller detects a hotplug event it resets
    the eUSB2 repeater. Afterwards, no new device is recognized before
    the DWC3 core and PHY are reset as well.

  - It's possible to completely break the dwc3 controller by switching
    it to device mode and unplugging the cable at just the wrong time.
    Even a CORESOFTRESET is not enough to allow new devices again.
    The only workaround is to trigger a hard reset of the entire
    dwc3 core. This also happens when running macOS on these
    machines.

Signed-off-by: Sven Peter <sven@svenpeter.dev>
---
v1 -> v2:
 - enable the quirk based on the compatible instead of a property
   as requested by robh

 drivers/usb/dwc3/core.c | 39 ++++++++++++++++++++++++++++++++++++---
 drivers/usb/dwc3/core.h |  6 ++++++
 drivers/usb/dwc3/drd.c  |  7 +++++++
 3 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 0104a80b185e..b76d7536eafe 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -115,6 +115,8 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode)
 }
 
 static int dwc3_core_soft_reset(struct dwc3 *dwc);
+static void dwc3_core_exit(struct dwc3 *dwc);
+static int dwc3_core_init_for_resume(struct dwc3 *dwc);
 
 static void __dwc3_set_mode(struct work_struct *work)
 {
@@ -130,10 +132,11 @@ static void __dwc3_set_mode(struct work_struct *work)
 	if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_OTG)
 		dwc3_otg_update(dwc, 0);
 
-	if (!dwc->desired_dr_role)
+	if (!dwc->desired_dr_role && !dwc->role_switch_reset_quirk)
 		goto out;
 
-	if (dwc->desired_dr_role == dwc->current_dr_role)
+	if (dwc->desired_dr_role == dwc->current_dr_role &&
+			!dwc->role_switch_reset_quirk)
 		goto out;
 
 	if (dwc->desired_dr_role == DWC3_GCTL_PRTCAP_OTG && dwc->edev)
@@ -158,6 +161,34 @@ static void __dwc3_set_mode(struct work_struct *work)
 		break;
 	}
 
+	if (dwc->role_switch_reset_quirk) {
+		if (dwc->current_dr_role) {
+			dwc->current_dr_role = 0;
+			dwc3_core_exit(dwc);
+		}
+
+		if (dwc->desired_dr_role) {
+			/*
+			 * the first call to __dwc3_set_mode comes from
+			 * dwc3_drd_init. In that case dwc3_core_init has been
+			 * called but dwc->current_dr_role is zero such that
+			 * we must not reinitialize the core again here.
+			 */
+			if (dwc->role_switch_reset_quirk_initialized) {
+				ret = dwc3_core_init_for_resume(dwc);
+				if (ret) {
+					dev_err(dwc->dev,
+					    "failed to reinitialize core\n");
+					goto out;
+				}
+			}
+
+			dwc->role_switch_reset_quirk_initialized = 1;
+		} else {
+			goto out;
+		}
+	}
+
 	/* For DRD host or device mode only */
 	if (dwc->desired_dr_role != DWC3_GCTL_PRTCAP_OTG) {
 		reg = dwc3_readl(dwc->regs, DWC3_GCTL);
@@ -1586,6 +1617,8 @@ static int dwc3_probe(struct platform_device *pdev)
 		else
 			dwc->num_clks = ret;
 
+		if (of_device_is_compatible(dev->of_node, "apple,dwc3"))
+			dwc->role_switch_reset_quirk = true;
 	}
 
 	ret = reset_control_deassert(dwc->reset);
@@ -1715,7 +1748,6 @@ static int dwc3_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM
 static int dwc3_core_init_for_resume(struct dwc3 *dwc)
 {
 	int ret;
@@ -1742,6 +1774,7 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
 	return ret;
 }
 
+#ifdef CONFIG_PM
 static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
 {
 	unsigned long	flags;
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 5612bfdf37da..3fc5c5bc4c57 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1080,6 +1080,9 @@ struct dwc3_scratchpad_array {
  *	3	- Reserved
  * @dis_metastability_quirk: set to disable metastability quirk.
  * @dis_split_quirk: set to disable split boundary.
+ * @role_switch_reset_quirk: set to force reinitialization after any role switch
+ * @role_switch_reset_quirk_initialized: set to true after the first role switch
+ *			which is triggered from dwc3_drd_init directly
  * @imod_interval: set the interrupt moderation interval in 250ns
  *			increments or 0 to disable.
  * @max_cfg_eps: current max number of IN eps used across all USB configs.
@@ -1291,6 +1294,9 @@ struct dwc3 {
 	unsigned		dis_split_quirk:1;
 	unsigned		async_callbacks:1;
 
+	unsigned		role_switch_reset_quirk:1;
+	unsigned		role_switch_reset_quirk_initialized:1;
+
 	u16			imod_interval;
 
 	int			max_cfg_eps;
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index d7f76835137f..403e88a72f0e 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -506,6 +506,9 @@ static int dwc3_usb_role_switch_set(struct usb_role_switch *sw,
 		break;
 	}
 
+	if (dwc->role_switch_reset_quirk && role == USB_ROLE_NONE)
+		mode = 0;
+
 	dwc3_set_mode(dwc, mode);
 	return 0;
 }
@@ -534,6 +537,10 @@ static enum usb_role dwc3_usb_role_switch_get(struct usb_role_switch *sw)
 			role = USB_ROLE_DEVICE;
 		break;
 	}
+
+	if (dwc->role_switch_reset_quirk && !dwc->current_dr_role)
+		role = USB_ROLE_NONE;
+
 	spin_unlock_irqrestore(&dwc->lock, flags);
 	return role;
 }
-- 
2.25.1

Re: [PATCH v2 2/2] usb: dwc3: Add role switch reset quirk for Apple DWC3

[Janne Grunau]

On 2021-11-08 18:09:46 +0100, Sven Peter wrote:
> As mad as it sounds, the dwc3 controller present on Apple SoCs must be
> reset and reinitialized whenever a device is unplugged from the root port
> and triggers a role switch notification from the USB PD controller.
> 
> This is required for at least two reasons:
> 
>   - The USB2 D+/D- lines are connected through a stateful eUSB2 repeater
>     which in turn is controlled by a variant of the TI TPS6598x USB PD
>     chip. When the USB PD controller detects a hotplug event it resets
>     the eUSB2 repeater. Afterwards, no new device is recognized before
>     the DWC3 core and PHY are reset as well.
> 
>   - It's possible to completely break the dwc3 controller by switching
>     it to device mode and unplugging the cable at just the wrong time.
>     Even a CORESOFTRESET is not enough to allow new devices again.
>     The only workaround is to trigger a hard reset of the entire
>     dwc3 core. This also happens when running macOS on these
>     machines.

This patch is necessary and works on M1 Max (Macbook Pro 14" 2021).  
Tested with compatible = "apple,t6000,dwc3", "apple,dwc3", "snps,dwc3";.

Feel free to add

Tested-by: Janne Grunau <j@jannau.net>

Janne

Re: [PATCH v2 1/2] dt-bindings: usb: Add Apple dwc3 bindings

[Rob Herring]

On Mon, Nov 08, 2021 at 06:09:45PM +0100, Sven Peter wrote:
> Apple Silicon SoCs such as the M1 have multiple USB controllers based on
> the Synopsys DesignWare USB3 controller.
> References to the ATC PHY required for SuperSpeed are left out for now
> until support has been upstreamed as well.
> 
> Signed-off-by: Sven Peter <sven@svenpeter.dev>
> ---
> v1 -> v2:
>  - added apple,dwc3 bindings instead of a property for the reset quirk
>    as requested by robh
> 
> I think I have to use GPL-2.0 for this binding since it's based
> on and references snps,dwc3.yaml which is also only GPL-2.0.
> Otherwise I'd be fine with the usual GPL/BSD dual license as well.
> 
>  .../devicetree/bindings/usb/apple,dwc3.yaml   | 64 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/apple,dwc3.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/apple,dwc3.yaml b/Documentation/devicetree/bindings/usb/apple,dwc3.yaml
> new file mode 100644
> index 000000000000..fb3b3489e6b2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/apple,dwc3.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license please.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/apple,dwc3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple Silicon DWC3 USB controller
> +
> +maintainers:
> +  - Sven Peter <sven@svenpeter.dev>
> +
> +description:
> +  On Apple Silicon SoCs such as the M1 each Type-C port has a corresponding
> +  USB controller based on the Synopsys DesignWare USB3 controller.
> +
> +  The common content of this binding is defined in snps,dwc3.yaml.
> +
> +allOf:
> +  - $ref: snps,dwc3.yaml#
> +
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        const: apple,dwc3

This needs to list all possible compatibles except snps,dwc3 so the 
schema is applied for any incorrect mixture of compatibles.

> +  required:
> +    - compatible
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - apple,t8103-dwc3
> +          - apple,t6000-dwc3
> +      - const: apple,dwc3
> +      - const: snps,dwc3
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/apple-aic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    usb@82280000 {
> +      compatible = "apple,t8103-dwc3", "apple,dwc3", "snps,dwc3";
> +      reg = <0x82280000 0x10000>;
> +      interrupts = <AIC_IRQ 777 IRQ_TYPE_LEVEL_HIGH>;
> +
> +      dr_mode = "otg";
> +      usb-role-switch;
> +      role-switch-default-mode = "host";
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3b79fd441dde..03e7cc48877a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1724,6 +1724,7 @@ T:	git https://github.com/AsahiLinux/linux.git
>  F:	Documentation/devicetree/bindings/arm/apple.yaml
>  F:	Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
>  F:	Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> +F:	Documentation/devicetree/bindings/usb/apple,dwc3.yaml
>  F:	arch/arm64/boot/dts/apple/
>  F:	drivers/irqchip/irq-apple-aic.c
>  F:	include/dt-bindings/interrupt-controller/apple-aic.h
> -- 
> 2.25.1
> 
> 

Re: [PATCH v2 1/2] dt-bindings: usb: Add Apple dwc3 bindings

[Sven Peter]

Hi,

Thanks for the review!

On Mon, Nov 29, 2021, at 02:27, Rob Herring wrote:
> On Mon, Nov 08, 2021 at 06:09:45PM +0100, Sven Peter wrote:
>> Apple Silicon SoCs such as the M1 have multiple USB controllers based on
>> the Synopsys DesignWare USB3 controller.
>> References to the ATC PHY required for SuperSpeed are left out for now
>> until support has been upstreamed as well.
>> 
>> Signed-off-by: Sven Peter <sven@svenpeter.dev>
>> ---
>> v1 -> v2:
>>  - added apple,dwc3 bindings instead of a property for the reset quirk
>>    as requested by robh
>> 
>> I think I have to use GPL-2.0 for this binding since it's based
>> on and references snps,dwc3.yaml which is also only GPL-2.0.
>> Otherwise I'd be fine with the usual GPL/BSD dual license as well.
>> 
>>  .../devicetree/bindings/usb/apple,dwc3.yaml   | 64 +++++++++++++++++++
>>  MAINTAINERS                                   |  1 +
>>  2 files changed, 65 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/usb/apple,dwc3.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/usb/apple,dwc3.yaml b/Documentation/devicetree/bindings/usb/apple,dwc3.yaml
>> new file mode 100644
>> index 000000000000..fb3b3489e6b2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/apple,dwc3.yaml
>> @@ -0,0 +1,64 @@
>> +# SPDX-License-Identifier: GPL-2.0
>
> Dual license please.

I'd like to but I'm not sure if I can do that. This binding is based on
snps,dwc3.yaml and rockchip,dwc3.yaml which are both only GPL-2.0.

>
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/usb/apple,dwc3.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Apple Silicon DWC3 USB controller
>> +
>> +maintainers:
>> +  - Sven Peter <sven@svenpeter.dev>
>> +
>> +description:
>> +  On Apple Silicon SoCs such as the M1 each Type-C port has a corresponding
>> +  USB controller based on the Synopsys DesignWare USB3 controller.
>> +
>> +  The common content of this binding is defined in snps,dwc3.yaml.
>> +
>> +allOf:
>> +  - $ref: snps,dwc3.yaml#
>> +
>> +select:
>> +  properties:
>> +    compatible:
>> +      contains:
>> +        const: apple,dwc3
>
> This needs to list all possible compatibles except snps,dwc3 so the 
> schema is applied for any incorrect mixture of compatibles.

Makes sense, will do that for the next version.



Thanks,

Sven