Re: [PATCH] serial: qcom-geni: add force suspend/resume to system sleep callbacks

[Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>]

Hi Praveen,

On 7/2/2026 8:18 AM, Praveen Talari wrote:
[...]
>>> +     * When no_console_suspend is set the console must remain active
>>> +     * across system sleep, so skip the force suspend path.
>>> +     */
>>> +    if (uart_console(uport) && !uport->suspended)
>>> +        return 0;
>> Rather use console_suspend_enabled and take action to go force suspend.
> In uart_suspend_port(), uport->suspended is updated only after the 
> console_suspend_enabled check. Therefore, its value directly reflects 
> whether the console suspend path was taken:
> 
> uport->suspended == 0 → the console was not suspended.
> uport->suspended == 1 → the console was suspended.
> 
> Looking at the code below, when console_suspend_enabled is disabled for 
> a console port, the function returns before setting uport->suspended = 
> 1. As a result, uport->suspended remains 0, which accurately indicates 
> that the console was not suspended.
> Therefore, I believe using uport->suspended is the more appropriate 
> check here. Please let me know your thoughts.
> 

I think it would be good to use console_suspend_enabled and take action. 
Not to depend on after effect of the primary decision maker variable.

> Code snippet from core layer
> 
> int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
> {
> [...]
> 
>      /*
>       * Nothing to do if the console is not suspending
>       * except stop_rx to prevent any asynchronous data
>       * over RX line. However ensure that we will be
>       * able to Re-start_rx later.
>       */
>      if (!console_suspend_enabled && uart_console(uport)) {
>          if (uport->ops->start_rx) {
>              guard(uart_port_lock_irq)(uport);
>              uport->ops->stop_rx(uport);
>          }
>          device_set_awake_path(uport->dev);
>          return 0;
>      }
> 
>      uport->suspended = 1;
> 
>      if (tty_port_initialized(port)) {
> [...]
> }
> 
>> Here, it sounds opposite, if port is resumed, you don't go to suspend 
>> within suspend function.
> It is straightforward: uport->suspended remains 0 even after 
> uart_suspend_port() is called, which indicates that the console has not 
> been suspended.
>>> +
>>> +    return pm_runtime_force_suspend(dev);
>> Is this really required ? if  uart_suspend_port() successful, what 
>> will happen with this ?
> 
> Yes, this is covered in the commit message. The key point is that 
> uart_suspend_port() may not trigger the runtime suspend callback if the 
> runtime-PM usage count remains non-zero. In such cases, 
> pm_runtime_force_suspend() is needed to ensure that the hardware 
> resources are properly suspended during system sleep like our i2c/spi 
> supported.
> 
I hardly see any other uart serial driver using 
pm_runtime_force_suspend(), any do not see serial driver using runtime 
PM ops directly. That being said, let's covered all use cases/testing 
with this change.

>> [...]
>>
>>