From 5eee3bb7103f4a66e4b90c2817f5e72509a2a607 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Fri, 20 Mar 2020 17:51:38 +0100 Subject: [PATCH 1/3] net: phy: add and use phy_check_downshift So far PHY drivers have to check whether a downshift occurred to be able to notify the user. To make life of drivers authors a little bit easier move the downshift notification to phylib. phy_check_downshift() compares the highest mutually advertised speed with the actual value of phydev->speed (typically read by the PHY driver from a vendor-specific register) to detect a downshift. v2: - Add downshift hint to phy_print_status Signed-off-by: Heiner Kallweit Signed-off-by: David S. Miller --- drivers/net/phy/phy-core.c | 38 ++++++++++++++++++++++++++++++++++++++ drivers/net/phy/phy.c | 4 +++- include/linux/phy.h | 3 +++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c index 94cd85b1e49b..66b8c61ca74c 100644 --- a/drivers/net/phy/phy-core.c +++ b/drivers/net/phy/phy-core.c @@ -329,6 +329,44 @@ void phy_resolve_aneg_linkmode(struct phy_device *phydev) } EXPORT_SYMBOL_GPL(phy_resolve_aneg_linkmode); +/** + * phy_check_downshift - check whether downshift occurred + * @phydev: The phy_device struct + * + * Check whether a downshift to a lower speed occurred. If this should be the + * case warn the user. + * Prerequisite for detecting downshift is that PHY driver implements the + * read_status callback and sets phydev->speed to the actual link speed. + */ +void phy_check_downshift(struct phy_device *phydev) +{ + __ETHTOOL_DECLARE_LINK_MODE_MASK(common); + int i, speed = SPEED_UNKNOWN; + + phydev->downshifted_rate = 0; + + if (phydev->autoneg == AUTONEG_DISABLE || + phydev->speed == SPEED_UNKNOWN) + return; + + linkmode_and(common, phydev->lp_advertising, phydev->advertising); + + for (i = 0; i < ARRAY_SIZE(settings); i++) + if (test_bit(settings[i].bit, common)) { + speed = settings[i].speed; + break; + } + + if (speed == SPEED_UNKNOWN || phydev->speed >= speed) + return; + + phydev_warn(phydev, "Downshift occurred from negotiated speed %s to actual speed %s, check cabling!\n", + phy_speed_to_str(speed), phy_speed_to_str(phydev->speed)); + + phydev->downshifted_rate = 1; +} +EXPORT_SYMBOL_GPL(phy_check_downshift); + static int phy_resolve_min_speed(struct phy_device *phydev, bool fdx_only) { __ETHTOOL_DECLARE_LINK_MODE_MASK(common); diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index d71212a418f3..72c69a9c8a98 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -96,9 +96,10 @@ void phy_print_status(struct phy_device *phydev) { if (phydev->link) { netdev_info(phydev->attached_dev, - "Link is Up - %s/%s - flow control %s\n", + "Link is Up - %s/%s %s- flow control %s\n", phy_speed_to_str(phydev->speed), phy_duplex_to_str(phydev->duplex), + phydev->downshifted_rate ? "(downshifted) " : "", phy_pause_str(phydev)); } else { netdev_info(phydev->attached_dev, "Link is Down\n"); @@ -507,6 +508,7 @@ static int phy_check_link_status(struct phy_device *phydev) return err; if (phydev->link && phydev->state != PHY_RUNNING) { + phy_check_downshift(phydev); phydev->state = PHY_RUNNING; phy_link_up(phydev); } else if (!phydev->link && phydev->state != PHY_NOLINK) { diff --git a/include/linux/phy.h b/include/linux/phy.h index 36d9dea04016..99b5e3c4b621 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -365,6 +365,7 @@ struct macsec_ops; * suspended_by_mdio_bus: Set to true if this phy was suspended by MDIO bus. * sysfs_links: Internal boolean tracking sysfs symbolic links setup/removal. * loopback_enabled: Set true if this phy has been loopbacked successfully. + * downshifted_rate: Set true if link speed has been downshifted. * state: state of the PHY for management purposes * dev_flags: Device-specific flags used by the PHY driver. * irq: IRQ number of the PHY's interrupt (-1 if none) @@ -405,6 +406,7 @@ struct phy_device { unsigned suspended_by_mdio_bus:1; unsigned sysfs_links:1; unsigned loopback_enabled:1; + unsigned downshifted_rate:1; unsigned autoneg:1; /* The most recently read link state */ @@ -698,6 +700,7 @@ static inline bool phy_is_started(struct phy_device *phydev) void phy_resolve_aneg_pause(struct phy_device *phydev); void phy_resolve_aneg_linkmode(struct phy_device *phydev); +void phy_check_downshift(struct phy_device *phydev); /** * phy_read - Convenience function for reading a given PHY register From efbd721ebfc2e45ef66f437da7d55d60d6366bb5 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Fri, 20 Mar 2020 17:52:10 +0100 Subject: [PATCH 2/3] net: phy: marvell: remove downshift warning now that phylib takes care Now that phylib notifies the user of a downshift we can remove this functionality from the driver. Signed-off-by: Heiner Kallweit Signed-off-by: David S. Miller --- drivers/net/phy/marvell.c | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 9a8badafea8a..4714ca0e0d4b 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -867,21 +867,6 @@ static int m88e1011_set_tunable(struct phy_device *phydev, } } -static void m88e1011_link_change_notify(struct phy_device *phydev) -{ - int status; - - if (phydev->state != PHY_RUNNING) - return; - - /* we may be on fiber page currently */ - status = phy_read_paged(phydev, MII_MARVELL_COPPER_PAGE, - MII_M1011_PHY_SSR); - - if (status > 0 && status & MII_M1011_PHY_SSR_DOWNSHIFT) - phydev_warn(phydev, "Downshift occurred! Cabling may be defective.\n"); -} - static int m88e1116r_config_init(struct phy_device *phydev) { int err; @@ -2201,7 +2186,6 @@ static struct phy_driver marvell_drivers[] = { .get_stats = marvell_get_stats, .get_tunable = m88e1011_get_tunable, .set_tunable = m88e1011_set_tunable, - .link_change_notify = m88e1011_link_change_notify, }, { .phy_id = MARVELL_PHY_ID_88E1111, @@ -2223,7 +2207,6 @@ static struct phy_driver marvell_drivers[] = { .get_stats = marvell_get_stats, .get_tunable = m88e1111_get_tunable, .set_tunable = m88e1111_set_tunable, - .link_change_notify = m88e1011_link_change_notify, }, { .phy_id = MARVELL_PHY_ID_88E1118, @@ -2264,7 +2247,6 @@ static struct phy_driver marvell_drivers[] = { .get_stats = marvell_get_stats, .get_tunable = m88e1011_get_tunable, .set_tunable = m88e1011_set_tunable, - .link_change_notify = m88e1011_link_change_notify, }, { .phy_id = MARVELL_PHY_ID_88E1318S, @@ -2308,7 +2290,6 @@ static struct phy_driver marvell_drivers[] = { .get_stats = marvell_get_stats, .get_tunable = m88e1111_get_tunable, .set_tunable = m88e1111_set_tunable, - .link_change_notify = m88e1011_link_change_notify, }, { .phy_id = MARVELL_PHY_ID_88E1149R, @@ -2364,7 +2345,6 @@ static struct phy_driver marvell_drivers[] = { .get_stats = marvell_get_stats, .get_tunable = m88e1011_get_tunable, .set_tunable = m88e1011_set_tunable, - .link_change_notify = m88e1011_link_change_notify, }, { .phy_id = MARVELL_PHY_ID_88E1510, @@ -2390,7 +2370,6 @@ static struct phy_driver marvell_drivers[] = { .set_loopback = genphy_loopback, .get_tunable = m88e1011_get_tunable, .set_tunable = m88e1011_set_tunable, - .link_change_notify = m88e1011_link_change_notify, }, { .phy_id = MARVELL_PHY_ID_88E1540, @@ -2413,7 +2392,6 @@ static struct phy_driver marvell_drivers[] = { .get_stats = marvell_get_stats, .get_tunable = m88e1540_get_tunable, .set_tunable = m88e1540_set_tunable, - .link_change_notify = m88e1011_link_change_notify, }, { .phy_id = MARVELL_PHY_ID_88E1545, @@ -2436,7 +2414,6 @@ static struct phy_driver marvell_drivers[] = { .get_stats = marvell_get_stats, .get_tunable = m88e1540_get_tunable, .set_tunable = m88e1540_set_tunable, - .link_change_notify = m88e1011_link_change_notify, }, { .phy_id = MARVELL_PHY_ID_88E3016, @@ -2479,7 +2456,6 @@ static struct phy_driver marvell_drivers[] = { .get_stats = marvell_get_stats, .get_tunable = m88e1540_get_tunable, .set_tunable = m88e1540_set_tunable, - .link_change_notify = m88e1011_link_change_notify, }, }; From 1ec32eb68562637ce2b83eebb6ca39a6b39a3a0b Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Fri, 20 Mar 2020 17:52:53 +0100 Subject: [PATCH 3/3] net: phy: aquantia: remove downshift warning now that phylib takes care Now that phylib notifies the user of a downshift we can remove this functionality from the driver. Signed-off-by: Heiner Kallweit Signed-off-by: David S. Miller --- drivers/net/phy/aquantia_main.c | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c index 31927b2c7d5a..837d5eaf9e76 100644 --- a/drivers/net/phy/aquantia_main.c +++ b/drivers/net/phy/aquantia_main.c @@ -290,17 +290,6 @@ static int aqr_read_status(struct phy_device *phydev) return genphy_c45_read_status(phydev); } -static int aqr107_read_downshift_event(struct phy_device *phydev) -{ - int val; - - val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_TX_VEND_INT_STATUS1); - if (val < 0) - return val; - - return !!(val & MDIO_AN_TX_VEND_INT_STATUS1_DOWNSHIFT); -} - static int aqr107_read_rate(struct phy_device *phydev) { int val; @@ -377,13 +366,7 @@ static int aqr107_read_status(struct phy_device *phydev) break; } - val = aqr107_read_downshift_event(phydev); - if (val <= 0) - return val; - - phydev_warn(phydev, "Downshift occurred! Cabling may be defective.\n"); - - /* Read downshifted rate from vendor register */ + /* Read possibly downshifted rate from vendor register */ return aqr107_read_rate(phydev); } @@ -506,9 +489,6 @@ static int aqr107_config_init(struct phy_device *phydev) if (!ret) aqr107_chip_info(phydev); - /* ensure that a latched downshift event is cleared */ - aqr107_read_downshift_event(phydev); - return aqr107_set_downshift(phydev, MDIO_AN_VEND_PROV_DOWNSHIFT_DFLT); } @@ -533,9 +513,6 @@ static int aqcs109_config_init(struct phy_device *phydev) if (ret) return ret; - /* ensure that a latched downshift event is cleared */ - aqr107_read_downshift_event(phydev); - return aqr107_set_downshift(phydev, MDIO_AN_VEND_PROV_DOWNSHIFT_DFLT); }