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

[Faisal Hassan]

On 02-11-2026 07:51 am, Peter Chen wrote:
> On 26-10-30 16:54:55, Faisal Hassan wrote:
> > 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.
>
> Okay, so here you may need another patch for error handling improvement.
Ok, I will put the failed-resume handling (the wakeup interrupts staying 
disarmed, with arm/disarm kept balanced) in its own patch.

> > > >    }
> > > >    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.
>
> Get it.
>
> At your second patch, it already knows the context between runtime
> and system suspend, you could disable wakeup enable if the device_may_wakeup
> return false, that could avoid introducing more judgements, and have this
> patch after your 2nd patch.
Agreed. I will move the PM/runtime split first. Then, in the system 
suspend path, I will disarm the wakeup interrupts when 
device_may_wakeup() is false (including when the controller is already 
runtime suspended and they were armed earlier). The resume side then 
needs no extra check. The failed-resume handling goes in a separate 
patch after that.

Thanks,
Faisal

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