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

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

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