Re: [PATCH v7 7/8] serial: qcom-geni: Enable PM runtime for serial driver

Praveen Talari
2025-08-26T10:29:15Z

Hi Alexey/Krzysztof,

On 8/26/2025 3:36 PM, Krzysztof Kozlowski wrote:
> On 26/08/2025 11:37, Alexey Klimov wrote:
>> On Tue Aug 26, 2025 at 10:21 AM BST, Krzysztof Kozlowski wrote:
>>> On 26/08/2025 11:18, Alexey Klimov wrote:
>>>>>> May i know what is testcase which you are running on target?
>>>>>
>>>>> Boot the board?
>>>>>
>>>>>> what is target?
>>>>>
>>>>> It is written in original report. Did you even read it?
>>>>>
>>>>>> Which usecase is this issue occurring in?
>>>>>
>>>>> Boot?
>>>>
>>>> FWIW, what said above by Krzysztof is correct, there is no usecase, just booting the board.
>>>>
>>> 12 days and nothing improved, right? if this was not dropped now,
>>> Alexey, can you send a revert? Author clearly approches stability with a
>>> very relaxed way and is just happy that patch was thrown over the wall
>>> and job is done.
>>>
>>>
>>> If you do not want to send revert, let me know, I will do it.
>>
>> I am okay with sending revert, just trying to see if there is any interest
>> in fixing this.
>
> Any interest should have happened after 1 day of reporting linux-next
> breakage. It has been like what? 12 days?
>
> That's typical throw the patch over the wall. Revert.

Really sorry for the delay.

I forgot to mention earlier that I’ve been actively investigating this
issue across different platform SoCs. I was able to reproduce the
problem on the SC7280.

Here’s a summary of the observed behavior:

The issue appears to originate from the qcom_geni_serial driver during
device runtime resume. It results in a blocked IRQ thread, which in turn
causes system instability.

The call trace suggests a deadlock scenario where the IRQ
thread—responsible for handling wake-up events—becomes unresponsive
while interacting with the pinctrl subsystem.

Specifically, the msm_pinmux_set_mux function attempts to invoke
disable_irq, which is problematic when called from an IRQ thread context.
Since the IRQ itself is a wake-up source, this leads to contention or a
self-deadlock situation.

I have verified below diff and about to post it

diff --git a/drivers/tty/serial/qcom_geni_serial.c
b/drivers/tty/serial/qcom_geni_serial.c
index c9c52c52a98d..cb3b4febd8c2 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1848,16 +1848,36 @@ static int __maybe_unused
qcom_geni_serial_runtime_suspend(struct device *dev)
{
struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
struct uart_port *uport = &port->uport;
+ int ret;
+
+ ret = geni_serial_resources_off(uport);
+ if(ret) {
+ if (device_may_wakeup(dev))
+ disable_irq_wake(port->wakeup_irq);
+ }

- return geni_serial_resources_off(uport);
+ if (device_may_wakeup(dev))
+ enable_irq_wake(port->wakeup_irq);
+
+ return ret;
}

static int __maybe_unused qcom_geni_serial_runtime_resume(struct
device *dev)
{
struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
struct uart_port *uport = &port->uport;
+ int ret;
+
+ if (device_may_wakeup(dev))
+ disable_irq_wake(port->wakeup_irq);

- return geni_serial_resources_on(uport);
+ ret = geni_serial_resources_on(uport);
+ if(ret) {
+ if (device_may_wakeup(dev))
+ enable_irq_wake(port->wakeup_irq);
+ }
+
+ return ret;
}

Thanks,
Praveen Talari

> Best regards,
> Krzysztof