Message-ID: <41bb000c-5643-4ed2-8d33-a6bd8a549409@kernel.org>
Date: Sun, 21 Sep 2025 15:40:28 +0200
From: Sven Peter <sven@...nel.org>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: 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>,
 Janne Grunau <j@...nau.net>, 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>, Frank Li <Frank.Li@....com>,
 Ran Wang <ran.wang_1@....com>, Peter Chen <peter.chen@....com>,
 "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 v2 04/22] usb: dwc3: Add Apple Silicon DWC3 glue layer
 driver

Hi,

On 20.09.25 00:40, Thinh Nguyen wrote:
> On Sat, Sep 06, 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.

[....]

>> +/**
>> + * struct dwc3_apple - Apple-specific DWC3 USB controller
>> + * @dwc: Core DWC3 structure
>> + * @dev: Pointer to the device structure
>> + * @mmio_resource: Resource to be passed to dwc3_core_probe
>> + * @apple_regs: Apple-specific DWC3 registers
>> + * @resets: Reset control
>> + * @role_sw: USB role switch
>> + * @lock: Mutex for synchronizing access
>> + * @core_probe_done: True if dwc3_core_probe was already called after the first plug
>> + * @mode: Current mode of the controller (off/host/device)
> 
> For this platform, current mode of the controller should only ever be
> host or device mode. Seems we're mixing power with usb role? ie. what
> DWC3_APPLE_OFF is being used for?
> 

So this platform is very messed up and in order the bring up dwc3 and 
the PHY there are four steps:

1) The PHY itself has to be brought up; for this we need to know the 
mode (USB3, USB3+DisplayPort, USB4, etc) and the lane orientation. This 
happens from typec_mux_set
2) DWC3 has to be brought up but we must not touch the gadget area or 
start xhci yet
3) The PHY bring-up has to be finalized and dwc3's PIPE interface has to 
be switched to the USB3 PHY, this is done inside phy_set_mode.
4) We can now initialize xhci or gadget mode.

I think we can switch 1 and 2 but 3 has to happen after (1 and 2) and 4 
has to happen after 3.

And then to bring this all down again:

1) DWC3 has to exit host or gadget mode and must no longer touch those 
registers
2) The PHY has to switch dwc3's PIPE interface back to the dummy backend
3) The PHY itself can be shut down, this happens from typec_mux_set

We also can't transition the PHY from one mode to another while dwc3 is 
up and running (this is slightly wrong, some transitions are possible, 
others aren't but because we have no documentation for this I'd rather 
play it safe).

After both the PHY and dwc3 are initialized we will also only ever see a 
single "new device connected" event. If we just keep them running only 
the first device plugged in will ever work. XHCI's port status register 
actually does show the correct state but no interrupt ever comes in. In 
gadget mode we don't even get a USBDisconnected event and everything 
looks like there's still something connected on the other end.


And to make this all extra fun: If we get the order of some of this 
wrong either the port is just broken until a phy+dwc3 reset, or it's 
broken until a full SoC reset (likely because we can't reset some parts 
of the PHY), or some watchdog kicks in after a few seconds and forces a 
full SoC reset (I've mostly seen this with USB4/Thunderbolt but there's 
clearly some watchdog that hates invalid states).


So there's really no good way to keep dwc3 fully up and running after we 
disconnect a cable because then we can't shut down the PHY anymore. And 
if we kept the PHY running in whatever mode until the next cable is 
connected we'd need to tear it all down and bring it back up again 
anyway to detect and use the next device.


Instead, I just shutdown everything once a cable is disconnected and 
that's this DWC3_APPLE_OFF state. Maybe I can put the explanation above 
as a comment in there and maybe also rename "mode" to "state" here 
because we may get something like DWC3_APPLE_USB4_TUNNEL in the future 
here as well because the sequence might be a bit different there.



>> + */
>> +struct dwc3_apple {

[...]

>> +	/*
>> +	 * Note that we only bring up dwc3 once the first device is attached because we need to know
>> +	 * the role (e.g. host), mode (e.g. USB3) and lane orientation to bring up the PHY which is
>> +	 * tightly coupled to dwc3.
>> +	 */
> 
> The wording here is odd. You can wait for attach to do this, but it
> should not be a requirement. You might not know whether you need to
> switch role, but you should be able to initialize the controller in
> either host or device mode prior to attachment.
> 
> Any particular reason we need to do this? If not, we can do away with
> the core_probe_done condition.

Because I can't really bring up dwc3 fully to any mode without 
cooperation from the PHY and bringing it up here doesn't really buy us 
much (see above). What I could do here is already call dwc3_core_probe 
and then immediately dwc3_core_exit again to get rid of that condition.



Thanks,


Sven

Powered by blists - more mailing lists