ARM: OMAP3+: VP: collate fixes

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

arch/arm/mach-omap2/vp.c +21 -0

index faefef7..4723879 100644
@@ -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)

arch/arm/mach-omap2/vp.c +6 -3

+		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

arch/arm/mach-omap2/vp.c +2 -0

+				    __func__, voltdm->name);
+	}
 
 	vp->enabled = false;
 

arch/arm/mach-omap2/vp.h +0 -0

index ac1d6cf..4655b39 100644
@@ -30,7 +30,11 @@ struct voltagedomain;
 #define OMAP4_VP_VDD_IVA_ID 1

Ideally in the flow of DVFS programming, VP should be in idle state (since we disabled it) before entering forceupdate. Ensure that this is the case. Not doing this could cause VP statemachine to enter invalid states. Use ratelimited warnings to prevent spam if VP state machine is stuck.

Cc Tony Lindgren <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Vinay Amancha <[email protected]>

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/vp.c |    7 +++++++
 1 file changed, 7 insertions(+)

arch/arm/mach-omap2/vp.c +0 -0

index 4723879..2a8a085 100644

arch/arm/mach-omap2/vp.c +4 -0

+				    __func__, voltdm->name, target_volt);
+		return ret;
+	}
+
 	ret = omap_vc_pre_scale(voltdm, target_volt, &target_vsel, &current_vsel);
 	if (ret)
 		return ret;

From: Wenbiao Wang <[email protected]>

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 <[email protected]>

Cc Kevin Hilman <[email protected]>

[nm at ti.com: port from android]

Signed-off-by Nishanth Menon <[email protected]>

Signed-off-by Wenbiao Wang <[email protected]>

---
 arch/arm/mach-omap2/vp.c |    4 ++++
 arch/arm/mach-omap2/vp.h |    5 +++--
 2 files changed, 7 insertions(+), 2 deletions(-)

arch/arm/mach-omap2/vp.c +0 -0

index 2a8a085..9a72deb 100644

arch/arm/mach-omap2/vp.c +2 -0

+				    __func__, voltdm->name);
+	}
 	/* Disable VP */
 	vpconfig = voltdm->read(vp->vpconfig);
 	vpconfig &= ~vp->common->vpconfig_vpenable;

arch/arm/mach-omap2/vp.h +3 -2

index 4655b39..4b4eeb6 100644
@@ -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
 
 /**

Quoted 30 lines · Nishanth Menon

From: Wenbiao Wang <[email protected]>

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

Quoted 4 lines

Cc Tony Lindgren <[email protected]>

Cc Kevin Hilman <[email protected]>

[nm at ti.com: port from android]

and you also convert to use new _vp_wait_for_idle()

Quoted 15 lines

Signed-off-by Nishanth Menon <[email protected]>

Signed-off-by Wenbiao Wang <[email protected]>

---
?arch/arm/mach-omap2/vp.c | ? ?4 ++++
?arch/arm/mach-omap2/vp.h | ? ?5 +++--
?2 files changed, 7 insertions(+), 2 deletions(-)

arch/arm/mach-omap2/vp.c +0 -0

index 2a8a085..9a72deb 100644
@@ -308,6 +308,10 @@ void omap_vp_disable(struct voltagedomain *voltdm)
 ? ? ? ? ? ? ? return;
 ? ? ? }
 

arch/arm/mach-omap2/vp.c void omap_vp_disable(struct voltagedomain *voltdm) · line 312

Quoted 2 lines

arch/arm/mach-omap2/vp.c +2 -0

+ ? ? 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 '!',

also I don't get the "Ignore and try" part

Quoted 23 lines

arch/arm/mach-omap2/vp.c +2 -0

+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __func__, voltdm->name);
+ ? ? }

? ? ? /* Disable VP */
? ? ? vpconfig = voltdm->read(vp->vpconfig);
? ? ? vpconfig &= ~vp->common->vpconfig_vpenable;

arch/arm/mach-omap2/vp.h +0 -0

index 4655b39..4b4eeb6 100644
@@ -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.
? */

arch/arm/mach-omap2/vp.h +1 -1

-#define VP_IDLE_TIMEOUT ? ? ? ? ? ? ?200
+#define VP_IDLE_TIMEOUT ? ? ? ? ? ? ?500

?#define VP_TRANXDONE_TIMEOUT 300

?/**

Regards,
Nishanth Menon

Quoted 54 lines · Kevin Hilman

Quoted 30 lines · Nishanth Menon

From: Wenbiao Wang <[email protected]>

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

Quoted 4 lines

Cc Tony Lindgren <[email protected]>

Cc Kevin Hilman <[email protected]>

[nm at ti.com: port from android]

and you also convert to use new _vp_wait_for_idle()

Quoted 15 lines

Signed-off-by Nishanth Menon <[email protected]>

Signed-off-by Wenbiao Wang <[email protected]>

---
?arch/arm/mach-omap2/vp.c | ? ?4 ++++
?arch/arm/mach-omap2/vp.h | ? ?5 +++--
?2 files changed, 7 insertions(+), 2 deletions(-)

arch/arm/mach-omap2/vp.c +0 -0

index 2a8a085..9a72deb 100644
@@ -308,6 +308,10 @@ void omap_vp_disable(struct voltagedomain *voltdm)
 ? ? ? ? ? ? ? return;
 ? ? ? }
 

Quoted 5 lines

arch/arm/mach-omap2/vp.c void omap_vp_disable(struct voltagedomain *voltdm) · line 312

Quoted 2 lines

arch/arm/mach-omap2/vp.c +2 -0

+ ? ? 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.

Quoted 1 line

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