Messages in this thread | | | From | "Sit, Michael Wei Hong" <> | Subject | RE: [PATCH net v2 1/2] net: stmmac: fix PHY handle parsing | Date | Tue, 21 Mar 2023 08:34:49 +0000 |
| |
> -----Original Message----- > From: Russell King <linux@armlinux.org.uk> > Sent: Saturday, March 18, 2023 4:58 AM > To: Andrew Lunn <andrew@lunn.ch> > Cc: Sit, Michael Wei Hong <michael.wei.hong.sit@intel.com>; > Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre > Torgue <alexandre.torgue@foss.st.com>; Jose Abreu > <joabreu@synopsys.com>; David S . Miller > <davem@davemloft.net>; Eric Dumazet > <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; > Paolo Abeni <pabeni@redhat.com>; Maxime Coquelin > <mcoquelin.stm32@gmail.com>; Ong, Boon Leong > <boon.leong.ong@intel.com>; netdev@vger.kernel.org; linux- > stm32@st-md-mailman.stormreply.com; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Looi, > Hong Aun <hong.aun.looi@intel.com>; Voon, Weifeng > <weifeng.voon@intel.com>; Lai, Peter Jun Ann > <peter.jun.ann.lai@intel.com> > Subject: Re: [PATCH net v2 1/2] net: stmmac: fix PHY handle > parsing > > On Fri, Mar 17, 2023 at 08:56:19PM +0100, Andrew Lunn wrote: > > On Tue, Mar 14, 2023 at 03:02:07PM +0800, Michael Sit Wei > Hong wrote: > > > phylink_fwnode_phy_connect returns 0 when set to > MLO_AN_INBAND. > > > This causes the PHY handle parsing to skip and the PHY will not > be > > > attached to the MAC. > > > > Please could you expand the commit message because i'm > having trouble > > following this. > > > > phylink_fwnode_phy_connect() says: > > > > /* Fixed links and 802.3z are handled without needing a > PHY */ > > if (pl->cfg_link_an_mode == MLO_AN_FIXED || > > (pl->cfg_link_an_mode == MLO_AN_INBAND && > > phy_interface_mode_is_8023z(pl->link_interface))) > > return 0; > > > > So your first statement is not true. It should be > MLO_AN_INBAND and > > phy_interface_mode_is_8023z. > > > > > Add additional check for PHY handle parsing when set to > MLO_AN_INBAND. > > > > Looking at the patch, there is no reference to > MLO_AN_INBAND, or > > managed = "in-band-status"; > > That's the pesky "xpcs_an_inband" which ends up as phylink's > "ovr_an_inband"... I'm sure these are random renames of stuff > to make sure that people struggle to follow the code. > It is as mentioned above, the "xpcs_an_inband" will end up as "ovr_an_inband" which will then set pl->cfg_link_an_mode = MLO_AN_INBAND in the phylink_parse_mode() in phylink.c
The phylink_fwnode_phy_connect() checks if both MLO_AN_INBAND && phy_interface_mode_is_8023z() true before returning 0.
But in our case, we only have MLO_AN_INBAND is true, which then goes to the next part of the code.
phy_fwnode = fwnode_get_phy_node(fwnode); if (IS_ERR(phy_fwnode)) { if (pl->cfg_link_an_mode == MLO_AN_PHY) return -ENODEV; return 0; }
Where here the IS_ERR(phy_fwnode) returns true, then it Checks for MLO_AN_PHY, which in our case is not, so it returns a 0.
When returned 0, our driver will then skip the manual phy parsing due to if (!fwnode || ret) > -- > RMK's Patch system: > https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at > last!
| |