On 29/01/2025 09:46, Abel Vesa wrote:
- Maybe the driver just lacks proper suspend/resume handling?
I assume all this happens during system suspend, so what certainty you have that your second work - pmic_glink_altmode_pdr_notify() - is not executed as well?
- Follow up: all other drivers and all other future use cases will be
affected as well. Basically what this patch is admitting is that driver can be executed anytime, even during suspend, so each call of pmic_glink_send() has to be audited. Now and in the future, because what stops some developer of adding one more path calling pmic_glink_send(), which also turns out to be executed during suspend?
So qcom_battmgr.c is buggy as well?
ucsi_glink.c? I don't see handling suspend, either...
Maybe the entire problem is how pmic glink was designed: not as proper bus driver which handles both child-parent relationship and system suspend.
The underlying problem is that GLINK register its interrupt as IRQF_NO_SUSPEND (for historical reasons) and as such incoming messages will be delivered in late suspend and early resume. In this specific case, a specific message is handled by pmic_glink_altmode_callback(), by invoking schedule_work() which in this case happens to schedule pmic_glink_altmode_worker before we've resumed the I2C controller.
I presume with your suggestion about a pmic_glink bus driver we'd come up with some mechanism for pmic_glink to defer these messages until resume has happened?
So is the suggestion here to rework the entire pmic_glink into a bus driver? (I like the sound of that)
I'd assume the new bus driver will have to queue the messages until it has resumed, which is fine.
Queue or just disable interrupts/notifications to clients.
But still doesn't solve the fact that we can't filter out when to wake-up or not. What am I missing here?
I think this was not the concern in my email. I was only wondering about the design flaw that we allow pmic glink to send notifications to children anytime. And that's not how the bus-like driver should be written.
Best regards, Krzysztof