Re: [PATCH net-next 3/6] net: dsa: add support for retrieving the interface mode

Vladimir Oltean
2022-07-21T13:47:30Z

On Thu, Jul 21, 2022 at 01:44:47AM +0300, Vladimir Oltean wrote:
> I really wish there was a ready-made helper for validating phylink's
> OF node; I mentioned this already, it needs to cater for all of
> fixed-link/phy-handle/managed/sfp.

While I was going to expand on this point and state that DSA doesn't
currently instantiate phylink for this OF node:

port@9 {
reg = <0x9>;
label = "cpu";
ethernet = <&eth1>;
phy-mode = "2500base-x";
managed = "in-band-status";
};

I was proven wrong. Today I learned that of_phy_is_fixed_link() returns
true if the "managed" property exists and its value differs from "auto".
So in the above case, of_phy_is_fixed_link() returns true, hmmm.

On the other hand I found arm64/boot/dts/marvell/cn9130-crb.dtsi, where
the switch, a "marvell,mv88e6190"-compatible (can't determine going just
by that what it actually is) has this:

port@a {
reg = <10>;
label = "cpu";
ethernet = <&cp0_eth0>;
};

To illustrate how odd the situation is, I am able to follow the phandle
to the CPU port and find a comment that it's a 88E6393X, and that the
CPU port uses managed = "in-band-status":

&cp0_eth0 {
/* This port is connected to 88E6393X switch */
status = "okay";
phy-mode = "10gbase-r";
managed = "in-band-status";
phys = <&cp0_comphy4 0>;
};

Open question: is it sane to even do what we're trying here, to create a
fixed-link for port@a (which makes the phylink instance use MLO_AN_FIXED)
when &cp0_eth0 uses MLO_AN_INBAND? My simple mind thinks that if all
involved drivers were to behave correctly and not have bugs that cancel
out other bugs, the above device tree shouldn't work. The host port
would expect a clause 37 base page exchange to take place, the switch
wouldn't send any in-band information, and the SERDES lane would never
transition to data mode. To fix the above, we'd really need to chase the
"ethernet" phandle and attempt to mimic what the DSA master did. This is
indeed logic that never existed before, and I don't particularly feel
like adding it. How far do we want to go? It seems like never-ending
insanity the more I look at it.