[PATCH] arm: dts: sun50i-h6-orangepi-3: disable aldo2 regulator
Mainline TF-A has a broken behavior - it enables all used AXP regulator outputs, without much reason.
It breaks PHY on Orange Pi 3 and other boards, because they a specific power-on sequence is required. aldo2 which is enabled by TF-A is just a half of PHY's power and the other half that is untouched by TF-A must be enabled simultaneously (even a small difference may cause a break-down). If not TF-A, kernel driver would do everything correctly.
However, some boards rely on this behavior, so it's impossible to get rid of it.
TF-A recently started checking regulator status, and now it enables a regulator only if it's status is "okay". Disabling regulator in U-Boot's dts workarounds the problem. --- arch/arm/dts/sun50i-h6-orangepi-3.dts | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/dts/sun50i-h6-orangepi-3.dts b/arch/arm/dts/sun50i-h6-orangepi-3.dts index 7e83f6146f..9f91e4ec88 100644 --- a/arch/arm/dts/sun50i-h6-orangepi-3.dts +++ b/arch/arm/dts/sun50i-h6-orangepi-3.dts @@ -207,6 +207,10 @@ regulator-min-microvolt = <3300000>; regulator-max-microvolt = <3300000>; regulator-name = "vcc33-audio-tv-ephy-mac"; + /* This regulator must be enabled by the kernel, + * not by u-boot or TF-A, otherwise power-on + * sequence will be wrong and PHY won't work */ + status = "disabled"; };
/* ALDO3 is shorted to CLDO1 */
On Tue, 7 Sep 2021 19:23:26 +0300 Maxim Karasev begs@disroot.org wrote:
Hi Maxim,
please add the respective maintainers in To: or CC:, as reported by scripts/get_maintainer.pl. Also please add at least "sunxi" or "allwinner" somewhere in the subject line, that helps the reduce the response time ;-)
Mainline TF-A has a broken behavior - it enables all used AXP regulator outputs, without much reason.
Without that code you could not use most peripherals powered by the PMIC in U-Boot, the PHY being the most prominent one. But yes, it's a long standing issue - at least for the OPi3, which seems to be one of the few boards really having an issue with that. And the real solution is to drop the regulator handling in TF-A, and use a DM compliant AXP driver in U-Boot. Samuel's recent work should have brought that a bit closer, but it's still quite some work ahead: https://oftc.irclog.whitequark.org/linux-sunxi/2021-10-11#30291278;
It breaks PHY on Orange Pi 3 and other boards, because they a specific power-on sequence is required. aldo2 which is enabled by TF-A is just a half of PHY's power and the other half that is untouched by TF-A must be enabled simultaneously (even a small difference may cause a break-down).
I wish OrangePi would have designed a bit less of a special snowflake here.
If not TF-A, kernel driver would do everything correctly.
However, some boards rely on this behavior, so it's impossible to get rid of it.
TF-A recently started checking regulator status, and now it enables a regulator only if it's status is "okay". Disabling regulator in U-Boot's dts workarounds the problem.
The problem is that there is no such thing as "U-Boot's dts". I always use that DT to pass it on to the kernel (via $fdtcontroladdr), and this is the designated way to use UEFI. And that is just one reason we don't accept DT hacks just in U-Boot.
I was thinking about adding a TF-A build option that would skip regulator setup altogether, which would then become standard in some happy feature when U-Boot takes proper care of regulators. Maybe you can just disable this already in your TF-A build, to see if that fixes your issues?
In any case your patch below is unfortunately not a solution.
Cheers, Andre
arch/arm/dts/sun50i-h6-orangepi-3.dts | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/dts/sun50i-h6-orangepi-3.dts b/arch/arm/dts/sun50i-h6-orangepi-3.dts index 7e83f6146f..9f91e4ec88 100644 --- a/arch/arm/dts/sun50i-h6-orangepi-3.dts +++ b/arch/arm/dts/sun50i-h6-orangepi-3.dts @@ -207,6 +207,10 @@ regulator-min-microvolt = <3300000>; regulator-max-microvolt = <3300000>; regulator-name = "vcc33-audio-tv-ephy-mac";
/* This regulator must be enabled by the kernel,* not by u-boot or TF-A, otherwise power-on* sequence will be wrong and PHY won't work */status = "disabled"; }; /* ALDO3 is shorted to CLDO1 */
Hi. Thanks for explanation, I'm new to mailing lists, so I appreciate this.
Also I'm happy to hear that any work is done to address that issue.
I was thinking about adding a TF-A build option that would skip regulator setup altogether, which would then become standard in some happy feature when U-Boot takes proper care of regulators. Maybe you can just disable this already in your TF-A build, to see if that fixes your issues?
It believe it should fix that exact issue, but in my situation (I'm a distro packager) such solutions (having a several atf versions) are unacceptable.
In any case your patch below is unfortunately not a solution.
It's rather the most painless workaround in my case.
On October 13, 2021 7:25:36 PM GMT+03:00, Andre Przywara andre.przywara@arm.com wrote:
On Tue, 7 Sep 2021 19:23:26 +0300 Maxim Karasev begs@disroot.org wrote:
Hi Maxim,
please add the respective maintainers in To: or CC:, as reported by scripts/get_maintainer.pl. Also please add at least "sunxi" or "allwinner" somewhere in the subject line, that helps the reduce the response time ;-)
Mainline TF-A has a broken behavior - it enables all used AXP regulator outputs, without much reason.
Without that code you could not use most peripherals powered by the PMIC in U-Boot, the PHY being the most prominent one. But yes, it's a long standing issue - at least for the OPi3, which seems to be one of the few boards really having an issue with that. And the real solution is to drop the regulator handling in TF-A, and use a DM compliant AXP driver in U-Boot. Samuel's recent work should have brought that a bit closer, but it's still quite some work ahead: https://oftc.irclog.whitequark.org/linux-sunxi/2021-10-11#30291278;
It breaks PHY on Orange Pi 3 and other boards, because they a specific power-on sequence is required. aldo2 which is enabled by TF-A is just a half of PHY's power and the other half that is untouched by TF-A must be enabled simultaneously (even a small difference may cause a break-down).
I wish OrangePi would have designed a bit less of a special snowflake here.
If not TF-A, kernel driver would do everything correctly.
However, some boards rely on this behavior, so it's impossible to get rid of it.
TF-A recently started checking regulator status, and now it enables a regulator only if it's status is "okay". Disabling regulator in U-Boot's dts workarounds the problem.
The problem is that there is no such thing as "U-Boot's dts". I always use that DT to pass it on to the kernel (via $fdtcontroladdr), and this is the designated way to use UEFI. And that is just one reason we don't accept DT hacks just in U-Boot.
Cheers, Andre
arch/arm/dts/sun50i-h6-orangepi-3.dts | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/dts/sun50i-h6-orangepi-3.dts b/arch/arm/dts/sun50i-h6-orangepi-3.dts index 7e83f6146f..9f91e4ec88 100644 --- a/arch/arm/dts/sun50i-h6-orangepi-3.dts +++ b/arch/arm/dts/sun50i-h6-orangepi-3.dts @@ -207,6 +207,10 @@ regulator-min-microvolt = <3300000>; regulator-max-microvolt = <3300000>; regulator-name = "vcc33-audio-tv-ephy-mac";
/* This regulator must be enabled by the kernel,* not by u-boot or TF-A, otherwise power-on* sequence will be wrong and PHY won't work */status = "disabled"; }; /* ALDO3 is shorted to CLDO1 */
On Wed, 20 Oct 2021 21:31:09 +0300 Maxim Karasev begs@disroot.org wrote:
Hi Maxim,
Hi. Thanks for explanation, I'm new to mailing lists, so I appreciate this.
Also I'm happy to hear that any work is done to address that issue.
I was thinking about adding a TF-A build option that would skip regulator setup altogether, which would then become standard in some happy feature when U-Boot takes proper care of regulators. Maybe you can just disable this already in your TF-A build, to see if that fixes your issues?
It believe it should fix that exact issue, but in my situation (I'm a distro packager) such solutions (having a several atf versions) are unacceptable.
What are you packaging, exactly? (And for what distro?) I guess it's some kind of U-Boot/firmware package [1]? So you build this per device then?
And this might not have been clear, but I was more asking for a *test* of this approach (rather than a random hack in your tree). I don't have this device, so I cannot test that easily. I made an easy patch the other day which introduces a TF-A build time option, so you would just build it for the OPi3 with "SUNXI_SKIP_REGULATOR_SETUP=1" on the make command line. Eventually, once U-Boot takes over the regulator setup, this would probably become the default, but for now this would just be a workaround for OPi3 users. It would be good to know if that fixes your problem.
In any case your patch below is unfortunately not a solution.
It's rather the most painless workaround in my case.
It's a workaround, yes. But what I was actually wondering about: where in the DT you use the MAC and those regulators? I don't see any mentioning of that in the kernel or U-Boot tree. So is this some off-tree .dts?
Cheers, Andre
[1] Please note that I consider *distros* packaging *firmware* the wrong approach. Firmware is device specific, not distro-specific. There should be no reason that the Debian firmware for some device is different from the Arch Linux firmware. I am asking for years for people to coordinate, and publish distro-generic firmware. This mostly works already with current mainline U-Boot & TF-A, but would need some testing and fine-tuning.
On October 13, 2021 7:25:36 PM GMT+03:00, Andre Przywara andre.przywara@arm.com wrote:
On Tue, 7 Sep 2021 19:23:26 +0300 Maxim Karasev begs@disroot.org wrote:
Hi Maxim,
please add the respective maintainers in To: or CC:, as reported by scripts/get_maintainer.pl. Also please add at least "sunxi" or "allwinner" somewhere in the subject line, that helps the reduce the response time ;-)
Mainline TF-A has a broken behavior - it enables all used AXP regulator outputs, without much reason.
Without that code you could not use most peripherals powered by the PMIC in U-Boot, the PHY being the most prominent one. But yes, it's a long standing issue - at least for the OPi3, which seems to be one of the few boards really having an issue with that. And the real solution is to drop the regulator handling in TF-A, and use a DM compliant AXP driver in U-Boot. Samuel's recent work should have brought that a bit closer, but it's still quite some work ahead: https://oftc.irclog.whitequark.org/linux-sunxi/2021-10-11#30291278;
It breaks PHY on Orange Pi 3 and other boards, because they a specific power-on sequence is required. aldo2 which is enabled by TF-A is just a half of PHY's power and the other half that is untouched by TF-A must be enabled simultaneously (even a small difference may cause a break-down).
I wish OrangePi would have designed a bit less of a special snowflake here.
If not TF-A, kernel driver would do everything correctly.
However, some boards rely on this behavior, so it's impossible to get rid of it.
TF-A recently started checking regulator status, and now it enables a regulator only if it's status is "okay". Disabling regulator in U-Boot's dts workarounds the problem.
The problem is that there is no such thing as "U-Boot's dts". I always use that DT to pass it on to the kernel (via $fdtcontroladdr), and this is the designated way to use UEFI. And that is just one reason we don't accept DT hacks just in U-Boot.
Cheers, Andre
arch/arm/dts/sun50i-h6-orangepi-3.dts | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/dts/sun50i-h6-orangepi-3.dts b/arch/arm/dts/sun50i-h6-orangepi-3.dts index 7e83f6146f..9f91e4ec88 100644 --- a/arch/arm/dts/sun50i-h6-orangepi-3.dts +++ b/arch/arm/dts/sun50i-h6-orangepi-3.dts @@ -207,6 +207,10 @@ regulator-min-microvolt = <3300000>; regulator-max-microvolt = <3300000>; regulator-name = "vcc33-audio-tv-ephy-mac";
/* This regulator must be enabled by the kernel,* not by u-boot or TF-A, otherwise power-on* sequence will be wrong and PHY won't work */status = "disabled"; }; /* ALDO3 is shorted to CLDO1 */
What are you packaging, exactly? (And for what distro?) I guess it's some kind of U-Boot/firmware package [1]?
Yes, it's U-Boot + ATF at Alpine and Linux + WiFi/BT firmware at postmarketOS.
So you build this per device then?
Well, not exactly. Alpine builds one shared ATF package that includes TF-A for several SoCs, plus U-Boot is a metapackage, which means that all device targets are built at the same time from the same source and then split into subpackages. Alpine wouldn't want to include separate board-specific U-Boot or ATF packages.
pmOS accepts such workarounds on the other hand, but it's still unwanted way.
And this might not have been clear, but I was more asking for a *test* of this approach (rather than a random hack in your tree). I don't have this device, so I cannot test that easily. I made an easy patch the other day which introduces a TF-A build time option, so you would just build it for the OPi3 with "SUNXI_SKIP_REGULATOR_SETUP=1" on the make command line. Eventually, once U-Boot takes over the regulator setup, this would probably become the default, but for now this would just be a workaround for OPi3 users. It would be good to know if that fixes your problem.
Following patch workarounds the issue:
diff --git a/drivers/allwinner/axp/common.c b/drivers/allwinner/axp/common.c index e98b16fdb..111a4d0d7 100644 --- a/drivers/allwinner/axp/common.c +++ b/drivers/allwinner/axp/common.c @@ -98,8 +98,9 @@ static int setup_regulator(const void *fdt, int node,
static bool should_enable_regulator(const void *fdt, int node) { - if (fdt_getprop(fdt, node, "phandle", NULL) != NULL) - return true; +//XXX: no need to enable referenced regulators +// if (fdt_getprop(fdt, node, "phandle", NULL) != NULL) +// return true; if (fdt_getprop(fdt, node, "regulator-always-on", NULL) != NULL) return true; return false;
However, idk if disabling regulator setup entirely won't break anything. I may test it, but I haven't got your patch.
Such config option would be pretty usable, e.g. creating a arm-trusted-firmware-no-reg-setup subpackage, which is built from same source, but with that config option, would be clean enough to be accepted in Alpine.
It's a workaround, yes. But what I was actually wondering about: where in the DT you use the MAC and those regulators? I don't see any mentioning of that in the kernel or U-Boot tree. So is this some off-tree .dts?
Yes, kernel source we use at pmOS for allwinner is megous.com/git/linux plus some extra device-specific patches.
Please note that I consider *distros* packaging *firmware* the wrong approach. Firmware is device specific, not distro-specific. There should be no reason that the Debian firmware for some device is different from the Arch Linux firmware. I am asking for years for people to coordinate, and publish distro-generic firmware. This mostly works already with current mainline U-Boot & TF-A, but would need some testing and fine-tuning.
Hm, I've heard about UEFI on U-Boot before and it sounds interesting, but I've never seen it in reality.
However, I see nothing wrong with distro providing a pre-build mainline U-Boot and ATF binaries somewhere at /usr/share to save user's time. It needs to be installed to the board somehow anyway, so, if not packaging, everyone would have to compile it for themselves.
pmOS just automates the process of creating a ready-to-go system images and provides useful tools and tolerance to hacks for supporting as many devices as possible.
participants (3)
-
Andre Przywara -
Andre Przywara -
Maxim Karasev