Message ID | 20211005155923.173399-7-marcan@marcan.st (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
Series | Apple SoC PMGR device power states driver | expand |
On 05/10/2021 17:59, Hector Martin wrote: > 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 <marcan@marcan.st> > --- > 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). > 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? > + 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
On Wed, Oct 6, 2021 at 9:44 AM Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> wrote: > > On 05/10/2021 17:59, Hector Martin wrote: > > 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 <marcan@marcan.st> > > --- > > 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). > > > 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. Why? What can be done differently if the resume fails? > 2. Why do you need to resume the device here? This appears to be to prevent the device from suspending after the given point. > > + > > 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? > > > + 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
On 06/10/2021 15:25, Rafael J. Wysocki wrote: > On Wed, Oct 6, 2021 at 9:44 AM Krzysztof Kozlowski > <krzysztof.kozlowski@canonical.com> wrote: >> >> On 05/10/2021 17:59, Hector Martin wrote: >>> 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 <marcan@marcan.st> >>> --- >>> 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). >> >>> 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. > > Why? What can be done differently if the resume fails? The answer is connected with the point (2) below. If there were for example register access here, maybe it should be simply skipped to avoid imprecise abort... > >> 2. Why do you need to resume the device here? > > This appears to be to prevent the device from suspending after the given point. Makes sense. > >>> + >>> 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? >> >>> + 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 Best regards, Krzysztof
On 06/10/2021 16.43, Krzysztof Kozlowski wrote: > On 05/10/2021 17:59, Hector Martin wrote: >> + 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). There are none though, this function always returns success past this point. >> 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? As Rafael mentioned, this is basically disabling PM so the device is enabled when not bound (which seems to be expected behavior). Not sure what I'd do if the resume fails... this is the remove path after all, it's not like we're doing anything else with the device at this point. >> + >> 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? Good question, I'm not entirely sure why these code paths have a check for NULL there. They were already there, do you happen to know why? To me it sounds like remove would only be called if probe succeeds, at which point drvdata should always be set.
On 11/10/2021 07:32, Hector Martin wrote: >>> + >>> 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? > > Good question, I'm not entirely sure why these code paths have a check > for NULL there. They were already there, do you happen to know why? To > me it sounds like remove would only be called if probe succeeds, at > which point drvdata should always be set. > Exactly, anyway it is not part of your patch, so no problem. Best regards, Krzysztof
On Mon, Oct 11, 2021 at 02:32:29PM +0900, Hector Martin wrote: > On 06/10/2021 16.43, Krzysztof Kozlowski wrote: > > On 05/10/2021 17:59, Hector Martin wrote: > >> 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? > > As Rafael mentioned, this is basically disabling PM so the device is > enabled when not bound (which seems to be expected behavior). Not sure > what I'd do if the resume fails... this is the remove path after all, > it's not like we're doing anything else with the device at this point. You still need to disable the clocks before returning from remove(). Consider what happens when the driver is rebound otherwise. Johan
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); + 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); + s3c24xx_serial_cpufreq_deregister(to_ourport(port)); uart_remove_one_port(&s3c24xx_uart_drv, port); + + pm_runtime_disable(&dev->dev); + 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) } /* UART power management code */ -#ifdef CONFIG_PM_SLEEP -static int s3c24xx_serial_suspend(struct device *dev) + +static int __maybe_unused s3c24xx_serial_suspend(struct device *dev) { struct uart_port *port = s3c24xx_dev_to_port(dev); @@ -2330,7 +2352,7 @@ static int s3c24xx_serial_suspend(struct device *dev) return 0; } -static int s3c24xx_serial_resume(struct device *dev) +static int __maybe_unused s3c24xx_serial_resume(struct device *dev) { struct uart_port *port = s3c24xx_dev_to_port(dev); struct s3c24xx_uart_port *ourport = to_ourport(port); @@ -2350,7 +2372,7 @@ static int s3c24xx_serial_resume(struct device *dev) return 0; } -static int s3c24xx_serial_resume_noirq(struct device *dev) +static int __maybe_unused s3c24xx_serial_resume_noirq(struct device *dev) { struct uart_port *port = s3c24xx_dev_to_port(dev); struct s3c24xx_uart_port *ourport = to_ourport(port); @@ -2420,16 +2442,14 @@ static int s3c24xx_serial_resume_noirq(struct device *dev) } static const struct dev_pm_ops s3c24xx_serial_pm_ops = { +#ifdef CONFIG_PM_SLEEP .suspend = s3c24xx_serial_suspend, .resume = s3c24xx_serial_resume, .resume_noirq = s3c24xx_serial_resume_noirq, +#endif + SET_RUNTIME_PM_OPS(s3c24xx_serial_runtime_suspend, + s3c24xx_serial_runtime_resume, NULL) }; -#define SERIAL_SAMSUNG_PM_OPS (&s3c24xx_serial_pm_ops) - -#else /* !CONFIG_PM_SLEEP */ - -#define SERIAL_SAMSUNG_PM_OPS NULL -#endif /* CONFIG_PM_SLEEP */ /* Console code */ @@ -2924,7 +2944,7 @@ static struct platform_driver samsung_serial_driver = { .id_table = s3c24xx_serial_driver_ids, .driver = { .name = "samsung-uart", - .pm = SERIAL_SAMSUNG_PM_OPS, + .pm = &s3c24xx_serial_pm_ops, .of_match_table = of_match_ptr(s3c24xx_uart_dt_match), }, };
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 <marcan@marcan.st> --- drivers/tty/serial/samsung_tty.c | 88 ++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 34 deletions(-)