* [PATCH] ARM:OMAP2: an issue about curr_pwrst, u8 is never < 0
@ 2013-03-14 4:21 Chen Gang
2013-04-01 10:25 ` Paul Walmsley
0 siblings, 1 reply; 3+ messages in thread
From: Chen Gang @ 2013-03-14 4:21 UTC (permalink / raw)
To: linux-arm-kernel
if pwrdm_read_pwrst returns negative number, curr_pwrst can not notice it.
since really need check curr_pwrst whether is negative,
need let the check valid in _pwrdm_save_clkdm_state_and_activate.
and also better to check the return value of pwrdm_read_pwrst, firstly.
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
arch/arm/mach-omap2/powerdomain.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 43ac20c..876d067 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -233,7 +233,7 @@ static u8 _pwrdm_save_clkdm_state_and_activate(struct powerdomain *pwrdm,
{
u8 sleep_switch;
- if (curr_pwrst < 0) {
+ if ((char)curr_pwrst < 0) {
WARN_ON(1);
sleep_switch = ERROR_SWITCH;
} else if (curr_pwrst < PWRDM_POWER_ON) {
@@ -1106,7 +1106,10 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 pwrst)
pwrdm_lock(pwrdm);
- curr_pwrst = pwrdm_read_pwrst(pwrdm);
+ ret = pwrdm_read_pwrst(pwrdm);
+ if (ret < 0)
+ goto osps_out;
+ curr_pwrst = (u8)(unsigned int)ret;
next_pwrst = pwrdm_read_next_pwrst(pwrdm);
if (curr_pwrst == pwrst && next_pwrst == pwrst)
goto osps_out;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH] ARM:OMAP2: an issue about curr_pwrst, u8 is never < 0
2013-03-14 4:21 [PATCH] ARM:OMAP2: an issue about curr_pwrst, u8 is never < 0 Chen Gang
@ 2013-04-01 10:25 ` Paul Walmsley
2013-04-01 10:41 ` Chen Gang
0 siblings, 1 reply; 3+ messages in thread
From: Paul Walmsley @ 2013-04-01 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Thu, 14 Mar 2013, Chen Gang wrote:
>
> if pwrdm_read_pwrst returns negative number, curr_pwrst can not notice it.
> since really need check curr_pwrst whether is negative,
> need let the check valid in _pwrdm_save_clkdm_state_and_activate.
> and also better to check the return value of pwrdm_read_pwrst, firstly.
>
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
thanks for reporting this and sending a suggested fix. The following
patch is what I've queued here, which should deal with the root cause
without the casts.
- Paul
From: Paul Walmsley <paul@pwsan.com>
Date: Sun, 31 Mar 2013 20:22:22 -0600
Subject: [PATCH] ARM: OMAP2+: powerdomain: avoid testing whether an unsigned
char is less than 0
_pwrdm_save_clkdm_state_and_activate() tried to test one of its
unsigned arguments to determine whether it was less than zero. Fix by
moving the error test to the caller.
Reported-by: Chen Gang <gang.chen@asianux.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
arch/arm/mach-omap2/powerdomain.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 8e61d80..89cad4a 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -52,7 +52,6 @@ enum {
#define ALREADYACTIVE_SWITCH 0
#define FORCEWAKEUP_SWITCH 1
#define LOWPOWERSTATE_SWITCH 2
-#define ERROR_SWITCH 3
/* pwrdm_list contains all registered struct powerdomains */
static LIST_HEAD(pwrdm_list);
@@ -233,10 +232,7 @@ static u8 _pwrdm_save_clkdm_state_and_activate(struct powerdomain *pwrdm,
{
u8 sleep_switch;
- if (curr_pwrst < 0) {
- WARN_ON(1);
- sleep_switch = ERROR_SWITCH;
- } else if (curr_pwrst < PWRDM_POWER_ON) {
+ if (curr_pwrst < PWRDM_POWER_ON) {
if (curr_pwrst > pwrst &&
pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE &&
arch_pwrdm->pwrdm_set_lowpwrstchange) {
@@ -1091,7 +1087,8 @@ int pwrdm_post_transition(struct powerdomain *pwrdm)
*/
int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 pwrst)
{
- u8 curr_pwrst, next_pwrst, sleep_switch;
+ u8 next_pwrst, sleep_switch;
+ int curr_pwrst;
int ret = 0;
bool hwsup = false;
@@ -1107,16 +1104,17 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 pwrst)
pwrdm_lock(pwrdm);
curr_pwrst = pwrdm_read_pwrst(pwrdm);
+ if (curr_pwrst < 0) {
+ ret = -EINVAL;
+ goto osps_out;
+ }
+
next_pwrst = pwrdm_read_next_pwrst(pwrdm);
if (curr_pwrst == pwrst && next_pwrst == pwrst)
goto osps_out;
sleep_switch = _pwrdm_save_clkdm_state_and_activate(pwrdm, curr_pwrst,
pwrst, &hwsup);
- if (sleep_switch == ERROR_SWITCH) {
- ret = -EINVAL;
- goto osps_out;
- }
ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
if (ret)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH] ARM:OMAP2: an issue about curr_pwrst, u8 is never < 0
2013-04-01 10:25 ` Paul Walmsley
@ 2013-04-01 10:41 ` Chen Gang
0 siblings, 0 replies; 3+ messages in thread
From: Chen Gang @ 2013-04-01 10:41 UTC (permalink / raw)
To: linux-arm-kernel
On 2013?04?01? 18:25, Paul Walmsley wrote:
> Hi,
>
> On Thu, 14 Mar 2013, Chen Gang wrote:
>
>>
>> if pwrdm_read_pwrst returns negative number, curr_pwrst can not notice it.
>> since really need check curr_pwrst whether is negative,
>> need let the check valid in _pwrdm_save_clkdm_state_and_activate.
>> and also better to check the return value of pwrdm_read_pwrst, firstly.
>>
>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
>
> thanks for reporting this and sending a suggested fix. The following
> patch is what I've queued here, which should deal with the root cause
> without the casts.
>
>
yours (patch below) is simpler and clearer than mine.
thanks.
:-)
> - Paul
>
> From: Paul Walmsley <paul@pwsan.com>
> Date: Sun, 31 Mar 2013 20:22:22 -0600
> Subject: [PATCH] ARM: OMAP2+: powerdomain: avoid testing whether an unsigned
> char is less than 0
>
> _pwrdm_save_clkdm_state_and_activate() tried to test one of its
> unsigned arguments to determine whether it was less than zero. Fix by
> moving the error test to the caller.
>
> Reported-by: Chen Gang <gang.chen@asianux.com>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> ---
> arch/arm/mach-omap2/powerdomain.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> index 8e61d80..89cad4a 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -52,7 +52,6 @@ enum {
> #define ALREADYACTIVE_SWITCH 0
> #define FORCEWAKEUP_SWITCH 1
> #define LOWPOWERSTATE_SWITCH 2
> -#define ERROR_SWITCH 3
>
> /* pwrdm_list contains all registered struct powerdomains */
> static LIST_HEAD(pwrdm_list);
> @@ -233,10 +232,7 @@ static u8 _pwrdm_save_clkdm_state_and_activate(struct powerdomain *pwrdm,
> {
> u8 sleep_switch;
>
> - if (curr_pwrst < 0) {
> - WARN_ON(1);
> - sleep_switch = ERROR_SWITCH;
> - } else if (curr_pwrst < PWRDM_POWER_ON) {
> + if (curr_pwrst < PWRDM_POWER_ON) {
> if (curr_pwrst > pwrst &&
> pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE &&
> arch_pwrdm->pwrdm_set_lowpwrstchange) {
> @@ -1091,7 +1087,8 @@ int pwrdm_post_transition(struct powerdomain *pwrdm)
> */
> int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 pwrst)
> {
> - u8 curr_pwrst, next_pwrst, sleep_switch;
> + u8 next_pwrst, sleep_switch;
> + int curr_pwrst;
> int ret = 0;
> bool hwsup = false;
>
> @@ -1107,16 +1104,17 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u8 pwrst)
> pwrdm_lock(pwrdm);
>
> curr_pwrst = pwrdm_read_pwrst(pwrdm);
> + if (curr_pwrst < 0) {
> + ret = -EINVAL;
> + goto osps_out;
> + }
> +
> next_pwrst = pwrdm_read_next_pwrst(pwrdm);
> if (curr_pwrst == pwrst && next_pwrst == pwrst)
> goto osps_out;
>
> sleep_switch = _pwrdm_save_clkdm_state_and_activate(pwrdm, curr_pwrst,
> pwrst, &hwsup);
> - if (sleep_switch == ERROR_SWITCH) {
> - ret = -EINVAL;
> - goto osps_out;
> - }
>
> ret = pwrdm_set_next_pwrst(pwrdm, pwrst);
> if (ret)
>
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-04-01 10:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-14 4:21 [PATCH] ARM:OMAP2: an issue about curr_pwrst, u8 is never < 0 Chen Gang
2013-04-01 10:25 ` Paul Walmsley
2013-04-01 10:41 ` Chen Gang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).