On 26/2/2025 11:40 pm, Russell King (Oracle) wrote:
On Wed, Feb 26, 2025 at 03:48:33PM +0800, Choong Yong Liang wrote:
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;
-
...
--- 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.
Thank you for pointing out the oversight. I will review the logic and make
the necessary corrections to ensure the patch does not disrupt the expected
behavior.
