linux-kernel - Re: [PATCH RFC 04/22] usb: dwc3: apple: Reset dwc3 during role switches

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250824131841.GG1270980@robin.jannau.net>
Date: Sun, 24 Aug 2025 15:18:41 +0200
From: Janne Grunau <j@...nau.net>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: Sven Peter <sven@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Felipe Balbi <balbi@...nel.org>,
	Alyssa Rosenzweig <alyssa@...enzweig.io>,
	Neal Gompa <neal@...pa.dev>, Vinod Koul <vkoul@...nel.org>,
	Kishon Vijay Abraham I <kishon@...nel.org>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	Philipp Zabel <p.zabel@...gutronix.de>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"asahi@...ts.linux.dev" <asahi@...ts.linux.dev>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	"linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>
Subject: Re: [PATCH RFC 04/22] usb: dwc3: apple: Reset dwc3 during role
 switches

On Thu, Aug 21, 2025 at 11:25:51PM +0000, Thinh Nguyen wrote:
> On Thu, Aug 21, 2025, Sven Peter wrote:
> > 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@...nel.org>
> > ---
> >  drivers/usb/dwc3/core.c | 61 +++++++++++++++++++++++++++++++++++++++++++++----
> >  drivers/usb/dwc3/core.h |  3 +++
> >  drivers/usb/dwc3/drd.c  | 11 ++++++++-
> >  drivers/usb/dwc3/host.c |  3 ++-
> >  4 files changed, 72 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 8002c23a5a02acb8f3e87b2662a53998a4cf4f5c..26aa507a738f001409a97ef563c6561433a1cac5 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c

...

> >  	/*
> >  	 * 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?

It reports DWC31, revision 190A and the unlisted version_types
0x736f3035 (M1), 0x736f3039 (M1 Pro/Max/Ultra) and 0x736f3130 (M2, M2
Pro/Max/Ultra).

> I'm concern how this condition is needed...
> 
> > +		(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);

Janne