On pon, 2014-05-05 at 23:27 +0800, Axel Lin wrote: > 2014-05-05 23:10 GMT+08:00 Krzysztof Kozlowski <[email protected]>: > > 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 <[email protected]> > > 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);
Yes, it looks better for S2MPA01. Thanks for idea, I'll send v2. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
