[net-next PATCH v2 00/11] net: phy: Add support for fbnic PHY w/ 25G, 50G, and 100G support

[net-next PATCH v2 00/11] net: phy: Add support for fbnic PHY w/ 25G, 50G, and 100G support

[Alexander Duyck]

This first half of this patch set introduces the necessary bits to the
generic c45 driver code to enable it to read 25G, 50G, and 100G speeds from
the PHY and updates the XPCS driver to to enable 25, 50G, and 100G
interfaces matching those supported by the fbnic driver.

The rest of this patch set enables the changes to fbnic to make use of
these interfaces and expose a phydev that can provide a necessary link
delay to avoid link flapping in the event that a cable is disconnected and
reconnected, and to correctly provide the count for the link down events.

With this we have the basic groundwork laid as with this all the bits and
pieces are in place in terms of reading the configuration. The general plan
for follow-on patch sets is to start enabling changing of the configuration
in environments where that is supported.

v2: Added XPCS code to the patch set
    Dropped adding bits for extended ability registers
    Switched from using generic c45 to fbnic_phy
    Fixed several bugs related to phy state machine and use of resume
    Moved PHY connection/disconnection into ndo_init/uninit
    Renamed fbnic_swmii.c to fbnic_mdio.c

---

Alexander Duyck (11):
      net: phy: Add support for 25, 50 and 100Gbps PMA to genphy_c45_read_pma
      net: phy: Add support for 25G, 50G, and 100G interfaces to xpcs driver
      net: phy: Fix PMA identifier handling in XPCS
      net: phy: Add identifier for fbnic PMA and use it to skip initial reset
      net: phy: Add fbnic specific PHY driver fbnic_phy
      fbnic: Rename PCS IRQ to MAC IRQ as it is actually a MAC interrupt
      fbnic: Add logic to track PMD state via MAC/PCS signals
      fbnic: Cleanup handling for link down event statistics
      fbnic: Add SW shim for MDIO interface to PMA/PMD and PCS
      fbnic: Add phydev representing PMD to phylink setup
      fbnic: Replace use of internal PCS w/ Designware XPCS


 MAINTAINERS                                   |   1 +
 drivers/net/ethernet/meta/Kconfig             |   2 +
 drivers/net/ethernet/meta/fbnic/Makefile      |   1 +
 drivers/net/ethernet/meta/fbnic/fbnic.h       |  15 +-
 drivers/net/ethernet/meta/fbnic/fbnic_csr.h   |   2 +
 .../net/ethernet/meta/fbnic/fbnic_ethtool.c   |   9 +
 drivers/net/ethernet/meta/fbnic/fbnic_irq.c   |  42 ++--
 drivers/net/ethernet/meta/fbnic/fbnic_mac.c   |  71 ++++---
 drivers/net/ethernet/meta/fbnic/fbnic_mac.h   |  40 +++-
 drivers/net/ethernet/meta/fbnic/fbnic_mdio.c  | 190 +++++++++++++++++
 .../net/ethernet/meta/fbnic/fbnic_netdev.c    |  27 ++-
 .../net/ethernet/meta/fbnic/fbnic_netdev.h    |   8 +-
 drivers/net/ethernet/meta/fbnic/fbnic_pci.c   |   8 +
 .../net/ethernet/meta/fbnic/fbnic_phylink.c   | 194 ++++++++++++------
 drivers/net/pcs/pcs-xpcs.c                    |  72 ++++++-
 drivers/net/phy/Kconfig                       |   6 +
 drivers/net/phy/Makefile                      |   1 +
 drivers/net/phy/fbnic_phy.c                   |  52 +++++
 drivers/net/phy/phy-c45.c                     |   9 +
 include/linux/pcs/pcs-xpcs.h                  |   4 +-
 include/uapi/linux/mdio.h                     |  14 ++
 21 files changed, 629 insertions(+), 139 deletions(-)
 create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_mdio.c
 create mode 100644 drivers/net/phy/fbnic_phy.c

--

[net-next PATCH v2 01/11] net: phy: Add support for 25, 50 and 100Gbps PMA to genphy_c45_read_pma

[Alexander Duyck]

From: Alexander Duyck <alexanderduyck@fb.com>

Add support for reading 25, 50, and 100G from the PMA interface for a C45
device. By doing this we enable support for future devices that support
higher speeds than the current limit of 10G.

Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
---
 drivers/net/phy/phy-c45.c |    9 +++++++++
 include/uapi/linux/mdio.h |    9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 61670be0f095..2b178a789941 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -626,6 +626,15 @@ int genphy_c45_read_pma(struct phy_device *phydev)
 	case MDIO_CTRL1_SPEED10G:
 		phydev->speed = SPEED_10000;
 		break;
+	case MDIO_CTRL1_SPEED25G:
+		phydev->speed = SPEED_25000;
+		break;
+	case MDIO_CTRL1_SPEED50G:
+		phydev->speed = SPEED_50000;
+		break;
+	case MDIO_CTRL1_SPEED100G:
+		phydev->speed = SPEED_100000;
+		break;
 	default:
 		phydev->speed = SPEED_UNKNOWN;
 		break;
diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
index 6975f182b22c..eee38690ddc4 100644
--- a/include/uapi/linux/mdio.h
+++ b/include/uapi/linux/mdio.h
@@ -116,6 +116,12 @@
 #define MDIO_CTRL1_SPEED10G		(MDIO_CTRL1_SPEEDSELEXT | 0x00)
 /* 10PASS-TS/2BASE-TL */
 #define MDIO_CTRL1_SPEED10P2B		(MDIO_CTRL1_SPEEDSELEXT | 0x04)
+/* 100 Gb/s */
+#define MDIO_CTRL1_SPEED100G		(MDIO_CTRL1_SPEEDSELEXT | 0x0c)
+/* 25 Gb/s */
+#define MDIO_CTRL1_SPEED25G		(MDIO_CTRL1_SPEEDSELEXT | 0x10)
+/* 50 Gb/s */
+#define MDIO_CTRL1_SPEED50G		(MDIO_CTRL1_SPEEDSELEXT | 0x14)
 /* 2.5 Gb/s */
 #define MDIO_CTRL1_SPEED2_5G		(MDIO_CTRL1_SPEEDSELEXT | 0x18)
 /* 5 Gb/s */
@@ -137,9 +143,12 @@
 #define MDIO_SPEED_10G			0x0001	/* 10G capable */
 #define MDIO_PMA_SPEED_2B		0x0002	/* 2BASE-TL capable */
 #define MDIO_PMA_SPEED_10P		0x0004	/* 10PASS-TS capable */
+#define MDIO_PMA_SPEED_50G		0x0008	/* 50G capable */
 #define MDIO_PMA_SPEED_1000		0x0010	/* 1000M capable */
 #define MDIO_PMA_SPEED_100		0x0020	/* 100M capable */
 #define MDIO_PMA_SPEED_10		0x0040	/* 10M capable */
+#define MDIO_PMA_SPEED_100G		0x0200	/* 100G capable */
+#define MDIO_PMA_SPEED_25G		0x0800	/* 25G capable */
 #define MDIO_PMA_SPEED_2_5G		0x2000	/* 2.5G capable */
 #define MDIO_PMA_SPEED_5G		0x4000	/* 5G capable */
 #define MDIO_PCS_SPEED_10P2B		0x0002	/* 10PASS-TS/2BASE-TL capable */

[net-next PATCH v2 02/11] net: phy: Add support for 25G, 50G, and 100G interfaces to xpcs driver

[Alexander Duyck]

From: Alexander Duyck <alexanderduyck@fb.com>

With this change we are adding support for 25G, 50G, and 100G interface
types to the XPCS driver. This had supposedly been enabled with the
addition of XLGMII but I don't see any capability for configuration there
so I suspect it may need to be refactored in the future.

With this change we can enable the XPCS driver with the selected interface
and it should be able to detect link, speed, and report the link status to
the phylink interface.

Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
---
 drivers/net/pcs/pcs-xpcs.c |   60 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 3d1bd5aac093..f2a6fdb972e7 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -37,6 +37,15 @@ static const int xpcs_10gkr_features[] = {
 	__ETHTOOL_LINK_MODE_MASK_NBITS,
 };
 
+static const int xpcs_25gmii_features[] = {
+	ETHTOOL_LINK_MODE_Pause_BIT,
+	ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+	ETHTOOL_LINK_MODE_25000baseCR_Full_BIT,
+	ETHTOOL_LINK_MODE_25000baseKR_Full_BIT,
+	ETHTOOL_LINK_MODE_25000baseSR_Full_BIT,
+	__ETHTOOL_LINK_MODE_MASK_NBITS,
+};
+
 static const int xpcs_xlgmii_features[] = {
 	ETHTOOL_LINK_MODE_Pause_BIT,
 	ETHTOOL_LINK_MODE_Asym_Pause_BIT,
@@ -67,6 +76,31 @@ static const int xpcs_xlgmii_features[] = {
 	__ETHTOOL_LINK_MODE_MASK_NBITS,
 };
 
+static const int xpcs_50gmii_features[] = {
+	ETHTOOL_LINK_MODE_Pause_BIT,
+	ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+	ETHTOOL_LINK_MODE_50000baseCR2_Full_BIT,
+	ETHTOOL_LINK_MODE_50000baseKR2_Full_BIT,
+	ETHTOOL_LINK_MODE_50000baseSR2_Full_BIT,
+	ETHTOOL_LINK_MODE_50000baseKR_Full_BIT,
+	ETHTOOL_LINK_MODE_50000baseSR_Full_BIT,
+	ETHTOOL_LINK_MODE_50000baseCR_Full_BIT,
+	ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT,
+	ETHTOOL_LINK_MODE_50000baseDR_Full_BIT,
+	__ETHTOOL_LINK_MODE_MASK_NBITS,
+};
+
+static const int xpcs_100gmii_features[] = {
+	ETHTOOL_LINK_MODE_Pause_BIT,
+	ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+	ETHTOOL_LINK_MODE_100000baseKR2_Full_BIT,
+	ETHTOOL_LINK_MODE_100000baseSR2_Full_BIT,
+	ETHTOOL_LINK_MODE_100000baseCR2_Full_BIT,
+	ETHTOOL_LINK_MODE_100000baseLR2_ER2_FR2_Full_BIT,
+	ETHTOOL_LINK_MODE_100000baseDR2_Full_BIT,
+	__ETHTOOL_LINK_MODE_MASK_NBITS,
+};
+
 static const int xpcs_10gbaser_features[] = {
 	ETHTOOL_LINK_MODE_Pause_BIT,
 	ETHTOOL_LINK_MODE_Asym_Pause_BIT,
@@ -533,9 +567,19 @@ static void xpcs_resolve_pma(struct dw_xpcs *xpcs,
 	case PHY_INTERFACE_MODE_10GKR:
 		state->speed = SPEED_10000;
 		break;
+	case PHY_INTERFACE_MODE_25GBASER:
+		state->speed = SPEED_25000;
+		break;
 	case PHY_INTERFACE_MODE_XLGMII:
 		state->speed = xpcs_get_max_xlgmii_speed(xpcs, state);
 		break;
+	case PHY_INTERFACE_MODE_50GBASER:
+	case PHY_INTERFACE_MODE_LAUI:
+		state->speed = SPEED_50000;
+		break;
+	case PHY_INTERFACE_MODE_100GBASEP:
+		state->speed = SPEED_100000;
+		break;
 	default:
 		state->speed = SPEED_UNKNOWN;
 		break;
@@ -1312,10 +1356,26 @@ static const struct dw_xpcs_compat synopsys_xpcs_compat[] = {
 		.interface = PHY_INTERFACE_MODE_10GKR,
 		.supported = xpcs_10gkr_features,
 		.an_mode = DW_AN_C73,
+	}, {
+		.interface = PHY_INTERFACE_MODE_25GBASER,
+		.supported = xpcs_25gmii_features,
+		.an_mode = DW_AN_C73,
 	}, {
 		.interface = PHY_INTERFACE_MODE_XLGMII,
 		.supported = xpcs_xlgmii_features,
 		.an_mode = DW_AN_C73,
+	}, {
+		.interface = PHY_INTERFACE_MODE_50GBASER,
+		.supported = xpcs_50gmii_features,
+		.an_mode = DW_AN_C73,
+	}, {
+		.interface = PHY_INTERFACE_MODE_LAUI,
+		.supported = xpcs_50gmii_features,
+		.an_mode = DW_AN_C73,
+	}, {
+		.interface = PHY_INTERFACE_MODE_100GBASEP,
+		.supported = xpcs_100gmii_features,
+		.an_mode = DW_AN_C73,
 	}, {
 		.interface = PHY_INTERFACE_MODE_10GBASER,
 		.supported = xpcs_10gbaser_features,

Re: [net-next PATCH v2 02/11] net: phy: Add support for 25G, 50G, and 100G interfaces to xpcs driver

[Russell King (Oracle)]

Subject should be "net: pcs: xpcs: " to signify that it's for the
PCS net subsystem, and the xpcs driver. Please review

git log drivers/net/pcs/pcs-xpcs.c

to see the normal prefix used when modifying any particular driver.

Thanks.

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

[net-next PATCH v2 03/11] net: phy: Fix PMA identifier handling in XPCS

[Alexander Duyck]

From: Alexander Duyck <alexanderduyck@fb.com>

The XPCS driver was mangling the PMA identifier as the original code
appears to have been focused on just capturing the OUI. Rather than store a
mangled ID it is better to work with the actual PMA ID and instead just
mask out the values that don't apply rather than shifting them and
reordering them as you still don't get the original OUI for the NIC without
having to bitswap the values as per the definition of the layout in IEEE
802.3-2022 22.2.4.3.1.

By laying it out as it was in the hardware it is also less likely for us to
have an unintentional collision as the enum values will occupy the revision
number area while the OUI occupies the upper 22 bits.

Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
---
 drivers/net/pcs/pcs-xpcs.c   |    9 ++++-----
 include/linux/pcs/pcs-xpcs.h |    2 +-
 include/uapi/linux/mdio.h    |    5 +++++
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index f2a6fdb972e7..fde9d9299756 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -1328,17 +1328,16 @@ static int xpcs_read_ids(struct dw_xpcs *xpcs)
 	if (ret < 0)
 		return ret;
 
-	id = ret;
+	id = ret << 16;
 
 	ret = xpcs_read(xpcs, MDIO_MMD_PMAPMD, MDIO_DEVID2);
 	if (ret < 0)
 		return ret;
 
-	/* Note the inverted dword order and masked out Model/Revision numbers
-	 * with respect to what is done with the PCS ID...
+	/* For now we only record the OUI for the PMAPMD, we may want to
+	 * add the model number at some point in the future.
 	 */
-	ret = (ret >> 10) & 0x3F;
-	id |= ret << 16;
+	id |= ret & MDIO_DEVID2_OUI;
 
 	/* Set the PMA ID if it hasn't been pre-initialized */
 	if (xpcs->info.pma == DW_XPCS_PMA_ID_NATIVE)
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index e40f554ff717..4cf6bd611e5a 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -38,7 +38,7 @@ enum dw_xpcs_pma_id {
 	DW_XPCS_PMA_GEN4_6G_ID,
 	DW_XPCS_PMA_GEN5_10G_ID,
 	DW_XPCS_PMA_GEN5_12G_ID,
-	WX_TXGBE_XPCS_PMA_10G_ID = 0x0018fc80,
+	WX_TXGBE_XPCS_PMA_10G_ID = 0xfc806000,
 };
 
 struct dw_xpcs_info {
diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
index eee38690ddc4..387c8b4eeb61 100644
--- a/include/uapi/linux/mdio.h
+++ b/include/uapi/linux/mdio.h
@@ -139,6 +139,11 @@
 #define MDIO_AN_STAT1_PAGE		0x0040	/* Page received */
 #define MDIO_AN_STAT1_XNP		0x0080	/* Extended next page status */
 
+/* Device Identifier 2 */
+#define MDIO_DEVID2_OUI			0xfc00	/* OUI Portion of PHY ID */
+#define MDIO_DEVID2_MODEL_NUM		0x03f0	/* Manufacturer's Model Number */
+#define MDIO_DEVID2_REV_NUM		0x000f	/* Revision Number */
+
 /* Speed register. */
 #define MDIO_SPEED_10G			0x0001	/* 10G capable */
 #define MDIO_PMA_SPEED_2B		0x0002	/* 2BASE-TL capable */

[net-next PATCH v2 04/11] net: phy: Add identifier for fbnic PMA and use it to skip initial reset

[Alexander Duyck]

From: Alexander Duyck <alexanderduyck@fb.com>

The fbnic driver is planning to make use of the XPCS driver to enable
support for PCS and better integration with phylink. To do this though we
will need to enable several workarounds since the PMA/PMD interface for
fbnic is likely to be unique since it is a mix of two different vendor
products with a unique wrapper around the IP.

As such I have generated a PHY identifier based on IEEE 802.3-2022
22.2.4.3.1 using the OUI belonging to Meta Platforms and used with our
NICs. Using this we will provide it as the PHY ID via the SW based MDIO
interface so that the fbnic device can be identified and necessary
workarounds enabled in the XPCS driver.

As an initial workaround this change adds an exception so that soft_reset
is not set when the driver is initially bound to the PCS.

Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
---
 drivers/net/pcs/pcs-xpcs.c   |    3 ++-
 include/linux/pcs/pcs-xpcs.h |    2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index fde9d9299756..a65a7474e490 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -1554,7 +1554,8 @@ static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev)
 
 	xpcs_get_interfaces(xpcs, xpcs->pcs.supported_interfaces);
 
-	if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID)
+	if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID ||
+	    xpcs->info.pma == MP_FBNIC_XPCS_PMA_100G_ID)
 		xpcs->pcs.poll = false;
 	else
 		xpcs->need_reset = true;
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index 4cf6bd611e5a..36073f7b6bb4 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -39,6 +39,8 @@ enum dw_xpcs_pma_id {
 	DW_XPCS_PMA_GEN5_10G_ID,
 	DW_XPCS_PMA_GEN5_12G_ID,
 	WX_TXGBE_XPCS_PMA_10G_ID = 0xfc806000,
+	/* Meta Platforms OUI 88:25:08, model 0, revision 0 */
+	MP_FBNIC_XPCS_PMA_100G_ID = 0x46904000,
 };
 
 struct dw_xpcs_info {

[net-next PATCH v2 05/11] net: phy: Add fbnic specific PHY driver fbnic_phy

[Alexander Duyck]

From: Alexander Duyck <alexanderduyck@fb.com>

With this change we are effectively adding a stub PHY driver for the fbnic
driver to enable it to report link state of the PMA/PMD separately from the
PCS. This is needed as the firmware will be performing link training when
the link is first detected and this will in turn cause the PCS to link flap
if we don't add a delay to the PMD link up process to allow for this.

With this change we are able to identify the device based on the PMA/PMD
and PCS pair being used. The logic is mostly in place to just handle the
link detection and report the correct speed for the link.

This patch is using the gen10g_config_aneg stub to skip doing any
configuration for now. Eventually this will likely be replaced as we
actually start adding configuration bits to the driver.

Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
---
 MAINTAINERS                 |    1 +
 drivers/net/phy/Kconfig     |    6 +++++
 drivers/net/phy/Makefile    |    1 +
 drivers/net/phy/fbnic_phy.c |   52 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 60 insertions(+)
 create mode 100644 drivers/net/phy/fbnic_phy.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 1ab7e8746299..ce18b92f3157 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16712,6 +16712,7 @@ R:	kernel-team@meta.com
 S:	Maintained
 F:	Documentation/networking/device_drivers/ethernet/meta/
 F:	drivers/net/ethernet/meta/
+F:	drivers/net/phy/fbnic_phy.c
 
 METHODE UDPU SUPPORT
 M:	Robert Marko <robert.marko@sartura.hr>
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 98700d069191..16d943bbb883 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -230,6 +230,12 @@ config DAVICOM_PHY
 	help
 	  Currently supports dm9161e and dm9131
 
+config FBNIC_PHY
+	tristate "FBNIC PHY"
+	help
+	  Supports the Meta Platforms 25G/50G/100G Ethernet PHY included in
+	  fbnic network driver.
+
 config ICPLUS_PHY
 	tristate "ICPlus PHYs"
 	help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 76e0db40f879..29b47d9d0425 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_DP83869_PHY)	+= dp83869.o
 obj-$(CONFIG_DP83TC811_PHY)	+= dp83tc811.o
 obj-$(CONFIG_DP83TD510_PHY)	+= dp83td510.o
 obj-$(CONFIG_DP83TG720_PHY)	+= dp83tg720.o
+obj-$(CONFIG_FBNIC_PHY)		+= fbnic_phy.o
 obj-$(CONFIG_FIXED_PHY)		+= fixed_phy.o
 obj-$(CONFIG_ICPLUS_PHY)	+= icplus.o
 obj-$(CONFIG_INTEL_XWAY_PHY)	+= intel-xway.o
diff --git a/drivers/net/phy/fbnic_phy.c b/drivers/net/phy/fbnic_phy.c
new file mode 100644
index 000000000000..5b9be27aec32
--- /dev/null
+++ b/drivers/net/phy/fbnic_phy.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) Meta Platforms, Inc. and affiliates. */
+
+#include <linux/ethtool.h>
+#include <linux/kernel.h>
+#include <linux/mdio.h>
+#include <linux/module.h>
+#include <linux/pcs/pcs-xpcs.h>
+#include <linux/phylink.h>
+
+MODULE_DESCRIPTION("Meta Platforms FBNIC PHY driver");
+MODULE_LICENSE("GPL");
+
+static int fbnic_phy_match_phy_device(struct phy_device *phydev,
+				      const struct phy_driver *phydrv)
+{
+	u32 *device_ids = phydev->c45_ids.device_ids;
+
+	return device_ids[MDIO_MMD_PMAPMD] == MP_FBNIC_XPCS_PMA_100G_ID &&
+	       device_ids[MDIO_MMD_PCS] == DW_XPCS_ID;
+}
+
+static int fbnic_phy_get_features(struct phy_device *phydev)
+{
+	phylink_set(phydev->supported, 100000baseCR2_Full);
+	phylink_set(phydev->supported, 50000baseCR_Full);
+	phylink_set(phydev->supported, 50000baseCR2_Full);
+	phylink_set(phydev->supported, 25000baseCR_Full);
+
+	return 0;
+}
+
+static struct phy_driver fbnic_phy_driver[] = {
+{
+	.phy_id			= MP_FBNIC_XPCS_PMA_100G_ID,
+	.phy_id_mask		= 0xffffffff,
+	.name			= "Meta Platforms FBNIC PHY Driver",
+	.match_phy_device	= fbnic_phy_match_phy_device,
+	.get_features		= fbnic_phy_get_features,
+	.read_status		= genphy_c45_read_status,
+	.config_aneg		= gen10g_config_aneg,
+},
+};
+
+module_phy_driver(fbnic_phy_driver);
+
+static const struct mdio_device_id __maybe_unused fbnic_phy_tbl[] = {
+	{ MP_FBNIC_XPCS_PMA_100G_ID, 0xffffffff },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(mdio, fbnic_phy_tbl);

Re: [net-next PATCH v2 05/11] net: phy: Add fbnic specific PHY driver fbnic_phy

[Heiner Kallweit]

On 11/3/2025 6:00 PM, Alexander Duyck wrote:
> From: Alexander Duyck <alexanderduyck@fb.com>
> 
> With this change we are effectively adding a stub PHY driver for the fbnic
> driver to enable it to report link state of the PMA/PMD separately from the
> PCS. This is needed as the firmware will be performing link training when
> the link is first detected and this will in turn cause the PCS to link flap
> if we don't add a delay to the PMD link up process to allow for this.
> 
> With this change we are able to identify the device based on the PMA/PMD
> and PCS pair being used. The logic is mostly in place to just handle the
> link detection and report the correct speed for the link.
> 
> This patch is using the gen10g_config_aneg stub to skip doing any
> configuration for now. Eventually this will likely be replaced as we
> actually start adding configuration bits to the driver.
> 
> Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
> ---
>  MAINTAINERS                 |    1 +
>  drivers/net/phy/Kconfig     |    6 +++++
>  drivers/net/phy/Makefile    |    1 +
>  drivers/net/phy/fbnic_phy.c |   52 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 60 insertions(+)
>  create mode 100644 drivers/net/phy/fbnic_phy.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1ab7e8746299..ce18b92f3157 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16712,6 +16712,7 @@ R:	kernel-team@meta.com
>  S:	Maintained
>  F:	Documentation/networking/device_drivers/ethernet/meta/
>  F:	drivers/net/ethernet/meta/
> +F:	drivers/net/phy/fbnic_phy.c
>  
>  METHODE UDPU SUPPORT
>  M:	Robert Marko <robert.marko@sartura.hr>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 98700d069191..16d943bbb883 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -230,6 +230,12 @@ config DAVICOM_PHY
>  	help
>  	  Currently supports dm9161e and dm9131
>  
> +config FBNIC_PHY
> +	tristate "FBNIC PHY"
> +	help
> +	  Supports the Meta Platforms 25G/50G/100G Ethernet PHY included in
> +	  fbnic network driver.
> +
>  config ICPLUS_PHY
>  	tristate "ICPlus PHYs"
>  	help
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 76e0db40f879..29b47d9d0425 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -59,6 +59,7 @@ obj-$(CONFIG_DP83869_PHY)	+= dp83869.o
>  obj-$(CONFIG_DP83TC811_PHY)	+= dp83tc811.o
>  obj-$(CONFIG_DP83TD510_PHY)	+= dp83td510.o
>  obj-$(CONFIG_DP83TG720_PHY)	+= dp83tg720.o
> +obj-$(CONFIG_FBNIC_PHY)		+= fbnic_phy.o
>  obj-$(CONFIG_FIXED_PHY)		+= fixed_phy.o
>  obj-$(CONFIG_ICPLUS_PHY)	+= icplus.o
>  obj-$(CONFIG_INTEL_XWAY_PHY)	+= intel-xway.o
> diff --git a/drivers/net/phy/fbnic_phy.c b/drivers/net/phy/fbnic_phy.c
> new file mode 100644
> index 000000000000..5b9be27aec32
> --- /dev/null
> +++ b/drivers/net/phy/fbnic_phy.c
> @@ -0,0 +1,52 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) Meta Platforms, Inc. and affiliates. */
> +
> +#include <linux/ethtool.h>
> +#include <linux/kernel.h>
> +#include <linux/mdio.h>
> +#include <linux/module.h>
> +#include <linux/pcs/pcs-xpcs.h>
> +#include <linux/phylink.h>
> +
> +MODULE_DESCRIPTION("Meta Platforms FBNIC PHY driver");
> +MODULE_LICENSE("GPL");
> +
> +static int fbnic_phy_match_phy_device(struct phy_device *phydev,
> +				      const struct phy_driver *phydrv)
> +{
> +	u32 *device_ids = phydev->c45_ids.device_ids;
> +
> +	return device_ids[MDIO_MMD_PMAPMD] == MP_FBNIC_XPCS_PMA_100G_ID &&
> +	       device_ids[MDIO_MMD_PCS] == DW_XPCS_ID;
> +}
> +
> +static int fbnic_phy_get_features(struct phy_device *phydev)
> +{
> +	phylink_set(phydev->supported, 100000baseCR2_Full);

Better not use phylink functionality in phylib. Use linkmode_set_bit()
instead, internally it uses __set_bit() as well.

> +	phylink_set(phydev->supported, 50000baseCR_Full);
> +	phylink_set(phydev->supported, 50000baseCR2_Full);
> +	phylink_set(phydev->supported, 25000baseCR_Full);
> +
> +	return 0;
> +}
> +
> +static struct phy_driver fbnic_phy_driver[] = {
> +{
> +	.phy_id			= MP_FBNIC_XPCS_PMA_100G_ID,
> +	.phy_id_mask		= 0xffffffff,

You can use helper macro PHY_ID_MATCH_EXACT here.

> +	.name			= "Meta Platforms FBNIC PHY Driver",
> +	.match_phy_device	= fbnic_phy_match_phy_device,
> +	.get_features		= fbnic_phy_get_features,
> +	.read_status		= genphy_c45_read_status,
> +	.config_aneg		= gen10g_config_aneg,
> +},
> +};
> +
> +module_phy_driver(fbnic_phy_driver);
> +
> +static const struct mdio_device_id __maybe_unused fbnic_phy_tbl[] = {
> +	{ MP_FBNIC_XPCS_PMA_100G_ID, 0xffffffff },

Here PHY_ID_MATCH_EXACT can be used too.

> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(mdio, fbnic_phy_tbl);
> 
> 

[net-next PATCH v2 06/11] fbnic: Rename PCS IRQ to MAC IRQ as it is actually a MAC interrupt

[Alexander Duyck]

From: Alexander Duyck <alexanderduyck@fb.com>

Throughout several spots in the code I had called out the IRQ as being
related to the PCS. However the actual IRQ is a part of the MAC and it is
just exposing PCS data. To more accurately reflect the owner of the calls
this change makes it so that we rename the functions and values that are
taking in the interrupt value and processing it to reflect that it is a MAC
call and not a PCS one.

This change is mostly motivated by the fact that we will be moving the
handling of this interrupt from being PCS focused to being more PMA/PMD
focused as this will drive the phydev driver that I am adding instead of
driving the PCS directly.

Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
---
 drivers/net/ethernet/meta/fbnic/fbnic.h         |    6 ++--
 drivers/net/ethernet/meta/fbnic/fbnic_irq.c     |   32 ++++++++++++-----------
 drivers/net/ethernet/meta/fbnic/fbnic_mac.c     |   14 +++++-----
 drivers/net/ethernet/meta/fbnic/fbnic_mac.h     |    8 +++---
 drivers/net/ethernet/meta/fbnic/fbnic_netdev.c  |    4 +--
 drivers/net/ethernet/meta/fbnic/fbnic_phylink.c |    2 +
 6 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
index b03e5a3d5144..98929add5f21 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
@@ -34,7 +34,7 @@ struct fbnic_dev {
 	u32 __iomem *uc_addr4;
 	const struct fbnic_mac *mac;
 	unsigned int fw_msix_vector;
-	unsigned int pcs_msix_vector;
+	unsigned int mac_msix_vector;
 	unsigned short num_irqs;
 
 	struct {
@@ -175,8 +175,8 @@ void fbnic_fw_free_mbx(struct fbnic_dev *fbd);
 void fbnic_hwmon_register(struct fbnic_dev *fbd);
 void fbnic_hwmon_unregister(struct fbnic_dev *fbd);
 
-int fbnic_pcs_request_irq(struct fbnic_dev *fbd);
-void fbnic_pcs_free_irq(struct fbnic_dev *fbd);
+int fbnic_mac_request_irq(struct fbnic_dev *fbd);
+void fbnic_mac_free_irq(struct fbnic_dev *fbd);
 
 void fbnic_napi_name_irqs(struct fbnic_dev *fbd);
 int fbnic_napi_request_irq(struct fbnic_dev *fbd,
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_irq.c b/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
index 1c88a2bf3a7a..145a33e231e7 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
@@ -118,12 +118,12 @@ void fbnic_fw_free_mbx(struct fbnic_dev *fbd)
 	fbd->fw_msix_vector = 0;
 }
 
-static irqreturn_t fbnic_pcs_msix_intr(int __always_unused irq, void *data)
+static irqreturn_t fbnic_mac_msix_intr(int __always_unused irq, void *data)
 {
 	struct fbnic_dev *fbd = data;
 	struct fbnic_net *fbn;
 
-	if (fbd->mac->pcs_get_link_event(fbd) == FBNIC_LINK_EVENT_NONE) {
+	if (fbd->mac->get_link_event(fbd) == FBNIC_LINK_EVENT_NONE) {
 		fbnic_wr32(fbd, FBNIC_INTR_MASK_CLEAR(0),
 			   1u << FBNIC_PCS_MSIX_ENTRY);
 		return IRQ_HANDLED;
@@ -131,26 +131,26 @@ static irqreturn_t fbnic_pcs_msix_intr(int __always_unused irq, void *data)
 
 	fbn = netdev_priv(fbd->netdev);
 
-	phylink_pcs_change(&fbn->phylink_pcs, false);
+	phylink_mac_change(fbn->phylink, false);
 
 	return IRQ_HANDLED;
 }
 
 /**
- * fbnic_pcs_request_irq - Configure the PCS to enable it to advertise link
+ * fbnic_mac_request_irq - Configure the MAC to enable it to advertise link
  * @fbd: Pointer to device to initialize
  *
- * This function provides basic bringup for the MAC/PCS IRQ. For now the IRQ
+ * This function provides basic bringup for the MAC/PHY IRQ. For now the IRQ
  * will remain disabled until we start the MAC/PCS/PHY logic via phylink.
  *
  * Return: non-zero on failure.
  **/
-int fbnic_pcs_request_irq(struct fbnic_dev *fbd)
+int fbnic_mac_request_irq(struct fbnic_dev *fbd)
 {
 	struct pci_dev *pdev = to_pci_dev(fbd->dev);
 	int vector, err;
 
-	WARN_ON(fbd->pcs_msix_vector);
+	WARN_ON(fbd->mac_msix_vector);
 
 	vector = pci_irq_vector(pdev, FBNIC_PCS_MSIX_ENTRY);
 	if (vector < 0)
@@ -159,7 +159,7 @@ int fbnic_pcs_request_irq(struct fbnic_dev *fbd)
 	/* Request the IRQ for PCS link vector.
 	 * Map PCS cause to it, and unmask it
 	 */
-	err = request_irq(vector, &fbnic_pcs_msix_intr, 0,
+	err = request_irq(vector, &fbnic_mac_msix_intr, 0,
 			  fbd->netdev->name, fbd);
 	if (err)
 		return err;
@@ -168,22 +168,22 @@ int fbnic_pcs_request_irq(struct fbnic_dev *fbd)
 	fbnic_wr32(fbd, FBNIC_INTR_MSIX_CTRL(FBNIC_INTR_MSIX_CTRL_PCS_IDX),
 		   FBNIC_PCS_MSIX_ENTRY | FBNIC_INTR_MSIX_CTRL_ENABLE);
 
-	fbd->pcs_msix_vector = vector;
+	fbd->mac_msix_vector = vector;
 
 	return 0;
 }
 
 /**
- * fbnic_pcs_free_irq - Teardown the PCS IRQ to prepare for stopping
+ * fbnic_mac_free_irq - Teardown the MAC IRQ to prepare for stopping
  * @fbd: Pointer to device that is stopping
  *
- * This function undoes the work done in fbnic_pcs_request_irq and prepares
+ * This function undoes the work done in fbnic_mac_request_irq and prepares
  * the device to no longer receive traffic on the host interface.
  **/
-void fbnic_pcs_free_irq(struct fbnic_dev *fbd)
+void fbnic_mac_free_irq(struct fbnic_dev *fbd)
 {
 	/* Vector has already been freed */
-	if (!fbd->pcs_msix_vector)
+	if (!fbd->mac_msix_vector)
 		return;
 
 	/* Disable interrupt */
@@ -192,14 +192,14 @@ void fbnic_pcs_free_irq(struct fbnic_dev *fbd)
 	fbnic_wrfl(fbd);
 
 	/* Synchronize IRQ to prevent race that would unmask vector */
-	synchronize_irq(fbd->pcs_msix_vector);
+	synchronize_irq(fbd->mac_msix_vector);
 
 	/* Mask the vector */
 	fbnic_wr32(fbd, FBNIC_INTR_MASK_SET(0), 1u << FBNIC_PCS_MSIX_ENTRY);
 
 	/* Free the vector */
-	free_irq(fbd->pcs_msix_vector, fbd);
-	fbd->pcs_msix_vector = 0;
+	free_irq(fbd->mac_msix_vector, fbd);
+	fbd->mac_msix_vector = 0;
 }
 
 void fbnic_synchronize_irq(struct fbnic_dev *fbd, int nr)
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_mac.c b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c
index 2a84bd1d7e26..28a2e1fd3760 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c
@@ -434,14 +434,14 @@ static void fbnic_mac_tx_pause_config(struct fbnic_dev *fbd, bool tx_pause)
 	wr32(fbd, FBNIC_RXB_PAUSE_DROP_CTRL, rxb_pause_ctrl);
 }
 
-static int fbnic_pcs_get_link_event_asic(struct fbnic_dev *fbd)
+static int fbnic_mac_get_link_event(struct fbnic_dev *fbd)
 {
-	u32 pcs_intr_mask = rd32(fbd, FBNIC_SIG_PCS_INTR_STS);
+	u32 intr_mask = rd32(fbd, FBNIC_SIG_PCS_INTR_STS);
 
-	if (pcs_intr_mask & FBNIC_SIG_PCS_INTR_LINK_DOWN)
+	if (intr_mask & FBNIC_SIG_PCS_INTR_LINK_DOWN)
 		return FBNIC_LINK_EVENT_DOWN;
 
-	return (pcs_intr_mask & FBNIC_SIG_PCS_INTR_LINK_UP) ?
+	return (intr_mask & FBNIC_SIG_PCS_INTR_LINK_UP) ?
 	       FBNIC_LINK_EVENT_UP : FBNIC_LINK_EVENT_NONE;
 }
 
@@ -521,7 +521,7 @@ static bool fbnic_mac_get_pcs_link_status(struct fbnic_dev *fbd)
 	return !lane_mask;
 }
 
-static bool fbnic_pcs_get_link_asic(struct fbnic_dev *fbd)
+static bool fbnic_mac_get_link(struct fbnic_dev *fbd)
 {
 	bool link;
 
@@ -869,8 +869,8 @@ static const struct fbnic_mac fbnic_mac_asic = {
 	.init_regs = fbnic_mac_init_regs,
 	.pcs_enable = fbnic_pcs_enable_asic,
 	.pcs_disable = fbnic_pcs_disable_asic,
-	.pcs_get_link = fbnic_pcs_get_link_asic,
-	.pcs_get_link_event = fbnic_pcs_get_link_event_asic,
+	.get_link = fbnic_mac_get_link,
+	.get_link_event = fbnic_mac_get_link_event,
 	.get_fec_stats = fbnic_mac_get_fec_stats,
 	.get_pcs_stats = fbnic_mac_get_pcs_stats,
 	.get_eth_mac_stats = fbnic_mac_get_eth_mac_stats,
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_mac.h b/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
index ede5ff0dae22..414c170abcba 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
@@ -59,9 +59,9 @@ enum fbnic_sensor_id {
  *	Configure and enable PCS to enable link if not already enabled
  * void (*pcs_disable)(struct fbnic_dev *fbd);
  *	Shutdown the link if we are the only consumer of it.
- * bool (*pcs_get_link)(struct fbnic_dev *fbd);
+ * bool (*get_link)(struct fbnic_dev *fbd);
  *	Check PCS link status
- * int (*pcs_get_link_event)(struct fbnic_dev *fbd)
+ * int (*get_link_event)(struct fbnic_dev *fbd)
  *	Get the current link event status, reports true if link has
  *	changed to either FBNIC_LINK_EVENT_DOWN or FBNIC_LINK_EVENT_UP
  *
@@ -76,8 +76,8 @@ struct fbnic_mac {
 
 	int (*pcs_enable)(struct fbnic_dev *fbd);
 	void (*pcs_disable)(struct fbnic_dev *fbd);
-	bool (*pcs_get_link)(struct fbnic_dev *fbd);
-	int (*pcs_get_link_event)(struct fbnic_dev *fbd);
+	bool (*get_link)(struct fbnic_dev *fbd);
+	int (*get_link_event)(struct fbnic_dev *fbd);
 
 	void (*get_fec_stats)(struct fbnic_dev *fbd, bool reset,
 			      struct fbnic_fec_stats *fec_stats);
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
index e95be0e7bd9e..2d5ae89b4a15 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
@@ -44,7 +44,7 @@ int __fbnic_open(struct fbnic_net *fbn)
 	if (err)
 		goto time_stop;
 
-	err = fbnic_pcs_request_irq(fbd);
+	err = fbnic_mac_request_irq(fbd);
 	if (err)
 		goto time_stop;
 
@@ -89,7 +89,7 @@ static int fbnic_stop(struct net_device *netdev)
 	phylink_suspend(fbn->phylink, fbnic_bmc_present(fbn->fbd));
 
 	fbnic_down(fbn);
-	fbnic_pcs_free_irq(fbn->fbd);
+	fbnic_mac_free_irq(fbn->fbd);
 
 	fbnic_time_stop(fbn);
 	fbnic_fw_xmit_ownership_msg(fbn->fbd, false);
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
index 7ce3fdd25282..3c0bd435ee28 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
@@ -132,7 +132,7 @@ fbnic_phylink_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
 
 	state->duplex = DUPLEX_FULL;
 
-	state->link = fbd->mac->pcs_get_link(fbd);
+	state->link = fbd->mac->get_link(fbd);
 }
 
 static int

Re: [net-next PATCH v2 06/11] fbnic: Rename PCS IRQ to MAC IRQ as it is actually a MAC interrupt

[Russell King (Oracle)]

On Mon, Nov 03, 2025 at 09:00:41AM -0800, Alexander Duyck wrote:
> @@ -131,26 +131,26 @@ static irqreturn_t fbnic_pcs_msix_intr(int __always_unused irq, void *data)
>  
>  	fbn = netdev_priv(fbd->netdev);
>  
> -	phylink_pcs_change(&fbn->phylink_pcs, false);
> +	phylink_mac_change(fbn->phylink, false);

Please don't. phylink_mac_change() is the older version from before
phylink ended up with split PCS support. Some drivers still use it
(because I haven't got around to sorting those out - I've got the
mess formerly known as stmmac that I'm dealing with.)

It only makes sense to tell phylink about a change in status if there
is a PCS, because that's the only way phylink can be told of updated
status.

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

Re: [net-next PATCH v2 06/11] fbnic: Rename PCS IRQ to MAC IRQ as it is actually a MAC interrupt

[Alexander Duyck]

On Mon, Nov 3, 2025 at 9:48 AM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Mon, Nov 03, 2025 at 09:00:41AM -0800, Alexander Duyck wrote:
> > @@ -131,26 +131,26 @@ static irqreturn_t fbnic_pcs_msix_intr(int __always_unused irq, void *data)
> >
> >       fbn = netdev_priv(fbd->netdev);
> >
> > -     phylink_pcs_change(&fbn->phylink_pcs, false);
> > +     phylink_mac_change(fbn->phylink, false);
>
> Please don't. phylink_mac_change() is the older version from before
> phylink ended up with split PCS support. Some drivers still use it
> (because I haven't got around to sorting those out - I've got the
> mess formerly known as stmmac that I'm dealing with.)
>
> It only makes sense to tell phylink about a change in status if there
> is a PCS, because that's the only way phylink can be told of updated
> status.

Okay. I will update that. But FYI this is just a transient change as
these all get replaced with calls to phy_mac_interrupt when we add the
phydev to deal with the PMD link state tracking.

[net-next PATCH v2 07/11] fbnic: Add logic to track PMD state via MAC/PCS signals

[Alexander Duyck]

From: Alexander Duyck <alexanderduyck@fb.com>

One complication with the design of our part is that the PMD doesn't
provide a direct signal to the host. Instead we have visibility to signals
that the PCS provides to the MAC that allow us to check the link state
through that.

That said we need to account for several things in the PMD and firmware
when managing the link. Specifically when the link first starts to come up
the PMD will cause the link to flap as the firmware will begin a training
cycle when the link is first detected. As a result this will cause link
flapping if we were to immediately report link up when the PCS first
detects it.

To address that we are adding a pmd_state variable that is meant to be a
countdown of sorts indicating the state of the PMD. If the link is down the
PMD will start out in the initialize state, otherwise if the link is up it
will start out in the send_data state. If link is detected while in the
initialize state the PMD state will switch to training, and if after 4
seconds the link is still stable we will transition to the send_data state.
With this we can avoid link flapping when a cable is first connected to the
NIC.

One side effect of this is that we need to pull the link state away from
the PCS. For now we use a union of the PCS link state register value and
the pmd_state. The plan is to add a phydev driver to report the pmd_state
to the phylink interface. With that we can then look at switching over to
the use of the XPCS driver for fbnic instead of having an internal one.

Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
---
 drivers/net/ethernet/meta/fbnic/fbnic.h         |    4 +
 drivers/net/ethernet/meta/fbnic/fbnic_csr.h     |    2 +
 drivers/net/ethernet/meta/fbnic/fbnic_irq.c     |    4 +
 drivers/net/ethernet/meta/fbnic/fbnic_mac.c     |   59 ++++++++++++------
 drivers/net/ethernet/meta/fbnic/fbnic_mac.h     |   35 ++++++++--
 drivers/net/ethernet/meta/fbnic/fbnic_netdev.h  |    2 -
 drivers/net/ethernet/meta/fbnic/fbnic_pci.c     |    4 +
 drivers/net/ethernet/meta/fbnic/fbnic_phylink.c |   77 +++++++++++++++++------
 8 files changed, 137 insertions(+), 50 deletions(-)

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
index 98929add5f21..783a1a91dd25 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
@@ -83,6 +83,10 @@ struct fbnic_dev {
 	/* Last @time_high refresh time in jiffies (to catch stalls) */
 	unsigned long last_read;
 
+	/* PMD specific data */
+	unsigned long start_of_pmd_training;
+	u8 pmd_state;
+
 	/* Local copy of hardware statistics */
 	struct fbnic_hw_stats hw_stats;
 
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_csr.h b/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
index d3a7ad921f18..422265dc7abd 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
@@ -787,6 +787,8 @@ enum {
 
 /* MAC PCS registers */
 #define FBNIC_CSR_START_PCS		0x10000 /* CSR section delimiter */
+#define FBNIC_PCS_PAGE(n)	(0x10000 + 0x400 * (n))	/* 0x40000 + 1024*n */
+#define FBNIC_PCS(reg, n)	((reg) + FBNIC_PCS_PAGE(n))
 #define FBNIC_CSR_END_PCS		0x10668 /* CSR section delimiter */
 
 #define FBNIC_CSR_START_RSFEC		0x10800 /* CSR section delimiter */
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_irq.c b/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
index 145a33e231e7..cd874dde41a2 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
@@ -131,7 +131,9 @@ static irqreturn_t fbnic_mac_msix_intr(int __always_unused irq, void *data)
 
 	fbn = netdev_priv(fbd->netdev);
 
-	phylink_mac_change(fbn->phylink, false);
+	/* Record link down events */
+	if (!fbd->mac->get_link(fbd, fbn->aui, fbn->fec))
+		phylink_mac_change(fbn->phylink, false);
 
 	return IRQ_HANDLED;
 }
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_mac.c b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c
index 28a2e1fd3760..5a219f03277b 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c
@@ -466,9 +466,8 @@ static u32 __fbnic_mac_cmd_config_asic(struct fbnic_dev *fbd,
 	return command_config;
 }
 
-static bool fbnic_mac_get_pcs_link_status(struct fbnic_dev *fbd)
+static bool fbnic_mac_get_link_status(struct fbnic_dev *fbd, u8 aui, u8 fec)
 {
-	struct fbnic_net *fbn = netdev_priv(fbd->netdev);
 	u32 pcs_status, lane_mask = ~0;
 
 	pcs_status = rd32(fbd, FBNIC_SIG_PCS_OUT0);
@@ -476,7 +475,7 @@ static bool fbnic_mac_get_pcs_link_status(struct fbnic_dev *fbd)
 		return false;
 
 	/* Define the expected lane mask for the status bits we need to check */
-	switch (fbn->aui) {
+	switch (aui) {
 	case FBNIC_AUI_100GAUI2:
 		lane_mask = 0xf;
 		break;
@@ -484,7 +483,7 @@ static bool fbnic_mac_get_pcs_link_status(struct fbnic_dev *fbd)
 		lane_mask = 3;
 		break;
 	case FBNIC_AUI_LAUI2:
-		switch (fbn->fec) {
+		switch (fec) {
 		case FBNIC_FEC_OFF:
 			lane_mask = 0x63;
 			break;
@@ -502,7 +501,7 @@ static bool fbnic_mac_get_pcs_link_status(struct fbnic_dev *fbd)
 	}
 
 	/* Use an XOR to remove the bits we expect to see set */
-	switch (fbn->fec) {
+	switch (fec) {
 	case FBNIC_FEC_OFF:
 		lane_mask ^= FIELD_GET(FBNIC_SIG_PCS_OUT0_BLOCK_LOCK,
 				       pcs_status);
@@ -521,7 +520,36 @@ static bool fbnic_mac_get_pcs_link_status(struct fbnic_dev *fbd)
 	return !lane_mask;
 }
 
-static bool fbnic_mac_get_link(struct fbnic_dev *fbd)
+static bool fbnic_pmd_update_state(struct fbnic_dev *fbd, bool signal_detect)
+{
+	/* Delay link up for 4 seconds to allow for link training.
+	 * The state transitions for this are as follows:
+	 *
+	 * All states have the following two transitions in common:
+	 *	Loss of signal -> FBNIC_PMD_INITIALIZE
+	 *		The condition handled below (!signal)
+	 *	Reconfiguration -> FBNIC_PMD_INITIALIZE
+	 *		Occurs when mac_prepare starts a PHY reconfig
+	 * FBNIC_PMD_TRAINING:
+	 *	signal still detected && 4s have passed -> Report link up
+	 *	When link is brought up in link_up -> FBNIC_PMD_SEND_DATA
+	 * FBNIC_PMD_INITIALIZE:
+	 *	signal detected -> FBNIC_PMD_TRAINING
+	 */
+	if (!signal_detect) {
+		fbd->pmd_state = FBNIC_PMD_INITIALIZE;
+	} else if (fbd->pmd_state == FBNIC_PMD_TRAINING &&
+		   time_before(fbd->start_of_pmd_training + 4 * HZ, jiffies)) {
+		return true;
+	} else if (fbd->pmd_state == FBNIC_PMD_INITIALIZE) {
+		fbd->start_of_pmd_training = jiffies;
+		fbd->pmd_state = FBNIC_PMD_TRAINING;
+	}
+
+	return fbd->pmd_state == FBNIC_PMD_SEND_DATA;
+}
+
+static bool fbnic_mac_get_link(struct fbnic_dev *fbd, u8 aui, u8 fec)
 {
 	bool link;
 
@@ -538,7 +566,8 @@ static bool fbnic_mac_get_link(struct fbnic_dev *fbd)
 	wr32(fbd, FBNIC_SIG_PCS_INTR_STS,
 	     FBNIC_SIG_PCS_INTR_LINK_DOWN | FBNIC_SIG_PCS_INTR_LINK_UP);
 
-	link = fbnic_mac_get_pcs_link_status(fbd);
+	link = fbnic_mac_get_link_status(fbd, aui, fec);
+	link = fbnic_pmd_update_state(fbd, link);
 
 	/* Enable interrupt to only capture changes in link state */
 	wr32(fbd, FBNIC_SIG_PCS_INTR_MASK,
@@ -586,20 +615,15 @@ void fbnic_mac_get_fw_settings(struct fbnic_dev *fbd, u8 *aui, u8 *fec)
 	}
 }
 
-static int fbnic_pcs_enable_asic(struct fbnic_dev *fbd)
+static void fbnic_mac_prepare(struct fbnic_dev *fbd, u8 aui, u8 fec)
 {
 	/* Mask and clear the PCS interrupt, will be enabled by link handler */
 	wr32(fbd, FBNIC_SIG_PCS_INTR_MASK, ~0);
 	wr32(fbd, FBNIC_SIG_PCS_INTR_STS, ~0);
 
-	return 0;
-}
-
-static void fbnic_pcs_disable_asic(struct fbnic_dev *fbd)
-{
-	/* Mask and clear the PCS interrupt */
-	wr32(fbd, FBNIC_SIG_PCS_INTR_MASK, ~0);
-	wr32(fbd, FBNIC_SIG_PCS_INTR_STS, ~0);
+	/* If we don't have link tear it all down and start over */
+	if (!fbnic_mac_get_link_status(fbd, aui, fec))
+		fbd->pmd_state = FBNIC_PMD_INITIALIZE;
 }
 
 static void fbnic_mac_link_down_asic(struct fbnic_dev *fbd)
@@ -867,10 +891,9 @@ static int fbnic_mac_get_sensor_asic(struct fbnic_dev *fbd, int id,
 
 static const struct fbnic_mac fbnic_mac_asic = {
 	.init_regs = fbnic_mac_init_regs,
-	.pcs_enable = fbnic_pcs_enable_asic,
-	.pcs_disable = fbnic_pcs_disable_asic,
 	.get_link = fbnic_mac_get_link,
 	.get_link_event = fbnic_mac_get_link_event,
+	.prepare = fbnic_mac_prepare,
 	.get_fec_stats = fbnic_mac_get_fec_stats,
 	.get_pcs_stats = fbnic_mac_get_pcs_stats,
 	.get_eth_mac_stats = fbnic_mac_get_eth_mac_stats,
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_mac.h b/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
index 414c170abcba..2b08046645f2 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
@@ -10,6 +10,23 @@ struct fbnic_dev;
 
 #define FBNIC_MAX_JUMBO_FRAME_SIZE	9742
 
+/* States loosely based on section 136.8.11.7.5 of IEEE 802.3-2022 Ethernet
+ * Standard.  These are needed to track the state of the PHY as it has a delay
+ * of several seconds from the time link comes up until it has completed
+ * training that we need to wait to report the link.
+ *
+ * Currently we treat training as a single block as this is managed by the
+ * firmware.
+ *
+ * We have FBNIC_PMD_SEND_DATA set to 0 as the expected default at driver load
+ * and we initialize the structure containing it to zero at allocation.
+ */
+enum {
+	FBNIC_PMD_SEND_DATA	= 0x0,
+	FBNIC_PMD_INITIALIZE	= 0x1,
+	FBNIC_PMD_TRAINING	= 0x2,
+};
+
 enum {
 	FBNIC_LINK_EVENT_NONE	= 0,
 	FBNIC_LINK_EVENT_UP	= 1,
@@ -55,15 +72,15 @@ enum fbnic_sensor_id {
  * void (*init_regs)(struct fbnic_dev *fbd);
  *	Initialize MAC registers to enable Tx/Rx paths and FIFOs.
  *
- * void (*pcs_enable)(struct fbnic_dev *fbd);
- *	Configure and enable PCS to enable link if not already enabled
- * void (*pcs_disable)(struct fbnic_dev *fbd);
- *	Shutdown the link if we are the only consumer of it.
- * bool (*get_link)(struct fbnic_dev *fbd);
- *	Check PCS link status
  * int (*get_link_event)(struct fbnic_dev *fbd)
  *	Get the current link event status, reports true if link has
  *	changed to either FBNIC_LINK_EVENT_DOWN or FBNIC_LINK_EVENT_UP
+ * bool (*get_link)(struct fbnic_dev *fbd, u8 aui, u8 fec);
+ *	Check link status
+ *
+ * void (*prepare)(struct fbnic_dev *fbd, u8 aui, u8 fec);
+ *	Prepare PHY for init by fetching settings, disabling interrupts,
+ *	and sending an updated PHY config to FW if needed.
  *
  * void (*link_down)(struct fbnic_dev *fbd);
  *	Configure MAC for link down event
@@ -74,10 +91,10 @@ enum fbnic_sensor_id {
 struct fbnic_mac {
 	void (*init_regs)(struct fbnic_dev *fbd);
 
-	int (*pcs_enable)(struct fbnic_dev *fbd);
-	void (*pcs_disable)(struct fbnic_dev *fbd);
-	bool (*get_link)(struct fbnic_dev *fbd);
 	int (*get_link_event)(struct fbnic_dev *fbd);
+	bool (*get_link)(struct fbnic_dev *fbd, u8 aui, u8 fec);
+
+	void (*prepare)(struct fbnic_dev *fbd, u8 aui, u8 fec);
 
 	void (*get_fec_stats)(struct fbnic_dev *fbd, bool reset,
 			      struct fbnic_fec_stats *fec_stats);
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
index b0a87c57910f..7b773c06e245 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
@@ -107,7 +107,7 @@ int fbnic_phylink_ethtool_ksettings_get(struct net_device *netdev,
 int fbnic_phylink_get_fecparam(struct net_device *netdev,
 			       struct ethtool_fecparam *fecparam);
 int fbnic_phylink_init(struct net_device *netdev);
-
+void fbnic_phylink_pmd_training_complete_notify(struct fbnic_net *fbn);
 bool fbnic_check_split_frames(struct bpf_prog *prog,
 			      unsigned int mtu, u32 hds_threshold);
 #endif /* _FBNIC_NETDEV_H_ */
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_pci.c b/drivers/net/ethernet/meta/fbnic/fbnic_pci.c
index 4620f1847f2e..428fc861deff 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_pci.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_pci.c
@@ -207,6 +207,10 @@ static void fbnic_service_task(struct work_struct *work)
 {
 	struct fbnic_dev *fbd = container_of(to_delayed_work(work),
 					     struct fbnic_dev, service_task);
+	struct fbnic_net *fbn = netdev_priv(fbd->netdev);
+
+	if (netif_running(fbd->netdev))
+		fbnic_phylink_pmd_training_complete_notify(fbn);
 
 	rtnl_lock();
 
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
index 3c0bd435ee28..66c4f8b3a917 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
@@ -132,25 +132,9 @@ fbnic_phylink_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
 
 	state->duplex = DUPLEX_FULL;
 
-	state->link = fbd->mac->get_link(fbd);
-}
-
-static int
-fbnic_phylink_pcs_enable(struct phylink_pcs *pcs)
-{
-	struct fbnic_net *fbn = fbnic_pcs_to_net(pcs);
-	struct fbnic_dev *fbd = fbn->fbd;
-
-	return fbd->mac->pcs_enable(fbd);
-}
-
-static void
-fbnic_phylink_pcs_disable(struct phylink_pcs *pcs)
-{
-	struct fbnic_net *fbn = fbnic_pcs_to_net(pcs);
-	struct fbnic_dev *fbd = fbn->fbd;
-
-	return fbd->mac->pcs_disable(fbd);
+	state->link = (fbd->pmd_state == FBNIC_PMD_SEND_DATA) &&
+		      (rd32(fbd, FBNIC_PCS(MDIO_STAT1, 0)) &
+		       MDIO_STAT1_LSTATUS);
 }
 
 static int
@@ -164,8 +148,6 @@ fbnic_phylink_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
 
 static const struct phylink_pcs_ops fbnic_phylink_pcs_ops = {
 	.pcs_config = fbnic_phylink_pcs_config,
-	.pcs_enable = fbnic_phylink_pcs_enable,
-	.pcs_disable = fbnic_phylink_pcs_disable,
 	.pcs_get_state = fbnic_phylink_pcs_get_state,
 };
 
@@ -179,12 +161,39 @@ fbnic_phylink_mac_select_pcs(struct phylink_config *config,
 	return &fbn->phylink_pcs;
 }
 
+static int
+fbnic_phylink_mac_prepare(struct phylink_config *config, unsigned int mode,
+			  phy_interface_t iface)
+{
+	struct net_device *netdev = to_net_dev(config->dev);
+	struct fbnic_net *fbn = netdev_priv(netdev);
+	struct fbnic_dev *fbd = fbn->fbd;
+
+	fbd->mac->prepare(fbd, fbn->aui, fbn->fec);
+
+	return 0;
+}
+
 static void
 fbnic_phylink_mac_config(struct phylink_config *config, unsigned int mode,
 			 const struct phylink_link_state *state)
 {
 }
 
+static int
+fbnic_phylink_mac_finish(struct phylink_config *config, unsigned int mode,
+			 phy_interface_t iface)
+{
+	struct net_device *netdev = to_net_dev(config->dev);
+	struct fbnic_net *fbn = netdev_priv(netdev);
+	struct fbnic_dev *fbd = fbn->fbd;
+
+	/* Retest the link state and restart interrupts */
+	fbd->mac->get_link(fbd, fbn->aui, fbn->fec);
+
+	return 0;
+}
+
 static void
 fbnic_phylink_mac_link_down(struct phylink_config *config, unsigned int mode,
 			    phy_interface_t interface)
@@ -213,7 +222,9 @@ fbnic_phylink_mac_link_up(struct phylink_config *config,
 
 static const struct phylink_mac_ops fbnic_phylink_mac_ops = {
 	.mac_select_pcs = fbnic_phylink_mac_select_pcs,
+	.mac_prepare = fbnic_phylink_mac_prepare,
 	.mac_config = fbnic_phylink_mac_config,
+	.mac_finish = fbnic_phylink_mac_finish,
 	.mac_link_down = fbnic_phylink_mac_link_down,
 	.mac_link_up = fbnic_phylink_mac_link_up,
 };
@@ -254,3 +265,27 @@ int fbnic_phylink_init(struct net_device *netdev)
 
 	return 0;
 }
+
+/**
+ * fbnic_phylink_pmd_training_complete_notify - PMD training complete notifier
+ * @fbn: FBNIC Netdev private data struct phylink device attached to
+ *
+ * The PMD wil have a period of 2 to 3 seconds where the link will flutter when
+ * the link first comes up due to link training. To avoid spamming the kernel
+ * log with messages about this we add a delay of 4 seconds from the time of
+ * the last PCS report of link so that we can guarantee we are unlikely to see
+ * any further link loss events due to link training.
+ **/
+void fbnic_phylink_pmd_training_complete_notify(struct fbnic_net *fbn)
+{
+	struct fbnic_dev *fbd = fbn->fbd;
+
+	if (fbd->pmd_state != FBNIC_PMD_TRAINING)
+		return;
+
+	if (!time_before(fbd->start_of_pmd_training + 4 * HZ, jiffies))
+		return;
+
+	fbd->pmd_state = FBNIC_PMD_SEND_DATA;
+	phylink_mac_change(fbn->phylink, true);
+}

[net-next PATCH v2 08/11] fbnic: Cleanup handling for link down event statistics

[Alexander Duyck]

From: Alexander Duyck <alexanderduyck@fb.com>

The code for handling link down event tracking wasn't working in the
existing code. Specifically we should be tracking unexpected link down
events, not expected ones.

To do this tracking we can use the pmd_state variable and track cases where
we transition from send_data to initialize in the interrupt. These should
be the cases where we would be seeing unexpected link down events.

In addition in order for the stat to have any value we have to display it
so this change adds logic to display it as a part of the ethtool stats.

Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
---
 drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c |    9 +++++++++
 drivers/net/ethernet/meta/fbnic/fbnic_irq.c     |   10 +++++++++-
 drivers/net/ethernet/meta/fbnic/fbnic_netdev.c  |    2 +-
 drivers/net/ethernet/meta/fbnic/fbnic_phylink.c |    2 --
 4 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
index 95fac020eb93..693ebdf38705 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
@@ -1863,6 +1863,14 @@ fbnic_get_rmon_stats(struct net_device *netdev,
 	*ranges = fbnic_rmon_ranges;
 }
 
+static void fbnic_get_link_ext_stats(struct net_device *netdev,
+				     struct ethtool_link_ext_stats *stats)
+{
+	struct fbnic_net *fbn = netdev_priv(netdev);
+
+	stats->link_down_events = fbn->link_down_events;
+}
+
 static const struct ethtool_ops fbnic_ethtool_ops = {
 	.cap_link_lanes_supported	= true,
 	.supported_coalesce_params	= ETHTOOL_COALESCE_USECS |
@@ -1874,6 +1882,7 @@ static const struct ethtool_ops fbnic_ethtool_ops = {
 	.get_regs_len			= fbnic_get_regs_len,
 	.get_regs			= fbnic_get_regs,
 	.get_link			= ethtool_op_get_link,
+	.get_link_ext_stats		= fbnic_get_link_ext_stats,
 	.get_coalesce			= fbnic_get_coalesce,
 	.set_coalesce			= fbnic_set_coalesce,
 	.get_ringparam			= fbnic_get_ringparam,
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_irq.c b/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
index cd874dde41a2..45af6c1331fb 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
@@ -122,6 +122,7 @@ static irqreturn_t fbnic_mac_msix_intr(int __always_unused irq, void *data)
 {
 	struct fbnic_dev *fbd = data;
 	struct fbnic_net *fbn;
+	u64 link_down_events;
 
 	if (fbd->mac->get_link_event(fbd) == FBNIC_LINK_EVENT_NONE) {
 		fbnic_wr32(fbd, FBNIC_INTR_MASK_CLEAR(0),
@@ -130,10 +131,17 @@ static irqreturn_t fbnic_mac_msix_intr(int __always_unused irq, void *data)
 	}
 
 	fbn = netdev_priv(fbd->netdev);
+	link_down_events = fbn->link_down_events;
+
+	/* If the link is up this would be a loss event */
+	if (fbd->pmd_state == FBNIC_PMD_SEND_DATA)
+		link_down_events++;
 
 	/* Record link down events */
-	if (!fbd->mac->get_link(fbd, fbn->aui, fbn->fec))
+	if (!fbd->mac->get_link(fbd, fbn->aui, fbn->fec)) {
+		fbn->link_down_events = link_down_events;
 		phylink_mac_change(fbn->phylink, false);
+	}
 
 	return IRQ_HANDLED;
 }
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
index 2d5ae89b4a15..65318a5b466e 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
@@ -86,10 +86,10 @@ static int fbnic_stop(struct net_device *netdev)
 {
 	struct fbnic_net *fbn = netdev_priv(netdev);
 
+	fbnic_mac_free_irq(fbn->fbd);
 	phylink_suspend(fbn->phylink, fbnic_bmc_present(fbn->fbd));
 
 	fbnic_down(fbn);
-	fbnic_mac_free_irq(fbn->fbd);
 
 	fbnic_time_stop(fbn);
 	fbnic_fw_xmit_ownership_msg(fbn->fbd, false);
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
index 66c4f8b3a917..e10fc08f22f2 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
@@ -203,8 +203,6 @@ fbnic_phylink_mac_link_down(struct phylink_config *config, unsigned int mode,
 	struct fbnic_dev *fbd = fbn->fbd;
 
 	fbd->mac->link_down(fbd);
-
-	fbn->link_down_events++;
 }
 
 static void

Re: [net-next PATCH v2 08/11] fbnic: Cleanup handling for link down event statistics

[Russell King (Oracle)]

On Mon, Nov 03, 2025 at 09:00:54AM -0800, Alexander Duyck wrote:
> @@ -86,10 +86,10 @@ static int fbnic_stop(struct net_device *netdev)
>  {
>  	struct fbnic_net *fbn = netdev_priv(netdev);
>  
> +	fbnic_mac_free_irq(fbn->fbd);
>  	phylink_suspend(fbn->phylink, fbnic_bmc_present(fbn->fbd));
>  
>  	fbnic_down(fbn);
> -	fbnic_mac_free_irq(fbn->fbd);

This change makes no sense to me, and doesn't seem to be described in
the commit message.

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

Re: [net-next PATCH v2 08/11] fbnic: Cleanup handling for link down event statistics

[Alexander Duyck]

On Mon, Nov 3, 2025 at 9:49 AM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Mon, Nov 03, 2025 at 09:00:54AM -0800, Alexander Duyck wrote:
> > @@ -86,10 +86,10 @@ static int fbnic_stop(struct net_device *netdev)
> >  {
> >       struct fbnic_net *fbn = netdev_priv(netdev);
> >
> > +     fbnic_mac_free_irq(fbn->fbd);
> >       phylink_suspend(fbn->phylink, fbnic_bmc_present(fbn->fbd));
> >
> >       fbnic_down(fbn);
> > -     fbnic_mac_free_irq(fbn->fbd);
>
> This change makes no sense to me, and doesn't seem to be described in
> the commit message.

It was mostly about just disabling the IRQ before we tear down the
link in the non-BMC case. Otherwise we run the risk of an interrupt
firing to indicate that the link is down and incrementing the
link_down_event counter when we do an ifconfig down and the link was
intended to be torn down.

[net-next PATCH v2 09/11] fbnic: Add SW shim for MDIO interface to PMA/PMD and PCS

[Alexander Duyck]

From: Alexander Duyck <alexanderduyck@fb.com>

In order for us to support a phydev and PCS device we need to add an MDIO
bus to allow the drivers to have access to the registers for the device.
This change adds such an interface.

The interface will consist of 2 PHYs each consisting of a PMA/PMD and a PCS
located at addresses 0 and 1. There is a need for 2 PHYs due to the fact
that in order to support the 2 lane modes we will needed to access and
configure the PCS vendor registers and RSFEC registers from the second lane
identical to the first.

One side effect of this is that we have to report config values for both
lanes of the PHY as those registers can be poked and technically they would
be valid. For now I am going to have the second lane report speeds
equivalent to the given config for 2 lanes as we should be configuring both
lanes identical for the 2 lane modes.

The plan is in the future to extend out this interface adding RSFEC support
to the PMA through a remapping our CSRs which will essentially convert the
standard c45 offsets to ones matching the setup within our device.

Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
---
 drivers/net/ethernet/meta/fbnic/Makefile     |    1 
 drivers/net/ethernet/meta/fbnic/fbnic.h      |    5 +
 drivers/net/ethernet/meta/fbnic/fbnic_mac.h  |    1 
 drivers/net/ethernet/meta/fbnic/fbnic_mdio.c |  190 ++++++++++++++++++++++++++
 drivers/net/ethernet/meta/fbnic/fbnic_pci.c  |    3 
 5 files changed, 200 insertions(+)
 create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_mdio.c

diff --git a/drivers/net/ethernet/meta/fbnic/Makefile b/drivers/net/ethernet/meta/fbnic/Makefile
index 15e8ff649615..72c41af65364 100644
--- a/drivers/net/ethernet/meta/fbnic/Makefile
+++ b/drivers/net/ethernet/meta/fbnic/Makefile
@@ -21,6 +21,7 @@ fbnic-y := fbnic_csr.o \
 	   fbnic_pci.o \
 	   fbnic_phylink.o \
 	   fbnic_rpc.o \
+	   fbnic_mdio.o \
 	   fbnic_time.o \
 	   fbnic_tlv.o \
 	   fbnic_txrx.o \
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
index 783a1a91dd25..4ed677f3232d 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
@@ -95,6 +95,9 @@ struct fbnic_dev {
 	u64 prev_firmware_time;
 
 	struct fbnic_fw_log fw_log;
+
+	/* MDIO bus for PHYs */
+	struct mii_bus *mdio_bus;
 };
 
 /* Reserve entry 0 in the MSI-X "others" array until we have filled all
@@ -204,6 +207,8 @@ void fbnic_dbg_exit(void);
 
 void fbnic_rpc_reset_valid_entries(struct fbnic_dev *fbd);
 
+int fbnic_mdiobus_create(struct fbnic_dev *fbd);
+
 void fbnic_csr_get_regs(struct fbnic_dev *fbd, u32 *data, u32 *regs_version);
 int fbnic_csr_regs_len(struct fbnic_dev *fbd);
 
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_mac.h b/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
index 2b08046645f2..2a9440df5e1d 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
@@ -55,6 +55,7 @@ enum {
 	FBNIC_AUI_50GAUI1	= 2,	/* 53.125GBd	53.125   * 1 */
 	FBNIC_AUI_100GAUI2	= 3,	/* 106.25GBd	53.125   * 2 */
 	FBNIC_AUI_UNKNOWN	= 4,
+	__FBNIC_AUI_MAX__
 };
 
 #define FBNIC_AUI_MODE_R2	(FBNIC_AUI_LAUI2)
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_mdio.c b/drivers/net/ethernet/meta/fbnic/fbnic_mdio.c
new file mode 100644
index 000000000000..80f2e8d2dca4
--- /dev/null
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_mdio.c
@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) Meta Platforms, Inc. and affiliates. */
+
+#include <linux/mdio.h>
+#include <linux/pcs/pcs-xpcs.h>
+
+#include "fbnic.h"
+#include "fbnic_netdev.h"
+
+#define DW_VENDOR		BIT(15)
+#define FBNIC_PCS_VENDOR	BIT(9)
+#define FBNIC_PCS_ZERO_MASK	(DW_VENDOR - FBNIC_PCS_VENDOR)
+
+static int
+fbnic_mdio_read_pcs(struct fbnic_dev *fbd, int addr, int regnum)
+{
+	int ret;
+
+	/* Report 0 for unsupported registers */
+	if (regnum & FBNIC_PCS_ZERO_MASK)
+		return 0;
+
+	/* Intercept and return correct ID for PCS */
+	if (regnum == MDIO_DEVID1)
+		return DW_XPCS_ID >> 16;
+	if (regnum == MDIO_DEVID2)
+		return DW_XPCS_ID & 0xffff;
+	if (regnum == MDIO_DEVS1)
+		return MDIO_DEVS_PMAPMD | MDIO_DEVS_PCS;
+
+	/* Swap vendor page bit for FBNIC PCS vendor page bit */
+	if (regnum & DW_VENDOR)
+		regnum ^= DW_VENDOR | FBNIC_PCS_VENDOR;
+
+	ret = fbnic_rd32(fbd, FBNIC_PCS_PAGE(addr) + regnum);
+
+	dev_dbg(fbd->dev,
+		"SWMII PCS Rd: Addr: %d RegNum: %d Value: 0x%04x\n",
+		addr, regnum, ret);
+
+	return ret;
+}
+
+static int
+fbnic_mdio_read_pmapmd(struct fbnic_dev *fbd, int addr, int regnum)
+{
+	u16 ctrl1[__FBNIC_AUI_MAX__][2] = {
+		{ MDIO_CTRL1_SPEED25G, MDIO_CTRL1_SPEED50G },
+		{ MDIO_CTRL1_SPEED50G, MDIO_CTRL1_SPEED50G },
+		{ MDIO_CTRL1_SPEED50G, MDIO_CTRL1_SPEED100G },
+		{ MDIO_CTRL1_SPEED100G, MDIO_CTRL1_SPEED100G },
+		{ 0, 0 }};
+	u8 aui = FBNIC_AUI_UNKNOWN;
+	struct fbnic_net *fbn;
+	int ret = 0;
+
+	if (fbd->netdev) {
+		fbn = netdev_priv(fbd->netdev);
+		if (fbn->aui < FBNIC_AUI_UNKNOWN)
+			aui = fbn->aui;
+	}
+
+	switch (regnum) {
+	case MDIO_CTRL1:
+		ret = ctrl1[aui][addr & 1];
+		break;
+	case MDIO_STAT1:
+		ret = fbd->pmd_state == FBNIC_PMD_SEND_DATA ?
+		      MDIO_STAT1_LSTATUS : 0;
+		break;
+	case MDIO_DEVID1:
+		ret = MP_FBNIC_XPCS_PMA_100G_ID >> 16;
+		break;
+	case MDIO_DEVID2:
+		ret = MP_FBNIC_XPCS_PMA_100G_ID & 0xffff;
+		break;
+	case MDIO_DEVS1:
+		ret = MDIO_DEVS_PMAPMD | MDIO_DEVS_PCS;
+		break;
+	case MDIO_STAT2:
+		ret = MDIO_STAT2_DEVPRST_VAL;
+		break;
+	default:
+		break;
+	}
+
+	dev_dbg(fbd->dev,
+		"SWMII PMAPMD Rd: Addr: %d RegNum: %d Value: 0x%04x\n",
+		addr, regnum, ret);
+
+	return ret;
+}
+
+static int
+fbnic_mdio_read_c45(struct mii_bus *bus, int addr, int devnum, int regnum)
+{
+	struct fbnic_dev *fbd = bus->priv;
+
+	if (addr & ~1)
+		return 0xffff;
+
+	if (devnum == MDIO_MMD_PCS)
+		return fbnic_mdio_read_pcs(fbd, addr, regnum);
+
+	if (devnum == MDIO_MMD_PMAPMD)
+		return fbnic_mdio_read_pmapmd(fbd, addr, regnum);
+
+	return 0xffff;
+}
+
+static void
+fbnic_mdio_write_pcs(struct fbnic_dev *fbd, int addr, int regnum, u16 val)
+{
+	/* Report 0 for unsupported registers */
+	if (regnum & FBNIC_PCS_ZERO_MASK)
+		return;
+
+	/* Swap vendor page bit for FBNIC PCS vendor page bit */
+	if (regnum & DW_VENDOR)
+		regnum ^= DW_VENDOR | FBNIC_PCS_VENDOR;
+
+	fbnic_wr32(fbd, FBNIC_PCS_PAGE(addr) + regnum, val);
+
+	dev_dbg(fbd->dev,
+		"SWMII PCS Wr: Addr: %d RegNum: %d Value: 0x%04x\n",
+		addr, regnum, val);
+}
+
+static void
+fbnic_mdio_write_pmapmd(struct fbnic_dev *fbd, int addr, int regnum, u16 val)
+{
+	dev_dbg(fbd->dev,
+		"SWMII PMAPMD Wr: Addr: %d RegNum: %d Value: 0x%04x\n",
+		addr, regnum, val);
+}
+
+static int
+fbnic_mdio_write_c45(struct mii_bus *bus, int addr, int devnum,
+		     int regnum, u16 val)
+{
+	struct fbnic_dev *fbd = bus->priv;
+
+	if (addr & ~1)
+		return 0;
+
+	if (devnum == MDIO_MMD_PCS)
+		fbnic_mdio_write_pcs(fbd, addr, regnum, val);
+
+	if (devnum == MDIO_MMD_PMAPMD)
+		fbnic_mdio_write_pmapmd(fbd, addr, regnum, val);
+
+	return 0;
+}
+
+/**
+ * fbnic_mdiobus_create - Create an MDIO bus to allow interfacing w/ PHYs
+ * @fbd: Pointer to FBNIC device structure to populate bus on
+ *
+ * Initialize an MDIO bus and place a pointer to it on the fbd struct. This bus
+ * will be used to interface with the PMA/PMD and PCS.
+ *
+ * Return: 0 on success, negative on failure
+ **/
+int fbnic_mdiobus_create(struct fbnic_dev *fbd)
+{
+	struct mii_bus *bus;
+	int err;
+
+	bus = devm_mdiobus_alloc(fbd->dev);
+	if (!bus)
+		return -ENOMEM;
+
+	bus->name = "fbnic_mii_bus";
+	bus->read_c45 = &fbnic_mdio_read_c45;
+	bus->write_c45 = &fbnic_mdio_write_c45;
+	bus->parent = fbd->dev;
+	bus->phy_mask = GENMASK(31, 2);
+	bus->priv = fbd;
+	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(fbd->dev));
+
+	err = devm_mdiobus_register(fbd->dev, bus);
+	if (err) {
+		dev_err(fbd->dev, "Failed to create MDIO bus: %d\n", err);
+		return err;
+	}
+
+	fbd->mdio_bus = bus;
+
+	return 0;
+}
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_pci.c b/drivers/net/ethernet/meta/fbnic/fbnic_pci.c
index 428fc861deff..b3f05bdb4f52 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_pci.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_pci.c
@@ -339,6 +339,9 @@ static int fbnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto init_failure_mode;
 	}
 
+	if (fbnic_mdiobus_create(fbd))
+		goto init_failure_mode;
+
 	netdev = fbnic_netdev_alloc(fbd);
 	if (!netdev) {
 		dev_err(&pdev->dev, "Netdev allocation failed\n");

Re: [net-next PATCH v2 09/11] fbnic: Add SW shim for MDIO interface to PMA/PMD and PCS

[Andrew Lunn]

> The interface will consist of 2 PHYs each consisting of a PMA/PMD and a PCS
> located at addresses 0 and 1.

I'm missing a bit of architecture here.

At least for speeds up to 10G, we have the MAC enumerate what it can
do, the PCS enumerates its capabilities, and we read the EERPOM of the
SFP to find out what it supports. From that, we can figure out the
subset of link modes which are supported, and configure the MAC and
PCS as required.

What information is missing from this picture that requires the
PMA/PMD to be represented? And how is this going to work when we do
have access to the SFPs EERPOM?

       Andrew

Re: [net-next PATCH v2 09/11] fbnic: Add SW shim for MDIO interface to PMA/PMD and PCS

[Alexander Duyck]

On Mon, Nov 3, 2025 at 10:59 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > The interface will consist of 2 PHYs each consisting of a PMA/PMD and a PCS
> > located at addresses 0 and 1.
>
> I'm missing a bit of architecture here.
>
> At least for speeds up to 10G, we have the MAC enumerate what it can
> do, the PCS enumerates its capabilities, and we read the EERPOM of the
> SFP to find out what it supports. From that, we can figure out the
> subset of link modes which are supported, and configure the MAC and
> PCS as required.

The hardware we have is divisible with multiple entities running it
parallel. It can be used as a single instance, or multiple. With our
hardware we have 2 MACs that are sharing a single QSFP connection, but
the hardware can in theory have 4 MACs sharing a QSFP-DD connection.
The basic limitation is that underneath each MAC we can support at
most 2 lanes of traffic, so just the Base-R/R2 modes. Effectively what
we would end up with is the SFP PHY having to be chained behind the
internal PHY if there is one. In the case of the CR/KR setups though
we are usually just running straight from point-to-point with a few
meter direct attach cable or internal backplane connection.

So from the MAC we have the XPCS which has support for 2 lanes. To
support that we will need to have access to 2 PCS instances as the IP
is divisible to support either 1 or 2 lanes through a single instance.
Then underneath that is an internal PCS PMA which I plan to merge in
with the PMA/PMD I am representing here as the RSFEC registers are
supposed to be a part of the PMA. Again with 2 lanes supported I need
to access two instances of it for the R2 modes. Then underneath that
we have the PMD which is configurable on a per-lane basis. Technically
it is just a SerDes PHY and doesn't have a link detection, it is just
detecting if there is a signal or not and then kicking off the
training, but we can essentially just represent it with the phydev so
that we can report if it is ready to handle the link or not.

> What information is missing from this picture that requires the
> PMA/PMD to be represented? And how is this going to work when we do
> have access to the SFPs EERPOM?

The issue is that the firmware is managing the PMD underneath us. As a
result we don't have full control of the link. One issue we are
running into is that the FW will start training when it first gets a
signal and it doesn't block the signal from getting to the PCS. The
PCS will see the signal and immediately report the link as "up" if the
quality is good enough. This results in us suddenly seeing the link
flapping for about 2-3 seconds while the training is happening. So to
prevent that from happening we are adding the phydev representing the
PMD to delay the link up by the needed 4 seconds to prevent the link
flap noise.

Re: [net-next PATCH v2 09/11] fbnic: Add SW shim for MDIO interface to PMA/PMD and PCS

[Andrew Lunn]

On Mon, Nov 03, 2025 at 12:18:38PM -0800, Alexander Duyck wrote:
> On Mon, Nov 3, 2025 at 10:59 AM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > The interface will consist of 2 PHYs each consisting of a PMA/PMD and a PCS
> > > located at addresses 0 and 1.
> >
> > I'm missing a bit of architecture here.
> >
> > At least for speeds up to 10G, we have the MAC enumerate what it can
> > do, the PCS enumerates its capabilities, and we read the EERPOM of the
> > SFP to find out what it supports. From that, we can figure out the
> > subset of link modes which are supported, and configure the MAC and
> > PCS as required.
> 
> The hardware we have is divisible with multiple entities running it
> parallel. It can be used as a single instance, or multiple. With our
> hardware we have 2 MACs that are sharing a single QSFP connection, but
> the hardware can in theory have 4 MACs sharing a QSFP-DD connection.
> The basic limitation is that underneath each MAC we can support at
> most 2 lanes of traffic, so just the Base-R/R2 modes. Effectively what
> we would end up with is the SFP PHY having to be chained behind the
> internal PHY if there is one. In the case of the CR/KR setups though
> we are usually just running straight from point-to-point with a few
> meter direct attach cable or internal backplane connection.

We need Russell to confirm, but i would expect the SFP driver will
enumerate the capabilities of the SFP and include all the -1, -2 and
-4 link modes. phylink will then call the pcs_validate, passing this
list of link modes. The PCS knows it only supports 1 or 2 lanes, so it
will remove all the -4 modes from the list. phylink will also pass the
list to the MAC driver, and it can remove any it does not support.

It also sounds like you need to ask the firmware about
provisioning. Does this instance have access to 1 or 2 lanes? That
could be done in either the PCS or the MAC? The .validate can then
remove even more link modes.

> To
> support that we will need to have access to 2 PCS instances as the IP
> is divisible to support either 1 or 2 lanes through a single instance.

Another architecture question.... Should phylink know there are two
PCS instances? Or should it see just one? 802.3 defines registers for
lanes 0-3, sometimes 0-7, sometimes 0-9, and even 0-19. So a single
PCS should be enough for 2 lanes, or 4 lanes.

> Then underneath that is an internal PCS PMA which I plan to merge in
> with the PMA/PMD I am representing here as the RSFEC registers are
> supposed to be a part of the PMA. Again with 2 lanes supported I need
> to access two instances of it for the R2 modes. Then underneath that
> we have the PMD which is configurable on a per-lane basis.

There is already some support for pma configuration in pcs-xpcs. See
pcs-xpcs-nxp.c. 

> The issue is that the firmware is managing the PMD underneath us. As a
> result we don't have full control of the link. One issue we are
> running into is that the FW will start training when it first gets a
> signal and it doesn't block the signal from getting to the PCS. The
> PCS will see the signal and immediately report the link as "up" if the
> quality is good enough. This results in us suddenly seeing the link
> flapping for about 2-3 seconds while the training is happening. So to
> prevent that from happening we are adding the phydev representing the
> PMD to delay the link up by the needed 4 seconds to prevent the link
> flap noise.

So it seems like you need to extend dw_xpcs_compat with a .get_state
callback. You can then have your own implementation which adds this 4
second delay, before chaining into xpcs_get_state() to return the true
state.

	Andrew

Re: [net-next PATCH v2 09/11] fbnic: Add SW shim for MDIO interface to PMA/PMD and PCS

[Alexander Duyck]

On Mon, Nov 3, 2025 at 1:49 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Mon, Nov 03, 2025 at 12:18:38PM -0800, Alexander Duyck wrote:
> > On Mon, Nov 3, 2025 at 10:59 AM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > > The interface will consist of 2 PHYs each consisting of a PMA/PMD and a PCS
> > > > located at addresses 0 and 1.
> > >
> > > I'm missing a bit of architecture here.
> > >
> > > At least for speeds up to 10G, we have the MAC enumerate what it can
> > > do, the PCS enumerates its capabilities, and we read the EERPOM of the
> > > SFP to find out what it supports. From that, we can figure out the
> > > subset of link modes which are supported, and configure the MAC and
> > > PCS as required.
> >
> > The hardware we have is divisible with multiple entities running it
> > parallel. It can be used as a single instance, or multiple. With our
> > hardware we have 2 MACs that are sharing a single QSFP connection, but
> > the hardware can in theory have 4 MACs sharing a QSFP-DD connection.
> > The basic limitation is that underneath each MAC we can support at
> > most 2 lanes of traffic, so just the Base-R/R2 modes. Effectively what
> > we would end up with is the SFP PHY having to be chained behind the
> > internal PHY if there is one. In the case of the CR/KR setups though
> > we are usually just running straight from point-to-point with a few
> > meter direct attach cable or internal backplane connection.
>
> We need Russell to confirm, but i would expect the SFP driver will
> enumerate the capabilities of the SFP and include all the -1, -2 and
> -4 link modes. phylink will then call the pcs_validate, passing this
> list of link modes. The PCS knows it only supports 1 or 2 lanes, so it
> will remove all the -4 modes from the list. phylink will also pass the
> list to the MAC driver, and it can remove any it does not support.

In the drivers the limiting would be done based on the interface at
the PCS level.  The way I added the 25G, 50G, and 100G features was
based on the interface type, so that interface type would be what puts
a limit on the number of lanes supported.

In the actual hardware though the limiting factor is that the PMA/PMD
is only 2 lanes. The PCS actually supports 4 lanes and is just being
subdivided to only use 2 of them per MAC.

> It also sounds like you need to ask the firmware about
> provisioning. Does this instance have access to 1 or 2 lanes? That
> could be done in either the PCS or the MAC? The .validate can then
> remove even more link modes.

The way the hardware is setup we always have 2 physical lanes. Where
we need to decide on using one or two is dependent on which mode it is
we are wanting to use in software and what our link partner supports.
For example, on one of our test setups we just have a QSFP-DD loopback
plug installed and we can configure for whatever we want and link up
and talk to ourselves.

That actually presents a number of challenges for us as the SFP driver
currently doesn't understand CMIS, and again we are stuck having to
emulate the I2C via the driver as it is hidden behind the FW
interface. This is one of the reasons why we end up currently with the
FW telling us what the expected link mode/AUI currently is.

> > To
> > support that we will need to have access to 2 PCS instances as the IP
> > is divisible to support either 1 or 2 lanes through a single instance.
>
> Another architecture question.... Should phylink know there are two
> PCS instances? Or should it see just one? 802.3 defines registers for
> lanes 0-3, sometimes 0-7, sometimes 0-9, and even 0-19. So a single
> PCS should be enough for 2 lanes, or 4 lanes.

I'm thinking the driver needs to know about one, but it needs access
to the registers for both in order to be able to configure the
multi-lane setup. The issue is that the IP was made so that the vendor
registers for both lanes need to be configured identical for the 2
lane modes in order to make the device work. That is why I thought I
would go ahead and enable both lanes for now, while only connecting
one of them to the driver.

If we had multiple MACs both of the PCS lanes could have been used in
parallel for the 1 lane setups, however since we only have one MAC it
ends up running both lanes. Since that was the case I thought I would
stick to what would have likely been the layout if we had multiple
MACs which was to expose both lanes as separate PHYs, but only map the
device on the first one. That said, if need be I could look at just
remapping the PCS for the second lane as a MDIO_MMD_VEND1/2. I would
just have to relocate the RSFEC registers for the second lane and the
PCS vendor registers to that device.

> > Then underneath that is an internal PCS PMA which I plan to merge in
> > with the PMA/PMD I am representing here as the RSFEC registers are
> > supposed to be a part of the PMA. Again with 2 lanes supported I need
> > to access two instances of it for the R2 modes. Then underneath that
> > we have the PMD which is configurable on a per-lane basis.
>
> There is already some support for pma configuration in pcs-xpcs. See
> pcs-xpcs-nxp.c.

I was dealing with different IP from different vendors so I didn't
want to throw the PMD code in with the PCS code. I suppose I could do
so though if that is what you are suggesting, I would essentially just
be up-leveling it to the PMA interface.

As it stood I was considering adding an MMD 8 interface to represent
the PMA on the PCS since that would probably be closer to what we
actually have going on where the PCS/FEC/PMA block on top is then
talking to the PMA/PMD which is then doing the equalization and
training before we send the data over the Direct Attach Copper cable.

I had chosen the phydev route as it already had a good way of handling
this. I was thinking the phydev could essentially become a fractional
QSFP bus as it will likely be some time before we could even get
support for a standard QSFP bus and CMIS upstream.

> > The issue is that the firmware is managing the PMD underneath us. As a
> > result we don't have full control of the link. One issue we are
> > running into is that the FW will start training when it first gets a
> > signal and it doesn't block the signal from getting to the PCS. The
> > PCS will see the signal and immediately report the link as "up" if the
> > quality is good enough. This results in us suddenly seeing the link
> > flapping for about 2-3 seconds while the training is happening. So to
> > prevent that from happening we are adding the phydev representing the
> > PMD to delay the link up by the needed 4 seconds to prevent the link
> > flap noise.
>
> So it seems like you need to extend dw_xpcs_compat with a .get_state
> callback. You can then have your own implementation which adds this 4
> second delay, before chaining into xpcs_get_state() to return the true
> state.

Seems like I might need to plug something into the spot where
xpcs_resolve_pma is called. The code as it stands now is just taking
the interface and using that to determine the speed after checking the
PCS for the link. I wonder if I should look at having this actually
look at the PMA if there is one and then just determine the speed
based on that.

[net-next PATCH v2 10/11] fbnic: Add phydev representing PMD to phylink setup

[Alexander Duyck]

From: Alexander Duyck <alexanderduyck@fb.com>

With this patch we add support for a phydev which represents the PMD state
to the phylink interface. As we now have this path we can separate the link
state from the PCS and instead report it through the phydev which allows us
to more easily transition to using the XPCS when the time comes.

Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
---
 drivers/net/ethernet/meta/Kconfig               |    1 +
 drivers/net/ethernet/meta/fbnic/fbnic_irq.c     |    2 +
 drivers/net/ethernet/meta/fbnic/fbnic_netdev.c  |   16 +++++++++
 drivers/net/ethernet/meta/fbnic/fbnic_netdev.h  |    1 +
 drivers/net/ethernet/meta/fbnic/fbnic_pci.c     |    1 +
 drivers/net/ethernet/meta/fbnic/fbnic_phylink.c |   40 +++++++++++++++++++++--
 6 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/meta/Kconfig b/drivers/net/ethernet/meta/Kconfig
index dff51f23d295..23676b530a83 100644
--- a/drivers/net/ethernet/meta/Kconfig
+++ b/drivers/net/ethernet/meta/Kconfig
@@ -24,6 +24,7 @@ config FBNIC
 	depends on MAX_SKB_FRAGS < 22
 	depends on PCI_MSI
 	depends on PTP_1588_CLOCK_OPTIONAL
+	select FBNIC_PHY
 	select NET_DEVLINK
 	select PAGE_POOL
 	select PHYLINK
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_irq.c b/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
index 45af6c1331fb..432b053b5ed6 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
@@ -140,7 +140,7 @@ static irqreturn_t fbnic_mac_msix_intr(int __always_unused irq, void *data)
 	/* Record link down events */
 	if (!fbd->mac->get_link(fbd, fbn->aui, fbn->fec)) {
 		fbn->link_down_events = link_down_events;
-		phylink_mac_change(fbn->phylink, false);
+		phy_mac_interrupt(fbd->netdev->phydev);
 	}
 
 	return IRQ_HANDLED;
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
index 65318a5b466e..51cf88b62927 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
@@ -101,6 +101,20 @@ static int fbnic_stop(struct net_device *netdev)
 	return 0;
 }
 
+static int fbnic_init(struct net_device *netdev)
+{
+	struct fbnic_net *fbn = netdev_priv(netdev);
+
+	return fbnic_phylink_connect(fbn);
+}
+
+static void fbnic_uninit(struct net_device *netdev)
+{
+	struct fbnic_net *fbn = netdev_priv(netdev);
+
+	phylink_disconnect_phy(fbn->phylink);
+}
+
 static int fbnic_uc_sync(struct net_device *netdev, const unsigned char *addr)
 {
 	struct fbnic_net *fbn = netdev_priv(netdev);
@@ -529,6 +543,8 @@ static int fbnic_bpf(struct net_device *netdev, struct netdev_bpf *bpf)
 static const struct net_device_ops fbnic_netdev_ops = {
 	.ndo_open		= fbnic_open,
 	.ndo_stop		= fbnic_stop,
+	.ndo_init		= fbnic_init,
+	.ndo_uninit		= fbnic_uninit,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_start_xmit		= fbnic_xmit_frame,
 	.ndo_features_check	= fbnic_features_check,
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
index 7b773c06e245..f8807f6e443d 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
@@ -107,6 +107,7 @@ int fbnic_phylink_ethtool_ksettings_get(struct net_device *netdev,
 int fbnic_phylink_get_fecparam(struct net_device *netdev,
 			       struct ethtool_fecparam *fecparam);
 int fbnic_phylink_init(struct net_device *netdev);
+int fbnic_phylink_connect(struct fbnic_net *fbn);
 void fbnic_phylink_pmd_training_complete_notify(struct fbnic_net *fbn);
 bool fbnic_check_split_frames(struct bpf_prog *prog,
 			      unsigned int mtu, u32 hds_threshold);
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_pci.c b/drivers/net/ethernet/meta/fbnic/fbnic_pci.c
index b3f05bdb4f52..befcb1e7747a 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_pci.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_pci.c
@@ -16,6 +16,7 @@
 char fbnic_driver_name[] = DRV_NAME;
 
 MODULE_DESCRIPTION(DRV_SUMMARY);
+MODULE_SOFTDEP("pre: fbnic_phy");
 MODULE_LICENSE("GPL");
 
 static const struct fbnic_info fbnic_asic_info = {
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
index e10fc08f22f2..59ee2fb32f91 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
@@ -132,9 +132,8 @@ fbnic_phylink_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
 
 	state->duplex = DUPLEX_FULL;
 
-	state->link = (fbd->pmd_state == FBNIC_PMD_SEND_DATA) &&
-		      (rd32(fbd, FBNIC_PCS(MDIO_STAT1, 0)) &
-		       MDIO_STAT1_LSTATUS);
+	state->link = !!(rd32(fbd, FBNIC_PCS(MDIO_STAT1, 0)) &
+			 MDIO_STAT1_LSTATUS);
 }
 
 static int
@@ -264,6 +263,39 @@ int fbnic_phylink_init(struct net_device *netdev)
 	return 0;
 }
 
+/**
+ * fbnic_phylink_connect - Connect phylink structure to IRQ and enable it
+ * @fbn: FBNIC Netdev private data struct phylink device attached to
+ *
+ * Return: zero on success, negative on failure
+ *
+ * This function connects the phylink structure to the IRQ and then enables it
+ * to resume operations. With this function completed the PHY will be able to
+ * obtain link and notify the netdev of its current state.
+ **/
+int fbnic_phylink_connect(struct fbnic_net *fbn)
+{
+	struct fbnic_dev *fbd = fbn->fbd;
+	struct phy_device *phydev;
+	int err;
+
+	phydev = mdiobus_get_phy(fbd->mdio_bus, 0);
+	if (!phydev) {
+		dev_err(fbd->dev, "No PHY found\n");
+		return -ENODEV;
+	}
+
+	/* We don't need to poll, the MAC will notify us of events */
+	phydev->irq = PHY_MAC_INTERRUPT;
+	phy_attached_info(phydev);
+
+	err = phylink_connect_phy(fbn->phylink, phydev);
+	if (err)
+		dev_err(fbd->dev, "Error connecting phy, err: %d\n", err);
+
+	return err;
+}
+
 /**
  * fbnic_phylink_pmd_training_complete_notify - PMD training complete notifier
  * @fbn: FBNIC Netdev private data struct phylink device attached to
@@ -285,5 +317,5 @@ void fbnic_phylink_pmd_training_complete_notify(struct fbnic_net *fbn)
 		return;
 
 	fbd->pmd_state = FBNIC_PMD_SEND_DATA;
-	phylink_mac_change(fbn->phylink, true);
+	phy_mac_interrupt(fbd->netdev->phydev);
 }

[net-next PATCH v2 11/11] fbnic: Replace use of internal PCS w/ Designware XPCS

[Alexander Duyck]

From: Alexander Duyck <alexanderduyck@fb.com>

As we have exposed the PCS registers via the SWMII we can now start looking
at connecting the XPCS driver to those registers and let it mange the PCS
instead of us doing it directly from the fbnic driver.

For now this just gets us the ability to detect link. The hop is in the
future to add some of the vendor specific registers to being enabling XPCS
configuration of the interface.

Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
---
 drivers/net/ethernet/meta/Kconfig               |    1 
 drivers/net/ethernet/meta/fbnic/fbnic_netdev.c  |    7 --
 drivers/net/ethernet/meta/fbnic/fbnic_netdev.h  |    5 +
 drivers/net/ethernet/meta/fbnic/fbnic_phylink.c |  101 +++++++++++------------
 4 files changed, 53 insertions(+), 61 deletions(-)

diff --git a/drivers/net/ethernet/meta/Kconfig b/drivers/net/ethernet/meta/Kconfig
index 23676b530a83..adb8ae29167a 100644
--- a/drivers/net/ethernet/meta/Kconfig
+++ b/drivers/net/ethernet/meta/Kconfig
@@ -27,6 +27,7 @@ config FBNIC
 	select FBNIC_PHY
 	select NET_DEVLINK
 	select PAGE_POOL
+	select PCS_XPCS
 	select PHYLINK
 	select PLDMFW
 	help
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
index 51cf88b62927..949901f51638 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
@@ -713,10 +713,7 @@ void fbnic_reset_queues(struct fbnic_net *fbn,
  **/
 void fbnic_netdev_free(struct fbnic_dev *fbd)
 {
-	struct fbnic_net *fbn = netdev_priv(fbd->netdev);
-
-	if (fbn->phylink)
-		phylink_destroy(fbn->phylink);
+	fbnic_phylink_destroy(fbd->netdev);
 
 	free_netdev(fbd->netdev);
 	fbd->netdev = NULL;
@@ -818,7 +815,7 @@ struct net_device *fbnic_netdev_alloc(struct fbnic_dev *fbd)
 
 	netif_tx_stop_all_queues(netdev);
 
-	if (fbnic_phylink_init(netdev)) {
+	if (fbnic_phylink_create(netdev)) {
 		fbnic_netdev_free(fbd);
 		return NULL;
 	}
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
index f8807f6e443d..8ac0e0c8ddf5 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
@@ -44,7 +44,7 @@ struct fbnic_net {
 
 	struct phylink *phylink;
 	struct phylink_config phylink_config;
-	struct phylink_pcs phylink_pcs;
+	struct phylink_pcs *pcs;
 
 	u8 aui;
 	u8 fec;
@@ -106,7 +106,8 @@ int fbnic_phylink_ethtool_ksettings_get(struct net_device *netdev,
 					struct ethtool_link_ksettings *cmd);
 int fbnic_phylink_get_fecparam(struct net_device *netdev,
 			       struct ethtool_fecparam *fecparam);
-int fbnic_phylink_init(struct net_device *netdev);
+int fbnic_phylink_create(struct net_device *netdev);
+void fbnic_phylink_destroy(struct net_device *netdev);
 int fbnic_phylink_connect(struct fbnic_net *fbn);
 void fbnic_phylink_pmd_training_complete_notify(struct fbnic_net *fbn);
 bool fbnic_check_split_frames(struct bpf_prog *prog,
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
index 59ee2fb32f91..28cd11e111e4 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) Meta Platforms, Inc. and affiliates. */
 
+#include <linux/pcs/pcs-xpcs.h>
 #include <linux/phy.h>
 #include <linux/phylink.h>
 
@@ -101,55 +102,6 @@ int fbnic_phylink_get_fecparam(struct net_device *netdev,
 	return 0;
 }
 
-static struct fbnic_net *
-fbnic_pcs_to_net(struct phylink_pcs *pcs)
-{
-	return container_of(pcs, struct fbnic_net, phylink_pcs);
-}
-
-static void
-fbnic_phylink_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
-			    struct phylink_link_state *state)
-{
-	struct fbnic_net *fbn = fbnic_pcs_to_net(pcs);
-	struct fbnic_dev *fbd = fbn->fbd;
-
-	switch (fbn->aui) {
-	case FBNIC_AUI_25GAUI:
-		state->speed = SPEED_25000;
-		break;
-	case FBNIC_AUI_LAUI2:
-	case FBNIC_AUI_50GAUI1:
-		state->speed = SPEED_50000;
-		break;
-	case FBNIC_AUI_100GAUI2:
-		state->speed = SPEED_100000;
-		break;
-	default:
-		state->link = 0;
-		return;
-	}
-
-	state->duplex = DUPLEX_FULL;
-
-	state->link = !!(rd32(fbd, FBNIC_PCS(MDIO_STAT1, 0)) &
-			 MDIO_STAT1_LSTATUS);
-}
-
-static int
-fbnic_phylink_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
-			 phy_interface_t interface,
-			 const unsigned long *advertising,
-			 bool permit_pause_to_mac)
-{
-	return 0;
-}
-
-static const struct phylink_pcs_ops fbnic_phylink_pcs_ops = {
-	.pcs_config = fbnic_phylink_pcs_config,
-	.pcs_get_state = fbnic_phylink_pcs_get_state,
-};
-
 static struct phylink_pcs *
 fbnic_phylink_mac_select_pcs(struct phylink_config *config,
 			     phy_interface_t interface)
@@ -157,7 +109,7 @@ fbnic_phylink_mac_select_pcs(struct phylink_config *config,
 	struct net_device *netdev = to_net_dev(config->dev);
 	struct fbnic_net *fbn = netdev_priv(netdev);
 
-	return &fbn->phylink_pcs;
+	return fbn->pcs;
 }
 
 static int
@@ -226,16 +178,35 @@ static const struct phylink_mac_ops fbnic_phylink_mac_ops = {
 	.mac_link_up = fbnic_phylink_mac_link_up,
 };
 
-int fbnic_phylink_init(struct net_device *netdev)
+/**
+ * fbnic_phylink_create - Phylink device creation
+ * @netdev: Network Device struct to attach phylink device
+ *
+ * Initialize and attach a phylink instance to the device. The phylink
+ * device will make use of the netdev struct to track carrier and will
+ * eventually be used to expose the current state of the MAC and PCS
+ * setup.
+ *
+ * Return: 0 on success, negative on failure
+ **/
+int fbnic_phylink_create(struct net_device *netdev)
 {
 	struct fbnic_net *fbn = netdev_priv(netdev);
 	struct fbnic_dev *fbd = fbn->fbd;
+	struct phylink_pcs *pcs;
 	struct phylink *phylink;
+	int err;
 
-	fbn->phylink_pcs.ops = &fbnic_phylink_pcs_ops;
+	pcs = xpcs_create_pcs_mdiodev(fbd->mdio_bus, 0);
+	if (IS_ERR(pcs)) {
+		err = PTR_ERR(pcs);
+		dev_err(fbd->dev, "Failed to create PCS device: %d\n", err);
+		return err;
+	}
 
 	fbn->phylink_config.dev = &netdev->dev;
 	fbn->phylink_config.type = PHYLINK_NETDEV;
+
 	fbn->phylink_config.mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE |
 					       MAC_25000FD | MAC_50000FD |
 					       MAC_100000FD;
@@ -255,10 +226,16 @@ int fbnic_phylink_init(struct net_device *netdev)
 	phylink = phylink_create(&fbn->phylink_config, NULL,
 				 fbnic_phylink_select_interface(fbn->aui),
 				 &fbnic_phylink_mac_ops);
-	if (IS_ERR(phylink))
-		return PTR_ERR(phylink);
+	if (IS_ERR(phylink)) {
+		err = PTR_ERR(phylink);
+		dev_err(netdev->dev.parent,
+			"Failed to create Phylink interface, err: %d\n", err);
+		xpcs_destroy_pcs(pcs);
+		return err;
+	}
 
 	fbn->phylink = phylink;
+	fbn->pcs = pcs;
 
 	return 0;
 }
@@ -296,6 +273,22 @@ int fbnic_phylink_connect(struct fbnic_net *fbn)
 	return err;
 }
 
+/**
+ * fbnic_phylink_destroy - Teardown phylink related interfaces
+ * @netdev: Network Device struct containing phylink device
+ *
+ * Detach and free resources related to phylink interface.
+ **/
+void fbnic_phylink_destroy(struct net_device *netdev)
+{
+	struct fbnic_net *fbn = netdev_priv(netdev);
+
+	if (fbn->phylink)
+		phylink_destroy(fbn->phylink);
+	if (fbn->pcs)
+		xpcs_destroy_pcs(fbn->pcs);
+}
+
 /**
  * fbnic_phylink_pmd_training_complete_notify - PMD training complete notifier
  * @fbn: FBNIC Netdev private data struct phylink device attached to