Re: [PATCH net-next v3 10/47] net: phylink: Adjust link settings based on rate adaptation
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
Date: 2022-07-18 16:14:54
Also in:
lkml, netdev
On Sat, Jul 16, 2022 at 06:37:22PM -0400, Sean Anderson wrote:
On 7/16/22 4:17 PM, Andrew Lunn wrote:quoted
On Fri, Jul 15, 2022 at 05:59:17PM -0400, Sean Anderson wrote:quoted
If the phy is configured to use pause-based rate adaptation, ensure that the link is full duplex with pause frame reception enabled. Note that these settings may be overridden by ethtool. Signed-off-by: Sean Anderson <redacted> --- Changes in v3: - New drivers/net/phy/phylink.c | 4 ++++ 1 file changed, 4 insertions(+)diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index 7fa21941878e..7f65413aa778 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c@@ -1445,6 +1445,10 @@ static void phylink_phy_change(struct phy_device *phydev, bool up) pl->phy_state.speed = phy_interface_speed(phydev->interface, phydev->speed); pl->phy_state.duplex = phydev->duplex; + if (phydev->rate_adaptation == RATE_ADAPT_PAUSE) { + pl->phy_state.duplex = DUPLEX_FULL; + rx_pause = true; + }I would not do this. If the requirements for rate adaptation are not fulfilled, you should turn off rate adaptation. A MAC which knows rate adaptation is going on can help out, by not advertising 10Half, 100Half etc. Autoneg will then fail for modes where rate adaptation does not work.OK, so maybe it is better to phylink_warn here. Something along the lines of "phy using pause-based rate adaptation, but duplex is %s".quoted
The MAC should also be declaring what sort of pause it supports, so disable rate adaptation if it does not have async pause.That's what we do in the previous patch. The problem is that rx_pause and tx_pause are resolved based on our advertisement and the link partner's advertisement. However, the link partner may not support pause frames at all. In that case, we will get rx_pause and tx_pause as false. However, we still want to enable rx_pause, because we know that the phy will be emitting pause frames. And of course the user can always force disable pause frames anyway through ethtool.
If you want the MAC to enable rx_pause, that ought to be handled separately in the mac_link_up() method, IMHO. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel