Messages in this thread Patch in this message | | | Date | Mon, 5 May 2014 23:27:43 +0800 | Subject | Re: [PATCH 2/2] regulator: s2mpa01: Fix accidental enable of buck4 ramp delay | From | Axel Lin <> |
| |
2014-05-05 23:10 GMT+08:00 Krzysztof Kozlowski <k.kozlowski@samsung.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@samsung.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);
| |