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 <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Rajendra nayak <[email protected]>
Signed-off-by: Sourav Poddar <[email protected]>
---
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
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<[email protected]> 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<[email protected]>
>>> Cc: Felipe Balbi<[email protected]>
>>> Cc: Rajendra nayak<[email protected]>
>>> Signed-off-by: Sourav Poddar<[email protected]>
>> 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);
>
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<[email protected]> 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<[email protected]>
>>>> Cc: Felipe Balbi<[email protected]>
>>>> Cc: Rajendra nayak<[email protected]>
>>>> Signed-off-by: Sourav Poddar<[email protected]>
>>> 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);
>>
>
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<[email protected]> 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<[email protected]>
>>>> Cc: Felipe Balbi<[email protected]>
>>>> Cc: Rajendra nayak<[email protected]>
>>>> Signed-off-by: Sourav Poddar<[email protected]>
>>> 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 <[email protected]>
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 <[email protected]>
---
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);
>>
>
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<[email protected]> 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<[email protected]>
>>>>> Cc: Felipe Balbi<[email protected]>
>>>>> Cc: Rajendra nayak<[email protected]>
>>>>> Signed-off-by: Sourav Poddar<[email protected]>
>>>> 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 <[email protected]>
> 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 <[email protected]>
> ---
> 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
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<[email protected]> 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<[email protected]>
>>>>>> Cc: Felipe Balbi<[email protected]>
>>>>>> Cc: Rajendra nayak<[email protected]>
>>>>>> Signed-off-by: Sourav Poddar<[email protected]>
>>>>> 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<[email protected]>
>> 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<[email protected]>
>> ---
>> 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
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<[email protected]> 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<[email protected]>
>>>>>>> Cc: Felipe Balbi<[email protected]>
>>>>>>> Cc: Rajendra nayak<[email protected]>
>>>>>>> Signed-off-by: Sourav Poddar<[email protected]>
>>>>>> 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<[email protected]>
>>> 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<[email protected]>
>>> ---
>>> 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
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<[email protected]> 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<[email protected]>
>>>>>>>> Cc: Felipe Balbi<[email protected]>
>>>>>>>> Cc: Rajendra nayak<[email protected]>
>>>>>>>> Signed-off-by: Sourav Poddar<[email protected]>
>>>>>>> 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<[email protected]>
>>>> 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<[email protected]>
>>>> ---
>>>> 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
Sourav Poddar <[email protected]> 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<[email protected]> 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<[email protected]>
>>>>> Cc: Felipe Balbi<[email protected]>
>>>>> Cc: Rajendra nayak<[email protected]>
>>>>> Signed-off-by: Sourav Poddar<[email protected]>
>>>> 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
Hi Kevin,
On Wednesday 03 April 2013 11:18 PM, Kevin Hilman wrote:
> Sourav Poddar<[email protected]> 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<[email protected]> 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<[email protected]>
>>>>>> Cc: Felipe Balbi<[email protected]>
>>>>>> Cc: Rajendra nayak<[email protected]>
>>>>>> Signed-off-by: Sourav Poddar<[email protected]>
>>>>> 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 <[email protected]>
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 <[email protected]>
---
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
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<[email protected]> 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<[email protected]> 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<[email protected]>
>>>>>>> Cc: Felipe Balbi<[email protected]>
>>>>>>> Cc: Rajendra nayak<[email protected]>
>>>>>>> Signed-off-by: Sourav Poddar<[email protected]>
>>>>>> 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 <[email protected]>
> 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 <[email protected]>
> ---
> 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);
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<[email protected]> 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<[email protected]> 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<[email protected]>
>>>>>>>> Cc: Felipe Balbi<[email protected]>
>>>>>>>> Cc: Rajendra nayak<[email protected]>
>>>>>>>> Signed-off-by: Sourav Poddar<[email protected]>
>>>>>>> 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<[email protected]>
>> 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<[email protected]>
>> ---
>> 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);