Nishanth Menon <nm@xxxxxx> 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 <tony@xxxxxxxxxxx>
> Cc: Kevin Hilman <khilman@xxxxxx>
> Cc: Eduardo Valentin <eduardo.valentin@xxxxxx>
> Signed-off-by: Nishanth Menon <nm@xxxxxx>
> ---
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
