Messages in this thread | ![/](/images/icornerl.gif) | | From | Julian Calaby <> | Date | Wed, 12 Dec 2018 00:14:57 +1100 | Subject | Re: [linux-sunxi] [PATCH 2/8] regulator: axp20x: add support for set_ramp_delay for AXP209 |
| |
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); > +} > +
-- Julian Calaby
Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/
| ![\](/images/icornerr.gif) |