Re: [PATCH net-next] net: dsa: mark parsed interface mode for legacy switch drivers

[Vladimir Oltean <olteanv@gmail.com>]

Hi Russell,

On Tue, Aug 15, 2023 at 11:13:07AM +0100, Russell King (Oracle) wrote:
> There is one case that I have missed, and it's totally screwed by
> this behaviour where a Marvell DSA switch is connected to a Marvell
> PHY via a RGMII connection:
> 
>    DSA <---------------------------------> PHY
>     v					    v
> dt-dsa-node {				phy: dt-phy-node {
>   phy-handle = <&phy>;			  ...
>   phy-mode = "rgmii-foo";		};
> };
> 
> parses phy mode
> configures for RGMII mode
> configures RGMII delays associated
>  with phy-mode
> calls phy_attach(..., mode);
> phylib sets phy_dev->interface
> 					PHY driver looks at
> 					phydev->interface and
> 					configures delays
> 
> In this case, we have *both* ends configuring the RGMII delays and it
> will not work - because having the DSA MAC end configure the delays
> breaks the phylib model where the MAC *shouldn't* be configuring the
> delays.
> 
> So, should mv88e6xxx_mac_config() also be forcing all RGMII modes
> in state->interface to be PHY_INTERFACE_MODE_RGMII when passing
> that into mv88e6xxx_port_config_interface() if, and only if the
> port is a user port? Or maybe if and only if the port is actually
> connected to a real PHY?

I'd tend to believe that you're right, this would be broken (and not
just with Marvell RGMII PHYs).

I was under the impression that mv88e6xxx_mac_config() will only
configure the RGMII delays associated with phy-mode if the port is in
fixed-link mode.

But looking at the actual condition upon which mv88e6xxx_mac_config()
decides to call mv88e6xxx_port_config_interface(), that is:

	if (mode != MLO_AN_PHY || !mv88e6xxx_phy_is_internal(chip, port))
		mv88e6xxx_port_config_interface()

and thus, an external PHY would return false for the first term of the
OR operation, but true for the second, and it would proceed to configure
the MAC-side RGMII delays, resulting in a double delay.

However, I am not fully confident in my analysis, since I don't have
mv88e6xxx hardware with an RGMII port to confirm.

It's interesting that we haven't seen reports?