Re: [PATCH v4 net-next 5/5] net: pcs: pcs-mtk-lynxi: deprecate "mediatek,pnswap"

[Vladimir Oltean <vladimir.oltean@nxp.com>]

[-- Attachment #1: Type: text/plain, Size: 2089 bytes --]

Hi Frank,

On Mon, Mar 30, 2026 at 05:52:17PM +0000, Frank Wunderlich wrote:
> Hi Vladimir
> 
> Thanks for the patch and sorry for my delay...i was away this weekend so i was not able to test.
> 
> traffic works again (but there is only read now) and this is the result of your debug prints:
> 
> root@bpi-r3:~# dmesg | grep SGMSYS_QPHY_WRAP_CTRL
> [    2.706963] SGMSYS_QPHY_WRAP_CTRL = 0x501, intending to write 0x500
> [    9.134081] SGMSYS_QPHY_WRAP_CTRL = 0x500, intending to write 0x500
> 
> R3/mt7986 has 2 MAC, and switch is on the first, so value will change, not sure why this is different.
> 
> i have not found SGMSYS_QPHY_WRAP_CTRL or something related with polarity in ethernet/mac-
> (drivers/net/ethernet/mediatek/mtk_eth_soc.c) or switch-driver (drivers/net/dsa/mt7530{,-mdio}.c)
> in case they manipulate this register too (of course they should not). Also looked into the pcs-handling
> in both drivers, but see nothing related to polarity. And looked for possible duplicate register const
> definition (other name for 0xec).

This result means that your default QPHY_WRAP_CTRL register value has
the SGMII_PN_SWAP_TX bit set. Whether that comes from U-Boot or hardware
default or otherwise, it doesn't really matter. Curious that the
SGMII_SW_RESET doesn't clear TX inversion, though. I guess you wouldn't
have documentation that would suggest this setting is sticky?

In Documentation/devicetree/bindings/net/pcs/mediatek,sgmiisys.yaml,
it is not specified what happens when the "mediatek,pnswap" property is
missing. I thought the most logical thing would be for the lane
polarities to not be swapped - because how would you describe normal
lane polarities otherwise? My bad for thinking the original vendor
bindings were more sane than they were.

The only way to describe the polarities that this SGMSYS block needs on
a particular board is to use the newly introduced 'rx-polarity =
<PHY_POL_NORMAL>' and 'tx-polarity = <PHY_POL_INVERT>'. Which I strongly
recommend you to do, even if the attached patch should restore
functionality with your current device tree.

[-- Attachment #2: 0001-net-pcs-pcs-mtk-lynxi-preserve-lane-polarities-when-.patch --]
[-- Type: text/x-diff, Size: 3080 bytes --]

From 6b5fe06ec16e8a1e752fc871c135d2f12a37ce33 Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Thu, 26 Mar 2026 23:46:46 +0200
Subject: [PATCH] net: pcs: pcs-mtk-lynxi: preserve lane polarities when not
 described in fwnode

Frank Wunderlich reports that the BPI-R3 board has a default
SGMSYS_QPHY_WRAP_CTRL = 0x501, aka TX inverted and RX not inverted.
At the same time, neither rx-polarity/tx-polarity nor mediatek,pnswap
are present in the device tree.

The original driver logic was to enable both TX inversion and RX
inversion when finding mediatek,pnswap in the device tree, and leave
SGMSYS_QPHY_WRAP_CTRL to its default value otherwise.

The blamed commit has broken that by assuming that a missing
mediatek,pnswap would mean non-inverting polarity.

Restore the original behaviour by reading the SGMSYS_QPHY_WRAP_CTRL
value and using it as a default polarity if mediatek,pnswap and the new
rx-polarity/tx-polarity are all unset.

Fixes: 8871389da151 ("net: pcs: pcs-mtk-lynxi: deprecate "mediatek,pnswap"")
Reported-by: Frank Wunderlich <frank.wunderlich@linux.dev>
Closes: https://lore.kernel.org/netdev/e0ad52862d34cf4e0169c9850a7f164f127d0093@linux.dev/
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/pcs/pcs-mtk-lynxi.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/net/pcs/pcs-mtk-lynxi.c b/drivers/net/pcs/pcs-mtk-lynxi.c
index c12f8087af9b..7518c98fa98a 100644
--- a/drivers/net/pcs/pcs-mtk-lynxi.c
+++ b/drivers/net/pcs/pcs-mtk-lynxi.c
@@ -126,11 +126,24 @@ static int mtk_pcs_config_polarity(struct mtk_pcs_lynxi *mpcs,
 {
 	struct fwnode_handle *fwnode = mpcs->fwnode, *pcs_fwnode;
 	unsigned int pol, default_pol = PHY_POL_NORMAL;
-	unsigned int val = 0;
+	unsigned int val = 0, orig;
+	bool has_legacy_prop;
 	int ret;
 
-	if (fwnode_property_read_bool(fwnode, "mediatek,pnswap"))
+	ret = regmap_read(mpcs->regmap, SGMSYS_QPHY_WRAP_CTRL, &orig);
+	if (ret)
+		return ret;
+
+	/* RX polarity:
+	 * - if standard 'rx-polarity' exists in 'pcs' subnode, follow that
+	 * - if 'mediatek,pnswap' is set, invert RX polarity
+	 * - otherwise, leave unchanged
+	 */
+	has_legacy_prop = fwnode_property_read_bool(fwnode, "mediatek,pnswap");
+	if (has_legacy_prop || FIELD_GET(SGMII_PN_SWAP_RX, orig))
 		default_pol = PHY_POL_INVERT;
+	else
+		default_pol = PHY_POL_NORMAL;
 
 	pcs_fwnode = fwnode_get_named_child_node(fwnode, "pcs");
 
@@ -144,6 +157,16 @@ static int mtk_pcs_config_polarity(struct mtk_pcs_lynxi *mpcs,
 	if (pol == PHY_POL_INVERT)
 		val |= SGMII_PN_SWAP_RX;
 
+	/* And TX polarity:
+	 * - if standard 'tx-polarity' exists in 'pcs' subnode, follow that
+	 * - if 'mediatek,pnswap' is set, invert TX polarity
+	 * - otherwise, leave unchanged
+	 */
+	if (has_legacy_prop || FIELD_GET(SGMII_PN_SWAP_TX, orig))
+		default_pol = PHY_POL_INVERT;
+	else
+		default_pol = PHY_POL_NORMAL;
+
 	ret = phy_get_tx_polarity(pcs_fwnode, phy_modes(interface),
 				  BIT(PHY_POL_NORMAL) | BIT(PHY_POL_INVERT),
 				  default_pol, &pol);
-- 
2.43.0