[PATCHv5 0/3] Serial fixes

[PATCHv5 0/3] Serial fixes

[Sourav Poddar]

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 <santosh.shilimkar@ti.com>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Rajendra nayak <rnayak@ti.com>

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(-)

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

[Sourav Poddar]

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 <santosh.shilimkar@ti.com>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Rajendra nayak <rnayak@ti.com>
Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
Reviewed-by: Felipe Balbi <balbi@ti.com>
---
 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

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

[Greg KH]

On Fri, Apr 26, 2013 at 03:03:07PM -0700, Kevin Hilman wrote:
> Hi Greg,
> 
> Sourav Poddar <sourav.poddar@ti.com> 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 <santosh.shilimkar@ti.com>
> > Cc: Felipe Balbi <balbi@ti.com>
> > Cc: Rajendra nayak <rnayak@ti.com>
> > Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
> > Reviewed-by: Felipe Balbi <balbi@ti.com>
> 
> 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 <khilman@linaro.org>
> Tested-by: Kevin Hilman <khilman@linaro.org>

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

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

[Sourav Poddar]

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<sourav.poddar@ti.com>  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<santosh.shilimkar@ti.com>
>>> Cc: Felipe Balbi<balbi@ti.com>
>>> Cc: Rajendra nayak<rnayak@ti.com>
>>> Signed-off-by: Sourav Poddar<sourav.poddar@ti.com>
>>> Reviewed-by: Felipe Balbi<balbi@ti.com>
>> 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<khilman@linaro.org>
>> Tested-by: Kevin Hilman<khilman@linaro.org>
> 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

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

[Greg KH]

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<sourav.poddar@ti.com>  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<santosh.shilimkar@ti.com>
> >>>Cc: Felipe Balbi<balbi@ti.com>
> >>>Cc: Rajendra nayak<rnayak@ti.com>
> >>>Signed-off-by: Sourav Poddar<sourav.poddar@ti.com>
> >>>Reviewed-by: Felipe Balbi<balbi@ti.com>
> >>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<khilman@linaro.org>
> >>Tested-by: Kevin Hilman<khilman@linaro.org>
> >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

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

[Sourav Poddar]

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<sourav.poddar@ti.com>   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<santosh.shilimkar@ti.com>
>>>>> Cc: Felipe Balbi<balbi@ti.com>
>>>>> Cc: Rajendra nayak<rnayak@ti.com>
>>>>> Signed-off-by: Sourav Poddar<sourav.poddar@ti.com>
>>>>> Reviewed-by: Felipe Balbi<balbi@ti.com>
>>>> 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<khilman@linaro.org>
>>>> Tested-by: Kevin Hilman<khilman@linaro.org>
>>> 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.

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

[Sourav Poddar]

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<sourav.poddar@ti.com>   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<santosh.shilimkar@ti.com>
>>>>>> Cc: Felipe Balbi<balbi@ti.com>
>>>>>> Cc: Rajendra nayak<rnayak@ti.com>
>>>>>> Signed-off-by: Sourav Poddar<sourav.poddar@ti.com>
>>>>>> Reviewed-by: Felipe Balbi<balbi@ti.com>
>>>>> 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<khilman@linaro.org>
>>>>> Tested-by: Kevin Hilman<khilman@linaro.org>
>>>> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Sourav Poddar]

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 <sourav.poddar@ti.com>
Reviewed-by: Felipe Balbi <balbi@ti.com>
---
 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

Re: [PATCHv5 0/3] Serial fixes

[Sourav Poddar]

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<santosh.shilimkar@ti.com>
> Cc: Felipe Balbi<balbi@ti.com>
> Cc: Rajendra nayak<rnayak@ti.com>
>
> 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.