Message-ID: <20251126103350.el6mzde47sg5v6od@skbuf>
Date: Wed, 26 Nov 2025 12:33:50 +0200
From: Vladimir Oltean <vladimir.oltean@....com>
To: Holger Brunck <holger.brunck@...achienergy.com>
Cc: Andrew Lunn <andrew@...n.ch>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	Daniel Golle <daniel@...rotopia.org>,
	Horatiu Vultur <horatiu.vultur@...rochip.com>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Vinod Koul <vkoul@...nel.org>,
	Kishon Vijay Abraham I <kishon@...nel.org>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	Eric Woudstra <ericwouds@...il.com>,
	Marek Beh√∫n <kabel@...nel.org>,
	Lee Jones <lee@...nel.org>,
	Patrice Chotard <patrice.chotard@...s.st.com>,
	Maxime Chevallier <maxime.chevallier@...tlin.com>
Subject: Re: [PATCH net-next 1/9] dt-bindings: phy: rename
 transmit-amplitude.yaml to phy-common-props.yaml

Hi Holger,

On Wed, Nov 26, 2025 at 09:32:30AM +0000, Holger Brunck wrote:
> Hi Vladimir,
>  
> > On Tue, Nov 25, 2025 at 11:33:09PM +0100, Andrew Lunn wrote:
> > > > Yeah, although as things currently stand, I'd say that is the lesser
> > > > of problems. The only user (mv88e6xxx) does something strange: it
> > > > says it wants to configure the TX amplitude of SerDes ports, but
> > > > instead follows the phy-handle and applies the amplitude specified in that
> > node.
> > > >
> > > > I tried to mentally follow how things would work in 2 cases:
> > > > 1. PHY referenced by phy-handle is internal, then by definition it's not
> > > >    a SerDes port.
> > > > 2. PHY referenced by phy-handle is external, then the mv88e6xxx driver
> > > >    looks at what is essentially a device tree description of the PHY's
> > > >    TX, and applies that as a mirror image to the local SerDes' TX.
> > > >
> > > > I think the logic is used in mv88e6xxx through case #2, i.e. we
> > > > externalize the mv88e6xxx SerDes electrical properties to an
> > > > unrelated OF node, the connected Ethernet PHY.
> > >
> > > My understanding of the code is the same, #2. Although i would
> > > probably not say it is an unrelated node. I expect the PHY is on the
> > > other end of the SERDES link which is having the TX amplitudes set.
> > > This clearly will not work if there is an SFP cage on the other end,
> > > but it does for an SGMII PHY.
> > 
> > It is unrelated in the sense that the SGMII PHY is a different kernel object, and
> > the mv88e6xxx is polluting its OF node with properties which it then interprets as
> > its own, when the PHY driver may have wanted to configure its SGMII TX
> > amplitude too, via those same generic properties.
> > 
> > > I guess this code is from before the time Russell converted the
> > > mv88e6xxx SERDES code into PCS drivers. The register being set is
> > > within the PCS register set.  The mv88e6xxx also does not make use of
> > > generic phys to represent the SERDES part of the PCS. So there is no
> > > phys phandle to follow since there is no phy.
> > 
> > In my view, the phy-common-props.yaml are supposed to be applicable to
> > either:
> > (1) a network PHY with SerDes host-side connection (I suppose the media
> >     side electrical properties would be covered by Maxime's phy_port
> >     work - Maxime, please confirm).
> > (2) a phylink_pcs with SerDes registers within the same register set
> > (3) a generic PHY
> > 
> > My patch 8/9 (net: phy: air_en8811h: deprecate "airoha,pnswap-rx" and
> > "airoha,pnswap-tx") is an example of case (1) for polarities. Also, for example,
> > at least Aquantia Gen3 PHYs (AQR111, AQR112) have a (not very well
> > documented) "SerDes Lane 0 Amplitude" field in the PHY XS Receive (XAUI TX)
> > Reserved Vendor Provisioning 4 register (address 4.E413).
> > 
> > My patch 7/9 (net: pcs: xpcs: allow lane polarity inversion) is an example of case
> > (2).
> > 
> > I haven't submitted an example of case (3) yet, but the Lynx PCS and Lynx SerDes
> > would fall into that category. The PCS would be free of describing electrical
> > properties, and those would go to the generic PHY (SerDes).
> > 
> > All I'm trying to say is that we're missing an OF node to describe mv88e6xxx PCS
> > electrical properties, because otherwise, it collides with case (1). My note
> > regarding "phys" was just a guess that the "phy-handle"
> > may have been mistaken for the port's SerDes PHY. Although there is a chance
> > Holger knew what he was doing. In any case, I think we need to sort this one
> > way or another, leaving the phy-handle logic a discouraged fallback path.
> > 
> 
> I was checking our use case, and it is a bit special. We have the port in question
> directly connected to a FPGA which has also have a SerDes interface. We are then
> configuring a fixed link to the FPGA without a phy in between so there is also no
> phy handle in our case. But in general, the board in question is now in maintenance
> and there will be no kernel update anymore in the future. Therefore, it is fine with
> me if you remove or rework the code in question completely. Hope that helps.
> 
> Best regards
> Holger

Thanks for the response. So given this clarification, how was commit
926eae604403 ("dsa: mv88e6xxx: make serdes SGMII/Fiber tx amplitude
configurable") useful for you?

Powered by blists - more mailing lists