Message-ID: <20250801010119.tvhb6tjlf4fq7zhg@synopsys.com>
Date: Fri, 1 Aug 2025 01:01:24 +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 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?

Thanks,
Thinh

Powered by blists - more mailing lists