From: khilman@ti•com (Kevin Hilman)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH V2 4/4] ARM: OMAP3+: PM: VP: ensure VP is idle before disable
Date: Mon, 04 Jun 2012 09:49:52 -0700 [thread overview]
Message-ID: <87vcj7f2dr.fsf@ti.com> (raw)
In-Reply-To: <CAOMWX4fyJNtjm+kGgY=5yWaMeSAEYMTxuzyyrQibcH3=3wfMoA@mail.gmail.com> (Nishanth Menon's message of "Fri, 1 Jun 2012 17:57:59 -0500")
"Menon, Nishanth" <nm@ti•com> writes:
> Regards,
> Nishanth Menon
>
>
> On Fri, Jun 1, 2012 at 4:03 PM, Kevin Hilman <khilman@ti•com> wrote:
>> Nishanth Menon <nm@ti•com> writes:
>>
>>> From: Wenbiao Wang <wwang@ti•com>
>>>
>>> Voltage Processor state machine transition to disable need to
>>> occur from IDLE state. When we transition OPP in a functioning
>>> system, the call sequence for an OPP transition is as follows:
>>> omap_sr_disable
>>> ? ? ? -> sr class 3 disable
>>> ? ? ? ? ? ?-> vp disable
>>> ? ? ? ? ? ?-> sr disable
>>> forceupdate to voltage/frequency scale depending on which OPP
>>> we are transitioning to.
>>>
>>> If we hit a critical timing window where SR had commanded VP
>>> for a voltage transition and VP is in the middle of operating
>>> on that command, it needs to go through a few states before
>>> going to update state(where it actually sends the command to
>>> VC). Initial view of h/w owners is that the state disable of VP
>>> is expected to be sampled for the next transition.
>>>
>>> Instead, to be on a safer side, we ensure that the valid states
>>> of the VP state machine is diligently followed by software. This
>>> can be done by waiting for VP to be in idle ?prior to disabling
>>> VP. Existing prints have been updated to ensure context is
>>> available on error messages.
>>>
>>> As part of this change, increase timeout for VP idle check to
>>> improbable 500uSec to be certain that system is indeed unable
>>> to continue before crashing out with error(worst case expectancy
>>> remains the same 3-100uSec depending on when we caught VP).
>>>
>>> Cc: Tony Lindgren <tony@atomide•com>
>>> Cc: Kevin Hilman <khilman@ti•com>
>>>
>>> [nm at ti.com: port from android]
>>
>> and you also convert to use new _vp_wait_for_idle()
>>
>>> Signed-off-by: Nishanth Menon <nm@ti•com>
>>> Signed-off-by: Wenbiao Wang <wwang@ti•com>
>>> ---
>>> ?arch/arm/mach-omap2/vp.c | ? ?4 ++++
>>> ?arch/arm/mach-omap2/vp.h | ? ?5 +++--
>>> ?2 files changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/vp.c b/arch/arm/mach-omap2/vp.c
>>> index 2a8a085..9a72deb 100644
>>> --- a/arch/arm/mach-omap2/vp.c
>>> +++ b/arch/arm/mach-omap2/vp.c
>>> @@ -308,6 +308,10 @@ void omap_vp_disable(struct voltagedomain *voltdm)
>>> ? ? ? ? ? ? ? return;
>>> ? ? ? }
>>>
>>> + ? ? if (_vp_wait_for_idle(voltdm, vp)) {
>>> + ? ? ? ? ? ? pr_warn_ratelimited("%s: vdd_%s timedout!Ignore and try\n",
>>
>> s/timedout/timed out/
>> no space after '!',
> Kinda wanted to stay under 80 character and not split string out to
> two lines and make sparse angry, yet did not want to loose information
> which was being presented out.
Readable error messages are more important.
>> also I don't get the "Ignore and try" part
>
> if we fail, just try the disable anyways..
> (at least till we have
> voltage processor recovery mechanism(cold reset) introduced upstream -
> the intent of the patch was not to introduce a recovery mechanism, but
> to ensure proper checkpoint is in place)..
I understand. My complaint is only about the readability of the error
messages. Seeing this go by:
omap_vp_disable: vdd_mpu timedout!Ignore and try
in the kernel logs will still make me ask "try what?" IMO, it should say
omap_vp_disable: WARNING: vdd_mpu timed out, ignoring.
Kevin
prev parent reply other threads:[~2012-06-04 16:49 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-01 1:41 [PATCH V4 0/4] ARM: OMAP3+: VP: collate fixes Nishanth Menon
2012-06-01 1:41 ` [PATCH V2 1/4] ARM: OMAP3+: PM: VP: check only the VPINIDLE status bit Nishanth Menon
2012-06-01 1:41 ` [PATCH V2 2/4] ARM: OMAP3+: PM: VP: move check of idle to separate function Nishanth Menon
2012-06-01 21:07 ` Kevin Hilman
2012-06-01 1:41 ` [PATCH V2 3/4] ARM: OMAP3+: PM: VP: check to ensure VP is idle before forceupdate Nishanth Menon
2012-06-01 21:08 ` Kevin Hilman
2012-06-01 1:41 ` [PATCH V2 4/4] ARM: OMAP3+: PM: VP: ensure VP is idle before disable Nishanth Menon
2012-06-01 21:03 ` Kevin Hilman
2012-06-01 22:57 ` Menon, Nishanth
2012-06-04 16:49 ` Kevin Hilman [this message]
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=87vcj7f2dr.fsf@ti.com \
--to=khilman@ti$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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