Re: [PATCH net v2] net: pcs: xpcs: make supported_interfaces authoritative

[Coia Prant]

On October 29, 2026 9:20:56 PM GMT+08:00, Andrew Lunn <andrew@xxxxxxx> wrote:
>On Thu, Oct 29, 2026 at 06:27:29PM +0800, Coia Prant wrote:
>> pcs.supported_interfaces is the mask of interfaces a given xpcs
>> instance actually supports. The compat table describes what the
>> Synopsys IP can do in general, but a specific integration may support
>> a different subset: the RK3568 XPCS supports 1000BASE-X, SGMII and
>> QSGMII in hardware, but does not implement the C73 10G or 2500BASE-X
>> modes that the Synopsys compat table advertises.
>> 
>> The Rockchip glue currently advertises only SGMII, since that is what
>> the RK3568 MAC side supports today. 1000BASE-X and QSGMII can be added
>> later when the MAC side gains support.
>> 
>> Without narrowing the mask, stmmac_phylink_setup() merges the PCS
>> supported_interfaces into the MAC mask with phy_interface_or(), so the
>> XPCS would contribute 1000BASE-X, 2500BASE-X, USXGMII and the 10G/25G
>> modes to the combined set. The MAC side forces the phy modes and
>> phylink start passes the mode explicitly, so this does not currently
>> make phylink switch to an unsupported mode, but the combined mask still
>> advertises interfaces the SoC cannot use.
>
>I don't know this code too well, but something does not feel right
>here. It might be due to the way stmmac does PCS with phylink, which
>is often considered broken.
>
>What i would expect to happen is that each piece of hardware indicates
>what it can do, and then phylink finds the common modes.
>
>So maybe stmmac_phylink_setup() is wrong when it ORs them together?
>
>Maybe look at other MAC drivers using pcs and phylink.
>
>      Andrew

Hi Andrew,

Thanks for the review. The comment is too terse, let me clarify the
scope of this patch.

The RK3568 XPCS supports 1000BASE-X, SGMII and QSGMII in hardware, but
the Rockchip glue advertises only SGMII for now because that is what
the RK3568 MAC side supports today. 1000BASE-X and QSGMII can be added
later when the MAC side gains support.

On the stmmac side: I agree the phy_interface_or() in
stmmac_phylink_setup() is not the usual intersect model, and it is one
of the reasons stmmac's PCS glue is often considered fragile. But the
DWMAC binding requires phy-mode globally (snps,dwmac.yaml), so the
dwmac-rk glue gets a fixed mode from DT and phylink start passes that
mode explicitly. The OR-merged mask therefore does not currently make
phylink switch to a mode the SoC cannot use.

I've only been touching this area for the last few months, so I'm
guessing it may have been intentional. You might want to ask Russell
King, who designed the original logic, for the rationale.

This patch only touches the PCS side. It makes supported_interfaces
authoritative on the PCS instance, so xpcs_find_compat() does not
configure an interface the glue has marked as unsupported. Reworking
stmmac's OR-merge into a proper intersect model is a separate, larger
topic, and I'd rather not mix it into this fix.

Coia