Message-ID: <aJOMk_PVasn-tvV6-jkeeping@inmusicbrands.com>
Date: Wed, 6 Aug 2025 18:10:43 +0100
From: John Keeping <jkeeping@...usicbrands.com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: Kishon Vijay Abraham I <kishon@...nel.org>,
	Neil Armstrong <neil.armstrong@...aro.org>,
	Heiko Stuebner <heiko@...ech.de>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	Sebastian Reichel <sebastian.reichel@...labora.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-rockchip@...ts.infradead.org" <linux-rockchip@...ts.infradead.org>,
	Vinod Koul <vkoul@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RFC/PATCH 1/2] usb: dwc3: disable for USB_ROLE_NONE

On Tue, Jul 29, 2025 at 10:10:35PM +0000, Thinh Nguyen wrote:
> On Mon, Jul 14, 2025, John Keeping wrote:
> > On Sat, Jul 12, 2025 at 12:11:38AM +0000, Thinh Nguyen wrote:
> > > On Thu, Jul 10, 2025, John Keeping wrote:
> > > > When the phy is acting as a Type C mux, it may need to reset when the
> > > > cable direction changes.  But this should not happen while DWC3 is
> > > > trying to use the USB connection.
> > > > 
> > > > In this case, there must be a connection manager to notify the phy of
> > > > the orientation change and tcpm_mux_set() ensures this happens before
> > > > DWC3's role switch is informed of a change.
> > > > 
> > > > It should not be possible to go directly from device->device or
> > > > host->host with a change in orientation without transitioning through
> > > > the "none" role as the cable is unplugged.  So ensuring that DWC3 always
> > > 
> > > The controller is either host or device mode. It's odd to use "none" to
> > > indicate connection.
> > 
> > There is no connection in this state.  When the type C controller
> > indicates that the role is "none" then there cannot be a USB connection.
> > 
> 
> It's not about connection. It's about the current mode of the USB
> controller. The mode of the controller isn't "none" on disconnection
> even if you try to role-switch to that via the typec switch.
> 
> > > > informs the phy of the new mode whenever a plug is detected should be
> > > > sufficient for the phy to safely reset itself at a time that is safe for
> > > > DWC3.
> > > 
> > > Couldn't the phy do this as it detects connection/disconnection.
> > 
> > I don't think the phy can detect a connection.  If it is configured for
> > the wrong orientation then it will not monitor the correct data lines.
> > The phy hardware does not signal any interrupts to the CPU for the
> > driver to react to.
> 
> Ok.
> 
> > 
> > > It seems what you need is a full controller initialization upon new
> > > connection on orientation change, and you're using role_switch selecting
> > > "none" to do so.
> > 
> > I'm not sure whether a complete initialization is necessary, but what I
> > want to avoid is the phy resetting while the controller is part-way
> > through device enumeration or setting up a gadget configuration.
> > 
> > It may be that simply avoiding resetting the phy if the orientation is
> > unchanged is enough to avoid some of this problem, but I suspect there
> > are still problems if the clocks from the phy to the controller are
> > stopped unexpectedly.  However, I have run some tests of this change and
> > it looks promising.
> 
> That sounds like the the phy reset is done after asserting the vbus
> valid line indicating connection to the controller. Can this be fixed in
> the phy or type-c driver?

I don't think so.  I've tried implementing this in the phy driver and it
does not work reliably.

> > > I'm not sure if role-switch has the right interface to do so. Perhaps we
> > > can introduce one? I don't think we should change the behavior of the
> > > current flow and apply that to all other platforms.
> > 
> > I don't think it's unreasonable for the controller to be idle when there
> > is an external type C controller notifying the connection state.
> > 
> > The hardware setup looks like this, with the Linux type C code notifying
> > the phy driver of the orientation change and the DWC3 driver of the role
> > change:
> > 
> >                       +------+          
> >                       | DWC3 |<----+    
> >                       +--^---+     |    
> >                          |         |    
> >                       +--v--+      |    
> >                 +---->| PHY |      |Role
> >                 |     +--^--+      |    
> >     Orientation |        |         |    
> >                 |   +----v----+    |    
> >                 +---+ FUSB302 +----+    
> >                     +---------+
> > 
> > The advantage of using the role hook is that we are guaranteed that it
> > is called after the phy has been notified of the orientation change.
> > 
> > Do you have an idea of a new interface?  Or do you think it is safe to
> > reset the phy underneath the controller when that will halt the clocks
> > from the phy to the controller for some period of time?
> 
> The proper fix should be in the phy where it should only assert vbus
> valid after processing orientation and phy reset. I'm not familiar on
> the communication between your PHY and FUSB302. There should be some
> indication from the phy that it's ready before you can assert vbus-valid
> (that can be from a callback, a status register etc).

The FUSB302 is connected to the CPU via I2C, there is no direct
communication between it and the phy, it all goes via the kernel's tcpm
subsystem.

The problem with re-initializing the phy is that this causes dwc3 gadget
configuration to fail with "failed to enable ep0out", and this happens
when VBUS valid is de-asserted.  This is 100% reproducible on boot for
me where the init scripts are setting up a gadget via configfs.

So it seems the timing of the phy setup needs to coordinate with the
dwc3 controller.

> What you've done is a teardown and reinit of the driver, triggering a
> soft-reset. This can re-sync the phy. That probably takes long enough
> that a phy-reset after orientation change will be completed before the
> initialization complete.
> 
> If there's no way for you to know when the phy complete its reset, we
> can add a quirk for your platform to always reinit on role-switch call.
> 
> Let me know if this is reasonable.

I'm happy to test patches.


Kind regards,
John

Powered by blists - more mailing lists