linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: khilman@ti.com (Kevin Hilman)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 5/7] Serial: OMAP: add runtime pm support for omap-serial driver
Date: Tue, 08 Mar 2011 17:48:17 -0800	[thread overview]
Message-ID: <87hbbdjc7y.fsf@ti.com> (raw)
In-Reply-To: <AANLkTim35McK1cFFEXyS49D6b8CT+4pQfmKAswOn3e7_@mail.gmail.com> (Govindraj's message of "Tue, 8 Mar 2011 19:34:01 +0530")

Govindraj <govindraj.ti@gmail.com> writes:

[...]

>> This function should not be needed.
>>
>> The timer should be replaced by the auto-suspend feature of runtime PM.
>
> If I use autosuspend based on timer runtime framework
> will disable clocks based on autosuspend timeout.
>
> But if I cut clocks outside sram idle path module level wakeup
> will not work. 
> if I cut func clocks outside prepare for idle
> wake_up is a problem. 

As I mentioned in the other thread, module wakeups are not the problem,
rather the module generating the interrupt is the problem.  So cutting
clocks outside prepare_for_idle path is not the problem.  It's the
re-enabling of the clocks that is important.

> So I am going with existing model where we are cutting clocks in
> prepare idle and enabling the clock back in resume_idle based on
> wakeup event as any other combination doesn't seem to work.
>
> [As stated in other thread to reproduce the issue
> with module level wakeup]
>
> Even with existing framework if I try to experiment
> by cutting clocks after uart timeout in allow_sleep
> func, module level wakeup doesn't seem to happen.
>>
>> Instead of calling omap_port_disable() callers should call
>> pm_runtime_put_autosuspend(), and the console_stop() should be part of
>> the ->runtime_suspend() callback.
>>
>> Also, why do you check for pm_runtime_suspended()? ?Just call it for
>> every time and we get runtime PM use-counting and statistics for free
>> and the ->runtime_suspend() callback will be called when the usecount
>> goes to zero.
>>
>
> Yes correct, but I can't balance the put_sync and get_sync
> as said above. I can call put_sync only once in prepare_idle
> to cut clocks. But having put_sync outside prepare_idle, if
> clocks are cut module level wakeup doesn't seem to happen.
>
>
>>> +static void serial_omap_inactivity_timer(unsigned long uart_no)
>>> +{
>>> + ? ? struct uart_omap_port *up = ui[uart_no];
>>> +
>>> + ? ? up->can_sleep = 1;
>>> + ? ? omap_uart_smart_idle(up);
>>> +}
>>
>> This will not be needed if using the autosuspend feature.
>
> Using autosuspend, module level wakeup will not happen
> since autosuspend cuts clocks outside prepare_idle
> based on autosuspend timeout.

OK, I get the point.  You mentioned module level wakeups about 10 times
in this message.  ;)

However, rather than continue to hack around this problem we need to
understand the root cause.  Thanks to Paul, I think I undersand the root
cause as explained in the other thread, and I think there's a relatively
easy way to make them work.

So here's an experiment to try with autosuspend.  I suspect this will
work, just hack it up to prove the concept.  If it works, we can make
something more generic.  Here are a few alternatives to try.  I may
experiment with some of them tomorrow as well, but please let me know
what you try:

Using autosuspend, clocks will get cut independently of the idle path.
Then, use the PRCM ISR detection of UART module wakeups to call the
UART's interrupt handler.  The interrupt handler will pm_runtime_get(),
enable the clocks, and then take care of the interrupt.  Done.

Alternatively, you could test it on current code by simply removing the
resume_from_idle call from the idle path and calling it instead from the
PRCM ISR when UART module wakeups are detected.

Another option to experiment with: use PRCM ISR to trigger a SW
interrupt of the UART IRQ (cf. MPU_INTC.INTCPS_ISR_SETn registers.)
This may be the cleanest approach, and not require calling into the
driver, but I'm not sure if the normal interrupt clearing process will
also clear the SW interrupt.

The best longer-term solution is will be to use the chained PRCM
interrupt handler[1] (not yet finished.)  Using that, drivers could
directly register for their individual module wakeup interrupts.

Kevin

[1] http://marc.info/?l=linux-omap&m=129258489308837&w=2

  reply	other threads:[~2011-03-09  1:48 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-28 14:39 [PATCH 0/7] OMAP2+: UART: runtime conversion + cleanup Govindraj.R
2011-02-28 14:39 ` [PATCH 1/7] OMAP2+ : hwmod_data: update uart hwmod data Govindraj.R
2011-02-28 14:39 ` [PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads Govindraj.R
2011-03-02  4:49   ` Varadarajan, Charulatha
2011-03-02 10:40     ` Govindraj
2011-02-28 14:39 ` [PATCH 3/7] OMAP2+: UART: Remove certain uart calls from sram_idle Govindraj.R
2011-02-28 14:39 ` [PATCH 4/7] OMAP2+: UART: Remove uart clock handling code serial.c Govindraj.R
2011-02-28 14:39 ` [PATCH 5/7] Serial: OMAP: add runtime pm support for omap-serial driver Govindraj.R
2011-03-05  1:59   ` Kevin Hilman
2011-03-08 14:04     ` Govindraj
2011-03-09  1:48       ` Kevin Hilman [this message]
2011-03-09  2:02         ` Paul Walmsley
2011-03-09 13:03           ` Govindraj
2011-03-09 15:07         ` Govindraj
2011-03-09 23:06           ` Kevin Hilman
2011-02-28 14:39 ` [PATCH 6/7] OMAP: Serial: Allow UART parameters to be configured from board file Govindraj.R
2011-03-01 19:16   ` Sricharan R
2011-03-02  7:40     ` Govindraj
2011-03-02  8:19       ` Sricharan R
2011-03-02 10:07         ` Govindraj
2011-03-02 18:24           ` Tony Lindgren
2011-03-03 12:14             ` Govindraj
2011-03-03  5:08           ` Sricharan R
2011-03-04  6:25             ` Govindraj
2011-02-28 14:39 ` [PATCH 7/7] Serial: OMAP2+: Make the RX_TIMEOUT for DMA configurable for each UART Govindraj.R

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87hbbdjc7y.fsf@ti.com \
    --to=khilman@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).