Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Martin Blumenstingl <martin.blumenstingl@googlemail.com>
To:     michael.wei.hong.sit@intel.com
Cc:     alexandre.torgue@foss.st.com, andrew@lunn.ch,
        boon.leong.ong@intel.com, davem@davemloft.net, edumazet@google.com,
        hkallweit1@gmail.com, hong.aun.looi@intel.com,
        joabreu@synopsys.com, kuba@kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-stm32@st-md-mailman.stormreply.com, linux@armlinux.org.uk,
        mcoquelin.stm32@gmail.com, netdev@vger.kernel.org,
        pabeni@redhat.com, peppe.cavallaro@st.com,
        peter.jun.ann.lai@intel.com, weifeng.voon@intel.com
Subject: RE [PATCH net v5 2/3] net: stmmac: check if MAC needs to attach to a PHY
Date:   Mon,  3 Apr 2023 23:24:34 +0200
Message-Id: <20230403212434.296975-1-martin.blumenstingl@googlemail.com>
In-Reply-To: <20230330091404.3293431-3-michael.wei.hong.sit@intel.com>
References: <20230330091404.3293431-3-michael.wei.hong.sit@intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hello,

[...]
> @@ -1144,10 +1145,11 @@ static int stmmac_init_phy(struct net_device *dev)
>  	if (fwnode)
>  		ret = phylink_fwnode_phy_connect(priv->phylink, fwnode, 0);
>  
> +	phy_needed = phylink_expects_phy(priv->phylink);
>  	/* Some DT bindings do not set-up the PHY handle. Let's try to
>  	 * manually parse it
>  	 */
> -	if (!fwnode || ret) {
> +	if (!fwnode || phy_needed || ret) {
Unfortunately this breaks Ethernet on my X96 Air board (the .dts file
can be found upstream in:
arch/arm64/boot/dts/amlogic/meson-sm1-x96-air-gbit.dts)

Working boot-log:
# dmesg | grep dwmac
[    3.699961] meson8b-dwmac ff3f0000.ethernet: IRQ eth_wake_irq not found
[    3.700944] meson8b-dwmac ff3f0000.ethernet: IRQ eth_lpi not found
[    3.707196] meson8b-dwmac ff3f0000.ethernet: PTP uses main clock
[    3.713688] meson8b-dwmac ff3f0000.ethernet: User ID: 0x11, Synopsys ID: 0x37
[    3.720201] meson8b-dwmac ff3f0000.ethernet:         DWMAC1000
[    3.725387] meson8b-dwmac ff3f0000.ethernet: DMA HW capability register supported
[    3.732832] meson8b-dwmac ff3f0000.ethernet: RX Checksum Offload Engine supported
[    3.740301] meson8b-dwmac ff3f0000.ethernet: COE Type 2
[    3.745491] meson8b-dwmac ff3f0000.ethernet: TX Checksum insertion supported
[    3.752504] meson8b-dwmac ff3f0000.ethernet: Wake-Up On Lan supported
[    3.758993] meson8b-dwmac ff3f0000.ethernet: Normal descriptors
[    3.764813] meson8b-dwmac ff3f0000.ethernet: Ring mode enabled
[    3.770629] meson8b-dwmac ff3f0000.ethernet: Enable RX Mitigation via HW Watchdog Timer
[   13.565781] meson8b-dwmac ff3f0000.ethernet end0: renamed from eth0
[   14.036061] meson8b-dwmac ff3f0000.ethernet end0: Register MEM_TYPE_PAGE_POOL RxQ-0
[   14.255617] meson8b-dwmac ff3f0000.ethernet end0: PHY [mdio_mux-0.0:00] driver [RTL8211F Gigabit Ethernet] (irq=33)
[   14.265404] meson8b-dwmac ff3f0000.ethernet end0: No Safety Features support found
[   14.267977] meson8b-dwmac ff3f0000.ethernet end0: PTP not supported by HW
[   14.275723] meson8b-dwmac ff3f0000.ethernet end0: configuring for phy/rgmii-txid link mode
[   17.394262] meson8b-dwmac ff3f0000.ethernet end0: Link is Up - 1Gbps/Full - flow control rx/tx

Non-working boot-log:
# dmesg | grep dwmac
[    3.730072] meson8b-dwmac ff3f0000.ethernet: IRQ eth_wake_irq not found
[    3.731053] meson8b-dwmac ff3f0000.ethernet: IRQ eth_lpi not found
[    3.737303] meson8b-dwmac ff3f0000.ethernet: PTP uses main clock
[    3.743795] meson8b-dwmac ff3f0000.ethernet: User ID: 0x11, Synopsys ID: 0x37
[    3.750311] meson8b-dwmac ff3f0000.ethernet:         DWMAC1000
[    3.755498] meson8b-dwmac ff3f0000.ethernet: DMA HW capability register supported
[    3.762944] meson8b-dwmac ff3f0000.ethernet: RX Checksum Offload Engine supported
[    3.770412] meson8b-dwmac ff3f0000.ethernet: COE Type 2
[    3.775603] meson8b-dwmac ff3f0000.ethernet: TX Checksum insertion supported
[    3.782615] meson8b-dwmac ff3f0000.ethernet: Wake-Up On Lan supported
[    3.789106] meson8b-dwmac ff3f0000.ethernet: Normal descriptors
[    3.794924] meson8b-dwmac ff3f0000.ethernet: Ring mode enabled
[    3.800738] meson8b-dwmac ff3f0000.ethernet: Enable RX Mitigation via HW Watchdog Timer
[   13.052942] meson8b-dwmac ff3f0000.ethernet end0: renamed from eth0
[   13.594285] meson8b-dwmac ff3f0000.ethernet end0: Register MEM_TYPE_PAGE_POOL RxQ-0
[   13.825578] meson8b-dwmac ff3f0000.ethernet end0: PHY [mdio_mux-0.0:00] driver [RTL8211F Gigabit Ethernet] (irq=33)
[   13.831358] meson8b-dwmac ff3f0000.ethernet end0: no phy found
[   13.836229] meson8b-dwmac ff3f0000.ethernet end0: __stmmac_open: Cannot attach to PHY (error: -19)

Reverting this patch fixes that problem.

I think the reason is a logic error in the patch:
As you can see the PHY is found and attached (my understanding is
that this happens through phylink_fwnode_phy_connect()). But we now
also go to that if block below even fwnode != NULL && ret == 0 (which
indicates that phylink_fwnode_phy_connect() was successful). Inside
that if block priv->plat->phy_addr then has the default value (-1)
that was set in stmmac_probe_config_dt().

I am running out of time for today. Could you please look into this
and follow up with a patch (on top of this one, as this one has
already been applied) that considers your original issues as well as
the case of my board (I suspect that all Amlogic boards that are
supported upstream are affected)? Please keep me Cc'ed so I can test
your additional patch and then add my Tested-by.


Thank you!
Martin