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

From: Sourav Poddar
Date: Tue Apr 02 2013 - 07:44:18 EST


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

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