[PATCH net-next] net: dsa: mxl-gsw1xx: fix SerDes RX polarity

[PATCH net-next] net: dsa: mxl-gsw1xx: fix SerDes RX polarity

[Daniel Golle]

According to MaxLinear engineer Benny Weng the RX lane of the SerDes
port of the GSW1xx switches is inverted in hardware, and the
SGMII_PHY_RX0_CFG2_INVERT bit is set by default in order to compensate
for that. Hence also set the SGMII_PHY_RX0_CFG2_INVERT bit by default in
gsw1xx_pcs_reset().

Fixes: 22335939ec90 ("net: dsa: add driver for MaxLinear GSW1xx switch family")
Reported-by: Rasmus Villemoes <ravi@prevas.dk>
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
Sent to net-next as the commit to be fixed is only in net-next.

 drivers/net/dsa/lantiq/mxl-gsw1xx.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.c b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
index 0816c61a47f12..cf33a16fd183b 100644
--- a/drivers/net/dsa/lantiq/mxl-gsw1xx.c
+++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
@@ -255,10 +255,16 @@ 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);
 
-	/* TODO: Take care of inverted RX pair once generic property is
+	/* 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;
+
 	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_RX0_CFG2, val);
 	if (ret < 0)
 		return ret;
-- 
2.52.0

Re: [PATCH net-next] net: dsa: mxl-gsw1xx: fix SerDes RX polarity

[Vladimir Oltean]

On Tue, Dec 02, 2025 at 09:57:21AM +0000, Daniel Golle wrote:
> According to MaxLinear engineer Benny Weng the RX lane of the SerDes
> port of the GSW1xx switches is inverted in hardware, and the
> SGMII_PHY_RX0_CFG2_INVERT bit is set by default in order to compensate
> for that. Hence also set the SGMII_PHY_RX0_CFG2_INVERT bit by default in
> gsw1xx_pcs_reset().
> 
> Fixes: 22335939ec90 ("net: dsa: add driver for MaxLinear GSW1xx switch family")
> Reported-by: Rasmus Villemoes <ravi@prevas.dk>
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---

This shouldn't impact the generic device tree property work, since as
stated there, there won't be any generically imposed default polarity if
the device tree property is missing.

We can perhaps use this thread to continue a philosophical debate on how
should the device tree deal with this situation of internally inverted
polarities (what does PHY_POL_NORMAL mean: the observable behaviour at
the external pins, or the hardware IP configuration?). I have more or
less the same debate going on with the XPCS polarity as set by
nxp_sja1105_sgmii_pma_config().

But the patch itself seems fine regardless of these side discussions.

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

> Sent to net-next as the commit to be fixed is only in net-next.
> 
>  drivers/net/dsa/lantiq/mxl-gsw1xx.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.c b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
> index 0816c61a47f12..cf33a16fd183b 100644
> --- a/drivers/net/dsa/lantiq/mxl-gsw1xx.c
> +++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
> @@ -255,10 +255,16 @@ 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);
>  
> -	/* TODO: Take care of inverted RX pair once generic property is
> +	/* 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;
> +
>  	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_RX0_CFG2, val);
>  	if (ret < 0)
>  		return ret;
> -- 
> 2.52.0

Re: [PATCH net-next] net: dsa: mxl-gsw1xx: fix SerDes RX polarity

[Daniel Golle]

On Wed, Dec 03, 2025 at 11:49:59AM +0200, Vladimir Oltean wrote:
> On Tue, Dec 02, 2025 at 09:57:21AM +0000, Daniel Golle wrote:
> > According to MaxLinear engineer Benny Weng the RX lane of the SerDes
> > port of the GSW1xx switches is inverted in hardware, and the
> > SGMII_PHY_RX0_CFG2_INVERT bit is set by default in order to compensate
> > for that. Hence also set the SGMII_PHY_RX0_CFG2_INVERT bit by default in
> > gsw1xx_pcs_reset().
> > 
> > Fixes: 22335939ec90 ("net: dsa: add driver for MaxLinear GSW1xx switch family")
> > Reported-by: Rasmus Villemoes <ravi@prevas.dk>
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> 
> This shouldn't impact the generic device tree property work, since as
> stated there, there won't be any generically imposed default polarity if
> the device tree property is missing.
> 
> We can perhaps use this thread to continue a philosophical debate on how
> should the device tree deal with this situation of internally inverted
> polarities (what does PHY_POL_NORMAL mean: the observable behaviour at
> the external pins, or the hardware IP configuration?). I have more or
> less the same debate going on with the XPCS polarity as set by
> nxp_sja1105_sgmii_pma_config().

In this case it is really just a bug in the datasheet, because the
switch does set the GSW1XX_SGMII_PHY_RX0_CFG2_INVERT bit by default
after reset, which results in RX polarity to be as expected (ie.
negative and positive pins as labeled).

However, the driver was overwriting the register content which resulted
in the polarity being inverted (despite the fact that the
GSW1XX_SGMII_PHY_RX0_CFG2_INVERT wasn't set, because it is actually
inverted internally, which just isn't well documented).

> 
> But the patch itself seems fine regardless of these side discussions.

As net-next-6.19 has been tagged by now, should I resend the patch
via 'net' tree after the tag was merged?

> 
> Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
> 
> > Sent to net-next as the commit to be fixed is only in net-next.
> > 
> >  drivers/net/dsa/lantiq/mxl-gsw1xx.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.c b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
> > index 0816c61a47f12..cf33a16fd183b 100644
> > --- a/drivers/net/dsa/lantiq/mxl-gsw1xx.c
> > +++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
> > @@ -255,10 +255,16 @@ 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);
> >  
> > -	/* TODO: Take care of inverted RX pair once generic property is
> > +	/* 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;
> > +
> >  	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_RX0_CFG2, val);
> >  	if (ret < 0)
> >  		return ret;
> > -- 
> > 2.52.0
> 

Re: [PATCH net-next] net: dsa: mxl-gsw1xx: fix SerDes RX polarity

[Rasmus Villemoes]

On Wed, Dec 03 2025, Daniel Golle <daniel@makrotopia.org> wrote:

> On Wed, Dec 03, 2025 at 11:49:59AM +0200, Vladimir Oltean wrote:
>> On Tue, Dec 02, 2025 at 09:57:21AM +0000, Daniel Golle wrote:
>> > According to MaxLinear engineer Benny Weng the RX lane of the SerDes
>> > port of the GSW1xx switches is inverted in hardware, and the
>> > SGMII_PHY_RX0_CFG2_INVERT bit is set by default in order to compensate
>> > for that. Hence also set the SGMII_PHY_RX0_CFG2_INVERT bit by default in
>> > gsw1xx_pcs_reset().
>> > 
>> > Fixes: 22335939ec90 ("net: dsa: add driver for MaxLinear GSW1xx switch family")
>> > Reported-by: Rasmus Villemoes <ravi@prevas.dk>
>> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
>> > ---
>> 
>> This shouldn't impact the generic device tree property work, since as
>> stated there, there won't be any generically imposed default polarity if
>> the device tree property is missing.
>> 
>> We can perhaps use this thread to continue a philosophical debate on how
>> should the device tree deal with this situation of internally inverted
>> polarities (what does PHY_POL_NORMAL mean: the observable behaviour at
>> the external pins, or the hardware IP configuration?). I have more or
>> less the same debate going on with the XPCS polarity as set by
>> nxp_sja1105_sgmii_pma_config().
>
> In this case it is really just a bug in the datasheet, because the
> switch does set the GSW1XX_SGMII_PHY_RX0_CFG2_INVERT bit by default
> after reset, which results in RX polarity to be as expected (ie.
> negative and positive pins as labeled).

Well, that "by default" actually depends. When the switch is strapped
PS_NOWAIT=0, the observed reset value of the register is simply 0, and
the host must do all the configuration, including setting that bit.

I suppose that's also the "hardware" reset value, but then in
PS_NOWAIT=1 mode, the ROM/bootloader code inside the switch sets the
register to 0x053a, which is then from the host's POV effectively the
reset value.

I suppose it ended up like this because they realized they'd
accidentally done the swapping in hardware, but then they could sort-of
fix up that by changing the ROM code, but neglected to update the data
sheet or publish an errata. The data sheet claims a reset value of
0x0532, and doesn't say a word about that hardware quirk.

Aside: Can somebody explain why the data sheet would talk about
"incoming data on rx0_data[19:0]" - how and where does the number 20
come into the picture?

Rasmus

Re: [PATCH net-next] net: dsa: mxl-gsw1xx: fix SerDes RX polarity

[Paolo Abeni]

On 12/4/25 12:35 AM, Daniel Golle wrote:
> On Wed, Dec 03, 2025 at 11:49:59AM +0200, Vladimir Oltean wrote:
>> On Tue, Dec 02, 2025 at 09:57:21AM +0000, Daniel Golle wrote:
>>> According to MaxLinear engineer Benny Weng the RX lane of the SerDes
>>> port of the GSW1xx switches is inverted in hardware, and the
>>> SGMII_PHY_RX0_CFG2_INVERT bit is set by default in order to compensate
>>> for that. Hence also set the SGMII_PHY_RX0_CFG2_INVERT bit by default in
>>> gsw1xx_pcs_reset().
>>>
>>> Fixes: 22335939ec90 ("net: dsa: add driver for MaxLinear GSW1xx switch family")
>>> Reported-by: Rasmus Villemoes <ravi@prevas.dk>
>>> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
>>> ---
>>
>> This shouldn't impact the generic device tree property work, since as
>> stated there, there won't be any generically imposed default polarity if
>> the device tree property is missing.
>>
>> We can perhaps use this thread to continue a philosophical debate on how
>> should the device tree deal with this situation of internally inverted
>> polarities (what does PHY_POL_NORMAL mean: the observable behaviour at
>> the external pins, or the hardware IP configuration?). I have more or
>> less the same debate going on with the XPCS polarity as set by
>> nxp_sja1105_sgmii_pma_config().
> 
> In this case it is really just a bug in the datasheet, because the
> switch does set the GSW1XX_SGMII_PHY_RX0_CFG2_INVERT bit by default
> after reset, which results in RX polarity to be as expected (ie.
> negative and positive pins as labeled).
> 
> However, the driver was overwriting the register content which resulted
> in the polarity being inverted (despite the fact that the
> GSW1XX_SGMII_PHY_RX0_CFG2_INVERT wasn't set, because it is actually
> inverted internally, which just isn't well documented).
> 
>>
>> But the patch itself seems fine regardless of these side discussions.
> 
> As net-next-6.19 has been tagged by now, should I resend the patch
> via 'net' tree after the tag was merged?

Not needed, I'm applying this patch to net directly.

Thanks,

Paolo

Re: [PATCH net-next] net: dsa: mxl-gsw1xx: fix SerDes RX polarity

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 2 Dec 2025 09:57:21 +0000 you wrote:
> According to MaxLinear engineer Benny Weng the RX lane of the SerDes
> port of the GSW1xx switches is inverted in hardware, and the
> SGMII_PHY_RX0_CFG2_INVERT bit is set by default in order to compensate
> for that. Hence also set the SGMII_PHY_RX0_CFG2_INVERT bit by default in
> gsw1xx_pcs_reset().
> 
> Fixes: 22335939ec90 ("net: dsa: add driver for MaxLinear GSW1xx switch family")
> Reported-by: Rasmus Villemoes <ravi@prevas.dk>
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> 
> [...]

Here is the summary with links:
  - [net-next] net: dsa: mxl-gsw1xx: fix SerDes RX polarity
    https://git.kernel.org/netdev/net/c/5b48f49ee948

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html