[PATCH v7 20/21] OMAP2+: UART: Avoid uart idling on suspend for no_console_suspend usecase

[PATCH v7 20/21] OMAP2+: UART: Avoid uart idling on suspend for no_console_suspend usecase

[Govindraj.R]

If no_console_suspend is used we have prevent uart idling during suspend
to provide debug prints.

Power domain hooks can idle uarts if left enabled during system wide suspend
so re-use the omap_device_disable_idle_on_suspend API's to ensure console_uart
is not idled during suspend.

omap_device_disable_idle_on_suspend API was used on all uarts since the uart
driver was not runtime adapted, now with runtime adaptation we can re-use this
API only for no_console_suspend use cases.

Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 arch/arm/mach-omap2/serial.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 55903f0..5e3bbce 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -64,6 +64,7 @@ struct omap_uart_state {
 static LIST_HEAD(uart_list);
 static u8 num_uarts;
 static u8 console_uart_id = -1;
+static u8 no_console_suspend;
 
 #define DEFAULT_RXDMA_POLLRATE		1	/* RX DMA polling rate (us) */
 #define DEFAULT_RXDMA_BUFSIZE		4096	/* RX DMA buffer size */
@@ -308,6 +309,10 @@ static int __init omap_serial_early_init(void)
 
 		if (cmdline_find_option(uart_name)) {
 			console_uart_id = uart->num;
+
+			if (cmdline_find_option("no_console_suspend"))
+				no_console_suspend = true;
+
 			/*
 			 * omap-uart can be used for earlyprintk logs
 			 * So if omap-uart is used as console then prevent
@@ -400,7 +405,9 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
 		od->pm_lats = console_uart_latency;
 	}
 
-	omap_device_disable_idle_on_suspend(pdev);
+	if (no_console_suspend)
+		omap_device_disable_idle_on_suspend(pdev);
+
 	oh->mux = omap_hwmod_mux_init(bdata->pads, bdata->pads_cnt);
 
 	uart->pdev = pdev;
-- 
1.7.4.1

[PATCH v7 21/21] OMAP2+: UART: Do not gate uart clocks if used for debug_prints

[Govindraj.R]

If OMAP UART is used as console uart and debug is enabled,
avoid gating of uart clocks to print all debug prints.

If uart clocks are gated then the debug prints from omap_device
framework or hwmod framework can cause uart to enter recursive
pm_runtime calls, which can cause a deadlock over power lock usage.

For example: Say, uart clocks are cut and we get a print from
omap_device_disable stating disabling uart clocks. This print
calls omap_uart driver console_write which will call runtime API
get_sync which means we enter from runtime API put context to
runtime API get context.

--> runtime put (take power lock)
    --> print disabling uart clocks
        --> call uart console write
            --> call get_sync (try to take power lock)

Also any clock enable API call from uart driver should not call any uart
operation until clocks are enabled back. Like get_sync having debug print
calling uart console write even before clocks are enabled.

So to avoid these scenarios, identify from bootargs if OMAP_UART(ttyO) is used
in debug mode. If so, do not set device_may_wakeup. This will prevent
pm_runtime_enable in uart driver and will avoid uart clock gating.

Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 arch/arm/mach-omap2/serial.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 5e3bbce..e83951a 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -65,6 +65,7 @@ static LIST_HEAD(uart_list);
 static u8 num_uarts;
 static u8 console_uart_id = -1;
 static u8 no_console_suspend;
+static u8 uart_debug;
 
 #define DEFAULT_RXDMA_POLLRATE		1	/* RX DMA polling rate (us) */
 #define DEFAULT_RXDMA_BUFSIZE		4096	/* RX DMA buffer size */
@@ -310,6 +311,13 @@ static int __init omap_serial_early_init(void)
 		if (cmdline_find_option(uart_name)) {
 			console_uart_id = uart->num;
 
+			if (cmdline_find_option("debug")) {
+				uart_debug = true;
+				pr_info("%s used as console in debug mode"
+						" uart%d clocks will not be"
+						" gated", uart_name, uart->num);
+			}
+
 			if (cmdline_find_option("no_console_suspend"))
 				no_console_suspend = true;
 
@@ -414,7 +422,8 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
 
 	oh->dev_attr = uart;
 
-	if ((cpu_is_omap34xx() || cpu_is_omap44xx()) && bdata->pads)
+	if (((cpu_is_omap34xx() || cpu_is_omap44xx()) && bdata->pads)
+			&& !uart_debug)
 		device_init_wakeup(&pdev->dev, true);
 }
 
-- 
1.7.4.1

[PATCH v7 21/21] OMAP2+: UART: Do not gate uart clocks if used for debug_prints

[Mohammed, Afzal]

Hi Govindraj,

On Tue, Oct 18, 2011 at 21:05:41, R, Govindraj wrote:
> If OMAP UART is used as console uart and debug is enabled,
> avoid gating of uart clocks to print all debug prints.
: 
:
>  			console_uart_id = uart->num;
>  
> +			if (cmdline_find_option("debug")) {
> +				uart_debug = true;

Shouldn't "loglevel" also be checked?

Regards
Afzal

[PATCH v7 21/21] OMAP2+: UART: Do not gate uart clocks if used for debug_prints

[Mohammed, Afzal]

Hi Govindraj,

On Wed, Oct 19, 2011 at 11:05:02, Mohammed, Afzal wrote:
> Hi Govindraj,
> 
> On Tue, Oct 18, 2011 at 21:05:41, R, Govindraj wrote:
> > If OMAP UART is used as console uart and debug is enabled,
> > avoid gating of uart clocks to print all debug prints.
> : 
> :
> >  			console_uart_id = uart->num;
> >  
> > +			if (cmdline_find_option("debug")) {
> > +				uart_debug = true;
> 
> Shouldn't "loglevel" also be checked?

Or use console_loglevel instead ?

Regards
Afzal

[PATCH v7 21/21] OMAP2+: UART: Do not gate uart clocks if used for debug_prints

[Govindraj]

On Wed, Oct 19, 2011 at 11:05 AM, Mohammed, Afzal <afzal@ti.com> wrote:
> Hi Govindraj,
>
> On Tue, Oct 18, 2011 at 21:05:41, R, Govindraj wrote:
>> If OMAP UART is used as console uart and debug is enabled,
>> avoid gating of uart clocks to print all debug prints.
> :
> :
>> ? ? ? ? ? ? ? ? ? ? ? console_uart_id = uart->num;
>>
>> + ? ? ? ? ? ? ? ? ? ? if (cmdline_find_option("debug")) {
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? uart_debug = true;
>
> Shouldn't "loglevel" also be checked?
>

Might not be necessary,

loglevel only sets the option in console driver how a printk should be handled
from vprintk so any printk should be to sent to console or to syslogd action
where it will be buffered to a file(var/log/messages).

Any print's with any log level should be handled and printed to console.

consider below path below path

uart->ops
 -> runtime_get_sync
      -> omap_device_enable
          -> clock_enable

All the prints in given path are dev_dbgs for which we need debug to be set from
command line argument.

As stated in change log any dev_dbg prints from these path can be
redirect to uart->ops
so even before our clock was enabled we got redirected back to uart code.

So for dev_dbg prints from above path we need debug to be set from command-line
and we can prevent uart clock gating to this scenario.

--
Govindraj.R

[PATCH v7 21/21] OMAP2+: UART: Do not gate uart clocks if used for debug_prints

[Mohammed, Afzal]

Hi Govindraj,

On Wed, Oct 19, 2011 at 18:22:58, Govindraj wrote:
> On Wed, Oct 19, 2011 at 11:05 AM, Mohammed, Afzal <afzal@ti.com> wrote:
> > Hi Govindraj,
> >
> > On Tue, Oct 18, 2011 at 21:05:41, R, Govindraj wrote:
> >> If OMAP UART is used as console uart and debug is enabled,
> >> avoid gating of uart clocks to print all debug prints.
> > :
> > :
> >> ? ? ? ? ? ? ? ? ? ? ? console_uart_id = uart->num;
> >>
> >> + ? ? ? ? ? ? ? ? ? ? if (cmdline_find_option("debug")) {
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? uart_debug = true;
> >
> > Shouldn't "loglevel" also be checked?
> >
> 
> Might not be necessary,
> 
> loglevel only sets the option in console driver how a printk should be handled
> from vprintk so any printk should be to sent to console or to syslogd action
> where it will be buffered to a file(var/log/messages).
> 
> Any print's with any log level should be handled and printed to console.
> 
> consider below path below path
> 
> uart->ops
>  -> runtime_get_sync
>       -> omap_device_enable
>           -> clock_enable
> 
> All the prints in given path are dev_dbgs for which we need debug to be set from
> command line argument.
> 
> As stated in change log any dev_dbg prints from these path can be
> redirect to uart->ops
> so even before our clock was enabled we got redirected back to uart code.
> 
> So for dev_dbg prints from above path we need debug to be set from command-line
> and we can prevent uart clock gating to this scenario.

Isn't passing bootargs "loglevel=10" same as "debug"? (init/main.c)

Regards
Afzal

[PATCH v7 21/21] OMAP2+: UART: Do not gate uart clocks if used for debug_prints

[Govindraj]

On Wed, Oct 19, 2011 at 7:50 PM, Mohammed, Afzal <afzal@ti.com> wrote:
> Hi Govindraj,
>
> On Wed, Oct 19, 2011 at 18:22:58, Govindraj wrote:
>> On Wed, Oct 19, 2011 at 11:05 AM, Mohammed, Afzal <afzal@ti.com> wrote:
>> > Hi Govindraj,
>> >
>> > On Tue, Oct 18, 2011 at 21:05:41, R, Govindraj wrote:
>> >> If OMAP UART is used as console uart and debug is enabled,
>> >> avoid gating of uart clocks to print all debug prints.
>> > :
>> > :
>> >> ? ? ? ? ? ? ? ? ? ? ? console_uart_id = uart->num;
>> >>
>> >> + ? ? ? ? ? ? ? ? ? ? if (cmdline_find_option("debug")) {
>> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? uart_debug = true;
>> >
>> > Shouldn't "loglevel" also be checked?
>> >
>>
>> Might not be necessary,
>>
>> loglevel only sets the option in console driver how a printk should be handled
>> from vprintk so any printk should be to sent to console or to syslogd action
>> where it will be buffered to a file(var/log/messages).
>>
>> Any print's with any log level should be handled and printed to console.
>>
>> consider below path below path
>>
>> uart->ops
>> ?-> runtime_get_sync
>> ? ? ? -> omap_device_enable
>> ? ? ? ? ? -> clock_enable
>>
>> All the prints in given path are dev_dbgs for which we need debug to be set from
>> command line argument.
>>
>> As stated in change log any dev_dbg prints from these path can be
>> redirect to uart->ops
>> so even before our clock was enabled we got redirected back to uart code.
>>
>> So for dev_dbg prints from above path we need debug to be set from command-line
>> and we can prevent uart clock gating to this scenario.
>
> Isn't passing bootargs "loglevel=10" same as "debug"? (init/main.c)

Yes correct, I missed that part here is the updated patch [1]
Updated change-log for debug/loglevel and checked console_loglevel
in addition to debug in bootargs.

Same is updated in the branch hosted for v7_runtime:
https://gitorious.org/runtime_3-0/runtime_3-0/commit/3775d72f1205d218eb423a83ed87afe4c643ef38

git://gitorious.org/runtime_3-0/runtime_3-0.git v7_rc9_uart_runtime

--
Thanks,
Govindraj.R

[1]:

[PATCH v7 21/21] OMAP2+: UART: Do not gate uart clocks if used for debug_prints

[Mohammed, Afzal]

Hi Govindraj,

On Thu, Oct 20, 2011 at 14:09:22, Govindraj wrote:
> Yes correct, I missed that part here is the updated patch [1]
> Updated change-log for debug/loglevel and checked console_loglevel
> in addition to debug in bootargs.

Perhaps checking console_loglevel only is sufficient, if I am not
missing anything, passing "debug" thr' bootargs only causes loglevel
to be set to 10 and nothing else.

Regards
Afzal

[PATCH v7 21/21] OMAP2+: UART: Do not gate uart clocks if used for debug_prints

[Govindraj]

On Thu, Oct 20, 2011 at 2:58 PM, Mohammed, Afzal <afzal@ti.com> wrote:
> Hi Govindraj,
>
> On Thu, Oct 20, 2011 at 14:09:22, Govindraj wrote:
>> Yes correct, I missed that part here is the updated patch [1]
>> Updated change-log for debug/loglevel and checked console_loglevel
>> in addition to debug in bootargs.
>
> Perhaps checking console_loglevel only is sufficient, if I am not
> missing anything, passing "debug" thr' bootargs only causes loglevel
> to be set to 10 and nothing else.

Agree, Updated [1]

--
Thanks,
Govindraj.R

[1]

https://gitorious.org/runtime_3-0/runtime_3-0/commit/4b04aa54e61540f1f358ef412f2aaa3bab8de7c1


diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 5e3bbce..4f41369 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -65,6 +65,7 @@ static LIST_HEAD(uart_list);
 static u8 num_uarts;
 static u8 console_uart_id = -1;
 static u8 no_console_suspend;
+static u8 uart_debug;

 #define DEFAULT_RXDMA_POLLRATE         1       /* RX DMA polling rate (us) */
 #define DEFAULT_RXDMA_BUFSIZE          4096    /* RX DMA buffer size */
@@ -310,6 +311,13 @@ static int __init omap_serial_early_init(void)
                if (cmdline_find_option(uart_name)) {
                        console_uart_id = uart->num;

+                       if (console_loglevel >= 10) {
+                               uart_debug = true;
+                               pr_info("%s used as console in debug mode"
+                                               " uart%d clocks will not be"
+                                               " gated", uart_name, uart->num);
+                       }
+
                        if (cmdline_find_option("no_console_suspend"))
                                no_console_suspend = true;

@@ -414,7 +422,8 @@ void __init omap_serial_init_port(struct
omap_board_data *bdata,

        oh->dev_attr = uart;

-       if ((cpu_is_omap34xx() || cpu_is_omap44xx()) && bdata->pads)
+       if (((cpu_is_omap34xx() || cpu_is_omap44xx()) && bdata->pads)
+                       && !uart_debug)
                device_init_wakeup(&pdev->dev, true);
 }