Re: [PATCH 2/4] regulator: ab8500-ext: Get rid of is_enabled from struct ab8500_ext_regulator_info

2013-04-08 Thread Lee Jones 
On Sun, 07 Apr 2013, Axel Lin wrote:

> The intention of this patch is to simplify the code.
> 
> Maintain the is_enabled flag is not trivial, it not only needs to set/clear 
> the
> flag in disable()/enable() but also needs to set the flag in is_enable() to 
> get
> initial status. The only benefit of keeping is_enabled flag is just save a
> register read when set_mode(). Remove is_enabled flag makes the code simpler.
> 
> Signed-off-by: Axel Lin 

Looks fine:

Acked-by: Lee Jones 

> ---
>  drivers/regulator/ab8500-ext.c |   14 +++---
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/regulator/ab8500-ext.c b/drivers/regulator/ab8500-ext.c
> index 57d43a1..5e604a2 100644
> --- a/drivers/regulator/ab8500-ext.c
> +++ b/drivers/regulator/ab8500-ext.c
> @@ -29,7 +29,6 @@
>   * @desc: regulator description
>   * @rdev: regulator device
>   * @cfg: regulator configuration (extension of regulator FW configuration)
> - * @is_enabled: status of regulator (on/off)
>   * @update_bank: bank to control on/off
>   * @update_reg: register to control on/off
>   * @update_mask: mask to enable/disable and set mode of regulator
> @@ -46,7 +45,6 @@ struct ab8500_ext_regulator_info {
>   struct regulator_desc desc;
>   struct regulator_dev *rdev;
>   struct ab8500_ext_regulator_cfg *cfg;
> - bool is_enabled;
>   u8 update_bank;
>   u8 update_reg;
>   u8 update_mask;
> @@ -78,8 +76,6 @@ static int enable(struct ab8500_ext_regulator_info *info, 
> u8 *regval)
>   return ret;
>   }
>  
> - info->is_enabled = true;
> -
>   return ret;
>  }
>  
> @@ -125,8 +121,6 @@ static int disable(struct ab8500_ext_regulator_info 
> *info, u8 *regval)
>   return ret;
>   }
>  
> - info->is_enabled = false;
> -
>   return ret;
>  }
>  
> @@ -177,11 +171,9 @@ static int ab8500_ext_regulator_is_enabled(struct 
> regulator_dev *rdev)
>  
>   if (((regval & info->update_mask) == info->update_val_lp) ||
>   ((regval & info->update_mask) == info->update_val_hp))
> - info->is_enabled = true;
> + return 1;
>   else
> - info->is_enabled = false;
> -
> - return info->is_enabled;
> + return 0;
>  }
>  
>  static int ab8500_ext_regulator_set_mode(struct regulator_dev *rdev,
> @@ -207,7 +199,7 @@ static int ab8500_ext_regulator_set_mode(struct 
> regulator_dev *rdev,
>   return -EINVAL;
>   }
>  
> - if (info->is_enabled) {
> + if (ab8500_ext_regulator_is_enabled(rdev)) {
>   u8 regval;
>  
>   ret = enable(info, );

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/4] regulator: ab8500-ext: Get rid of is_enabled from struct ab8500_ext_regulator_info

2013-04-08 Thread Lee Jones 
On Sun, 07 Apr 2013, Axel Lin wrote:

 The intention of this patch is to simplify the code.
 
 Maintain the is_enabled flag is not trivial, it not only needs to set/clear 
 the
 flag in disable()/enable() but also needs to set the flag in is_enable() to 
 get
 initial status. The only benefit of keeping is_enabled flag is just save a
 register read when set_mode(). Remove is_enabled flag makes the code simpler.
 
 Signed-off-by: Axel Lin axel@ingics.com

Looks fine:

Acked-by: Lee Jones lee.jo...@linaro.org

 ---
  drivers/regulator/ab8500-ext.c |   14 +++---
  1 file changed, 3 insertions(+), 11 deletions(-)
 
 diff --git a/drivers/regulator/ab8500-ext.c b/drivers/regulator/ab8500-ext.c
 index 57d43a1..5e604a2 100644
 --- a/drivers/regulator/ab8500-ext.c
 +++ b/drivers/regulator/ab8500-ext.c
 @@ -29,7 +29,6 @@
   * @desc: regulator description
   * @rdev: regulator device
   * @cfg: regulator configuration (extension of regulator FW configuration)
 - * @is_enabled: status of regulator (on/off)
   * @update_bank: bank to control on/off
   * @update_reg: register to control on/off
   * @update_mask: mask to enable/disable and set mode of regulator
 @@ -46,7 +45,6 @@ struct ab8500_ext_regulator_info {
   struct regulator_desc desc;
   struct regulator_dev *rdev;
   struct ab8500_ext_regulator_cfg *cfg;
 - bool is_enabled;
   u8 update_bank;
   u8 update_reg;
   u8 update_mask;
 @@ -78,8 +76,6 @@ static int enable(struct ab8500_ext_regulator_info *info, 
 u8 *regval)
   return ret;
   }
  
 - info-is_enabled = true;
 -
   return ret;
  }
  
 @@ -125,8 +121,6 @@ static int disable(struct ab8500_ext_regulator_info 
 *info, u8 *regval)
   return ret;
   }
  
 - info-is_enabled = false;
 -
   return ret;
  }
  
 @@ -177,11 +171,9 @@ static int ab8500_ext_regulator_is_enabled(struct 
 regulator_dev *rdev)
  
   if (((regval  info-update_mask) == info-update_val_lp) ||
   ((regval  info-update_mask) == info-update_val_hp))
 - info-is_enabled = true;
 + return 1;
   else
 - info-is_enabled = false;
 -
 - return info-is_enabled;
 + return 0;
  }
  
  static int ab8500_ext_regulator_set_mode(struct regulator_dev *rdev,
 @@ -207,7 +199,7 @@ static int ab8500_ext_regulator_set_mode(struct 
 regulator_dev *rdev,
   return -EINVAL;
   }
  
 - if (info-is_enabled) {
 + if (ab8500_ext_regulator_is_enabled(rdev)) {
   u8 regval;
  
   ret = enable(info, regval);

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/4] regulator: ab8500-ext: Get rid of is_enabled from struct ab8500_ext_regulator_info

2013-04-07 Thread Axel Lin 
The intention of this patch is to simplify the code.

Maintain the is_enabled flag is not trivial, it not only needs to set/clear the
flag in disable()/enable() but also needs to set the flag in is_enable() to get
initial status. The only benefit of keeping is_enabled flag is just save a
register read when set_mode(). Remove is_enabled flag makes the code simpler.

Signed-off-by: Axel Lin 
---
 drivers/regulator/ab8500-ext.c |   14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/regulator/ab8500-ext.c b/drivers/regulator/ab8500-ext.c
index 57d43a1..5e604a2 100644
--- a/drivers/regulator/ab8500-ext.c
+++ b/drivers/regulator/ab8500-ext.c
@@ -29,7 +29,6 @@
  * @desc: regulator description
  * @rdev: regulator device
  * @cfg: regulator configuration (extension of regulator FW configuration)
- * @is_enabled: status of regulator (on/off)
  * @update_bank: bank to control on/off
  * @update_reg: register to control on/off
  * @update_mask: mask to enable/disable and set mode of regulator
@@ -46,7 +45,6 @@ struct ab8500_ext_regulator_info {
struct regulator_desc desc;
struct regulator_dev *rdev;
struct ab8500_ext_regulator_cfg *cfg;
-   bool is_enabled;
u8 update_bank;
u8 update_reg;
u8 update_mask;
@@ -78,8 +76,6 @@ static int enable(struct ab8500_ext_regulator_info *info, u8 
*regval)
return ret;
}
 
-   info->is_enabled = true;
-
return ret;
 }
 
@@ -125,8 +121,6 @@ static int disable(struct ab8500_ext_regulator_info *info, 
u8 *regval)
return ret;
}
 
-   info->is_enabled = false;
-
return ret;
 }
 
@@ -177,11 +171,9 @@ static int ab8500_ext_regulator_is_enabled(struct 
regulator_dev *rdev)
 
if (((regval & info->update_mask) == info->update_val_lp) ||
((regval & info->update_mask) == info->update_val_hp))
-   info->is_enabled = true;
+   return 1;
else
-   info->is_enabled = false;
-
-   return info->is_enabled;
+   return 0;
 }
 
 static int ab8500_ext_regulator_set_mode(struct regulator_dev *rdev,
@@ -207,7 +199,7 @@ static int ab8500_ext_regulator_set_mode(struct 
regulator_dev *rdev,
return -EINVAL;
}
 
-   if (info->is_enabled) {
+   if (ab8500_ext_regulator_is_enabled(rdev)) {
u8 regval;
 
ret = enable(info, );
-- 
1.7.10.4



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/4] regulator: ab8500-ext: Get rid of is_enabled from struct ab8500_ext_regulator_info

2013-04-07 Thread Axel Lin 
The intention of this patch is to simplify the code.

Maintain the is_enabled flag is not trivial, it not only needs to set/clear the
flag in disable()/enable() but also needs to set the flag in is_enable() to get
initial status. The only benefit of keeping is_enabled flag is just save a
register read when set_mode(). Remove is_enabled flag makes the code simpler.

Signed-off-by: Axel Lin axel@ingics.com
---
 drivers/regulator/ab8500-ext.c |   14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/regulator/ab8500-ext.c b/drivers/regulator/ab8500-ext.c
index 57d43a1..5e604a2 100644
--- a/drivers/regulator/ab8500-ext.c
+++ b/drivers/regulator/ab8500-ext.c
@@ -29,7 +29,6 @@
  * @desc: regulator description
  * @rdev: regulator device
  * @cfg: regulator configuration (extension of regulator FW configuration)
- * @is_enabled: status of regulator (on/off)
  * @update_bank: bank to control on/off
  * @update_reg: register to control on/off
  * @update_mask: mask to enable/disable and set mode of regulator
@@ -46,7 +45,6 @@ struct ab8500_ext_regulator_info {
struct regulator_desc desc;
struct regulator_dev *rdev;
struct ab8500_ext_regulator_cfg *cfg;
-   bool is_enabled;
u8 update_bank;
u8 update_reg;
u8 update_mask;
@@ -78,8 +76,6 @@ static int enable(struct ab8500_ext_regulator_info *info, u8 
*regval)
return ret;
}
 
-   info-is_enabled = true;
-
return ret;
 }
 
@@ -125,8 +121,6 @@ static int disable(struct ab8500_ext_regulator_info *info, 
u8 *regval)
return ret;
}
 
-   info-is_enabled = false;
-
return ret;
 }
 
@@ -177,11 +171,9 @@ static int ab8500_ext_regulator_is_enabled(struct 
regulator_dev *rdev)
 
if (((regval  info-update_mask) == info-update_val_lp) ||
((regval  info-update_mask) == info-update_val_hp))
-   info-is_enabled = true;
+   return 1;
else
-   info-is_enabled = false;
-
-   return info-is_enabled;
+   return 0;
 }
 
 static int ab8500_ext_regulator_set_mode(struct regulator_dev *rdev,
@@ -207,7 +199,7 @@ static int ab8500_ext_regulator_set_mode(struct 
regulator_dev *rdev,
return -EINVAL;
}
 
-   if (info-is_enabled) {
+   if (ab8500_ext_regulator_is_enabled(rdev)) {
u8 regval;
 
ret = enable(info, regval);
-- 
1.7.10.4



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