Message-ID: <20250807005814.nlf7aotts5o7navl@synopsys.com>
Date: Thu, 7 Aug 2025 00:58:15 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: John Keeping <jkeeping@...usicbrands.com>
CC: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        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 Wed, Aug 06, 2025, John Keeping wrote:
> 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.

Ok.

> 
> > 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.
> 

Sure. I'll need to get back on this.

BR,
Thinh

Powered by blists - more mailing lists