From patchwork Wed Sep 12 09:55:31 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jean Pihet X-Patchwork-Id: 1441901 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (unknown [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id EF8FFDF28C for ; Wed, 12 Sep 2012 10:07:50 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TBjh0-0006xp-De; Wed, 12 Sep 2012 09:57:10 +0000 Received: from mail-wi0-f177.google.com ([209.85.212.177]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1TBjfo-0006hk-LD for linux-arm-kernel@lists.infradead.org; Wed, 12 Sep 2012 09:56:06 +0000 Received: by wibhn17 with SMTP id hn17so1291378wib.0 for ; Wed, 12 Sep 2012 02:55:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:subject:date:message-id:x-mailer:in-reply-to:references :x-gm-message-state; bh=gSbFsEnfPkyNapQpCc5PvZyL7AMtBJ4hEXTCcKEgwRo=; b=foYJf0soIYyVuS4cudkqyncMEypPbulDw6QsCxvdBbiZgyRaKPlTv4ckVD9RBm9zQo X2Rjq6BoxXds59c6OyivcMwuGmq2QDI6RtdrdDPY3Qqcv/fEUitRVyykClwfeSN/K8s0 8ftgc9llg5s7LuSUfNnt8eOvdeHqaLQ/RfPRW+2LKxnyMC8jQ56MKAIS3hYUD9qMWuYX 5Ajce3tlFkuCQsT5dfghFjQRUDPV0HKASu1UhKtOnBX7sYNwE5GDLqnDuS6pnoNmXzsL sCYV/vTyIs83pwpiGuNePIB2t5KCXqLHhCd6KoVxhDhP566UUr695xp4vaB4tIYGkcxU SZMw== Received: by 10.180.100.35 with SMTP id ev3mr31579272wib.7.1347443755457; Wed, 12 Sep 2012 02:55:55 -0700 (PDT) Received: from localhost.localdomain (52.60-66-87.adsl-dyn.isp.belgacom.be. [87.66.60.52]) by mx.google.com with ESMTPS id w7sm7312997wiz.0.2012.09.12.02.55.54 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 12 Sep 2012 02:55:54 -0700 (PDT) From: Jean Pihet To: linux-omap@vger.kernel.org, paul@pwsan.com, linux-arm-kernel@lists.infradead.org, khilman@ti.com, Benoit Cousson , Santosh Shilimkar , Nishanth Menon , Rajendra Nayak Subject: [PATCH 6/7] ARM: OMAP2+: powerdomain: add error logs Date: Wed, 12 Sep 2012 11:55:31 +0200 Message-Id: <1347443732-7411-7-git-send-email-j-pihet@ti.com> X-Mailer: git-send-email 1.7.7.6 In-Reply-To: <1347443732-7411-1-git-send-email-j-pihet@ti.com> References: <1347443732-7411-1-git-send-email-j-pihet@ti.com> X-Gm-Message-State: ALoCoQk8gi6l/G+ENAR8ah5JGJTaBqTSgc4Nf+x7hzvdZSWNGiEFtE3zIpmzc8LLVQaw42fv1/J+ X-Spam-Note: CRM114 invocation failed X-Spam-Score: -2.6 (--) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-2.6 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [209.85.212.177 listed in list.dnswl.org] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org From: Nishanth Menon 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 --- 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 #include #include +#include #include #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; }