Message-ID: <20250924223622.r3ozwbm35jn3vg7m@synopsys.com>
Date: Wed, 24 Sep 2025 22:36:26 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Sven Peter <sven@...nel.org>
CC: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        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

On Sun, Sep 21, 2025, Sven Peter wrote:
> 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.
> 
> 

Nice work figuring this all out! All of this information is very
valuable. I think it will benefit everyone if you can capture this
finding somewhere. Base on your description, probably "state" is a
better name.

> 
> > > + */
> > > +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.
> 

Understood. Thanks for the explanation.

BR,
Thinh

Powered by blists - more mailing lists