linux-kernel - Re: [PATCH 2/2] regulator: s2mpa01: Fix accidental enable of buck4 ramp delay

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 5 May 2014 23:27:43 +0800
From:	Axel Lin <axel.lin@...ics.com>
To:	Krzysztof Kozlowski <k.kozlowski@...sung.com>
Cc:	Sangbeom Kim <sbkim73@...sung.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	Mark Brown <broonie@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Sachin Kamat <sachin.kamat@...aro.org>
Subject: Re: [PATCH 2/2] regulator: s2mpa01: Fix accidental enable of buck4
 ramp delay

2014-05-05 23:10 GMT+08:00 Krzysztof Kozlowski <k.kozlowski@...sung.com>:
> S2MPA01 supports enabling/disabling ramp delay only for buck[1234].
> Other bucks have ramp delay enabled always.
>
> However the bit shift for enabling buck4 ramp delay in register is equal
> to 0. When ramp delay was set for these other bucks (buck[56789] and
> buck10), the ramp delay for buck4 was also enabled.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@...sung.com>
> Fixes: f7b1a8dc1c1c ("regulator: s2mpa01: Don't check enable_shift before setting enable ramp rate")
> ---
>  drivers/regulator/s2mpa01.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
> index f19a30f0fb42..901504880ab6 100644
> --- a/drivers/regulator/s2mpa01.c
> +++ b/drivers/regulator/s2mpa01.c
> @@ -99,7 +99,8 @@ static int s2mpa01_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
>  {
>         struct s2mpa01_info *s2mpa01 = rdev_get_drvdata(rdev);
>         unsigned int ramp_val, ramp_shift, ramp_reg = S2MPA01_REG_RAMP2;
> -       unsigned int ramp_enable = 1, enable_shift = 0;
> +       unsigned int ramp_enable = 1;
> +       int enable_shift = -1;
Hi Krzysztof,
This makes the meaning of enable_shift looks confusing.
I think below change has better readability.

diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
index f19a30f..eb5ce18 100644
--- a/drivers/regulator/s2mpa01.c
+++ b/drivers/regulator/s2mpa01.c
@@ -192,11 +192,15 @@ static int s2mpa01_set_ramp_delay(struct
regulator_dev *rdev, int ramp_delay)
  if (!ramp_enable)
  goto ramp_disable;

- ret = regmap_update_bits(rdev->regmap, S2MPA01_REG_RAMP1,
- 1 << enable_shift, 1 << enable_shift);
- if (ret) {
- dev_err(&rdev->dev, "failed to enable ramp rate\n");
- return ret;
+ /* S2MPA01 supports enabling/disabling ramp delay only for buck[1234] */
+ if (rdev->desc->id >= S2MPA01_BUCK1 &&
+    rdev->desc->id <= S2MPA01_BUCK4) {
+ ret = regmap_update_bits(rdev->regmap, S2MPA01_REG_RAMP1,
+ 1 << enable_shift, 1 << enable_shift);
+ if (ret) {
+ dev_err(&rdev->dev, "failed to enable ramp rate\n");
+ return ret;
+ }
  }

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