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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <8ac9c98a-5184-4b5a-93d6-a30ba6f20696@quicinc.com>
Date: Thu, 28 Aug 2025 10:43:01 +0530
From: Praveen Talari <quic_ptalari@...cinc.com>
To: Alexey Klimov <alexey.klimov@...aro.org>,
        Krzysztof Kozlowski
	<krzk@...nel.org>
CC: Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio
	<konradybcio@...nel.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-serial@...r.kernel.org>,
        Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>,
        <linux-kernel@...r.kernel.org>, <dmitry.baryshkov@....qualcomm.com>,
        <psodagud@...cinc.com>, <djaggi@...cinc.com>,
        <quic_msavaliy@...cinc.com>, <quic_vtanuku@...cinc.com>,
        <quic_arandive@...cinc.com>, <quic_cchiluve@...cinc.com>,
        <quic_shazhuss@...cinc.com>, Jiri Slaby
	<jirislaby@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski
	<krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, <devicetree@...r.kernel.org>,
        <bryan.odonoghue@...aro.org>, <neil.armstrong@...aro.org>,
        <srini@...nel.org>
Subject: Re: [PATCH v7 7/8] serial: qcom-geni: Enable PM runtime for serial
 driver

HI Alexey,

On 8/27/2025 1:24 AM, Alexey Klimov wrote:
> On Tue Aug 26, 2025 at 11:29 AM BST, Praveen Talari wrote:
>> 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
> 
> Was the original patch, that introduced the regression, also created by AI tools?
> Just trying to understand how we ended up with untested commit in -master.
> 
> Did you test the change below on real hardware?

Yes, i have tested on SA7280 SOC.

Thanks,
Praveen Talari
> 
> 
>> 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;
>>    }
> 
> Best regards,
> Alexey
>