Re: [PATCH net-next v6 2/7] net: phy: phylink: add helper to modify pause
From: Andrew Lunn <andrew@lunn.ch>
Date: 2026-07-10 17:02:37
Also in:
lkml
On Fri, Jul 10, 2026 at 12:05:06PM +0200, Maxime Chevallier wrote:
Hi, On 7/9/26 12:02, javen wrote:quoted
From: Javen Xu <redacted> For Realtek nics, when we enable jumbo, pause are not supported. So we must check the pause capabilities from ourself and lp. Signed-off-by: Javen Xu <redacted> --- Changes in v5: - no changes, new file Changes in v6: - rename phylink_update_mac_pause_capabilities(), this function only changes mac pause capability - set asym pause and pause according to config->pause tx and rx - add phydev->lock when change pl->phydev->advertising --- drivers/net/phy/phylink.c | 87 +++++++++++++++++++++++++++++++++++++++ include/linux/phylink.h | 2 + 2 files changed, 89 insertions(+)diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index 59dfe35afa54..9e9cd79301d6 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c@@ -1828,6 +1828,93 @@ int phylink_set_fixed_link(struct phylink *pl, } EXPORT_SYMBOL_GPL(phylink_set_fixed_link); +/** + * phylink_update_mac_pause_capabilities() - Dynamically update MAC pause + * @pl: a pointer to a &struct phylink returned from phylink_create() + * @mac_pause: the new MAC pause capabilities mask + * + * This function allows a MAC driver to dynamically change its pause state, + * such as losing/gaining Pause frame support based on MTU size. + * It recalculates supported link modes and triggers renegotiation if needed. + */ +void phylink_update_mac_pause_capabilities(struct phylink *pl, unsigned long mac_pause) +{ + struct phylink_link_state *config = &pl->link_config; + unsigned long old_pause, caps_added, caps_removed; + bool pause_adv, asym_adv; + + ASSERT_RTNL(); + + if (mac_pause & ~(MAC_SYM_PAUSE | MAC_ASYM_PAUSE)) { + phylink_err(pl, "Attempted to dynamically change non-pause MAC capabilities\n"); + return; + } + + old_pause = pl->config->mac_capabilities & (MAC_SYM_PAUSE | MAC_ASYM_PAUSE); + caps_added = mac_pause & ~old_pause; + caps_removed = old_pause & ~mac_pause; + + if (!caps_added && !caps_removed) + return; + + mutex_lock(&pl->state_mutex); + + pl->config->mac_capabilities &= ~(MAC_SYM_PAUSE | MAC_ASYM_PAUSE); + pl->config->mac_capabilities |= mac_pause; + + if (caps_removed & MAC_SYM_PAUSE) + linkmode_clear_bit(ETHTOOL_LINK_MODE_Pause_BIT, pl->supported); + if (caps_removed & MAC_ASYM_PAUSE) + linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, pl->supported); + + linkmode_and(config->advertising, config->advertising, pl->supported); + + if (caps_added & MAC_SYM_PAUSE) { + linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, pl->supported); + if (pl->phydev && !phylink_test(pl->phydev->supported, Pause)) + linkmode_clear_bit(ETHTOOL_LINK_MODE_Pause_BIT, pl->supported);Why look at what the PHY can do here ? You shouldn't need to.
I question the whole idea here. Why is this so complex? phylink should already have all the needed code, once you change pl->config->mac_capabilities you just need to call that code. Yes, it might need refactoring into a helper, but that would be a preparatory patch. Once you start reusing existing code, i then wounder if it is just as simple to allow any of the mac_capabilities to be changed, not just pause? Andrew