Re: [PATCH 2/3] USB: dwc3: qcom: fix wakeup after probe deferral

Krishna Kurapati PSSNV
2023-11-21T12:55:59Z

>
>> I get that dwc3_qcom_enable_interrupts() limits the scope of what wakes us
>> up to what we expect given the current device (or lack thereof), but it
>> doesn't seem like you're really meant to play with the IRQ triggers,
>> or at least the warning you shared makes me think it is not a great idea
>> if you plan to probe the device ever again in the future.
>>
>> I'll post the current comment in dwc3_qcom_enable_interrupts() to
>> explain the "limits the scope of what wakes us up" a bit more clearly:
>>
>> /*
>> * Configure DP/DM line interrupts based on the USB2 device attached to
>> * the root hub port. When HS/FS device is connected, configure the DP line
>> * as falling edge to detect both disconnect and remote wakeup scenarios. When
>> * LS device is connected, configure DM line as falling edge to detect both
>> * disconnect and remote wakeup. When no device is connected, configure both
>> * DP and DM lines as rising edge to detect HS/HS/LS device connect scenario.
>> */
>
> Yes, that is how it is currently implemented and I intend to change that
> shortly. I just wanted to get the fixes out first.
>
> Specifically, I consider the current implementation to be broken in that
> it generates wakeup events on disconnect which is generally not want you
> want. Consider closing the lid of your laptop and disconnecting a USB
> mouse before putting it in your backpack. Now it's no longer suspended
> as you would expect it to be.
>
> With the devictrees soon fixed, we could also do away with changing the
> trigger type, but since this is how it was implemented initially we now
> need to consider backward compatibility with the broken DTs. We've dealt
> with that before, but yeah, getting things right from the start would
> have been so much better.
>

Hi Johan,

Just one query. Even if it wakes up after closing the lid and removing
the mouse, wouldn't pm suspend be triggered again later by the system
once it sees that usb is also good to be suspended again ? I presume a
laptop form factor would be having this facility of re-trigerring
suspend. Let me know if this is not the case.

Also, the warning you are mentioning in [1] comes because this is a
laptop form factor and we have some firmware running (I don't know much
about ACPI and stuff) ?

[1]:
https://lore.kernel.org/all/20231120161607.7405-3-johan+linaro@kernel.org/

Regards,
Krishna,