[PATCH V2 4/4] ARM: OMAP3+: PM: VP: ensure VP is idle before disable

Menon, Nishanth nm at ti.com
Fri Jun 1 18:57:59 EDT 2012


Regards,
Nishanth Menon


On Fri, Jun 1, 2012 at 4:03 PM, Kevin Hilman <khilman at ti.com> wrote:
> Nishanth Menon <nm at ti.com> writes:
>
>> From: Wenbiao Wang <wwang at 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 at atomide.com>
>> Cc: Kevin Hilman <khilman at 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 at ti.com>
>> Signed-off-by: Wenbiao Wang <wwang at 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.

> 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)..

>
>> +                                 __func__, voltdm->name);
>> +     }
>>       /* Disable VP */
>>       vpconfig = voltdm->read(vp->vpconfig);
>>       vpconfig &= ~vp->common->vpconfig_vpenable;
>> diff --git a/arch/arm/mach-omap2/vp.h b/arch/arm/mach-omap2/vp.h
>> index 4655b39..4b4eeb6 100644
>> --- a/arch/arm/mach-omap2/vp.h
>> +++ b/arch/arm/mach-omap2/vp.h
>> @@ -33,9 +33,10 @@ struct voltagedomain;
>>  /*
>>   * Time out for Voltage processor in micro seconds. Typical latency is < 2uS,
>>   * but worst case latencies could be around 3-200uS depending on where we
>> - * interrupted VP's operation.
>> + * interrupted VP's operation. Use an improbable timeout value to be
>> + * sure that timeout events are beyond doubt.
>>   */
>> -#define VP_IDLE_TIMEOUT              200
>> +#define VP_IDLE_TIMEOUT              500
>>  #define VP_TRANXDONE_TIMEOUT 300
>>
>>  /**



More information about the linux-arm-kernel mailing list