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@xxxxxxxxxx/
Regards,
Krishna,
