Re: [PATCH v4 5/7] usb: dwc3: qcom: Support broken suspend for SA8255P

[Faisal Hassan <faisal.hassan@oss.qualcomm.com>]

On 02-11-2026 09:02 am, Peter Chen wrote:
> On 26-10-29 00:29:35, Faisal Hassan wrote:
>> During APPS Suspend, the clock controller unit disables all clocks
>> to the CPU and subsystems, causing the controller to enter the
>> Power-On Reset (POR) state. This results in the loss of
>> Qscratch programming.
>>
>> Upon APPS Resume, the USB controller does not have the proper
>> PIPE_UTMI_CLK_SEL or VBUS_VALID. Hence, if only the High-Speed
>> PHY is used for a controller capable of SuperSpeed operations,
>> the controller will assume it should use the USB3 PHY and not
>> the HS PHY as configured before APPS Suspend. This behavior
>> disrupts the normal operation of the USB controller. Similarly,
>> if the controller was operating in device mode prior to
>> APPS suspend, and VBUS_VALID signal is not set after APPS resume,
>> this will cause the PHYs to transition into the suspend state.
>> This behavior disrupts the normal operation of the USB controller.
>>
>> Address the issues by ensuring that the PIPE_UTMI_CLK_SEL and
>> VBUS_VALID are handled properly after the system resumes from
>> APPS Suspend. This is crucial for maintaining the correct
>> operational state of the USB controller and preventing
>> unintended suspensions of the PHYs.
>>
>> Also, as the controller is starting from POR state, make
>> sure the controller Reset is performed.
>>
>> Because the controller is reset on every system resume, the
>> Qscratch restore is done whether or not wakeup was enabled, and
>> dwc3 core is told via needs_full_reinit to fully re-initialize the
>> core on system resume even in host mode with wakeup enabled. Wakeup
>> on this platform comes from the PHY interrupts routed through PDC,
>> not from the controller staying powered. VBUS_VALID is only forced
>> outside host mode, matching what probe does.
>>
>> The XHCI controller is reset by the same event, so also set the
>> xhci_reset_on_resume dwc3 property. dwc3 passes it on to the xhci-plat
>> device as xhci-reset-on-resume-quirk, which makes xhci-plat set
>> XHCI_RESET_ON_RESUME.
>>
>> Also make the .prepare callback return 0 on these platforms. A
>> runtime-suspended device in device mode would otherwise be left in
>> place across system sleep (direct_complete), skipping the system
>> resume callback and with it the reset and Qscratch restore.
> 
> Faisal, there are several issues in this patch, would you try to split them?
> - XHCI_RESET_ON_RESUME quirk support for dwc3
> - PIPE_UTMI_CLK_SEL restore after system resume, there is a similar submission recently [1]
> - VBUS override restore after system resume
> - The improvement for system resume dues to HW POR or lost of power.
> 
> [1]: https://lore.kernel.org/all/20261027-dwc3-utmi-resume-v1-1-ca9f0aa65783@gmail.com/
> 
Yes. For v5 I will split it into:

1. Controller recovery from power loss across system suspend (reset on 
system resume, full re-init, the flag and its match data).
2. XHCI_RESET_ON_RESUME support for dwc3.
3. VBUS override restore after system resume.

The PIPE_UTMI_CLK_SEL restore is covered by the submission you pointed 
to [1]. I will drop it from my series and rebase on top of [1]. The 
reset in my resume path will be placed before the UTMI clock selection, 
so [1] still takes effect after the reset.

>>
>>   struct dwc3_qcom {
>>   	struct device		*dev;
>>   	void __iomem		*qscratch_base;
>> @@ -86,6 +90,13 @@ struct dwc3_qcom {
>>   	struct icc_path		*icc_path_apps;
>>   
>>   	enum usb_role		current_role;
>> +
>> +	bool			broken_suspend;
>> +	bool			ignore_pipe_clk;
>> +};
>> +
>> +static const struct dwc3_qcom_priv_data sa8255p_dwc3_qcom_priv_data = {
>> +	.broken_suspend		= true,
>>   };
> 
> Why you consider it is "broken", or is it a normal resume recovery due to lost of status
> for controller and PHY? Why other SoCs don't need it?
> 

It is a normal recovery, and "broken" is the wrong name. On this 
automotive platform the GDSC must be powered down and the APPS enters 
VDDmin during system suspend, so the controller loses its state, 
including the Qscratch programming. This is the same case that 
needs_full_reinit was added for, and dwc3-imx sets it unconditionally 
and restores its glue registers on resume for the same reason ("Upon 
power loss any previous configuration is lost, restore it"). I will 
rename the flag to describe the power loss (power_lost_in_suspend) and 
reword the subject and commit message.

>>   static int dwc3_qcom_prepare(struct device *dev)
>>   {
>>   	struct dwc3 *dwc = dev_get_drvdata(dev);
>> +	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
>> +
>> +	/*
>> +	 * The controller loses power across system suspend, so the system
>> +	 * resume callback must always run to reset and reprogram it. Don't
>> +	 * let the PM core leave a runtime-suspended device as-is.
>> +	 */
>> +	if (qcom->broken_suspend)
>> +		return 0;
> 
> Why we need this? I see dwc3 core do dwc3_core_init_for_resume at device mode
> during the resume.
> 

That only helps if the system resume callback runs. dwc3_pm_prepare() 
returns 1 when the controller is in device mode and already runtime 
suspended, so the PM core takes the direct_complete path and skips the 
system resume callbacks. Neither the core re-init nor the glue's reset 
and Qscratch restore would run after the power loss, and the runtime 
resume path does not do the glue part. Returning 0 from .prepare makes 
system resume run.

I am going to confirm this on hardware and will put the result in the v5 
commit message.

>>   
>>   	return dwc3_pm_prepare(dwc);
>>   }
>