[PATCH net-next v2 0/3] net: dsa: mxl-gsw1xx: setup polarities and validate chip

[PATCH net-next v2 0/3] net: dsa: mxl-gsw1xx: setup polarities and validate chip

[Daniel Golle]

Now that common PHY properties make it easy to configure the SerDes RX
and TX polarities, use that for the SGMII/1000Base-X/2500Base-X PCS of
the MaxLinear GSW1xx switches.

Also, validate hardware in probe() function to make sure the switch is
actually present and MDIO communication works properly.
---
Changes since initial submission:
 * use allOf to include phy-common-props in dt-schema
 * use phy_get_manual_rx_polarity and phy_get_manual_tx_polarity helpers
   instead of open-coding them

Daniel Golle (4):
  net: dsa: mxl-gsw1xx: setup polarities and validate chip
  dt-bindings: net: dsa: lantiq,gswip: reference common PHY properties
  net: dsa: mxl-gsw1xx: configure PCS polarities
  net: dsa: mxl-gsw1xx: validate chip ID

 .../bindings/net/dsa/lantiq,gswip.yaml        |  4 ++
 drivers/net/dsa/lantiq/Kconfig                |  1 +
 drivers/net/dsa/lantiq/mxl-gsw1xx.c           | 61 +++++++++++++++----
 drivers/net/dsa/lantiq/mxl-gsw1xx.h           |  9 +++
 4 files changed, 64 insertions(+), 11 deletions(-)

-- 
2.52.0

[PATCH net-next v2 1/3] dt-bindings: net: dsa: lantiq,gswip: reference common PHY properties

[Daniel Golle]

Reference the common PHY properties so RX and TX SerDes lane polarity
of the SGMII/1000Base-X/2500Base-X PCS can be configured.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: use allOf to include PHY common properties, add example use

 Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml b/Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml
index f601e5f9fa6a..b4a31cde4322 100644
--- a/Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml
@@ -105,6 +105,8 @@ patternProperties:
     patternProperties:
       "^(ethernet-)?port@[0-6]$":
         $ref: dsa-port.yaml#
+        allOf:
+          - $ref: /schemas/phy/phy-common-props.yaml#
         unevaluatedProperties: false
 
         properties:
@@ -288,6 +290,7 @@ examples:
 
   - |
     #include <dt-bindings/leds/common.h>
+    #include <dt-bindings/phy/phy.h>
 
     mdio {
         #address-cells = <1>;
@@ -320,6 +323,7 @@ examples:
                     label = "wan";
                     phy-mode = "1000base-x";
                     managed = "in-band-status";
+                    rx-polarity = <PHY_POL_INVERT>;
                 };
 
                 port@5 {
-- 
2.52.0

[PATCH net-next v2 2/3] net: dsa: mxl-gsw1xx: configure PCS polarities

[Daniel Golle]

Configure SerDes PCS RX and TX polarities using the newly
introduced generic properties.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: use phy_get_manual_rx_polarity and phy_get_manual_tx_polarity

 drivers/net/dsa/lantiq/Kconfig      |  1 +
 drivers/net/dsa/lantiq/mxl-gsw1xx.c | 34 ++++++++++++++++++++---------
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/net/dsa/lantiq/Kconfig b/drivers/net/dsa/lantiq/Kconfig
index bad13817af25..98efeef2661b 100644
--- a/drivers/net/dsa/lantiq/Kconfig
+++ b/drivers/net/dsa/lantiq/Kconfig
@@ -15,6 +15,7 @@ config NET_DSA_MXL_GSW1XX
 	tristate "MaxLinear GSW1xx Ethernet switch support"
 	select NET_DSA_TAG_MXL_GSW1XX
 	select NET_DSA_LANTIQ_COMMON
+	select PHY_COMMON_PROPS
 	help
 	  This enables support for the Intel/MaxLinear GSW1xx family of 1GE
 	  switches.
diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.c b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
index 79cf72cc77be..df89707dc59e 100644
--- a/drivers/net/dsa/lantiq/mxl-gsw1xx.c
+++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
@@ -15,6 +15,8 @@
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/of_mdio.h>
+#include <linux/phy/phy-common-props.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/workqueue.h>
 #include <net/dsa.h>
@@ -229,11 +231,17 @@ static int gsw1xx_pcs_phy_xaui_write(struct gsw1xx_priv *priv, u16 addr,
 					1000, 100000);
 }
 
-static int gsw1xx_pcs_reset(struct gsw1xx_priv *priv)
+static int gsw1xx_pcs_reset(struct gsw1xx_priv *priv, phy_interface_t interface)
 {
+	struct dsa_port *pcs_port;
+	unsigned int pol;
 	int ret;
 	u16 val;
 
+	pcs_port = dsa_to_port(priv->gswip.ds, GSW1XX_SGMII_PORT);
+	if (!pcs_port)
+		return -EINVAL;
+
 	/* Assert and deassert SGMII shell reset */
 	ret = regmap_set_bits(priv->shell, GSW1XX_SHELL_RST_REQ,
 			      GSW1XX_RST_REQ_SGMII_SHELL);
@@ -260,15 +268,17 @@ static int gsw1xx_pcs_reset(struct gsw1xx_priv *priv)
 	      FIELD_PREP(GSW1XX_SGMII_PHY_RX0_CFG2_FILT_CNT,
 			 GSW1XX_SGMII_PHY_RX0_CFG2_FILT_CNT_DEF);
 
+	ret = phy_get_manual_rx_polarity(of_fwnode_handle(pcs_port->dn),
+					 phy_modes(interface), &pol);
+	if (ret)
+		return ret;
+
 	/* RX lane seems to be inverted internally, so bit
 	 * GSW1XX_SGMII_PHY_RX0_CFG2_INVERT needs to be set for normal
 	 * (ie. non-inverted) operation.
-	 *
-	 * TODO: Take care of inverted RX pair once generic property is
-	 *       available
 	 */
-
-	val |= GSW1XX_SGMII_PHY_RX0_CFG2_INVERT;
+	if (pol == PHY_POL_NORMAL)
+		val |= GSW1XX_SGMII_PHY_RX0_CFG2_INVERT;
 
 	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_RX0_CFG2, val);
 	if (ret < 0)
@@ -277,9 +287,13 @@ static int gsw1xx_pcs_reset(struct gsw1xx_priv *priv)
 	val = FIELD_PREP(GSW1XX_SGMII_PHY_TX0_CFG3_VBOOST_LEVEL,
 			 GSW1XX_SGMII_PHY_TX0_CFG3_VBOOST_LEVEL_DEF);
 
-	/* TODO: Take care of inverted TX pair once generic property is
-	 *       available
-	 */
+	ret = phy_get_manual_tx_polarity(of_fwnode_handle(pcs_port->dn),
+					 phy_modes(interface), &pol);
+	if (ret)
+		return ret;
+
+	if (pol == PHY_POL_INVERT)
+		val |= GSW1XX_SGMII_PHY_TX0_CFG3_INVERT;
 
 	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_TX0_CFG3, val);
 	if (ret < 0)
@@ -336,7 +350,7 @@ static int gsw1xx_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
 	priv->tbi_interface = PHY_INTERFACE_MODE_NA;
 
 	if (!reconf)
-		ret = gsw1xx_pcs_reset(priv);
+		ret = gsw1xx_pcs_reset(priv, interface);
 
 	if (ret)
 		return ret;
-- 
2.52.0

Re: [PATCH net-next v2 2/3] net: dsa: mxl-gsw1xx: configure PCS polarities

[Vladimir Oltean]

On Fri, Jan 30, 2026 at 12:49:55AM +0000, Daniel Golle wrote:
> Configure SerDes PCS RX and TX polarities using the newly
> introduced generic properties.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> @@ -260,15 +268,17 @@ static int gsw1xx_pcs_reset(struct gsw1xx_priv *priv)
>  	      FIELD_PREP(GSW1XX_SGMII_PHY_RX0_CFG2_FILT_CNT,
>  			 GSW1XX_SGMII_PHY_RX0_CFG2_FILT_CNT_DEF);
>  
> +	ret = phy_get_manual_rx_polarity(of_fwnode_handle(pcs_port->dn),
> +					 phy_modes(interface), &pol);
> +	if (ret)
> +		return ret;
> +
>  	/* RX lane seems to be inverted internally, so bit
>  	 * GSW1XX_SGMII_PHY_RX0_CFG2_INVERT needs to be set for normal
>  	 * (ie. non-inverted) operation.

Sorry, I just noticed this existing comment now.

I think this patch set has the right solution with the wrong explanation,
and as someone who cares about logical consistency as much as about
functionality, I think it's worth resending to clarify something.

There exists a problem which, at a high level, is that the signal can
be inverted at multiple levels in its path. When multiple levels are
configurable, it becomes important to know "which level does this DT
property correspond to".  Otherwise it's not clear how inversions at
multiple layers relate to each other.

For someone who doesn't study the problem deeply, it will be confusing
that this approach is inconsistent with what I intend to do for XPCS on
SJA1105:
https://lore.kernel.org/netdev/20260122105654.105600-16-vladimir.oltean@nxp.com/

That is, if one needs to invert the TX lane signal in the PCS to achieve
normal polarity at the pins (to compensate for internal inversion in the
SerDes/PMA), then the PCS node needs to have "tx-polarity = <PHY_POL_INVERT>".

In Documentation/devicetree/bindings/phy/phy-common-props.yaml it
explicitly says that "If the property is absent, the default value is
undefined." in order to permit drivers to call phy_get_rx_polarity()
with a custom default_val, rather than phy_get_manual_rx_polarity()
which implies a predefined default_val of PHY_POL_NORMAL.

This is the opposite of what you're doing. You need to invert the RX
signal in the SerDes for what can be assumed to be a hidden inversion in
the PCS, but you make that invisible in the device tree, interpreting
PHY_POL_NORMAL as "inverted" when programming the SerDes.

Only difference, which also makes your approach self-consistent, is that
the rx-polarity property goes neither in the PCS nor the SerDes OF nodes
(which don't exist), but in the port node. If we interpret the port node
as what happens at the pins, the totality of the internal data path
layers with no further insight into sub-components, then it's OK and is
the simplest solution to the problem.

But it needs to be presented 100% clearly, with a very clear distinction
between the PCS, the SerDes PHY and the port as a whole.  Patches 1 and
2 use these 3 concepts very interchangeably, ranging from comments/commit
messages ("Reference the common PHY properties so RX and TX SerDes lane
polarity of the SGMII/1000Base-X/2500Base-X PCS can be configured") to
code placement (SerDes configuration done in gsw1xx_pcs_reset()) to
variable names ("pcs_port" could lose the "pcs" portion, to avoid somebody
unfamiliar with the HW doing some refactoring where they take the port
OF node as the PCS OF node, for other purposes as well, perhaps not applicable).

> -	 *
> -	 * TODO: Take care of inverted RX pair once generic property is
> -	 *       available
>  	 */
> -
> -	val |= GSW1XX_SGMII_PHY_RX0_CFG2_INVERT;
> +	if (pol == PHY_POL_NORMAL)
> +		val |= GSW1XX_SGMII_PHY_RX0_CFG2_INVERT;
>  
>  	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_RX0_CFG2, val);
>  	if (ret < 0)
> @@ -277,9 +287,13 @@ static int gsw1xx_pcs_reset(struct gsw1xx_priv *priv)
>  	val = FIELD_PREP(GSW1XX_SGMII_PHY_TX0_CFG3_VBOOST_LEVEL,
>  			 GSW1XX_SGMII_PHY_TX0_CFG3_VBOOST_LEVEL_DEF);
>  
> -	/* TODO: Take care of inverted TX pair once generic property is
> -	 *       available
> -	 */
> +	ret = phy_get_manual_tx_polarity(of_fwnode_handle(pcs_port->dn),
> +					 phy_modes(interface), &pol);
> +	if (ret)
> +		return ret;
> +
> +	if (pol == PHY_POL_INVERT)
> +		val |= GSW1XX_SGMII_PHY_TX0_CFG3_INVERT;
>  
>  	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_TX0_CFG3, val);
>  	if (ret < 0)
> @@ -336,7 +350,7 @@ static int gsw1xx_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
>  	priv->tbi_interface = PHY_INTERFACE_MODE_NA;
>  
>  	if (!reconf)
> -		ret = gsw1xx_pcs_reset(priv);
> +		ret = gsw1xx_pcs_reset(priv, interface);
>  
>  	if (ret)
>  		return ret;
> -- 
> 2.52.0

[PATCH net-next v2 3/3] net: dsa: mxl-gsw1xx: validate chip ID

[Daniel Golle]

No check for actually present hardware is being performed in the probe
function of the mxl-gsw1xx switch driver. So even if the switch isn't
present at the configured MDIO bus address the driver wrongly tells the
user that a "GSWIP version 0 mod 0" was found, outputting errors about
PHY capabilities not matching.

Read and validate the chip MANU_ID and PNUM_ID registers and output
information while probing, but return an error and abort probing in case
the hardware is not actually present.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: no changes

 drivers/net/dsa/lantiq/mxl-gsw1xx.c | 27 ++++++++++++++++++++++++++-
 drivers/net/dsa/lantiq/mxl-gsw1xx.h |  9 +++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.c b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
index df89707dc59e..69858d73245c 100644
--- a/drivers/net/dsa/lantiq/mxl-gsw1xx.c
+++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
@@ -685,7 +685,9 @@ static int gsw1xx_probe(struct mdio_device *mdiodev)
 {
 	struct device *dev = &mdiodev->dev;
 	struct gsw1xx_priv *priv;
-	u32 version;
+	u32 version, val;
+	u8 shellver;
+	u16 pnum;
 	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -733,6 +735,27 @@ static int gsw1xx_probe(struct mdio_device *mdiodev)
 	if (IS_ERR(priv->shell))
 		return PTR_ERR(priv->shell);
 
+	ret = regmap_read(priv->shell, GSW1XX_SHELL_MANU_ID, &val);
+	if (ret < 0)
+		return ret;
+
+	/* validate chip ID */
+	if (FIELD_GET(GSW1XX_SHELL_MANU_ID_FIX1, val) != 1)
+		return -ENODEV;
+
+	if (FIELD_GET(GSW1XX_SHELL_MANU_ID_MANID, val) !=
+	    GSW1XX_SHELL_MANU_ID_MANID_VAL)
+		return -ENODEV;
+
+	pnum = FIELD_GET(GSW1XX_SHELL_MANU_ID_PNUML, val);
+
+	ret = regmap_read(priv->shell, GSW1XX_SHELL_PNUM_ID, &val);
+	if (ret < 0)
+		return ret;
+
+	pnum |= FIELD_GET(GSW1XX_SHELL_PNUM_ID_PNUMM, val) << 4;
+	shellver = FIELD_GET(GSW1XX_SHELL_PNUM_ID_VER, val);
+
 	ret = gsw1xx_serdes_pcs_init(priv);
 	if (ret < 0)
 		return ret;
@@ -753,6 +776,8 @@ static int gsw1xx_probe(struct mdio_device *mdiodev)
 	if (ret)
 		return ret;
 
+	dev_info(dev, "standalone switch part number 0x%x v1.%u\n", pnum, shellver);
+
 	dev_set_drvdata(dev, &priv->gswip);
 
 	return 0;
diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.h b/drivers/net/dsa/lantiq/mxl-gsw1xx.h
index d1fded56e967..caa8f1008587 100644
--- a/drivers/net/dsa/lantiq/mxl-gsw1xx.h
+++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.h
@@ -110,6 +110,15 @@
 #define GSW1XX_SHELL_BASE			0xfa00
 #define  GSW1XX_SHELL_RST_REQ			0x01
 #define   GSW1XX_RST_REQ_SGMII_SHELL		BIT(5)
+#define  GSW1XX_SHELL_MANU_ID			0x10
+#define   GSW1XX_SHELL_MANU_ID_PNUML		GENMASK(15, 12)
+#define   GSW1XX_SHELL_MANU_ID_MANID		GENMASK(11, 1)
+#define    GSW1XX_SHELL_MANU_ID_MANID_VAL	0x389
+#define   GSW1XX_SHELL_MANU_ID_FIX1		BIT(0)
+#define  GSW1XX_SHELL_PNUM_ID			0x11
+#define   GSW1XX_SHELL_PNUM_ID_VER		GENMASK(15, 12)
+#define   GSW1XX_SHELL_PNUM_ID_PNUMM		GENMASK(11, 0)
+
 /* RGMII PAD Slew Control Register */
 #define  GSW1XX_SHELL_RGMII_SLEW_CFG		0x78
 #define   RGMII_SLEW_CFG_DRV_TXC		BIT(2)
-- 
2.52.0