Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>]

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

Regards,
Krishna,