2013-04-26 20:05:09

by Sourav Poddar

[permalink] [raw]
Subject: [PATCHv5 0/3] Serial fixes

Hi,

This patch series contains fixes around the issue that
the console UART should not idled on suspend while using
"no_console_suspend" in bootargs.

The approach thought of is to modify the serial core/serial driver to bypass
runtime PM if the UART in contention is a console and we are using "no_console_suspend"
in our bootargs.

There were discussions about how to handle "no_idle_on_suspend" issue and all the
discussions are as follows:
https://lkml.org/lkml/2013/4/5/239
https://lkml.org/lkml/2013/4/2/350
https://lkml.org/lkml/2013/3/18/199
https://lkml.org/lkml/2013/3/18/295
Due to the amount of change in approach, I am posting
this as a new series.

v4->v5
1. Add comments
2. Formatting.

v3->v4
1. check for console in runtime api.

v2->v3
1. Use "-EBUSY" for no_console_suspend case
2. Bypass runtime PM only during suspend
3. Improve the commit log based on community suggestion.

v1->v2
1. Remove the prepare/complete callback.
2. Adapt runtime PM callback to deal with the issue.
3. Fold patch(1,2) of previous series into 1.
4. Reordered the patch.
5. Change $subject and chage log for few patches.

Cc: Santosh Shilimkar <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Rajendra nayak <[email protected]>

Test info
Omap4430sdp:
- Tested wakeup from UART after suspend for dt and non dt case.
Omap5430evm:
- Tested wakeup from UART after suspend for dt case.

These patches were the part of the bigger series[1]. Breaking them into
two relevant series as they go through the different tree.

[1]: http://lkml.org/lkml/2013/4/26/274

This patches are based on 3.9-rc8

The following changes since commit 824282ca7d250bd7c301f221c3cd902ce906d731:

Merge branch 'upstream' of git://git.linux-mips.org/pub/scm/ralf/upstream-linus (2013-04-22 15:00:59 -0700)

are available in the git repository at:

git://gitorious.org/linux-connectivity/linux-connectivity.git serial_fix

Sourav Poddar (2):
driver: tty: serial: Move "uart_console" def to core header file.
driver: serial: omap: prevent runtime PM for "no_console_suspend"

drivers/tty/serial/mpc52xx_uart.c | 10 ----------
drivers/tty/serial/omap-serial.c | 34 +++++++++++++++++++++++++++++++++-
drivers/tty/serial/serial_core.c | 6 ------
include/linux/serial_core.h | 7 +++++++
4 files changed, 40 insertions(+), 17 deletions(-)


2013-04-26 20:05:14

by Sourav Poddar

[permalink] [raw]
Subject: [PATCHv5 2/2] driver: serial: omap: prevent runtime PM for "no_console_suspend"

The driver manages "no_console_suspend" by preventing runtime PM
during the suspend path, which forces the console UART to stay awake.

Signed-off-by: Sourav Poddar <[email protected]>
Reviewed-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/omap-serial.c | 34 +++++++++++++++++++++++++++++++++-
1 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 30d4f7a..9457fe3 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -161,6 +161,7 @@ struct uart_omap_port {
u32 calc_latency;
struct work_struct qos_work;
struct pinctrl *pins;
+ bool is_suspending;
};

#define to_uart_omap_port(p) ((container_of((p), struct uart_omap_port, port)))
@@ -1312,6 +1313,22 @@ 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);
+
+ up->is_suspending = true;
+
+ return 0;
+}
+
+static void serial_omap_complete(struct device *dev)
+{
+ struct uart_omap_port *up = dev_get_drvdata(dev);
+
+ up->is_suspending = false;
+}
+
static int serial_omap_suspend(struct device *dev)
{
struct uart_omap_port *up = dev_get_drvdata(dev);
@@ -1330,7 +1347,10 @@ static int serial_omap_resume(struct device *dev)

return 0;
}
-#endif
+#else
+#define serial_omap_prepare NULL
+#define serial_omap_prepare NULL
+#endif /* CONFIG_PM_SLEEP */

static void omap_serial_fill_features_erratas(struct uart_omap_port *up)
{
@@ -1616,6 +1636,16 @@ static int serial_omap_runtime_suspend(struct device *dev)
struct uart_omap_port *up = dev_get_drvdata(dev);
struct omap_uart_port_info *pdata = dev->platform_data;

+ /*
+ * 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 (up->is_suspending && !console_suspend_enabled &&
+ uart_console(&up->port))
+ return -EBUSY;
+
if (!up)
return -EINVAL;

@@ -1666,6 +1696,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)
--
1.7.1

2013-04-26 20:05:44

by Sourav Poddar

[permalink] [raw]
Subject: [PATCHv5 1/2] driver: tty: serial: Move "uart_console" def to core header file.

Move "uart_console" definition to serial core header file, so that it can be
used by serial drivers.
Get rid of the uart_console defintion from mpc52xx_uart driver.

Cc: Santosh Shilimkar <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Rajendra nayak <[email protected]>
Signed-off-by: Sourav Poddar <[email protected]>
Reviewed-by: Felipe Balbi <[email protected]>
---
drivers/tty/serial/mpc52xx_uart.c | 10 ----------
drivers/tty/serial/serial_core.c | 6 ------
include/linux/serial_core.h | 7 +++++++
3 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/serial/mpc52xx_uart.c b/drivers/tty/serial/mpc52xx_uart.c
index 018bad9..d74ac06 100644
--- a/drivers/tty/serial/mpc52xx_uart.c
+++ b/drivers/tty/serial/mpc52xx_uart.c
@@ -84,16 +84,6 @@ static void mpc52xx_uart_of_enumerate(void);
static irqreturn_t mpc52xx_uart_int(int irq, void *dev_id);
static irqreturn_t mpc5xxx_uart_process_int(struct uart_port *port);

-
-/* Simple macro to test if a port is console or not. This one is taken
- * for serial_core.c and maybe should be moved to serial_core.h ? */
-#ifdef CONFIG_SERIAL_CORE_CONSOLE
-#define uart_console(port) \
- ((port)->cons && (port)->cons->index == (port)->line)
-#else
-#define uart_console(port) (0)
-#endif
-
/* ======================================================================== */
/* PSC fifo operations for isolating differences between 52xx and 512x */
/* ======================================================================== */
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index a400002..b5f74bf 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -50,12 +50,6 @@ static struct lock_class_key port_lock_key;

#define HIGH_BITS_OFFSET ((sizeof(long)-sizeof(int))*8)

-#ifdef CONFIG_SERIAL_CORE_CONSOLE
-#define uart_console(port) ((port)->cons && (port)->cons->index == (port)->line)
-#else
-#define uart_console(port) (0)
-#endif
-
static void uart_change_speed(struct tty_struct *tty, struct uart_state *state,
struct ktermios *old_termios);
static void uart_wait_until_sent(struct tty_struct *tty, int timeout);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 87d4bbc..b98291a 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -31,6 +31,13 @@
#include <linux/sysrq.h>
#include <uapi/linux/serial_core.h>

+#ifdef CONFIG_SERIAL_CORE_CONSOLE
+#define uart_console(port) \
+ ((port)->cons && (port)->cons->index == (port)->line)
+#else
+#define uart_console(port) (0)
+#endif
+
struct uart_port;
struct serial_struct;
struct device;
--
1.7.1

2013-04-26 20:22:00

by Sourav Poddar

[permalink] [raw]
Subject: Re: [PATCHv5 0/3] Serial fixes

On Saturday 27 April 2013 01:34 AM, Sourav Poddar wrote:
> Hi,
>
> This patch series contains fixes around the issue that
> the console UART should not idled on suspend while using
> "no_console_suspend" in bootargs.
>
> The approach thought of is to modify the serial core/serial driver to bypass
> runtime PM if the UART in contention is a console and we are using "no_console_suspend"
> in our bootargs.
>
> There were discussions about how to handle "no_idle_on_suspend" issue and all the
> discussions are as follows:
> https://lkml.org/lkml/2013/4/5/239
> https://lkml.org/lkml/2013/4/2/350
> https://lkml.org/lkml/2013/3/18/199
> https://lkml.org/lkml/2013/3/18/295
> Due to the amount of change in approach, I am posting
> this as a new series.
>
> v4->v5
> 1. Add comments
> 2. Formatting.
>
> v3->v4
> 1. check for console in runtime api.
>
> v2->v3
> 1. Use "-EBUSY" for no_console_suspend case
> 2. Bypass runtime PM only during suspend
> 3. Improve the commit log based on community suggestion.
>
> v1->v2
> 1. Remove the prepare/complete callback.
> 2. Adapt runtime PM callback to deal with the issue.
> 3. Fold patch(1,2) of previous series into 1.
> 4. Reordered the patch.
> 5. Change $subject and chage log for few patches.
>
> Cc: Santosh Shilimkar<[email protected]>
> Cc: Felipe Balbi<[email protected]>
> Cc: Rajendra nayak<[email protected]>
>
> Test info
> Omap4430sdp:
> - Tested wakeup from UART after suspend for dt and non dt case.
> Omap5430evm:
> - Tested wakeup from UART after suspend for dt case.
>
> These patches were the part of the bigger series[1]. Breaking them into
> two relevant series as they go through the different tree.
>
> [1]: http://lkml.org/lkml/2013/4/26/274
>
> This patches are based on 3.9-rc8
>
> The following changes since commit 824282ca7d250bd7c301f221c3cd902ce906d731:
>
> Merge branch 'upstream' of git://git.linux-mips.org/pub/scm/ralf/upstream-linus (2013-04-22 15:00:59 -0700)
>
> are available in the git repository at:
>
> git://gitorious.org/linux-connectivity/linux-connectivity.git serial_fix
>
> Sourav Poddar (2):
> driver: tty: serial: Move "uart_console" def to core header file.
> driver: serial: omap: prevent runtime PM for "no_console_suspend"
>
> drivers/tty/serial/mpc52xx_uart.c | 10 ----------
> drivers/tty/serial/omap-serial.c | 34 +++++++++++++++++++++++++++++++++-
> drivers/tty/serial/serial_core.c | 6 ------
> include/linux/serial_core.h | 7 +++++++
> 4 files changed, 40 insertions(+), 17 deletion
Discard this cover..resend the cover with correct subject.

2013-04-26 22:18:22

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCHv5 1/2] driver: tty: serial: Move "uart_console" def to core header file.

On Fri, Apr 26, 2013 at 03:03:07PM -0700, Kevin Hilman wrote:
> Hi Greg,
>
> Sourav Poddar <[email protected]> writes:
>
> > Move "uart_console" definition to serial core header file, so that it can be
> > used by serial drivers.
> > Get rid of the uart_console defintion from mpc52xx_uart driver.
> >
> > Cc: Santosh Shilimkar <[email protected]>
> > Cc: Felipe Balbi <[email protected]>
> > Cc: Rajendra nayak <[email protected]>
> > Signed-off-by: Sourav Poddar <[email protected]>
> > Reviewed-by: Felipe Balbi <[email protected]>
>
> Can you queue up this patch (and 2/2)? Once these are in, I will queue
> up the corresponding arch/arm changes which can go independently.
>
> And feel free to add
>
> Reviewed-by: Kevin Hilman <[email protected]>
> Tested-by: Kevin Hilman <[email protected]>

I will do so after 3.10-rc1 is out, it is _way_ too late in the
development cycle for me to add these to my trees right now, considering
they are pretty much closed at the moment.

thanks,

greg k-h

2013-05-15 14:44:30

by Sourav Poddar

[permalink] [raw]
Subject: Re: [PATCHv5 1/2] driver: tty: serial: Move "uart_console" def to core header file.

Hi Greg,
On Saturday 27 April 2013 03:48 AM, Greg KH wrote:
> On Fri, Apr 26, 2013 at 03:03:07PM -0700, Kevin Hilman wrote:
>> Hi Greg,
>>
>> Sourav Poddar<[email protected]> writes:
>>
>>> Move "uart_console" definition to serial core header file, so that it can be
>>> used by serial drivers.
>>> Get rid of the uart_console defintion from mpc52xx_uart driver.
>>>
>>> Cc: Santosh Shilimkar<[email protected]>
>>> Cc: Felipe Balbi<[email protected]>
>>> Cc: Rajendra nayak<[email protected]>
>>> Signed-off-by: Sourav Poddar<[email protected]>
>>> Reviewed-by: Felipe Balbi<[email protected]>
>> Can you queue up this patch (and 2/2)? Once these are in, I will queue
>> up the corresponding arch/arm changes which can go independently.
>>
>> And feel free to add
>>
>> Reviewed-by: Kevin Hilman<[email protected]>
>> Tested-by: Kevin Hilman<[email protected]>
> I will do so after 3.10-rc1 is out, it is _way_ too late in the
> development cycle for me to add these to my trees right now, considering
> they are pretty much closed at the moment.
>
Do you want me to rebase this patches on top of 3.10-rc1?
> thanks,
>
> greg k-h

2013-05-15 14:57:03

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCHv5 1/2] driver: tty: serial: Move "uart_console" def to core header file.

On Wed, May 15, 2013 at 08:13:09PM +0530, Sourav Poddar wrote:
> Hi Greg,
> On Saturday 27 April 2013 03:48 AM, Greg KH wrote:
> >On Fri, Apr 26, 2013 at 03:03:07PM -0700, Kevin Hilman wrote:
> >>Hi Greg,
> >>
> >>Sourav Poddar<[email protected]> writes:
> >>
> >>>Move "uart_console" definition to serial core header file, so that it can be
> >>>used by serial drivers.
> >>>Get rid of the uart_console defintion from mpc52xx_uart driver.
> >>>
> >>>Cc: Santosh Shilimkar<[email protected]>
> >>>Cc: Felipe Balbi<[email protected]>
> >>>Cc: Rajendra nayak<[email protected]>
> >>>Signed-off-by: Sourav Poddar<[email protected]>
> >>>Reviewed-by: Felipe Balbi<[email protected]>
> >>Can you queue up this patch (and 2/2)? Once these are in, I will queue
> >>up the corresponding arch/arm changes which can go independently.
> >>
> >>And feel free to add
> >>
> >>Reviewed-by: Kevin Hilman<[email protected]>
> >>Tested-by: Kevin Hilman<[email protected]>
> >I will do so after 3.10-rc1 is out, it is _way_ too late in the
> >development cycle for me to add these to my trees right now, considering
> >they are pretty much closed at the moment.
> >
> Do you want me to rebase this patches on top of 3.10-rc1?

That would be great to have, thanks.

greg k-h

2013-05-15 14:58:26

by Sourav Poddar

[permalink] [raw]
Subject: Re: [PATCHv5 1/2] driver: tty: serial: Move "uart_console" def to core header file.

On Wednesday 15 May 2013 08:27 PM, Greg KH wrote:
> On Wed, May 15, 2013 at 08:13:09PM +0530, Sourav Poddar wrote:
>> Hi Greg,
>> On Saturday 27 April 2013 03:48 AM, Greg KH wrote:
>>> On Fri, Apr 26, 2013 at 03:03:07PM -0700, Kevin Hilman wrote:
>>>> Hi Greg,
>>>>
>>>> Sourav Poddar<[email protected]> writes:
>>>>
>>>>> Move "uart_console" definition to serial core header file, so that it can be
>>>>> used by serial drivers.
>>>>> Get rid of the uart_console defintion from mpc52xx_uart driver.
>>>>>
>>>>> Cc: Santosh Shilimkar<[email protected]>
>>>>> Cc: Felipe Balbi<[email protected]>
>>>>> Cc: Rajendra nayak<[email protected]>
>>>>> Signed-off-by: Sourav Poddar<[email protected]>
>>>>> Reviewed-by: Felipe Balbi<[email protected]>
>>>> Can you queue up this patch (and 2/2)? Once these are in, I will queue
>>>> up the corresponding arch/arm changes which can go independently.
>>>>
>>>> And feel free to add
>>>>
>>>> Reviewed-by: Kevin Hilman<[email protected]>
>>>> Tested-by: Kevin Hilman<[email protected]>
>>> I will do so after 3.10-rc1 is out, it is _way_ too late in the
>>> development cycle for me to add these to my trees right now, considering
>>> they are pretty much closed at the moment.
>>>
>> Do you want me to rebase this patches on top of 3.10-rc1?
> That would be great to have, thanks.
>
> greg k-h
Ok. Will do that and resend.

2013-05-15 15:43:37

by Sourav Poddar

[permalink] [raw]
Subject: Re: [PATCHv5 1/2] driver: tty: serial: Move "uart_console" def to core header file.

Hi,
On Wednesday 15 May 2013 08:27 PM, Sourav Poddar wrote:
> On Wednesday 15 May 2013 08:27 PM, Greg KH wrote:
>> On Wed, May 15, 2013 at 08:13:09PM +0530, Sourav Poddar wrote:
>>> Hi Greg,
>>> On Saturday 27 April 2013 03:48 AM, Greg KH wrote:
>>>> On Fri, Apr 26, 2013 at 03:03:07PM -0700, Kevin Hilman wrote:
>>>>> Hi Greg,
>>>>>
>>>>> Sourav Poddar<[email protected]> writes:
>>>>>
>>>>>> Move "uart_console" definition to serial core header file, so
>>>>>> that it can be
>>>>>> used by serial drivers.
>>>>>> Get rid of the uart_console defintion from mpc52xx_uart driver.
>>>>>>
>>>>>> Cc: Santosh Shilimkar<[email protected]>
>>>>>> Cc: Felipe Balbi<[email protected]>
>>>>>> Cc: Rajendra nayak<[email protected]>
>>>>>> Signed-off-by: Sourav Poddar<[email protected]>
>>>>>> Reviewed-by: Felipe Balbi<[email protected]>
>>>>> Can you queue up this patch (and 2/2)? Once these are in, I will
>>>>> queue
>>>>> up the corresponding arch/arm changes which can go independently.
>>>>>
>>>>> And feel free to add
>>>>>
>>>>> Reviewed-by: Kevin Hilman<[email protected]>
>>>>> Tested-by: Kevin Hilman<[email protected]>
>>>> I will do so after 3.10-rc1 is out, it is _way_ too late in the
>>>> development cycle for me to add these to my trees right now,
>>>> considering
>>>> they are pretty much closed at the moment.
>>>>
>>> Do you want me to rebase this patches on top of 3.10-rc1?
>> That would be great to have, thanks.
>>
[Greg]:
Have resend the patch series based on 3.10-rc1.

[Kevin]: Should I resend the omap specific changes also after rebasing it on
3.10-rc1, once greg pull in the serial part. ?
>> greg k-h
> Ok. Will do that and resend.
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-serial" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html