| Message ID | cover.4110554ae84987dedb8b58888a8a63d6b7f98c72.1544466940.git-series.plaes@plaes.org |
|---|---|
| Headers | show |
| Series | This is a second edition of a series that implements voltage | expand |
On Mon, Dec 10, 2018 at 08:42:11PM +0200, Priit Laes wrote: > ramping for AXP209 DCDC2 and LDO3 regulators and software > based soft-start for AXP209 LDO3 regulator. Ugh.. managed to botch this series. I'll send a fixed one today. > > Both features are needed to work around a PMIC shutdown when > toggling LDO3 on certain boards with high capacitance on the > LDO3 output. > > Similar features (or workarounds) have been also implemented > on u-boot side [1]. > > Changes since v1: > - Rebased on top of next and dropped already merged patches. > - Dropped LDO4 full range devicetree change for Lime2 (prev patch 9) > in favor of general pin-bank regulator dependency [2]. > - Fixed paths in devicetree bindings (patch 3) > - Added note about software based soft-start for LDO3 (patch 5) > > [1] https://lists.denx.de/pipermail/u-boot/2018-November/348612.html > [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-December/618459.html > > Olliver Schinagl (8): > mfd: axp20x: name voltage ramping define properly > regulator: axp20x: add support for set_ramp_delay for AXP209 > dt-bindings: mfd: axp20x: add support for regulator-ramp-delay for AXP209 > regulator: axp20x: add software based soft_start for AXP209 LDO3 > dt-bindings: mfd: axp20x: Add software based soft_start for AXP209 LDO3 > regulator: dts: enable soft-start and ramp delay for the OLinuXino Lime2 > mfd: axp20x: Clean up included headers > mfd: axp20x: use explicit bit defines > > Documentation/devicetree/bindings/mfd/axp20x.txt | 9 +- > arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts | 2 +- > drivers/mfd/axp20x.c | 13 +- > drivers/regulator/axp20x-regulator.c | 142 +++++++++++++++- > include/linux/mfd/axp20x.h | 4 +- > 5 files changed, 161 insertions(+), 9 deletions(-) > > base-commit: 14cf8c1d5b90a0cf6a8ba51ef59db8da8c7a2622 > -- > git-series 0.9.1 > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.
On Tue, 11 Dec 2018, Priit Laes wrote: > On Mon, Dec 10, 2018 at 08:42:11PM +0200, Priit Laes wrote: > > ramping for AXP209 DCDC2 and LDO3 regulators and software > > based soft-start for AXP209 LDO3 regulator. > > Ugh.. managed to botch this series. I'll send a fixed one > today. While you're at it, please don't forget to add the version number in the patch subject lines. Each patch should have read "[PATCH v2] <...>" in this submission. > > Both features are needed to work around a PMIC shutdown when > > toggling LDO3 on certain boards with high capacitance on the > > LDO3 output. > > > > Similar features (or workarounds) have been also implemented > > on u-boot side [1]. > > > > Changes since v1: > > - Rebased on top of next and dropped already merged patches. > > - Dropped LDO4 full range devicetree change for Lime2 (prev patch 9) > > in favor of general pin-bank regulator dependency [2]. > > - Fixed paths in devicetree bindings (patch 3) > > - Added note about software based soft-start for LDO3 (patch 5) > > > > [1] https://lists.denx.de/pipermail/u-boot/2018-November/348612.html > > [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-December/618459.html > > > > Olliver Schinagl (8): > > mfd: axp20x: name voltage ramping define properly > > regulator: axp20x: add support for set_ramp_delay for AXP209 > > dt-bindings: mfd: axp20x: add support for regulator-ramp-delay for AXP209 > > regulator: axp20x: add software based soft_start for AXP209 LDO3 > > dt-bindings: mfd: axp20x: Add software based soft_start for AXP209 LDO3 > > regulator: dts: enable soft-start and ramp delay for the OLinuXino Lime2 > > mfd: axp20x: Clean up included headers > > mfd: axp20x: use explicit bit defines > > > > Documentation/devicetree/bindings/mfd/axp20x.txt | 9 +- > > arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts | 2 +- > > drivers/mfd/axp20x.c | 13 +- > > drivers/regulator/axp20x-regulator.c | 142 +++++++++++++++- > > include/linux/mfd/axp20x.h | 4 +- > > 5 files changed, 161 insertions(+), 9 deletions(-) > > > > base-commit: 14cf8c1d5b90a0cf6a8ba51ef59db8da8c7a2622
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/
On Mon, Dec 10, 2018 at 08:42:12PM +0200, Priit Laes wrote: > Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> > Signed-off-by: Priit Laes <plaes@plaes.org> > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> What's going on with this patch? Will you be applying it or is this an ack for it to be applied with the rest of the series?
On Thu, 13 Dec 2018, Mark Brown wrote: > On Mon, Dec 10, 2018 at 08:42:12PM +0200, Priit Laes wrote: > > > Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> > > Signed-off-by: Priit Laes <plaes@plaes.org> > > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > > What's going on with this patch? Will you be applying it or is this an > ack for it to be applied with the rest of the series? I don't have anything else queued for include/linux/mfd/axp20x.h, so you can take it if you want. Just make sure you 's/-for-MFD//'.
On Thu, Dec 13, 2018 at 01:28:02PM +0000, Lee Jones wrote: > On Thu, 13 Dec 2018, Mark Brown wrote: > > What's going on with this patch? Will you be applying it or is this an > > ack for it to be applied with the rest of the series? > I don't have anything else queued for include/linux/mfd/axp20x.h, so > you can take it if you want. Just make sure you 's/-for-MFD//'. Great, thanks!