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

[Peter Chen]

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@xxxxxxxxx/

> 
>  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?

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

>  
>  	return dwc3_pm_prepare(dwc);
>  }

-- 

Thanks,
Peter Chen