From 5f15eed245bc6d7c82d44f0ebcaf62071a9d55bd Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Wed, 5 Dec 2018 21:49:40 +0100 Subject: net: mii: Fix autoneg in mii_lpa_to_linkmode_lpa_t() mii_adv_to_linkmode_adv_t() clears all bits before setting it needs to set. This means the freshly set Autoneg gets cleared. Change the order, and add comments about it clearing the old content of the bitmap. Fixes: c0ec3c273677 ("net: phy: Convert u32 phydev->lp_advertising to linkmode") Reported-by: Heiner Kallweit Signed-off-by: Andrew Lunn Signed-off-by: David S. Miller --- include/linux/mii.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/include/linux/mii.h b/include/linux/mii.h index fb7ae4ae8ce3..57365224306c 100644 --- a/include/linux/mii.h +++ b/include/linux/mii.h @@ -378,7 +378,8 @@ static inline u32 mii_lpa_to_ethtool_lpa_x(u32 lpa) * @adv: value of the MII_ADVERTISE register * * A small helper function that translates MII_ADVERTISE bits - * to linkmode advertisement settings. + * to linkmode advertisement settings. Clears the old value + * of advertising. */ static inline void mii_adv_to_linkmode_adv_t(unsigned long *advertising, u32 adv) @@ -408,16 +409,18 @@ static inline void mii_adv_to_linkmode_adv_t(unsigned long *advertising, * @adv: value of the MII_LPA register * * A small helper function that translates MII_LPA bits, when in - * 1000Base-T mode, to linkmode LP advertisement settings. + * 1000Base-T mode, to linkmode LP advertisement settings. Clears the + * old value of advertising */ static inline void mii_lpa_to_linkmode_lpa_t(unsigned long *lp_advertising, u32 lpa) { + mii_adv_to_linkmode_adv_t(lp_advertising, lpa); + if (lpa & LPA_LPACK) linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, lp_advertising); - mii_adv_to_linkmode_adv_t(lp_advertising, lpa); } /** -- cgit v1.2.3-59-g8ed1b From 78a24df370072ea3b7c0a466efd776fc8f87c73a Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Wed, 5 Dec 2018 21:49:41 +0100 Subject: net: mii: Rename mii_stat1000_to_linkmode_lpa_t Rename mii_stat1000_to_linkmode_lpa_t to mii_stat1000_mod_linkmode_lpa_t to indicate it modifies the passed linkmode bitmap, without clearing any other bits. Add a helper to set/clear bits in a linkmode. Use this helper to ensure bit are clear which the stat1000 indicates should not be set. Fixes: c0ec3c273677 ("net: phy: Convert u32 phydev->lp_advertising to linkmode") Suggested-by: Heiner Kallweit Signed-off-by: Andrew Lunn Signed-off-by: David S. Miller --- drivers/net/phy/marvell.c | 2 +- drivers/net/phy/marvell10g.c | 2 +- drivers/net/phy/phy_device.c | 4 ++-- include/linux/linkmode.h | 9 +++++++++ include/linux/mii.h | 20 ++++++++++---------- 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 6a9881942e53..03dafe0e68a2 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -1138,7 +1138,7 @@ static int marvell_read_status_page_an(struct phy_device *phydev, if (!fiber) { mii_lpa_to_linkmode_lpa_t(phydev->lp_advertising, lpa); - mii_stat1000_to_linkmode_lpa_t(phydev->lp_advertising, lpagb); + mii_stat1000_mod_linkmode_lpa_t(phydev->lp_advertising, lpagb); if (phydev->duplex == DUPLEX_FULL) { phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0; diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c index 6f6e886fc836..82ab6ed3b74e 100644 --- a/drivers/net/phy/marvell10g.c +++ b/drivers/net/phy/marvell10g.c @@ -490,7 +490,7 @@ static int mv3310_read_status(struct phy_device *phydev) if (val < 0) return val; - mii_stat1000_to_linkmode_lpa_t(phydev->lp_advertising, val); + mii_stat1000_mod_linkmode_lpa_t(phydev->lp_advertising, val); if (phydev->autoneg == AUTONEG_ENABLE) phy_resolve_aneg_linkmode(phydev); diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index e6720e2a2da6..c20b5ecc0f4b 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1739,8 +1739,8 @@ int genphy_read_status(struct phy_device *phydev) return -ENOLINK; } - mii_stat1000_to_linkmode_lpa_t(phydev->lp_advertising, - lpagb); + mii_stat1000_mod_linkmode_lpa_t(phydev->lp_advertising, + lpagb); common_adv_gb = lpagb & adv << 2; } diff --git a/include/linux/linkmode.h b/include/linux/linkmode.h index 22443d7fb5cd..a99c58866860 100644 --- a/include/linux/linkmode.h +++ b/include/linux/linkmode.h @@ -57,6 +57,15 @@ static inline void linkmode_clear_bit(int nr, volatile unsigned long *addr) __clear_bit(nr, addr); } +static inline void linkmode_mod_bit(int nr, volatile unsigned long *addr, + int set) +{ + if (set) + linkmode_set_bit(nr, addr); + else + linkmode_clear_bit(nr, addr); +} + static inline void linkmode_change_bit(int nr, volatile unsigned long *addr) { __change_bit(nr, addr); diff --git a/include/linux/mii.h b/include/linux/mii.h index 57365224306c..b915ef6c3692 100644 --- a/include/linux/mii.h +++ b/include/linux/mii.h @@ -288,22 +288,22 @@ static inline u32 mii_stat1000_to_ethtool_lpa_t(u32 lpa) } /** - * mii_stat1000_to_linkmode_lpa_t + * mii_stat1000_mod_linkmode_lpa_t * @advertising: target the linkmode advertisement settings * @adv: value of the MII_STAT1000 register * * A small helper function that translates MII_STAT1000 bits, when in - * 1000Base-T mode, to linkmode advertisement settings. + * 1000Base-T mode, to linkmode advertisement settings. Other bits in + * advertising are not changes. */ -static inline void mii_stat1000_to_linkmode_lpa_t(unsigned long *advertising, - u32 lpa) +static inline void mii_stat1000_mod_linkmode_lpa_t(unsigned long *advertising, + u32 lpa) { - if (lpa & LPA_1000HALF) - linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, - advertising); - if (lpa & LPA_1000FULL) - linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, - advertising); + linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, + advertising, lpa & LPA_1000HALF); + + linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, + advertising, lpa & LPA_1000FULL); } /** -- cgit v1.2.3-59-g8ed1b From ab9cb729ab0d4151f7ba76dd3ff592242a694754 Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Wed, 5 Dec 2018 21:49:42 +0100 Subject: phy: marvell: Rename mii_lpa_to_linkmode_lpa_t Rename mii_lpa_to_linkmode_lpa_t to mii_lpa_mod_linkmode_lpa_t to indicate it modifies the passed linkmode bitmap, without clearing any other bits. Also, ensure bit are clear which the lpa indicates should not be set. Fixes: c0ec3c273677 ("net: phy: Convert u32 phydev->lp_advertising to linkmode") Suggested-by: Heiner Kallweit Signed-off-by: Andrew Lunn Signed-off-by: David S. Miller --- drivers/net/phy/marvell.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 03dafe0e68a2..a9c7c7f41b0c 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -1047,21 +1047,21 @@ static int m88e1145_config_init(struct phy_device *phydev) } /** - * fiber_lpa_to_linkmode_lpa_t + * fiber_lpa_mod_linkmode_lpa_t * @advertising: the linkmode advertisement settings * @lpa: value of the MII_LPA register for fiber link * - * A small helper function that translates MII_LPA - * bits to linkmode LP advertisement settings. + * A small helper function that translates MII_LPA bits to linkmode LP + * advertisement settings. Other bits in advertising are left + * unchanged. */ -static void fiber_lpa_to_linkmode_lpa_t(unsigned long *advertising, u32 lpa) +static void fiber_lpa_mod_linkmode_lpa_t(unsigned long *advertising, u32 lpa) { - if (lpa & LPA_FIBER_1000HALF) - linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, - advertising); - if (lpa & LPA_FIBER_1000FULL) - linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, - advertising); + linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, + advertising, lpa & LPA_FIBER_1000HALF); + + linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, + advertising, lpa & LPA_FIBER_1000FULL); } /** @@ -1146,7 +1146,7 @@ static int marvell_read_status_page_an(struct phy_device *phydev, } } else { /* The fiber link is only 1000M capable */ - fiber_lpa_to_linkmode_lpa_t(phydev->lp_advertising, lpa); + fiber_lpa_mod_linkmode_lpa_t(phydev->lp_advertising, lpa); if (phydev->duplex == DUPLEX_FULL) { if (!(lpa & LPA_PAUSE_FIBER)) { -- cgit v1.2.3-59-g8ed1b From d3351931a37bdb329b5ea761424579fa91c866ee Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Wed, 5 Dec 2018 21:49:43 +0100 Subject: net: mii: Add mii_lpa_mod_linkmode_lpa_t Add a _mod_ variant of mii_lpa_to_linkmode_lpa_t. Use this to fix the genphy_read_status() where the 1G link partner features are getting lost. Fixes: c0ec3c273677 ("net: phy: Convert u32 phydev->lp_advertising to linkmode") Reported-by: Heiner Kallweit Signed-off-by: Andrew Lunn Signed-off-by: David S. Miller --- drivers/net/phy/phy_device.c | 2 +- include/linux/mii.h | 68 +++++++++++++++++++++++++++++++++----------- 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index c20b5ecc0f4b..7d5d698604aa 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1748,7 +1748,7 @@ int genphy_read_status(struct phy_device *phydev) if (lpa < 0) return lpa; - mii_lpa_to_linkmode_lpa_t(phydev->lp_advertising, lpa); + mii_lpa_mod_linkmode_lpa_t(phydev->lp_advertising, lpa); adv = phy_read(phydev, MII_ADVERTISE); if (adv < 0) diff --git a/include/linux/mii.h b/include/linux/mii.h index b915ef6c3692..e72447778a08 100644 --- a/include/linux/mii.h +++ b/include/linux/mii.h @@ -372,6 +372,36 @@ static inline u32 mii_lpa_to_ethtool_lpa_x(u32 lpa) return result | mii_adv_to_ethtool_adv_x(lpa); } +/** + * mii_adv_mod_linkmode_adv_t + * @advertising:pointer to destination link mode. + * @adv: value of the MII_ADVERTISE register + * + * A small helper function that translates MII_ADVERTISE bits to + * linkmode advertisement settings. Leaves other bits unchanged. + */ +static inline void mii_adv_mod_linkmode_adv_t(unsigned long *advertising, + u32 adv) +{ + linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, + advertising, adv & ADVERTISE_10HALF); + + linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, + advertising, adv & ADVERTISE_10FULL); + + linkmode_mod_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, + advertising, adv & ADVERTISE_100HALF); + + linkmode_mod_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, + advertising, adv & ADVERTISE_100FULL); + + linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT, advertising, + adv & ADVERTISE_PAUSE_CAP); + + linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, + advertising, adv & ADVERTISE_PAUSE_ASYM); +} + /** * mii_adv_to_linkmode_adv_t * @advertising:pointer to destination link mode. @@ -386,22 +416,7 @@ static inline void mii_adv_to_linkmode_adv_t(unsigned long *advertising, { linkmode_zero(advertising); - if (adv & ADVERTISE_10HALF) - linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, - advertising); - if (adv & ADVERTISE_10FULL) - linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, - advertising); - if (adv & ADVERTISE_100HALF) - linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, - advertising); - if (adv & ADVERTISE_100FULL) - linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, - advertising); - if (adv & ADVERTISE_PAUSE_CAP) - linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, advertising); - if (adv & ADVERTISE_PAUSE_ASYM) - linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, advertising); + mii_adv_mod_linkmode_adv_t(advertising, adv); } /** @@ -423,6 +438,27 @@ static inline void mii_lpa_to_linkmode_lpa_t(unsigned long *lp_advertising, } +/** + * mii_lpa_mod_linkmode_lpa_t + * @adv: value of the MII_LPA register + * + * A small helper function that translates MII_LPA bits, when in + * 1000Base-T mode, to linkmode LP advertisement settings. Leaves + * other bits unchanged. + */ +static inline void mii_lpa_mod_linkmode_lpa_t(unsigned long *lp_advertising, + u32 lpa) +{ + mii_adv_mod_linkmode_adv_t(lp_advertising, lpa); + + if (lpa & LPA_LPACK) + linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, + lp_advertising); + else + linkmode_clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, + lp_advertising); +} + /** * linkmode_adv_to_lcl_adv_t * @advertising:pointer to linkmode advertising -- cgit v1.2.3-59-g8ed1b From 6dbd0090f999c443763c0742b574da1ce189404c Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Wed, 5 Dec 2018 21:49:44 +0100 Subject: net: mii: mii_lpa_mod_linkmode_lpa_t: Make use of linkmode_mod_bit helper Replace the if else code structure with a call to the helper linkmode_mod_bit. Signed-off-by: Andrew Lunn Signed-off-by: David S. Miller --- include/linux/mii.h | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/include/linux/mii.h b/include/linux/mii.h index e72447778a08..6fee8b1a4400 100644 --- a/include/linux/mii.h +++ b/include/linux/mii.h @@ -451,12 +451,8 @@ static inline void mii_lpa_mod_linkmode_lpa_t(unsigned long *lp_advertising, { mii_adv_mod_linkmode_adv_t(lp_advertising, lpa); - if (lpa & LPA_LPACK) - linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, - lp_advertising); - else - linkmode_clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, - lp_advertising); + linkmode_mod_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, + lp_advertising, lpa & LPA_LPACK); } /** -- cgit v1.2.3-59-g8ed1b From 9db299c736eea35ea97dbc9d80a58befc067bcd8 Mon Sep 17 00:00:00 2001 From: Andrew Lunn Date: Wed, 5 Dec 2018 21:49:45 +0100 Subject: net: phy: Fix ioctl handler when modifing MII_ADVERTISE When the MII_ADVERTISE register is modified by the IOCTL handler, phydev->advertising needs recalculating. Use the _mod_ variant of mii_adv_to_linkmode_adv_t so that bits outside of the advertise registers are not cleared. Fixes: c0ec3c273677 ("net: phy: Convert u32 phydev->lp_advertising to linkmode") Reported-by: Heiner Kallweit Signed-off-by: Andrew Lunn Signed-off-by: David S. Miller --- drivers/net/phy/phy.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index e1a1e54baac2..e24708f1fc16 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -437,8 +437,8 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd) } break; case MII_ADVERTISE: - mii_adv_to_linkmode_adv_t(phydev->advertising, - val); + mii_adv_mod_linkmode_adv_t(phydev->advertising, + val); change_autoneg = true; break; default: -- cgit v1.2.3-59-g8ed1b