Re: [PATCH 1/2] serial: sifive: Add suspend and resume operations

[Ben Dooks <ben.dooks@codethink.co.uk>]

On 08/08/2023 08:26, Nick Hu wrote:
> If the Sifive Uart is not used as the wake up source, suspend the uart
> before the system enter the suspend state to prevent it woken up by
> unexpected uart interrupt. Resume the uart once the system woken up.
> 
> Signed-off-by: Nick Hu <nick.hu@sifive.com>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>

^ This should be Reviewed-by, as I did review on this earlier.

> ---
>   drivers/tty/serial/sifive.c | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
> index a19db49327e2..87994cb69007 100644
> --- a/drivers/tty/serial/sifive.c
> +++ b/drivers/tty/serial/sifive.c
> @@ -1022,6 +1022,31 @@ static int sifive_serial_remove(struct platform_device *dev)
>   	return 0;
>   }
>   
> +static int sifive_serial_suspend(struct device *dev)
> +{
> +	int ret = 0;
> +	struct sifive_serial_port *ssp = dev_get_drvdata(dev);

Minor annyonance is ordering of ssp and ret, I think the showrter one
last is the nicest looking.

> +	if (ssp && ssp->port.type != PORT_UNKNOWN)
> +		ret = uart_suspend_port(&sifive_serial_uart_driver, &ssp->port);

Do we really need a test for ssp being valid if the device is bound.
Not sure if the port.type is also useful?

> +	return ret;
> +}
> +
> +static int sifive_serial_resume(struct device *dev)
> +{
> +	int ret = 0;
> +	struct sifive_serial_port *ssp = dev_get_drvdata(dev);
> +
> +	if (ssp && ssp->port.type != PORT_UNKNOWN)
> +		ret = uart_resume_port(&sifive_serial_uart_driver, &ssp->port);
> +
> +	return ret;
> +}
> +
> +DEFINE_SIMPLE_DEV_PM_OPS(sifive_uart_pm_ops, sifive_serial_suspend,
> +			 sifive_serial_resume);
> +
>   static const struct of_device_id sifive_serial_of_match[] = {
>   	{ .compatible = "sifive,fu540-c000-uart0" },
>   	{ .compatible = "sifive,uart0" },
> @@ -1034,6 +1059,7 @@ static struct platform_driver sifive_serial_platform_driver = {
>   	.remove		= sifive_serial_remove,
>   	.driver		= {
>   		.name	= SIFIVE_SERIAL_NAME,
> +		.pm = pm_sleep_ptr(&sifive_uart_pm_ops),
>   		.of_match_table = of_match_ptr(sifive_serial_of_match),
>   	},
>   };

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html