Re: [PATCH net-next] net: phy: populate host_interfaces when attaching PHY

[Daniel Golle]

Hi Russell,

On Wed, Oct 09, 2024 at 10:01:09AM +0100, Russell King (Oracle) wrote:
> On Wed, Oct 09, 2024 at 02:57:03AM +0100, Daniel Golle wrote:
> > Use bitmask of interfaces supported by the MAC for the PHY to choose
> > from if the declared interface mode is among those using a single pair
> > of SerDes lanes.
> > This will allow 2500Base-T PHYs to switch to SGMII on most hosts, which
> > results in half-duplex being supported in case the MAC supports that.
> > Without this change, 2500Base-T PHYs will always operate in 2500Base-X
> > mode with rate-matching, which is not only wasteful in terms of energy
> > consumption, but also limits the supported interface modes to
> > full-duplex only.
> 
> We've had a similar patch before, and it's been NAK'd. The problem is
> that supplying the host_interfaces for built-in PHYs means that the
> hardware strapping for the PHY interface mode becomes useless, as does
> the DT property specifying it - and thus we may end up selecting a
> mode that both the MAC and PHY support, but the hardware design
> doesn't (e.g. signals aren't connected, signal speed to fast.)
> 
> For example, take a board designed to use RXAUI and the host supports
> 10GBASE-R. The first problem is, RXAUI is not listed in the SFP
> interface list because it's not usable over a SFP cage.

I thought about that, also boards configured for RGMII but both MAC
and PHY supporting SGMII or even 2500Base-X would be such a case.
In order to make sure we don't switch to link modes not supported
by the design I check if the interface mode configured in DT is
among those suitable for use with an SFP (ie. using a single pair
of SerDes lanes):
if (test_bit(pl->link_interface, phylink_sfp_interfaces))
	phy_interface_and(phy_dev->host_interfaces, phylink_sfp_interfaces,
			  pl->config->supported_interfaces);

Neither RXAUI nor RGMII modes are among phylink_sfp_interfaces, so
cases in which those modes are configured in DT are already excluded.

> So, the
> host_interfaces excludes that, and thus the PHY thinks that's not
> supported. It looks at the mask and sees only 10GBASE-R, and
> decides to use that instead with rate matching. The MAC doesn't have
> support for flow control, and thus can't use rate matching.
> 
> Not only have the electrical charateristics been violated by selecting
> a faster interface than the hardware was designed for, but we now have
> rate matching being used when it shouldn't be.

As we are also using using rate matching right now in cases when it
should not (and thereby inhibiting support for half-duplex modes), I
suppose the only good solution would be to allow a set of interface
modes in DT instead of only a single one.

Or, as that is the only really relevant case, we can be more strict
on the condition and additional modes to be added, ie. check if both
PHY and MAC support both 2500Base-X and SGMII, and only add SGMII
in case 2500Base-X is selected in DT.

I have never seen designs on which SGMII and 2500Base-X would both
be supported by the SoC but use a different set of pins. Also, as
2500Base-X is 2.5x as fast as SGMII, it's safe to assume that a
board which has been designed for 2500Base-X would also be fine
using SGMII.

Let me know of either of the above would be acceptable.