On Wed, Nov 26, 2025 at 09:32:30AM +0000, Holger Brunck wrote:

quoted

Hi Vladimir,

quoted

On Tue, Nov 25, 2025 at 11:33:09PM +0100, Andrew Lunn wrote:

quoted

quoted

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.

quoted

quoted

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.

quoted

quoted

quoted

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).

quoted

quoted

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"

quoted

quoted

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.

quoted

quoted

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.

quoted

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?

the Kirkwood based board in question was OOT. Due to the patch we were
able to use the mainline driver without patching it to configure the value we
wanted.

The DTS node looked like this:

&mdio {
        status = "okay";

        switch@10 {
                compatible = "marvell,mv88e6085";
                #address-cells = <1>;
                #size-cells = <0>;
                reg = <0x10>;
                ports {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        port@4 {
                                reg = <4>;
                                label = "port4";
                                phy-connection-type = "sgmii";
                                tx-p2p-microvolt = <604000>;
                                fixed-link {
                                        speed = <1000>;
                                        full-duplex;
                                };
                        };
	};
};