sunxi: A20-OLinuXino-LIME2: Fix ldo3/ldo4 in DTS

LDO3 and LDO4 are set to regulator-always-on which causes crashes on my A20-OLinuXino-LIME2 once axp20x-i2c.ko is loaded.

This crash is observed in combination with recent versions of Das U-Boot starting from their commit 02cc27c74f9b884b538bcd1b93342a4c05b5d608.

Hans de Goede suggests fixing the Linux DTS file via the u-boot list:

This patch implements the suggested changes. It is not enough to set the voltage to 2800000 to avoid the crash. Hence, I have also removed regulator-always-on.

---
 arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

index d5c796c..d5ff2e9 100644
@@ -140,15 +140,13 @@
 			};
 
 			vcc_csi0: ldo3 {
-				regulator-min-microvolt = <700000>;
-				regulator-max-microvolt = <3500000>;
-				regulator-always-on;
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
 			};
 
 			vcc_csi1: ldo4 {
-				regulator-min-microvolt = <1250000>;
-				regulator-max-microvolt = <3300000>;
-				regulator-always-on;
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
 			};
 
 			vdd_cpu: dcdc2 {

2.7.2

The A20-OLinuXino-LIME2 uses the AXP209 PMIC. This patch adds the corresponding usb-power-supply node to the DTS.
If CONFIG_AXP20X_POWER is set, this patch enables the /sys/class/power_supply/ diirectory.

---
 arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts | 3 +++
 1 file changed, 3 insertions(+)

index d5ff2e9..d370166 100644
@@ -161,6 +161,9 @@
 				regulator-always-on;
 			};
 		};
+		usb_power_supply: usb_power_supply {
+			compatible = "x-powers,axp202-usb-power-supply";
+		};
 	};
 };
 

2.7.2

Hi,

Is there *any* reason to not use the AXP209 dtsi where it's already defined?

Thanks,
Maxime

Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160402/fe44389a/attachment-0001.sig>

Hi Maxime,

do you have any preference for this being in the AXP209 dtsi? I've been thinking about it and it makes sense to enable the power supply nodes for all devices using the AXP209. They will just report themselves as offline if the PMIC indicates nothing is connected.

Best,

Michael

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160501/5dcb9301/attachment.sig>

Yes, I'd prefer that a lot :)

It avoids enabling it on all the boards.

Thanks,
Maxime

Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160502/c0040715/attachment-0001.sig>

The A20 processor provides a third I2C bus on pins PB20 and PB21. The A20-OLinuXino-LIME2 exposes this bus via its GPIO1 port.

Olimex also provide a breakout board called the A20-OLinuXino-LIME2-UEXT. This change is required to properly support I2C on the UEXT connector found there.

---
 arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts | 6 ++++++
 1 file changed, 6 insertions(+)

index d370166..17791ef 100644
@@ -179,6 +179,12 @@
 	};
 };
 
+&i2c2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c2_pins_a>;
+	status = "okay";
+};
+
 &mmc0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin_reference_design>;

2.7.2

This didn't go in originally simply because not everyone will have the breakout and they may want to use those pins for other things.

Hi,

What are the other options on that pin? Is it something exclusively dedicated to i2c on the pin headers and the documentation, or is anyone free to use that pin for whatever he wishes?

Thanks,
Maxime

Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160402/d886af41/attachment.sig>

Documentation for Olimex UEXT interface:
https://www.olimex.com/Products/Modules/UEXT/resources/UEXT_rev_B.pdf

Basically it *should* by default work as a board to board connector which supports three serial communication interfaces: I2C, SPI and UART (original document mentions RS232, but official modules work only with 3.3V logic levels).

Hello Maxime,

thank you for taking the time to review this patch set.

I assume you are talking about the UEXT connector, not about PB20 and PB21. These are regular GPIO pins multiplexed with I2C, but no other function.

Regarding UEXT: the A20-OLinuXino-LIME2-UEXT breakout board supports two use cases here.

The first one: provide a simple adapter from the 0.05" pin headers on the olinuxino to the more common world of 0.1" headers. In this use case, you can plug the A20-OLinuXino-LIME2-UEXT intoany of LCD_CON or GIO-{1,2,3} and you're good to go.

The second one: here, you want to use UEXT modules provided by Olimex. It's a simple connector exposing I2C, SPI and UART as mentioned by Priit. To use UEXT, you have to use GPIO-1 to get the correct pin mapping.

Regarding the official documentation provided by Olimex. The A20-OLinuXino-Lime2_Rev_c.pdf shows the pins on GPIO-1 as PB20 and PB21. In the schematics for the breakout board, A20-OLinuXino-Lime2-UEXT_sch.pdf, the pins for the UEXT connector are labeled as PB20/SCK and PB21/SDA. This acknowledges the double use. However, PB20 and PB21 have pull-ups by default according to A20-OLinuXino-Lime2_Rev_c.pdf,
which makes them a lot more useful as I2C than as GPIO.

What originally prompted me to submit this patch: I bought a breakout board and a small LCD with UEXT connector together with my board. Then I found out that it doesn't work out of the box because i2c2 is not enabled. Since using UEXT is probably common for that particular vendor, I believe it should be enabled by default.

Michael

Hi Michael,

Thanks for the explanation.

Unfortunately, if it isn't present on the main board, it shouldn't be in the DT for that board. Otherwise, people relying on these pins and not using the UEXT connector would be force fed a configuration that they don't want, without any way to remove it.

That kind of adjustements is a perfect use case for the overlays though, so I'd rather suggest writing one for the UEXT connector.

Thanks!
Maxime

Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160410/9d53644d/attachment.sig>

NAK.

If setting both min and max to 2800000 isn't working then I'd suggest you're just papering over the issue and not actually solving it.

It should be fairly clear that whenever those regulators are turned on by some driver you're going to see the same issue, you've just delayed the problem by removing the regulator-always-on item.

The real problem here is people monkeying with the regulators without understanding what they're doing, bothering to test the results, or caring about the consequences.

Anyway, Hans was correct in that the datasheet we have access to says that the defaults for these regulators are undefined.
I suspect that's not the whole story though, if these regulators had random values at powerup we'd have seen problems before now.
Most likely the values we think we know are simply incorrect plus there's probably some element of sequencing involved - perhaps LDO4 needs turned on first.
However I don't see sufficient detail in the datasheets to confirm any of that, so we shouldn't assume anything.
Electrically on the lime2 LDO3 goes to VCC-PE and LDO4 to VCC-PG. Neither groups E or G appear to be connected to anything other than gpio headers, so this has little to do with board design, it's something internal to the SoC that the datasheet is lacking on detail about.

One of the reasons for the regulator-always-on being in the dts was so that the regulator didn't get touched and so didn't break anything.

All said and done, it only takes a minute with a multimeter to determine actual defaults. Helpful to have multiple boards to gain some confidence that they behave the same, but still basing values like this on tiny sample sizes is probably unwise.

Looking at other A20 based boards, Cubieboards for example, shows LDO3/4 being unused and the VCC-PE/PG pins they power on the lime2 being tied directly to 3.3v. No issues there simply because there's no way to mess with the power to those blocks.

The patch you really want is something like the one below which works on my boards regardless of the presence of regulator-always-on. Verified on four boards with u-boot 2016.03 and kernel 4.5.0.

Whether you decide to remove regulator-always-on or not is a different question, but please use 2.3v, or some other proven working value, for ldo3. At least that way you have a fighting chance of the board remaining functional when something wants to turn the regulator on.

It's a good bet that the same u-boot change will have implications for other boards, if not now then at some future point.

Hans, take note, the schematic is wrong. You shouldn't assume either the schematic or you are correct. Ill considered changes to regulators can cause real damage to peoples boards.
I know we've discussed regulators before and that my opinion falls on deaf ears, but worth one last try..

Author: Iain Paton <[email protected]>
Date:   Fri Mar 25 22:07:55 2016 +0000

sunxi: A20-OLinuXino-LIME2: Update ldo3/ldo4 in DTS

A recent ill-considered u-boot change:

causes the lime2 to lockup/crash when LDO3/4 are re-enabled by the kernel.

The AXP209 datasheet shows the powerup values for these regulators to be undefined and the 2.8v suggested by the lime2 schematic also causes a lockup when the regulators are re-enabled so is obviously also incorrect.

Empirical measurement suggests LDO3 @ 2.3v and LDO4 @ 2.8v are the actual defaults, but is based on a small sample size.

Verified to work with u-boot 2016.03, kernel 4.5.0 on four lime2 boards.

---

index d5c796c..e665d22 100644
@@ -140,14 +140,14 @@
                        };
 
                        vcc_csi0: ldo3 {
-                               regulator-min-microvolt = <700000>;
-                               regulator-max-microvolt = <3500000>;
+                               regulator-min-microvolt = <2300000>;
+                               regulator-max-microvolt = <2300000>;
                                regulator-always-on;
                        };
 
                        vcc_csi1: ldo4 {
-                               regulator-min-microvolt = <1250000>;
-                               regulator-max-microvolt = <3300000>;
+                               regulator-min-microvolt = <2800000>;
+                               regulator-max-microvolt = <2800000>;
                                regulator-always-on;
                        };
 

Having reread the A20 datasheet yesterday, something in the back of my mind was bothering me overnight so I took some time to check this morning.

On the lime2 schematic we see the pins labeled as follows:

F19 : VCC_CSI0
E18 : VCC_CSI1

However the A20 datasheet doesn't label them this way, instead using:

F19 : VCC-PE : Port E Power Supply
E18 : VCC-PG : Port G Power Supply

no mention at all of CSI. CSI just happens to be a function that can be multiplexed onto ports E & G.

So lets assume for a moment we don't have a CSI device, or a CSI driver and are not using those pins for CSI functions but are instead using their default GPIO purpose, or in the case of PG perhaps for UART3 or UART4.

What happens when you disable the regulator? Hopefully from the above you'll already have worked out that you lose ports E & G when you turn their I/O power supply off and anything multiplexed onto those ports becomes unuseable.

I haven't checked other schematics, but all of the Olimex A10/A20 boards along with Cubieboard & CubieTruck label these pins as VCC_CSIn It would be my guess that there's a reference design being given to manufacturers, perhaps the original Cubieboard even, that makes this mistake and everyone else has blindly copied it.

So, unless we want to lose access to 24 GPIO pins for no real reason I'd suggest that the u-boot patch is reverted and/or the dts is altered such that the gpio driver claims the regulators and prevents them being turned off.

The u-boot commit message and reasoning given on list are trivially proved incorrect. Ports E & G are not CSI-only, losing all other functions on these ports is not acceptable.

I've also just tested building u-boot with LDO3 & 4 voltages set to 3.3v to be the same as CubieBoard, this works for lime2. So it's unlikely that the 2.8v & 2.3v settings from my previous patch make sense. The crash is actually being caused by something else, probably sequencing, due to unnecessarily turning the regulators off. Whatever the reason, we don't have enough information in the datasheets to know for sure so we simply shouldn't turn them off to begin with.

The people behind this need to take responsibility and clean up their mess.

Iain

Hi Iain,

I took some time to go through the data sheets and your analysis is on point.

I did raise an issue with Olimex via github [0]. Maybe they will update their schematics.

I do have my breakout board connected to GPIO-1.. and it turns out that mostly port G is exposed on GPIO-1. I'll be reverting Hans' patch locally so I can toggle some pins today. I am sure he will contribute his own thoughts on the matter.

I just tried that on linux and axp20x-regulator.ko refuses to set the voltage
on ldo4:

[   88.914653] at24 1-0050: 2048 byte 24c16 EEPROM, writable, 16 bytes/write
[   96.893312] axp20x-i2c 0-0034: AXP20x variant AXP209 found
[   96.910631] axp20x-i2c 0-0034: AXP20X driver loaded
[   96.955579] input: axp20x-pek as
/devices/platform/soc at 01c00000/1c2ac00.i2c/i2c-0/0-0034/axp20x-pek/input/input0
[   96.971734] ldo4: failed to apply 3300000uV constraint(-22)
[   96.990646] axp20x-regulator axp20x-regulator: Failed to register ldo4
[   97.012686] axp20x-regulator: probe of axp20x-regulator failed with

error -22

Perhaps this requires another patch [1], but unless there's a real need for me to verify this,
I'll simply revert the u-boot patch and enjoy working GPIO.

Michael

[0] https://github.com/OLIMEX/OLINUXINO/issues/35
[1] https://www.spinics.net/lists/devicetree/msg118871.html

Hi,

Yes setting the constraints to 3.3v requires the patch you link for things to work.

Given the entire discussion in this thread, I agree that fixing this in u-boot is best. But I do not see reverting the u-boot patch disabling ldo3/4 by default as the solution. ldo3/4 are unused on most boards, so disabling them by default clearly is the right thing todo IMHO.

Instead please submit a patch for configs/A20-OLinuXino-Lime2_defconfig which configures them at 2800 mV there (per the Lime2 schematic).

It would be good to also check the Lime1 schematic if they are used in the same way there, I would also welcome a patch for:

configs/A10-OLinuXino-Lime_defconfig
configs/A20-OLinuXino-Lime_defconfig

Regards,

Hans

Adding a comment to the DT summing up that whole discussion would be great too.

Maxime

Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160329/ecb1c060/attachment.sig>