> +static int qca807x_read_fiber_status(struct phy_device *phydev) > +{ > + int ss, err, lpa, old_link = phydev->link; > + > + /* Update the link, but return if there was an error */ > + err = genphy_update_link(phydev); > + if (err) > + return err; > + > + /* why bother the PHY if nothing can have changed */ > + if (phydev->autoneg == AUTONEG_ENABLE && old_link && phydev->link) > + return 0; > + > + phydev->speed = SPEED_UNKNOWN; > + phydev->duplex = DUPLEX_UNKNOWN; > + phydev->pause = 0; > + phydev->asym_pause = 0; > + > + if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) { > + lpa = phy_read(phydev, MII_LPA); > + if (lpa < 0) > + return lpa; > + > + linkmode_mod_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, > + phydev->lp_advertising, lpa & LPA_LPACK); > + linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, > + phydev->lp_advertising, lpa & LPA_1000XFULL); > + linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT, > + phydev->lp_advertising, lpa & LPA_1000XPAUSE); > + linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, > + phydev->lp_advertising, > + lpa & LPA_1000XPAUSE_ASYM); > + > + phy_resolve_aneg_linkmode(phydev); > + } This looks a lot like genphy_c37_read_status(). Can it be used? > + > + /* Read the QCA807x PHY-Specific Status register fiber page, > + * which indicates the speed and duplex that the PHY is actually > + * using, irrespective of whether we are in autoneg mode or not. > + */ > + ss = phy_read(phydev, AT803X_SPECIFIC_STATUS); > + if (ss < 0) > + return ss; > + > + if (ss & AT803X_SS_SPEED_DUPLEX_RESOLVED) { > + switch (FIELD_GET(AT803X_SS_SPEED_MASK, ss)) { > + case AT803X_SS_SPEED_100: > + phydev->speed = SPEED_100; > + break; > + case AT803X_SS_SPEED_1000: > + phydev->speed = SPEED_1000; > + break; > + } > + > + if (ss & AT803X_SS_DUPLEX) > + phydev->duplex = DUPLEX_FULL; > + else > + phydev->duplex = DUPLEX_HALF; > + } > + > + return 0; > +} > +static int qca807x_phy_package_probe_once(struct phy_device *phydev) > +{ > + struct phy_package_shared *shared = phydev->shared; > + struct qca807x_shared_priv *priv = shared->priv; > + unsigned int tx_driver_strength = 0; > + const char *package_mode_name; > + > + of_property_read_u32(shared->np, "qcom,tx-driver-strength", > + &tx_driver_strength); > + switch (tx_driver_strength) { > + case 140: > + priv->tx_driver_strength = PQSGMII_TX_DRIVER_140MV; > + break; > + case 160: > + priv->tx_driver_strength = PQSGMII_TX_DRIVER_160MV; > + break; > + case 180: > + priv->tx_driver_strength = PQSGMII_TX_DRIVER_180MV; > + break; > + case 200: ... > + case 500: > + priv->tx_driver_strength = PQSGMII_TX_DRIVER_500MV; > + break; > + case 600: > + default: If its missing default to 600. But if its an invalid value, return -EINVAL. > + priv->tx_driver_strength = PQSGMII_TX_DRIVER_600MV; > + } > + > + priv->package_mode = PHY_INTERFACE_MODE_NA; > + if (!of_property_read_string(shared->np, "qcom,package-mode", > + &package_mode_name)) { > + if (!strcasecmp(package_mode_name, > + phy_modes(PHY_INTERFACE_MODE_PSGMII))) > + priv->package_mode = PHY_INTERFACE_MODE_PSGMII; > + > + if (!strcasecmp(package_mode_name, > + phy_modes(PHY_INTERFACE_MODE_QSGMII))) > + priv->package_mode = PHY_INTERFACE_MODE_QSGMII; Again, return -EINVAL if it is neither. > +static int qca807x_phy_package_config_init_once(struct phy_device *phydev) > +{ > + struct phy_package_shared *shared = phydev->shared; > + struct qca807x_shared_priv *priv = shared->priv; > + int val, ret; > + > + phy_lock_mdio_bus(phydev); > + > + /* Set correct PHY package mode */ > + val = __phy_package_read(phydev, QCA807X_COMBO_ADDR, > + QCA807X_CHIP_CONFIGURATION); > + val &= ~QCA807X_CHIP_CONFIGURATION_MODE_CFG_MASK; > + if (priv->package_mode == PHY_INTERFACE_MODE_QSGMII) > + val |= QCA807X_CHIP_CONFIGURATION_MODE_QSGMII_SGMII; > + else > + val |= QCA807X_CHIP_CONFIGURATION_MODE_PSGMII_ALL_COPPER; What about priv->package_mode == PHY_INTERFACE_MODE_NA; Andrew