KSZ9477 variants of PHYs are not completely compatible with generic
phy_ethtool_get/set_eee() handlers. For example MDIO_PCS_EEE_ABLE acts
like a mirror of MDIO_AN_EEE_ADV register. If MDIO_AN_EEE_ADV set to 0,
MDIO_PCS_EEE_ABLE will be 0 too. It means, if we do
"ethtool --set-eee lan2 eee off", we won't be able to enable it again.
With this patch, instead of reading MDIO_PCS_EEE_ABLE register, the
driver will provide proper abilities.
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/phy/micrel.c | 81 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 81 insertions(+)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index d5b80c31ab91..dca61a73c144 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -1370,6 +1370,85 @@ static int ksz9131_config_aneg(struct phy_device *phydev)
return genphy_config_aneg(phydev);
}
+static void ksz9477_get_eee_caps(struct phy_device *phydev,
+ struct ethtool_eee *data)
+{
+ /* At least on KSZ8563 (which has same PHY_ID as KSZ9477), the
+ * MDIO_PCS_EEE_ABLE register is a mirror of MDIO_AN_EEE_ADV register.
+ * So, we need to provide this information by driver.
+ */
+ data->supported = SUPPORTED_100baseT_Full;
+
+ /* KSZ8563 is able to advertise not supported MDIO_EEE_1000T.
+ * We need to test if the PHY is 1Gbit capable.
+ */
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+ phydev->supported))
+ data->supported |= SUPPORTED_1000baseT_Full;
+}
+
+static int ksz9477_get_eee(struct phy_device *phydev, struct ethtool_eee *data)
+{
+ int val;
+
+ ksz9477_get_eee_caps(phydev, data);
+
+ /* Get advertisement EEE */
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV);
+ if (val < 0)
+ return val;
+ data->advertised = mmd_eee_adv_to_ethtool_adv_t(val);
+ data->eee_enabled = !!data->advertised;
+
+ /* Get LP advertisement EEE */
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_LPABLE);
+ if (val < 0)
+ return val;
+ data->lp_advertised = mmd_eee_adv_to_ethtool_adv_t(val);
+
+ data->eee_active = !!(data->advertised & data->lp_advertised);
+
+ return 0;
+}
+
+static int ksz9477_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
+{
+ int old_adv, adv = 0, ret;
+
+ ksz9477_get_eee_caps(phydev, data);
+
+ old_adv = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV);
+ if (old_adv < 0)
+ return old_adv;
+
+ if (data->eee_enabled) {
+ if (!data->advertised)
+ adv = ethtool_adv_to_mmd_eee_adv_t(data->supported);
+ else
+ adv = ethtool_adv_to_mmd_eee_adv_t(data->advertised &
+ data->supported);
+ /* Mask prohibited EEE modes */
+ adv &= ~phydev->eee_broken_modes;
+ }
+
+ if (old_adv != adv) {
+ ret = phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, adv);
+ if (ret < 0)
+ return ret;
+
+ /* Restart autonegotiation so the new modes get sent to the
+ * link partner.
+ */
+ if (phydev->autoneg == AUTONEG_ENABLE) {
+ ret = phy_restart_aneg(phydev);
+ if (ret < 0)
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
#define KSZ8873MLL_GLOBAL_CONTROL_4 0x06
#define KSZ8873MLL_GLOBAL_CONTROL_4_DUPLEX BIT(6)
#define KSZ8873MLL_GLOBAL_CONTROL_4_SPEED BIT(4)
@@ -3422,6 +3501,8 @@ static struct phy_driver ksphy_driver[] = {
.handle_interrupt = kszphy_handle_interrupt,
.suspend = genphy_suspend,
.resume = genphy_resume,
+ .get_eee = ksz9477_get_eee,
+ .set_eee = ksz9477_set_eee,
} };
module_phy_driver(ksphy_driver);
--
2.30.2
Hi Oleksij,
On Fri, 2023-01-20 at 10:20 +0100, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> KSZ9477 variants of PHYs are not completely compatible with generic
> phy_ethtool_get/set_eee() handlers. For example MDIO_PCS_EEE_ABLE
> acts
> like a mirror of MDIO_AN_EEE_ADV register. If MDIO_AN_EEE_ADV set to
> 0,
> MDIO_PCS_EEE_ABLE will be 0 too. It means, if we do
> "ethtool --set-eee lan2 eee off", we won't be able to enable it
> again.
>
> With this patch, instead of reading MDIO_PCS_EEE_ABLE register, the
> driver will provide proper abilities.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---
> drivers/net/phy/micrel.c | 81
> ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 81 insertions(+)
>
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index d5b80c31ab91..dca61a73c144 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -1370,6 +1370,85 @@ static int ksz9131_config_aneg(struct
> phy_device *phydev)
> return genphy_config_aneg(phydev);
> }
>
> +static int ksz9477_set_eee(struct phy_device *phydev, struct
> ethtool_eee *data)
> +{
> + int old_adv, adv = 0, ret;
nit: you can consider declaring variable in two lines, one with
initialized and other with uninitialized.
int old_adv, ret;
int adv = 0;
> +
> + ksz9477_get_eee_caps(phydev, data);
> +
> + old_adv = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV);
> + if (old_adv < 0)
> + return old_adv;
> +
> + if (data->eee_enabled) {
> + if (!data->advertised)
> + adv = ethtool_adv_to_mmd_eee_adv_t(data-
> >supported);
> + else
> + adv = ethtool_adv_to_mmd_eee_adv_t(data-
> >advertised &
> + data-
> >supported);
> + /* Mask prohibited EEE modes */
> + adv &= ~phydev->eee_broken_modes;
> + }
> +
> + if (old_adv != adv) {
> + ret = phy_write_mmd(phydev, MDIO_MMD_AN,
> MDIO_AN_EEE_ADV, adv);
> + if (ret < 0)
> + return ret;
> +
> + /* Restart autonegotiation so the new modes get sent
> to the
> + * link partner.
> + */
> + if (phydev->autoneg == AUTONEG_ENABLE) {
> + ret = phy_restart_aneg(phydev);
> + if (ret < 0)
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
>
>