| Message ID | 0b1aa5b28cb5efe17c04150a181ef1fa4027bc55.1543245984.git-series.plaes@plaes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | regulator: axp20x: Stop AXP209 from crashing when enabling LDO3 | expand |
On Mon, Nov 26, 2018 at 05:27:50PM +0200, Priit Laes wrote: > 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. In general you want to run regulators at the lowest voltage you can, this tends to reduce power consumption. > 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. > ®_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"; > }; This is obviously broken even according to your analysis above - if you have consumers for which 2.8V is too low allowing other consumers to set even lower voltages is not going to help as soon as they start doing that.
On Mon, Nov 26, 2018 at 05:27:50PM +0200, Priit Laes wrote: > From: Olliver Schinagl <oliver@schinagl.nl> > > 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: > > ®_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 <oliver@schinagl.nl> > Signed-off-by: Priit Laes <plaes@plaes.org> > --- > 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 @@ > }; > > ®_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. Maxime
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 <oliver@schinagl.nl> > > > > 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: > > > > ®_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 <oliver@schinagl.nl> > > Signed-off-by: Priit Laes <plaes@plaes.org> > > --- > > 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 @@ > > }; > > > > ®_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
On Wed, Nov 28, 2018 at 10:56:39AM +0100, Maxime Ripard wrote: > 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 <oliver@schinagl.nl> > > > > > > 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. > > > [ ... ] > > 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 Quick test (after applying this: http://code.bulix.org/w01xab-518044?raw ) results in failure: [snip] dmesg -t |egrep "(regu|seria)" sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply vcc-pb not found, using dummy regulator sun4i-pinctrl 1c20800.pinctrl: Linked as a consumer to regulator.0 sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply vcc-pb not found, using dummy regulator 1c28000.serial: ttyS0 at MMIO 0x1c28000 (irq = 47, base_baud = 1500000) is a U6_16550A sun4i-pinctrl 1c20800.pinctrl: Couldn't get bank PG regulator sun4i-pinctrl 1c20800.pinctrl: pin-198 (1c28c00.serial) status -22 dw-apb-uart 1c28c00.serial: Error applying setting, reverse things back dw-apb-uart: probe of 1c28c00.serial failed with error -22 sun4i-pinctrl 1c20800.pinctrl: Couldn't get bank PG regulator sun4i-pinctrl 1c20800.pinctrl: pin-202 (1c29000.serial) status -22 dw-apb-uart 1c29000.serial: Error applying setting, reverse things back dw-apb-uart: probe of 1c29000.serial failed with error -22 sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply vcc-pa not found, using dummy regulator sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply vcc-pa not found, using dummy regulator sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply vcc-pa not found, using dummy regulator sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply vcc-pa not found, using dummy regulator sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply vcc-pa not found, using dummy regulator sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply vcc-pa not found, using dummy regulator sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply vcc-pa not found, using dummy regulator sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply vcc-pa not found, using dummy regulator sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply vcc-pa not found, using dummy regulator sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply vcc-pa not found, using dummy regulator sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply vcc-pa not found, using dummy regulator sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply vcc-pa not found, using dummy regulator sun4i-pinctrl 1c20800.pinctrl: 1c20800.pinctrl supply vcc-pa not found, using dummy regulator ... [/snip] And I cannot actually see the link between pinctrl and ldo4. There's currently only this link: sun4i-pinctrl 1c20800.pinctrl: Linked as a consumer to regulator.0 $ cat /sys/class/regulator/regulator.0/name regulator-dummy > > Thanks! > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
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 @@ }; ®_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"; }; ®_usb0_vbus {