On Sat, Jul 27, 2024 at 02:20:31AM -0700, Frank.Sae wrote:
> +/**
> + * yt8821_config_init() - phy initializatioin
> + * @phydev: a pointer to a &struct phy_device
> + *
> + * returns 0 or negative errno code
> + */
> +static int yt8821_config_init(struct phy_device *phydev)
> +{
> + struct yt8821_priv *priv = phydev->priv;
> + int ret, val;
> +
> + phydev->irq = PHY_POLL;
> +
> + val = ytphy_read_ext_with_lock(phydev, YT8521_CHIP_CONFIG_REG);
> + if (priv->chip_mode == YT8821_CHIP_MODE_AUTO_BX2500_SGMII) {
> + ret = ytphy_modify_ext_with_lock(phydev,
> + YT8521_CHIP_CONFIG_REG,
> + YT8521_CCR_MODE_SEL_MASK,
> + FIELD_PREP(YT8521_CCR_MODE_SEL_MASK, 0));
> + if (ret < 0)
> + return ret;
> +
> + __assign_bit(PHY_INTERFACE_MODE_2500BASEX,
> + phydev->possible_interfaces,
> + true);
> + __assign_bit(PHY_INTERFACE_MODE_SGMII,
> + phydev->possible_interfaces,
> + true);
Before each and every call to .config_init, phydev->possible_interfaces
will be cleared. So, please use __set_bit() here.
> +static int yt8821_read_status(struct phy_device *phydev)
> +{
> + struct yt8821_priv *priv = phydev->priv;
> + int old_page;
> + int ret = 0;
> + int link;
> + int val;
> +
> + if (phydev->autoneg == AUTONEG_ENABLE) {
> + int lpadv = phy_read_mmd(phydev,
> + MDIO_MMD_AN, MDIO_AN_10GBT_STAT);
> +
> + if (lpadv < 0)
> + return lpadv;
> +
> + mii_10gbt_stat_mod_linkmode_lpa_t(phydev->lp_advertising,
> + lpadv);
> + }
> +
> + ret = ytphy_write_ext_with_lock(phydev,
> + YT8521_REG_SPACE_SELECT_REG,
> + YT8521_RSSR_UTP_SPACE);
> + if (ret < 0)
> + return ret;
> +
> + ret = genphy_read_status(phydev);
> + if (ret < 0)
> + return ret;
> +
> + old_page = phy_select_page(phydev, YT8521_RSSR_UTP_SPACE);
> + if (old_page < 0)
> + goto err_restore_page;
> +
> + val = __phy_read(phydev, YTPHY_SPECIFIC_STATUS_REG);
> + if (val < 0) {
> + ret = val;
> + goto err_restore_page;
> + }
> +
> + link = val & YTPHY_SSR_LINK;
What link status is this reporting? For interface switching to work,
phydev->link must _only_ indicate whether the _media_ side interface
is up or down. It must _not_ include the status of the MAC facing
interface from the PHY.
Why? The interface configuration of the MAC is only performed when
the _media_ link comes up, denoted by phydev->link becoming true.
If the MAC interface configuration mismatches the PHY interface
configuration, then the MAC facing interface of the PHY will
remain down, and if phydev->link is forced to false, then the link
will never come up.
So, I hope that this isn't testing the MAC facing interface status
of the PHY!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
