* [net-next PATCH 0/8] net: phy: Add support for fbnic PHY w/ 25G, 50G, and 100G support
@ 2025-10-24 20:40 Alexander Duyck
2025-10-24 20:40 ` [net-next PATCH 1/8] net: phy: Add support for 25, 50 and 100Gbps PMA to genphy_c45_read_pma Alexander Duyck
` (7 more replies)
0 siblings, 8 replies; 25+ messages in thread
From: Alexander Duyck @ 2025-10-24 20:40 UTC (permalink / raw)
To: netdev; +Cc: kuba, kernel-team, andrew+netdev, hkallweit1, linux, pabeni,
davem
To enable eventually transitioning the fbnic driver to using the XPCS
driver we first need to address the fact that we need a representation for
the FW managed PMA/PMD that is actually a SerDes PHY to handle link
bouncing during link training.
To enable that this patch set first introduces the necessary bits to the
generic c45 driver to enable it to read 25G, 50G, and 100G modes from the
PHY. One complication to this though is the fact that 50GBase-CR2 doesn't
exist in the IEEE version of the specification. For now I am taking the
approach that the PMA can show a speed of 50G in the CTRL1 register and set
the PMA_CTRL2 register for 100GBase-CR4 as I am using that as an alias for
the 50R2 due to the fact that the media used is 100R4, it is just shared
between 2 instaces the 50G interface as per the Consortium specification. I
am still open to suggestions on other ways to implement this as it isn't
set in stone and changing it would be pretty straight forward since the
only thing currently impementing this is a SW based MII interface.
Otherwise for 25R1, 50R1, and 100R2 the interface matches the IEEE
specification.
The rest of this patch set enables the changes to fbnic to make use of this
interface 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.
---
Alexander Duyck (8):
net: phy: Add support for 25, 50 and 100Gbps PMA to genphy_c45_read_pma
net: phy: Avoid reusing val in genphy_c45_pma_read_ext_abilities
net: phy: Add 25G-CR, 50G-CR, 100G-CR2 support to C45 genphy
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 MII interface to PMA/PMD
fbnic: Add phydev representing PMD to phylink setup
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 | 69 +++++---
drivers/net/ethernet/meta/fbnic/fbnic_mac.h | 39 +++--
.../net/ethernet/meta/fbnic/fbnic_netdev.c | 7 +-
.../net/ethernet/meta/fbnic/fbnic_netdev.h | 3 +-
drivers/net/ethernet/meta/fbnic/fbnic_pci.c | 7 +
.../net/ethernet/meta/fbnic/fbnic_phylink.c | 121 ++++++++++---
drivers/net/ethernet/meta/fbnic/fbnic_swmii.c | 145 ++++++++++++++++
drivers/net/phy/phy-c45.c | 164 ++++++++++++++++--
include/uapi/linux/mdio.h | 43 ++++-
14 files changed, 568 insertions(+), 99 deletions(-)
create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_swmii.c
--
^ permalink raw reply [flat|nested] 25+ messages in thread
* [net-next PATCH 1/8] net: phy: Add support for 25, 50 and 100Gbps PMA to genphy_c45_read_pma
2025-10-24 20:40 [net-next PATCH 0/8] net: phy: Add support for fbnic PHY w/ 25G, 50G, and 100G support Alexander Duyck
@ 2025-10-24 20:40 ` Alexander Duyck
2025-10-28 3:12 ` Andrew Lunn
2025-10-24 20:40 ` [net-next PATCH 2/8] net: phy: Avoid reusing val in genphy_c45_pma_read_ext_abilities Alexander Duyck
` (6 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Alexander Duyck @ 2025-10-24 20:40 UTC (permalink / raw)
To: netdev; +Cc: kuba, kernel-team, andrew+netdev, hkallweit1, linux, pabeni,
davem
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..ff8b6423bd1e 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 0x0800 /* 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 */
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [net-next PATCH 2/8] net: phy: Avoid reusing val in genphy_c45_pma_read_ext_abilities
2025-10-24 20:40 [net-next PATCH 0/8] net: phy: Add support for fbnic PHY w/ 25G, 50G, and 100G support Alexander Duyck
2025-10-24 20:40 ` [net-next PATCH 1/8] net: phy: Add support for 25, 50 and 100Gbps PMA to genphy_c45_read_pma Alexander Duyck
@ 2025-10-24 20:40 ` Alexander Duyck
2025-10-24 20:40 ` [net-next PATCH 3/8] net: phy: Add 25G-CR, 50G-CR, 100G-CR2 support to C45 genphy Alexander Duyck
` (5 subsequent siblings)
7 siblings, 0 replies; 25+ messages in thread
From: Alexander Duyck @ 2025-10-24 20:40 UTC (permalink / raw)
To: netdev; +Cc: kuba, kernel-team, andrew+netdev, hkallweit1, linux, pabeni,
davem
From: Alexander Duyck <alexanderduyck@fb.com>
In planning to add support for additional capabilities I realized we have a
bit of an issue in the way that genphy_c45_pma_read_ext_abilities is setup.
It is reusing val for both an error return and the tracking of the PMA
extended abilities register. As such if any one ability is enabled it will
end up overwriting it with 0 and block the checks for other abilities.
Rather than do that I am updating the code to contain the individual
ability register checks into separate functions and adding an err value to
handle the returns of those calls.
Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
---
drivers/net/phy/phy-c45.c | 43 +++++++++++++++++++++++++++----------------
1 file changed, 27 insertions(+), 16 deletions(-)
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 2b178a789941..4210863c1b6e 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -958,6 +958,26 @@ int genphy_c45_an_config_eee_aneg(struct phy_device *phydev)
}
EXPORT_SYMBOL_GPL(genphy_c45_an_config_eee_aneg);
+static int genphy_c45_pma_ng_read_abilities(struct phy_device *phydev)
+{
+ int val;
+
+ val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD,
+ MDIO_PMA_NG_EXTABLE);
+ if (val < 0)
+ return val;
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
+ phydev->supported,
+ val & MDIO_PMA_NG_EXTABLE_2_5GBT);
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
+ phydev->supported,
+ val & MDIO_PMA_NG_EXTABLE_5GBT);
+
+ return 0;
+}
+
/**
* genphy_c45_pma_baset1_read_abilities - read supported baset1 link modes from PMA
* @phydev: target phy_device struct
@@ -1005,7 +1025,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_pma_baset1_read_abilities);
*/
int genphy_c45_pma_read_ext_abilities(struct phy_device *phydev)
{
- int val;
+ int val, err;
val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_EXTABLE);
if (val < 0)
@@ -1045,24 +1065,15 @@ int genphy_c45_pma_read_ext_abilities(struct phy_device *phydev)
val & MDIO_PMA_EXTABLE_10BT);
if (val & MDIO_PMA_EXTABLE_NBT) {
- val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD,
- MDIO_PMA_NG_EXTABLE);
- if (val < 0)
- return val;
-
- linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
- phydev->supported,
- val & MDIO_PMA_NG_EXTABLE_2_5GBT);
-
- linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
- phydev->supported,
- val & MDIO_PMA_NG_EXTABLE_5GBT);
+ err = genphy_c45_pma_ng_read_abilities(phydev);
+ if (err < 0)
+ return err;
}
if (val & MDIO_PMA_EXTABLE_BT1) {
- val = genphy_c45_pma_baset1_read_abilities(phydev);
- if (val < 0)
- return val;
+ err = genphy_c45_pma_baset1_read_abilities(phydev);
+ if (err < 0)
+ return err;
}
return 0;
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [net-next PATCH 3/8] net: phy: Add 25G-CR, 50G-CR, 100G-CR2 support to C45 genphy
2025-10-24 20:40 [net-next PATCH 0/8] net: phy: Add support for fbnic PHY w/ 25G, 50G, and 100G support Alexander Duyck
2025-10-24 20:40 ` [net-next PATCH 1/8] net: phy: Add support for 25, 50 and 100Gbps PMA to genphy_c45_read_pma Alexander Duyck
2025-10-24 20:40 ` [net-next PATCH 2/8] net: phy: Avoid reusing val in genphy_c45_pma_read_ext_abilities Alexander Duyck
@ 2025-10-24 20:40 ` Alexander Duyck
2025-10-28 7:32 ` Maxime Chevallier
` (2 more replies)
2025-10-24 20:41 ` [net-next PATCH 4/8] fbnic: Rename PCS IRQ to MAC IRQ as it is actually a MAC interrupt Alexander Duyck
` (4 subsequent siblings)
7 siblings, 3 replies; 25+ messages in thread
From: Alexander Duyck @ 2025-10-24 20:40 UTC (permalink / raw)
To: netdev; +Cc: kuba, kernel-team, andrew+netdev, hkallweit1, linux, pabeni,
davem
From: Alexander Duyck <alexanderduyck@fb.com>
Add support for 25G-CR, 50G-CR, 50G-CR2, and 100G-CR2 the c45 genphy. Note
that 3 of the 4 are IEEE compliant so they are a direct copy from the
clause 45 specification, the only exception to this is 50G-CR2 which is
part of the Ethernet Consortium specification which never referenced how to
handle this in the MDIO registers.
Since 50GBase-CR2 isn't an IEEE standard it doesn't have a value in the
extended capabilities registers. To account for that I am adding a define
that is aliasing the 100GBase-CR4 to represent it as that is the media type
used to carry data for 50R2, it is just that the PHY is carrying two 2 with
2 lanes each over the 4 lane cable. For now I am representing it with ctrl1
set to 50G and ctrl2 being set to 100R4, and using the 100R4 capability to
identify if it is supported or not.
Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
---
drivers/net/phy/phy-c45.c | 114 +++++++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/mdio.h | 34 +++++++++++++
2 files changed, 147 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 4210863c1b6e..65d6f45c898d 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -161,6 +161,38 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev)
/* Assume 10Gbase-T */
ctrl2 |= MDIO_PMA_CTRL2_10GBT;
break;
+ case SPEED_25000:
+ ctrl1 |= MDIO_CTRL1_SPEED25G;
+ /* Assume 25Gbase-CR */
+ ctrl2 |= MDIO_PMA_CTRL2_25GBCR;
+ break;
+ case SPEED_50000:
+ ctrl1 |= MDIO_CTRL1_SPEED50G;
+ /* There are currently 2 supported modes for 50G.
+ * The first is 50Gbase-CR which is in the IEEE standard.
+ * The second is 50Gbase-CR2 which isn't. It is intended
+ * to piggy-back on 100Gbase-CR4 and only use 2 lanes, so
+ * we can use the interface type to identify which is which.
+ */
+ if (phydev->interface == PHY_INTERFACE_MODE_50GBASER)
+ ctrl2 |= MDIO_PMA_CTRL2_50GBCR;
+ else if (phydev->interface == PHY_INTERFACE_MODE_LAUI)
+ ctrl2 |= MDIO_PMA_CTRL2_50GBCR2;
+ else
+ return -EINVAL;
+ break;
+ case SPEED_100000:
+ ctrl1 |= MDIO_CTRL1_SPEED100G;
+ /* For now we only have support for 2 lane devices, so
+ * 100Gbase-CR2 is the limit. We might look at enabling
+ * 100Gbase-CR4 in the future with additional devices to
+ * test this code on.
+ */
+ if (phydev->interface == PHY_INTERFACE_MODE_100GBASEP)
+ ctrl2 |= MDIO_PMA_CTRL2_100GBCR2;
+ else
+ return -EINVAL;
+ break;
default:
return -EINVAL;
}
@@ -958,6 +990,34 @@ int genphy_c45_an_config_eee_aneg(struct phy_device *phydev)
}
EXPORT_SYMBOL_GPL(genphy_c45_an_config_eee_aneg);
+static int genphy_c45_pma_40_100g_read_abilities(struct phy_device *phydev)
+{
+ int val;
+
+ val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD,
+ MDIO_PMA_40G_EXTABLE);
+ if (val < 0)
+ return val;
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT,
+ phydev->supported,
+ val & MDIO_PMA_40G_EXTABLE_100GBCR4);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_50000baseCR2_Full_BIT,
+ phydev->supported,
+ val & MDIO_PMA_40G_EXTABLE_50GBCR2);
+
+ val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD,
+ MDIO_PMA_100G_EXTABLE);
+ if (val < 0)
+ return val;
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_100000baseCR2_Full_BIT,
+ phydev->supported,
+ val & MDIO_PMA_100G_EXTABLE_100GBCR2);
+
+ return 0;
+}
+
static int genphy_c45_pma_ng_read_abilities(struct phy_device *phydev)
{
int val;
@@ -978,6 +1038,22 @@ static int genphy_c45_pma_ng_read_abilities(struct phy_device *phydev)
return 0;
}
+static int genphy_c45_pma_25g_read_abilities(struct phy_device *phydev)
+{
+ int val;
+
+ val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD,
+ MDIO_PMA_25G_EXTABLE);
+ if (val < 0)
+ return val;
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_25000baseCR_Full_BIT,
+ phydev->supported,
+ val & MDIO_PMA_25G_EXTABLE_25GBCR);
+
+ return 0;
+}
+
/**
* genphy_c45_pma_baset1_read_abilities - read supported baset1 link modes from PMA
* @phydev: target phy_device struct
@@ -1016,6 +1092,22 @@ int genphy_c45_pma_baset1_read_abilities(struct phy_device *phydev)
}
EXPORT_SYMBOL_GPL(genphy_c45_pma_baset1_read_abilities);
+static int genphy_c45_pma_50g_read_abilities(struct phy_device *phydev)
+{
+ int val;
+
+ val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD,
+ MDIO_PMA_50G_EXTABLE);
+ if (val < 0)
+ return val;
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_50000baseCR_Full_BIT,
+ phydev->supported,
+ val & MDIO_PMA_50G_EXTABLE_50GBCR);
+
+ return 0;
+}
+
/**
* genphy_c45_pma_read_ext_abilities - read supported link modes from PMA
* @phydev: target phy_device struct
@@ -1064,18 +1156,40 @@ int genphy_c45_pma_read_ext_abilities(struct phy_device *phydev)
phydev->supported,
val & MDIO_PMA_EXTABLE_10BT);
+ if (val & MDIO_PMA_EXTABLE_40_100G) {
+ err = genphy_c45_pma_40_100g_read_abilities(phydev);
+ if (err < 0)
+ return err;
+ }
+
if (val & MDIO_PMA_EXTABLE_NBT) {
err = genphy_c45_pma_ng_read_abilities(phydev);
if (err < 0)
return err;
}
+ if (val & MDIO_PMA_EXTABLE_25G) {
+ err = genphy_c45_pma_25g_read_abilities(phydev);
+ if (err < 0)
+ return err;
+ }
+
if (val & MDIO_PMA_EXTABLE_BT1) {
err = genphy_c45_pma_baset1_read_abilities(phydev);
if (err < 0)
return err;
}
+ val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_EXTABLE2);
+ if (val < 0)
+ return val;
+
+ if (val & MDIO_PMA_EXTABLE2_50G) {
+ err = genphy_c45_pma_50g_read_abilities(phydev);
+ if (err < 0)
+ return err;
+ }
+
return 0;
}
EXPORT_SYMBOL_GPL(genphy_c45_pma_read_ext_abilities);
diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
index ff8b6423bd1e..8f9a4328daf9 100644
--- a/include/uapi/linux/mdio.h
+++ b/include/uapi/linux/mdio.h
@@ -41,15 +41,20 @@
#define MDIO_PMA_TXDIS 9 /* 10G PMA/PMD transmit disable */
#define MDIO_PMA_RXDET 10 /* 10G PMA/PMD receive signal detect */
#define MDIO_PMA_EXTABLE 11 /* 10G PMA/PMD extended ability */
+#define MDIO_PMA_40G_EXTABLE 13 /* 40G/100G PMA/PMD extended ability */
#define MDIO_PKGID1 14 /* Package identifier */
#define MDIO_PKGID2 15
#define MDIO_AN_ADVERTISE 16 /* AN advertising (base page) */
#define MDIO_AN_LPA 19 /* AN LP abilities (base page) */
+#define MDIO_PMA_25G_EXTABLE 19 /* 25G PMA/PMD extended ability */
#define MDIO_PCS_EEE_ABLE 20 /* EEE Capability register */
+#define MDIO_PMA_50G_EXTABLE 20 /* 50G PMA/PMD extended ability */
#define MDIO_PCS_EEE_ABLE2 21 /* EEE Capability register 2 */
#define MDIO_PMA_NG_EXTABLE 21 /* 2.5G/5G PMA/PMD extended ability */
#define MDIO_PCS_EEE_WK_ERR 22 /* EEE wake error counter */
#define MDIO_PHYXS_LNSTAT 24 /* PHY XGXS lane state */
+#define MDIO_PMA_EXTABLE2 25 /* PMA/PMD extended ability 2 */
+#define MDIO_PMA_100G_EXTABLE 26 /* 40G/100G PMA/PMD extended ability 2 */
#define MDIO_AN_EEE_ADV 60 /* EEE advertisement */
#define MDIO_AN_EEE_LPABLE 61 /* EEE link partner ability */
#define MDIO_AN_EEE_ADV2 62 /* EEE advertisement 2 */
@@ -187,9 +192,18 @@
#define MDIO_PMA_CTRL2_1000BKX 0x000d /* 1000BASE-KX type */
#define MDIO_PMA_CTRL2_100BTX 0x000e /* 100BASE-TX type */
#define MDIO_PMA_CTRL2_10BT 0x000f /* 10BASE-T type */
+#define MDIO_PMA_CTRL2_100GBCR4 0x002e /* 100GBASE-CR4 type */
+/* 50GBase-CR2 isn't an IEEE media type, as such there isn't a defined
+ * value for it. However as it is meant to be a reuse of 100GBase-CR4 we
+ * will reuse the value here so that both report the same value.
+ */
+#define MDIO_PMA_CTRL2_50GBCR2 MDIO_PMA_CTRL2_100GBCR4
#define MDIO_PMA_CTRL2_2_5GBT 0x0030 /* 2.5GBaseT type */
#define MDIO_PMA_CTRL2_5GBT 0x0031 /* 5GBaseT type */
+#define MDIO_PMA_CTRL2_25GBCR 0x0038 /* 25GBASE-CR type */
#define MDIO_PMA_CTRL2_BASET1 0x003D /* BASE-T1 type */
+#define MDIO_PMA_CTRL2_50GBCR 0x0041 /* 50GBASE-CR type */
+#define MDIO_PMA_CTRL2_100GBCR2 0x0049 /* 100GBASE-CR2 type */
#define MDIO_PCS_CTRL2_TYPE 0x0003 /* PCS type selection */
#define MDIO_PCS_CTRL2_10GBR 0x0000 /* 10GBASE-R type */
#define MDIO_PCS_CTRL2_10GBX 0x0001 /* 10GBASE-X type */
@@ -233,7 +247,7 @@
#define MDIO_PMD_RXDET_2 0x0008 /* PMD RX signal detect 2 */
#define MDIO_PMD_RXDET_3 0x0010 /* PMD RX signal detect 3 */
-/* Extended abilities register. */
+/* PMA/PMD extended ability register. */
#define MDIO_PMA_EXTABLE_10GCX4 0x0001 /* 10GBASE-CX4 ability */
#define MDIO_PMA_EXTABLE_10GBLRM 0x0002 /* 10GBASE-LRM ability */
#define MDIO_PMA_EXTABLE_10GBT 0x0004 /* 10GBASE-T ability */
@@ -243,9 +257,14 @@
#define MDIO_PMA_EXTABLE_1000BKX 0x0040 /* 1000BASE-KX ability */
#define MDIO_PMA_EXTABLE_100BTX 0x0080 /* 100BASE-TX ability */
#define MDIO_PMA_EXTABLE_10BT 0x0100 /* 10BASE-T ability */
+#define MDIO_PMA_EXTABLE_40_100G 0x0400 /* 40G/100G ability */
#define MDIO_PMA_EXTABLE_BT1 0x0800 /* BASE-T1 ability */
+#define MDIO_PMA_EXTABLE_25G 0x1000 /* 25G ability */
#define MDIO_PMA_EXTABLE_NBT 0x4000 /* 2.5/5GBASE-T ability */
+/* PMA/PMD extended ability 2 register. */
+#define MDIO_PMA_EXTABLE2_50G 0x0001 /* 50G ability */
+
/* AN Clause 73 linkword */
#define MDIO_AN_C73_0_S_MASK GENMASK(4, 0)
#define MDIO_AN_C73_0_E_MASK GENMASK(9, 5)
@@ -436,10 +455,23 @@
/* AN MultiGBASE-T AN control 2 */
#define MDIO_AN_THP_BP2_5GT 0x0008 /* 2.5GT THP bypass request */
+/* 40G/100G PMA/PMD Extended ability register */
+#define MDIO_PMA_40G_EXTABLE_100GBCR4 0x4000 /* 100GBASE-CR4 ability */
+#define MDIO_PMA_40G_EXTABLE_50GBCR2 MDIO_PMA_40G_EXTABLE_100GBCR4
+
+/* 25G PMA/PMD Extended ability register */
+#define MDIO_PMA_25G_EXTABLE_25GBCR 0x0008 /* 25GBASE-CR ability */
+
+/* 50G PMA/PMD Extended ability register */
+#define MDIO_PMA_50G_EXTABLE_50GBCR 0x0002 /* 50GBASE-CR ability */
+
/* 2.5G/5G Extended abilities register. */
#define MDIO_PMA_NG_EXTABLE_2_5GBT 0x0001 /* 2.5GBASET ability */
#define MDIO_PMA_NG_EXTABLE_5GBT 0x0002 /* 5GBASET ability */
+/* 40G/100G PMA/PMD Extended ability 2 register */
+#define MDIO_PMA_100G_EXTABLE_100GBCR2 0x0100 /* 100GBASE-CR2 ability */
+
/* LASI RX_ALARM control/status registers. */
#define MDIO_PMA_LASI_RX_PHYXSLFLT 0x0001 /* PHY XS RX local fault */
#define MDIO_PMA_LASI_RX_PCSLFLT 0x0008 /* PCS RX local fault */
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [net-next PATCH 4/8] fbnic: Rename PCS IRQ to MAC IRQ as it is actually a MAC interrupt
2025-10-24 20:40 [net-next PATCH 0/8] net: phy: Add support for fbnic PHY w/ 25G, 50G, and 100G support Alexander Duyck
` (2 preceding siblings ...)
2025-10-24 20:40 ` [net-next PATCH 3/8] net: phy: Add 25G-CR, 50G-CR, 100G-CR2 support to C45 genphy Alexander Duyck
@ 2025-10-24 20:41 ` Alexander Duyck
2025-10-24 20:41 ` [net-next PATCH 5/8] fbnic: Add logic to track PMD state via MAC/PCS signals Alexander Duyck
` (3 subsequent siblings)
7 siblings, 0 replies; 25+ messages in thread
From: Alexander Duyck @ 2025-10-24 20:41 UTC (permalink / raw)
To: netdev; +Cc: kuba, kernel-team, andrew+netdev, hkallweit1, linux, pabeni,
davem
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 exposting 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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [net-next PATCH 5/8] fbnic: Add logic to track PMD state via MAC/PCS signals
2025-10-24 20:40 [net-next PATCH 0/8] net: phy: Add support for fbnic PHY w/ 25G, 50G, and 100G support Alexander Duyck
` (3 preceding siblings ...)
2025-10-24 20:41 ` [net-next PATCH 4/8] fbnic: Rename PCS IRQ to MAC IRQ as it is actually a MAC interrupt Alexander Duyck
@ 2025-10-24 20:41 ` Alexander Duyck
2025-10-24 20:41 ` [net-next PATCH 6/8] fbnic: Cleanup handling for link down event statistics Alexander Duyck
` (2 subsequent siblings)
7 siblings, 0 replies; 25+ messages in thread
From: Alexander Duyck @ 2025-10-24 20:41 UTC (permalink / raw)
To: netdev; +Cc: kuba, kernel-team, andrew+netdev, hkallweit1, linux, pabeni,
davem
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 | 57 +++++++++++------
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, 134 insertions(+), 51 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..08db58368911 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_pcs_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_pcs_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,11 @@ 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)
{
/* 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);
}
static void fbnic_mac_link_down_asic(struct fbnic_dev *fbd)
@@ -867,10 +887,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..f831eba02730 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);
+ * 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);
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..8d32a12c8efa 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);
+
+ 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);
+}
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [net-next PATCH 6/8] fbnic: Cleanup handling for link down event statistics
2025-10-24 20:40 [net-next PATCH 0/8] net: phy: Add support for fbnic PHY w/ 25G, 50G, and 100G support Alexander Duyck
` (4 preceding siblings ...)
2025-10-24 20:41 ` [net-next PATCH 5/8] fbnic: Add logic to track PMD state via MAC/PCS signals Alexander Duyck
@ 2025-10-24 20:41 ` Alexander Duyck
2025-10-24 20:41 ` [net-next PATCH 7/8] fbnic: Add SW shim for MII interface to PMA/PMD Alexander Duyck
2025-10-24 20:41 ` [net-next PATCH 8/8] fbnic: Add phydev representing PMD to phylink setup Alexander Duyck
7 siblings, 0 replies; 25+ messages in thread
From: Alexander Duyck @ 2025-10-24 20:41 UTC (permalink / raw)
To: netdev; +Cc: kuba, kernel-team, andrew+netdev, hkallweit1, linux, pabeni,
davem
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_phylink.c | 2 --
3 files changed, 18 insertions(+), 3 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_phylink.c b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
index 8d32a12c8efa..a9b2fc8108b7 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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [net-next PATCH 7/8] fbnic: Add SW shim for MII interface to PMA/PMD
2025-10-24 20:40 [net-next PATCH 0/8] net: phy: Add support for fbnic PHY w/ 25G, 50G, and 100G support Alexander Duyck
` (5 preceding siblings ...)
2025-10-24 20:41 ` [net-next PATCH 6/8] fbnic: Cleanup handling for link down event statistics Alexander Duyck
@ 2025-10-24 20:41 ` Alexander Duyck
2025-10-28 0:43 ` Jakub Kicinski
` (2 more replies)
2025-10-24 20:41 ` [net-next PATCH 8/8] fbnic: Add phydev representing PMD to phylink setup Alexander Duyck
7 siblings, 3 replies; 25+ messages in thread
From: Alexander Duyck @ 2025-10-24 20:41 UTC (permalink / raw)
To: netdev; +Cc: kuba, kernel-team, andrew+netdev, hkallweit1, linux, pabeni,
davem
From: Alexander Duyck <alexanderduyck@fb.com>
In order for us to support a phydev device we need to add an MII bus to
allow the phydev driver to have access to the registers for the device.
This change adds such an interface, currently as a read only interface for
a single PHY.
The plan is in the future to extend out this interface adding RSFEC support
to the PMA, and eventually adding PCS register access through a remapping
of our CSRs which will essentialy 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_pci.c | 3 +
drivers/net/ethernet/meta/fbnic/fbnic_swmii.c | 145 +++++++++++++++++++++++++
4 files changed, 154 insertions(+)
create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_swmii.c
diff --git a/drivers/net/ethernet/meta/fbnic/Makefile b/drivers/net/ethernet/meta/fbnic/Makefile
index 15e8ff649615..b15616c3523c 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_swmii.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..4a77ea12ddec 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;
+
+ /* SW MII bus for FW PHY */
+ struct mii_bus *mii_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_swmii_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_pci.c b/drivers/net/ethernet/meta/fbnic/fbnic_pci.c
index 428fc861deff..a5390996393c 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_swmii_create(fbd))
+ goto init_failure_mode;
+
netdev = fbnic_netdev_alloc(fbd);
if (!netdev) {
dev_err(&pdev->dev, "Netdev allocation failed\n");
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_swmii.c b/drivers/net/ethernet/meta/fbnic/fbnic_swmii.c
new file mode 100644
index 000000000000..7698fb60f660
--- /dev/null
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_swmii.c
@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) Meta Platforms, Inc. and affiliates. */
+
+#include <linux/mdio.h>
+
+#include "fbnic.h"
+#include "fbnic_netdev.h"
+
+static int
+fbnic_swmii_read_pmapmd(struct fbnic_dev *fbd, int regnum)
+{
+ u16 ctrl1 = 0, ctrl2 = 0;
+ struct fbnic_net *fbn;
+ int ret = 0;
+ u8 aui;
+
+ if (fbd->netdev) {
+ fbn = netdev_priv(fbd->netdev);
+ aui = fbn->aui;
+ }
+
+ switch (aui) {
+ case FBNIC_AUI_25GAUI:
+ ctrl1 = MDIO_CTRL1_SPEED25G;
+ ctrl2 = MDIO_PMA_CTRL2_25GBCR;
+ break;
+ case FBNIC_AUI_LAUI2:
+ ctrl1 = MDIO_CTRL1_SPEED50G;
+ ctrl2 = MDIO_PMA_CTRL2_50GBCR2;
+ break;
+ case FBNIC_AUI_50GAUI1:
+ ctrl1 = MDIO_CTRL1_SPEED50G;
+ ctrl2 = MDIO_PMA_CTRL2_50GBCR;
+ break;
+ case FBNIC_AUI_100GAUI2:
+ ctrl1 = MDIO_CTRL1_SPEED100G;
+ ctrl2 = MDIO_PMA_CTRL2_100GBCR2;
+ break;
+ default:
+ break;
+ }
+
+ switch (regnum) {
+ case MDIO_CTRL1:
+ ret = ctrl1;
+ break;
+ case MDIO_STAT1:
+ ret = fbd->pmd_state == FBNIC_PMD_SEND_DATA ?
+ MDIO_STAT1_LSTATUS : 0;
+ break;
+ case MDIO_DEVS1:
+ ret = MDIO_DEVS_PMAPMD;
+ break;
+ case MDIO_CTRL2:
+ ret = ctrl2;
+ break;
+ case MDIO_STAT2:
+ ret = MDIO_STAT2_DEVPRST_VAL |
+ MDIO_PMA_STAT2_EXTABLE;
+ break;
+ case MDIO_PMA_EXTABLE:
+ ret = MDIO_PMA_EXTABLE_40_100G |
+ MDIO_PMA_EXTABLE_25G;
+ break;
+ case MDIO_PMA_40G_EXTABLE:
+ ret = MDIO_PMA_40G_EXTABLE_50GBCR2;
+ break;
+ case MDIO_PMA_25G_EXTABLE:
+ ret = MDIO_PMA_25G_EXTABLE_25GBCR;
+ break;
+ case MDIO_PMA_50G_EXTABLE:
+ ret = MDIO_PMA_50G_EXTABLE_50GBCR;
+ break;
+ case MDIO_PMA_EXTABLE2:
+ ret = MDIO_PMA_EXTABLE2_50G;
+ break;
+ case MDIO_PMA_100G_EXTABLE:
+ ret = MDIO_PMA_100G_EXTABLE_100GBCR2;
+ break;
+ default:
+ break;
+ }
+
+ return ret;
+}
+
+static int
+fbnic_swmii_read_c45(struct mii_bus *bus, int addr, int devnum, int regnum)
+{
+ struct fbnic_dev *fbd = bus->priv;
+
+ if (addr != 0)
+ return 0xffff;
+
+ if (devnum == MDIO_MMD_PMAPMD)
+ return fbnic_swmii_read_pmapmd(fbd, regnum);
+
+ return 0xffff;
+}
+
+static int
+fbnic_swmii_write_c45(struct mii_bus *bus, int addr, int devnum,
+ int regnum, u16 val)
+{
+ /* Currently PHY setup is meant to be read-only */
+ return 0;
+}
+
+/**
+ * fbnic_swmii_create - Create a swmii to allow interfacing phydev w/ FW PHY
+ * @fbd: Pointer to FBNIC device structure to populate bus on
+ *
+ * Initialize an MII bus and place a pointer to it on the fbd struct. This bus
+ * will be used to interface with the PMA/PMD for now, and may add support for
+ * the PCS in the future.
+ *
+ * Return: 0 on success, negative on failure
+ **/
+int fbnic_swmii_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_swmii_read_c45;
+ bus->write_c45 = &fbnic_swmii_write_c45;
+ bus->parent = fbd->dev;
+ bus->phy_mask = GENMASK(31, 1);
+ 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->mii_bus = bus;
+
+ return 0;
+}
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [net-next PATCH 8/8] fbnic: Add phydev representing PMD to phylink setup
2025-10-24 20:40 [net-next PATCH 0/8] net: phy: Add support for fbnic PHY w/ 25G, 50G, and 100G support Alexander Duyck
` (6 preceding siblings ...)
2025-10-24 20:41 ` [net-next PATCH 7/8] fbnic: Add SW shim for MII interface to PMA/PMD Alexander Duyck
@ 2025-10-24 20:41 ` Alexander Duyck
2025-10-28 21:02 ` Andrew Lunn
7 siblings, 1 reply; 25+ messages in thread
From: Alexander Duyck @ 2025-10-24 20:41 UTC (permalink / raw)
To: netdev; +Cc: kuba, kernel-team, andrew+netdev, hkallweit1, linux, pabeni,
davem
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/fbnic/fbnic_irq.c | 2 -
drivers/net/ethernet/meta/fbnic/fbnic_netdev.c | 7 +--
drivers/net/ethernet/meta/fbnic/fbnic_netdev.h | 1
drivers/net/ethernet/meta/fbnic/fbnic_phylink.c | 50 +++++++++++++++++++++--
4 files changed, 51 insertions(+), 9 deletions(-)
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 2d5ae89b4a15..1d732cf22ec5 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_mac_request_irq(fbd);
+ err = fbnic_phylink_connect(fbn);
if (err)
goto time_stop;
@@ -52,8 +52,6 @@ int __fbnic_open(struct fbnic_net *fbn)
fbnic_bmc_rpc_init(fbd);
fbnic_rss_reinit(fbd, fbn);
- phylink_resume(fbn->phylink);
-
return 0;
time_stop:
fbnic_time_stop(fbn);
@@ -86,10 +84,11 @@ static int fbnic_stop(struct net_device *netdev)
{
struct fbnic_net *fbn = netdev_priv(netdev);
+ fbnic_mac_free_irq(fbn->fbd);
+ phylink_disconnect_phy(fbn->phylink);
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_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_phylink.c b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
index a9b2fc8108b7..b42cc5ad3055 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,49 @@ int fbnic_phylink_init(struct net_device *netdev)
return 0;
}
+/**
+ * fbnic_phylink_connect - Connect phylink structure to IRQ, PHY, and enable it
+ * @fbn: FBNIC Netdev private data struct phylink device attached to
+ *
+ * This function connects the phylink structure to the PHY and IRQ and then
+ * enables it to resuem 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 = phy_find_first(fbd->mii_bus);
+ 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;
+ }
+
+ err = fbnic_mac_request_irq(fbd);
+ if (err) {
+ phylink_disconnect_phy(fbn->phylink);
+ dev_err(fbd->dev, "Error requesting MAC IRQ, err: %d", err);
+ return err;
+ }
+
+ phylink_resume(fbn->phylink);
+
+ return 0;
+}
+
/**
* fbnic_phylink_pmd_training_complete_notify - PMD training complete notifier
* @fbn: FBNIC Netdev private data struct phylink device attached to
@@ -285,5 +327,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);
}
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [net-next PATCH 7/8] fbnic: Add SW shim for MII interface to PMA/PMD
2025-10-24 20:41 ` [net-next PATCH 7/8] fbnic: Add SW shim for MII interface to PMA/PMD Alexander Duyck
@ 2025-10-28 0:43 ` Jakub Kicinski
2025-10-28 20:51 ` Andrew Lunn
2025-10-28 20:52 ` kernel test robot
2 siblings, 0 replies; 25+ messages in thread
From: Jakub Kicinski @ 2025-10-28 0:43 UTC (permalink / raw)
To: Alexander Duyck
Cc: netdev, kernel-team, andrew+netdev, hkallweit1, linux, pabeni,
davem
On Fri, 24 Oct 2025 13:41:21 -0700 Alexander Duyck wrote:
> + u8 aui;
> +
> + if (fbd->netdev) {
> + fbn = netdev_priv(fbd->netdev);
> + aui = fbn->aui;
> + }
> +
> + switch (aui) {
Compiler says:
drivers/net/ethernet/meta/fbnic/fbnic_swmii.c:17:6: warning: variable 'aui' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
15 | u8 aui;
| ^
| = '\0'
--
pw-bot: cr
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [net-next PATCH 1/8] net: phy: Add support for 25, 50 and 100Gbps PMA to genphy_c45_read_pma
2025-10-24 20:40 ` [net-next PATCH 1/8] net: phy: Add support for 25, 50 and 100Gbps PMA to genphy_c45_read_pma Alexander Duyck
@ 2025-10-28 3:12 ` Andrew Lunn
2025-10-28 15:47 ` Alexander Duyck
0 siblings, 1 reply; 25+ messages in thread
From: Andrew Lunn @ 2025-10-28 3:12 UTC (permalink / raw)
To: Alexander Duyck
Cc: netdev, kuba, kernel-team, andrew+netdev, hkallweit1, linux,
pabeni, davem
> #define MDIO_PMA_SPEED_2B 0x0002 /* 2BASE-TL capable */
> #define MDIO_PMA_SPEED_10P 0x0004 /* 10PASS-TS capable */
> +#define MDIO_PMA_SPEED_50G 0x0800 /* 50G capable */
This is 45.2.1.4 PMA/PMD speed ability (Register 1.4) ??
50G is bit 3. So is 0x0800 correct? I think it should be 0x0008.
Andrew
---
pw-bot: cr
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [net-next PATCH 3/8] net: phy: Add 25G-CR, 50G-CR, 100G-CR2 support to C45 genphy
2025-10-24 20:40 ` [net-next PATCH 3/8] net: phy: Add 25G-CR, 50G-CR, 100G-CR2 support to C45 genphy Alexander Duyck
@ 2025-10-28 7:32 ` Maxime Chevallier
2025-10-28 22:40 ` Andrew Lunn
2025-10-28 12:53 ` Andrew Lunn
2025-10-28 12:57 ` Andrew Lunn
2 siblings, 1 reply; 25+ messages in thread
From: Maxime Chevallier @ 2025-10-28 7:32 UTC (permalink / raw)
To: Alexander Duyck, netdev
Cc: kuba, kernel-team, andrew+netdev, hkallweit1, linux, pabeni,
davem
Hi Alexander,
On 24/10/2025 22:40, Alexander Duyck wrote:
> From: Alexander Duyck <alexanderduyck@fb.com>
>
> Add support for 25G-CR, 50G-CR, 50G-CR2, and 100G-CR2 the c45 genphy. Note
> that 3 of the 4 are IEEE compliant so they are a direct copy from the
> clause 45 specification, the only exception to this is 50G-CR2 which is
> part of the Ethernet Consortium specification which never referenced how to
> handle this in the MDIO registers.
>
> Since 50GBase-CR2 isn't an IEEE standard it doesn't have a value in the
> extended capabilities registers. To account for that I am adding a define
> that is aliasing the 100GBase-CR4 to represent it as that is the media type
> used to carry data for 50R2, it is just that the PHY is carrying two 2 with
> 2 lanes each over the 4 lane cable. For now I am representing it with ctrl1
> set to 50G and ctrl2 being set to 100R4, and using the 100R4 capability to
> identify if it is supported or not.I
If 50GBase-CR2 isn't part of IEEE standards and doesn't appear in the
C45 ext caps, does it really belong in a genphy helper ?
You should be able to support it through the .config_anef() callback in
the PHY driver. I'm absolutely not familiar with these high-speed interfaces,
so maybe this is very common...
Maxime
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [net-next PATCH 3/8] net: phy: Add 25G-CR, 50G-CR, 100G-CR2 support to C45 genphy
2025-10-24 20:40 ` [net-next PATCH 3/8] net: phy: Add 25G-CR, 50G-CR, 100G-CR2 support to C45 genphy Alexander Duyck
2025-10-28 7:32 ` Maxime Chevallier
@ 2025-10-28 12:53 ` Andrew Lunn
2025-10-28 12:57 ` Andrew Lunn
2 siblings, 0 replies; 25+ messages in thread
From: Andrew Lunn @ 2025-10-28 12:53 UTC (permalink / raw)
To: Alexander Duyck
Cc: netdev, kuba, kernel-team, andrew+netdev, hkallweit1, linux,
pabeni, davem
On Fri, Oct 24, 2025 at 01:40:53PM -0700, Alexander Duyck wrote:
> From: Alexander Duyck <alexanderduyck@fb.com>
>
> Add support for 25G-CR, 50G-CR, 50G-CR2, and 100G-CR2 the c45 genphy. Note
> that 3 of the 4 are IEEE compliant so they are a direct copy from the
> clause 45 specification, the only exception to this is 50G-CR2 which is
> part of the Ethernet Consortium specification which never referenced how to
> handle this in the MDIO registers.
Please split this into two patches. Adding standard conforming bits is
not an issue. But we need to be careful of something which is not
clearly define anywhere, and might need to be reverted at some point,
and that is easier to do when it is in a patch of its own.
Andrew
---
pw-bot: cr
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [net-next PATCH 3/8] net: phy: Add 25G-CR, 50G-CR, 100G-CR2 support to C45 genphy
2025-10-24 20:40 ` [net-next PATCH 3/8] net: phy: Add 25G-CR, 50G-CR, 100G-CR2 support to C45 genphy Alexander Duyck
2025-10-28 7:32 ` Maxime Chevallier
2025-10-28 12:53 ` Andrew Lunn
@ 2025-10-28 12:57 ` Andrew Lunn
2025-10-28 15:25 ` Alexander Duyck
2 siblings, 1 reply; 25+ messages in thread
From: Andrew Lunn @ 2025-10-28 12:57 UTC (permalink / raw)
To: Alexander Duyck
Cc: netdev, kuba, kernel-team, andrew+netdev, hkallweit1, linux,
pabeni, davem
On Fri, Oct 24, 2025 at 01:40:53PM -0700, Alexander Duyck wrote:
> From: Alexander Duyck <alexanderduyck@fb.com>
>
> Add support for 25G-CR, 50G-CR, 50G-CR2, and 100G-CR2 the c45 genphy. Note
> that 3 of the 4 are IEEE compliant so they are a direct copy from the
> clause 45 specification, the only exception to this is 50G-CR2 which is
> part of the Ethernet Consortium specification which never referenced how to
> handle this in the MDIO registers.
Does the Ethernet Consortium have other media types which are not in
802.3? Does your scheme work for all of them?
Andrew
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [net-next PATCH 3/8] net: phy: Add 25G-CR, 50G-CR, 100G-CR2 support to C45 genphy
2025-10-28 12:57 ` Andrew Lunn
@ 2025-10-28 15:25 ` Alexander Duyck
0 siblings, 0 replies; 25+ messages in thread
From: Alexander Duyck @ 2025-10-28 15:25 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, kuba, kernel-team, andrew+netdev, hkallweit1, linux,
pabeni, davem
On Tue, Oct 28, 2025 at 5:57 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Fri, Oct 24, 2025 at 01:40:53PM -0700, Alexander Duyck wrote:
> > From: Alexander Duyck <alexanderduyck@fb.com>
> >
> > Add support for 25G-CR, 50G-CR, 50G-CR2, and 100G-CR2 the c45 genphy. Note
> > that 3 of the 4 are IEEE compliant so they are a direct copy from the
> > clause 45 specification, the only exception to this is 50G-CR2 which is
> > part of the Ethernet Consortium specification which never referenced how to
> > handle this in the MDIO registers.
I will go ahead and split this up as you suggested in the other email.
> Does the Ethernet Consortium have other media types which are not in
> 802.3? Does your scheme work for all of them?
>
> Andrew
Looking at the latest spec on the consortium website
(https://ethernettechnologyconsortium.org/wp-content/uploads/2021/10/Ethernet-Technology-Consortium_800G-Specification_r1.1.pdf)
it looks like they are adding 800G-KR8/CR8 and 400G-KR8/CR8. In the
case of these two we would have to come up with a different approach
as the implementation for them appears to be doing some sort of
bonding of a pair of 4 lane links.
The only other "consortium mode" is another implementation of
25G-KR/CR which I could have followed the same approach on. The IEEE
mode is close enough for now that there wasn't a point in splitting it
off as a type of its own. In fact I got this idea from copying how the
SFP bus code handles this
(https://elixir.bootlin.com/linux/v6.18-rc3/source/drivers/net/phy/sfp-bus.c#L251).
There is already logic that is setting the 25G link capability when it
detects the 100G cable. The general idea would be that we would end up
with the 50R2 eventually slipping in there as well since the same
cable can support all 3 types.
I suppose if we wanted to be more consistent between these setups we
could treat this more like the setup described for the 400/800 setup
and instead of having one PHY doing 1/2 of 100G we could treat it as a
bonded pair of PHYs doing 25G. As it stands I am going to have to
present 2 instances of the PCS/PMA anyway as the vendor config and
RSFEC ultimately has to be setup twice even though we are managing the
core PCS logic through only the first instance.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [net-next PATCH 1/8] net: phy: Add support for 25, 50 and 100Gbps PMA to genphy_c45_read_pma
2025-10-28 3:12 ` Andrew Lunn
@ 2025-10-28 15:47 ` Alexander Duyck
0 siblings, 0 replies; 25+ messages in thread
From: Alexander Duyck @ 2025-10-28 15:47 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, kuba, kernel-team, andrew+netdev, hkallweit1, linux,
pabeni, davem
On Mon, Oct 27, 2025 at 8:12 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > #define MDIO_PMA_SPEED_2B 0x0002 /* 2BASE-TL capable */
> > #define MDIO_PMA_SPEED_10P 0x0004 /* 10PASS-TS capable */
> > +#define MDIO_PMA_SPEED_50G 0x0800 /* 50G capable */
>
> This is 45.2.1.4 PMA/PMD speed ability (Register 1.4) ??
>
> 50G is bit 3. So is 0x0800 correct? I think it should be 0x0008.
>
> Andrew
Yeah, looks like it was a copy/paste error as that was the correct
value for 25G.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [net-next PATCH 7/8] fbnic: Add SW shim for MII interface to PMA/PMD
2025-10-24 20:41 ` [net-next PATCH 7/8] fbnic: Add SW shim for MII interface to PMA/PMD Alexander Duyck
2025-10-28 0:43 ` Jakub Kicinski
@ 2025-10-28 20:51 ` Andrew Lunn
2025-10-28 22:26 ` Alexander Duyck
2025-10-28 20:52 ` kernel test robot
2 siblings, 1 reply; 25+ messages in thread
From: Andrew Lunn @ 2025-10-28 20:51 UTC (permalink / raw)
To: Alexander Duyck
Cc: netdev, kuba, kernel-team, andrew+netdev, hkallweit1, linux,
pabeni, davem
> +static int
> +fbnic_swmii_read_pmapmd(struct fbnic_dev *fbd, int regnum)
> +{
> + u16 ctrl1 = 0, ctrl2 = 0;
> + struct fbnic_net *fbn;
> + int ret = 0;
> + u8 aui;
> +
> + if (fbd->netdev) {
Is that even impossible? I don't remember the core code, but usually
the priv structure is appended to the end of the net_device structure.
> + fbn = netdev_priv(fbd->netdev);
> + aui = fbn->aui;
> + }
> +
> + switch (aui) {
> + case FBNIC_AUI_25GAUI:
> + ctrl1 = MDIO_CTRL1_SPEED25G;
> + ctrl2 = MDIO_PMA_CTRL2_25GBCR;
> + break;
> + case FBNIC_AUI_LAUI2:
> + ctrl1 = MDIO_CTRL1_SPEED50G;
> + ctrl2 = MDIO_PMA_CTRL2_50GBCR2;
> + break;
> + case FBNIC_AUI_50GAUI1:
> + ctrl1 = MDIO_CTRL1_SPEED50G;
> + ctrl2 = MDIO_PMA_CTRL2_50GBCR;
> + break;
> + case FBNIC_AUI_100GAUI2:
> + ctrl1 = MDIO_CTRL1_SPEED100G;
> + ctrl2 = MDIO_PMA_CTRL2_100GBCR2;
> + break;
> + default:
> + break;
If it is something else, is that a bug? Would it be better to return
-EINVAL?
> + }
> +
> + switch (regnum) {
> + case MDIO_CTRL1:
> + ret = ctrl1;
> + break;
> + case MDIO_STAT1:
> + ret = fbd->pmd_state == FBNIC_PMD_SEND_DATA ?
> + MDIO_STAT1_LSTATUS : 0;
> + break;
> + case MDIO_DEVS1:
> + ret = MDIO_DEVS_PMAPMD;
> + break;
> + case MDIO_CTRL2:
> + ret = ctrl2;
> + break;
> + case MDIO_STAT2:
> + ret = MDIO_STAT2_DEVPRST_VAL |
> + MDIO_PMA_STAT2_EXTABLE;
> + break;
> + case MDIO_PMA_EXTABLE:
> + ret = MDIO_PMA_EXTABLE_40_100G |
> + MDIO_PMA_EXTABLE_25G;
> + break;
> + case MDIO_PMA_40G_EXTABLE:
> + ret = MDIO_PMA_40G_EXTABLE_50GBCR2;
> + break;
> + case MDIO_PMA_25G_EXTABLE:
> + ret = MDIO_PMA_25G_EXTABLE_25GBCR;
> + break;
> + case MDIO_PMA_50G_EXTABLE:
> + ret = MDIO_PMA_50G_EXTABLE_50GBCR;
> + break;
> + case MDIO_PMA_EXTABLE2:
> + ret = MDIO_PMA_EXTABLE2_50G;
> + break;
> + case MDIO_PMA_100G_EXTABLE:
> + ret = MDIO_PMA_100G_EXTABLE_100GBCR2;
> + break;
> + default:
> + break;
So the intention here is to return 0, meaning the register does not
exist, as defined by 802.3 for C45? Maybe add a comment?
I'm just wondering how robust this is. Maybe at a dev_dbg() printing
the regnum?
> + }
> +
> + return ret;
> +}
> +
> +static int
> +fbnic_swmii_read_c45(struct mii_bus *bus, int addr, int devnum, int regnum)
> +{
> + struct fbnic_dev *fbd = bus->priv;
> +
> + if (addr != 0)
> + return 0xffff;
> +
> + if (devnum == MDIO_MMD_PMAPMD)
> + return fbnic_swmii_read_pmapmd(fbd, regnum);
> +
> + return 0xffff;
For C45 you are supposed to return 0 if the register does not exist. It says:
45.2 MDIO Interface registers
If a device supports the MDIO interface it shall respond to all
possible register addresses for the device and return a value of
zero for undefined and unsupported registers.
> +static int
> +fbnic_swmii_write_c45(struct mii_bus *bus, int addr, int devnum,
> + int regnum, u16 val)
> +{
> + /* Currently PHY setup is meant to be read-only */
> + return 0;
-EOPNOTSUPP? WARN_ON_ONCE()?
If it does happen, i assume that means your assumptions are wrong?
Don't you want to know?
Andrew
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [net-next PATCH 7/8] fbnic: Add SW shim for MII interface to PMA/PMD
2025-10-24 20:41 ` [net-next PATCH 7/8] fbnic: Add SW shim for MII interface to PMA/PMD Alexander Duyck
2025-10-28 0:43 ` Jakub Kicinski
2025-10-28 20:51 ` Andrew Lunn
@ 2025-10-28 20:52 ` kernel test robot
2 siblings, 0 replies; 25+ messages in thread
From: kernel test robot @ 2025-10-28 20:52 UTC (permalink / raw)
To: Alexander Duyck, netdev
Cc: oe-kbuild-all, kuba, kernel-team, andrew+netdev, hkallweit1,
linux, pabeni, davem
Hi Alexander,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Alexander-Duyck/net-phy-Add-support-for-25-50-and-100Gbps-PMA-to-genphy_c45_read_pma/20251025-044506
base: net-next/main
patch link: https://lore.kernel.org/r/176133848134.2245037.8819965842869649833.stgit%40ahduyck-xeon-server.home.arpa
patch subject: [net-next PATCH 7/8] fbnic: Add SW shim for MII interface to PMA/PMD
config: loongarch-allyesconfig (https://download.01.org/0day-ci/archive/20251029/202510290438.wwPPh1zV-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project e1ae12640102fd2b05bc567243580f90acb1135f)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251029/202510290438.wwPPh1zV-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510290438.wwPPh1zV-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/net/ethernet/meta/fbnic/fbnic_swmii.c:17:6: warning: variable 'aui' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
17 | if (fbd->netdev) {
| ^~~~~~~~~~~
drivers/net/ethernet/meta/fbnic/fbnic_swmii.c:22:10: note: uninitialized use occurs here
22 | switch (aui) {
| ^~~
drivers/net/ethernet/meta/fbnic/fbnic_swmii.c:17:2: note: remove the 'if' if its condition is always true
17 | if (fbd->netdev) {
| ^~~~~~~~~~~~~~~~
drivers/net/ethernet/meta/fbnic/fbnic_swmii.c:15:8: note: initialize the variable 'aui' to silence this warning
15 | u8 aui;
| ^
| = '\0'
1 warning generated.
vim +17 drivers/net/ethernet/meta/fbnic/fbnic_swmii.c
8
9 static int
10 fbnic_swmii_read_pmapmd(struct fbnic_dev *fbd, int regnum)
11 {
12 u16 ctrl1 = 0, ctrl2 = 0;
13 struct fbnic_net *fbn;
14 int ret = 0;
15 u8 aui;
16
> 17 if (fbd->netdev) {
18 fbn = netdev_priv(fbd->netdev);
19 aui = fbn->aui;
20 }
21
22 switch (aui) {
23 case FBNIC_AUI_25GAUI:
24 ctrl1 = MDIO_CTRL1_SPEED25G;
25 ctrl2 = MDIO_PMA_CTRL2_25GBCR;
26 break;
27 case FBNIC_AUI_LAUI2:
28 ctrl1 = MDIO_CTRL1_SPEED50G;
29 ctrl2 = MDIO_PMA_CTRL2_50GBCR2;
30 break;
31 case FBNIC_AUI_50GAUI1:
32 ctrl1 = MDIO_CTRL1_SPEED50G;
33 ctrl2 = MDIO_PMA_CTRL2_50GBCR;
34 break;
35 case FBNIC_AUI_100GAUI2:
36 ctrl1 = MDIO_CTRL1_SPEED100G;
37 ctrl2 = MDIO_PMA_CTRL2_100GBCR2;
38 break;
39 default:
40 break;
41 }
42
43 switch (regnum) {
44 case MDIO_CTRL1:
45 ret = ctrl1;
46 break;
47 case MDIO_STAT1:
48 ret = fbd->pmd_state == FBNIC_PMD_SEND_DATA ?
49 MDIO_STAT1_LSTATUS : 0;
50 break;
51 case MDIO_DEVS1:
52 ret = MDIO_DEVS_PMAPMD;
53 break;
54 case MDIO_CTRL2:
55 ret = ctrl2;
56 break;
57 case MDIO_STAT2:
58 ret = MDIO_STAT2_DEVPRST_VAL |
59 MDIO_PMA_STAT2_EXTABLE;
60 break;
61 case MDIO_PMA_EXTABLE:
62 ret = MDIO_PMA_EXTABLE_40_100G |
63 MDIO_PMA_EXTABLE_25G;
64 break;
65 case MDIO_PMA_40G_EXTABLE:
66 ret = MDIO_PMA_40G_EXTABLE_50GBCR2;
67 break;
68 case MDIO_PMA_25G_EXTABLE:
69 ret = MDIO_PMA_25G_EXTABLE_25GBCR;
70 break;
71 case MDIO_PMA_50G_EXTABLE:
72 ret = MDIO_PMA_50G_EXTABLE_50GBCR;
73 break;
74 case MDIO_PMA_EXTABLE2:
75 ret = MDIO_PMA_EXTABLE2_50G;
76 break;
77 case MDIO_PMA_100G_EXTABLE:
78 ret = MDIO_PMA_100G_EXTABLE_100GBCR2;
79 break;
80 default:
81 break;
82 }
83
84 return ret;
85 }
86
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [net-next PATCH 8/8] fbnic: Add phydev representing PMD to phylink setup
2025-10-24 20:41 ` [net-next PATCH 8/8] fbnic: Add phydev representing PMD to phylink setup Alexander Duyck
@ 2025-10-28 21:02 ` Andrew Lunn
2025-10-28 22:32 ` Alexander Duyck
0 siblings, 1 reply; 25+ messages in thread
From: Andrew Lunn @ 2025-10-28 21:02 UTC (permalink / raw)
To: Alexander Duyck
Cc: netdev, kuba, kernel-team, andrew+netdev, hkallweit1, linux,
pabeni, davem
> +/**
> + * fbnic_phylink_connect - Connect phylink structure to IRQ, PHY, and enable it
> + * @fbn: FBNIC Netdev private data struct phylink device attached to
> + *
> + * This function connects the phylink structure to the PHY and IRQ and then
> + * enables it to resuem operations. With this function completed the PHY will
resume
> + * 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 = phy_find_first(fbd->mii_bus);
phy_find_first() is generally used when you have no idea what address
the PHY is using. It can cause future surprises when additional
devices appear on the bus.
In this case, you know what address the device is on the bus, so
mdiobus_get_phy() would be better.
> + 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;
> + }
> +
> + err = fbnic_mac_request_irq(fbd);
> + if (err) {
> + phylink_disconnect_phy(fbn->phylink);
> + dev_err(fbd->dev, "Error requesting MAC IRQ, err: %d", err);
> + return err;
> + }
> +
> + phylink_resume(fbn->phylink);
When was is suspended?
Andrew
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [net-next PATCH 7/8] fbnic: Add SW shim for MII interface to PMA/PMD
2025-10-28 20:51 ` Andrew Lunn
@ 2025-10-28 22:26 ` Alexander Duyck
0 siblings, 0 replies; 25+ messages in thread
From: Alexander Duyck @ 2025-10-28 22:26 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, kuba, kernel-team, andrew+netdev, hkallweit1, linux,
pabeni, davem
On Tue, Oct 28, 2025 at 1:51 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > +static int
> > +fbnic_swmii_read_pmapmd(struct fbnic_dev *fbd, int regnum)
> > +{
> > + u16 ctrl1 = 0, ctrl2 = 0;
> > + struct fbnic_net *fbn;
> > + int ret = 0;
> > + u8 aui;
> > +
> > + if (fbd->netdev) {
>
> Is that even impossible? I don't remember the core code, but usually
> the priv structure is appended to the end of the net_device structure.
In the case of fbnic it is possible as the fbd is appended to the
devlink structure. So it exists before the netdev does.
> > + fbn = netdev_priv(fbd->netdev);
> > + aui = fbn->aui;
> > + }
> > +
> > + switch (aui) {
> > + case FBNIC_AUI_25GAUI:
> > + ctrl1 = MDIO_CTRL1_SPEED25G;
> > + ctrl2 = MDIO_PMA_CTRL2_25GBCR;
> > + break;
> > + case FBNIC_AUI_LAUI2:
> > + ctrl1 = MDIO_CTRL1_SPEED50G;
> > + ctrl2 = MDIO_PMA_CTRL2_50GBCR2;
> > + break;
> > + case FBNIC_AUI_50GAUI1:
> > + ctrl1 = MDIO_CTRL1_SPEED50G;
> > + ctrl2 = MDIO_PMA_CTRL2_50GBCR;
> > + break;
> > + case FBNIC_AUI_100GAUI2:
> > + ctrl1 = MDIO_CTRL1_SPEED100G;
> > + ctrl2 = MDIO_PMA_CTRL2_100GBCR2;
> > + break;
> > + default:
> > + break;
>
> If it is something else, is that a bug? Would it be better to return
> -EINVAL?
The problem with returning an error is that it causes other code to do
weird things. I figure we are better off just not returning a speed in
that case.
> > + }
> > +
> > + switch (regnum) {
> > + case MDIO_CTRL1:
> > + ret = ctrl1;
> > + break;
> > + case MDIO_STAT1:
> > + ret = fbd->pmd_state == FBNIC_PMD_SEND_DATA ?
> > + MDIO_STAT1_LSTATUS : 0;
> > + break;
> > + case MDIO_DEVS1:
> > + ret = MDIO_DEVS_PMAPMD;
> > + break;
> > + case MDIO_CTRL2:
> > + ret = ctrl2;
> > + break;
> > + case MDIO_STAT2:
> > + ret = MDIO_STAT2_DEVPRST_VAL |
> > + MDIO_PMA_STAT2_EXTABLE;
> > + break;
> > + case MDIO_PMA_EXTABLE:
> > + ret = MDIO_PMA_EXTABLE_40_100G |
> > + MDIO_PMA_EXTABLE_25G;
> > + break;
> > + case MDIO_PMA_40G_EXTABLE:
> > + ret = MDIO_PMA_40G_EXTABLE_50GBCR2;
> > + break;
> > + case MDIO_PMA_25G_EXTABLE:
> > + ret = MDIO_PMA_25G_EXTABLE_25GBCR;
> > + break;
> > + case MDIO_PMA_50G_EXTABLE:
> > + ret = MDIO_PMA_50G_EXTABLE_50GBCR;
> > + break;
> > + case MDIO_PMA_EXTABLE2:
> > + ret = MDIO_PMA_EXTABLE2_50G;
> > + break;
> > + case MDIO_PMA_100G_EXTABLE:
> > + ret = MDIO_PMA_100G_EXTABLE_100GBCR2;
> > + break;
> > + default:
> > + break;
>
> So the intention here is to return 0, meaning the register does not
> exist, as defined by 802.3 for C45? Maybe add a comment?
>
> I'm just wondering how robust this is. Maybe at a dev_dbg() printing
> the regnum?
I will look at adding it. Probably have it output the regnum and the value.
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int
> > +fbnic_swmii_read_c45(struct mii_bus *bus, int addr, int devnum, int regnum)
> > +{
> > + struct fbnic_dev *fbd = bus->priv;
> > +
> > + if (addr != 0)
> > + return 0xffff;
> > +
> > + if (devnum == MDIO_MMD_PMAPMD)
> > + return fbnic_swmii_read_pmapmd(fbd, regnum);
> > +
> > + return 0xffff;
>
> For C45 you are supposed to return 0 if the register does not exist. It says:
>
> 45.2 MDIO Interface registers
>
> If a device supports the MDIO interface it shall respond to all
> possible register addresses for the device and return a value of
> zero for undefined and unsupported registers.
>
> > +static int
> > +fbnic_swmii_write_c45(struct mii_bus *bus, int addr, int devnum,
> > + int regnum, u16 val)
> > +{
> > + /* Currently PHY setup is meant to be read-only */
> > + return 0;
>
> -EOPNOTSUPP? WARN_ON_ONCE()?
It will cause stuff to blow up. There are several writes that are done
that can be safely ignored. The issue is if I start returning an error
I get no phy at all.
> If it does happen, i assume that means your assumptions are wrong?
> Don't you want to know?
I had based the logic on the fixed_phy approach
(https://elixir.bootlin.com/linux/v6.18-rc3/C/ident/fixed_mdio_write).
Basically it just returns 0 and doesn't do anything in response to
writes. Seems like we do the same thing for phylink assuming we aren't
passing it to an actual phy in the phylink_mii_read/write calls.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [net-next PATCH 8/8] fbnic: Add phydev representing PMD to phylink setup
2025-10-28 21:02 ` Andrew Lunn
@ 2025-10-28 22:32 ` Alexander Duyck
2025-10-28 22:56 ` Andrew Lunn
0 siblings, 1 reply; 25+ messages in thread
From: Alexander Duyck @ 2025-10-28 22:32 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, kuba, kernel-team, andrew+netdev, hkallweit1, linux,
pabeni, davem
On Tue, Oct 28, 2025 at 2:02 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > +/**
> > + * fbnic_phylink_connect - Connect phylink structure to IRQ, PHY, and enable it
> > + * @fbn: FBNIC Netdev private data struct phylink device attached to
> > + *
> > + * This function connects the phylink structure to the PHY and IRQ and then
> > + * enables it to resuem operations. With this function completed the PHY will
>
> resume
>
> > + * 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 = phy_find_first(fbd->mii_bus);
>
> phy_find_first() is generally used when you have no idea what address
> the PHY is using. It can cause future surprises when additional
> devices appear on the bus.
>
> In this case, you know what address the device is on the bus, so
> mdiobus_get_phy() would be better.
I'll make the switch then.
> > + 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;
> > + }
> > +
> > + err = fbnic_mac_request_irq(fbd);
> > + if (err) {
> > + phylink_disconnect_phy(fbn->phylink);
> > + dev_err(fbd->dev, "Error requesting MAC IRQ, err: %d", err);
> > + return err;
> > + }
> > +
> > + phylink_resume(fbn->phylink);
>
> When was is suspended?
We don't use the start/stop calls. Instead we use the resume/suspend
calls in order to deal with the fact that we normally aren't fully
resetting the link. The first call automatically gets converted to a
phylink_start because the bit isn't set for the MAC_WOL, however all
subsequent setups it becomes a resume so that we aren't tearing the
link down fully in order to avoid blocking the BMC which is sharing
the link similar to how a WOL connection would.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [net-next PATCH 3/8] net: phy: Add 25G-CR, 50G-CR, 100G-CR2 support to C45 genphy
2025-10-28 7:32 ` Maxime Chevallier
@ 2025-10-28 22:40 ` Andrew Lunn
2025-10-28 22:49 ` Alexander Duyck
0 siblings, 1 reply; 25+ messages in thread
From: Andrew Lunn @ 2025-10-28 22:40 UTC (permalink / raw)
To: Maxime Chevallier
Cc: Alexander Duyck, netdev, kuba, kernel-team, andrew+netdev,
hkallweit1, linux, pabeni, davem
On Tue, Oct 28, 2025 at 08:32:03AM +0100, Maxime Chevallier wrote:
> Hi Alexander,
>
> On 24/10/2025 22:40, Alexander Duyck wrote:
> > From: Alexander Duyck <alexanderduyck@fb.com>
> >
> > Add support for 25G-CR, 50G-CR, 50G-CR2, and 100G-CR2 the c45 genphy. Note
> > that 3 of the 4 are IEEE compliant so they are a direct copy from the
> > clause 45 specification, the only exception to this is 50G-CR2 which is
> > part of the Ethernet Consortium specification which never referenced how to
> > handle this in the MDIO registers.
> >
> > Since 50GBase-CR2 isn't an IEEE standard it doesn't have a value in the
> > extended capabilities registers. To account for that I am adding a define
> > that is aliasing the 100GBase-CR4 to represent it as that is the media type
> > used to carry data for 50R2, it is just that the PHY is carrying two 2 with
> > 2 lanes each over the 4 lane cable. For now I am representing it with ctrl1
> > set to 50G and ctrl2 being set to 100R4, and using the 100R4 capability to
> > identify if it is supported or not.I
>
> If 50GBase-CR2 isn't part of IEEE standards and doesn't appear in the
> C45 ext caps, does it really belong in a genphy helper ?
I agree with you here. We should not pollute our nice clean 802.3
implementation. If the Ethernet Consortium had defined how these modes
are represented in MDIO registers, we could of added helpers which
look at the vendor registers they chose to use. We have done this in
the past, for the Open Alliance TC14 10BASE-T1S PLCA. But since each
vendor is going to implement this differently, it should not be in the
core.
> You should be able to support it through the .config_aneg() callback in
> the PHY driver.
It is probably a little more than .config_aneg(), but yes.
I assume FB/META have an OUI for their MAC addresses? I _think_ the
same OUI can be used for registers 2 and 3 in MDIO. So your fake PHY
can indicate it is a FB/META PHY and cause the FB/META PHY driver to
load. The .get_features callback can append this 50GBase-CR2 link
mode. The .read_status() can indicate if it is in use etc. And you can
do all the other link modes by just calling the helpers. I assume you
are currently using the genphy_c45_driver? That can be used as a
template.
Andrew
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [net-next PATCH 3/8] net: phy: Add 25G-CR, 50G-CR, 100G-CR2 support to C45 genphy
2025-10-28 22:40 ` Andrew Lunn
@ 2025-10-28 22:49 ` Alexander Duyck
0 siblings, 0 replies; 25+ messages in thread
From: Alexander Duyck @ 2025-10-28 22:49 UTC (permalink / raw)
To: Andrew Lunn
Cc: Maxime Chevallier, netdev, kuba, kernel-team, andrew+netdev,
hkallweit1, linux, pabeni, davem
On Tue, Oct 28, 2025 at 3:40 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Tue, Oct 28, 2025 at 08:32:03AM +0100, Maxime Chevallier wrote:
> > Hi Alexander,
> >
> > On 24/10/2025 22:40, Alexander Duyck wrote:
> > > From: Alexander Duyck <alexanderduyck@fb.com>
> > >
> > > Add support for 25G-CR, 50G-CR, 50G-CR2, and 100G-CR2 the c45 genphy. Note
> > > that 3 of the 4 are IEEE compliant so they are a direct copy from the
> > > clause 45 specification, the only exception to this is 50G-CR2 which is
> > > part of the Ethernet Consortium specification which never referenced how to
> > > handle this in the MDIO registers.
> > >
> > > Since 50GBase-CR2 isn't an IEEE standard it doesn't have a value in the
> > > extended capabilities registers. To account for that I am adding a define
> > > that is aliasing the 100GBase-CR4 to represent it as that is the media type
> > > used to carry data for 50R2, it is just that the PHY is carrying two 2 with
> > > 2 lanes each over the 4 lane cable. For now I am representing it with ctrl1
> > > set to 50G and ctrl2 being set to 100R4, and using the 100R4 capability to
> > > identify if it is supported or not.I
> >
> > If 50GBase-CR2 isn't part of IEEE standards and doesn't appear in the
> > C45 ext caps, does it really belong in a genphy helper ?
>
> I agree with you here. We should not pollute our nice clean 802.3
> implementation. If the Ethernet Consortium had defined how these modes
> are represented in MDIO registers, we could of added helpers which
> look at the vendor registers they chose to use. We have done this in
> the past, for the Open Alliance TC14 10BASE-T1S PLCA. But since each
> vendor is going to implement this differently, it should not be in the
> core.
>
> > You should be able to support it through the .config_aneg() callback in
> > the PHY driver.
>
> It is probably a little more than .config_aneg(), but yes.
>
> I assume FB/META have an OUI for their MAC addresses? I _think_ the
> same OUI can be used for registers 2 and 3 in MDIO. So your fake PHY
> can indicate it is a FB/META PHY and cause the FB/META PHY driver to
> load. The .get_features callback can append this 50GBase-CR2 link
> mode. The .read_status() can indicate if it is in use etc. And you can
> do all the other link modes by just calling the helpers. I assume you
> are currently using the genphy_c45_driver? That can be used as a
> template.
Yeah, I was already starting down some of this path as I was going to
have to provide a FB/META PMA ID in order to work with the XPCS driver
in terms of handling the config.
I can probably look at pushing the fixes to get the handling of the
PHY ID sorted out and then look at adding a new driver to handle the
50R2 without touching the genphy code for now.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [net-next PATCH 8/8] fbnic: Add phydev representing PMD to phylink setup
2025-10-28 22:32 ` Alexander Duyck
@ 2025-10-28 22:56 ` Andrew Lunn
2025-10-28 23:07 ` Alexander Duyck
0 siblings, 1 reply; 25+ messages in thread
From: Andrew Lunn @ 2025-10-28 22:56 UTC (permalink / raw)
To: Alexander Duyck
Cc: netdev, kuba, kernel-team, andrew+netdev, hkallweit1, linux,
pabeni, davem
> > > + phylink_resume(fbn->phylink);
> >
> > When was is suspended?
>
> We don't use the start/stop calls. Instead we use the resume/suspend
> calls in order to deal with the fact that we normally aren't fully
> resetting the link. The first call automatically gets converted to a
> phylink_start because the bit isn't set for the MAC_WOL, however all
> subsequent setups it becomes a resume so that we aren't tearing the
> link down fully in order to avoid blocking the BMC which is sharing
> the link similar to how a WOL connection would.
/**
* phylink_resume() - handle a network device resume event
* @pl: a pointer to a &struct phylink returned from phylink_create()
*
* Undo the effects of phylink_suspend(), returning the link to an
* operational state.
*/
There needs to be a call to phylink_suspend() before you call
phylink_resume(). If there is a prior call to phylink_suspend() all is
O.K.
Russell gets unhappy if you don't follow the documentation. The
documentation is part of the API, part of the contract. If you break
the contract, don't be surprised is your driver breaks sometime in the
future.
Andrew
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [net-next PATCH 8/8] fbnic: Add phydev representing PMD to phylink setup
2025-10-28 22:56 ` Andrew Lunn
@ 2025-10-28 23:07 ` Alexander Duyck
0 siblings, 0 replies; 25+ messages in thread
From: Alexander Duyck @ 2025-10-28 23:07 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, kuba, kernel-team, andrew+netdev, hkallweit1, linux,
pabeni, davem
On Tue, Oct 28, 2025 at 3:56 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > > + phylink_resume(fbn->phylink);
> > >
> > > When was is suspended?
> >
> > We don't use the start/stop calls. Instead we use the resume/suspend
> > calls in order to deal with the fact that we normally aren't fully
> > resetting the link. The first call automatically gets converted to a
> > phylink_start because the bit isn't set for the MAC_WOL, however all
> > subsequent setups it becomes a resume so that we aren't tearing the
> > link down fully in order to avoid blocking the BMC which is sharing
> > the link similar to how a WOL connection would.
>
> /**
> * phylink_resume() - handle a network device resume event
> * @pl: a pointer to a &struct phylink returned from phylink_create()
> *
> * Undo the effects of phylink_suspend(), returning the link to an
> * operational state.
> */
>
> There needs to be a call to phylink_suspend() before you call
> phylink_resume(). If there is a prior call to phylink_suspend() all is
> O.K.
>
> Russell gets unhappy if you don't follow the documentation. The
> documentation is part of the API, part of the contract. If you break
> the contract, don't be surprised is your driver breaks sometime in the
> future.
We have had some back and forth in the past about it. Basically we
need to work through and come up with a better way of handling the BMC
portion of this. As it stands there are still some side effects but it
can't really be helped as the resume logic will force the MAC down
briefly.
I am still also using wol_enabled and haven't come up with a good
alternative yet to say that a BMC is present on the link.
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2025-10-28 23:08 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-24 20:40 [net-next PATCH 0/8] net: phy: Add support for fbnic PHY w/ 25G, 50G, and 100G support Alexander Duyck
2025-10-24 20:40 ` [net-next PATCH 1/8] net: phy: Add support for 25, 50 and 100Gbps PMA to genphy_c45_read_pma Alexander Duyck
2025-10-28 3:12 ` Andrew Lunn
2025-10-28 15:47 ` Alexander Duyck
2025-10-24 20:40 ` [net-next PATCH 2/8] net: phy: Avoid reusing val in genphy_c45_pma_read_ext_abilities Alexander Duyck
2025-10-24 20:40 ` [net-next PATCH 3/8] net: phy: Add 25G-CR, 50G-CR, 100G-CR2 support to C45 genphy Alexander Duyck
2025-10-28 7:32 ` Maxime Chevallier
2025-10-28 22:40 ` Andrew Lunn
2025-10-28 22:49 ` Alexander Duyck
2025-10-28 12:53 ` Andrew Lunn
2025-10-28 12:57 ` Andrew Lunn
2025-10-28 15:25 ` Alexander Duyck
2025-10-24 20:41 ` [net-next PATCH 4/8] fbnic: Rename PCS IRQ to MAC IRQ as it is actually a MAC interrupt Alexander Duyck
2025-10-24 20:41 ` [net-next PATCH 5/8] fbnic: Add logic to track PMD state via MAC/PCS signals Alexander Duyck
2025-10-24 20:41 ` [net-next PATCH 6/8] fbnic: Cleanup handling for link down event statistics Alexander Duyck
2025-10-24 20:41 ` [net-next PATCH 7/8] fbnic: Add SW shim for MII interface to PMA/PMD Alexander Duyck
2025-10-28 0:43 ` Jakub Kicinski
2025-10-28 20:51 ` Andrew Lunn
2025-10-28 22:26 ` Alexander Duyck
2025-10-28 20:52 ` kernel test robot
2025-10-24 20:41 ` [net-next PATCH 8/8] fbnic: Add phydev representing PMD to phylink setup Alexander Duyck
2025-10-28 21:02 ` Andrew Lunn
2025-10-28 22:32 ` Alexander Duyck
2025-10-28 22:56 ` Andrew Lunn
2025-10-28 23:07 ` Alexander Duyck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).