Thread (1 message) 1 message, 1 author, 2025-10-21

Re: [PATCH] net-phy-bcm84881: add support for other 2.5G / 5G / 10G phys

From: Florian Fainelli <florian.fainelli@broadcom.com>
Date: 2025-10-21 16:36:08 

Hello,

On 10/20/2025 3:25 PM, 'Balázs Triszka' via BCM-KERNEL-FEEDBACK-LIST,PDL 
wrote:
This patch add support for mako, orca, blackfin, shortfin and
longfin phys to bcm84881 driver.

Signed-off-by: Balázs Triszka <redacted>
Cc:florian.fainelli@broadcom.com
Cc:andrew@lunn.ch
You need to run ./scripts/get_maintainer.pl to get the proper 
recipients, right now this patch has not been sent to all of them, 
specifically the driver author, Russell, is critically missing from the 
recipients list.
quoted hunk
---
  drivers/net/phy/bcm84881.c | 1127 +++++++++++++++++++++++++++++++++++-
  1 file changed, 1110 insertions(+), 17 deletions(-)

diff --git a/drivers/net/phy/bcm84881.c b/drivers/net/phy/bcm84881.c
index d7f7cc44c532..d8dc32aa4ada 100644
--- a/drivers/net/phy/bcm84881.c
+++ b/drivers/net/phy/bcm84881.c
@@ -16,6 +16,106 @@
  #include <linux/module.h>
  #include <linux/phy.h>
  
+#define PHYID_BCM4912			0x359050cd
+#define PHYID_BCM4912M			0x359051cd
+#define PHYID_BCM50991EL_A0		0x359050c8
+#define PHYID_BCM50991EL_B0		0x359050c9
+#define PHYID_BCM50991ELM_B0	0x3590518d
+#define PHYID_BCM50994E_A0		0x359050f8
+#define PHYID_BCM50994E_B0		0x359050f9
+#define PHYID_BCM54991E_A0		0x35905098
+#define PHYID_BCM54991E_B0		0x35905099
+#define PHYID_BCM54991EL_A0		0x35905088
+#define PHYID_BCM54991EL_B0		0x35905089
+#define PHYID_BCM54991ELM_A0	0x35905188
+#define PHYID_BCM54991ELM_B0	0x35905189
+#define PHYID_BCM54991EM_A0		0x35905198
+#define PHYID_BCM54991EM_B0		0x35905199
+#define PHYID_BCM54992E_A0		0x359050a8
+#define PHYID_BCM54992E_B0		0x359050a9
+#define PHYID_BCM54992EM_A0		0x359051a8
+#define PHYID_BCM54992EM_B0		0x359051a9
+#define PHYID_BCM54994E_A0		0x359050b8
+#define PHYID_BCM54994E_B0		0x359050b9
+#define PHYID_BCM54994EM_A0		0x359051b8
+#define PHYID_BCM54994EM_B0		0x359051b9
+
+#define PHYID_BCM49418			0x359050c1
+#define PHYID_BCM49418M			0x359051c1
+#define PHYID_BCM54991_A0		0x35905094
+#define PHYID_BCM54991_B0		0x35905095
+#define PHYID_BCM54991L_A0		0x35905084
+#define PHYID_BCM54991L_B0		0x35905085
+#define PHYID_BCM54991M_A0		0x35905194
+#define PHYID_BCM54991M_B0		0x35905195
+#define PHYID_BCM54992_A0		0x359050a4
+#define PHYID_BCM54992_B0		0x359050a5
+#define PHYID_BCM54992M_A0		0x359051a4
+#define PHYID_BCM54992M_B0		0x359051a5
+#define PHYID_BCM54994_A0		0x359050b4
+#define PHYID_BCM54994_B0		0x359050b5
+#define PHYID_BCM54994M_A0		0x359051b4
+#define PHYID_BCM54994M_B0		0x359051b5
+#define PHYID_BCM84860_A0		0xae025048
+#define PHYID_BCM84861_A0		0xae025040
+#define PHYID_BCM84880_A0		0xae025158
+#define PHYID_BCM84880_B0		0xae025159
+#define PHYID_BCM84884_A0		0xae025148
+#define PHYID_BCM84884_B0		0xae025149
+#define PHYID_BCM84884E_A0		0xae025168
+#define PHYID_BCM84884E_B0		0xae025169
+#define PHYID_BCM84885_A0		0xae025178
+#define PHYID_BCM84885_B0		0xae025179
+
+#define PHYID_BCM54991H_A0		0x359050d0
+#define PHYID_BCM54991H_A1		0x359051d0
+#define PHYID_BCM54991H_B0		0x359050d1
+#define PHYID_BCM54991H_B1		0x359051d1
+#define PHYID_BCM54991LM_A0		0x35905184
+#define PHYID_BCM54991LM_B0		0x35905185
+#define PHYID_BCM54991SK_B0		0x359051d5
+#define PHYID_BCM54994EL_B0		0x3590501d
+#define PHYID_BCM54994H_A0		0x359050f0
+#define PHYID_BCM54994H_A1		0x359051f0
+#define PHYID_BCM54994H_B0		0x359050f1
+#define PHYID_BCM54994H_B1		0x359051f1
+#define PHYID_BCM54994L_B0		0x35905019
+#define PHYID_BCM54994SK_B0		0x359051f5
+#define PHYID_BCM54998_B0		0x35905011
+#define PHYID_BCM54998E_B0		0x35905015
+#define PHYID_BCM54998ES_B0		0x3590500d
+#define PHYID_BCM54998S_B0		0x35905009
+#define PHYID_BCM84881_A0		0xae025150
+#define PHYID_BCM84881_B0		0xae025151
+#define PHYID_BCM84886_A0		0xae025170
+#define PHYID_BCM84886_B0		0xae025171
+#define PHYID_BCM84887_A0		0xae025144
+#define PHYID_BCM84887_B0		0xae025145
+#define PHYID_BCM84888_A0		0xae025140
+#define PHYID_BCM84888_B0		0xae025141
+#define PHYID_BCM84888E_A0		0xae025160
+#define PHYID_BCM84888E_B0		0xae025161
+#define PHYID_BCM84888S_A0		0xae025174
+#define PHYID_BCM84888S_B0		0xae025175
+#define PHYID_BCM84891_A0		0x35905090
+#define PHYID_BCM84891_B0		0x35905091
+#define PHYID_BCM84891L_A0		0x35905080
+#define PHYID_BCM84891L_B0		0x35905081
+#define PHYID_BCM84891LM_A0		0x35905180
+#define PHYID_BCM84891LM_B0		0x35905181
+#define PHYID_BCM84891M_A0		0x35905190
+#define PHYID_BCM84891M_B0		0x35905191
+#define PHYID_BCM84892_A0		0x359050a0
+#define PHYID_BCM84892_B0		0x359050a1
+#define PHYID_BCM84892M_A0		0x359051a0
+#define PHYID_BCM84892M_B0		0x359051a1
+#define PHYID_BCM84894_A0		0x359050b0
+#define PHYID_BCM84894_B0		0x359050b1
+#define PHYID_BCM84894M_A0		0x359051b0
+#define PHYID_BCM84894M_B0		0x359051b1
+#define PHYID_BCM84896_B0		0x35905005
+#define PHYID_BCM84898_B0		0x35905001
PHY identifiers are all maintained in include/linux/brcmphy.h, also 
there is no need to be maintaining per-revision PHY identifiers, just 
the main chip identifiers are enough.
quoted hunk
+
  enum {
  	MDIO_AN_C22 = 0xffe0,
  };
@@ -29,22 +129,57 @@ static int bcm84881_wait_init(struct phy_device *phydev)
  					 100000, 2000000, false);
  }
  
-static void bcm84881_fill_possible_interfaces(struct phy_device *phydev)
+static int bcm84881_config_init_2500(struct phy_device *phydev)
  {
  	unsigned long *possible = phydev->possible_interfaces;
  
  	__set_bit(PHY_INTERFACE_MODE_SGMII, possible);
  	__set_bit(PHY_INTERFACE_MODE_2500BASEX, possible);
-	__set_bit(PHY_INTERFACE_MODE_10GBASER, possible);
+
+	switch (phydev->interface) {
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_2500BASEX:
+		break;
+	default:
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int bcm84881_config_init_5G(struct phy_device *phydev)
Why not bcm84881_config_init_5000() for consistenty with 
bcm84881_config_init_2500()?

+{
+	unsigned long *possible = phydev->possible_interfaces;
+
+	__set_bit(PHY_INTERFACE_MODE_SGMII, possible);
+	__set_bit(PHY_INTERFACE_MODE_2500BASEX, possible);
+	__set_bit(PHY_INTERFACE_MODE_5GBASER, possible);
+
+	switch (phydev->interface) {
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_2500BASEX:
+	case PHY_INTERFACE_MODE_5GBASER:
+		break;
+	default:
+		return -ENODEV;
+	}
+
+	return 0;
  }
  
-static int bcm84881_config_init(struct phy_device *phydev)
+static int bcm84881_config_init_10G(struct phy_device *phydev)
Likewise.
quoted hunk
  {
-	bcm84881_fill_possible_interfaces(phydev);
+	unsigned long *possible = phydev->possible_interfaces;
+
+	__set_bit(PHY_INTERFACE_MODE_SGMII, possible);
+	__set_bit(PHY_INTERFACE_MODE_2500BASEX, possible);
+	__set_bit(PHY_INTERFACE_MODE_5GBASER, possible);
+	__set_bit(PHY_INTERFACE_MODE_10GBASER, possible);
  
  	switch (phydev->interface) {
  	case PHY_INTERFACE_MODE_SGMII:
  	case PHY_INTERFACE_MODE_2500BASEX:
+	case PHY_INTERFACE_MODE_5GBASER:
  	case PHY_INTERFACE_MODE_10GBASER:
  		break;
  	default:
@@ -208,26 +343,25 @@ static int bcm84881_read_status(struct phy_device *phydev)
  	 */
  	val = phy_read_mmd(phydev, MDIO_MMD_VEND1, 0x4011);
  	mode = (val & 0x1e) >> 1;
-	if (mode == 1 || mode == 2)
-		phydev->interface = PHY_INTERFACE_MODE_SGMII;
-	else if (mode == 3)
-		phydev->interface = PHY_INTERFACE_MODE_10GBASER;
-	else if (mode == 4)
-		phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
  	switch (mode & 7) {
  	case 1:
It would be a good idea to get some defines for these numbers.
quoted hunk
+		phydev->interface = PHY_INTERFACE_MODE_SGMII;
  		phydev->speed = SPEED_100;
  		break;
  	case 2:
+		phydev->interface = PHY_INTERFACE_MODE_SGMII;
  		phydev->speed = SPEED_1000;
  		break;
  	case 3:
+		phydev->interface = PHY_INTERFACE_MODE_10GBASER;
  		phydev->speed = SPEED_10000;
  		break;
  	case 4:
+		phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
  		phydev->speed = SPEED_2500;
  		break;
  	case 5:
+		phydev->interface = PHY_INTERFACE_MODE_5GBASER;
  		phydev->speed = SPEED_5000;
  		break;
  	}
@@ -246,11 +380,970 @@ static unsigned int bcm84881_inband_caps(struct phy_device *phydev,
  
  static struct phy_driver bcm84881_drivers[] = {
  	{
-		.phy_id		= 0xae025150,
-		.phy_id_mask	= 0xfffffff0,
-		.name		= "Broadcom BCM84881",
+		PHY_ID_MATCH_EXACT(PHYID_BCM4912),
+		.name		= "Broadcom BCM4912",
+		.inband_caps	= bcm84881_inband_caps,
+		.config_init	= bcm84881_config_init_2500,
+		.probe		= bcm84881_probe,
+		.get_features	= bcm84881_get_features,
+		.config_aneg	= bcm84881_config_aneg,
+		.aneg_done	= bcm84881_aneg_done,
+		.read_status	= bcm84881_read_status,
Please create a macro that simplifies the creation of such entries:

#define BCM84881_PHY_ENTRY(id, speed)			\
{							\
	PHY_ID_MATCH_EXACT(PHYID_##id),			\
	.name	= "Broadcom BCM" # __stringify(id),	\
	.inband_caps	= bcm84881_inband_caps,		\
	.config_init	= bcm84881_config_init_## speed, \
	.probe		= bcm84881_probe,		\
	.get_Features	= bcm84881_get_features,	\
	.aneg_done	= bcm84881_aneg_done,		\
	.read_status	= bcm84881_read_status,		\
}

And then it just becomes:

	BCM84881_PHY_ENTRY(4912, 2500),
	BCM84881_PHY_ENTRY(4912M, 2500),

etc.
-- 
Florian
Related discussions
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help