Nishanth Menon <[email protected]> writes:

> In general, the idle check tends to be time sensitive so that the
> followon operation after the idle status verification is done in
> a timely manner. However, since we'd be using this in multiple
> places in follow-on patches, isolate this out to a separate inline
> function.
>
> As part of this change, convert the pr_warning to a rate limited
> warning as the situation is most probably going to stick around
> (since Voltage Processor state machine might be stuck) + a WARN_ONCE
> to ensure adequate attention is received. Document the information
> about idle timeout. Based on the idea from Kevin Hilman.
>
> Cc: Tony Lindgren <[email protected]>
> Cc: Kevin Hilman <[email protected]>
> Cc: Eduardo Valentin <[email protected]>
> Signed-off-by: Nishanth Menon <[email protected]>
> ---
>  arch/arm/mach-omap2/vp.c |   55 
> ++++++++++++++++++++++++++++++++++++----------
>  arch/arm/mach-omap2/vp.h |    6 ++++-
>  2 files changed, 49 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/vp.c b/arch/arm/mach-omap2/vp.c
> index faefef7..4723879 100644
> --- a/arch/arm/mach-omap2/vp.c
> +++ b/arch/arm/mach-omap2/vp.c
> @@ -1,5 +1,6 @@
>  #include <linux/kernel.h>
>  #include <linux/init.h>
> +#include <linux/ratelimit.h>
>  
>  #include "common.h"
>  
> @@ -9,6 +10,45 @@
>  #include "prm-regbits-44xx.h"
>  #include "prm44xx.h"
>  
> +/**
> + * _vp_wait_for_idle() - wait for voltage processor to idle
> + * @voltdm:  voltage domain
> + * @vp:              voltage processor instance
> + *
> + * In some conditions, it is important to ensure that Voltage Processor
> + * is idle before performing operations on the Voltage Processor(VP).
> + * This is primarily to ensure that VP state machine does not enter into
> + * invalid state.
> + *
> + * Returns -ETIMEDOUT if timeout occurs - This could be critical failure
> + * as it indicates that Voltage processor might have it's state machine
> + * stuck up without recovering out(theoretically should never happen
> + * ofcourse). Returns 0 if idle state is detected.
> + *
> + * Note: callers are expected to ensure requisite checks are performed
> + * on the pointers passed.
> + */
> +static inline int _vp_wait_for_idle(struct voltagedomain *voltdm,
> +                                 struct omap_vp_instance *vp)
> +{
> +     int timeout;
> +
> +     omap_test_timeout((voltdm->read(vp->vstatus) &
> +                        vp->common->vstatus_vpidle), VP_IDLE_TIMEOUT,
> +                       timeout);
> +
> +     if (timeout >= VP_IDLE_TIMEOUT) {
> +             /* Dont spam the console but ensure we catch attention */
> +             pr_warn_ratelimited("%s: vdd_%s idle timedout\n",

s/idle timedout/timeout waiting for VP idle/

> +                                 __func__, voltdm->name);
> +             WARN_ONCE("vdd_%s idle timedout\n", voltdm->name);
> +

Maybe just leave the WARN_ONCE() here since all the callers are using
the rate-limited pr_warn() in the case of error.  Otherwise, there will
b a bunch of duplicated messages on the console.

> +             return -ETIMEDOUT;
> +     }
> +
> +     return 0;
> +}
> +
>  static u32 _vp_set_init_voltage(struct voltagedomain *voltdm, u32 volt)
>  {
>       struct omap_vp_instance *vp = voltdm->vp;
> @@ -241,7 +281,6 @@ void omap_vp_disable(struct voltagedomain *voltdm)
>  {
>       struct omap_vp_instance *vp;
>       u32 vpconfig;
> -     int timeout;
>  
>       if (!voltdm || IS_ERR(voltdm)) {
>               pr_warning("%s: VDD specified does not exist!\n", __func__);
> @@ -267,16 +306,10 @@ void omap_vp_disable(struct voltagedomain *voltdm)
>       vpconfig &= ~vp->common->vpconfig_vpenable;
>       voltdm->write(vpconfig, vp->vpconfig);
>  
> -     /*
> -      * Wait for VP idle Typical latency is <2us. Maximum latency is ~100us
> -      */
> -     omap_test_timeout((voltdm->read(vp->vstatus) &
> -                        vp->common->vstatus_vpidle), VP_IDLE_TIMEOUT,
> -                       timeout);
> -
> -     if (timeout >= VP_IDLE_TIMEOUT)
> -             pr_warning("%s: vdd_%s idle timedout\n",
> -                     __func__, voltdm->name);
> +     if (_vp_wait_for_idle(voltdm, vp)) {
> +             pr_warn_ratelimited("%s: vdd_%s timedout after disable!!\n",

s/timedout/VP idle timeout/

> +                                 __func__, voltdm->name);
> +     }
>  
>       vp->enabled = false;
>  
> diff --git a/arch/arm/mach-omap2/vp.h b/arch/arm/mach-omap2/vp.h
> index ac1d6cf..4655b39 100644
> --- a/arch/arm/mach-omap2/vp.h
> +++ b/arch/arm/mach-omap2/vp.h
> @@ -30,7 +30,11 @@ struct voltagedomain;
>  #define OMAP4_VP_VDD_IVA_ID 1
>  #define OMAP4_VP_VDD_MPU_ID 2
>  
> -/* XXX document */
> +/*
> + * 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.
> + */
>  #define VP_IDLE_TIMEOUT              200
>  #define VP_TRANXDONE_TIMEOUT 300

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to