Re: [PATCH RFC] soc: qcom: pmic_glink: Fix device access from worker during suspend

[Krzysztof Kozlowski <krzk@kernel.org>]

On 29/01/2025 09:46, Abel Vesa wrote:
>>
>>> 1. 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?
>>>
>>> 2. 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?
>>>
>>> 3. So qcom_battmgr.c is buggy as well?
>>>
>>> 4. 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