[PATCH/Resend 2/2] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"

[PATCH/Resend 2/2] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"

[Sourav Poddar]

With dt boot, uart wakeup after suspend is non functional on omap4/5 while using
"no_console_suspend" in the bootargs. With "no_console_suspend" used, od->flags
should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not allowing the console
to idle in the suspend path. For non-dt case, this was taken care by platform data.

Tested on omap5430evm, omap4430sdp.

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>
---
 arch/arm/mach-omap2/omap_device.c |   34 +++++++++++++++++++++++++++++++++-
 1 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
index 381be7a..71f5a73 100644
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -35,11 +35,17 @@
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
 #include <linux/notifier.h>
+#include <linux/platform_data/serial-omap.h>
 
 #include "soc.h"
 #include "omap_device.h"
 #include "omap_hwmod.h"
 
+#define MAX_UART_HWMOD_NAME_LEN         16
+
+static u8 no_console_suspend;
+static u8 console_uart_num;
+
 /* Private functions */
 
 static void _add_clkdev(struct omap_device *od, const char *clk_alias,
@@ -108,6 +114,12 @@ static void _add_hwmod_clocks_clkdev(struct omap_device *od,
 		_add_clkdev(od, oh->opt_clks[i].role, oh->opt_clks[i].clk);
 }
 
+static char *cmdline_find_option(char *str)
+{
+	extern char *saved_command_line;
+
+	return strstr(saved_command_line, str);
+}
 
 /**
  * omap_device_build_from_dt - build an omap_device with multiple hwmods
@@ -129,6 +141,7 @@ static int omap_device_build_from_dt(struct platform_device *pdev)
 	struct device_node *node = pdev->dev.of_node;
 	const char *oh_name;
 	int oh_cnt, i, ret = 0;
+	static u8 console_uart_id;
 
 	oh_cnt = of_property_count_strings(node, "ti,hwmods");
 	if (!oh_cnt || IS_ERR_VALUE(oh_cnt)) {
@@ -170,7 +183,12 @@ static int omap_device_build_from_dt(struct platform_device *pdev)
 			r->name = dev_name(&pdev->dev);
 	}
 
-	if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
+	if (no_console_suspend && !strncmp(oh->name, "uart", 4)) {
+		if (console_uart_num == console_uart_id)
+			omap_device_disable_idle_on_suspend(pdev);
+		else
+			console_uart_id++;
+	} else if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
 		omap_device_disable_idle_on_suspend(pdev);
 
 	pdev->dev.pm_domain = &omap_device_pm_domain;
@@ -838,7 +856,21 @@ static struct notifier_block platform_nb = {
 
 static int __init omap_device_init(void)
 {
+	int i;
+	char uart_name[MAX_UART_HWMOD_NAME_LEN];
+
 	bus_register_notifier(&platform_bus_type, &platform_nb);
+
+	if (cmdline_find_option("no_console_suspend")) {
+		no_console_suspend = true;
+		for (i = 0; i < OMAP_MAX_HSUART_PORTS; i++) {
+			snprintf(uart_name, MAX_UART_HWMOD_NAME_LEN,
+				"%s%d", OMAP_SERIAL_NAME, i);
+			if (cmdline_find_option(uart_name))
+				console_uart_num = i;
+		}
+	}
+
 	return 0;
 }
 omap_core_initcall(omap_device_init);
-- 
1.7.1

Re: [PATCH/Resend 2/2] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"

[Sourav Poddar]

Realised the list  to whom the patch was send got dropped. Ccing them all..
On Wednesday 20 March 2013 05:18 PM, Sourav Poddar wrote:
> Hi Kevin,
> On Tuesday 19 March 2013 12:24 AM, Kevin Hilman wrote:
>> Sourav Poddar<sourav.poddar@ti.com>  writes:
>>
>>> With dt boot, uart wakeup after suspend is non functional on omap4/5 
>>> while using
>>> "no_console_suspend" in the bootargs. With "no_console_suspend" 
>>> used, od->flags
>>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not 
>>> allowing the console
>>> to idle in the suspend path. For non-dt case, this was taken care by 
>>> platform data.
>>>
>>> Tested on omap5430evm, omap4430sdp.
>>>
>>> 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>
>> This patch creates a dependency between omap_device (generic,
>> device-independent code) and a specific driver (UART.)
>>
>> If you need to do something like this that's DT boot specific, then
>> we probably need some late initcall in serial.c to handle this.  It does
>> not belong in omap_device.
>>
> The following function "omap_device_disable_idle_on_suspend(pdev)" 
> should only
> be called once the omap device has been build, which in the case of 
> device tree is
> done in omap_device.c file. Moreover, the above call should be 
> executed conditionally
> and should depend on the following two parameter.
>
> [1]  a. Whether "no_console_suspend" is set and
>      b.  the device build is a console uart.
>
> When I look closely into the serial.c file, I realised that
> "core_initcall(omap_serial_early_init)" gets called irrespective
> of dt/non dt boot and will take care of most of the stuff(checking 
> whether
> "no_console_suspend" is used and which uart is used as a console uart) 
> which the
> $subject patch is proposing.
>
> But the problem is that we need to exchange the parsed information
> from serial.c to the omap_device file for the condtional execution of
> "omap_device_disable_idle_on_suspend"
>
> In this case,
> from "serial.c" we need
>     1. no_console_suspend = true
>     2. strcpy(console_name, oh_name), where oh_name corresponds to the 
> console uart.
>
> then in "omap_device.c" do
>     if (no_console_suspend && !strcmp(oh->name, console_name))
>                 omap_device_disable_idle_on_suspend(pdev);
>
> Please correct if I am understanding it incorrectly.
>
> If the above understanding looks good to you, is there a way we can 
> make this
> exchange of information happen between serial.c and omap_device.c file?
>> Kevin
>>
>>> ---
>>>   arch/arm/mach-omap2/omap_device.c |   34 
>>> +++++++++++++++++++++++++++++++++-
>>>   1 files changed, 33 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_device.c 
>>> b/arch/arm/mach-omap2/omap_device.c
>>> index 381be7a..71f5a73 100644
>>> --- a/arch/arm/mach-omap2/omap_device.c
>>> +++ b/arch/arm/mach-omap2/omap_device.c
>>> @@ -35,11 +35,17 @@
>>>   #include<linux/pm_runtime.h>
>>>   #include<linux/of.h>
>>>   #include<linux/notifier.h>
>>> +#include<linux/platform_data/serial-omap.h>
>>>
>>>   #include "soc.h"
>>>   #include "omap_device.h"
>>>   #include "omap_hwmod.h"
>>>
>>> +#define MAX_UART_HWMOD_NAME_LEN         16
>>> +
>>> +static u8 no_console_suspend;
>>> +static u8 console_uart_num;
>>> +
>>>   /* Private functions */
>>>
>>>   static void _add_clkdev(struct omap_device *od, const char 
>>> *clk_alias,
>>> @@ -108,6 +114,12 @@ static void _add_hwmod_clocks_clkdev(struct 
>>> omap_device *od,
>>>           _add_clkdev(od, oh->opt_clks[i].role, oh->opt_clks[i].clk);
>>>   }
>>>
>>> +static char *cmdline_find_option(char *str)
>>> +{
>>> +    extern char *saved_command_line;
>>> +
>>> +    return strstr(saved_command_line, str);
>>> +}
>>>
>>>   /**
>>>    * omap_device_build_from_dt - build an omap_device with multiple 
>>> hwmods
>>> @@ -129,6 +141,7 @@ static int omap_device_build_from_dt(struct 
>>> platform_device *pdev)
>>>       struct device_node *node = pdev->dev.of_node;
>>>       const char *oh_name;
>>>       int oh_cnt, i, ret = 0;
>>> +    static u8 console_uart_id;
>>>
>>>       oh_cnt = of_property_count_strings(node, "ti,hwmods");
>>>       if (!oh_cnt || IS_ERR_VALUE(oh_cnt)) {
>>> @@ -170,7 +183,12 @@ static int omap_device_build_from_dt(struct 
>>> platform_device *pdev)
>>>               r->name = dev_name(&pdev->dev);
>>>       }
>>>
>>> -    if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
>>> +    if (no_console_suspend&&  !strncmp(oh->name, "uart", 4)) {
>>> +        if (console_uart_num == console_uart_id)
>>> +            omap_device_disable_idle_on_suspend(pdev);
>>> +        else
>>> +            console_uart_id++;
>>> +    } else if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
>>>           omap_device_disable_idle_on_suspend(pdev);
>>>
>>>       pdev->dev.pm_domain =&omap_device_pm_domain;
>>> @@ -838,7 +856,21 @@ static struct notifier_block platform_nb = {
>>>
>>>   static int __init omap_device_init(void)
>>>   {
>>> +    int i;
>>> +    char uart_name[MAX_UART_HWMOD_NAME_LEN];
>>> +
>>>       bus_register_notifier(&platform_bus_type,&platform_nb);
>>> +
>>> +    if (cmdline_find_option("no_console_suspend")) {
>>> +        no_console_suspend = true;
>>> +        for (i = 0; i<  OMAP_MAX_HSUART_PORTS; i++) {
>>> +            snprintf(uart_name, MAX_UART_HWMOD_NAME_LEN,
>>> +                "%s%d", OMAP_SERIAL_NAME, i);
>>> +            if (cmdline_find_option(uart_name))
>>> +                console_uart_num = i;
>>> +        }
>>> +    }
>>> +
>>>       return 0;
>>>   }
>>>   omap_core_initcall(omap_device_init);
>

Re: [PATCH/Resend 2/2] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"

[Sourav Poddar]

Hi Kevin,
On Wednesday 20 March 2013 05:36 PM, Sourav Poddar wrote:
> Realised the list  to whom the patch was send got dropped. Ccing them 
> all..
> On Wednesday 20 March 2013 05:18 PM, Sourav Poddar wrote:
>> Hi Kevin,
>> On Tuesday 19 March 2013 12:24 AM, Kevin Hilman wrote:
>>> Sourav Poddar<sourav.poddar@ti.com>  writes:
>>>
>>>> With dt boot, uart wakeup after suspend is non functional on 
>>>> omap4/5 while using
>>>> "no_console_suspend" in the bootargs. With "no_console_suspend" 
>>>> used, od->flags
>>>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not 
>>>> allowing the console
>>>> to idle in the suspend path. For non-dt case, this was taken care 
>>>> by platform data.
>>>>
>>>> Tested on omap5430evm, omap4430sdp.
>>>>
>>>> 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>
>>> This patch creates a dependency between omap_device (generic,
>>> device-independent code) and a specific driver (UART.)
>>>
>>> If you need to do something like this that's DT boot specific, then
>>> we probably need some late initcall in serial.c to handle this.  It 
>>> does
>>> not belong in omap_device.
>>>
>> The following function "omap_device_disable_idle_on_suspend(pdev)" 
>> should only
>> be called once the omap device has been build, which in the case of 
>> device tree is
>> done in omap_device.c file. Moreover, the above call should be 
>> executed conditionally
>> and should depend on the following two parameter.
>>
>> [1]  a. Whether "no_console_suspend" is set and
>>      b.  the device build is a console uart.
>>
>> When I look closely into the serial.c file, I realised that
>> "core_initcall(omap_serial_early_init)" gets called irrespective
>> of dt/non dt boot and will take care of most of the stuff(checking 
>> whether
>> "no_console_suspend" is used and which uart is used as a console 
>> uart) which the
>> $subject patch is proposing.
>>
>> But the problem is that we need to exchange the parsed information
>> from serial.c to the omap_device file for the condtional execution of
>> "omap_device_disable_idle_on_suspend"
>>
>> In this case,
>> from "serial.c" we need
>>     1. no_console_suspend = true
>>     2. strcpy(console_name, oh_name), where oh_name corresponds to 
>> the console uart.
>>
>> then in "omap_device.c" do
>>     if (no_console_suspend && !strcmp(oh->name, console_name))
>>                 omap_device_disable_idle_on_suspend(pdev);
>>
>> Please correct if I am understanding it incorrectly.
>>
>> If the above understanding looks good to you, is there a way we can 
>> make this
>> exchange of information happen between serial.c and omap_device.c file?
Any Input on this?

Thanks,
Sourav
>>> Kevin
>>>
>>>> ---
>>>>   arch/arm/mach-omap2/omap_device.c |   34 
>>>> +++++++++++++++++++++++++++++++++-
>>>>   1 files changed, 33 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/omap_device.c 
>>>> b/arch/arm/mach-omap2/omap_device.c
>>>> index 381be7a..71f5a73 100644
>>>> --- a/arch/arm/mach-omap2/omap_device.c
>>>> +++ b/arch/arm/mach-omap2/omap_device.c
>>>> @@ -35,11 +35,17 @@
>>>>   #include<linux/pm_runtime.h>
>>>>   #include<linux/of.h>
>>>>   #include<linux/notifier.h>
>>>> +#include<linux/platform_data/serial-omap.h>
>>>>
>>>>   #include "soc.h"
>>>>   #include "omap_device.h"
>>>>   #include "omap_hwmod.h"
>>>>
>>>> +#define MAX_UART_HWMOD_NAME_LEN         16
>>>> +
>>>> +static u8 no_console_suspend;
>>>> +static u8 console_uart_num;
>>>> +
>>>>   /* Private functions */
>>>>
>>>>   static void _add_clkdev(struct omap_device *od, const char 
>>>> *clk_alias,
>>>> @@ -108,6 +114,12 @@ static void _add_hwmod_clocks_clkdev(struct 
>>>> omap_device *od,
>>>>           _add_clkdev(od, oh->opt_clks[i].role, oh->opt_clks[i].clk);
>>>>   }
>>>>
>>>> +static char *cmdline_find_option(char *str)
>>>> +{
>>>> +    extern char *saved_command_line;
>>>> +
>>>> +    return strstr(saved_command_line, str);
>>>> +}
>>>>
>>>>   /**
>>>>    * omap_device_build_from_dt - build an omap_device with multiple 
>>>> hwmods
>>>> @@ -129,6 +141,7 @@ static int omap_device_build_from_dt(struct 
>>>> platform_device *pdev)
>>>>       struct device_node *node = pdev->dev.of_node;
>>>>       const char *oh_name;
>>>>       int oh_cnt, i, ret = 0;
>>>> +    static u8 console_uart_id;
>>>>
>>>>       oh_cnt = of_property_count_strings(node, "ti,hwmods");
>>>>       if (!oh_cnt || IS_ERR_VALUE(oh_cnt)) {
>>>> @@ -170,7 +183,12 @@ static int omap_device_build_from_dt(struct 
>>>> platform_device *pdev)
>>>>               r->name = dev_name(&pdev->dev);
>>>>       }
>>>>
>>>> -    if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
>>>> +    if (no_console_suspend&&  !strncmp(oh->name, "uart", 4)) {
>>>> +        if (console_uart_num == console_uart_id)
>>>> +            omap_device_disable_idle_on_suspend(pdev);
>>>> +        else
>>>> +            console_uart_id++;
>>>> +    } else if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
>>>>           omap_device_disable_idle_on_suspend(pdev);
>>>>
>>>>       pdev->dev.pm_domain =&omap_device_pm_domain;
>>>> @@ -838,7 +856,21 @@ static struct notifier_block platform_nb = {
>>>>
>>>>   static int __init omap_device_init(void)
>>>>   {
>>>> +    int i;
>>>> +    char uart_name[MAX_UART_HWMOD_NAME_LEN];
>>>> +
>>>>       bus_register_notifier(&platform_bus_type,&platform_nb);
>>>> +
>>>> +    if (cmdline_find_option("no_console_suspend")) {
>>>> +        no_console_suspend = true;
>>>> +        for (i = 0; i<  OMAP_MAX_HSUART_PORTS; i++) {
>>>> +            snprintf(uart_name, MAX_UART_HWMOD_NAME_LEN,
>>>> +                "%s%d", OMAP_SERIAL_NAME, i);
>>>> +            if (cmdline_find_option(uart_name))
>>>> +                console_uart_num = i;
>>>> +        }
>>>> +    }
>>>> +
>>>>       return 0;
>>>>   }
>>>>   omap_core_initcall(omap_device_init);
>>
>

Re: [PATCH/Resend 2/2] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"

[Sourav Poddar]

Hi Kevin,
On Wednesday 20 March 2013 05:36 PM, Sourav Poddar wrote:
> Realised the list  to whom the patch was send got dropped. Ccing them 
> all..
> On Wednesday 20 March 2013 05:18 PM, Sourav Poddar wrote:
>> Hi Kevin,
>> On Tuesday 19 March 2013 12:24 AM, Kevin Hilman wrote:
>>> Sourav Poddar<sourav.poddar@ti.com>  writes:
>>>
>>>> With dt boot, uart wakeup after suspend is non functional on 
>>>> omap4/5 while using
>>>> "no_console_suspend" in the bootargs. With "no_console_suspend" 
>>>> used, od->flags
>>>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not 
>>>> allowing the console
>>>> to idle in the suspend path. For non-dt case, this was taken care 
>>>> by platform data.
>>>>
>>>> Tested on omap5430evm, omap4430sdp.
>>>>
>>>> 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>
>>> This patch creates a dependency between omap_device (generic,
>>> device-independent code) and a specific driver (UART.)
>>>
>>> If you need to do something like this that's DT boot specific, then
>>> we probably need some late initcall in serial.c to handle this.  It 
>>> does
>>> not belong in omap_device.
>>>
>> The following function "omap_device_disable_idle_on_suspend(pdev)" 
>> should only
>> be called once the omap device has been build, which in the case of 
>> device tree is
>> done in omap_device.c file. Moreover, the above call should be 
>> executed conditionally
>> and should depend on the following two parameter.
>>
>> [1]  a. Whether "no_console_suspend" is set and
>>      b.  the device build is a console uart.
>>
>> When I look closely into the serial.c file, I realised that
>> "core_initcall(omap_serial_early_init)" gets called irrespective
>> of dt/non dt boot and will take care of most of the stuff(checking 
>> whether
>> "no_console_suspend" is used and which uart is used as a console 
>> uart) which the
>> $subject patch is proposing.
>>
>> But the problem is that we need to exchange the parsed information
>> from serial.c to the omap_device file for the condtional execution of
>> "omap_device_disable_idle_on_suspend"
>>
>> In this case,
>> from "serial.c" we need
>>     1. no_console_suspend = true
>>     2. strcpy(console_name, oh_name), where oh_name corresponds to 
>> the console uart.
>>
>> then in "omap_device.c" do
>>     if (no_console_suspend && !strcmp(oh->name, console_name))
>>                 omap_device_disable_idle_on_suspend(pdev);
>>
>> Please correct if I am understanding it incorrectly.
>>
>> If the above understanding looks good to you, is there a way we can 
>> make this
>> exchange of information happen between serial.c and omap_device.c file?
Any input on this?
As I explained earlier, that there is a need to parse information in 
serial.c and use that in
omap_device.c only after the device is build.

Below is the patch (inlined) which further explains my point. The patch 
is "just for the
idea" I am trying to express.
I have used extern variables to exchange information between serial.c 
and omap_device.c.
Is there is a better way, we can do this "information exchange" without 
using extern variables?


-----
From: Sourav Poddar <sourav.poddar@ti.com>
Date: Wed, 13 Mar 2013 20:32:36 +0530
Subject: [RFC/PATCH] arm: mach-omap2: prevent UART console idle on 
suspend while using "no_console_suspend"

With dt boot on omap5, uart wakeup after suspend is non functional while 
using
"no_console_suspend" in the bootargs. With "no_console_suspend" used, 
od->flags
should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not 
allowing the console
to idle in the suspend path. For non-dt case, this was taken care by 
platform data.

Tested on omap5430evm, omap4430sdp.

Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
---
The problem is I need to use couple of "extern variables" which
can be used in the omap_device file, after the device is built from dt.

  arch/arm/mach-omap2/omap_device.c |    7 ++++++-
  arch/arm/mach-omap2/serial.c      |    4 +++-
  2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_device.c 
b/arch/arm/mach-omap2/omap_device.c
index e065daa..f4ebf9f 100644
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -96,6 +96,9 @@
  #define USE_WAKEUP_LAT            0
  #define IGNORE_WAKEUP_LAT        1

+extern u8 no_console_suspend;
+extern char console_uart_name[];
+
  static int omap_early_device_register(struct platform_device *pdev);

  static struct omap_device_pm_latency omap_default_latency[] = {
@@ -372,7 +375,9 @@ static int omap_device_build_from_dt(struct 
platform_device *pdev)
              r->name = dev_name(&pdev->dev);
      }

-    if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
+    if (no_console_suspend && !strcmp(oh->name, console_uart_name))
+        omap_device_disable_idle_on_suspend(pdev);
+    else if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
          omap_device_disable_idle_on_suspend(pdev);

      pdev->dev.pm_domain = &omap_device_pm_domain;
diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 037e691..f841ab5 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -63,8 +63,9 @@ struct omap_uart_state {
  static LIST_HEAD(uart_list);
  static u8 num_uarts;
  static u8 console_uart_id = -1;
-static u8 no_console_suspend;
  static u8 uart_debug;
+u8 no_console_suspend;
+char console_uart_name[MAX_UART_HWMOD_NAME_LEN];

  #define DEFAULT_RXDMA_POLLRATE        1    /* RX DMA polling rate (us) */
  #define DEFAULT_RXDMA_BUFSIZE        4096    /* RX DMA buffer size */
@@ -199,6 +200,7 @@ static int __init omap_serial_early_init(void)
                  "%s%d", OMAP_SERIAL_NAME, uart->num);

          if (cmdline_find_option(uart_name)) {
+            strcpy(console_uart_name, oh_name);
              console_uart_id = uart->num;

              if (console_loglevel >= 10) {
-- 
1.7.1

~Sourav
>>> Kevin
>>>
>>>> ---
>>>>   arch/arm/mach-omap2/omap_device.c |   34 
>>>> +++++++++++++++++++++++++++++++++-
>>>>   1 files changed, 33 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/omap_device.c 
>>>> b/arch/arm/mach-omap2/omap_device.c
>>>> index 381be7a..71f5a73 100644
>>>> --- a/arch/arm/mach-omap2/omap_device.c
>>>> +++ b/arch/arm/mach-omap2/omap_device.c
>>>> @@ -35,11 +35,17 @@
>>>>   #include<linux/pm_runtime.h>
>>>>   #include<linux/of.h>
>>>>   #include<linux/notifier.h>
>>>> +#include<linux/platform_data/serial-omap.h>
>>>>
>>>>   #include "soc.h"
>>>>   #include "omap_device.h"
>>>>   #include "omap_hwmod.h"
>>>>
>>>> +#define MAX_UART_HWMOD_NAME_LEN         16
>>>> +
>>>> +static u8 no_console_suspend;
>>>> +static u8 console_uart_num;
>>>> +
>>>>   /* Private functions */
>>>>
>>>>   static void _add_clkdev(struct omap_device *od, const char 
>>>> *clk_alias,
>>>> @@ -108,6 +114,12 @@ static void _add_hwmod_clocks_clkdev(struct 
>>>> omap_device *od,
>>>>           _add_clkdev(od, oh->opt_clks[i].role, oh->opt_clks[i].clk);
>>>>   }
>>>>
>>>> +static char *cmdline_find_option(char *str)
>>>> +{
>>>> +    extern char *saved_command_line;
>>>> +
>>>> +    return strstr(saved_command_line, str);
>>>> +}
>>>>
>>>>   /**
>>>>    * omap_device_build_from_dt - build an omap_device with multiple 
>>>> hwmods
>>>> @@ -129,6 +141,7 @@ static int omap_device_build_from_dt(struct 
>>>> platform_device *pdev)
>>>>       struct device_node *node = pdev->dev.of_node;
>>>>       const char *oh_name;
>>>>       int oh_cnt, i, ret = 0;
>>>> +    static u8 console_uart_id;
>>>>
>>>>       oh_cnt = of_property_count_strings(node, "ti,hwmods");
>>>>       if (!oh_cnt || IS_ERR_VALUE(oh_cnt)) {
>>>> @@ -170,7 +183,12 @@ static int omap_device_build_from_dt(struct 
>>>> platform_device *pdev)
>>>>               r->name = dev_name(&pdev->dev);
>>>>       }
>>>>
>>>> -    if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
>>>> +    if (no_console_suspend&&  !strncmp(oh->name, "uart", 4)) {
>>>> +        if (console_uart_num == console_uart_id)
>>>> +            omap_device_disable_idle_on_suspend(pdev);
>>>> +        else
>>>> +            console_uart_id++;
>>>> +    } else if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
>>>>           omap_device_disable_idle_on_suspend(pdev);
>>>>
>>>>       pdev->dev.pm_domain =&omap_device_pm_domain;
>>>> @@ -838,7 +856,21 @@ static struct notifier_block platform_nb = {
>>>>
>>>>   static int __init omap_device_init(void)
>>>>   {
>>>> +    int i;
>>>> +    char uart_name[MAX_UART_HWMOD_NAME_LEN];
>>>> +
>>>>       bus_register_notifier(&platform_bus_type,&platform_nb);
>>>> +
>>>> +    if (cmdline_find_option("no_console_suspend")) {
>>>> +        no_console_suspend = true;
>>>> +        for (i = 0; i<  OMAP_MAX_HSUART_PORTS; i++) {
>>>> +            snprintf(uart_name, MAX_UART_HWMOD_NAME_LEN,
>>>> +                "%s%d", OMAP_SERIAL_NAME, i);
>>>> +            if (cmdline_find_option(uart_name))
>>>> +                console_uart_num = i;
>>>> +        }
>>>> +    }
>>>> +
>>>>       return 0;
>>>>   }
>>>>   omap_core_initcall(omap_device_init);
>>
>

Re: [PATCH/Resend 2/2] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"

[Michael Trimarchi]

Hi 

On 02/04/13 11:50, Sourav Poddar wrote:
> Hi Kevin,
> On Wednesday 20 March 2013 05:36 PM, Sourav Poddar wrote:
>> Realised the list  to whom the patch was send got dropped. Ccing them all..
>> On Wednesday 20 March 2013 05:18 PM, Sourav Poddar wrote:
>>> Hi Kevin,
>>> On Tuesday 19 March 2013 12:24 AM, Kevin Hilman wrote:
>>>> Sourav Poddar<sourav.poddar@ti.com>  writes:
>>>>
>>>>> With dt boot, uart wakeup after suspend is non functional on omap4/5 while using
>>>>> "no_console_suspend" in the bootargs. With "no_console_suspend" used, od->flags
>>>>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not allowing the console
>>>>> to idle in the suspend path. For non-dt case, this was taken care by platform data.
>>>>>
>>>>> Tested on omap5430evm, omap4430sdp.
>>>>>
>>>>> 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>
>>>> This patch creates a dependency between omap_device (generic,
>>>> device-independent code) and a specific driver (UART.)
>>>>
>>>> If you need to do something like this that's DT boot specific, then
>>>> we probably need some late initcall in serial.c to handle this.  It does
>>>> not belong in omap_device.
>>>>
>>> The following function "omap_device_disable_idle_on_suspend(pdev)" should only
>>> be called once the omap device has been build, which in the case of device tree is
>>> done in omap_device.c file. Moreover, the above call should be executed conditionally
>>> and should depend on the following two parameter.
>>>
>>> [1]  a. Whether "no_console_suspend" is set and
>>>      b.  the device build is a console uart.
>>>
>>> When I look closely into the serial.c file, I realised that
>>> "core_initcall(omap_serial_early_init)" gets called irrespective
>>> of dt/non dt boot and will take care of most of the stuff(checking whether
>>> "no_console_suspend" is used and which uart is used as a console uart) which the
>>> $subject patch is proposing.
>>>
>>> But the problem is that we need to exchange the parsed information
>>> from serial.c to the omap_device file for the condtional execution of
>>> "omap_device_disable_idle_on_suspend"
>>>
>>> In this case,
>>> from "serial.c" we need
>>>     1. no_console_suspend = true
>>>     2. strcpy(console_name, oh_name), where oh_name corresponds to the console uart.
>>>
>>> then in "omap_device.c" do
>>>     if (no_console_suspend && !strcmp(oh->name, console_name))
>>>                 omap_device_disable_idle_on_suspend(pdev);
>>>
>>> Please correct if I am understanding it incorrectly.
>>>
>>> If the above understanding looks good to you, is there a way we can make this
>>> exchange of information happen between serial.c and omap_device.c file?
> Any input on this?
> As I explained earlier, that there is a need to parse information in serial.c and use that in
> omap_device.c only after the device is build.
> 
> Below is the patch (inlined) which further explains my point. The patch is "just for the
> idea" I am trying to express.
> I have used extern variables to exchange information between serial.c and omap_device.c.
> Is there is a better way, we can do this "information exchange" without using extern variables?
> 
> 
> -----
> From: Sourav Poddar <sourav.poddar@ti.com>
> Date: Wed, 13 Mar 2013 20:32:36 +0530
> Subject: [RFC/PATCH] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"
> 
> With dt boot on omap5, uart wakeup after suspend is non functional while using
> "no_console_suspend" in the bootargs. With "no_console_suspend" used, od->flags
> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not allowing the console
> to idle in the suspend path. For non-dt case, this was taken care by platform data.
> 
> Tested on omap5430evm, omap4430sdp.
> 
> Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
> ---
> The problem is I need to use couple of "extern variables" which
> can be used in the omap_device file, after the device is built from dt.
> 
>  arch/arm/mach-omap2/omap_device.c |    7 ++++++-
>  arch/arm/mach-omap2/serial.c      |    4 +++-
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
> index e065daa..f4ebf9f 100644
> --- a/arch/arm/mach-omap2/omap_device.c
> +++ b/arch/arm/mach-omap2/omap_device.c
> @@ -96,6 +96,9 @@
>  #define USE_WAKEUP_LAT            0
>  #define IGNORE_WAKEUP_LAT        1
> 
> +extern u8 no_console_suspend;
> +extern char console_uart_name[];
> +
>  static int omap_early_device_register(struct platform_device *pdev);
> 
>  static struct omap_device_pm_latency omap_default_latency[] = {
> @@ -372,7 +375,9 @@ static int omap_device_build_from_dt(struct platform_device *pdev)
>              r->name = dev_name(&pdev->dev);
>      }
> 
> -    if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
> +    if (no_console_suspend && !strcmp(oh->name, console_uart_name))
> +        omap_device_disable_idle_on_suspend(pdev);
> +    else if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
>          omap_device_disable_idle_on_suspend(pdev);

Why do not use some flags instead of external variable?


> 
>      pdev->dev.pm_domain = &omap_device_pm_domain;
> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
> index 037e691..f841ab5 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -63,8 +63,9 @@ struct omap_uart_state {
>  static LIST_HEAD(uart_list);
>  static u8 num_uarts;
>  static u8 console_uart_id = -1;
> -static u8 no_console_suspend;
>  static u8 uart_debug;
> +u8 no_console_suspend;
> +char console_uart_name[MAX_UART_HWMOD_NAME_LEN];
> 
>  #define DEFAULT_RXDMA_POLLRATE        1    /* RX DMA polling rate (us) */
>  #define DEFAULT_RXDMA_BUFSIZE        4096    /* RX DMA buffer size */
> @@ -199,6 +200,7 @@ static int __init omap_serial_early_init(void)
>                  "%s%d", OMAP_SERIAL_NAME, uart->num);
> 
>          if (cmdline_find_option(uart_name)) {
> +            strcpy(console_uart_name, oh_name);
>              console_uart_id = uart->num;
> 
>              if (console_loglevel >= 10) {

Michael

Re: [PATCH/Resend 2/2] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"

[Sourav Poddar]

Hi,
On Tuesday 02 April 2013 03:36 PM, Michael Trimarchi wrote:
> Hi
>
> On 02/04/13 11:50, Sourav Poddar wrote:
>> Hi Kevin,
>> On Wednesday 20 March 2013 05:36 PM, Sourav Poddar wrote:
>>> Realised the list  to whom the patch was send got dropped. Ccing them all..
>>> On Wednesday 20 March 2013 05:18 PM, Sourav Poddar wrote:
>>>> Hi Kevin,
>>>> On Tuesday 19 March 2013 12:24 AM, Kevin Hilman wrote:
>>>>> Sourav Poddar<sourav.poddar@ti.com>   writes:
>>>>>
>>>>>> With dt boot, uart wakeup after suspend is non functional on omap4/5 while using
>>>>>> "no_console_suspend" in the bootargs. With "no_console_suspend" used, od->flags
>>>>>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not allowing the console
>>>>>> to idle in the suspend path. For non-dt case, this was taken care by platform data.
>>>>>>
>>>>>> Tested on omap5430evm, omap4430sdp.
>>>>>>
>>>>>> 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>
>>>>> This patch creates a dependency between omap_device (generic,
>>>>> device-independent code) and a specific driver (UART.)
>>>>>
>>>>> If you need to do something like this that's DT boot specific, then
>>>>> we probably need some late initcall in serial.c to handle this.  It does
>>>>> not belong in omap_device.
>>>>>
>>>> The following function "omap_device_disable_idle_on_suspend(pdev)" should only
>>>> be called once the omap device has been build, which in the case of device tree is
>>>> done in omap_device.c file. Moreover, the above call should be executed conditionally
>>>> and should depend on the following two parameter.
>>>>
>>>> [1]  a. Whether "no_console_suspend" is set and
>>>>       b.  the device build is a console uart.
>>>>
>>>> When I look closely into the serial.c file, I realised that
>>>> "core_initcall(omap_serial_early_init)" gets called irrespective
>>>> of dt/non dt boot and will take care of most of the stuff(checking whether
>>>> "no_console_suspend" is used and which uart is used as a console uart) which the
>>>> $subject patch is proposing.
>>>>
>>>> But the problem is that we need to exchange the parsed information
>>>> from serial.c to the omap_device file for the condtional execution of
>>>> "omap_device_disable_idle_on_suspend"
>>>>
>>>> In this case,
>>>> from "serial.c" we need
>>>>      1. no_console_suspend = true
>>>>      2. strcpy(console_name, oh_name), where oh_name corresponds to the console uart.
>>>>
>>>> then in "omap_device.c" do
>>>>      if (no_console_suspend&&  !strcmp(oh->name, console_name))
>>>>                  omap_device_disable_idle_on_suspend(pdev);
>>>>
>>>> Please correct if I am understanding it incorrectly.
>>>>
>>>> If the above understanding looks good to you, is there a way we can make this
>>>> exchange of information happen between serial.c and omap_device.c file?
>> Any input on this?
>> As I explained earlier, that there is a need to parse information in serial.c and use that in
>> omap_device.c only after the device is build.
>>
>> Below is the patch (inlined) which further explains my point. The patch is "just for the
>> idea" I am trying to express.
>> I have used extern variables to exchange information between serial.c and omap_device.c.
>> Is there is a better way, we can do this "information exchange" without using extern variables?
>>
>>
>> -----
>> From: Sourav Poddar<sourav.poddar@ti.com>
>> Date: Wed, 13 Mar 2013 20:32:36 +0530
>> Subject: [RFC/PATCH] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"
>>
>> With dt boot on omap5, uart wakeup after suspend is non functional while using
>> "no_console_suspend" in the bootargs. With "no_console_suspend" used, od->flags
>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not allowing the console
>> to idle in the suspend path. For non-dt case, this was taken care by platform data.
>>
>> Tested on omap5430evm, omap4430sdp.
>>
>> Signed-off-by: Sourav Poddar<sourav.poddar@ti.com>
>> ---
>> The problem is I need to use couple of "extern variables" which
>> can be used in the omap_device file, after the device is built from dt.
>>
>>   arch/arm/mach-omap2/omap_device.c |    7 ++++++-
>>   arch/arm/mach-omap2/serial.c      |    4 +++-
>>   2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
>> index e065daa..f4ebf9f 100644
>> --- a/arch/arm/mach-omap2/omap_device.c
>> +++ b/arch/arm/mach-omap2/omap_device.c
>> @@ -96,6 +96,9 @@
>>   #define USE_WAKEUP_LAT            0
>>   #define IGNORE_WAKEUP_LAT        1
>>
>> +extern u8 no_console_suspend;
>> +extern char console_uart_name[];
>> +
>>   static int omap_early_device_register(struct platform_device *pdev);
>>
>>   static struct omap_device_pm_latency omap_default_latency[] = {
>> @@ -372,7 +375,9 @@ static int omap_device_build_from_dt(struct platform_device *pdev)
>>               r->name = dev_name(&pdev->dev);
>>       }
>>
>> -    if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
>> +    if (no_console_suspend&&  !strcmp(oh->name, console_uart_name))
>> +        omap_device_disable_idle_on_suspend(pdev);
>> +    else if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
>>           omap_device_disable_idle_on_suspend(pdev);
> Why do not use some flags instead of external variable?
>
>
That flag also need to be set in serial.c and used in omap_device.c.
Moreover, I am doing a strcmp so I need a console uart name to be passed
from serial.c to omap_device.c
>>       pdev->dev.pm_domain =&omap_device_pm_domain;
>> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
>> index 037e691..f841ab5 100644
>> --- a/arch/arm/mach-omap2/serial.c
>> +++ b/arch/arm/mach-omap2/serial.c
>> @@ -63,8 +63,9 @@ struct omap_uart_state {
>>   static LIST_HEAD(uart_list);
>>   static u8 num_uarts;
>>   static u8 console_uart_id = -1;
>> -static u8 no_console_suspend;
>>   static u8 uart_debug;
>> +u8 no_console_suspend;
>> +char console_uart_name[MAX_UART_HWMOD_NAME_LEN];
>>
>>   #define DEFAULT_RXDMA_POLLRATE        1    /* RX DMA polling rate (us) */
>>   #define DEFAULT_RXDMA_BUFSIZE        4096    /* RX DMA buffer size */
>> @@ -199,6 +200,7 @@ static int __init omap_serial_early_init(void)
>>                   "%s%d", OMAP_SERIAL_NAME, uart->num);
>>
>>           if (cmdline_find_option(uart_name)) {
>> +            strcpy(console_uart_name, oh_name);
>>               console_uart_id = uart->num;
>>
>>               if (console_loglevel>= 10) {
> Michael
~Sourav

Re: [PATCH/Resend 2/2] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"

[Michael Trimarchi]

Hi

On 02/04/13 12:39, Sourav Poddar wrote:
> Hi,
> On Tuesday 02 April 2013 03:36 PM, Michael Trimarchi wrote:
>> Hi
>>
>> On 02/04/13 11:50, Sourav Poddar wrote:
>>> Hi Kevin,
>>> On Wednesday 20 March 2013 05:36 PM, Sourav Poddar wrote:
>>>> Realised the list  to whom the patch was send got dropped. Ccing them all..
>>>> On Wednesday 20 March 2013 05:18 PM, Sourav Poddar wrote:
>>>>> Hi Kevin,
>>>>> On Tuesday 19 March 2013 12:24 AM, Kevin Hilman wrote:
>>>>>> Sourav Poddar<sourav.poddar@ti.com>   writes:
>>>>>>
>>>>>>> With dt boot, uart wakeup after suspend is non functional on omap4/5 while using
>>>>>>> "no_console_suspend" in the bootargs. With "no_console_suspend" used, od->flags
>>>>>>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not allowing the console
>>>>>>> to idle in the suspend path. For non-dt case, this was taken care by platform data.
>>>>>>>
>>>>>>> Tested on omap5430evm, omap4430sdp.
>>>>>>>
>>>>>>> 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>
>>>>>> This patch creates a dependency between omap_device (generic,
>>>>>> device-independent code) and a specific driver (UART.)
>>>>>>
>>>>>> If you need to do something like this that's DT boot specific, then
>>>>>> we probably need some late initcall in serial.c to handle this.  It does
>>>>>> not belong in omap_device.
>>>>>>
>>>>> The following function "omap_device_disable_idle_on_suspend(pdev)" should only
>>>>> be called once the omap device has been build, which in the case of device tree is
>>>>> done in omap_device.c file. Moreover, the above call should be executed conditionally
>>>>> and should depend on the following two parameter.
>>>>>
>>>>> [1]  a. Whether "no_console_suspend" is set and
>>>>>       b.  the device build is a console uart.
>>>>>
>>>>> When I look closely into the serial.c file, I realised that
>>>>> "core_initcall(omap_serial_early_init)" gets called irrespective
>>>>> of dt/non dt boot and will take care of most of the stuff(checking whether
>>>>> "no_console_suspend" is used and which uart is used as a console uart) which the
>>>>> $subject patch is proposing.
>>>>>
>>>>> But the problem is that we need to exchange the parsed information
>>>>> from serial.c to the omap_device file for the condtional execution of
>>>>> "omap_device_disable_idle_on_suspend"
>>>>>
>>>>> In this case,
>>>>> from "serial.c" we need
>>>>>      1. no_console_suspend = true
>>>>>      2. strcpy(console_name, oh_name), where oh_name corresponds to the console uart.
>>>>>
>>>>> then in "omap_device.c" do
>>>>>      if (no_console_suspend&&  !strcmp(oh->name, console_name))
>>>>>                  omap_device_disable_idle_on_suspend(pdev);
>>>>>
>>>>> Please correct if I am understanding it incorrectly.
>>>>>
>>>>> If the above understanding looks good to you, is there a way we can make this
>>>>> exchange of information happen between serial.c and omap_device.c file?
>>> Any input on this?
>>> As I explained earlier, that there is a need to parse information in serial.c and use that in
>>> omap_device.c only after the device is build.
>>>
>>> Below is the patch (inlined) which further explains my point. The patch is "just for the
>>> idea" I am trying to express.
>>> I have used extern variables to exchange information between serial.c and omap_device.c.
>>> Is there is a better way, we can do this "information exchange" without using extern variables?
>>>
>>>
>>> -----
>>> From: Sourav Poddar<sourav.poddar@ti.com>
>>> Date: Wed, 13 Mar 2013 20:32:36 +0530
>>> Subject: [RFC/PATCH] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"
>>>
>>> With dt boot on omap5, uart wakeup after suspend is non functional while using
>>> "no_console_suspend" in the bootargs. With "no_console_suspend" used, od->flags
>>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not allowing the console
>>> to idle in the suspend path. For non-dt case, this was taken care by platform data.
>>>
>>> Tested on omap5430evm, omap4430sdp.
>>>
>>> Signed-off-by: Sourav Poddar<sourav.poddar@ti.com>
>>> ---
>>> The problem is I need to use couple of "extern variables" which
>>> can be used in the omap_device file, after the device is built from dt.
>>>
>>>   arch/arm/mach-omap2/omap_device.c |    7 ++++++-
>>>   arch/arm/mach-omap2/serial.c      |    4 +++-
>>>   2 files changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
>>> index e065daa..f4ebf9f 100644
>>> --- a/arch/arm/mach-omap2/omap_device.c
>>> +++ b/arch/arm/mach-omap2/omap_device.c
>>> @@ -96,6 +96,9 @@
>>>   #define USE_WAKEUP_LAT            0
>>>   #define IGNORE_WAKEUP_LAT        1
>>>
>>> +extern u8 no_console_suspend;
>>> +extern char console_uart_name[];
>>> +
>>>   static int omap_early_device_register(struct platform_device *pdev);
>>>
>>>   static struct omap_device_pm_latency omap_default_latency[] = {
>>> @@ -372,7 +375,9 @@ static int omap_device_build_from_dt(struct platform_device *pdev)
>>>               r->name = dev_name(&pdev->dev);
>>>       }
>>>
>>> -    if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
>>> +    if (no_console_suspend&&  !strcmp(oh->name, console_uart_name))
>>> +        omap_device_disable_idle_on_suspend(pdev);
>>> +    else if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
>>>           omap_device_disable_idle_on_suspend(pdev);
>> Why do not use some flags instead of external variable?
>>
>>
> That flag also need to be set in serial.c and used in omap_device.c.
> Moreover, I am doing a strcmp so I need a console uart name to be passed
> from serial.c to omap_device.c

Just an idea, I didn't check the code

if (oh->flags & OMAP_FORCE_DISABLE_IDLE_ON_SUSPEND || of_get_property(node, "ti,no_idle_on_suspend", NULL))
	omap_device_disable_idle_on_suspend(pdev);

And I don't know if you can set such flags in serial.c

Michael


>>>       pdev->dev.pm_domain =&omap_device_pm_domain;
>>> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
>>> index 037e691..f841ab5 100644
>>> --- a/arch/arm/mach-omap2/serial.c
>>> +++ b/arch/arm/mach-omap2/serial.c
>>> @@ -63,8 +63,9 @@ struct omap_uart_state {
>>>   static LIST_HEAD(uart_list);
>>>   static u8 num_uarts;
>>>   static u8 console_uart_id = -1;
>>> -static u8 no_console_suspend;
>>>   static u8 uart_debug;
>>> +u8 no_console_suspend;
>>> +char console_uart_name[MAX_UART_HWMOD_NAME_LEN];
>>>
>>>   #define DEFAULT_RXDMA_POLLRATE        1    /* RX DMA polling rate (us) */
>>>   #define DEFAULT_RXDMA_BUFSIZE        4096    /* RX DMA buffer size */
>>> @@ -199,6 +200,7 @@ static int __init omap_serial_early_init(void)
>>>                   "%s%d", OMAP_SERIAL_NAME, uart->num);
>>>
>>>           if (cmdline_find_option(uart_name)) {
>>> +            strcpy(console_uart_name, oh_name);
>>>               console_uart_id = uart->num;
>>>
>>>               if (console_loglevel>= 10) {
>> Michael
> ~Sourav

Re: [PATCH/Resend 2/2] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"

[Sourav Poddar]

Hi,
On Tuesday 02 April 2013 04:14 PM, Michael Trimarchi wrote:
> Hi
>
> On 02/04/13 12:39, Sourav Poddar wrote:
>> Hi,
>> On Tuesday 02 April 2013 03:36 PM, Michael Trimarchi wrote:
>>> Hi
>>>
>>> On 02/04/13 11:50, Sourav Poddar wrote:
>>>> Hi Kevin,
>>>> On Wednesday 20 March 2013 05:36 PM, Sourav Poddar wrote:
>>>>> Realised the list  to whom the patch was send got dropped. Ccing them all..
>>>>> On Wednesday 20 March 2013 05:18 PM, Sourav Poddar wrote:
>>>>>> Hi Kevin,
>>>>>> On Tuesday 19 March 2013 12:24 AM, Kevin Hilman wrote:
>>>>>>> Sourav Poddar<sourav.poddar@ti.com>    writes:
>>>>>>>
>>>>>>>> With dt boot, uart wakeup after suspend is non functional on omap4/5 while using
>>>>>>>> "no_console_suspend" in the bootargs. With "no_console_suspend" used, od->flags
>>>>>>>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not allowing the console
>>>>>>>> to idle in the suspend path. For non-dt case, this was taken care by platform data.
>>>>>>>>
>>>>>>>> Tested on omap5430evm, omap4430sdp.
>>>>>>>>
>>>>>>>> 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>
>>>>>>> This patch creates a dependency between omap_device (generic,
>>>>>>> device-independent code) and a specific driver (UART.)
>>>>>>>
>>>>>>> If you need to do something like this that's DT boot specific, then
>>>>>>> we probably need some late initcall in serial.c to handle this.  It does
>>>>>>> not belong in omap_device.
>>>>>>>
>>>>>> The following function "omap_device_disable_idle_on_suspend(pdev)" should only
>>>>>> be called once the omap device has been build, which in the case of device tree is
>>>>>> done in omap_device.c file. Moreover, the above call should be executed conditionally
>>>>>> and should depend on the following two parameter.
>>>>>>
>>>>>> [1]  a. Whether "no_console_suspend" is set and
>>>>>>        b.  the device build is a console uart.
>>>>>>
>>>>>> When I look closely into the serial.c file, I realised that
>>>>>> "core_initcall(omap_serial_early_init)" gets called irrespective
>>>>>> of dt/non dt boot and will take care of most of the stuff(checking whether
>>>>>> "no_console_suspend" is used and which uart is used as a console uart) which the
>>>>>> $subject patch is proposing.
>>>>>>
>>>>>> But the problem is that we need to exchange the parsed information
>>>>>> from serial.c to the omap_device file for the condtional execution of
>>>>>> "omap_device_disable_idle_on_suspend"
>>>>>>
>>>>>> In this case,
>>>>>> from "serial.c" we need
>>>>>>       1. no_console_suspend = true
>>>>>>       2. strcpy(console_name, oh_name), where oh_name corresponds to the console uart.
>>>>>>
>>>>>> then in "omap_device.c" do
>>>>>>       if (no_console_suspend&&   !strcmp(oh->name, console_name))
>>>>>>                   omap_device_disable_idle_on_suspend(pdev);
>>>>>>
>>>>>> Please correct if I am understanding it incorrectly.
>>>>>>
>>>>>> If the above understanding looks good to you, is there a way we can make this
>>>>>> exchange of information happen between serial.c and omap_device.c file?
>>>> Any input on this?
>>>> As I explained earlier, that there is a need to parse information in serial.c and use that in
>>>> omap_device.c only after the device is build.
>>>>
>>>> Below is the patch (inlined) which further explains my point. The patch is "just for the
>>>> idea" I am trying to express.
>>>> I have used extern variables to exchange information between serial.c and omap_device.c.
>>>> Is there is a better way, we can do this "information exchange" without using extern variables?
>>>>
>>>>
>>>> -----
>>>> From: Sourav Poddar<sourav.poddar@ti.com>
>>>> Date: Wed, 13 Mar 2013 20:32:36 +0530
>>>> Subject: [RFC/PATCH] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"
>>>>
>>>> With dt boot on omap5, uart wakeup after suspend is non functional while using
>>>> "no_console_suspend" in the bootargs. With "no_console_suspend" used, od->flags
>>>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not allowing the console
>>>> to idle in the suspend path. For non-dt case, this was taken care by platform data.
>>>>
>>>> Tested on omap5430evm, omap4430sdp.
>>>>
>>>> Signed-off-by: Sourav Poddar<sourav.poddar@ti.com>
>>>> ---
>>>> The problem is I need to use couple of "extern variables" which
>>>> can be used in the omap_device file, after the device is built from dt.
>>>>
>>>>    arch/arm/mach-omap2/omap_device.c |    7 ++++++-
>>>>    arch/arm/mach-omap2/serial.c      |    4 +++-
>>>>    2 files changed, 9 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
>>>> index e065daa..f4ebf9f 100644
>>>> --- a/arch/arm/mach-omap2/omap_device.c
>>>> +++ b/arch/arm/mach-omap2/omap_device.c
>>>> @@ -96,6 +96,9 @@
>>>>    #define USE_WAKEUP_LAT            0
>>>>    #define IGNORE_WAKEUP_LAT        1
>>>>
>>>> +extern u8 no_console_suspend;
>>>> +extern char console_uart_name[];
>>>> +
>>>>    static int omap_early_device_register(struct platform_device *pdev);
>>>>
>>>>    static struct omap_device_pm_latency omap_default_latency[] = {
>>>> @@ -372,7 +375,9 @@ static int omap_device_build_from_dt(struct platform_device *pdev)
>>>>                r->name = dev_name(&pdev->dev);
>>>>        }
>>>>
>>>> -    if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
>>>> +    if (no_console_suspend&&   !strcmp(oh->name, console_uart_name))
>>>> +        omap_device_disable_idle_on_suspend(pdev);
>>>> +    else if (of_get_property(node, "ti,no_idle_on_suspend", NULL))
>>>>            omap_device_disable_idle_on_suspend(pdev);
>>> Why do not use some flags instead of external variable?
>>>
>>>
>> That flag also need to be set in serial.c and used in omap_device.c.
>> Moreover, I am doing a strcmp so I need a console uart name to be passed
>> from serial.c to omap_device.c
> Just an idea, I didn't check the code
>
> if (oh->flags&  OMAP_FORCE_DISABLE_IDLE_ON_SUSPEND || of_get_property(node, "ti,no_idle_on_suspend", NULL))
> 	omap_device_disable_idle_on_suspend(pdev);
>
> And I don't know if you can set such flags in serial.c
>
Yes, it should be possible.  I will try this out.
Thanks for the suggestion.

~Sourav
> Michael
>
>
>>>>        pdev->dev.pm_domain =&omap_device_pm_domain;
>>>> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
>>>> index 037e691..f841ab5 100644
>>>> --- a/arch/arm/mach-omap2/serial.c
>>>> +++ b/arch/arm/mach-omap2/serial.c
>>>> @@ -63,8 +63,9 @@ struct omap_uart_state {
>>>>    static LIST_HEAD(uart_list);
>>>>    static u8 num_uarts;
>>>>    static u8 console_uart_id = -1;
>>>> -static u8 no_console_suspend;
>>>>    static u8 uart_debug;
>>>> +u8 no_console_suspend;
>>>> +char console_uart_name[MAX_UART_HWMOD_NAME_LEN];
>>>>
>>>>    #define DEFAULT_RXDMA_POLLRATE        1    /* RX DMA polling rate (us) */
>>>>    #define DEFAULT_RXDMA_BUFSIZE        4096    /* RX DMA buffer size */
>>>> @@ -199,6 +200,7 @@ static int __init omap_serial_early_init(void)
>>>>                    "%s%d", OMAP_SERIAL_NAME, uart->num);
>>>>
>>>>            if (cmdline_find_option(uart_name)) {
>>>> +            strcpy(console_uart_name, oh_name);
>>>>                console_uart_id = uart->num;
>>>>
>>>>                if (console_loglevel>= 10) {
>>> Michael
>> ~Sourav

Re: [PATCH/Resend 2/2] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"

[Kevin Hilman]

Sourav Poddar <sourav.poddar@ti.com> writes:

> Hi Kevin,
> On Wednesday 20 March 2013 05:36 PM, Sourav Poddar wrote:
>> Realised the list  to whom the patch was send got dropped. Ccing
>> them all..
>> On Wednesday 20 March 2013 05:18 PM, Sourav Poddar wrote:
>>> Hi Kevin,
>>> On Tuesday 19 March 2013 12:24 AM, Kevin Hilman wrote:
>>>> Sourav Poddar<sourav.poddar@ti.com>  writes:
>>>>
>>>>> With dt boot, uart wakeup after suspend is non functional on
>>>>> omap4/5 while using
>>>>> "no_console_suspend" in the bootargs. With "no_console_suspend"
>>>>> used, od->flags
>>>>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not
>>>>> allowing the console
>>>>> to idle in the suspend path. For non-dt case, this was taken care
>>>>> by platform data.
>>>>>
>>>>> Tested on omap5430evm, omap4430sdp.
>>>>>
>>>>> 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>
>>>> This patch creates a dependency between omap_device (generic,
>>>> device-independent code) and a specific driver (UART.)
>>>>
>>>> If you need to do something like this that's DT boot specific, then
>>>> we probably need some late initcall in serial.c to handle this.
>>>> It does
>>>> not belong in omap_device.
>>>>
>>> The following function "omap_device_disable_idle_on_suspend(pdev)"
>>> should only
>>> be called once the omap device has been build, which in the case of
>>> device tree is
>>> done in omap_device.c file. Moreover, the above call should be
>>> executed conditionally
>>> and should depend on the following two parameter.
>>>
>>> [1]  a. Whether "no_console_suspend" is set and
>>>      b.  the device build is a console uart.
>>>
>>> When I look closely into the serial.c file, I realised that
>>> "core_initcall(omap_serial_early_init)" gets called irrespective
>>> of dt/non dt boot and will take care of most of the stuff(checking
>>> whether
>>> "no_console_suspend" is used and which uart is used as a console
>>> uart) which the
>>> $subject patch is proposing.
>>>
>>> But the problem is that we need to exchange the parsed information
>>> from serial.c to the omap_device file for the condtional execution of
>>> "omap_device_disable_idle_on_suspend"
>>>
>>> In this case,
>>> from "serial.c" we need
>>>     1. no_console_suspend = true
>>>     2. strcpy(console_name, oh_name), where oh_name corresponds to
>>> the console uart.
>>>
>>> then in "omap_device.c" do
>>>     if (no_console_suspend && !strcmp(oh->name, console_name))
>>>                 omap_device_disable_idle_on_suspend(pdev);
>>>
>>> Please correct if I am understanding it incorrectly.
>>>
>>> If the above understanding looks good to you, is there a way we can
>>> make this
>>> exchange of information happen between serial.c and omap_device.c file?
> Any input on this?
> As I explained earlier, that there is a need to parse information in
> serial.c and use that in
> omap_device.c only after the device is build.

As I explained earlier, any device specific hacks inside omap_device
should be a red flag that something has gone wrong.

How about fixing the UART driver/core to not runtime suspend if
no_console_suspend is given?  

Then we can get rid of this no_idle_on_suspend hack all together since
UART is the only remaining user.

Kevin

Re: [PATCH/Resend 2/2] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"

[Sourav Poddar]

Hi Kevin,
On Wednesday 03 April 2013 11:18 PM, Kevin Hilman wrote:
> Sourav Poddar<sourav.poddar@ti.com>  writes:
>
>> Hi Kevin,
>> On Wednesday 20 March 2013 05:36 PM, Sourav Poddar wrote:
>>> Realised the list  to whom the patch was send got dropped. Ccing
>>> them all..
>>> On Wednesday 20 March 2013 05:18 PM, Sourav Poddar wrote:
>>>> Hi Kevin,
>>>> On Tuesday 19 March 2013 12:24 AM, Kevin Hilman wrote:
>>>>> Sourav Poddar<sourav.poddar@ti.com>   writes:
>>>>>
>>>>>> With dt boot, uart wakeup after suspend is non functional on
>>>>>> omap4/5 while using
>>>>>> "no_console_suspend" in the bootargs. With "no_console_suspend"
>>>>>> used, od->flags
>>>>>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not
>>>>>> allowing the console
>>>>>> to idle in the suspend path. For non-dt case, this was taken care
>>>>>> by platform data.
>>>>>>
>>>>>> Tested on omap5430evm, omap4430sdp.
>>>>>>
>>>>>> 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>
>>>>> This patch creates a dependency between omap_device (generic,
>>>>> device-independent code) and a specific driver (UART.)
>>>>>
>>>>> If you need to do something like this that's DT boot specific, then
>>>>> we probably need some late initcall in serial.c to handle this.
>>>>> It does
>>>>> not belong in omap_device.
>>>>>
>>>> The following function "omap_device_disable_idle_on_suspend(pdev)"
>>>> should only
>>>> be called once the omap device has been build, which in the case of
>>>> device tree is
>>>> done in omap_device.c file. Moreover, the above call should be
>>>> executed conditionally
>>>> and should depend on the following two parameter.
>>>>
>>>> [1]  a. Whether "no_console_suspend" is set and
>>>>       b.  the device build is a console uart.
>>>>
>>>> When I look closely into the serial.c file, I realised that
>>>> "core_initcall(omap_serial_early_init)" gets called irrespective
>>>> of dt/non dt boot and will take care of most of the stuff(checking
>>>> whether
>>>> "no_console_suspend" is used and which uart is used as a console
>>>> uart) which the
>>>> $subject patch is proposing.
>>>>
>>>> But the problem is that we need to exchange the parsed information
>>>> from serial.c to the omap_device file for the condtional execution of
>>>> "omap_device_disable_idle_on_suspend"
>>>>
>>>> In this case,
>>>> from "serial.c" we need
>>>>      1. no_console_suspend = true
>>>>      2. strcpy(console_name, oh_name), where oh_name corresponds to
>>>> the console uart.
>>>>
>>>> then in "omap_device.c" do
>>>>      if (no_console_suspend&&  !strcmp(oh->name, console_name))
>>>>                  omap_device_disable_idle_on_suspend(pdev);
>>>>
>>>> Please correct if I am understanding it incorrectly.
>>>>
>>>> If the above understanding looks good to you, is there a way we can
>>>> make this
>>>> exchange of information happen between serial.c and omap_device.c file?
>> Any input on this?
>> As I explained earlier, that there is a need to parse information in
>> serial.c and use that in
>> omap_device.c only after the device is build.
> As I explained earlier, any device specific hacks inside omap_device
> should be a red flag that something has gone wrong.
>
> How about fixing the UART driver/core to not runtime suspend if
> no_console_suspend is given?
>
> Then we can get rid of this no_idle_on_suspend hack all together since
> UART is the only remaining user.
>

Yes, that can be done.

I cooked up an experimental patch, based on your above suggestion. The 
patch is
inlined along with  the mail. I tested the patch and it resolves
the issue.

Depending on whether no_console_suspend is used or not, we can set a 
variable
which can be used as a condition in runtime api's in serial driver.

The information whether "no_console_suspend" is used or not is parsed in
serial.c file, and this need to be communicated to the driver layer.

In my experimental patch below, I have used an extern variable
"force_console_suspend_disable".

Do you see a better way of handling this without using extern?

---------
From: Sourav Poddar <sourav.poddar@ti.com>
Date: Wed, 13 Mar 2013 20:32:36 +0530
Subject: [PATCH] arm: mach-omap2: prevent UART console idle on suspend 
while using "no_console_suspend"

With dt boot, uart wakeup after suspend is non functional while using
"no_console_suspend" in the bootargs. With "no_console_suspend" used, 
od->flags
should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not 
allowing the console
to idle in the suspend path.

Tested on omap5430evm, omap4430sdp.

Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
---
  arch/arm/mach-omap2/serial.c     |    6 ++++--
  drivers/tty/serial/omap-serial.c |    6 +++++-
  2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index f660156..427c407 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -51,7 +51,7 @@
  #define DEFAULT_AUTOSUSPEND_DELAY    -1

  #define MAX_UART_HWMOD_NAME_LEN        16
-
+int force_console_suspend_disable;
  struct omap_uart_state {
      int num;

@@ -207,8 +207,10 @@ static int __init omap_serial_early_init(void)
                      uart_name, uart->num);
              }

-            if (cmdline_find_option("no_console_suspend"))
+            if (cmdline_find_option("no_console_suspend")) {
+                force_console_suspend_disable = 1;
                  no_console_suspend = true;
+            }

              /*
               * omap-uart can be used for earlyprintk logs
diff --git a/drivers/tty/serial/omap-serial.c 
b/drivers/tty/serial/omap-serial.c
index 08332f3..395ade8 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -43,6 +43,7 @@
  #include <linux/pinctrl/consumer.h>
  #include <linux/platform_data/serial-omap.h>

+extern int force_console_suspend_disable;
  #define OMAP_MAX_HSUART_PORTS    6

  #define UART_BUILD_REVISION(x, y)    (((x) << 8) | (y))
@@ -1582,7 +1583,7 @@ 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;

-    if (!up)
+    if (!up || force_console_suspend_disable)
          return -EINVAL;

      if (!pdata)
@@ -1614,6 +1615,9 @@ static int serial_omap_runtime_resume(struct 
device *dev)

      int loss_cnt = serial_omap_get_context_loss_count(up);

+    if (force_console_suspend_disable)
+        return -EINVAL;
+
      if (loss_cnt < 0) {
          dev_err(dev, "serial_omap_get_context_loss_count failed : %d\n",
              loss_cnt);
-- 
1.7.1

Re: [PATCH/Resend 2/2] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"

[Santosh Shilimkar]

On Friday 05 April 2013 12:38 PM, Sourav Poddar wrote:
> Hi Kevin,
> On Wednesday 03 April 2013 11:18 PM, Kevin Hilman wrote:
>> Sourav Poddar<sourav.poddar@ti.com>  writes:
>>
>>> Hi Kevin,
>>> On Wednesday 20 March 2013 05:36 PM, Sourav Poddar wrote:
>>>> Realised the list  to whom the patch was send got dropped. Ccing
>>>> them all..
>>>> On Wednesday 20 March 2013 05:18 PM, Sourav Poddar wrote:
>>>>> Hi Kevin,
>>>>> On Tuesday 19 March 2013 12:24 AM, Kevin Hilman wrote:
>>>>>> Sourav Poddar<sourav.poddar@ti.com>   writes:
>>>>>>
>>>>>>> With dt boot, uart wakeup after suspend is non functional on
>>>>>>> omap4/5 while using
>>>>>>> "no_console_suspend" in the bootargs. With "no_console_suspend"
>>>>>>> used, od->flags
>>>>>>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not
>>>>>>> allowing the console
>>>>>>> to idle in the suspend path. For non-dt case, this was taken care
>>>>>>> by platform data.
>>>>>>>
>>>>>>> Tested on omap5430evm, omap4430sdp.
>>>>>>>
>>>>>>> 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>
>>>>>> This patch creates a dependency between omap_device (generic,
>>>>>> device-independent code) and a specific driver (UART.)
>>>>>>
>>>>>> If you need to do something like this that's DT boot specific, then
>>>>>> we probably need some late initcall in serial.c to handle this.
>>>>>> It does
>>>>>> not belong in omap_device.
>>>>>>
>>>>> The following function "omap_device_disable_idle_on_suspend(pdev)"
>>>>> should only
>>>>> be called once the omap device has been build, which in the case of
>>>>> device tree is
>>>>> done in omap_device.c file. Moreover, the above call should be
>>>>> executed conditionally
>>>>> and should depend on the following two parameter.
>>>>>
>>>>> [1]  a. Whether "no_console_suspend" is set and
>>>>>       b.  the device build is a console uart.
>>>>>
>>>>> When I look closely into the serial.c file, I realised that
>>>>> "core_initcall(omap_serial_early_init)" gets called irrespective
>>>>> of dt/non dt boot and will take care of most of the stuff(checking
>>>>> whether
>>>>> "no_console_suspend" is used and which uart is used as a console
>>>>> uart) which the
>>>>> $subject patch is proposing.
>>>>>
>>>>> But the problem is that we need to exchange the parsed information
>>>>> from serial.c to the omap_device file for the condtional execution of
>>>>> "omap_device_disable_idle_on_suspend"
>>>>>
>>>>> In this case,
>>>>> from "serial.c" we need
>>>>>      1. no_console_suspend = true
>>>>>      2. strcpy(console_name, oh_name), where oh_name corresponds to
>>>>> the console uart.
>>>>>
>>>>> then in "omap_device.c" do
>>>>>      if (no_console_suspend&&  !strcmp(oh->name, console_name))
>>>>>                  omap_device_disable_idle_on_suspend(pdev);
>>>>>
>>>>> Please correct if I am understanding it incorrectly.
>>>>>
>>>>> If the above understanding looks good to you, is there a way we can
>>>>> make this
>>>>> exchange of information happen between serial.c and omap_device.c file?
>>> Any input on this?
>>> As I explained earlier, that there is a need to parse information in
>>> serial.c and use that in
>>> omap_device.c only after the device is build.
>> As I explained earlier, any device specific hacks inside omap_device
>> should be a red flag that something has gone wrong.
>>
>> How about fixing the UART driver/core to not runtime suspend if
>> no_console_suspend is given?
>>
>> Then we can get rid of this no_idle_on_suspend hack all together since
>> UART is the only remaining user.
>>
> 
> Yes, that can be done.
> 
> I cooked up an experimental patch, based on your above suggestion. The patch is
> inlined along with  the mail. I tested the patch and it resolves
> the issue.
> 
Would be good to start a new thread.

> Depending on whether no_console_suspend is used or not, we can set a variable
> which can be used as a condition in runtime api's in serial driver.
> 
> The information whether "no_console_suspend" is used or not is parsed in
> serial.c file, and this need to be communicated to the driver layer.
> 
WHY ?
Serial core already knows about 'no_console_suspend' and thats used
to manage console lock.

> In my experimental patch below, I have used an extern variable
> "force_console_suspend_disable".
> 
> Do you see a better way of handling this without using extern?
> 
> ---------
> From: Sourav Poddar <sourav.poddar@ti.com>
> Date: Wed, 13 Mar 2013 20:32:36 +0530
> Subject: [PATCH] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"
> 
> With dt boot, uart wakeup after suspend is non functional while using
> "no_console_suspend" in the bootargs. With "no_console_suspend" used, od->flags
> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not allowing the console
> to idle in the suspend path.
> 
> Tested on omap5430evm, omap4430sdp.
> 
> Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
> ---
>  arch/arm/mach-omap2/serial.c     |    6 ++++--
>  drivers/tty/serial/omap-serial.c |    6 +++++-
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
> index f660156..427c407 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -51,7 +51,7 @@
>  #define DEFAULT_AUTOSUSPEND_DELAY    -1
> 
>  #define MAX_UART_HWMOD_NAME_LEN        16
> -
> +int force_console_suspend_disable;
>  struct omap_uart_state {
>      int num;
> 
> @@ -207,8 +207,10 @@ static int __init omap_serial_early_init(void)
>                      uart_name, uart->num);
>              }
> 
> -            if (cmdline_find_option("no_console_suspend"))
> +            if (cmdline_find_option("no_console_suspend")) {
> +                force_console_suspend_disable = 1;
>                  no_console_suspend = true;
> +            }
> 
>              /*
>               * omap-uart can be used for earlyprintk logs
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index 08332f3..395ade8 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -43,6 +43,7 @@
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/platform_data/serial-omap.h>
> 
> +extern int force_console_suspend_disable;
This is no good. You need to use the information from
core serial layer

>  #define OMAP_MAX_HSUART_PORTS    6
> 
>  #define UART_BUILD_REVISION(x, y)    (((x) << 8) | (y))
> @@ -1582,7 +1583,7 @@ 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;
> 
> -    if (!up)
> +    if (!up || force_console_suspend_disable)
>          return -EINVAL;
> 
>      if (!pdata)
> @@ -1614,6 +1615,9 @@ static int serial_omap_runtime_resume(struct device *dev)
> 
>      int loss_cnt = serial_omap_get_context_loss_count(up);
> 
> +    if (force_console_suspend_disable)
> +        return -EINVAL;
> +
>      if (loss_cnt < 0) {
>          dev_err(dev, "serial_omap_get_context_loss_count failed : %d\n",
>              loss_cnt);

Re: [PATCH/Resend 2/2] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"

[Sourav Poddar]

Hi Santosh,
On Friday 05 April 2013 12:45 PM, Santosh Shilimkar wrote:
> On Friday 05 April 2013 12:38 PM, Sourav Poddar wrote:
>> Hi Kevin,
>> On Wednesday 03 April 2013 11:18 PM, Kevin Hilman wrote:
>>> Sourav Poddar<sourav.poddar@ti.com>   writes:
>>>
>>>> Hi Kevin,
>>>> On Wednesday 20 March 2013 05:36 PM, Sourav Poddar wrote:
>>>>> Realised the list  to whom the patch was send got dropped. Ccing
>>>>> them all..
>>>>> On Wednesday 20 March 2013 05:18 PM, Sourav Poddar wrote:
>>>>>> Hi Kevin,
>>>>>> On Tuesday 19 March 2013 12:24 AM, Kevin Hilman wrote:
>>>>>>> Sourav Poddar<sourav.poddar@ti.com>    writes:
>>>>>>>
>>>>>>>> With dt boot, uart wakeup after suspend is non functional on
>>>>>>>> omap4/5 while using
>>>>>>>> "no_console_suspend" in the bootargs. With "no_console_suspend"
>>>>>>>> used, od->flags
>>>>>>>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not
>>>>>>>> allowing the console
>>>>>>>> to idle in the suspend path. For non-dt case, this was taken care
>>>>>>>> by platform data.
>>>>>>>>
>>>>>>>> Tested on omap5430evm, omap4430sdp.
>>>>>>>>
>>>>>>>> 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>
>>>>>>> This patch creates a dependency between omap_device (generic,
>>>>>>> device-independent code) and a specific driver (UART.)
>>>>>>>
>>>>>>> If you need to do something like this that's DT boot specific, then
>>>>>>> we probably need some late initcall in serial.c to handle this.
>>>>>>> It does
>>>>>>> not belong in omap_device.
>>>>>>>
>>>>>> The following function "omap_device_disable_idle_on_suspend(pdev)"
>>>>>> should only
>>>>>> be called once the omap device has been build, which in the case of
>>>>>> device tree is
>>>>>> done in omap_device.c file. Moreover, the above call should be
>>>>>> executed conditionally
>>>>>> and should depend on the following two parameter.
>>>>>>
>>>>>> [1]  a. Whether "no_console_suspend" is set and
>>>>>>        b.  the device build is a console uart.
>>>>>>
>>>>>> When I look closely into the serial.c file, I realised that
>>>>>> "core_initcall(omap_serial_early_init)" gets called irrespective
>>>>>> of dt/non dt boot and will take care of most of the stuff(checking
>>>>>> whether
>>>>>> "no_console_suspend" is used and which uart is used as a console
>>>>>> uart) which the
>>>>>> $subject patch is proposing.
>>>>>>
>>>>>> But the problem is that we need to exchange the parsed information
>>>>>> from serial.c to the omap_device file for the condtional execution of
>>>>>> "omap_device_disable_idle_on_suspend"
>>>>>>
>>>>>> In this case,
>>>>>> from "serial.c" we need
>>>>>>       1. no_console_suspend = true
>>>>>>       2. strcpy(console_name, oh_name), where oh_name corresponds to
>>>>>> the console uart.
>>>>>>
>>>>>> then in "omap_device.c" do
>>>>>>       if (no_console_suspend&&   !strcmp(oh->name, console_name))
>>>>>>                   omap_device_disable_idle_on_suspend(pdev);
>>>>>>
>>>>>> Please correct if I am understanding it incorrectly.
>>>>>>
>>>>>> If the above understanding looks good to you, is there a way we can
>>>>>> make this
>>>>>> exchange of information happen between serial.c and omap_device.c file?
>>>> Any input on this?
>>>> As I explained earlier, that there is a need to parse information in
>>>> serial.c and use that in
>>>> omap_device.c only after the device is build.
>>> As I explained earlier, any device specific hacks inside omap_device
>>> should be a red flag that something has gone wrong.
>>>
>>> How about fixing the UART driver/core to not runtime suspend if
>>> no_console_suspend is given?
>>>
>>> Then we can get rid of this no_idle_on_suspend hack all together since
>>> UART is the only remaining user.
>>>
>> Yes, that can be done.
>>
>> I cooked up an experimental patch, based on your above suggestion. The patch is
>> inlined along with  the mail. I tested the patch and it resolves
>> the issue.
>>
> Would be good to start a new thread.
>
I will post a new version based on your comments below.
>> Depending on whether no_console_suspend is used or not, we can set a variable
>> which can be used as a condition in runtime api's in serial driver.
>>
>> The information whether "no_console_suspend" is used or not is parsed in
>> serial.c file, and this need to be communicated to the driver layer.
>>
> WHY ?
> Serial core already knows about 'no_console_suspend' and thats used
> to manage console lock.
>
True.
>> In my experimental patch below, I have used an extern variable
>> "force_console_suspend_disable".
>>
>> Do you see a better way of handling this without using extern?
>>
>> ---------
>> From: Sourav Poddar<sourav.poddar@ti.com>
>> Date: Wed, 13 Mar 2013 20:32:36 +0530
>> Subject: [PATCH] arm: mach-omap2: prevent UART console idle on suspend while using "no_console_suspend"
>>
>> With dt boot, uart wakeup after suspend is non functional while using
>> "no_console_suspend" in the bootargs. With "no_console_suspend" used, od->flags
>> should be ORed with "OMAP_DEVICE_NO_IDLE_ON_SUSPEND", thereby not allowing the console
>> to idle in the suspend path.
>>
>> Tested on omap5430evm, omap4430sdp.
>>
>> Signed-off-by: Sourav Poddar<sourav.poddar@ti.com>
>> ---
>>   arch/arm/mach-omap2/serial.c     |    6 ++++--
>>   drivers/tty/serial/omap-serial.c |    6 +++++-
>>   2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
>> index f660156..427c407 100644
>> --- a/arch/arm/mach-omap2/serial.c
>> +++ b/arch/arm/mach-omap2/serial.c
>> @@ -51,7 +51,7 @@
>>   #define DEFAULT_AUTOSUSPEND_DELAY    -1
>>
>>   #define MAX_UART_HWMOD_NAME_LEN        16
>> -
>> +int force_console_suspend_disable;
>>   struct omap_uart_state {
>>       int num;
>>
>> @@ -207,8 +207,10 @@ static int __init omap_serial_early_init(void)
>>                       uart_name, uart->num);
>>               }
>>
>> -            if (cmdline_find_option("no_console_suspend"))
>> +            if (cmdline_find_option("no_console_suspend")) {
>> +                force_console_suspend_disable = 1;
>>                   no_console_suspend = true;
>> +            }
>>
>>               /*
>>                * omap-uart can be used for earlyprintk logs
>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>> index 08332f3..395ade8 100644
>> --- a/drivers/tty/serial/omap-serial.c
>> +++ b/drivers/tty/serial/omap-serial.c
>> @@ -43,6 +43,7 @@
>>   #include<linux/pinctrl/consumer.h>
>>   #include<linux/platform_data/serial-omap.h>
>>
>> +extern int force_console_suspend_disable;
> This is no good. You need to use the information from
> core serial layer
>
Ok. Will change.
>>   #define OMAP_MAX_HSUART_PORTS    6
>>
>>   #define UART_BUILD_REVISION(x, y)    (((x)<<  8) | (y))
>> @@ -1582,7 +1583,7 @@ 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;
>>
>> -    if (!up)
>> +    if (!up || force_console_suspend_disable)
>>           return -EINVAL;
>>
>>       if (!pdata)
>> @@ -1614,6 +1615,9 @@ static int serial_omap_runtime_resume(struct device *dev)
>>
>>       int loss_cnt = serial_omap_get_context_loss_count(up);
>>
>> +    if (force_console_suspend_disable)
>> +        return -EINVAL;
>> +
>>       if (loss_cnt<  0) {
>>           dev_err(dev, "serial_omap_get_context_loss_count failed : %d\n",
>>               loss_cnt);