From: rnayak@ti.com (Rajendra Nayak)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] OMAP4: clockdomain: Follow recommended enable sequence
Date: Tue, 29 Mar 2011 12:25:11 +0530 [thread overview]
Message-ID: <4D91824F.8020702@ti.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1103281036060.26455@utopia.booyaka.com>
Hi Paul,
<snip>...
>>> Well after the UART timeouts expired, I do not see any powerdomains
>>> transitioning from ON.
>>>
>>> What's even more strange is that the same thing is working fine on all
>>> the other OMAP3 platforms I tested: 3430/n900, 3630/zoom3 and even a
>>> different 3530-based platform, the OMAP3EVM.
>>
>> I tried to reproduce this on a Beagle rev C3, but I don't seem
>> to be seeing this issue. I was able to hit OFF mode in both
>> suspend and idle.
>>
>> I also tried removing autodeps completely on OMAP3 and ran
>> into some issues/aborts with GPIO restore path with
>> OFF mode enabled.
>>
>> Besides these, debugging some McBSP related crashes showed
>> up another issue with this patch.
>> Since the clockdomain is programmed back to HW_AUTO (if supported)
>> in the clock framework, this happens *before* waiting for the
>> module to become accessible. (On OMAP4, the check to make sure
>> the module is accessible happens in the hwmod framework, unlike
>> in older OMAP's, where it was part of the clock framework)
>>
>> So instead of implementing the recommended sequence of
>> -1- Force clkdm to SW_WKUP
>> -2- Configure desired module mode to "enable" or "auto"
>> -3- Wait for the desired module idle status to be FUNC
>> -4- Program clkdm in HW_AUTO(if supported)
>>
>> ..it was actually implementing the wrong sequence as below
>> -1- Force clkdm to SW_WKUP
>> -2- Configure desired module mode to "enable" or "auto"
>> -3- Program clkdm in HW_AUTO(if supported)
>> -4- Wait for the desired module idle status to be FUNC
>
> Hmmm, right now OMAP4 only appears to be forcing the clkdm to SW_WKUP if
> the clockdomain is in software-supervised mode
> (clockdomain44xx.c:omap4_clkdm_clk_enable()). Doesn't seem like that
> follows either sequence?
Yes, the current implementation only forces it to SW_WKUP
if the clockdomain is in software supervised mode which is
not correct.
Thats what this patch intended to fix, but like I said
the sequence followed in this patch was still not right.
>
>> This however was only a problem on OMAP4.
>>
>> Fixing this would require moving the clockdomain programming
>> back to HW_AUTO as part of the hwmod framework.
>
> In omap_hwmod.c:_enable(), what do you think about:
>
> 1. saving the current idle mode of the clockdomain,
>
> 2. forcing the clockdomain to software-supervised wakeup.
>
> 3. enabling clocks and waiting for the module as we currently do, then
>
> 4. switching the clockdomain's idle mode back to its original state?
>
> Seems like that would be a reasonable approach for the short term, at
> least for drivers that have been converted to PM runtime.
Ok, I'll try and get some RFC patches in these lines soon.
>
>> However this sequence is recommended even for optional clock enabling,
>> and hence might have to be handled at the clock framework as well.
>
> Might be worth finding out the reasoning behind this recommendation. Is
> this only for optional clocks that are used for functional purposes, e.g.,
> for modules that use HWMOD_CONTROL_OPT_CLKS_IN_RESET ?
Ok, I'll try and dig some more details on this.
regards,
Rajendra
>
>> (Since optional clocks are still controlled by drivers using clock
>> framework directly).
>
> Yeah, if this really turns out to be needed, sounds like we'll have to
> tightly couple the hwmod code with the OMAP clock code :-( I'd suggest
> that we find out why this is recommended first.
>
>> Any suggestions on how to handle this without duplicating
>> much of this across clock and hwmod framework?
>
>
> - Paul
next prev parent reply other threads:[~2011-03-29 6:55 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-03-04 13:25 [PATCH] OMAP4: clockdomain: Follow recommended enable sequence Rajendra Nayak
2011-03-09 3:50 ` Paul Walmsley
2011-03-09 10:19 ` Rajendra Nayak
2011-03-09 16:28 ` Kevin Hilman
2011-03-09 21:44 ` Paul Walmsley
2011-03-10 12:18 ` Paul Walmsley
2011-03-10 12:58 ` Rajendra Nayak
2011-03-10 13:16 ` Paul Walmsley
2011-03-10 13:17 ` Paul Walmsley
2011-03-10 13:34 ` Ben Dooks
2011-03-10 13:36 ` Paul Walmsley
2011-03-10 14:39 ` Paul Walmsley
2011-03-11 13:26 ` Rajendra Nayak
2011-03-11 16:47 ` Kevin Hilman
2011-03-12 7:53 ` Rajendra Nayak
2011-03-14 10:58 ` Rajendra Nayak
2011-03-21 8:51 ` Rajendra Nayak
2011-03-28 17:04 ` Paul Walmsley
2011-03-29 6:55 ` Rajendra Nayak [this message]
2011-04-01 14:51 ` Rajendra Nayak
2011-04-01 15:40 ` Rajendra Nayak
2011-04-04 6:47 ` Paul Walmsley
2011-04-04 6:57 ` Santosh Shilimkar
2011-04-05 12:47 ` Rajendra Nayak
2011-03-23 23:29 ` Paul Walmsley
2011-04-20 19:42 ` Paul Walmsley
2011-04-21 4:47 ` Santosh Shilimkar
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=4D91824F.8020702@ti.com \
--to=rnayak@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).