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: <DCCMFVC0DW1I.GXZVG2BQEFX7@linaro.org>
Date: Tue, 26 Aug 2025 20:54:38 +0100
From: "Alexey Klimov" <alexey.klimov@...aro.org>
To: "Praveen Talari" <quic_ptalari@...cinc.com>, "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

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?


> 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