Re: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Kevin,
On Friday 05 April 2013 11:10 PM, Kevin Hilman wrote:
Sourav Poddar<sourav.poddar@xxxxxx>  writes:

With dt boot, uart wakeup after suspend is non functional while using
"no_console_suspend" in the bootargs. With "no_console_suspend" used, we
should prevent the runtime suspend of the uart port which is getting used
as an console.

Cc: Santosh Shilimkar<santosh.shilimkar@xxxxxx>
Cc: Felipe Balbi<balbi@xxxxxx>
Cc: Rajendra nayak<rnayak@xxxxxx>
Tested on omap5430evm, omap4430sdp.

Signed-off-by: Sourav Poddar<sourav.poddar@xxxxxx>
Rather than make these special checks inside the driver's runtime PM
callbacks, you should just disable runtime PM (pm_runtime_disable())

Then, this should be broken into 2 patches.

1) serial core: add the '->is_console' flag.  (nit on naming: don't call
    it port_is_console, since the struct is already a uart_port)

2) In the OMAP UART driver's ->prepare callback, check the is_console flag
    and pm_runtime_disable() accordingly  (then pm_runtime_enable() in
    the drivers's ->complete callback.

Kevin

I was working on your above suggestions, but realised there is not only console
uart which has the requirement of keeping the clocks enabled while going on
suspend.

If you see arch/arm/boot/dts/am33xx.dtsi, there is a ocmcram which has
"no_idle_on_suspend" property used.

    ocmcram: ocmcram@40300000 {
                        compatible = "ti,am3352-ocmcram";
                        reg = <0x40300000 0x10000>;
                        ti,hwmods = "ocmcram";
                        ti,no_idle_on_suspend;
        };
This property gets checked in omap_device file and correspondingly od->flags is set.

Based on your above inputs, the patches which I cooked up is inlined[1]. Though, the below patches works fine for uart case. The patches will effect ocmcram case and I am inling them
"just for discussion".

I am not sure, if there is any other cleaner way of getting around this "no_idle_on_suspend" flag
as they are getting used for ocmcram also. ?

Thanks,
Sourav

[1]:
From: Sourav Poddar <sourav.poddar@xxxxxx>
Date: Wed, 13 Mar 2013 20:32:36 +0530
Subject: [PATCH 1/2] arm: mach-omap2: remove "OMAP_DEVICE_NO_IDLE_ON_SUSPEND" check

Remove the "OMAP_DEVICE_NO_IDLE_ON_SUSPEND" check, since serial core and driver
takes care of the case when "no_console_suspend" is used in the bootargs and
you need to keep the clock enable for console even while suspend.

Tested on omap5430evm, omap4430sdp.

Signed-off-by: Sourav Poddar <sourav.poddar@xxxxxx>
---
 arch/arm/mach-omap2/omap_device.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
index 381be7a..d6dce8f 100644
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -620,11 +620,8 @@ static int _od_suspend_noirq(struct device *dev)
        ret = pm_generic_suspend_noirq(dev);

        if (!ret && !pm_runtime_status_suspended(dev)) {
-               if (pm_generic_runtime_suspend(dev) == 0) {
-                       if (!(od->flags & OMAP_DEVICE_NO_IDLE_ON_SUSPEND))
-                               omap_device_idle(pdev);
+               if (pm_generic_runtime_suspend(dev) == 0)
                        od->flags |= OMAP_DEVICE_SUSPENDED;
-               }
        }

        return ret;
@@ -638,8 +635,6 @@ static int _od_resume_noirq(struct device *dev)
        if ((od->flags & OMAP_DEVICE_SUSPENDED) &&
            !pm_runtime_status_suspended(dev)) {
                od->flags &= ~OMAP_DEVICE_SUSPENDED;
-               if (!(od->flags & OMAP_DEVICE_NO_IDLE_ON_SUSPEND))
-                       omap_device_enable(pdev);
                pm_generic_runtime_resume(dev);
        }

--

From: Sourav Poddar <sourav.poddar@xxxxxx>
Date: Wed, 13 Mar 2013 20:32:36 +0530
Subject: [PATCH 2/2] driver: serial: omap: add prepare/complete callback for "no_console_suspend" case

This patch adapt the serial core/driver to take care of the case when "no_console_suspend" is used in the bootargs. This patch will remove dependency to set od->flags to "OMAP_DEVICE_NO_IDLE_ON_SUSPEND" in serial.c(non dt case) and omap_device.c(dt case).

Prepare and complete callbacks will ensure that clocks remain active for the console
uart when "no_console_suspend" is used in the bootargs.

Tested on omap5430evm, omap4430sdp.

Signed-off-by: Sourav Poddar <sourav.poddar@xxxxxx>
---
 drivers/tty/serial/omap-serial.c |   20 ++++++++++++++++++++
 drivers/tty/serial/serial_core.c |    3 +++
 include/linux/serial_core.h      |    1 +
 3 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 08332f3..b726b2b 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1278,6 +1278,24 @@ static struct uart_driver serial_omap_reg = {
 };

 #ifdef CONFIG_PM_SLEEP
+static int serial_omap_prepare(struct device *dev)
+{
+       struct uart_omap_port *up = dev_get_drvdata(dev);
+
+       if (!console_suspend_enabled && up->port.is_console)
+               pm_runtime_disable(dev);
+
+       return 0;
+}
+
+static void serial_omap_complete(struct device *dev)
+{
+       struct uart_omap_port *up = dev_get_drvdata(dev);
+
+       if (!console_suspend_enabled && up->port.is_console)
+               pm_runtime_enable(dev);
+}
+
 static int serial_omap_suspend(struct device *dev)
 {
        struct uart_omap_port *up = dev_get_drvdata(dev);
@@ -1632,6 +1650,8 @@ static const struct dev_pm_ops serial_omap_dev_pm_ops = {
        SET_SYSTEM_SLEEP_PM_OPS(serial_omap_suspend, serial_omap_resume)
        SET_RUNTIME_PM_OPS(serial_omap_runtime_suspend,
                                serial_omap_runtime_resume, NULL)
+       .prepare        = serial_omap_prepare,
+       .complete       = serial_omap_complete,
 };

 #if defined(CONFIG_OF)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index a400002..c4d9328 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2594,6 +2594,9 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
        uport->cons = drv->cons;
        uport->state = state;

+       if (uart_console(uport))
+               uport->is_console = true;
+
        /*
         * If this port is a console, then the spinlock is already
         * initialised.
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 87d4bbc..7fcdd90 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -194,6 +194,7 @@ struct uart_port {
        unsigned char           irq_wake;
        unsigned char           unused[2];
void *private_data; /* generic platform data pointer */
+       bool                    is_console;
 };

 static inline int serial_port_in(struct uart_port *up, int offset)

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux