On Thu, Feb 06, 2025 at 09:18:54PM +0800, Choong Yong Liang wrote: > The xpcs_switch_interface_mode function was introduced to handle > interface switching. > > According to the XPCS datasheet, a soft reset is required to initiate > Clause 37 auto-negotiation when the XPCS switches interface modes. Hmm. Given that description, taking it literally, claus 37 auto-negotiation is 1000BASE-X, not Cisco SGMII (which isn't an IEEE 802.3 standard.) Are you absolutely sure that this applies to Cisco SGMII? If the reset is required when switching to SGMII, should it be done before or after configuring the XPCS for SGMII? Also, if the reset is required, what happens if we're already using SGMII, but AN has been disabled temporarily and is then re-enabled? Is a reset required? What about 1000BASE-X when AN is enabled or disabled and then switching to SGMII? > +static int xpcs_switch_to_aneg_c37_sgmii(const struct dw_xpcs_compat *compat, > + struct dw_xpcs *xpcs, > + unsigned int neg_mode) > +{ > + bool an_c37_enabled; > + int ret, mdio_ctrl; > + > + if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) { > + mdio_ctrl = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_BMCR); > + if (mdio_ctrl < 0) > + return mdio_ctrl; > + > + an_c37_enabled = mdio_ctrl & BMCR_ANENABLE; > + if (!an_c37_enabled) { I don't think that we need "an_c37_enabled" here, I think simply: if (!(mdio_ctrl & BMCR_ANENABLE)) { would be sufficient. > + //Perform soft reset to initiate C37 auto-negotiation > + ret = xpcs_soft_reset(xpcs, compat); > + if (ret) > + return ret; > + } > + } > + return 0; I'm also wondering (as above) whether this soft reset needs to happen _after_ xpcs_config_aneg_c37_sgmii() has done its work - this function temporarily disables AN while it's doing its work. I'm also wondering whether AN being disabled is really a deciding factor (e.g. when switching from 1000BASE-X AN-enabled to SGMII, is a reset required?) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!