[PATCH] net: phy: marvell10g: support XFI rate matching mode

[PATCH] net: phy: marvell10g: support XFI rate matching mode

[Baruch Siach]

When the hardware MACTYPE hardware configuration pins are set to "XFI
with Rate Matching" the PHY interface operate at fixed 10Gbps speed. The
MAC buffer packets in both directions to match various wire speeds.

Read the MAC Type field in the Port Control register, and set the MAC
interface speed accordingly.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 drivers/net/phy/marvell10g.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index d4c2e62b2439..0f157c338c55 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -80,6 +80,8 @@ enum {
 	MV_V2_PORT_CTRL		= 0xf001,
 	MV_V2_PORT_CTRL_SWRST	= BIT(15),
 	MV_V2_PORT_CTRL_PWRDOWN = BIT(11),
+	MV_V2_PORT_MAC_TYPE_MASK = 0x7,
+	MV_V2_PORT_MAC_TYPE_RATE_MATCH = 0x6,
 	/* Temperature control/read registers (88X3310 only) */
 	MV_V2_TEMP_CTRL		= 0xf08a,
 	MV_V2_TEMP_CTRL_MASK	= 0xc000,
@@ -579,8 +581,24 @@ static int mv3310_aneg_done(struct phy_device *phydev)
 	return genphy_c45_aneg_done(phydev);
 }
 
-static void mv3310_update_interface(struct phy_device *phydev)
+static int mv3310_update_interface(struct phy_device *phydev)
 {
+	int val;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_VEND2, MV_V2_PORT_CTRL);
+	if (val < 0)
+		return val;
+
+	/* In "XFI with Rate Matching" mode the PHY interface is fixed at
+	 * 10Gb. The PHY adapts the rate to actual wire speed with help of
+	 * internal 16KB buffer.
+	 */
+	if ((val & MV_V2_PORT_MAC_TYPE_MASK) ==
+			MV_V2_PORT_MAC_TYPE_RATE_MATCH) {
+		phydev->interface = PHY_INTERFACE_MODE_10GBASER;
+		return 0;
+	}
+
 	if ((phydev->interface == PHY_INTERFACE_MODE_SGMII ||
 	     phydev->interface == PHY_INTERFACE_MODE_2500BASEX ||
 	     phydev->interface == PHY_INTERFACE_MODE_10GBASER) &&
@@ -607,6 +625,8 @@ static void mv3310_update_interface(struct phy_device *phydev)
 			break;
 		}
 	}
+
+	return 0;
 }
 
 /* 10GBASE-ER,LR,LRM,SR do not support autonegotiation. */
@@ -719,8 +739,11 @@ static int mv3310_read_status(struct phy_device *phydev)
 	if (err < 0)
 		return err;
 
-	if (phydev->link)
-		mv3310_update_interface(phydev);
+	if (phydev->link) {
+		err = mv3310_update_interface(phydev);
+		if (err < 0)
+			return err;
+	}
 
 	return 0;
 }
-- 
2.27.0

Re: [PATCH] net: phy: marvell10g: support XFI rate matching mode

[Russell King - ARM Linux admin]

On Thu, Jun 25, 2020 at 08:19:21PM +0300, Baruch Siach wrote:
> When the hardware MACTYPE hardware configuration pins are set to "XFI
> with Rate Matching" the PHY interface operate at fixed 10Gbps speed. The
> MAC buffer packets in both directions to match various wire speeds.
> 
> Read the MAC Type field in the Port Control register, and set the MAC
> interface speed accordingly.

Rate matching brings with it a whole host of issues, not just the
interface type, but also the phydev->speed, which is commonly used
to program the MAC.

Rate matching is also used with the unsupported 3310 PHY on the ZII
devel rev C board, and there we need the PHY to also report a speed
of 10G as well as the interface type correctly.  The whole thing
gets quite yucky when you have a 10baseT link on a 3310 PHY with
the host interface running with 10GBASE-R but the MAC programmed for
10Mbps.

The approach I hacked up was to split the current link_state into
media_state and mac_state, and then do this:

+       /* If the PHY supports rate-matching, it will report slower speeds
+        * for these fixed-speed interface modes. Force the MAC side to
+        * full speed.
+        */
+       if (mac_state.interface == PHY_INTERFACE_MODE_XAUI ||
+           mac_state.interface == PHY_INTERFACE_MODE_RXAUI ||
+           mac_state.interface == PHY_INTERFACE_MODE_10GBASER) {
+               mac_state.speed = SPEED_10000;
+               mac_state.duplex = DUPLEX_FULL;
+       }

which is really a dirty hack.  I'd need to re-read the switch
documentation and review what we're doing to check whether such a
thing is really necessary, or whether merely using 10GBASE-R but
programming the MAC to 10Mbps (e.g.) is actually acceptable.

What I'm basically saying is, there could be way more to this than
just setting the interface mode.

We also /should/ be setting the 3310's interface mode according to
how the PHY is configured - but that is slightly complicated by the
various different modes presented by different variants of the PHY
(the P variant vs the non-P variant.)  I seem to remember that
disambiguating them requires looking at the revision bits in the
PHY ID registers, and I've been debating whether we should not be
using the standard mask when matching them.

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

Re: [PATCH] net: phy: marvell10g: support XFI rate matching mode

[Russell King - ARM Linux admin]

On Thu, Jun 25, 2020 at 06:50:12PM +0100, Russell King - ARM Linux admin wrote:
> On Thu, Jun 25, 2020 at 08:19:21PM +0300, Baruch Siach wrote:
> > When the hardware MACTYPE hardware configuration pins are set to "XFI
> > with Rate Matching" the PHY interface operate at fixed 10Gbps speed. The
> > MAC buffer packets in both directions to match various wire speeds.
> > 
> > Read the MAC Type field in the Port Control register, and set the MAC
> > interface speed accordingly.
> 
> Rate matching brings with it a whole host of issues, not just the
> interface type, but also the phydev->speed, which is commonly used
> to program the MAC.
> 
> Rate matching is also used with the unsupported 3310 PHY on the ZII
> devel rev C board, and there we need the PHY to also report a speed
> of 10G as well as the interface type correctly.  The whole thing
> gets quite yucky when you have a 10baseT link on a 3310 PHY with
> the host interface running with 10GBASE-R but the MAC programmed for
> 10Mbps.
> 
> The approach I hacked up was to split the current link_state into
> media_state and mac_state, and then do this:
> 
> +       /* If the PHY supports rate-matching, it will report slower speeds
> +        * for these fixed-speed interface modes. Force the MAC side to
> +        * full speed.
> +        */
> +       if (mac_state.interface == PHY_INTERFACE_MODE_XAUI ||
> +           mac_state.interface == PHY_INTERFACE_MODE_RXAUI ||
> +           mac_state.interface == PHY_INTERFACE_MODE_10GBASER) {
> +               mac_state.speed = SPEED_10000;
> +               mac_state.duplex = DUPLEX_FULL;
> +       }
> 
> which is really a dirty hack.  I'd need to re-read the switch
> documentation and review what we're doing to check whether such a
> thing is really necessary, or whether merely using 10GBASE-R but
> programming the MAC to 10Mbps (e.g.) is actually acceptable.
> 
> What I'm basically saying is, there could be way more to this than
> just setting the interface mode.
> 
> We also /should/ be setting the 3310's interface mode according to
> how the PHY is configured - but that is slightly complicated by the
> various different modes presented by different variants of the PHY
> (the P variant vs the non-P variant.)  I seem to remember that
> disambiguating them requires looking at the revision bits in the
> PHY ID registers, and I've been debating whether we should not be
> using the standard mask when matching them.

Okay, it's not P vs non-P, it's 3310 vs 3340, which use bit 3 in
register 3 to disambiguate between the two.

For both, yes, value 6 is the 10G with rate matching setting, but
there's some variance between the 3310 and 3340 for the other host
interface codes.  So let's not worry about that too much.

Can I suggest that we read this field at probe time or config_init
time, as an efficiency measure?

The 3310 on the ZII rev C (sorry) is operating in XAUI mode (== 1).
Having just played around with the switch configuration for this PHY,
even though the switch port is in XAUI mode, the speed needs to be
set to 10G in the switch, even if the media side is running at
(e.g.) 100M.

That brings up the question - where should that be handled - phylink
will currently report for 100M on the media side with an XAUI or
10GBASE-R interface mode a speed of 100M to the MAC.  Is that
reasonable?

I've also remembered that how rate matching mode works is dependent
on other configuration in the 3310 - not all devices are capable of
producing pause frames in rate matching mode, and the PHY expects
the host to limit the egress packet rate.

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