Message-ID: <20250805231808.cjht64mfabaw2ik5@synopsys.com>
Date: Tue, 5 Aug 2025 23:18:09 +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 Tue, Aug 05, 2025, Krishna Kurapati wrote:
> 
> 
> On 8/1/2025 6:31 AM, Thinh Nguyen wrote:
> > On Wed, Jul 30, 2025, Krishna Kurapati wrote:
> > > 
> > > 
> > > On 7/30/2025 6:53 AM, Thinh Nguyen wrote:
> > > > 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?
> > > 
> > > My response didn't show up on lore since the mail client I used before sent
> > > the message in HTML format. So resending my response again.
> > > 
> > > Hi Thinh,
> > > 
> > >   Yes this is for cases where role switching is present (either with a Type-C
> > > controller, USB-conn-gpio, or a glink based role-switch).
> > > 
> > >   Actually the requirement is as follows:
> > >   1. When in device mode, if we receive a cable disconnect, we need to clear
> > > OTG_VBUS_VALID bit of QSCRATCH register in glue address space.
> > >   2. When cable is connected in device mode, we need to set the
> > > OTG_VBUS_VALID bit of QSCRATCH register in glue address space.
> > >   3. When the runstop is set back after a suspend rotuine, then we need to
> > > set  OTG_VBUS_VALID bit of QSCRATCH register in glueaddress space.
> > > 
> > >   To take care of all the 3 scenarios above, the set_role and run_stop
> > > notifiers have been added.
> > > 
> > >   The role info propagates only from core to glue. It is not sent to the phy.
> > > 
> > 
> > When does ROLE NONE occur? Did you have the type-c driver set the role
> > switch to "none" indicate disconnect?
> > 
> > The vbus-valid is only for gadget, and you only care about the
> > OTG_VBUS_VALID right? Can we just use the dwc3->connected flag? Just
> > make sure that it's cleared on role-switch, which should be the case
> > because we always perform soft-disconnect on gadget unbind, and make
> > sure to set vbus-valid on pullup or gadget binding. Is there some
> > scenarios that dwc->connected does not cover?
> > 
> 
> Hi Thinh,
> 
>  In case of just a cable disconnect in device mode (and default dr mode is
> peripheral only), there would be no role switch. In that scenario, connected
> flag would stay "true" even after removing cable. In that case, we can
> generate disconnect interrupt only by clearing this VBUS_VALID bit and
> inturn the suspend will succeed. So wanted to use notification from set_role
> which would cover all cases:
> 1. cable disconnect/cable connect
> 2. Role switch from host->device and device->host
> 

Ok. Thanks for the explanation. How everything tied together seems
awkward: The connector does a role-switch to "none" to trigger a
role-switch in dwc3, which then triggers a callback to dwc3-qcom to
clear vbus_valid, which then allows the controller to see a disconnect
event in dwc3. Is that right?

The connector driver should have a separate interface for
attach/deattach in addition to role-switch. But I don't think type-c
controller, USB-conn-gpio, and glink share the same way to handle
connection/disconnection.

Let's keep what you proposed, and keep in mind that ROLE NONE is for
selecting the default mode and not necessarily mean disconnection.

BR,
Thinh

Powered by blists - more mailing lists