On 22.08.25 01:25, Thinh Nguyen wrote:

On Thu, Aug 21, 2025, Sven Peter wrote:

quoted

As mad as it sounds, the dwc3 controller present on the Apple M1 must be
reset and reinitialized whenever a device is unplugged from the root
port or when the PHY mode is changed.

This is required for at least the following 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 because the eUSB2 repeater
     and the PHY/dwc3 block disagree about the current state.

   - It's possible to completely break the dwc3 controller by switching
     it to device mode and unplugging the cable at just the wrong time.
     If this happens dwc3 behaves as if no device is connected.
     CORESOFTRESET will also never clear after it has been set. The only
     workaround is to trigger a hard reset of the entire dwc3 core with
     its external reset line.

   - Whenever the PHY mode is changed (to e.g. transition to DisplayPort
     alternate mode or USB4) dwc3 has to be shutdown and reinitialized.
     Otherwise the Type-C port will not be usable until the entire SoC
     has been reset.

All of this can be easily worked around by respecting transitions to
USB_ROLE_NONE and making sure the external reset line is asserted when
switching roles. We additionally have to ensure that the PHY is
suspended during init.

Signed-off-by: Sven Peter <sven@kernel.org>
---
  drivers/usb/dwc3/core.c | 61 +++++++++++++++++++++++++++++++++++++++++++++---

[...]

quoted

+			dwc3_core_exit(dwc);
+		}
+
+		if (desired_dr_role) {
+			ret = dwc3_core_init_for_resume(dwc);

The dwc3_core_init_for_resume() is for PM, reusing this with its
current name is confusing.

Ack, I was going to clean this up later and wanted to get feedback on 
this entire approach first. Won't be used anymore when moving to a 
glue.h based approach anyway.

quoted

+			if (ret) {
+				dev_err(dwc->dev,
+				    "failed to reinitialize core\n");
+				goto out;
+			}
+		} else {
+			goto out;
+		}
+	}
+
  	/*
  	 * When current_dr_role is not set, there's no role switching.
  	 * Only perform GCTL.CoreSoftReset when there's DRD role switching.
  	 */
-	if (dwc->current_dr_role && ((DWC3_IP_IS(DWC3) ||
+	if (dwc->role_switch_reset_quirk ||

Don't override the use of GCTL.CoreSoftReset with this quirk. Not all
controller versions should use GCTL.CoreSoftReset, the new controller
version don't even have it. What version is this vendor using?

I'm concern how this condition is needed...

This is actually a leftover from the first attempts at making this work. 
I didn't know about the external reset line back then and had to 
soft-reset it here because it would not see new devices otherwise IIRC.
Since we're going through a hard-reset now anyway this can be dropped
and this entire commit will disappear in favor of a glue.h based driver 
anyway.

quoted

+		(dwc->current_dr_role && ((DWC3_IP_IS(DWC3) ||
  			DWC3_VER_IS_PRIOR(DWC31, 190A)) &&
-			desired_dr_role != DWC3_GCTL_PRTCAP_OTG)) {
+			desired_dr_role != DWC3_GCTL_PRTCAP_OTG))) {
  		reg = dwc3_readl(dwc->regs, DWC3_GCTL);
  		reg |= DWC3_GCTL_CORESOFTRESET;
  		dwc3_writel(dwc->regs, DWC3_GCTL, reg);

@@ -1372,6 +1394,9 @@ static int dwc3_core_init(struct dwc3 *dwc)
  	if (ret)
  		goto err_exit_phy;
  
+	if (dwc->role_switch_reset_quirk)
+		dwc3_enable_susphy(dwc, true);
+

Why do you need to enable susphy here?

The only place we actually need it is when we shut down the Type-C PHY 
due some what I assume is some hardware quirk, i.e. just before 
dwc3_core_exit.

The PHY will otherwise not be able to acquire some hardware lock (which 
they call PIPEHANDLER lock in debug strings) to switch from e.g. USB3 
PHY to a dummy PHY for USB2 only. It then can't shut down cleanly 
anymore and will get stuck in a weird state where the port refuses to 
work until I reset everything.
Originally it was added because we just undid some commit where susphy 
handling was made unconditional IIRC.

I'll move this to the glue driver with a comment explaining why it's 
required.

quoted

  	dwc3_core_setup_global_control(dwc);

[...]

quoted

+	if (dev->of_node) {
+		if (of_device_is_compatible(dev->of_node, "apple,t8103-dwc3")) {
+			if (!IS_ENABLED(CONFIG_USB_ROLE_SWITCH) ||
+			    !IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)) {
+				dev_err(dev,
+				    "Apple DWC3 requires role switch support.\n"
+				    );
+				ret = -EINVAL;
+				goto err_put_psy;
+			}
+
+			dwc->dr_mode = USB_DR_MODE_OTG;
+			dwc->role_switch_reset_quirk = true;

Put this in your glue driver or device tree.

Ack.

quoted

+		}
+	}

[...]

quoted

+
+	if (dwc->role_switch_reset_quirk && !dwc->current_dr_role)
+		role = USB_ROLE_NONE;

Don't return USB_ROLE_NONE on role_switch get. The USB_ROLE_NONE is the
default role. The role_switch get() should return exactly which role the
controller is currently in, and the driver can figure that out.

Ack, will also happen from inside the glue driver now.




Sven