Message-ID: <20250730012307.7rwh3lzz4ohd4mob@synopsys.com>
Date: Wed, 30 Jul 2025 01:23:08 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Krishna Kurapati <krishna.kurapati@....qualcomm.com>
CC: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Bjorn Andersson <bjorn.andersson@....qualcomm.com>,
        "linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to
 facilitate runtime suspend

On Sun, Jul 13, 2025, Krishna Kurapati wrote:
> 
> 
> On 7/12/2025 4:59 AM, Thinh Nguyen wrote:
> > On Tue, Jun 24, 2025, Krishna Kurapati wrote:
> > > 
> > > 
> > > On 6/24/2025 5:02 AM, Thinh Nguyen wrote:
> > > > On Tue, Jun 10, 2025, Krishna Kurapati wrote:
> > > > > On Qualcomm DWC3 dual-role controllers, the conndone/disconnect events in
> > > > > device mode are generated by controller when software writes to QSCRATCH
> > > > > registers in Qualcomm Glue layer rather than the vbus line being routed to
> > > > > dwc3 core IP for it to recognize and generate these events.
> > > > > 
> > > > > UTMI_OTG_VBUS_VALID  bit of QSCRATCH_HS_PHY_CTRL register needs to be set
> > > > > to generate a connection done event and to be cleared for the controller to
> > > > > generate a disconnect event during cable removal. When the disconnect is
> > > > > not generated upon cable removal, the "connected" flag of dwc3 is left
> > > > > marked as "true" and it blocks suspend routines and for that to happen upon
> > > > > cable removal, the cable disconnect notification coming in via set_role
> > > > > call need to be provided to the Qualcomm glue layer as well.
> > > > > 
> > > > > Currently, the way DWC3 core and Qualcomm legacy glue driver are designed,
> > > > > there is no mechanism through which the DWC3 core can notify the Qualcomm
> > > > > glue layer of any role changes which it receives via role switch. To
> > > > > register these glue callbacks at probe time, for enabling core to notify
> > > > > glue layer, the legacy Qualcomm driver has no way to find out when the
> > > > > child driver probe was successful since it does not check for the same
> > > > > during of_platform_populate.
> > > > > 
> > > > > Hence implement the following glue callbacks for flattened Qualcomm glue
> > > > > driver:
> > > > > 
> > > > > 1. set_role: To pass role switching information from drd layer to glue.
> > > > > This information is needed to identify NONE/DEVICE mode switch and modify
> > > > > QSCRATCH to generate connect-done event on device mode entry and disconnect
> > > > > event on cable removal in device mode.
> > > > > 
> > > > > 2. run_stop: When booting up in device mode, if autouspend is enabled and
> > > > > userspace doesn't write UDC on boot, controller enters autosuspend. After
> > > > > this, if the userspace writes to UDC in the future, run_stop notifier is
> > > > > required to enable UTMI_OTG_VBUS_VALID of QSCRATCH so that connect done
> > > > > event is generated after run_stop(1) is done to finish enumeration.
> > > > > 
> > > > > Signed-off-by: Krishna Kurapati <krishna.kurapati@....qualcomm.com>
> > > > > ---
> > > > >    drivers/usb/dwc3/dwc3-qcom.c | 82 ++++++++++++++++++++++++++++++++----
> > > > >    1 file changed, 73 insertions(+), 9 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> > > > > index ca7e1c02773a..d40b52e2ba01 100644
> > > > > --- a/drivers/usb/dwc3/dwc3-qcom.c
> > > > > +++ b/drivers/usb/dwc3/dwc3-qcom.c
> > > > > @@ -89,6 +89,12 @@ struct dwc3_qcom {
> > > > >    	bool			pm_suspended;
> > > > >    	struct icc_path		*icc_path_ddr;
> > > > >    	struct icc_path		*icc_path_apps;
> > > > > +
> > > > > +	/*
> > > > > +	 * Current role changes via usb_role_switch_set_role callback protected
> > > > > +	 * internally by mutex lock.
> > > > > +	 */
> > > > > +	enum usb_role		current_role;
> > > > 
> > > > Can we just track the current role through dwc3 core instead of an
> > > > addition field in the glue?
> > > > 
> > > 
> > > Core caches only mode. We need ROLE NONE to identify cable connect. So
> > > adding that in glue to keep track.
> > > 
> > 
> > The controller is always either host or device, not somewhere in
> > between. You're using ROLE_NONE to indicate connection, which is a
> > separate state.
> > 
> 
> Yes, but there is no flag that indicates that in dwc structure today. Also
> since only dwc3-qcom needs it at the moment, I added that role info in glue
> layer.

How are you using ROLE NONE? Do you send a role-switch call to "none" to
indicate disconnect? Let's not do that. Currently the dwc3 driver would
switch back to the default mode if "none" is selected, but this is not
well defined and implementation specific. It can be no-op and would not
violate the interface.

The role-switch interface should only be used for role-switching and not
connection/disconnection.

> 
> > I feel this should be tracked separately for clarity. The dwc3 also
> > tracks the connection state, can we use that?
> > 
> 
> Are you referring to the "connected" flag in DWC structure ? I see that it
> indicates connection status only in gadget mode.
> 

Yes, that flag is only for gadget.

Can you provide more info of the setup? Is there a type-c controller or
phy that can detect attach/deattach? Or it only propagates to the usb
controller?

BR,
Thinh

Powered by blists - more mailing lists