Re: [PATCH v3] usb: dwc3: Notify the glue of role changes from the mode switch work

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

On Wed, Nov 11, 2026, Vladyslav Tymoshchuk wrote:
> dwc3_usb_role_switch_set() and dwc3_drd_notifier() call the glue's
> pre_set_role() synchronously, from the context of whoever changes the
> role, while the mode switch itself runs from drd_work on a freezable
> workqueue. Not every caller is frozen during system sleep: the UCSI
> connector work, for one, runs on the non-freezable system workqueue.
> 
> On SM8750, when the USB cable is unplugged while the system is in
> s2idle, the UCSI connector work for the change runs before the devices
> have resumed. It switches the role to USB_ROLE_NONE, and
> dwc3_qcom_set_role_notifier() accesses QSCRATCH while the glue clocks
> are still off. The access does not complete and the SoC resets.
> pm_runtime_resume_and_get() does not prevent this: runtime PM is
> disabled at that point and the device is still marked active, so it
> returns success without resuming anything.
> 
> Record the role and call pre_set_role() from __dwc3_set_mode() instead,
> so that the glue is not called while the system is suspended. The mode
> alone can't carry this: USB_ROLE_NONE maps to the default mode, so a
> switch between it and that role leaves the mode unchanged, and the glue
> still has to be told. Make the call before the checks that skip the
> mode switch, and only when a role was recorded, as the mode can also
> change without one from debugfs and from dwc3_setup_role_switch().
> 
> Role changes that come in while the work is frozen collapse into the
> last one, which is the state the hardware has to end up in.
> 
> Fixes: 21188e8d6d75 ("usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend")
> Cc: stable@vger.kernel.org
> Suggested-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> Signed-off-by: Vladyslav Tymoshchuk <tvlad744@gmail.com>
> ---
> Changes in v3:
> - Fix this in the dwc3 core instead of in the Qualcomm glue. The code is
>   Thinh's change from [1]; I only updated the two comments that went
>   stale with it. All glue drivers benefit, and dwc3-qcom needs no
>   deferral or lock of its own.
> - The role is recorded in struct dwc3, as the mode alone can't tell
>   USB_ROLE_NONE from the role it maps to (Prashanth, [2]).
> - The extcon path goes through the same work.
> - Drop Krishna's Tested-by, the code is different now.
> - Link to v2: https://urldefense.com/v3/__https://lore.kernel.org/all/20261022224220.480-1-tvlad744@gmail.com/__;!!A4F2R9G_pg!eKOqFW5EvS63b73MwgODm6dZ1aOBc_KIiaxbf0F-4cQcKocNHs2hwe3B0uJT4CH03Fa3_SgHFx7g2fq13Nmtxw$ 
> 
> Tested on OnePlus Pad 3 (SM8750, UCSI over pmic_glink) with v7.3-rc5
> plus board patches, details in [3]. Without the change, unplugging the
> cable in s2idle resets the SoC. With it there was no reset in five
> suspend runs in device mode (unplug, plug in, both, cable left in) and
> five in host mode (USB stick left in or unplugged). The host-mode runs
> were on a kernel that also keeps the USB GDSCs in retention [4]. The
> comment updates were not in the tested kernel.
> 
> I used an AI assistant (Claude) for the analysis and to draft the
> description; I reviewed the result and ran the tests on my device.
> 
> [1] https://urldefense.com/v3/__https://lore.kernel.org/all/auu9hx_Xnjbi3WkB@vbox/__;!!A4F2R9G_pg!eKOqFW5EvS63b73MwgODm6dZ1aOBc_KIiaxbf0F-4cQcKocNHs2hwe3B0uJT4CH03Fa3_SgHFx7g2fqItiPPZQ$ 
> [2] https://urldefense.com/v3/__https://lore.kernel.org/all/f9d27fd8-cf33-4e6a-a9fc-6770a933cfb3@oss.qualcomm.com/__;!!A4F2R9G_pg!eKOqFW5EvS63b73MwgODm6dZ1aOBc_KIiaxbf0F-4cQcKocNHs2hwe3B0uJT4CH03Fa3_SgHFx7g2frebDF9aw$ 
> [3] https://urldefense.com/v3/__https://lore.kernel.org/all/20261110172148.1640-1-tvlad744@gmail.com/__;!!A4F2R9G_pg!eKOqFW5EvS63b73MwgODm6dZ1aOBc_KIiaxbf0F-4cQcKocNHs2hwe3B0uJT4CH03Fa3_SgHFx7g2folWmClAQ$ 
> [4] https://urldefense.com/v3/__https://lore.kernel.org/all/20261110213346.1359-1-tvlad744@gmail.com/__;!!A4F2R9G_pg!eKOqFW5EvS63b73MwgODm6dZ1aOBc_KIiaxbf0F-4cQcKocNHs2hwe3B0uJT4CH03Fa3_SgHFx7g2frSDJpOFQ$ 
> 
>  drivers/usb/dwc3/core.c      | 25 ++++++++++++++++++++++++-
>  drivers/usb/dwc3/core.h      |  8 +++++++-
>  drivers/usb/dwc3/drd.c       |  6 ++----
>  drivers/usb/dwc3/dwc3-qcom.c |  5 +----
>  4 files changed, 34 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index fd5c2cd36c59..99a39c828543 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -169,15 +169,23 @@ static void __dwc3_set_mode(struct work_struct *work)
>  	int ret;
>  	u32 reg;
>  	u32 desired_dr_role;
> +	enum usb_role requested_role;
> +	bool notify_glue;
>  	int i;
>  
>  	mutex_lock(&dwc->mutex);
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	desired_dr_role = dwc->desired_dr_role;
> +	requested_role = dwc->requested_role;
> +	notify_glue = dwc->requested_role_pending;
> +	dwc->requested_role_pending = false;
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
>  	pm_runtime_get_sync(dwc->dev);
>  
> +	if (notify_glue)
> +		dwc3_pre_set_role(dwc, requested_role);
> +
>  	if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_OTG)
>  		dwc3_otg_update(dwc, 0);
>  
> @@ -287,7 +295,8 @@ static void __dwc3_set_mode(struct work_struct *work)
>  	mutex_unlock(&dwc->mutex);
>  }
>  
> -void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
> +static void __dwc3_queue_mode(struct dwc3 *dwc, u32 mode, enum usb_role role,
> +			      bool notify_glue)
>  {
>  	unsigned long flags;
>  
> @@ -296,11 +305,25 @@ void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	dwc->desired_dr_role = mode;
> +	if (notify_glue) {
> +		dwc->requested_role = role;
> +		dwc->requested_role_pending = true;
> +	}
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
>  	queue_work(system_freezable_wq, &dwc->drd_work);
>  }
>  
> +void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
> +{
> +	__dwc3_queue_mode(dwc, mode, USB_ROLE_NONE, false);
> +}
> +
> +void dwc3_set_role(struct dwc3 *dwc, u32 mode, enum usb_role role)
> +{
> +	__dwc3_queue_mode(dwc, mode, role, true);
> +}
> +
>  u32 dwc3_core_fifo_space(struct dwc3_ep *dep, u8 type)
>  {
>  	struct dwc3		*dwc = dep->dwc;
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index f3dabb37f806..dc7cd751ed4d 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1000,7 +1000,8 @@ struct dwc3_scratchpad_array {
>  /**
>   * struct dwc3_glue_ops - The ops indicate the notifications that
>   *				need to be passed on to glue layer
> - * @pre_set_role: Notify glue of role switch notifications
> + * @pre_set_role: Notify glue of role switch notifications, called from
> + *		  drd_work so that it does not run during system sleep
>   * @pre_run_stop: Notify run stop enable/disable information to glue
>   */
>  struct dwc3_glue_ops {
> @@ -1058,6 +1059,8 @@ struct dwc3_glue_ops {
>   * @dr_mode: requested mode of operation
>   * @current_dr_role: current role of operation when in dual-role mode
>   * @desired_dr_role: desired role of operation when in dual-role mode
> + * @requested_role: role requested
> + * @requested_role_pending: @requested_role is yet to be passed to the glue
>   * @edev: extcon handle
>   * @edev_nb: extcon notifier
>   * @hsphy_mode: UTMI phy mode, one of following:
> @@ -1261,6 +1264,8 @@ struct dwc3 {
>  	enum usb_dr_mode	dr_mode;
>  	u32			current_dr_role;
>  	u32			desired_dr_role;
> +	enum usb_role		requested_role;
> +	bool			requested_role_pending;
>  	struct extcon_dev	*edev;
>  	struct notifier_block	edev_nb;
>  	enum usb_phy_interface	hsphy_mode;
> @@ -1614,6 +1619,7 @@ struct dwc3_gadget_ep_cmd_params {
>  /* prototypes */
>  void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode, bool ignore_susphy);
>  void dwc3_set_mode(struct dwc3 *dwc, u32 mode);
> +void dwc3_set_role(struct dwc3 *dwc, u32 mode, enum usb_role role);
>  u32 dwc3_core_fifo_space(struct dwc3_ep *dep, u8 type);
>  
>  #define DWC3_IP_IS(_ip)							\
> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> index f3e37d383627..55036466ec7a 100644
> --- a/drivers/usb/dwc3/drd.c
> +++ b/drivers/usb/dwc3/drd.c
> @@ -439,8 +439,7 @@ static int dwc3_drd_notifier(struct notifier_block *nb,
>  	enum usb_role role = mode == DWC3_GCTL_PRTCAP_HOST ?
>  				     USB_ROLE_HOST : USB_ROLE_DEVICE;
>  
> -	dwc3_pre_set_role(dwc, role);
> -	dwc3_set_mode(dwc, mode);
> +	dwc3_set_role(dwc, mode, role);
>  
>  	return NOTIFY_DONE;
>  }
> @@ -468,8 +467,7 @@ static int dwc3_usb_role_switch_set(struct usb_role_switch *sw,
>  		break;
>  	}
>  
> -	dwc3_pre_set_role(dwc, role);
> -	dwc3_set_mode(dwc, mode);
> +	dwc3_set_role(dwc, mode, role);
>  	return 0;
>  }
>  
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index ac68b4218b56..2e263b378966 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -578,10 +578,7 @@ static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_rol
>  	pm_runtime_mark_last_busy(qcom->dev);
>  	pm_runtime_put_sync(qcom->dev);
>  
> -	/*
> -	 * Current role changes via usb_role_switch_set_role callback protected
> -	 * internally by mutex lock.
> -	 */
> +	/* Role changes are serialized by dwc->mutex in drd_work */
>  	qcom->current_role = next_role;
>  }
>  
> 
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh