Message-ID: <cm5e7g2ozviyin6p3dh2qtuh4i3f2sbqq4c2arjtdektgk7i6c@l7aawekyj6t2>
Date:   Wed, 13 Dec 2023 12:55:58 +0000
From:   Anne Macedo <retpolanne@...teo.net>
To:     Andre Przywara <andre.przywara@....com>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Chen-Yu Tsai <wens@...e.org>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Samuel Holland <samuel@...lland.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        "moderated list:ARM/Allwinner sunXi SoC support" 
        <linux-arm-kernel@...ts.infradead.org>,
        "open list:ARM/Allwinner sunXi SoC support" 
        <linux-sunxi@...ts.linux.dev>,
        open list <linux-kernel@...r.kernel.org>,
        Corentin Labbe <clabbe@...libre.com>
Subject: Re: [PATCH] arm64: dts: allwinner: Orange Pi One Plus PHY support

On Wed, Dec 13, 2023 at 12:25:23PM +0000, Andre Przywara wrote:
> On Wed, 13 Dec 2023 11:02:39 +0000
> Anne Macedo <retpolanne@...teo.net> wrote:
> 
> Hi Anne,
> 
> > On Wed, Dec 13, 2023 at 01:35:44AM +0000, Andre Przywara wrote:
> > > On Tue, 12 Dec 2023 19:27:14 +0000
> > > Anne Macedo <retpolanne@...teo.net> wrote:
> > > 
> > > Hi Anne,
> > >   
> > > > On Tue, Dec 12, 2023 at 04:22:00PM +0000, Andre Przywara wrote:  
> > > > > On Tue, 12 Dec 2023 12:28:30 +0000
> > > > > Anne Macedo <retpolanne@...teo.net> wrote:
> > > > > 
> > > > > Hi Anne,
> > > > >     
> > > > > > Adds compatible values to mdio subnodes for Ethernet PHY representing
> > > > > > Realtek 8211 PHY to Orange Pi One Plus.    
> > > > > 
> > > > > So can you state why this would be needed? This is the RTL8211 ID,    
> > > > 
> > > > Apologies, I completely forgot to include some context. 
> > > >   
> > > > > right? Which should be autodetected via MDIO. Looking back in my inbox
> > > > > you proposed this change before, for U-Boot, specifically, but I fail to
> > > > > find a solution or explanation what really happens here. Two Renesas .dts
> > > > > files have the same compatible, and the commit message talks about the
> > > > > reset line there, is this related?
> > > > > 
> > > > > So can you please give some more background and explanation? That would be
> > > > > part of a good commit message anyway ("why", not "what").    
> > > > 
> > > > Should I resend the commit with a more meaningful explanation? The
> > > > context is the following:
> > > > 
> > > > currently, ethernet doesn't seem to work on both u-boot and Linux on the
> > > > Orange Pi One Plus board. 
> > > > 
> > > > On the kernel, this error shows up:
> > > > 
> > > > Configuring network interfaces... [    5.992589] dwmac-sun8i 5020000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
> > > > [    6.000823] dwmac-sun8i 5020000.ethernet eth0: __stmmac_open: Cannot attach to PHY (error: -19)
> > > > 
> > > > After applying this fix, the PHY gets attached: 
> > > > 
> > > > Configuring network interfaces... [    6.060020] dwmac-sun8i 5020000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
> > > > [    6.069460] dwmac-sun8i 5020000.ethernet eth0: PHY [stmmac-0:01] driver [RTL8211E Gigabit Ethernet] (irq=POLL)
> > > > 
> > > > The previous compatible list that had ethernet-phy-ieee802.3-c22 fails
> > > > to find a PHY, so this patch includes the correct PHY ID with the
> > > > RTL8211 ID. 
> > > > 
> > > > The behaviour is described on [1].  
> > > 
> > > So this is all an observation, but no real explanation, isn't it?  
> > 
> > I've made some analysis on [3] on this bug, but it was based solely on
> > u-boot. I was having trouble with the regulator and on u-boot nothing
> > would trigger the GPIO PD6 and the vcc-gmac-3v3 regulator, so the NIC
> > was completely dead. Next I did an analysis based on [2] because the
> > u-boot PHY initialization was flaky.
> > 
> > > To cite [1]: "If the PHY reports an incorrect ID (or none at all) ...".
> > > I am pretty sure this is not the case here, instead we are looking at
> > > some missing platform bits, like a missing clock, reset, or most likely
> > > regulator. Or one of the existing resources is wrongly assigned or  
> > 
> > As I mentioned, PHY initialization is flaky on u-boot, so maybe that
> > assumption is correct. 
> > 
> > > configured? If the PHY is not (yet?) powered correctly when the code
> > > does the auto-detection via the MDIO bus, then the initialisation would  
> > 
> > If I recall correctly (I don't know if I kept it in my notes :c), that
> > could be the case. regulator-boot-on makes the NIC work (LEDs blink, at
> > least) but it doesn't get initialized. 
> > 
> > > fail. But since it works when overriding the auto-detection, I feel
> > > like we are papering over something here.
> > > Do you have the schematics for this board? I can only find the one for
> > > the Orange Pi Plus 2E, and I don't know how similar those two are. This
> > > shows *two* regulators, but both are activated by the same GPIO.  
> > 
> > I do. It's available on [4]
> 
> Oh damn it, I got lost in Orange Pi's naming maze again - and was looking
> for the wrong board! So thanks for the link, and this clears things up!

Yay! 

> 
> So yes, the Orange Pi *One* Plus, much like the Orange Pi 3, uses *two*
> regulators for Ethernet: one 3.3V from the PMIC's ALDO2 rail to power the
> PHY, and a discrete 2.5V regulator, enabled by GPIO PD6, for the voltage

Oh! I didn't know about the PMIC's ALDO2.

> level on the MDIO lines. On top of this there is a reset line for the PHY,
> though this is held up by a pull-up resistor, so it *should* work,
> although we should describe this in the DT.

Noting here to take a look at the reset line so I can add it as well to
the DT.

> 
> So the DT looks wrong then: The reg_gmac_3v3 is actually a 2.5V regulator,
> and phy-supply is aldo2. I think it was done the way it is to somehow make
> it work with the current DT binding and code, which just supports one
> regulator. And aldo2 is referenced as the source of reg_gmac_3v3, which
> smells like another hack to me.

	reg_gmac_3v3: gmac-3v3 {
		compatible = "regulator-fixed";
		regulator-name = "vcc-gmac-3v3";
		regulator-min-microvolt = <3300000>;
		regulator-max-microvolt = <3300000>;
		startup-delay-us = <100000>;
		enable-active-high;
		gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
		vin-supply = <&reg_aldo2>;
	};

Interesting, I see the reg_aldo2 on vin-supply for reg_gmac_3v3. I don't
understand how it works, is it linking the regulator with the ALDO2 but
it also enables the PD6 GPIO?

> 
> > > It would also be interesting to see if any of Corentin's work for the
> > > Orange Pi 3 helps here?  
> > 
> > Adding [5] for reference here, thanks! Will check it out. 
> 
> This is an older version, there are actually updates. And he also mentions
> your board as well, so I think it just can sail in the wake of the OPi 3
> Ethernet enablement.
> 
> Can you try if this change, just applied to your .dts instead, works?
> https://github.com/montjoie/linux/commit/cf6e192eca1d59be630e6729d2cef9e897b3da8c

Will do! I'll be out of my lab today but will try it at night Brazil
time. 
> 
> Cheers,
> Andre
> 
> P.S. Is there any chance where I can reply/comment on your blog? It seems
> like I can clear some things up...

Please send me suggestions off thread, the blog is a static GHPages
blog, so I didn't implement replies yet. I will be happy to include them
to my notes :) 

> 
> > [3] https://blog.retpolanne.com/hardware/embedded/2023/07/07/embedded-phy.html
> > [4] https://linux-sunxi.org/images/7/7c/OrangePi_OnePlus_Schematics_v2.0.pdf
> > [5] https://lore.kernel.org/netdev/20220509074857.195302-1-clabbe@baylibre.com/
> > 
> > Regards, Anne
> 

Regards, Anne

Powered by blists - more mailing lists