[PATCH 6/7] ARM: OMAP2+: powerdomain: add error logs

Jean Pihet jean.pihet at newoldbits.com
Wed Sep 12 05:55:31 EDT 2012


From: Nishanth Menon <nm at ti.com>

Silent failure makes debug hard. So, provide rate limited error
messages in functional and oft-used code to prevent spam
when something goes wrong.

Signed-off-by: Nishanth Menon <nm at ti.com>
---
 arch/arm/mach-omap2/powerdomain.c |  177 +++++++++++++++++++++++++++----------
 1 files changed, 130 insertions(+), 47 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 2277ad3..37dfabf 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -19,6 +19,7 @@
 #include <linux/list.h>
 #include <linux/errno.h>
 #include <linux/string.h>
+#include <linux/ratelimit.h>
 #include <trace/events/power.h>
 
 #include "cm2xxx_3xxx.h"
@@ -146,8 +147,10 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 
 	int prev, next, state, trace_state;
 
-	if (pwrdm == NULL)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	state = pwrdm_read_fpwrst(pwrdm);
 
@@ -174,6 +177,8 @@ static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 		}
 		break;
 	default:
+		pr_err_ratelimited("%s: powerdomain %s: bad flag %0x\n",
+				   __func__, pwrdm->name, flag);
 		return -EINVAL;
 	}
 
@@ -385,6 +390,8 @@ static int _pwrdm_pwrst_to_fpwrst(struct powerdomain *pwrdm, u8 pwrst, u8 logic)
 		ret = PWRDM_FUNC_PWRST_OFF;
 		break;
 	default:
+		pr_err_ratelimited("%s: powerdomain %s: bad pwrst %0x\n",
+				   __func__, pwrdm->name, pwrst);
 		ret = -EINVAL;
 	}
 
@@ -697,8 +704,8 @@ int pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst)
 	unsigned long flags;
 
 	if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0) || (logic < 0)) {
-		pr_debug("%s: invalid params: pwrdm=%p, fpwrst=%0x\n",
-			 __func__, pwrdm, fpwrst);
+		pr_err_ratelimited("%s: invalid params: pwrdm=%p, fpwrst=%0x\n",
+				   __func__, pwrdm, fpwrst);
 		return -EINVAL;
 	}
 
@@ -727,13 +734,17 @@ int pwrdm_set_fpwrst(struct powerdomain *pwrdm, enum pwrdm_func_state fpwrst)
 	trace_power_domain_target(pwrdm->name, next_fpwrst,
 				  smp_processor_id());
 
-	if (logic != pwrdm_read_logic_retst(pwrdm))
-		pwrdm_set_logic_retst(pwrdm, logic);
+	if (logic != pwrdm_read_logic_retst(pwrdm)) {
+		ret = pwrdm_set_logic_retst(pwrdm, logic);
+		if (ret)
+			pr_err_ratelimited("%s: unable to set logic state %0x of powerdomain %s, ret=%d\n",
+					   __func__, logic, pwrdm->name, ret);
+	}
 
 	ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
 	if (ret)
-		pr_err("%s: unable to set power state of powerdomain: %s\n",
-		       __func__, pwrdm->name);
+		pr_err_ratelimited("%s: unable to set power state %0x of powerdomain %s, ret=%d\n",
+				   __func__, pwrst, pwrdm->name, ret);
 
 	switch (sleep_switch) {
 	case FORCEWAKEUP_SWITCH:
@@ -770,8 +781,8 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
 	unsigned long flags;
 
 	if (!pwrdm || IS_ERR(pwrdm) || (pwrst < 0) || (logic < 0)) {
-		pr_debug("%s: invalid params: pwrdm=%p, fpwrst=%0x\n",
-			 __func__, pwrdm, fpwrst);
+		pr_err_ratelimited("%s: invalid params: pwrdm=%p, fpwrst=%0x\n",
+				   __func__, pwrdm, fpwrst);
 		return -EINVAL;
 	}
 
@@ -786,13 +797,13 @@ int pwrdm_set_next_fpwrst(struct powerdomain *pwrdm,
 
 	ret = pwrdm_set_logic_retst(pwrdm, logic);
 	if (ret)
-		pr_err("%s: unable to set logic state %0x of powerdomain: %s\n",
-		       __func__, logic, pwrdm->name);
+		pr_err_ratelimited("%s: unable to set logic state %0x of powerdomain %s, ret=%d\n",
+				   __func__, logic, pwrdm->name, ret);
 
 	ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
 	if (ret)
-		pr_err("%s: unable to set power state %0x of powerdomain: %s\n",
-		       __func__, pwrst, pwrdm->name);
+		pr_err_ratelimited("%s: unable to set power state %0x of powerdomain %s, ret=%d\n",
+				   __func__, pwrst, pwrdm->name, ret);
 
 	spin_unlock_irqrestore(&pwrdm->lock, flags);
 	return ret;
@@ -813,8 +824,10 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	/*
 	 * Do nothing in case the power domain does not have any
@@ -823,8 +836,11 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 	if (!pwrdm->pwrsts)
 		return 0;
 
-	if (!(pwrdm->pwrsts & (1 << pwrst)))
+	if (!(pwrdm->pwrsts & (1 << pwrst))) {
+		pr_err_ratelimited("%s: powerdomain %s: unsupported pwrst %0x\n",
+				   __func__, pwrdm->name, pwrst);
 		return -EINVAL;
+	}
 
 	pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
 		 pwrdm->name, pwrst);
@@ -847,8 +863,10 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	if (arch_pwrdm && arch_pwrdm->pwrdm_read_next_pwrst)
 		ret = arch_pwrdm->pwrdm_read_next_pwrst(pwrdm);
@@ -889,8 +907,10 @@ int pwrdm_read_pwrst(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	if (pwrdm->pwrsts == PWRSTS_ON)
 		return PWRDM_POWER_ON;
@@ -929,8 +949,10 @@ int pwrdm_read_prev_pwrst(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	if (arch_pwrdm && arch_pwrdm->pwrdm_read_prev_pwrst)
 		ret = arch_pwrdm->pwrdm_read_prev_pwrst(pwrdm);
@@ -969,8 +991,10 @@ int pwrdm_set_logic_retst(struct powerdomain *pwrdm, u8 pwrst)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	/*
 	 * Do nothing in case the power domain does not have any
@@ -979,8 +1003,11 @@ int pwrdm_set_logic_retst(struct powerdomain *pwrdm, u8 pwrst)
 	if (!pwrdm->pwrsts_logic_ret)
 		return 0;
 
-	if (!(pwrdm->pwrsts_logic_ret & (1 << pwrst)))
+	if (!(pwrdm->pwrsts_logic_ret & (1 << pwrst))) {
+		pr_err_ratelimited("%s: powerdomain %s: bad pwrst %0x\n",
+				   __func__, pwrdm->name, pwrst);
 		return -EINVAL;
+	}
 
 	pr_debug("powerdomain: setting next logic powerstate for %s to %0x\n",
 		 pwrdm->name, pwrst);
@@ -1010,14 +1037,22 @@ int pwrdm_set_mem_onst(struct powerdomain *pwrdm, u8 bank, u8 pwrst)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
-	if (pwrdm->banks < (bank + 1))
+	if (pwrdm->banks < (bank + 1)) {
+		pr_err_ratelimited("%s: powerdomain %s: bad bank %d\n",
+				   __func__, pwrdm->name, bank);
 		return -EEXIST;
+	}
 
-	if (!(pwrdm->pwrsts_mem_on[bank] & (1 << pwrst)))
+	if (!(pwrdm->pwrsts_mem_on[bank] & (1 << pwrst))) {
+		pr_err_ratelimited("%s: powerdomain %s: bank %d bad pwrst %0x\n",
+				   __func__, pwrdm->name, bank, pwrst);
 		return -EINVAL;
+	}
 
 	pr_debug("powerdomain: setting next memory powerstate for domain %s "
 		 "bank %0x while pwrdm-ON to %0x\n", pwrdm->name, bank, pwrst);
@@ -1048,14 +1083,22 @@ int pwrdm_set_mem_retst(struct powerdomain *pwrdm, u8 bank, u8 pwrst)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
-	if (pwrdm->banks < (bank + 1))
+	if (pwrdm->banks < (bank + 1)) {
+		pr_err_ratelimited("%s: powerdomain %s: bad bank %d\n",
+				   __func__, pwrdm->name, bank);
 		return -EEXIST;
+	}
 
-	if (!(pwrdm->pwrsts_mem_ret[bank] & (1 << pwrst)))
+	if (!(pwrdm->pwrsts_mem_ret[bank] & (1 << pwrst))) {
+		pr_err_ratelimited("%s: powerdomain %s: bank %d bad pwrst %0x\n",
+				   __func__, pwrdm->name, bank, pwrst);
 		return -EINVAL;
+	}
 
 	pr_debug("powerdomain: setting next memory powerstate for domain %s "
 		 "bank %0x while pwrdm-RET to %0x\n", pwrdm->name, bank, pwrst);
@@ -1079,8 +1122,10 @@ int pwrdm_read_logic_pwrst(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	if (arch_pwrdm && arch_pwrdm->pwrdm_read_logic_pwrst)
 		ret = arch_pwrdm->pwrdm_read_logic_pwrst(pwrdm);
@@ -1100,8 +1145,10 @@ int pwrdm_read_prev_logic_pwrst(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	if (arch_pwrdm && arch_pwrdm->pwrdm_read_prev_logic_pwrst)
 		ret = arch_pwrdm->pwrdm_read_prev_logic_pwrst(pwrdm);
@@ -1121,8 +1168,10 @@ int pwrdm_read_logic_retst(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	if (arch_pwrdm && arch_pwrdm->pwrdm_read_logic_retst)
 		ret = arch_pwrdm->pwrdm_read_logic_retst(pwrdm);
@@ -1144,11 +1193,16 @@ int pwrdm_read_mem_pwrst(struct powerdomain *pwrdm, u8 bank)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return ret;
+	}
 
-	if (pwrdm->banks < (bank + 1))
+	if (pwrdm->banks < (bank + 1)) {
+		pr_err_ratelimited("%s: powerdomain %s: bad bank %d\n",
+				   __func__, pwrdm->name, bank);
 		return ret;
+	}
 
 	if (pwrdm->flags & PWRDM_HAS_MPU_QUIRK)
 		bank = 1;
@@ -1174,11 +1228,16 @@ int pwrdm_read_prev_mem_pwrst(struct powerdomain *pwrdm, u8 bank)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return ret;
+	}
 
-	if (pwrdm->banks < (bank + 1))
+	if (pwrdm->banks < (bank + 1)) {
+		pr_err_ratelimited("%s: powerdomain %s: bad bank %d\n",
+				   __func__, pwrdm->name, bank);
 		return ret;
+	}
 
 	if (pwrdm->flags & PWRDM_HAS_MPU_QUIRK)
 		bank = 1;
@@ -1203,11 +1262,16 @@ int pwrdm_read_mem_retst(struct powerdomain *pwrdm, u8 bank)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return ret;
+	}
 
-	if (pwrdm->banks < (bank + 1))
+	if (pwrdm->banks < (bank + 1)) {
+		pr_err_ratelimited("%s: powerdomain %s: bad bank %d\n",
+				   __func__, pwrdm->name, bank);
 		return ret;
+	}
 
 	if (arch_pwrdm && arch_pwrdm->pwrdm_read_mem_retst)
 		ret = arch_pwrdm->pwrdm_read_mem_retst(pwrdm, bank);
@@ -1228,8 +1292,10 @@ int pwrdm_clear_all_prev_pwrst(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return ret;
+	}
 
 	/*
 	 * XXX should get the powerdomain's current state here;
@@ -1260,11 +1326,16 @@ int pwrdm_enable_hdwr_sar(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return ret;
+	}
 
-	if (!(pwrdm->flags & PWRDM_HAS_HDWR_SAR))
+	if (!(pwrdm->flags & PWRDM_HAS_HDWR_SAR)) {
+		pr_err_ratelimited("%s: powerdomain %s: no HDSAR in flag %0x\n",
+				   __func__, pwrdm->name, pwrdm->flags);
 		return ret;
+	}
 
 	pr_debug("powerdomain: %s: setting SAVEANDRESTORE bit\n",
 		 pwrdm->name);
@@ -1290,11 +1361,16 @@ int pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return ret;
+	}
 
-	if (!(pwrdm->flags & PWRDM_HAS_HDWR_SAR))
+	if (!(pwrdm->flags & PWRDM_HAS_HDWR_SAR)) {
+		pr_err_ratelimited("%s: powerdomain %s: no HDSAR in flag %0x\n",
+				   __func__, pwrdm->name, pwrdm->flags);
 		return ret;
+	}
 
 	pr_debug("powerdomain: %s: clearing SAVEANDRESTORE bit\n",
 		 pwrdm->name);
@@ -1331,11 +1407,16 @@ int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
-	if (!(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE))
+	if (!(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
+		pr_err_ratelimited("%s: powerdomain %s:no lowpwrch in flag %0x\n",
+				   __func__, pwrdm->name, pwrdm->flags);
 		return -EINVAL;
+	}
 
 	pr_debug("powerdomain: %s: setting LOWPOWERSTATECHANGE bit\n",
 		 pwrdm->name);
@@ -1360,8 +1441,10 @@ int pwrdm_wait_transition(struct powerdomain *pwrdm)
 {
 	int ret = -EINVAL;
 
-	if (!pwrdm)
+	if (!pwrdm) {
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -EINVAL;
+	}
 
 	if (arch_pwrdm && arch_pwrdm->pwrdm_wait_transition)
 		ret = arch_pwrdm->pwrdm_wait_transition(pwrdm);
@@ -1406,14 +1489,14 @@ int pwrdm_post_transition(struct powerdomain *pwrdm)
  *
  * Context loss count is the sum of powerdomain off-mode counter, the
  * logic off counter and the per-bank memory off counter.  Returns negative
- * (and WARNs) upon error, otherwise, returns the context loss count.
+ * and report upon error, otherwise, returns the context loss count.
  */
 int pwrdm_get_context_loss_count(struct powerdomain *pwrdm)
 {
 	int i, count;
 
 	if (!pwrdm) {
-		WARN(1, "powerdomain: %s: pwrdm is null\n", __func__);
+		pr_err_ratelimited("%s: NULL pwrdm\n", __func__);
 		return -ENODEV;
 	}
 
@@ -1452,8 +1535,8 @@ bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm)
 	int i;
 
 	if (IS_ERR_OR_NULL(pwrdm)) {
-		pr_debug("powerdomain: %s: invalid powerdomain pointer\n",
-			 __func__);
+		pr_err_ratelimited("powerdomain: %s: invalid pwrdm=%p\nr",
+				   __func__, pwrdm);
 		return 1;
 	}
 
-- 
1.7.7.6




More information about the linux-arm-kernel mailing list