Re: [linux-next-v2 5/5] arm64: dts: rockchip: Add missing of ethernet-phy-id to reset the phy on Rock 3A SBC

From: Peter Geis <pgwipeout@gmail.com>
To: Anand Moon <linux.amoon@gmail.com>
Cc: Robin Murphy <robin.murphy@arm.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Heiko Stuebner <heiko@sntech.de>, Chukun Pan <amadeus@jmu.edu.cn>,
	Michael Riesch <michael.riesch@wolfvision.net>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [linux-next-v2 5/5] arm64: dts: rockchip: Add missing of ethernet-phy-id to reset the phy on Rock 3A SBC
Date: Thu, 17 Nov 2022 14:10:07 -0500	[thread overview]
Message-ID: <CAMdYzYqkHb0_Rwp+fAMwS=-VrNow6yFw1HErz28EGKd6bLzY4A@mail.gmail.com> (raw)
In-Reply-To: <CANAwSgT7rasp=QYdvukAO-y6NzsOfKsjdy0jAEubvMymmfDqWA@mail.gmail.com>

On Thu, Nov 17, 2022 at 9:56 AM Anand Moon <linux.amoon@gmail.com> wrote:
>
> Hi Robin
>
> On Thu, 17 Nov 2022 at 16:24, Robin Murphy <robin.murphy@arm.com> wrote:
> >
> > On 2022-11-17 05:57, Anand Moon wrote:
> > > Hi Peter,
> > >
> > > On Thu, 17 Nov 2022 at 02:16, Peter Geis <pgwipeout@gmail.com> wrote:
> > >>
> > >> On Wed, Nov 16, 2022 at 3:02 PM Anand Moon <linux.amoon@gmail.com> wrote:
> > >>>
> > >>> Add MDIO description with ethernet-phy-id compatible string
> > >>> which enable calling reset of the phy. The PHY will then be probed,
> > >>> independent of if it can be found on the bus or not,
> > >>> and that probing will enable the GPIO.
> > >>>
> > >>> ethernet-phy-id is read from ethenet register dump reg2 and reg3.
> > >>>
> > >>> Fix following warning.
> > >>> [   12.323417] rk_gmac-dwmac fe010000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
> > >>> [   12.324078] rk_gmac-dwmac fe010000.ethernet eth0: no phy at addr -1
> > >>> [   12.324099] rk_gmac-dwmac fe010000.ethernet eth0: __stmmac_open: Cannot attach to PHY (error: -19)
> > >>>
> > >>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > >>> ---
> > >>> V2: new to the patch series.
> > >>>
> > >>> alarm@rock-3a:~$ sudo ethtool -d eth0
> > >>> [sudo] password for alarm:
> > >>> ST GMAC Registers
> > >>> GMAC Registers
> > >>> Reg0  0x08072203
> > >>> Reg1  0x00000000
> > >>> Reg2  0x00000404
> > >>> Reg3  0x00000000
> > >>> Reg4  0x00000002
> > >>> ---
> > >>>   arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 2 +-
> > >>>   1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > >>> index 9f84a23a8789..fe36156a5017 100644
> > >>> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > >>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> > >>> @@ -585,7 +585,7 @@ &i2s2_2ch {
> > >>>
> > >>>   &mdio1 {
> > >>>          rgmii_phy1: ethernet-phy@0 {
> > >>> -               compatible = "ethernet-phy-ieee802.3-c22";
> > >>> +               compatible = "ethernet-phy-id0000.0404", "ethernet-phy-ieee802.3-c22";
> > >>>                  reg = <0x0>;
> > >>>                  pinctrl-names = "default";
> > >>>                  pinctrl-0 = <&eth_phy_rst>, <&eth_phy_int>;
> > >>
> > >> Have you tried instead moving the reset to the mdio bus? I've had
> > >> success with this, though you'll need to change the reset assert and
> > >> deassert timing handles, they are different for the bus.
> > >>
> > > No can you share some examples?
> > > If you got a better way to solve this issue please let me know.
> > > I will give this a try.
> >
> > Note that the Rock 3A schematic says the phy is configured for address
> > 1, not 0. From what I remember of adding the MDIO node for NanoiPi4,
> > that didn't work if I got the address wrong, despite the fact that the
> > auto-detection when the MDIO node is omitted claimed to find the same
> > phy on both addresses 0 and 1.
> >

From the net-dev folk, mdio address 0 is a broadcast address. All
functional phys we have on the mdio bus should respond to it. The
problem I've run into is with the reset on the phy node the reset
triggers too late for the dwmac to detect the phy correctly. However
moving it to the mdio bus node makes the reset happen at roughly the
same time as the depreciated dwmac reset and the phy detects without
any weird hacks.

>
> Yes, I have tested with these changes still this does not work, with
> out this fix as of now..
>
> We can check this comment in the below commit id
> 70f04e9a3358404367030493dc36718d4495a9a5  ARM: dts: imx6ul-14x14-evk:
> Enable the GPIO expander
>
> maybe something is still missing to be configured.
>
> Thanks
> -Anand
>
>
>
>
>
> > Robin.
> >
> > >
> > > Thanks
> > > -Anand
> > >>> --
> > >>> 2.38.1
> > >>>
> > >
> > > _______________________________________________
> > > Linux-rockchip mailing list
> > > Linux-rockchip@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-rockchip