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

From: Lee Jones
Date: Mon Apr 08 2013 - 03:55:04 EST


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.lin@xxxxxxxxxx>

Looks fine:

Acked-by: Lee Jones <lee.jones@xxxxxxxxxx>

> ---
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/