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

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

[Praveen Talari]

During system sleep the hardware resources (clocks, interconnect) are
not gated because the runtime-suspend callback is never invoked from
the system sleep path.  This prevents the platform from reaching its
lowest idle state.

The system sleep callbacks qcom_geni_serial_suspend() and
qcom_geni_serial_resume() rely solely on uart_suspend_port() /
uart_resume_port() to manage power.  uart_suspend_port() drives the
UART PM state machine to UART_PM_STATE_OFF, which in turn calls
pm_runtime_put_sync() and eventually the runtime-suspend callback.
However, if the runtime-PM usage count is still elevated at the time
of system sleep (e.g. the port is held active by an open file
descriptor), the runtime-suspend callback is never invoked and the
hardware resources (clocks, interconnect) remain enabled across
suspend, preventing the platform from reaching its lowest idle state.

Fix this by calling pm_runtime_force_suspend() at the end of
qcom_geni_serial_suspend() so that the runtime-suspend callback is
always executed regardless of the usage count, and by calling
pm_runtime_force_resume() at the start of qcom_geni_serial_resume()
to restore those resources before uart_resume_port() re-opens the
port.

Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
 drivers/tty/serial/qcom_geni_serial.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index e6b0a55f0cfb..ea8a1f0a9e3d 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1954,6 +1954,7 @@ static int qcom_geni_serial_suspend(struct device *dev)
 	struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
 	struct uart_port *uport = &port->uport;
 	struct qcom_geni_private_data *private_data = uport->private_data;
+	int ret;
 
 	/*
 	 * This is done so we can hit the lowest possible state in suspend
@@ -1963,7 +1964,19 @@ static int qcom_geni_serial_suspend(struct device *dev)
 		geni_icc_set_tag(&port->se, QCOM_ICC_TAG_ACTIVE_ONLY);
 		geni_icc_set_bw(&port->se);
 	}
-	return uart_suspend_port(private_data->drv, uport);
+
+	ret = uart_suspend_port(private_data->drv, uport);
+	if (ret)
+		return ret;
+
+	/*
+	 * 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;
+
+	return pm_runtime_force_suspend(dev);
 }
 
 static int qcom_geni_serial_resume(struct device *dev)
@@ -1973,6 +1986,10 @@ static int qcom_geni_serial_resume(struct device *dev)
 	struct uart_port *uport = &port->uport;
 	struct qcom_geni_private_data *private_data = uport->private_data;
 
+	ret = pm_runtime_force_resume(dev);
+	if (ret)
+		return ret;
+
 	ret = uart_resume_port(private_data->drv, uport);
 	if (uart_console(uport)) {
 		geni_icc_set_tag(&port->se, QCOM_ICC_TAG_ALWAYS);

---
base-commit: 1f5ffc672165ff851063a5fd044b727ab2517ae3
change-id: 20260617-add_force_suspend_resume_to_system_sleep_callbacks-ff2a94756306

Best regards,
--  
Praveen Talari <praveen.talari@oss.qualcomm.com>

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

[Mukesh Savaliya]

On 7/1/2026 11:27 AM, Praveen Talari wrote:
> During system sleep the hardware resources (clocks, interconnect) are
> not gated because the runtime-suspend callback is never invoked from
> the system sleep path.  This prevents the platform from reaching its
> lowest idle state.
> 
> The system sleep callbacks qcom_geni_serial_suspend() and
> qcom_geni_serial_resume() rely solely on uart_suspend_port() /
> uart_resume_port() to manage power.  uart_suspend_port() drives the
> UART PM state machine to UART_PM_STATE_OFF, which in turn calls
> pm_runtime_put_sync() and eventually the runtime-suspend callback.
> However, if the runtime-PM usage count is still elevated at the time
> of system sleep (e.g. the port is held active by an open file
> descriptor), the runtime-suspend callback is never invoked and the
> hardware resources (clocks, interconnect) remain enabled across
> suspend, preventing the platform from reaching its lowest idle state.
> 
> Fix this by calling pm_runtime_force_suspend() at the end of
> qcom_geni_serial_suspend() so that the runtime-suspend callback is
> always executed regardless of the usage count, and by calling
> pm_runtime_force_resume() at the start of qcom_geni_serial_resume()
> to restore those resources before uart_resume_port() re-opens the
> port.
> 
> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
> ---
[...]
> @@ -1963,7 +1964,19 @@ static int qcom_geni_serial_suspend(struct device *dev)
>   		geni_icc_set_tag(&port->se, QCOM_ICC_TAG_ACTIVE_ONLY);
>   		geni_icc_set_bw(&port->se);
>   	}
> -	return uart_suspend_port(private_data->drv, uport);
> +
> +	ret = uart_suspend_port(private_data->drv, uport);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * 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.
Here, it sounds opposite, if port is resumed, you don't go to suspend 
within suspend function.
> +
> +	return pm_runtime_force_suspend(dev);
Is this really required ? if  uart_suspend_port() successful, what will 
happen with this ?

>   }
>   
[...]

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

[Praveen Talari]

HI Mukesh

On 01-07-2026 20:47, Mukesh Savaliya wrote:
>
>
> On 7/1/2026 11:27 AM, Praveen Talari wrote:
>> During system sleep the hardware resources (clocks, interconnect) are
>> not gated because the runtime-suspend callback is never invoked from
>> the system sleep path.  This prevents the platform from reaching its
>> lowest idle state.
>>
>> The system sleep callbacks qcom_geni_serial_suspend() and
>> qcom_geni_serial_resume() rely solely on uart_suspend_port() /
>> uart_resume_port() to manage power.  uart_suspend_port() drives the
>> UART PM state machine to UART_PM_STATE_OFF, which in turn calls
>> pm_runtime_put_sync() and eventually the runtime-suspend callback.
>> However, if the runtime-PM usage count is still elevated at the time
>> of system sleep (e.g. the port is held active by an open file
>> descriptor), the runtime-suspend callback is never invoked and the
>> hardware resources (clocks, interconnect) remain enabled across
>> suspend, preventing the platform from reaching its lowest idle state.
>>
>> Fix this by calling pm_runtime_force_suspend() at the end of
>> qcom_geni_serial_suspend() so that the runtime-suspend callback is
>> always executed regardless of the usage count, and by calling
>> pm_runtime_force_resume() at the start of qcom_geni_serial_resume()
>> to restore those resources before uart_resume_port() re-opens the
>> port.
>>
>> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
>> ---
> [...]
>> @@ -1963,7 +1964,19 @@ static int qcom_geni_serial_suspend(struct 
>> device *dev)
>>           geni_icc_set_tag(&port->se, QCOM_ICC_TAG_ACTIVE_ONLY);
>>           geni_icc_set_bw(&port->se);
>>       }
>> -    return uart_suspend_port(private_data->drv, uport);
>> +
>> +    ret = uart_suspend_port(private_data->drv, uport);
>> +    if (ret)
>> +        return ret;
>> +
>> +    /*
>> +     * 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.

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.

Thanks,

Praveen Talari

>
>>   }
> [...]
>
>

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

[Mukesh Savaliya]

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.

>> [...]
>>
>>

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

[Praveen Talari]

Hi Mukesh

On 02-07-2026 14:47, Mukesh Savaliya wrote:
> 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.

Yes but i don't see strong reason not to use uport->suspended.

>
>> 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.

It depends on driver design and using of PM runtime framework usage.

This change was tested two devices like system suspend and wakeup irq 
functionality.


Thanks,

Praveen Talari

>
>>> [...]
>>>
>>>
>