Re: serial: 8250_omap: suspend issue with console_suspend disabled

[Thomas Richard <thomas.richard@bootlin.com>]

On 9/20/23 07:38, Tony Lindgren wrote:
> Hi,
> 
> * Thomas Richard <thomas.richard@bootlin.com> [230915 09:57]:
>> The regression was introduced in commit 20a41a62618d "serial: 8250_omap:
>> Use force_suspend and resume for system suspend"
> ...
> 
>> --- a/drivers/tty/serial/8250/8250_omap.c
>> +++ b/drivers/tty/serial/8250/8250_omap.c
>> @@ -1630,7 +1630,7 @@ static int omap8250_suspend(struct device *dev)
>>         err = pm_runtime_force_suspend(dev);
>>         flush_work(&priv->qos_work);
>>
>> -       return err;
>> +       return 0;
>>  }
> 
> Maybe we can now just simplify things a bit here with the patch below.
> Care to give it a try, it's compile tested only so far.
> 
>> Once the omap8250_suspend doesn't return an error, the suspend sequence
>> can continue, but I get an other issue.
>> This issue is not related to commit 20a41a62618d, it has already been
>> present.
>> The power domain of the console is powered-off, so no more messages are
>> printed, and the SoC is stucked.
>> As the uart port is used as console, we don't want to power-off it.
>> My workaround is to set the corresponding power domain to
>> GENPD_FLAG_ALWAYS_ON, so the uart port is not powered-off.
> 
> The runtime PM usage count should keep the related power domain on though,
> sounds like this issue somewhere else if the power domains get force
> suspended with runtime PM usage count?

If I understand correctly, there are 2 solutions to keep the power
domain on through.
The first one is to set the flag GENPD_FLAG_ALWAYS_ON for the power domain.
The second one is to set the wakup_path of the device (using
device_set_wakeup_path) and set the flag GENPD_FLAG_ACTIVE_WAKEUP to the
power domain.

I didn't found any flag or option to say that the device is not
suspended, but it is not an error, so we don't want to poweroff the
power domain.

Regards,

Thomas

> 
> Regards,
> 
> Tony
> 
> 8< ------------------------------
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -1617,7 +1617,7 @@ static int omap8250_suspend(struct device *dev)
>  {
>  	struct omap8250_priv *priv = dev_get_drvdata(dev);
>  	struct uart_8250_port *up = serial8250_get_port(priv->line);
> -	int err;
> +	int err = 0;
>  
>  	serial8250_suspend_port(priv->line);
>  
> @@ -1627,7 +1627,8 @@ static int omap8250_suspend(struct device *dev)
>  	if (!device_may_wakeup(dev))
>  		priv->wer = 0;
>  	serial_out(up, UART_OMAP_WER, priv->wer);
> -	err = pm_runtime_force_suspend(dev);
> +	if (uart_console(&up->port) && console_suspend_enabled)
> +		err = pm_runtime_force_suspend(dev);
>  	flush_work(&priv->qos_work);
>  
>  	return err;
> @@ -1636,11 +1637,15 @@ static int omap8250_suspend(struct device *dev)
>  static int omap8250_resume(struct device *dev)
>  {
>  	struct omap8250_priv *priv = dev_get_drvdata(dev);
> +	struct uart_8250_port *up = serial8250_get_port(priv->line);
>  	int err;
>  
> -	err = pm_runtime_force_resume(dev);
> -	if (err)
> -		return err;
> +	if (uart_console(&up->port) && console_suspend_enabled) {
> +		err = pm_runtime_force_resume(dev);
> +		if (err)
> +			return err;
> +	}
> +
>  	serial8250_resume_port(priv->line);
>  	/* Paired with pm_runtime_resume_and_get() in omap8250_suspend() */
>  	pm_runtime_mark_last_busy(dev);
> @@ -1717,16 +1722,6 @@ static int omap8250_runtime_suspend(struct device *dev)
>  
>  	if (priv->line >= 0)
>  		up = serial8250_get_port(priv->line);
> -	/*
> -	 * When using 'no_console_suspend', the console UART must not be
> -	 * suspended. Since driver suspend is managed by runtime suspend,
> -	 * preventing runtime suspend (by returning error) will keep device
> -	 * active during suspend.
> -	 */
> -	if (priv->is_suspending && !console_suspend_enabled) {
> -		if (up && uart_console(&up->port))
> -			return -EBUSY;
> -	}
>  
>  	if (priv->habit & UART_ERRATA_CLOCK_DISABLE) {
>  		int ret;