[PATCH] arm: dts: sun50i-h6-orangepi-3: disable aldo2 regulator

[Andre Przywara]

On Wed, 20 Oct 2021 21:31:09 +0300
Maxim Karasev <begs at 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 at arm.com> wrote:
> >On Tue,  7 Sep 2021 19:23:26 +0300
> >Maxim Karasev <begs at 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 */  
> >