[PATCH] net: dpaa: fman_memac: accept phy-interface-type = "10gbase-r" in the device tree

[Vladimir Oltean]

We support the phy-interface-mode = "xgmii" conversion to "10gbase-r"
through code, but not actually through the device tree proper. This is
because boards such as LS1046A-RDB do not define pcs-handle-names in the
ethernet@f0000 device tree node, and the code only has fallback xfi_pcs
determination logic for "xgmii".

By reversing the order between the fallback xfi_pcs assignment and the
"xgmii" overwrite with "10gbase-r", we are able to support both values
in the device tree, with identical behavior.

Signed-off-by: Vladimir Oltean <vladimir.oltean@xxxxxxx>
---
 .../net/ethernet/freescale/fman/fman_memac.c   | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c b/drivers/net/ethernet/freescale/fman/fman_memac.c
index e30bf75b1d48..0996759907a8 100644
--- a/drivers/net/ethernet/freescale/fman/fman_memac.c
+++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
@@ -1076,6 +1076,14 @@ int memac_initialization(struct mac_device *mac_dev,
 	unsigned long		 capabilities;
 	unsigned long		*supported;
 
+	/* The internal connection to the serdes is XGMII, but this isn't
+	 * really correct for the phy mode (which is the external connection).
+	 * However, this is how all older device trees say that they want
+	 * 10GBASE-R (aka XFI), so just convert it for them.
+	 */
+	if (mac_dev->phy_if == PHY_INTERFACE_MODE_XGMII)
+		mac_dev->phy_if = PHY_INTERFACE_MODE_10GBASER;
+
 	mac_dev->phylink_ops		= &memac_mac_ops;
 	mac_dev->set_promisc		= memac_set_promiscuous;
 	mac_dev->change_addr		= memac_modify_mac_address;
@@ -1142,7 +1150,7 @@ int memac_initialization(struct mac_device *mac_dev,
 	 * (and therefore that xfi_pcs cannot be set). If we are defaulting to
 	 * XGMII, assume this is for XFI. Otherwise, assume it is for SGMII.
 	 */
-	if (err && mac_dev->phy_if == PHY_INTERFACE_MODE_XGMII)
+	if (err && mac_dev->phy_if == PHY_INTERFACE_MODE_10GBASER)
 		memac->xfi_pcs = pcs;
 	else
 		memac->sgmii_pcs = pcs;
@@ -1156,14 +1164,6 @@ int memac_initialization(struct mac_device *mac_dev,
 		goto _return_fm_mac_free;
 	}
 
-	/* The internal connection to the serdes is XGMII, but this isn't
-	 * really correct for the phy mode (which is the external connection).
-	 * However, this is how all older device trees say that they want
-	 * 10GBASE-R (aka XFI), so just convert it for them.
-	 */
-	if (mac_dev->phy_if == PHY_INTERFACE_MODE_XGMII)
-		mac_dev->phy_if = PHY_INTERFACE_MODE_10GBASER;
-
 	/* TODO: The following interface modes are supported by (some) hardware
 	 * but not by this driver:
 	 * - 1000BASE-KX

But!

Device tree is stable ABI, and changes made to the device tree file are
meant to be backwards and forwards compatible with the code (it can be
provided separately and not necessarily in lockstep with the kernel
version. For example, I understand Arm SystemReady IR wants U-Boot to
provide its own device tree to Linux through EFI). So, in general,
device tree changes which only work with a corresponding kernel change
are frowned upon (unless maybe if the kernel change is a bug fix that is
backported to all relevant stable kernel branches).

So at this stage we should take a step back and re-analyze the cost/benefit.
You said there is a stack trace in the Aquantia PHY driver, which there
is not (in current mainline kernels). I _think_ you are seeing the stack
trace with LSDK, which is currently distributed on top of linux-6.1.y
and has not yet integrated the fman_memac conversion to phylink - thus,
it does not contain commit 5d93cfcf7360 ("net: dpaa: Convert to phylink").
At least, I do see this stack trace there. I think it can also be seen
with mainline kernels before the phylink conversion, but I did not test
those.

The main take-away is: ALWAYS test the patch you are submitting to the
target kernel it is going to be applied to. Especially in the area of
device tree bindings for DPAA1, things are rarely as simple as they
appear :) If you don't, you will have an unintended negative effect
upon current mainline kernels (which must still work), and not the
intended effect upon LSDK (more below).

There are a few options to go forward from here.

As there is nothing broken in the mainline kernel where you are
submitting this patch, the simplest one, as bland as it may sound, is
simply to wait for a new LSDK release on top of linux-6.6.y. Even in
lf-6.1.y, AFAICS, nothing is broken except for the stack trace. You can
keep the patch in your local kernel tree copy to suppress that.

The other option would be to submit the fman_memac change as a bug fix
for stable, wait for a while for it to have time to propagate, then
modify the device tree as well. But, it is much higher effort, and
there is no procedure in place, AFAIK, for LSDK to integrate your patch
(other than through upstream + a few months of waiting). The upcoming
LSDK release will be on top of linux-6.6.y, it will make your effort
irrelevant if it's only for suppressing the stack trace, and you are
racing against it. This path is only worth it if you have the dedication
to dig a bit deeper and tidy things up in the DPAA1 kernel support
(which would be appreciated though).

_If_ you are using an older linux-stable branch but not LSDK, yes, the
feedback loop between your patch and its effect will close much sooner,
but you will have to convince the linux-stable people that it's worth
accepting your driver rework patches for a functional reason and not
just aesthetic (see Documentation/process/stable-kernel-rules.rst for
reference).