Re: [PATCH v4 1/7] usb: dwc3: qcom: Keep wakeup IRQ enable/disable balanced on failed resume

[Faisal Hassan]

On 30-10-2026 02:54 pm, Peter Chen wrote:
> On 26-10-29 00:29:31, Faisal Hassan wrote:
> > dwc3_qcom_resume() disables the wakeup interrupts before enabling the
> > clocks. If clk_bulk_prepare_enable() fails, it returns with them
> > disabled.
>
> What's the relationship between clk_bulk_prepare_enable() fails and this patch?
Hi Peter,

The wakeup interrupts were disarmed before the clock enable. If that
failed, is_suspended stayed set, the next suspend returned early, and
the next resume disarmed them again. disable_irq_nosync() and
disable_irq_wake() nest, so each failed resume left the disable depth
one level deeper. This is the rarer case, so I will reword the commit
message to lead with the runtime suspend case below.

> > The next suspend returns early on is_suspended without
> > re-enabling them, and the next resume disables them again, so every
> > failed resume leaves the disable depth one level deeper and host-mode
> > wakeup is lost for good.
> >
> > Re-enabling them on the error path is not safe either: the
> > level-triggered ss_phy interrupt can stay asserted, and the handler
> > can't make progress while the controller is not resumed, so it would
> > fire continuously.
> >
> > Track whether the wakeup interrupts are armed and make enabling and
> > disabling them idempotent. A failed resume now leaves them disarmed and
> > balanced, and the next suspend arms them again. Disarm them in resume
> > regardless of the wakeup setting, so interrupts armed by a runtime
> > suspend are also disarmed when system resume runs with device wakeup
> > disabled.
> >
> > Signed-off-by: Faisal Hassan <faisal.hassan@xxxxxxxxxxxxxxxx>
> > ---
> >   drivers/usb/dwc3/dwc3-qcom.c | 20 ++++++++++++++++++--
> >   1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> > index d44f093ac243..d4fda4dd81f6 100644
> > --- a/drivers/usb/dwc3/dwc3-qcom.c
> > +++ b/drivers/usb/dwc3/dwc3-qcom.c
> > @@ -81,6 +81,7 @@ struct dwc3_qcom {
> >   	enum usb_dr_mode	mode;
> >   	bool			is_suspended;
> >   	bool			pm_suspended;
> > +	bool			wakeup_irqs_armed;
> >   	struct icc_path		*icc_path_ddr;
> >   	struct icc_path		*icc_path_apps;
> >   
> > @@ -323,16 +324,26 @@ static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
> >   {
> >   	int i;
> >   
> > +	if (!qcom->wakeup_irqs_armed)
> > +		return;
> > +
> >   	for (i = 0; i < qcom->num_ports; i++)
> >   		dwc3_qcom_disable_port_interrupts(&qcom->ports[i]);
> > +
> > +	qcom->wakeup_irqs_armed = false;
> >   }
> >   
> >   static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
> >   {
> >   	int i;
> >   
> > +	if (qcom->wakeup_irqs_armed)
> > +		return;
> > +
> >   	for (i = 0; i < qcom->num_ports; i++)
> >   		dwc3_qcom_enable_port_interrupts(&qcom->ports[i]);
> > +
> > +	qcom->wakeup_irqs_armed = true;
> >   }
> >   
> >   static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
> > @@ -377,8 +388,13 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup)
> >   	if (!qcom->is_suspended)
> >   		return 0;
> >   
> > -	if (dwc3_qcom_is_host(qcom) && wakeup)
> > -		dwc3_qcom_disable_interrupts(qcom);
> > +	/*
> > +	 * Wakeup interrupts may have been armed by an earlier runtime suspend
> > +	 * even if wakeup is now disabled,
>
> How wakeup interrupt is triggered even the wake interrupt is disabled? Is the
> hardware default value makes it enabled? If it is, we need to disable all
> wakeup interrupt at probe, otherwise, there are un-expected wakeup interrupts.
>
> Peter

They are not. They are requested with IRQF_NO_AUTOEN, so they stay
disabled until the first suspend and nothing needs disabling at probe.
The issue is that runtime suspend always calls
dwc3_qcom_suspend(qcom, true), which arms them in host mode regardless
of device_may_wakeup(). The old resume only disarmed them when wakeup
was enabled, so a system resume with device wakeup disabled left them
armed. The comment read as if the interrupts trigger while disabled,
so I will reword it as:

    /*
     * Runtime suspend arms the wakeup interrupts regardless of the
     * device wakeup setting, so always disarm them here. On failure
     * they stay disarmed, as a level-triggered wakeup line could
     * otherwise keep firing while the controller can't be resumed.
     */

I will update the subject and commit message for v5 to cover this case.

Thanks,
Faisal
> > so always disarm them. On failure
> > +	 * they stay disarmed, as a level-triggered wakeup line could otherwise
> > +	 * keep firing while the controller can't be resumed.
> > +	 */
> > +	dwc3_qcom_disable_interrupts(qcom);
> >   
> >   	ret = clk_bulk_prepare_enable(qcom->num_clocks, qcom->clks);
> >   	if (ret < 0)
> > --
> > 2.34.1