Re: [PATCH net-next v5] net: phy: Add driver for Motorcomm Quad 2.5GbE phy

[Andrew Lunn]

On Fri, Jul 24, 2026 at 03:18:47PM +0200, Andrew Lunn wrote:
1;4000;47c> > Ans: It is different from the m88e1111 PHY, which may support UTP, fiber, 
> >      or combo mode. However, the PHY8824 is only used with UTPs, and 
> >      its structure is as follows, which includes UTP0, UTP1,UTP2,UTP3 and USXGMII.
> > 
> >      RJ45 <----> UTP0 <------>
> >      RJ45 <----> UTP1 <------>   USXGMII  <----->  USXGMII(the side of MAC)
> >      RJ45 <----> UTP2 <------>
> >      RJ45 <----> UTP3 <------>
> > 
> >      It is similar to phy8821 driver in motorcomm.c, with the difference 
> >      being that one has one UTP port and phy8824 has four UTP ports.
> >      YT8824_RSSR_FIBER_SPACE is used to access USXGMII reg space, Perhaps it 
> >      would be more accurate to call it YT8824_RSSR_USXGMII_SPACE or 
> >      YT8824_RSSR_SERDES_SPACE.
> 
> O.K, that completely changes my understanding of this device. Yes,
> YT8824_RSSR_FIBER_SPACE should change name. And i would include this
> diagram in the driver, and indicate how YT8824_RSSR_*_SPACE map to
> this.
> 
> For the locking, look thought all the code which is touching the
> USXGMII side and see if it can be moved into probe().

So thinking about locking:

+static int yt8824_config_aneg(struct phy_device *phydev)
+{
+	int phy_ctrl = 0;
+	int ret = 0;
+
+	ret = phy8824_page_write_lock(phydev, YT8824_RSSR_UTP_SPACE);
+	if (ret < 0)
+		return ret;
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
+			      phydev->advertising))
+		phy_ctrl = MDIO_AN_10GBT_CTRL_ADV2_5G;
+
+	ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN,
+				     MDIO_AN_10GBT_CTRL,
+				     MDIO_AN_10GBT_CTRL_ADV2_5G,
+				     phy_ctrl);
+	if (ret)
+		return ret;
+
+	return genphy_config_aneg(phydev);
+

This is only touching the UTP side of things. What a PHY advertises
should not affect the USXGMII side. Why does it need
YT8824_RSSR_UTP_SPACE? What would happen if YT8824_RSSR_USXGMII_SPACE
has selected?

    Andrew