| Message ID | 2bfe47f173fe72a30b0036dcdeebc2123962ff33.1544466940.git-series.plaes@plaes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | This is a second edition of a series that implements voltage | expand |
Hi Priit and Olliver, On Tue, Dec 11, 2018 at 5:42 AM Priit Laes <plaes@plaes.org> wrote: > > From: Olliver Schinagl <oliver@schinagl.nl> > > The AXP209 supports ramping up voltages on several regulators such as > DCDC2 and LDO3. > > This patch adds preliminary support for the regulator-ramp-delay property > for these 2 regulators. Note that the voltage ramp only works when > regulator is already enabled. E.g. when going from say 0.7 V to 3.6 V. > > When turning on the regulator, no voltage ramp is performed in hardware. > > What this means, is that if the bootloader brings up the voltage at 0.7 V, > the ramp delay property is properly applied. If however, the bootloader > leaves the power off, no ramp delay is applied when the power is > enabled by the regulator framework. > > Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> > Signed-off-by: Priit Laes <plaes@plaes.org> > --- > drivers/regulator/axp20x-regulator.c | 85 +++++++++++++++++++++++++++++- > 1 file changed, 85 insertions(+) > > diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c > index 9a2db28..1d9fa62 100644 > --- a/drivers/regulator/axp20x-regulator.c > +++ b/drivers/regulator/axp20x-regulator.c > @@ -346,6 +357,79 @@ > .ops = &axp20x_ops_range, \ > } > > +static const int axp209_dcdc2_ldo3_slew_rates[] = { > + 1600, > + 800, > +}; > + > +static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp) > +{ > + struct axp20x_dev *axp20x = rdev_get_drvdata(rdev); > + const struct regulator_desc *desc = rdev->desc; > + u8 reg, mask, enable, cfg = 0xff; > + const int *slew_rates; > + int rate_count = 0; > + > + if (!rdev) > + return -EINVAL; > + > + switch (axp20x->variant) { > + case AXP209_ID: > + if (desc->id == AXP20X_DCDC2) { Is slew_rates supposed to be set here? > + rate_count = ARRAY_SIZE(axp209_dcdc2_ldo3_slew_rates); > + reg = AXP20X_DCDC2_LDO3_V_RAMP; > + mask = AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_RATE_MASK | > + AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN_MASK; > + enable = (ramp > 0) ? > + AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN : > + !AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN; > + break; > + } > + > + if (desc->id == AXP20X_LDO3) { > + slew_rates = axp209_dcdc2_ldo3_slew_rates; > + rate_count = ARRAY_SIZE(axp209_dcdc2_ldo3_slew_rates); > + reg = AXP20X_DCDC2_LDO3_V_RAMP; > + mask = AXP20X_DCDC2_LDO3_V_RAMP_LDO3_RATE_MASK | > + AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN_MASK; > + enable = (ramp > 0) ? > + AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN : > + !AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN; > + break; > + } > + > + if (rate_count > 0) > + break; You could save one to two tests by combining the above three if statements, i.e. if (DCDC2) { // set DCDC2 stuff break; } else if (LDO3) { // set LDO3 stuff break; } As written, the rate_count > 0 test will never be true as every block that sets rate_count breaks out of the switch block. You could then calculate rate_count below the switch block. > + > + /* fall through */ > + default: > + /* Not supported for this regulator */ > + return -ENOTSUPP; > + } > + > + if (ramp == 0) { > + cfg = enable; > + } else { > + int i; > + > + for (i = 0; i < rate_count; i++) { > + if (ramp <= slew_rates[i]) > + cfg = AXP20X_DCDC2_LDO3_V_RAMP_LDO3_RATE(i); > + else > + break; > + } > + > + if (cfg == 0xff) { > + dev_err(axp20x->dev, "unsupported ramp value %d", ramp); > + return -EINVAL; > + } > + > + cfg |= enable; > + } > + > + return regmap_update_bits(axp20x->regmap, reg, mask, cfg); > +} > +
On Wed, Dec 12, 2018 at 12:14:57AM +1100, Julian Calaby wrote: > Hi Priit and Olliver, > > On Tue, Dec 11, 2018 at 5:42 AM Priit Laes <plaes@plaes.org> wrote: > > > > From: Olliver Schinagl <oliver@schinagl.nl> > > > > The AXP209 supports ramping up voltages on several regulators such as > > DCDC2 and LDO3. > > > > This patch adds preliminary support for the regulator-ramp-delay property > > for these 2 regulators. Note that the voltage ramp only works when > > regulator is already enabled. E.g. when going from say 0.7 V to 3.6 V. > > > > When turning on the regulator, no voltage ramp is performed in hardware. > > > > What this means, is that if the bootloader brings up the voltage at 0.7 V, > > the ramp delay property is properly applied. If however, the bootloader > > leaves the power off, no ramp delay is applied when the power is > > enabled by the regulator framework. > > > > Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> > > Signed-off-by: Priit Laes <plaes@plaes.org> > > --- > > drivers/regulator/axp20x-regulator.c | 85 +++++++++++++++++++++++++++++- > > 1 file changed, 85 insertions(+) > > > > diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c > > index 9a2db28..1d9fa62 100644 > > --- a/drivers/regulator/axp20x-regulator.c > > +++ b/drivers/regulator/axp20x-regulator.c > > @@ -346,6 +357,79 @@ > > .ops = &axp20x_ops_range, \ > > } > > > > +static const int axp209_dcdc2_ldo3_slew_rates[] = { > > + 1600, > > + 800, > > +}; > > + > > +static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp) > > +{ > > + struct axp20x_dev *axp20x = rdev_get_drvdata(rdev); > > + const struct regulator_desc *desc = rdev->desc; > > + u8 reg, mask, enable, cfg = 0xff; > > + const int *slew_rates; > > + int rate_count = 0; > > + > > + if (!rdev) > > + return -EINVAL; > > + > > + switch (axp20x->variant) { > > + case AXP209_ID: > > + if (desc->id == AXP20X_DCDC2) { > > Is slew_rates supposed to be set here? Yes, nice catch. > > > + rate_count = ARRAY_SIZE(axp209_dcdc2_ldo3_slew_rates); > > + reg = AXP20X_DCDC2_LDO3_V_RAMP; > > + mask = AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_RATE_MASK | > > + AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN_MASK; > > + enable = (ramp > 0) ? > > + AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN : > > + !AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN; > > + break; > > + } > > + > > + if (desc->id == AXP20X_LDO3) { > > + slew_rates = axp209_dcdc2_ldo3_slew_rates; > > + rate_count = ARRAY_SIZE(axp209_dcdc2_ldo3_slew_rates); > > + reg = AXP20X_DCDC2_LDO3_V_RAMP; > > + mask = AXP20X_DCDC2_LDO3_V_RAMP_LDO3_RATE_MASK | > > + AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN_MASK; > > + enable = (ramp > 0) ? > > + AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN : > > + !AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN; > > + break; > > + } > > + > > + if (rate_count > 0) > > + break; > > You could save one to two tests by combining the above three if statements, i.e. > > if (DCDC2) { > // set DCDC2 stuff > > break; > } else if (LDO3) { > // set LDO3 stuff > > break; > } OK, will look into it. > > As written, the rate_count > 0 test will never be true as every block > that sets rate_count breaks out of the switch block. Yes, it is somewhat superfluous, as each regulator which supports ramping should break out of the switch itself. > You could then calculate rate_count below the switch block. > > > + > > + /* fall through */ > > + default: > > + /* Not supported for this regulator */ > > + return -ENOTSUPP; > > + } > > + > > + if (ramp == 0) { > > + cfg = enable; > > + } else { > > + int i; > > + > > + for (i = 0; i < rate_count; i++) { > > + if (ramp <= slew_rates[i]) > > + cfg = AXP20X_DCDC2_LDO3_V_RAMP_LDO3_RATE(i); > > + else > > + break; > > + } > > + > > + if (cfg == 0xff) { > > + dev_err(axp20x->dev, "unsupported ramp value %d", ramp); > > + return -EINVAL; > > + } > > + > > + cfg |= enable; > > + } > > + > > + return regmap_update_bits(axp20x->regmap, reg, mask, cfg); > > +} > > + > > > > -- > Julian Calaby > > Email: julian.calaby@gmail.com > Profile: http://www.google.com/profiles/julian.calaby/
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c index 9a2db28..1d9fa62 100644 --- a/drivers/regulator/axp20x-regulator.c +++ b/drivers/regulator/axp20x-regulator.c @@ -51,6 +51,17 @@ #define AXP20X_PWR_OUT_DCDC2_MASK BIT_MASK(4) #define AXP20X_PWR_OUT_LDO3_MASK BIT_MASK(6) +#define AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_RATE_MASK BIT_MASK(0) +#define AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_RATE(x) \ + ((x) << 0) +#define AXP20X_DCDC2_LDO3_V_RAMP_LDO3_RATE_MASK BIT_MASK(1) +#define AXP20X_DCDC2_LDO3_V_RAMP_LDO3_RATE(x) \ + ((x) << 1) +#define AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN_MASK BIT_MASK(2) +#define AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN BIT(2) +#define AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN_MASK BIT_MASK(3) +#define AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN BIT(3) + #define AXP20X_LDO4_V_OUT_1250mV_START 0x0 #define AXP20X_LDO4_V_OUT_1250mV_STEPS 0 #define AXP20X_LDO4_V_OUT_1250mV_END \ @@ -346,6 +357,79 @@ .ops = &axp20x_ops_range, \ } +static const int axp209_dcdc2_ldo3_slew_rates[] = { + 1600, + 800, +}; + +static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp) +{ + struct axp20x_dev *axp20x = rdev_get_drvdata(rdev); + const struct regulator_desc *desc = rdev->desc; + u8 reg, mask, enable, cfg = 0xff; + const int *slew_rates; + int rate_count = 0; + + if (!rdev) + return -EINVAL; + + switch (axp20x->variant) { + case AXP209_ID: + if (desc->id == AXP20X_DCDC2) { + rate_count = ARRAY_SIZE(axp209_dcdc2_ldo3_slew_rates); + reg = AXP20X_DCDC2_LDO3_V_RAMP; + mask = AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_RATE_MASK | + AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN_MASK; + enable = (ramp > 0) ? + AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN : + !AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN; + break; + } + + if (desc->id == AXP20X_LDO3) { + slew_rates = axp209_dcdc2_ldo3_slew_rates; + rate_count = ARRAY_SIZE(axp209_dcdc2_ldo3_slew_rates); + reg = AXP20X_DCDC2_LDO3_V_RAMP; + mask = AXP20X_DCDC2_LDO3_V_RAMP_LDO3_RATE_MASK | + AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN_MASK; + enable = (ramp > 0) ? + AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN : + !AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN; + break; + } + + if (rate_count > 0) + break; + + /* fall through */ + default: + /* Not supported for this regulator */ + return -ENOTSUPP; + } + + if (ramp == 0) { + cfg = enable; + } else { + int i; + + for (i = 0; i < rate_count; i++) { + if (ramp <= slew_rates[i]) + cfg = AXP20X_DCDC2_LDO3_V_RAMP_LDO3_RATE(i); + else + break; + } + + if (cfg == 0xff) { + dev_err(axp20x->dev, "unsupported ramp value %d", ramp); + return -EINVAL; + } + + cfg |= enable; + } + + return regmap_update_bits(axp20x->regmap, reg, mask, cfg); +} + static const struct regulator_ops axp20x_ops_fixed = { .list_voltage = regulator_list_voltage_linear, }; @@ -366,6 +450,7 @@ static const struct regulator_ops axp20x_ops = { .enable = regulator_enable_regmap, .disable = regulator_disable_regmap, .is_enabled = regulator_is_enabled_regmap, + .set_ramp_delay = axp20x_set_ramp_delay, }; static const struct regulator_ops axp20x_ops_sw = {