Re: [PATCH net-next v8 2/6] net: pcs: xpcs: re-initiate clause 37 Auto-negotiation
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
Date: 2025-02-26 15:40:54
Also in:
lkml, netdev, platform-driver-x86
On Wed, Feb 26, 2025 at 03:48:33PM +0800, Choong Yong Liang wrote:
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/pcs/pcs-xpcs-wx.c b/drivers/net/pcs/pcs-xpcs-wx.c index fc52f7aa5f59..f73ab04d09f0 100644 --- a/drivers/net/pcs/pcs-xpcs-wx.c +++ b/drivers/net/pcs/pcs-xpcs-wx.c@@ -172,11 +172,9 @@ int txgbe_xpcs_switch_mode(struct dw_xpcs *xpcs, phy_interface_t interface) return 0; } - if (xpcs->interface == interface && !txgbe_xpcs_mode_quirk(xpcs)) + if (!txgbe_xpcs_mode_quirk(xpcs)) return 0; - xpcs->interface = interface; -
...
quoted hunk ↗ jump to hunk
--- a/drivers/net/pcs/pcs-xpcs.c +++ b/drivers/net/pcs/pcs-xpcs.c@@ -602,12 +602,37 @@ static void xpcs_get_interfaces(struct dw_xpcs *xpcs, unsigned long *interfaces) __set_bit(compat->interface, interfaces); } +static int xpcs_switch_interface_mode(struct dw_xpcs *xpcs, + phy_interface_t interface) +{ + int ret = 0; + + if (xpcs->interface != interface) { + if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID) { + ret = txgbe_xpcs_switch_mode(xpcs, interface); + if (ret) + return ret;
The above modification changes the functionality. In the old code, txgbe_xpcs_switch_mode() does its work when xpcs->interface is not the same as interface OR txgbe_xpcs_mode_quirk() is true. Your replacement code calls txgbe_xpcs_switch_mode() when xpcs->interface is not the same as interface, *and* it can do its work when txgbe_xpcs_mode_quirk() returns true. So, e.g. when txgbe_xpcs_mode_quirk() returns false, but the interface changes, txgbe_xpcs_mode_quirk() used to do its work, but as a result if your changes, it becomes a no-op. The point of txgbe_xpcs_mode_quirk() is to always do the work if it returns true, even if the interface mode doesn't change. Therefore, this patch is logically incorrect, and likely breaks TXGBE. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!