Re: [PATCH net-next 8/8] net: phy: marvell10g: select host interface configuration

On Thu, Nov 18, 2021 at 05:09:51PM +0200, Vladimir Oltean wrote:

On Thu, Nov 18, 2021 at 02:47:44PM +0000, Russell King (Oracle) wrote:

quoted

You're going to get into problems with this on Layerscape, because
reconfiguring the Serdes etc is something I've tried to highlight
as being necessary to NXP since SolidRun started using LX2160A. I
think there's some slow progress towards that, but it's so slow that
I've basically given up caring about it on the Honeycomb/Clearfog CX
boards now.

All the SFP cages on my Honeycomb have been configured for the most
useful mode to me - 1000BASE-X/SGMII, and I've given up caring about
USXGMII/10GBASE-R on those ports.

Speaking of that, do you know of any SFP modules that would use USXGMII?
It doesn't appear to be listed in the spec sheet when looking for that.

I only know of one possibility, which is the DM7052 which uses a
Broadcom BCM84881 PHY. I believe the PHY is capable of USXGMII (there's
various references to it on the 'net) to some extent. By that, I mean
it probably does not provide the 16-bit configuration word in USXGMII
mode - just like it doesn't provide it in SGMII mode.

On the DM7052 module, the PHY is not in USXGMII mode, it will switch
interface modes according to the media speed just like the Marvell
88X3310 does. So, it's vital to use the PHY specific driver for this
PHY, and not the generic driver.

Since the datasheet for the PHY is unavailable, it is not known how to
switch it to USXGMII mode, so it may not be possible to do so except
by modifying the pin strapping on the PHY. It may be possible to do it
through vendor registers.

quoted

quoted

I see that this patch set basically introduces the phydev->host_interfaces
bitmap which is an attempt to find the answer to that question. But when
will we know enough about phydev->host_interfaces in order to safely
make decisions in the PHY driver based on it? phylink sets it, phylib
does not.

It won't be something phylib could set because phylib doesn't know
the capabilities of its user - it's information that would need to be
provided to phylib.

So you're saying it would be in phylib's best interest to not set it at
all, not even to a single bit corresponding to phydev->interface. So PHY
drivers could work out this way whether they should operate in backwards
compatibility mode or they could change MACTYPE at will.

That's what I'm thinking - if we end up with a single bit set in the
host interface, does that mean "the host only supports a single
interface type" or does it mean "DT only specified one interface type
and we need to operate in backwards compatibility mode".

If we provide an empty host_interfaces bitmap, then we can easily
detect that it's not set and fallback to compatibility mode - in the
case of 88x3310, that would basically mean not attempting to set
MACTYPE.

quoted

quoted

And many Aquantia systems use the generic PHY driver, as mentioned.
Additionally, there are old device trees at play here, which only define
the initial SERDES protocol. Would we be changing the behavior for those,
in that we would be configuring the PHY to keep the SERDES protocol
fixed whereas it would have dynamically changed before?

We have the same situation on Macchiatobin. The 88X3310 there defaults
to MACTYPE mode 4, and we've supported this for years with DT describing
the interface as 10GBASE-R - because we haven't actually cared very much
what DT says up to this point for the 88X3310. As I said in my previous
reply, the 88X3310 effectively dictates what the PHY interface mode will
be, and that is communicated back through phylib to whoever is using
phylib.

So what is the full backwards compatibility strategy with old DT blobs?
Is it in this patch set? I didn't notice it.

Marek has attempted to create a backwards compatibility in phylink for
this. See phylink_update_phy_modes().

quoted

quoted

Another question is what to do if there are multiple ways of
establishing a system-side link. For example 1000 Mbps can be achieved
either through SGMII, or USXGMII with symbol replication, or 2500base-x
with flow control, or 10GBaseR with flow control. And I want to test
them all. What would I need to do to change the SERDES protocol from one
value to the other? Changing the phy-mode array in the device tree would
be one option, but that may not always be possible.

First point to make here is that rate adaption at the PHY is really
not well supported in Linux, and there is no way to know via phylib if
a PHY is capable or not of rate adaption.

Today, if you have a 10GBASE-R link between a PHY doing rate adaption
and the "MAC", then what you will get from phylib is:

	phydev->interface = PHY_INTERFACE_MODE_10GBASER;
	phydev->speed = SPEED_1000;	// result of media negotiation
	phydev->duplex = DUPLEX_FULL;	// result of media negotiation
	phydev->pause = ...;		// result of media negotiation
	phydev->asym_pause = ...;	// result of media negotiation

which will, for the majority of implementations, result in the MAC being
forced to a 1G speed, possibly with or without pause enabled.

Due to this, if phylink is being used, the parameters given to
mac_link_up/pcs_link_up will be the result of the media negotiation, not
what is required on the actual link.

You mention "10GBaseR with flow control" but there is another
possibility that exists in real hardware out there. "10GBaseR without
flow control" and in that case, the MAC needs to pace its transmission
for the media speed (which is a good reason why mac_link_up should be
given the result of the media negotiation so it can do transmission
pacing.)

I have a follow-up to the response I gave to Sean Anderson on rate-
adapting PHYs that I need to finish and send, and it would be better
to have any discussion on this topic after I've sent that reply and
follow-up to that reply.

Ok, how would the MAC pace itself to send at a lower data rate, if the
SERDES protocol is 10G and the PHY doesn't send it PAUSE frames back?
At least Layerscape systems can't do this AFAIK.

I'm afraid that is an exercise for the reader/MAC driver author since
it's dependent on the hardware.

One would hope that no one would create a system where the PHY needs to
use rate adaption and requires the MAC to pace itself, but the MAC has
no way to achieve that. If such a hardware combination exists, I don't
see how it could work reliably.

However, bear in mind that the 88X3310 on Macchiatobin boards is the
one without MACSEC, so if it is configured to only operate at 10GBASE-R
with rate adaption, then it will not be generating pause frames and it
will expect the MAC to pace. This is about the only platform I have
which I could experiment with a PHY performing rate adaption. However,
I'm not currently sure how useful that would be - it would be nothing
more than an experimentation exercise, and would require MAC pacing
to be implemented in the mvpp2 driver.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!