Thread (37 messages) 37 messages, 9 authors, 2021-10-11

Re: [PATCH 6/7] tty: serial: samsung_tty: Support runtime PM

From: Krzysztof Kozlowski <hidden>
Date: 2021-10-06 07:44:06
Also in: linux-arm-kernel, linux-pm, linux-samsung-soc, linux-serial, lkml

On 05/10/2021 17:59, Hector Martin wrote:
quoted hunk ↗ jump to hunk
This allows idle UART devices to be suspended using the standard
runtime-PM framework. The logic is modeled after stm32-usart.

Signed-off-by: Hector Martin <redacted>
---
 drivers/tty/serial/samsung_tty.c | 88 ++++++++++++++++++++------------
 1 file changed, 54 insertions(+), 34 deletions(-)
diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index e2f49863e9c2..d68e3341adc6 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -40,6 +40,7 @@
 #include <linux/clk.h>
 #include <linux/cpufreq.h>
 #include <linux/of.h>
+#include <linux/pm_runtime.h>
 #include <asm/irq.h>
 
 /* UART name and device definitions */
@@ -1381,31 +1382,49 @@ static void exynos_usi_init(struct uart_port *port)
 
 /* power power management control */
 
-static void s3c24xx_serial_pm(struct uart_port *port, unsigned int level,
-			      unsigned int old)
+static int __maybe_unused s3c24xx_serial_runtime_suspend(struct device *dev)
 {
+	struct uart_port *port = dev_get_drvdata(dev);
 	struct s3c24xx_uart_port *ourport = to_ourport(port);
 	int timeout = 10000;
 
-	ourport->pm_level = level;
+	while (--timeout && !s3c24xx_serial_txempty_nofifo(port))
+		udelay(100);
 
-	switch (level) {
-	case 3:
-		while (--timeout && !s3c24xx_serial_txempty_nofifo(port))
-			udelay(100);
+	if (!IS_ERR(ourport->baudclk))
+		clk_disable_unprepare(ourport->baudclk);
 
-		if (!IS_ERR(ourport->baudclk))
-			clk_disable_unprepare(ourport->baudclk);
+	clk_disable_unprepare(ourport->clk);
+	return 0;
+};
 
-		clk_disable_unprepare(ourport->clk);
-		break;
+static int __maybe_unused s3c24xx_serial_runtime_resume(struct device *dev)
+{
+	struct uart_port *port = dev_get_drvdata(dev);
+	struct s3c24xx_uart_port *ourport = to_ourport(port);
 
-	case 0:
-		clk_prepare_enable(ourport->clk);
+	clk_prepare_enable(ourport->clk);
 
-		if (!IS_ERR(ourport->baudclk))
-			clk_prepare_enable(ourport->baudclk);
+	if (!IS_ERR(ourport->baudclk))
+		clk_prepare_enable(ourport->baudclk);
+	return 0;
+};
+
+static void s3c24xx_serial_pm(struct uart_port *port, unsigned int level,
+			      unsigned int old)
+{
+	struct s3c24xx_uart_port *ourport = to_ourport(port);
+
+	ourport->pm_level = level;
 
+	switch (level) {
+	case UART_PM_STATE_OFF:
+		pm_runtime_mark_last_busy(port->dev);
+		pm_runtime_put_sync(port->dev);
+		break;
+
+	case UART_PM_STATE_ON:
+		pm_runtime_get_sync(port->dev);
 		exynos_usi_init(port);
 		break;
 	default:
@@ -2282,18 +2301,15 @@ static int s3c24xx_serial_probe(struct platform_device *pdev)
 		}
 	}
 
+	pm_runtime_get_noresume(&pdev->dev);
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
You need to cleanup in error paths (put/disable).
quoted hunk ↗ jump to hunk
 	dev_dbg(&pdev->dev, "%s: adding port\n", __func__);
 	uart_add_one_port(&s3c24xx_uart_drv, &ourport->port);
 	platform_set_drvdata(pdev, &ourport->port);
 
-	/*
-	 * Deactivate the clock enabled in s3c24xx_serial_init_port here,
-	 * so that a potential re-enablement through the pm-callback overlaps
-	 * and keeps the clock enabled in this case.
-	 */
-	clk_disable_unprepare(ourport->clk);
-	if (!IS_ERR(ourport->baudclk))
-		clk_disable_unprepare(ourport->baudclk);
+	pm_runtime_put_sync(&pdev->dev);
 
 	ret = s3c24xx_serial_cpufreq_register(ourport);
 	if (ret < 0)
@@ -2309,8 +2325,14 @@ static int s3c24xx_serial_remove(struct platform_device *dev)
 	struct uart_port *port = s3c24xx_dev_to_port(&dev->dev);
 
 	if (port) {
+		pm_runtime_get_sync(&dev->dev);
1. You need to check return status.
2. Why do you need to resume the device here?
+
 		s3c24xx_serial_cpufreq_deregister(to_ourport(port));
 		uart_remove_one_port(&s3c24xx_uart_drv, port);
+
+		pm_runtime_disable(&dev->dev);
Why disabling it only if port!=NULL? Can remove() be called if
platform_set_drvdata() was not?
quoted hunk ↗ jump to hunk
+		pm_runtime_set_suspended(&dev->dev);
+		pm_runtime_put_noidle(&dev->dev);
 	}
 
 	uart_unregister_driver(&s3c24xx_uart_drv);
@@ -2319,8 +2341,8 @@ static int s3c24xx_serial_remove(struct platform_device *dev)
 }
 
Best regards,
Krzysztof
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help