subject:"\[PATCH\] ARM\:OMAP2\: an issue about curr

Re: [PATCH] ARM:OMAP2: an issue about curr_pwrst, u8 is never 0

2013-04-01 Thread Paul Walmsley

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.c...@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 p...@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.c...@asianux.com
Signed-off-by: Paul Walmsley p...@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

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM:OMAP2: an issue about curr_pwrst, u8 is never 0

2013-04-01 Thread Chen Gang

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.c...@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 p...@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.c...@asianux.com
 Signed-off-by: Paul Walmsley p...@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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ARM:OMAP2: an issue about curr_pwrst, u8 is never 0

2013-03-13 Thread Chen Gang


  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.c...@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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM:OMAP2: an issue about curr_pwrst, u8 is never 0

Re: [PATCH] ARM:OMAP2: an issue about curr_pwrst, u8 is never 0

[PATCH] ARM:OMAP2: an issue about curr_pwrst, u8 is never 0

3 matches

Site Navigation

Mail list logo

Footer information