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

Praveen Talari posted 8 patches 2 months, 2 weeks ago
[PATCH v7 7/8] serial: qcom-geni: Enable PM runtime for serial driver
Posted by Praveen Talari 2 months, 2 weeks ago
The GENI serial driver currently handles power resource management
through calls to the statically defined geni_serial_resources_on() and
geni_serial_resources_off() functions. This approach reduces modularity
and limits support for platforms with diverse power management
mechanisms, including resource managed by firmware.

Improve modularity and enable better integration with platform-specific
power management, introduce support for runtime PM. Use
pm_runtime_resume_and_get() and pm_runtime_put_sync() within the
qcom_geni_serial_pm() callback to control resource power state
transitions based on UART power state changes.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
---
v6 -> v7
From Bjorn:
- used devm_pm_runtime_enable() instead of pm_runtime_enable()
- updated commit text.

v5 -> v6
- added reviewed-by tag in commit
- added __maybe_unused to PM callback functions to avoid
  warnings of defined but not used
---
 drivers/tty/serial/qcom_geni_serial.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 81f385d900d0..aa08de659e34 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1713,10 +1713,10 @@ static void qcom_geni_serial_pm(struct uart_port *uport,
 		old_state = UART_PM_STATE_OFF;
 
 	if (new_state == UART_PM_STATE_ON && old_state == UART_PM_STATE_OFF)
-		geni_serial_resources_on(uport);
+		pm_runtime_resume_and_get(uport->dev);
 	else if (new_state == UART_PM_STATE_OFF &&
 		 old_state == UART_PM_STATE_ON)
-		geni_serial_resources_off(uport);
+		pm_runtime_put_sync(uport->dev);
 
 }
 
@@ -1878,6 +1878,8 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	devm_pm_runtime_enable(port->se.dev);
+
 	ret = uart_add_one_port(drv, uport);
 	if (ret)
 		return ret;
@@ -1909,6 +1911,22 @@ static void qcom_geni_serial_remove(struct platform_device *pdev)
 	uart_remove_one_port(drv, &port->uport);
 }
 
+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;
+
+	return geni_serial_resources_off(uport);
+}
+
+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;
+
+	return geni_serial_resources_on(uport);
+}
+
 static int qcom_geni_serial_suspend(struct device *dev)
 {
 	struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
@@ -1952,6 +1970,8 @@ static const struct qcom_geni_device_data qcom_geni_uart_data = {
 };
 
 static const struct dev_pm_ops qcom_geni_serial_pm_ops = {
+	SET_RUNTIME_PM_OPS(qcom_geni_serial_runtime_suspend,
+			   qcom_geni_serial_runtime_resume, NULL)
 	SYSTEM_SLEEP_PM_OPS(qcom_geni_serial_suspend, qcom_geni_serial_resume)
 };
 
-- 
2.17.1
Re: [PATCH v7 7/8] serial: qcom-geni: Enable PM runtime for serial driver
Posted by Alexey Klimov 1 month, 3 weeks ago
(c/c Neil and Srini)

On Mon Jul 21, 2025 at 6:45 PM BST, Praveen Talari wrote:
> The GENI serial driver currently handles power resource management
> through calls to the statically defined geni_serial_resources_on() and
> geni_serial_resources_off() functions. This approach reduces modularity
> and limits support for platforms with diverse power management
> mechanisms, including resource managed by firmware.
>
> Improve modularity and enable better integration with platform-specific
> power management, introduce support for runtime PM. Use
> pm_runtime_resume_and_get() and pm_runtime_put_sync() within the
> qcom_geni_serial_pm() callback to control resource power state
> transitions based on UART power state changes.
>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>


This breaks at least RB1 (QRB2210), maybe others.
Currently broken on -master and on linux-next.

Upon login prompt random parts of kernel seems to be off/failed and
debugging led to udev being stuck:

[   85.369834] INFO: task kworker/u16:0:12 blocked for more than 42 seconds.
[   85.376699]       Not tainted 6.17.0-rc1-00004-g53e760d89498 #9
[   85.382660] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[   85.390547] task:kworker/u16:0   state:D stack:0     pid:12    tgid:12    ppid:2      task_flags:0x4208060 flags:0x00000010
[   85.401748] Workqueue: async async_run_entry_fn
[   85.406349] Call trace:
[   85.408828]  __switch_to+0xe8/0x1a0 (T)
[   85.412724]  __schedule+0x290/0x7c0
[   85.416275]  schedule+0x34/0x118
[   85.419554]  rpm_resume+0x14c/0x66c
[   85.423111]  rpm_resume+0x2a4/0x66c
[   85.426647]  rpm_resume+0x2a4/0x66c
[   85.430188]  rpm_resume+0x2a4/0x66c
[   85.433722]  __pm_runtime_resume+0x50/0x9c
[   85.437869]  __driver_probe_device+0x58/0x120
[   85.442287]  driver_probe_device+0x3c/0x154
[   85.446523]  __driver_attach_async_helper+0x4c/0xc0
[   85.451446]  async_run_entry_fn+0x34/0xe0
[   85.455504]  process_one_work+0x148/0x290
[   85.459565]  worker_thread+0x2c4/0x3e0
[   85.463368]  kthread+0x118/0x1c0
[   85.466651]  ret_from_fork+0x10/0x20
[   85.470337] INFO: task irq/92-4a8c000.:71 blocked for more than 42 seconds.
[   85.477351]       Not tainted 6.17.0-rc1-00004-g53e760d89498 #9
[   85.483323] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[   85.491195] task:irq/92-4a8c000. state:D stack:0     pid:71    tgid:71    ppid:2      task_flags:0x208040 flags:0x00000010
[   85.502290] Call trace:
[   85.504786]  __switch_to+0xe8/0x1a0 (T)
[   85.508687]  __schedule+0x290/0x7c0
[   85.512231]  schedule+0x34/0x118
[   85.515504]  __synchronize_irq+0x60/0xa0
[   85.519483]  disable_irq+0x3c/0x4c
[   85.522929]  msm_pinmux_set_mux+0x3a8/0x44c
[   85.527167]  pinmux_enable_setting+0x1c4/0x28c
[   85.531665]  pinctrl_commit_state+0xa0/0x260
[   85.535989]  pinctrl_pm_select_default_state+0x4c/0xa0
[   85.541182]  geni_se_resources_on+0xd0/0x15c
[   85.545522]  geni_serial_resource_state+0x8c/0xbc
[   85.550282]  qcom_geni_serial_runtime_resume+0x24/0x3c
[   85.555470]  pm_generic_runtime_resume+0x2c/0x44
[   85.560139]  __rpm_callback+0x48/0x1e0
[   85.563949]  rpm_callback+0x74/0x80
[   85.567494]  rpm_resume+0x39c/0x66c
[   85.571040]  __pm_runtime_resume+0x50/0x9c
[   85.575193]  handle_threaded_wake_irq+0x30/0x80
[   85.579771]  irq_thread_fn+0x2c/0xb0
[   85.583443]  irq_thread+0x16c/0x278
[   85.587003]  kthread+0x118/0x1c0
[   85.590283]  ret_from_fork+0x10/0x20
[   85.593943] INFO: task (udev-worker):228 blocked for more than 42 seconds.
[   85.600873]       Not tainted 6.17.0-rc1-00004-g53e760d89498 #9
[   85.606846] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[   85.614717] task:(udev-worker)   state:D stack:0     pid:228   tgid:228   ppid:222    task_flags:0x400140 flags:0x00000818
[   85.625823] Call trace:
[   85.628316]  __switch_to+0xe8/0x1a0 (T)
[   85.632217]  __schedule+0x290/0x7c0
[   85.635765]  schedule+0x34/0x118
[   85.639044]  async_synchronize_cookie_domain.part.0+0x50/0xa4
[   85.644854]  async_synchronize_full+0x78/0xa0
[   85.649270]  do_init_module+0x190/0x23c
[   85.653154]  load_module+0x1708/0x1ca0
[   85.656952]  init_module_from_file+0x74/0xa0
[   85.661273]  __arm64_sys_finit_module+0x130/0x2f8
[   85.666023]  invoke_syscall+0x48/0x104
[   85.669842]  el0_svc_common.constprop.0+0xc0/0xe0
[   85.674604]  do_el0_svc+0x1c/0x28
[   85.677973]  el0_svc+0x2c/0x84
[   85.681078]  el0t_64_sync_handler+0xa0/0xe4
[   85.685316]  el0t_64_sync+0x198/0x19c
[   85.689032] INFO: task (udev-worker):229 blocked for more than 42 seconds.


Usually wifi, all remoteprocs and anything that depends on lpass/pinctrl fail to probe.

Reverting these:
86fa39dd6fb7 serial: qcom-geni: Enable Serial on SA8255p Qualcomm platforms
1afa70632c39 serial: qcom-geni: Enable PM runtime for serial driver

resolves the regression. Couldn't say if we should go with reverting since 86fa39dd6fb7
adds support of serial on SA8255p and for clean revert both have to be reverted.

Any thoughts?

Best regards,
Alexey





> ---
> v6 -> v7
> From Bjorn:
> - used devm_pm_runtime_enable() instead of pm_runtime_enable()
> - updated commit text.
>
> v5 -> v6
> - added reviewed-by tag in commit
> - added __maybe_unused to PM callback functions to avoid
>   warnings of defined but not used
> ---
>  drivers/tty/serial/qcom_geni_serial.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 81f385d900d0..aa08de659e34 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -1713,10 +1713,10 @@ static void qcom_geni_serial_pm(struct uart_port *uport,
>  		old_state = UART_PM_STATE_OFF;
>  
>  	if (new_state == UART_PM_STATE_ON && old_state == UART_PM_STATE_OFF)
> -		geni_serial_resources_on(uport);
> +		pm_runtime_resume_and_get(uport->dev);
>  	else if (new_state == UART_PM_STATE_OFF &&
>  		 old_state == UART_PM_STATE_ON)
> -		geni_serial_resources_off(uport);
> +		pm_runtime_put_sync(uport->dev);
>  
>  }
>  
> @@ -1878,6 +1878,8 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	devm_pm_runtime_enable(port->se.dev);
> +
>  	ret = uart_add_one_port(drv, uport);
>  	if (ret)
>  		return ret;
> @@ -1909,6 +1911,22 @@ static void qcom_geni_serial_remove(struct platform_device *pdev)
>  	uart_remove_one_port(drv, &port->uport);
>  }
>  
> +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;
> +
> +	return geni_serial_resources_off(uport);
> +}
> +
> +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;
> +
> +	return geni_serial_resources_on(uport);
> +}
> +
>  static int qcom_geni_serial_suspend(struct device *dev)
>  {
>  	struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
> @@ -1952,6 +1970,8 @@ static const struct qcom_geni_device_data qcom_geni_uart_data = {
>  };
>  
>  static const struct dev_pm_ops qcom_geni_serial_pm_ops = {
> +	SET_RUNTIME_PM_OPS(qcom_geni_serial_runtime_suspend,
> +			   qcom_geni_serial_runtime_resume, NULL)
>  	SYSTEM_SLEEP_PM_OPS(qcom_geni_serial_suspend, qcom_geni_serial_resume)
>  };
>  
Re: [PATCH v7 7/8] serial: qcom-geni: Enable PM runtime for serial driver
Posted by Praveen Talari 1 month, 2 weeks ago
Hi Alexey.

Thank you for your patience,

On 8/12/2025 3:35 PM, Alexey Klimov wrote:
> (c/c Neil and Srini)
> 
> On Mon Jul 21, 2025 at 6:45 PM BST, Praveen Talari wrote:
>> The GENI serial driver currently handles power resource management
>> through calls to the statically defined geni_serial_resources_on() and
>> geni_serial_resources_off() functions. This approach reduces modularity
>> and limits support for platforms with diverse power management
>> mechanisms, including resource managed by firmware.
>>
>> Improve modularity and enable better integration with platform-specific
>> power management, introduce support for runtime PM. Use
>> pm_runtime_resume_and_get() and pm_runtime_put_sync() within the
>> qcom_geni_serial_pm() callback to control resource power state
>> transitions based on UART power state changes.
>>
>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
> 
> 
> This breaks at least RB1 (QRB2210), maybe others.
> Currently broken on -master and on linux-next.
> 
> Upon login prompt random parts of kernel seems to be off/failed and
> debugging led to udev being stuck:
> 
> [   85.369834] INFO: task kworker/u16:0:12 blocked for more than 42 seconds.
> [   85.376699]       Not tainted 6.17.0-rc1-00004-g53e760d89498 #9
> [   85.382660] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [   85.390547] task:kworker/u16:0   state:D stack:0     pid:12    tgid:12    ppid:2      task_flags:0x4208060 flags:0x00000010
> [   85.401748] Workqueue: async async_run_entry_fn
> [   85.406349] Call trace:
> [   85.408828]  __switch_to+0xe8/0x1a0 (T)
> [   85.412724]  __schedule+0x290/0x7c0
> [   85.416275]  schedule+0x34/0x118
> [   85.419554]  rpm_resume+0x14c/0x66c
> [   85.423111]  rpm_resume+0x2a4/0x66c
> [   85.426647]  rpm_resume+0x2a4/0x66c
> [   85.430188]  rpm_resume+0x2a4/0x66c
> [   85.433722]  __pm_runtime_resume+0x50/0x9c
> [   85.437869]  __driver_probe_device+0x58/0x120
> [   85.442287]  driver_probe_device+0x3c/0x154
> [   85.446523]  __driver_attach_async_helper+0x4c/0xc0
> [   85.451446]  async_run_entry_fn+0x34/0xe0
> [   85.455504]  process_one_work+0x148/0x290
> [   85.459565]  worker_thread+0x2c4/0x3e0
> [   85.463368]  kthread+0x118/0x1c0
> [   85.466651]  ret_from_fork+0x10/0x20
> [   85.470337] INFO: task irq/92-4a8c000.:71 blocked for more than 42 seconds.
> [   85.477351]       Not tainted 6.17.0-rc1-00004-g53e760d89498 #9
> [   85.483323] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [   85.491195] task:irq/92-4a8c000. state:D stack:0     pid:71    tgid:71    ppid:2      task_flags:0x208040 flags:0x00000010
> [   85.502290] Call trace:
> [   85.504786]  __switch_to+0xe8/0x1a0 (T)
> [   85.508687]  __schedule+0x290/0x7c0
> [   85.512231]  schedule+0x34/0x118
> [   85.515504]  __synchronize_irq+0x60/0xa0
> [   85.519483]  disable_irq+0x3c/0x4c
> [   85.522929]  msm_pinmux_set_mux+0x3a8/0x44c
> [   85.527167]  pinmux_enable_setting+0x1c4/0x28c
> [   85.531665]  pinctrl_commit_state+0xa0/0x260
> [   85.535989]  pinctrl_pm_select_default_state+0x4c/0xa0
> [   85.541182]  geni_se_resources_on+0xd0/0x15c
> [   85.545522]  geni_serial_resource_state+0x8c/0xbc
> [   85.550282]  qcom_geni_serial_runtime_resume+0x24/0x3c
> [   85.555470]  pm_generic_runtime_resume+0x2c/0x44
> [   85.560139]  __rpm_callback+0x48/0x1e0
> [   85.563949]  rpm_callback+0x74/0x80
> [   85.567494]  rpm_resume+0x39c/0x66c
> [   85.571040]  __pm_runtime_resume+0x50/0x9c
> [   85.575193]  handle_threaded_wake_irq+0x30/0x80
> [   85.579771]  irq_thread_fn+0x2c/0xb0
> [   85.583443]  irq_thread+0x16c/0x278
> [   85.587003]  kthread+0x118/0x1c0
> [   85.590283]  ret_from_fork+0x10/0x20
> [   85.593943] INFO: task (udev-worker):228 blocked for more than 42 seconds.
> [   85.600873]       Not tainted 6.17.0-rc1-00004-g53e760d89498 #9
> [   85.606846] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [   85.614717] task:(udev-worker)   state:D stack:0     pid:228   tgid:228   ppid:222    task_flags:0x400140 flags:0x00000818
> [   85.625823] Call trace:
> [   85.628316]  __switch_to+0xe8/0x1a0 (T)
> [   85.632217]  __schedule+0x290/0x7c0
> [   85.635765]  schedule+0x34/0x118
> [   85.639044]  async_synchronize_cookie_domain.part.0+0x50/0xa4
> [   85.644854]  async_synchronize_full+0x78/0xa0
> [   85.649270]  do_init_module+0x190/0x23c
> [   85.653154]  load_module+0x1708/0x1ca0
> [   85.656952]  init_module_from_file+0x74/0xa0
> [   85.661273]  __arm64_sys_finit_module+0x130/0x2f8
> [   85.666023]  invoke_syscall+0x48/0x104
> [   85.669842]  el0_svc_common.constprop.0+0xc0/0xe0
> [   85.674604]  do_el0_svc+0x1c/0x28
> [   85.677973]  el0_svc+0x2c/0x84
> [   85.681078]  el0t_64_sync_handler+0xa0/0xe4
> [   85.685316]  el0t_64_sync+0x198/0x19c
> [   85.689032] INFO: task (udev-worker):229 blocked for more than 42 seconds.
> 
> 
> Usually wifi, all remoteprocs and anything that depends on lpass/pinctrl fail to probe.

May i know what is testcase which you are running on target?
what is target?
Which usecase is this issue occurring in?

Thanks,
Praveen Talari
> 
> Reverting these:
> 86fa39dd6fb7 serial: qcom-geni: Enable Serial on SA8255p Qualcomm platforms
> 1afa70632c39 serial: qcom-geni: Enable PM runtime for serial driver
> 
> resolves the regression. Couldn't say if we should go with reverting since 86fa39dd6fb7
> adds support of serial on SA8255p and for clean revert both have to be reverted.
> 
> Any thoughts?
> 
> Best regards,
> Alexey
> 
> 
> 
> 
> 
>> ---
>> v6 -> v7
>>  From Bjorn:
>> - used devm_pm_runtime_enable() instead of pm_runtime_enable()
>> - updated commit text.
>>
>> v5 -> v6
>> - added reviewed-by tag in commit
>> - added __maybe_unused to PM callback functions to avoid
>>    warnings of defined but not used
>> ---
>>   drivers/tty/serial/qcom_geni_serial.c | 24 ++++++++++++++++++++++--
>>   1 file changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
>> index 81f385d900d0..aa08de659e34 100644
>> --- a/drivers/tty/serial/qcom_geni_serial.c
>> +++ b/drivers/tty/serial/qcom_geni_serial.c
>> @@ -1713,10 +1713,10 @@ static void qcom_geni_serial_pm(struct uart_port *uport,
>>   		old_state = UART_PM_STATE_OFF;
>>   
>>   	if (new_state == UART_PM_STATE_ON && old_state == UART_PM_STATE_OFF)
>> -		geni_serial_resources_on(uport);
>> +		pm_runtime_resume_and_get(uport->dev);
>>   	else if (new_state == UART_PM_STATE_OFF &&
>>   		 old_state == UART_PM_STATE_ON)
>> -		geni_serial_resources_off(uport);
>> +		pm_runtime_put_sync(uport->dev);
>>   
>>   }
>>   
>> @@ -1878,6 +1878,8 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		return ret;
>>   
>> +	devm_pm_runtime_enable(port->se.dev);
>> +
>>   	ret = uart_add_one_port(drv, uport);
>>   	if (ret)
>>   		return ret;
>> @@ -1909,6 +1911,22 @@ static void qcom_geni_serial_remove(struct platform_device *pdev)
>>   	uart_remove_one_port(drv, &port->uport);
>>   }
>>   
>> +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;
>> +
>> +	return geni_serial_resources_off(uport);
>> +}
>> +
>> +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;
>> +
>> +	return geni_serial_resources_on(uport);
>> +}
>> +
>>   static int qcom_geni_serial_suspend(struct device *dev)
>>   {
>>   	struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
>> @@ -1952,6 +1970,8 @@ static const struct qcom_geni_device_data qcom_geni_uart_data = {
>>   };
>>   
>>   static const struct dev_pm_ops qcom_geni_serial_pm_ops = {
>> +	SET_RUNTIME_PM_OPS(qcom_geni_serial_runtime_suspend,
>> +			   qcom_geni_serial_runtime_resume, NULL)
>>   	SYSTEM_SLEEP_PM_OPS(qcom_geni_serial_suspend, qcom_geni_serial_resume)
>>   };
>>   
>
Re: [PATCH v7 7/8] serial: qcom-geni: Enable PM runtime for serial driver
Posted by Krzysztof Kozlowski 1 month, 2 weeks ago
On 19/08/2025 08:50, Praveen Talari wrote:
> Hi Alexey.
> 
> Thank you for your patience,
> 
> On 8/12/2025 3:35 PM, Alexey Klimov wrote:
>> (c/c Neil and Srini)
>>
>> On Mon Jul 21, 2025 at 6:45 PM BST, Praveen Talari wrote:
>>> The GENI serial driver currently handles power resource management
>>> through calls to the statically defined geni_serial_resources_on() and
>>> geni_serial_resources_off() functions. This approach reduces modularity
>>> and limits support for platforms with diverse power management
>>> mechanisms, including resource managed by firmware.
>>>
>>> Improve modularity and enable better integration with platform-specific
>>> power management, introduce support for runtime PM. Use
>>> pm_runtime_resume_and_get() and pm_runtime_put_sync() within the
>>> qcom_geni_serial_pm() callback to control resource power state
>>> transitions based on UART power state changes.
>>>
>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>> Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
>>
>>
>> This breaks at least RB1 (QRB2210), maybe others.
>> Currently broken on -master and on linux-next.
>>
>> Upon login prompt random parts of kernel seems to be off/failed and
>> debugging led to udev being stuck:
>>
>> [   85.369834] INFO: task kworker/u16:0:12 blocked for more than 42 seconds.
>> [   85.376699]       Not tainted 6.17.0-rc1-00004-g53e760d89498 #9
>> [   85.382660] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>> [   85.390547] task:kworker/u16:0   state:D stack:0     pid:12    tgid:12    ppid:2      task_flags:0x4208060 flags:0x00000010
>> [   85.401748] Workqueue: async async_run_entry_fn
>> [   85.406349] Call trace:
>> [   85.408828]  __switch_to+0xe8/0x1a0 (T)
>> [   85.412724]  __schedule+0x290/0x7c0
>> [   85.416275]  schedule+0x34/0x118
>> [   85.419554]  rpm_resume+0x14c/0x66c
>> [   85.423111]  rpm_resume+0x2a4/0x66c
>> [   85.426647]  rpm_resume+0x2a4/0x66c
>> [   85.430188]  rpm_resume+0x2a4/0x66c
>> [   85.433722]  __pm_runtime_resume+0x50/0x9c
>> [   85.437869]  __driver_probe_device+0x58/0x120
>> [   85.442287]  driver_probe_device+0x3c/0x154
>> [   85.446523]  __driver_attach_async_helper+0x4c/0xc0
>> [   85.451446]  async_run_entry_fn+0x34/0xe0
>> [   85.455504]  process_one_work+0x148/0x290
>> [   85.459565]  worker_thread+0x2c4/0x3e0
>> [   85.463368]  kthread+0x118/0x1c0
>> [   85.466651]  ret_from_fork+0x10/0x20
>> [   85.470337] INFO: task irq/92-4a8c000.:71 blocked for more than 42 seconds.
>> [   85.477351]       Not tainted 6.17.0-rc1-00004-g53e760d89498 #9
>> [   85.483323] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>> [   85.491195] task:irq/92-4a8c000. state:D stack:0     pid:71    tgid:71    ppid:2      task_flags:0x208040 flags:0x00000010
>> [   85.502290] Call trace:
>> [   85.504786]  __switch_to+0xe8/0x1a0 (T)
>> [   85.508687]  __schedule+0x290/0x7c0
>> [   85.512231]  schedule+0x34/0x118
>> [   85.515504]  __synchronize_irq+0x60/0xa0
>> [   85.519483]  disable_irq+0x3c/0x4c
>> [   85.522929]  msm_pinmux_set_mux+0x3a8/0x44c
>> [   85.527167]  pinmux_enable_setting+0x1c4/0x28c
>> [   85.531665]  pinctrl_commit_state+0xa0/0x260
>> [   85.535989]  pinctrl_pm_select_default_state+0x4c/0xa0
>> [   85.541182]  geni_se_resources_on+0xd0/0x15c
>> [   85.545522]  geni_serial_resource_state+0x8c/0xbc
>> [   85.550282]  qcom_geni_serial_runtime_resume+0x24/0x3c
>> [   85.555470]  pm_generic_runtime_resume+0x2c/0x44
>> [   85.560139]  __rpm_callback+0x48/0x1e0
>> [   85.563949]  rpm_callback+0x74/0x80
>> [   85.567494]  rpm_resume+0x39c/0x66c
>> [   85.571040]  __pm_runtime_resume+0x50/0x9c
>> [   85.575193]  handle_threaded_wake_irq+0x30/0x80
>> [   85.579771]  irq_thread_fn+0x2c/0xb0
>> [   85.583443]  irq_thread+0x16c/0x278
>> [   85.587003]  kthread+0x118/0x1c0
>> [   85.590283]  ret_from_fork+0x10/0x20
>> [   85.593943] INFO: task (udev-worker):228 blocked for more than 42 seconds.
>> [   85.600873]       Not tainted 6.17.0-rc1-00004-g53e760d89498 #9
>> [   85.606846] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>> [   85.614717] task:(udev-worker)   state:D stack:0     pid:228   tgid:228   ppid:222    task_flags:0x400140 flags:0x00000818
>> [   85.625823] Call trace:
>> [   85.628316]  __switch_to+0xe8/0x1a0 (T)
>> [   85.632217]  __schedule+0x290/0x7c0
>> [   85.635765]  schedule+0x34/0x118
>> [   85.639044]  async_synchronize_cookie_domain.part.0+0x50/0xa4
>> [   85.644854]  async_synchronize_full+0x78/0xa0
>> [   85.649270]  do_init_module+0x190/0x23c
>> [   85.653154]  load_module+0x1708/0x1ca0
>> [   85.656952]  init_module_from_file+0x74/0xa0
>> [   85.661273]  __arm64_sys_finit_module+0x130/0x2f8
>> [   85.666023]  invoke_syscall+0x48/0x104
>> [   85.669842]  el0_svc_common.constprop.0+0xc0/0xe0
>> [   85.674604]  do_el0_svc+0x1c/0x28
>> [   85.677973]  el0_svc+0x2c/0x84
>> [   85.681078]  el0t_64_sync_handler+0xa0/0xe4
>> [   85.685316]  el0t_64_sync+0x198/0x19c
>> [   85.689032] INFO: task (udev-worker):229 blocked for more than 42 seconds.
>>
>>
>> Usually wifi, all remoteprocs and anything that depends on lpass/pinctrl fail to probe.
> 
> 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?

Best regards,
Krzysztof
Re: [PATCH v7 7/8] serial: qcom-geni: Enable PM runtime for serial driver
Posted by Alexey Klimov 1 month, 1 week ago
Hi Praveen,

Hi Praveen,

On Tue Aug 19, 2025 at 8:16 AM BST, Krzysztof Kozlowski wrote:
> On 19/08/2025 08:50, Praveen Talari wrote:
>> Hi Alexey.
>> 
>> Thank you for your patience,

Well, any update on this?



>> On 8/12/2025 3:35 PM, Alexey Klimov wrote:
>>> (c/c Neil and Srini)
>>>
>>> On Mon Jul 21, 2025 at 6:45 PM BST, Praveen Talari wrote:
>>>> The GENI serial driver currently handles power resource management
>>>> through calls to the statically defined geni_serial_resources_on() and
>>>> geni_serial_resources_off() functions. This approach reduces modularity
>>>> and limits support for platforms with diverse power management
>>>> mechanisms, including resource managed by firmware.
>>>>
>>>> Improve modularity and enable better integration with platform-specific
>>>> power management, introduce support for runtime PM. Use
>>>> pm_runtime_resume_and_get() and pm_runtime_put_sync() within the
>>>> qcom_geni_serial_pm() callback to control resource power state
>>>> transitions based on UART power state changes.
>>>>
>>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>>> Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
>>>
>>>
>>> This breaks at least RB1 (QRB2210), maybe others.
>>> Currently broken on -master and on linux-next.
>>>
>>> Upon login prompt random parts of kernel seems to be off/failed and
>>> debugging led to udev being stuck:
>>>
>>> [   85.369834] INFO: task kworker/u16:0:12 blocked for more than 42 seconds.
>>> [   85.376699]       Not tainted 6.17.0-rc1-00004-g53e760d89498 #9
>>> [   85.382660] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>>> [   85.390547] task:kworker/u16:0   state:D stack:0     pid:12    tgid:12    ppid:2      task_flags:0x4208060 flags:0x00000010
>>> [   85.401748] Workqueue: async async_run_entry_fn
>>> [   85.406349] Call trace:
>>> [   85.408828]  __switch_to+0xe8/0x1a0 (T)
>>> [   85.412724]  __schedule+0x290/0x7c0
>>> [   85.416275]  schedule+0x34/0x118
>>> [   85.419554]  rpm_resume+0x14c/0x66c
>>> [   85.423111]  rpm_resume+0x2a4/0x66c
>>> [   85.426647]  rpm_resume+0x2a4/0x66c
>>> [   85.430188]  rpm_resume+0x2a4/0x66c
>>> [   85.433722]  __pm_runtime_resume+0x50/0x9c
>>> [   85.437869]  __driver_probe_device+0x58/0x120
>>> [   85.442287]  driver_probe_device+0x3c/0x154
>>> [   85.446523]  __driver_attach_async_helper+0x4c/0xc0
>>> [   85.451446]  async_run_entry_fn+0x34/0xe0
>>> [   85.455504]  process_one_work+0x148/0x290
>>> [   85.459565]  worker_thread+0x2c4/0x3e0
>>> [   85.463368]  kthread+0x118/0x1c0
>>> [   85.466651]  ret_from_fork+0x10/0x20
>>> [   85.470337] INFO: task irq/92-4a8c000.:71 blocked for more than 42 seconds.
>>> [   85.477351]       Not tainted 6.17.0-rc1-00004-g53e760d89498 #9
>>> [   85.483323] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>>> [   85.491195] task:irq/92-4a8c000. state:D stack:0     pid:71    tgid:71    ppid:2      task_flags:0x208040 flags:0x00000010
>>> [   85.502290] Call trace:
>>> [   85.504786]  __switch_to+0xe8/0x1a0 (T)
>>> [   85.508687]  __schedule+0x290/0x7c0
>>> [   85.512231]  schedule+0x34/0x118
>>> [   85.515504]  __synchronize_irq+0x60/0xa0
>>> [   85.519483]  disable_irq+0x3c/0x4c
>>> [   85.522929]  msm_pinmux_set_mux+0x3a8/0x44c
>>> [   85.527167]  pinmux_enable_setting+0x1c4/0x28c
>>> [   85.531665]  pinctrl_commit_state+0xa0/0x260
>>> [   85.535989]  pinctrl_pm_select_default_state+0x4c/0xa0
>>> [   85.541182]  geni_se_resources_on+0xd0/0x15c
>>> [   85.545522]  geni_serial_resource_state+0x8c/0xbc
>>> [   85.550282]  qcom_geni_serial_runtime_resume+0x24/0x3c
>>> [   85.555470]  pm_generic_runtime_resume+0x2c/0x44
>>> [   85.560139]  __rpm_callback+0x48/0x1e0
>>> [   85.563949]  rpm_callback+0x74/0x80
>>> [   85.567494]  rpm_resume+0x39c/0x66c
>>> [   85.571040]  __pm_runtime_resume+0x50/0x9c
>>> [   85.575193]  handle_threaded_wake_irq+0x30/0x80
>>> [   85.579771]  irq_thread_fn+0x2c/0xb0
>>> [   85.583443]  irq_thread+0x16c/0x278
>>> [   85.587003]  kthread+0x118/0x1c0
>>> [   85.590283]  ret_from_fork+0x10/0x20
>>> [   85.593943] INFO: task (udev-worker):228 blocked for more than 42 seconds.
>>> [   85.600873]       Not tainted 6.17.0-rc1-00004-g53e760d89498 #9
>>> [   85.606846] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>>> [   85.614717] task:(udev-worker)   state:D stack:0     pid:228   tgid:228   ppid:222    task_flags:0x400140 flags:0x00000818
>>> [   85.625823] Call trace:
>>> [   85.628316]  __switch_to+0xe8/0x1a0 (T)
>>> [   85.632217]  __schedule+0x290/0x7c0
>>> [   85.635765]  schedule+0x34/0x118
>>> [   85.639044]  async_synchronize_cookie_domain.part.0+0x50/0xa4
>>> [   85.644854]  async_synchronize_full+0x78/0xa0
>>> [   85.649270]  do_init_module+0x190/0x23c
>>> [   85.653154]  load_module+0x1708/0x1ca0
>>> [   85.656952]  init_module_from_file+0x74/0xa0
>>> [   85.661273]  __arm64_sys_finit_module+0x130/0x2f8
>>> [   85.666023]  invoke_syscall+0x48/0x104
>>> [   85.669842]  el0_svc_common.constprop.0+0xc0/0xe0
>>> [   85.674604]  do_el0_svc+0x1c/0x28
>>> [   85.677973]  el0_svc+0x2c/0x84
>>> [   85.681078]  el0t_64_sync_handler+0xa0/0xe4
>>> [   85.685316]  el0t_64_sync+0x198/0x19c
>>> [   85.689032] INFO: task (udev-worker):229 blocked for more than 42 seconds.
>>>
>>>
>>> Usually wifi, all remoteprocs and anything that depends on lpass/pinctrl fail to probe.
>> 
>> 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.

Best regards,
Alexey
Re: [PATCH v7 7/8] serial: qcom-geni: Enable PM runtime for serial driver
Posted by Krzysztof Kozlowski 1 month, 1 week ago
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.

Best regards,
Krzysztof
Re: [PATCH v7 7/8] serial: qcom-geni: Enable PM runtime for serial driver
Posted by Alexey Klimov 1 month, 1 week ago
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.

Thanks,
Alexey
Re: [PATCH v7 7/8] serial: qcom-geni: Enable PM runtime for serial driver
Posted by Krzysztof Kozlowski 1 month, 1 week ago
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.
Best regards,
Krzysztof
Re: [PATCH v7 7/8] serial: qcom-geni: Enable PM runtime for serial driver
Posted by Praveen Talari 1 month, 1 week ago
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
Re: [PATCH v7 7/8] serial: qcom-geni: Enable PM runtime for serial driver
Posted by Alexey Klimov 1 month, 1 week ago
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
Re: [PATCH v7 7/8] serial: qcom-geni: Enable PM runtime for serial driver
Posted by Praveen Talari 1 month, 1 week ago
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
> 
Re: [PATCH v7 7/8] serial: qcom-geni: Enable PM runtime for serial driver
Posted by Bryan O'Donoghue 1 month, 1 week ago
On 26/08/2025 11:29, 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
> 
> 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

Don't forget to include a Fixes: tag for this change.

---
bod