From 8062e2333f8f7dcd5627e22b99e18d1cbb53eedb Mon Sep 17 00:00:00 2001 From: Russell King Date: Sat, 15 Feb 2020 23:57:36 +0000 Subject: net: linkmode: make linkmode_test_bit() take const pointer linkmode_test_bit() does not modify the address; test_bit() is also declared const volatile for the same reason. There's no need for linkmode_test_bit() to be any different, and allows implementation of helpers that take a const linkmode pointer. Reviewed-by: Andrew Lunn Signed-off-by: Russell King Signed-off-by: David S. Miller --- include/linux/linkmode.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/linux') diff --git a/include/linux/linkmode.h b/include/linux/linkmode.h index fe740031339d..8e5b352e44f2 100644 --- a/include/linux/linkmode.h +++ b/include/linux/linkmode.h @@ -71,7 +71,7 @@ static inline void linkmode_change_bit(int nr, volatile unsigned long *addr) __change_bit(nr, addr); } -static inline int linkmode_test_bit(int nr, volatile unsigned long *addr) +static inline int linkmode_test_bit(int nr, const volatile unsigned long *addr) { return test_bit(nr, addr); } -- cgit v1.2.3 From a87ae8a963bde755b0962bcc18db83d611f63e7a Mon Sep 17 00:00:00 2001 From: Russell King Date: Sat, 15 Feb 2020 15:49:27 +0000 Subject: net: add helpers to resolve negotiated flow control Add a couple of helpers to resolve negotiated flow control. Two helpers are provided: - linkmode_resolve_pause() which takes the link partner and local advertisements, and decodes whether we should enable TX or RX pause at the MAC. This is useful outside of phylib, e.g. in phylink. - phy_get_pause(), which returns the TX/RX enablement status for the current negotiation results of the PHY. This allows us to centralise the flow control resolution, rather than spreading it around. Signed-off-by: Russell King Reviewed-by: Andrew Lunn Signed-off-by: David S. Miller --- drivers/net/phy/Makefile | 3 ++- drivers/net/phy/linkmode.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ drivers/net/phy/phy_device.c | 26 ++++++++++++++++++++++++++ include/linux/linkmode.h | 4 ++++ include/linux/phy.h | 3 +++ 5 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 drivers/net/phy/linkmode.c (limited to 'include/linux') diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index fe5badf13b65..d523fd5670e4 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -1,7 +1,8 @@ # SPDX-License-Identifier: GPL-2.0 # Makefile for Linux PHY drivers and MDIO bus drivers -libphy-y := phy.o phy-c45.o phy-core.o phy_device.o +libphy-y := phy.o phy-c45.o phy-core.o phy_device.o \ + linkmode.o mdio-bus-y += mdio_bus.o mdio_device.o ifdef CONFIG_MDIO_DEVICE diff --git a/drivers/net/phy/linkmode.c b/drivers/net/phy/linkmode.c new file mode 100644 index 000000000000..969918795228 --- /dev/null +++ b/drivers/net/phy/linkmode.c @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: GPL-2.0+ +#include + +/** + * linkmode_resolve_pause - resolve the allowable pause modes + * @local_adv: local advertisement in ethtool format + * @partner_adv: partner advertisement in ethtool format + * @tx_pause: pointer to bool to indicate whether transmit pause should be + * enabled. + * @rx_pause: pointer to bool to indicate whether receive pause should be + * enabled. + * + * Flow control is resolved according to our and the link partners + * advertisements using the following drawn from the 802.3 specs: + * Local device Link partner + * Pause AsymDir Pause AsymDir Result + * 0 X 0 X Disabled + * 0 1 1 0 Disabled + * 0 1 1 1 TX + * 1 0 0 X Disabled + * 1 X 1 X TX+RX + * 1 1 0 1 RX + */ +void linkmode_resolve_pause(const unsigned long *local_adv, + const unsigned long *partner_adv, + bool *tx_pause, bool *rx_pause) +{ + __ETHTOOL_DECLARE_LINK_MODE_MASK(m); + + linkmode_and(m, local_adv, partner_adv); + if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT, m)) { + *tx_pause = true; + *rx_pause = true; + } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, m)) { + *tx_pause = linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT, + partner_adv); + *rx_pause = linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT, + local_adv); + } else { + *tx_pause = false; + *rx_pause = false; + } +} +EXPORT_SYMBOL_GPL(linkmode_resolve_pause); diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 6a5056e0ae77..f5a7a077ec1f 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -2409,6 +2409,32 @@ bool phy_validate_pause(struct phy_device *phydev, } EXPORT_SYMBOL(phy_validate_pause); +/** + * phy_get_pause - resolve negotiated pause modes + * @phydev: phy_device struct + * @tx_pause: pointer to bool to indicate whether transmit pause should be + * enabled. + * @rx_pause: pointer to bool to indicate whether receive pause should be + * enabled. + * + * Resolve and return the flow control modes according to the negotiation + * result. This includes checking that we are operating in full duplex mode. + * See linkmode_resolve_pause() for further details. + */ +void phy_get_pause(struct phy_device *phydev, bool *tx_pause, bool *rx_pause) +{ + if (phydev->duplex != DUPLEX_FULL) { + *tx_pause = false; + *rx_pause = false; + return; + } + + return linkmode_resolve_pause(phydev->advertising, + phydev->lp_advertising, + tx_pause, rx_pause); +} +EXPORT_SYMBOL(phy_get_pause); + static bool phy_drv_supports_irq(struct phy_driver *phydrv) { return phydrv->config_intr && phydrv->ack_interrupt; diff --git a/include/linux/linkmode.h b/include/linux/linkmode.h index 8e5b352e44f2..9ec210f31d06 100644 --- a/include/linux/linkmode.h +++ b/include/linux/linkmode.h @@ -88,4 +88,8 @@ static inline int linkmode_subset(const unsigned long *src1, return bitmap_subset(src1, src2, __ETHTOOL_LINK_MODE_MASK_NBITS); } +void linkmode_resolve_pause(const unsigned long *local_adv, + const unsigned long *partner_adv, + bool *tx_pause, bool *rx_pause); + #endif /* __LINKMODE_H */ diff --git a/include/linux/phy.h b/include/linux/phy.h index c570e162e05e..80f8b2158271 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -1257,6 +1257,9 @@ void phy_set_sym_pause(struct phy_device *phydev, bool rx, bool tx, void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx); bool phy_validate_pause(struct phy_device *phydev, struct ethtool_pauseparam *pp); +void phy_get_pause(struct phy_device *phydev, bool *tx_pause, bool *rx_pause); +void phy_resolve_pause(unsigned long *local_adv, unsigned long *partner_adv, + bool *tx_pause, bool *rx_pause); int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask, int (*run)(struct phy_device *)); -- cgit v1.2.3 From 45c767faef151899ac1a5e14a59c0e0a5bdba27b Mon Sep 17 00:00:00 2001 From: Russell King Date: Sat, 15 Feb 2020 15:49:32 +0000 Subject: net: add linkmode helper for setting flow control advertisement Add a linkmode helper to set the flow control advertisement in an ethtool linkmode mask according to the tx/rx capabilities. This implementation is moved from phylib, and documented with an analysis of its shortcomings. Signed-off-by: Russell King Reviewed-by: Andrew Lunn Signed-off-by: David S. Miller --- drivers/net/phy/linkmode.c | 51 ++++++++++++++++++++++++++++++++++++++++++++ drivers/net/phy/phy_device.c | 17 +-------------- include/linux/linkmode.h | 2 ++ 3 files changed, 54 insertions(+), 16 deletions(-) (limited to 'include/linux') diff --git a/drivers/net/phy/linkmode.c b/drivers/net/phy/linkmode.c index 969918795228..f60560fe3499 100644 --- a/drivers/net/phy/linkmode.c +++ b/drivers/net/phy/linkmode.c @@ -42,3 +42,54 @@ void linkmode_resolve_pause(const unsigned long *local_adv, } } EXPORT_SYMBOL_GPL(linkmode_resolve_pause); + +/** + * linkmode_set_pause - set the pause mode advertisement + * @advertisement: advertisement in ethtool format + * @tx: boolean from ethtool struct ethtool_pauseparam tx_pause member + * @rx: boolean from ethtool struct ethtool_pauseparam rx_pause member + * + * Configure the advertised Pause and Asym_Pause bits according to the + * capabilities of provided in @tx and @rx. + * + * We convert as follows: + * tx rx Pause AsymDir + * 0 0 0 0 + * 0 1 1 1 + * 1 0 0 1 + * 1 1 1 0 + * + * Note: this translation from ethtool tx/rx notation to the advertisement + * is actually very problematical. Here are some examples: + * + * For tx=0 rx=1, meaning transmit is unsupported, receive is supported: + * + * Local device Link partner + * Pause AsymDir Pause AsymDir Result + * 1 1 1 0 TX + RX - but we have no TX support. + * 1 1 0 1 Only this gives RX only + * + * For tx=1 rx=1, meaning we have the capability to transmit and receive + * pause frames: + * + * Local device Link partner + * Pause AsymDir Pause AsymDir Result + * 1 0 0 1 Disabled - but since we do support tx and rx, + * this should resolve to RX only. + * + * Hence, asking for: + * rx=1 tx=0 gives Pause+AsymDir advertisement, but we may end up + * resolving to tx+rx pause or only rx pause depending on + * the partners advertisement. + * rx=0 tx=1 gives AsymDir only, which will only give tx pause if + * the partners advertisement allows it. + * rx=1 tx=1 gives Pause only, which will only allow tx+rx pause + * if the other end also advertises Pause. + */ +void linkmode_set_pause(unsigned long *advertisement, bool tx, bool rx) +{ + linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT, advertisement, rx); + linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, advertisement, + rx ^ tx); +} +EXPORT_SYMBOL_GPL(linkmode_set_pause); diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index f5a7a077ec1f..2a973265de80 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -2361,22 +2361,7 @@ void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx) __ETHTOOL_DECLARE_LINK_MODE_MASK(oldadv); linkmode_copy(oldadv, phydev->advertising); - - linkmode_clear_bit(ETHTOOL_LINK_MODE_Pause_BIT, - phydev->advertising); - linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, - phydev->advertising); - - if (rx) { - linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, - phydev->advertising); - linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, - phydev->advertising); - } - - if (tx) - linkmode_change_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, - phydev->advertising); + linkmode_set_pause(phydev->advertising, tx, rx); if (!linkmode_equal(oldadv, phydev->advertising) && phydev->autoneg) diff --git a/include/linux/linkmode.h b/include/linux/linkmode.h index 9ec210f31d06..c664c27a29a0 100644 --- a/include/linux/linkmode.h +++ b/include/linux/linkmode.h @@ -92,4 +92,6 @@ void linkmode_resolve_pause(const unsigned long *local_adv, const unsigned long *partner_adv, bool *tx_pause, bool *rx_pause); +void linkmode_set_pause(unsigned long *advertisement, bool tx, bool rx); + #endif /* __LINKMODE_H */ -- cgit v1.2.3 From 4e5aeb4157c879a021e6d92373dc7e4684ebd8c0 Mon Sep 17 00:00:00 2001 From: Russell King Date: Sat, 15 Feb 2020 15:49:53 +0000 Subject: net: phylink: resolve fixed link flow control Resolve the fixed link flow control using the recently introduced linkmode_resolve_pause() helper, which we use in phylink_get_fixed_state() only when operating in full duplex mode. Signed-off-by: Russell King Reviewed-by: Andrew Lunn Signed-off-by: David S. Miller --- drivers/net/phy/phylink.c | 70 +++++++++++++++++++++-------------------------- include/linux/phylink.h | 8 ++---- 2 files changed, 34 insertions(+), 44 deletions(-) (limited to 'include/linux') diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index e65e9c9dc759..c29648b90ce7 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -181,9 +181,11 @@ static int phylink_parse_fixedlink(struct phylink *pl, /* We treat the "pause" and "asym-pause" terminology as * defining the link partner's ability. */ if (fwnode_property_read_bool(fixed_node, "pause")) - pl->link_config.pause |= MLO_PAUSE_SYM; + __set_bit(ETHTOOL_LINK_MODE_Pause_BIT, + pl->link_config.lp_advertising); if (fwnode_property_read_bool(fixed_node, "asym-pause")) - pl->link_config.pause |= MLO_PAUSE_ASYM; + __set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, + pl->link_config.lp_advertising); if (ret == 0) { desc = fwnode_gpiod_get_index(fixed_node, "link", 0, @@ -215,9 +217,11 @@ static int phylink_parse_fixedlink(struct phylink *pl, DUPLEX_FULL : DUPLEX_HALF; pl->link_config.speed = prop[2]; if (prop[3]) - pl->link_config.pause |= MLO_PAUSE_SYM; + __set_bit(ETHTOOL_LINK_MODE_Pause_BIT, + pl->link_config.lp_advertising); if (prop[4]) - pl->link_config.pause |= MLO_PAUSE_ASYM; + __set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, + pl->link_config.lp_advertising); } } @@ -351,6 +355,22 @@ static void phylink_apply_manual_flow(struct phylink *pl, state->pause = pl->link_config.pause; } +static void phylink_resolve_flow(struct phylink_link_state *state) +{ + bool tx_pause, rx_pause; + + state->pause = MLO_PAUSE_NONE; + if (state->duplex == DUPLEX_FULL) { + linkmode_resolve_pause(state->advertising, + state->lp_advertising, + &tx_pause, &rx_pause); + if (tx_pause) + state->pause |= MLO_PAUSE_TX; + if (rx_pause) + state->pause |= MLO_PAUSE_RX; + } +} + static void phylink_mac_config(struct phylink *pl, const struct phylink_link_state *state) { @@ -399,44 +419,16 @@ static void phylink_mac_pcs_get_state(struct phylink *pl, /* The fixed state is... fixed except for the link state, * which may be determined by a GPIO or a callback. */ -static void phylink_get_fixed_state(struct phylink *pl, struct phylink_link_state *state) +static void phylink_get_fixed_state(struct phylink *pl, + struct phylink_link_state *state) { *state = pl->link_config; if (pl->get_fixed_state) pl->get_fixed_state(pl->netdev, state); else if (pl->link_gpio) state->link = !!gpiod_get_value_cansleep(pl->link_gpio); -} -/* Flow control is resolved according to our and the link partners - * advertisements using the following drawn from the 802.3 specs: - * Local device Link partner - * Pause AsymDir Pause AsymDir Result - * 1 X 1 X TX+RX - * 0 1 1 1 TX - * 1 1 0 1 RX - */ -static void phylink_resolve_flow(struct phylink *pl, - struct phylink_link_state *state) -{ - int new_pause = 0; - int pause = 0; - - if (phylink_test(pl->link_config.advertising, Pause)) - pause |= MLO_PAUSE_SYM; - if (phylink_test(pl->link_config.advertising, Asym_Pause)) - pause |= MLO_PAUSE_ASYM; - - pause &= state->pause; - - if (pause & MLO_PAUSE_SYM) - new_pause = MLO_PAUSE_TX | MLO_PAUSE_RX; - else if (pause & MLO_PAUSE_ASYM) - new_pause = state->pause & MLO_PAUSE_SYM ? - MLO_PAUSE_TX : MLO_PAUSE_RX; - - state->pause &= ~MLO_PAUSE_TXRX_MASK; - state->pause |= new_pause; + phylink_resolve_flow(state); } static const char *phylink_pause_to_str(int pause) @@ -1393,8 +1385,7 @@ int phylink_ethtool_set_pauseparam(struct phylink *pl, !pause->autoneg && pause->rx_pause != pause->tx_pause) return -EINVAL; - config->pause &= ~(MLO_PAUSE_AN | MLO_PAUSE_TXRX_MASK); - + config->pause = 0; if (pause->autoneg) config->pause |= MLO_PAUSE_AN; if (pause->rx_pause) @@ -1505,13 +1496,14 @@ static int phylink_mii_emul_read(unsigned int reg, struct phylink_link_state *state) { struct fixed_phy_status fs; + unsigned long *lpa = state->lp_advertising; int val; fs.link = state->link; fs.speed = state->speed; fs.duplex = state->duplex; - fs.pause = state->pause & MLO_PAUSE_SYM; - fs.asym_pause = state->pause & MLO_PAUSE_ASYM; + fs.pause = test_bit(ETHTOOL_LINK_MODE_Pause_BIT, lpa); + fs.asym_pause = test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, lpa); val = swphy_read_reg(reg, &fs); if (reg == MII_BMSR) { diff --git a/include/linux/phylink.h b/include/linux/phylink.h index 523209e70947..0d6073c2b2b7 100644 --- a/include/linux/phylink.h +++ b/include/linux/phylink.h @@ -12,12 +12,10 @@ struct net_device; enum { MLO_PAUSE_NONE, - MLO_PAUSE_ASYM = BIT(0), - MLO_PAUSE_SYM = BIT(1), - MLO_PAUSE_RX = BIT(2), - MLO_PAUSE_TX = BIT(3), + MLO_PAUSE_RX = BIT(0), + MLO_PAUSE_TX = BIT(1), MLO_PAUSE_TXRX_MASK = MLO_PAUSE_TX | MLO_PAUSE_RX, - MLO_PAUSE_AN = BIT(4), + MLO_PAUSE_AN = BIT(2), MLO_AN_PHY = 0, /* Conventional PHY */ MLO_AN_FIXED, /* Fixed-link mode */ -- cgit v1.2.3 From b70486f94bb4820e84491089da5e30d29e774b0d Mon Sep 17 00:00:00 2001 From: Russell King Date: Sat, 15 Feb 2020 15:50:09 +0000 Subject: net: phylink: clarify flow control settings in documentation Clarify the expected flow control settings operation in the phylink documentation for each negotiation mode. Signed-off-by: Russell King Reviewed-by: Andrew Lunn Signed-off-by: David S. Miller --- include/linux/phylink.h | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) (limited to 'include/linux') diff --git a/include/linux/phylink.h b/include/linux/phylink.h index 0d6073c2b2b7..812357c03df4 100644 --- a/include/linux/phylink.h +++ b/include/linux/phylink.h @@ -152,13 +152,20 @@ void mac_pcs_get_state(struct phylink_config *config, * guaranteed to be correct, and so any mac_config() implementation must * never reference these fields. * + * In all negotiation modes, as defined by @mode, @state->pause indicates the + * pause settings which should be applied as follows. If %MLO_PAUSE_AN is not + * set, %MLO_PAUSE_TX and %MLO_PAUSE_RX indicate whether the MAC should send + * pause frames and/or act on received pause frames respectively. Otherwise, + * the results of in-band negotiation/status from the MAC PCS should be used + * to control the MAC pause mode settings. + * * The action performed depends on the currently selected mode: * * %MLO_AN_FIXED, %MLO_AN_PHY: - * Configure the specified @state->speed, @state->duplex and - * @state->pause (%MLO_PAUSE_TX / %MLO_PAUSE_RX) modes over a link - * specified by @state->interface. @state->advertising may be used, - * but is not required. Other members of @state must be ignored. + * Configure the specified @state->speed and @state->duplex over a link + * specified by @state->interface. @state->advertising may be used, but + * is not required. Pause modes as above. Other members of @state must + * be ignored. * * Valid state members: interface, speed, duplex, pause, advertising. * @@ -170,11 +177,14 @@ void mac_pcs_get_state(struct phylink_config *config, * mac_pcs_get_state() callback. Changes in link state must be made * by calling phylink_mac_change(). * + * Interface mode specific details are mentioned below. + * * If in 802.3z mode, the link speed is fixed, dependent on the - * @state->interface. Duplex is negotiated, and pause is advertised - * according to @state->an_enabled, @state->pause and - * @state->advertising flags. Beware of MACs which only support full - * duplex at gigabit and higher speeds. + * @state->interface. Duplex and pause modes are negotiated via + * the in-band configuration word. Advertised pause modes are set + * according to the @state->an_enabled and @state->advertising + * flags. Beware of MACs which only support full duplex at gigabit + * and higher speeds. * * If in Cisco SGMII mode, the link speed and duplex mode are passed * in the serial bitstream 16-bit configuration word, and the MAC -- cgit v1.2.3