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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aTDJNvLR9evdCaDl@makrotopia.org>
Date: Wed, 3 Dec 2025 23:35:18 +0000
From: Daniel Golle <daniel@...rotopia.org>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Hauke Mehrtens <hauke@...ke-m.de>, Andrew Lunn <andrew@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	Rasmus Villemoes <ravi@...vas.dk>,
	"Benny (Ying-Tsan) Weng" <yweng@...linear.com>
Subject: Re: [PATCH net-next] net: dsa: mxl-gsw1xx: fix SerDes RX polarity

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@...vas.dk>
> > Signed-off-by: Daniel Golle <daniel@...rotopia.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@...il.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
>