[permalink] [raw]

Subject: [PATCH 05/14] dt-bindings: mfd: axp20x: add support for regulator-ramp-delay for AXP209

From: Olliver Schinagl <[email protected]>

The AXP209 supports ramping up voltages on several regulators such as
DCDC2 and LDO3, therefore we can use the standard 'regulator-ramp-delay'
property for those 2 regulators.

Note that the voltage ramp only works when the 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 <[email protected]>
Signed-off-by: Priit Laes <[email protected]>
---
Documentation/devicetree/bindings/mfd/axp20x.txt | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
index 188f037..5542212 100644
--- a/Documentation/devicetree/bindings/mfd/axp20x.txt
+++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
@@ -32,6 +32,11 @@ Required properties:
- interrupt-controller: The PMIC has its own internal IRQs
- #interrupt-cells: Should be set to 1

+Supported common regulator properties, see regulator.txt for more information:
+- regulator-ramp-delay: sets the ramp up delay in uV/us
+ AXP20x/DCDC2: 1600, 800
+ AXP20x/LDO3: 1600, 800
+
Optional properties:
- x-powers,dcdc-freq: defines the work frequency of DC-DC in KHz
AXP152/20X: range: 750-1875, Default: 1.5 MHz
--
git-series 0.9.1

2018-11-26 15:34:27

by Priit Laes

[permalink] [raw]

Subject: [PATCH 02/14] regulator: axp20x: name voltage ramping define properly

From: Olliver Schinagl <[email protected]>

The current axp20x names the ramping register 'scal' which probably
means scaling. Since the register really has nothing to do with
scaling, but really is the voltage ramp we rename it appropriately.

Signed-off-by: Olliver Schinagl <[email protected]>
Signed-off-by: Priit Laes <[email protected]>
---
include/linux/mfd/axp20x.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 517e60e..1293695 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -35,7 +35,7 @@ enum axp20x_variants {
#define AXP152_ALDO_OP_MODE 0x13
#define AXP152_LDO0_CTRL 0x15
#define AXP152_DCDC2_V_OUT 0x23
-#define AXP152_DCDC2_V_SCAL 0x25
+#define AXP152_DCDC2_V_RAMP 0x25
#define AXP152_DCDC1_V_OUT 0x26
#define AXP152_DCDC3_V_OUT 0x27
#define AXP152_ALDO12_V_OUT 0x28
@@ -53,7 +53,7 @@ enum axp20x_variants {
#define AXP20X_USB_OTG_STATUS 0x02
#define AXP20X_PWR_OUT_CTRL 0x12
#define AXP20X_DCDC2_V_OUT 0x23
-#define AXP20X_DCDC2_LDO3_V_SCAL 0x25
+#define AXP20X_DCDC2_LDO3_V_RAMP 0x25
#define AXP20X_DCDC3_V_OUT 0x27
#define AXP20X_LDO24_V_OUT 0x28
#define AXP20X_LDO3_V_OUT 0x29
--
git-series 0.9.1

2018-11-26 15:34:30

by Priit Laes

[permalink] [raw]

Subject: [PATCH 03/14] regulator: core: enable power when setting up constraints

From: Olliver Schinagl <[email protected]>

When a regulator is marked as always on, it is enabled early on, when
checking and setting up constraints. It makes the assumption that the
bootloader properly initialized the regulator, and just in case enables
the regulator anyway.

Some constraints however currently get missed, such as the soft-start
and ramp-delay. This causes the regulator to be enabled, without the
soft-start and ramp-delay being applied, which in turn can cause
high-currents or other start-up problems.

By moving the always-enabled constraints later in the constraints check,
we can at least ensure all constraints for the regulator are followed.

Signed-off-by: Olliver Schinagl <[email protected]>
Signed-off-by: Priit Laes <[email protected]>
---
drivers/regulator/core.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 2c66b52..6e14610 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1158,17 +1158,6 @@ static int set_machine_constraints(struct regulator_dev *rdev,
}
}

- /* If the constraints say the regulator should be on at this point
- * and we have control then make sure it is enabled.
- */
- if (rdev->constraints->always_on || rdev->constraints->boot_on) {
- ret = _regulator_do_enable(rdev);
- if (ret < 0 && ret != -EINVAL) {
- rdev_err(rdev, "failed to enable\n");
- return ret;
- }
- }
-
if ((rdev->constraints->ramp_delay || rdev->constraints->ramp_disable)
&& ops->set_ramp_delay) {
ret = ops->set_ramp_delay(rdev, rdev->constraints->ramp_delay);
@@ -1214,6 +1203,17 @@ static int set_machine_constraints(struct regulator_dev *rdev,
}
}

+ /* If the constraints say the regulator should be on at this point
+ * and we have control then make sure it is enabled.
+ */
+ if (rdev->constraints->always_on || rdev->constraints->boot_on) {
+ ret = _regulator_do_enable(rdev);
+ if (ret < 0 && ret != -EINVAL) {
+ rdev_err(rdev, "failed to enable\n");
+ return ret;
+ }
+ }
+
print_constraints(rdev);
return 0;
}
--
git-series 0.9.1

2018-11-26 15:34:45

by Priit Laes

On Tue, Nov 27, 2018 at 5:37 PM Maxime Ripard <[email protected]> wrote:
>
> On Mon, Nov 26, 2018 at 05:27:51PM +0200, Priit Laes wrote:
> > From: Olliver Schinagl <[email protected]>
> >
> > The lradc's analog reference voltage is set to 3.0 volt in the
> > hardware. This is more or less set in copper for at least lradc0. Set the
> > property in the dts to ensure the lradc is referenced properly.
> >
> > Signed-off-by: Olliver Schinagl <[email protected]>
> > Signed-off-by: Priit Laes <[email protected]>
>
> I'm not sure why that patch is part of this series, but I applied
> it. Thanks!

Please help fix up the subject. Should read

"ARM: dts: sun7i: set proper lradc vref on OLinuXino Lime2"

or

"ARM: dts: sun7i: olinoxino-lime2: set proper lradc vref"

ChenYu

2018-11-27 13:06:41

On Tue, Nov 27, 2018 at 10:38:52AM +0100, Maxime Ripard wrote:
> On Mon, Nov 26, 2018 at 05:27:50PM +0200, Priit Laes wrote:
> > From: Olliver Schinagl <[email protected]>
> >
> > With commit b43776d65a33b46092 ("ARM: dts: sunxi: Use axp209.dtsi for
> > Olinuxino Lime2") we force them an arbitrary 2.8 volts. Granted, for
> > LDO3 this may be less arbitrary, but for LDO4 this is just wrong.
> >
> > In the defense of LDO3, LDO3 is the regulator that feeds port bank E,
> > which has no other purpose then a CSI/TS interface, however the case
> > may still be, that the connected IO may be just as well be 3.3 volts.
> > The big misnomer is however, that the schematic names GPIO-2 pin4
> > LDO3_2.8V, rather then VDD-CSI0 or similar.
> >
> > This is much worse for LDO4 however, which is not referenced on any
> > pin, is now set to 2.8 volts, but port bank G can also support various
> > other peripherals such as UARTS etc.
> >
> > By having 2.8 volts however for LDO4, we thus now have peripherals that
> > no longer function properly all of the time.
> >
> > Ideally, we want to set a supply voltage for each port bank, but the
> > monolithic nature of the sunxi pinctroller currently prevents this and
> > as such, the board should at least configure the LDO4 with the proper
> > ranges.
> >
> > Until we can set the consumer at the port bank level, a child
> > device-tree has to do something like:
> >
> > &reg_ldo4 {
> > regulator-min-microvolt = <3300000>;
> > regulator-max-microvolt = <3300000>;
> > };
> >
> > While doing this the same way results in the same solution currently,
> > we force the hack into the final devicetree rather then having it wrong
> > at the board level.
> >
> > Signed-off-by: Olliver Schinagl <[email protected]>
> > Signed-off-by: Priit Laes <[email protected]>
> > ---
> > arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > index ffafe97..1b9867f 100644
> > --- a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > +++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
> > @@ -250,9 +250,10 @@
> > };
> >
> > &reg_ldo4 {
> > - regulator-min-microvolt = <2800000>;
> > - regulator-max-microvolt = <2800000>;
> > - regulator-name = "vddio-csi1";
> > + regulator-always-on;
> > + regulator-min-microvolt = <1250000>;
> > + regulator-max-microvolt = <3300000>;
> > + regulator-name = "vdd-io-pg";
>
> As we discussed on the U-Boot ML already, this shouldn't be made
> always-on but tied to the consumer device (the pinctrl one) instead.

Can you test that patch (not tested):
http://code.bulix.org/h02vha-514284?raw

Thanks!
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Attachments:

(No filename) (2.87 kB)
signature.asc (235.00 B)
Download all attachments

2018-12-04 13:33:10

by Priit Laes

[permalink] [raw]

Subject: Re: [PATCH 06/14] regulator: axp20x: add software based soft_start for AXP209 LDO3

On Tue, Nov 27, 2018 at 10:36:19AM +0100, Maxime Ripard wrote:
> On Mon, Nov 26, 2018 at 05:27:47PM +0200, Priit Laes wrote:
> > From: Olliver Schinagl <[email protected]>
> >
> > In the past, there have been words on various lists that if LDO3 is
> > disabled in u-boot, but enabled in the DTS, the axp209 driver would
> > fail to continue/hang. Several enable/disable patches have been
> > issues to devicetree's in both the kernel and u-boot to address
> > this issue.
> >
> > What really happened however, was that the AXP209 shuts down without
> > a notice and without setting an interrupt. This is caused when LDO3
> > gets overloaded, for example with large capacitors on the LDO3 output.
> >
> > Normally, we would expect that AXP209 would source 200 mA as per
> > datasheet and set and trigger an interrupt when being overloaded.
> > For some reason however, this does not happen.
> >
> > As a work-around, we use the soft-start constraint of the regulator
> > node to first bring up the LDO3 to the lowest possible voltage and
> > then enable the LDO. After that, we can set the requested voltage
> > as usual.
> >
> > Combining this setting with the regulator-ramp-delay allows LDO3 to
> > enable voltage slowly and staggered, potentially reducing overall
> > inrush current.
> >
> > Signed-off-by: Olliver Schinagl <[email protected]>
> > Signed-off-by: Priit Laes <[email protected]>
> > ---
> > drivers/regulator/axp20x-regulator.c | 57 ++++++++++++++++++++++++++++-
> > 1 file changed, 56 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> > index 1d9fa62..e8a895b 100644
> > --- a/drivers/regulator/axp20x-regulator.c
> > +++ b/drivers/regulator/axp20x-regulator.c
> > @@ -14,6 +14,7 @@
> > */
> >
> > #include <linux/bitops.h>
> > +#include <linux/delay.h>
> > #include <linux/err.h>
> > #include <linux/init.h>
> > #include <linux/mfd/axp20x.h>
> > @@ -23,6 +24,7 @@
> > #include <linux/platform_device.h>
> > #include <linux/regmap.h>
> > #include <linux/regulator/driver.h>
> > +#include <linux/regulator/machine.h>
> > #include <linux/regulator/of_regulator.h>
> >
> > #define AXP20X_GPIO0_FUNC_MASK GENMASK(3, 0)
> > @@ -430,6 +432,59 @@ static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
> > return regmap_update_bits(axp20x->regmap, reg, mask, cfg);
> > }
> >
> > +static int axp20x_regulator_enable_regmap(struct regulator_dev *rdev)
> > +{
> > + struct axp20x_dev *axp20x = rdev_get_drvdata(rdev);
> > + const struct regulator_desc *desc = rdev->desc;
> > +
> > + if (!rdev)
> > + return -EINVAL;
> > +
> > + switch (axp20x->variant) {
> > + case AXP209_ID:
> > + if ((desc->id == AXP20X_LDO3) &&
> > + rdev->constraints && rdev->constraints->soft_start) {
> > + int v_out;
> > + int ret;
> > +
> > + /*
> > + * On some boards, the LDO3 can be overloaded when
> > + * turning on, causing the entire PMIC to shutdown
> > + * without warning. Turning it on at the minimal voltage
> > + * and then setting the voltage to the requested value
> > + * works reliably.
> > + */
> > + if (regulator_is_enabled_regmap(rdev))
> > + break;
> > +
> > + v_out = regulator_get_voltage_sel_regmap(rdev);
> > + if (v_out < 0)
> > + return v_out;
> > +
> > + if (v_out == 0)
> > + break;
> > +
> > + ret = regulator_set_voltage_sel_regmap(rdev, 0x00);
> > + /*
> > + * A small pause is needed between
> > + * setting the voltage and enabling the LDO to give the
> > + * internal state machine time to process the request.
> > + */
> > + usleep_range(1000, 5000);
> > + ret |= regulator_enable_regmap(rdev);
> > + ret |= regulator_set_voltage_sel_regmap(rdev, v_out);
> > +
> > + return ret;
> > + }
> > + break;
> > + default:
> > + /* No quirks */
> > + break;
> > + }
> > +
> > + return regulator_enable_regmap(rdev);
> > +};
> > +
>
> This is some pretty generic code, and could be useful to some other
> users. I guess a generic function would be better for this.

Yes, makes sense. Although, should we then also distinguish between
regulators which support soft-start in hardware and devices which emulate
it by delay, like in this case?

P?ikest,
Priit

2018-12-04 14:50:16

by Priit Laes

Hi,

On Mon, Nov 26, 2018 at 05:27:54PM +0200, Priit Laes wrote:
> From: Olliver Schinagl <[email protected]>
>
> The axp20x_usb_power driver uses BIT() operations but lacks the include
> for it. Include the bitops.h header file.
>
> Signed-off-by: Olliver Schinagl <[email protected]>
> Signed-off-by: Priit Laes <[email protected]>
> ---
> drivers/power/supply/axp20x_usb_power.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
> index 42001df..f52fe77 100644
> --- a/drivers/power/supply/axp20x_usb_power.c
> +++ b/drivers/power/supply/axp20x_usb_power.c
> @@ -10,6 +10,7 @@
> * option) any later version.
> */
>
> +#include <linux/bitops.h>
> #include <linux/device.h>
> #include <linux/init.h>
> #include <linux/interrupt.h>
> --
> git-series 0.9.1

Thanks, queued to power-supply-next.

-- Sebastian

Attachments:

(No filename) (943.00 B)
signature.asc (849.00 B)
Download all attachments