[PATCH net-next] net: phy: bcm84881: add BCM84891/BCM84892 support

[PATCH net-next] net: phy: bcm84881: add BCM84891/BCM84892 support

[Daniel Wagner]

The BCM84891 and BCM84892 are 10GBASE-T PHYs in the same family as the
BCM84881, sharing the register map and most callbacks. They add USXGMII
as a host interface mode.

There are three behavioral differences from BCM84881:

- USXGMII host interface: the 0x4011 host mode register reports 10GBASER
  (the USXGMII SerDes line rate) regardless of the negotiated copper
  speed. Skip it for USXGMII; phy_resolve_aneg_linkmode() has already
  set the correct speed from standard C45 AN resolution.

- Low power: the PHY firmware has been observed setting MDIO_CTRL1_LPOWER
  autonomously (e.g. during SerDes reconfiguration). Clear it in
  config_aneg() so AN can proceed.

- Speed LED: bicolor (green/amber) via vendor registers Dev1:0xa82f
  (amber gate) and Dev1:0xa83b (color select). 10G = green, else amber,
  matching vendor firmware behavior. Cleared at config_init() since the
  bootloader may leave it on.

The is_bcm8489x() helper checks phydev->drv->phy_id rather than
phydev->phy_id: for C45-enumerated devices, phy_id is left at 0 by
get_phy_device() and matching uses c45_ids.device_ids[].

Tested on TRENDnet TEG-S750 (RTL9303 + 1x BCM84891 + 4x BCM84892)
running OpenWrt, where the MDIO controller driver is currently
OpenWrt-specific. Link verified at 100M, 1G, 2.5G, 10G.

Signed-off-by: Daniel Wagner <wagner.daniel.t@gmail.com>
---
 drivers/net/phy/bcm84881.c | 97 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/bcm84881.c b/drivers/net/phy/bcm84881.c
index d7f7cc44c5..e95242486d 100644
--- a/drivers/net/phy/bcm84881.c
+++ b/drivers/net/phy/bcm84881.c
@@ -20,6 +20,10 @@ enum {
 	MDIO_AN_C22 = 0xffe0,
 };
 
+/* BCM84891/92 vendor registers */
+#define BCM8489X_LED_GATE		0xa82f	/* Dev1: gates amber only */
+#define BCM8489X_LED_COLOR		0xa83b	/* Dev1: color select */
+
 static int bcm84881_wait_init(struct phy_device *phydev)
 {
 	int val;
@@ -29,6 +33,17 @@ static int bcm84881_wait_init(struct phy_device *phydev)
 					 100000, 2000000, false);
 }
 
+static bool bcm84881_is_bcm8489x(struct phy_device *phydev)
+{
+	/* For C45 PHYs, phydev->phy_id is 0; match via the driver entry's
+	 * declared ID, which is what phy_bus_match() used to bind us.
+	 */
+	u32 id = phydev->drv->phy_id;
+
+	return (id & 0xfffffff0) == 0x35905080 ||
+	       (id & 0xfffffff0) == 0x359050a0;
+}
+
 static void bcm84881_fill_possible_interfaces(struct phy_device *phydev)
 {
 	unsigned long *possible = phydev->possible_interfaces;
@@ -42,10 +57,22 @@ static int bcm84881_config_init(struct phy_device *phydev)
 {
 	bcm84881_fill_possible_interfaces(phydev);
 
+	if (bcm84881_is_bcm8489x(phydev)) {
+		__set_bit(PHY_INTERFACE_MODE_USXGMII,
+			  phydev->possible_interfaces);
+		/* Bootloader may have left the speed LED on, and
+		 * link_change_notify won't fire for ports with no cable.
+		 * Clear both color bits (green ignores the gate register).
+		 */
+		phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD,
+				   BCM8489X_LED_COLOR, 0x0012);
+	}
+
 	switch (phydev->interface) {
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_2500BASEX:
 	case PHY_INTERFACE_MODE_10GBASER:
+	case PHY_INTERFACE_MODE_USXGMII:
 		break;
 	default:
 		return -ENODEV;
@@ -96,6 +123,17 @@ static int bcm84881_config_aneg(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
+	/* BCM84891/92 firmware has been observed setting MDIO_CTRL1_LPOWER
+	 * autonomously (e.g. during SerDes reconfiguration). Clear it so AN
+	 * can proceed.
+	 */
+	if (bcm84881_is_bcm8489x(phydev)) {
+		ret = phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1,
+					 MDIO_CTRL1_LPOWER);
+		if (ret < 0)
+			return ret;
+	}
+
 	/* We don't support manual MDI control */
 	phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
 
@@ -201,6 +239,15 @@ static int bcm84881_read_status(struct phy_device *phydev)
 		return 0;
 	}
 
+	/* For BCM84891/92 in USXGMII mode, the host-side register 0x4011
+	 * reports 10GBASER (the USXGMII SerDes line rate) regardless of the
+	 * negotiated copper speed. Skip it; phy_resolve_aneg_linkmode()
+	 * above already set the correct speed from standard AN resolution.
+	 */
+	if (bcm84881_is_bcm8489x(phydev) &&
+	    phydev->interface == PHY_INTERFACE_MODE_USXGMII)
+		return genphy_c45_read_mdix(phydev);
+
 	/* Set the host link mode - we set the phy interface mode and
 	 * the speed according to this register so that downshift works.
 	 * We leave the duplex setting as per the resolution from the
@@ -235,6 +282,26 @@ static int bcm84881_read_status(struct phy_device *phydev)
 	return genphy_c45_read_mdix(phydev);
 }
 
+/* BCM8489x speed LED control.
+ * Dev1:a83b bit 1 (0x0002) = green, bit 4 (0x0010) = amber.
+ * Dev1:a82f = 0x0020 gates amber on; green ignores the gate.
+ * Writes are best-effort (LED is cosmetic; this callback is void).
+ */
+static void bcm84881_link_change_notify(struct phy_device *phydev)
+{
+	if (phydev->link) {
+		/* 10G = green, else amber; matches vendor firmware */
+		u16 color = (phydev->speed == SPEED_10000) ? 0x0002 : 0x0010;
+
+		phy_write_mmd(phydev, MDIO_MMD_PMAPMD, BCM8489X_LED_GATE, 0x0020);
+		phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, BCM8489X_LED_COLOR,
+			       0x0012, color);
+	} else {
+		phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD,
+				   BCM8489X_LED_COLOR, 0x0012);
+	}
+}
+
 /* The Broadcom BCM84881 in the Methode DM7052 is unable to provide a SGMII
  * or 802.3z control word, so inband will not work.
  */
@@ -257,6 +324,32 @@ static struct phy_driver bcm84881_drivers[] = {
 		.aneg_done	= bcm84881_aneg_done,
 		.read_status	= bcm84881_read_status,
 	},
+	{
+		.phy_id		= 0x35905081,
+		.phy_id_mask	= 0xfffffff0,
+		.name		= "Broadcom BCM84891",
+		.inband_caps	= bcm84881_inband_caps,
+		.config_init	= bcm84881_config_init,
+		.probe		= bcm84881_probe,
+		.get_features	= bcm84881_get_features,
+		.config_aneg	= bcm84881_config_aneg,
+		.aneg_done	= bcm84881_aneg_done,
+		.read_status	= bcm84881_read_status,
+		.link_change_notify = bcm84881_link_change_notify,
+	},
+	{
+		.phy_id		= 0x359050a1,
+		.phy_id_mask	= 0xfffffff0,
+		.name		= "Broadcom BCM84892",
+		.inband_caps	= bcm84881_inband_caps,
+		.config_init	= bcm84881_config_init,
+		.probe		= bcm84881_probe,
+		.get_features	= bcm84881_get_features,
+		.config_aneg	= bcm84881_config_aneg,
+		.aneg_done	= bcm84881_aneg_done,
+		.read_status	= bcm84881_read_status,
+		.link_change_notify = bcm84881_link_change_notify,
+	},
 };
 
 module_phy_driver(bcm84881_drivers);
@@ -264,9 +357,11 @@ module_phy_driver(bcm84881_drivers);
 /* FIXME: module auto-loading for Clause 45 PHYs seems non-functional */
 static const struct mdio_device_id __maybe_unused bcm84881_tbl[] = {
 	{ 0xae025150, 0xfffffff0 },
+	{ 0x35905081, 0xfffffff0 },
+	{ 0x359050a1, 0xfffffff0 },
 	{ },
 };
 MODULE_AUTHOR("Russell King");
-MODULE_DESCRIPTION("Broadcom BCM84881 PHY driver");
+MODULE_DESCRIPTION("Broadcom BCM84881/BCM84891/BCM84892 PHY driver");
 MODULE_DEVICE_TABLE(mdio, bcm84881_tbl);
 MODULE_LICENSE("GPL");
-- 
2.47.3

Re: [PATCH net-next] net: phy: bcm84881: add BCM84891/BCM84892 support

[Russell King (Oracle)]

On Tue, Mar 24, 2026 at 03:25:04PM +0000, Daniel Wagner wrote:
> +static bool bcm84881_is_bcm8489x(struct phy_device *phydev)
> +{
> +	/* For C45 PHYs, phydev->phy_id is 0; match via the driver entry's
> +	 * declared ID, which is what phy_bus_match() used to bind us.
> +	 */
> +	u32 id = phydev->drv->phy_id;
> +
> +	return (id & 0xfffffff0) == 0x35905080 ||
> +	       (id & 0xfffffff0) == 0x359050a0;

With the .phy_id's fixed (dropping the revision field) you can simply
test phydev->drv->phy_id here. Alternatively, consider using
phy_id_compare_model().

> +}
> +
>  static void bcm84881_fill_possible_interfaces(struct phy_device *phydev)
>  {
>  	unsigned long *possible = phydev->possible_interfaces;
> @@ -42,10 +57,22 @@ static int bcm84881_config_init(struct phy_device *phydev)
>  {
>  	bcm84881_fill_possible_interfaces(phydev);
>  
> +	if (bcm84881_is_bcm8489x(phydev)) {
> +		__set_bit(PHY_INTERFACE_MODE_USXGMII,
> +			  phydev->possible_interfaces);
> +		/* Bootloader may have left the speed LED on, and
> +		 * link_change_notify won't fire for ports with no cable.
> +		 * Clear both color bits (green ignores the gate register).
> +		 */
> +		phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD,
> +				   BCM8489X_LED_COLOR, 0x0012);
> +	}
> +
>  	switch (phydev->interface) {
>  	case PHY_INTERFACE_MODE_SGMII:
>  	case PHY_INTERFACE_MODE_2500BASEX:
>  	case PHY_INTERFACE_MODE_10GBASER:
> +	case PHY_INTERFACE_MODE_USXGMII:

This also allows USXGMII for BCM84881, which it doesn't support. This is
wrong. Does the bcm8489x support these other modes? If not, this is more
wrong - it should be a separate function - bcm8489x_config_init().

>  		break;
>  	default:
>  		return -ENODEV;
> @@ -96,6 +123,17 @@ static int bcm84881_config_aneg(struct phy_device *phydev)
>  	if (ret)
>  		return ret;
>  
> +	/* BCM84891/92 firmware has been observed setting MDIO_CTRL1_LPOWER
> +	 * autonomously (e.g. during SerDes reconfiguration). Clear it so AN
> +	 * can proceed.
> +	 */
> +	if (bcm84881_is_bcm8489x(phydev)) {
> +		ret = phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1,
> +					 MDIO_CTRL1_LPOWER);
> +		if (ret < 0)
> +			return ret;
> +	}
> +

This function only gets called when the PHY is being started, resumed,
or when the user changes the advertisement. If firmware switches to low
power when e.g. the results of the media negotiation change, then this
isn't the correct place to do this. When exactly does firmware set the
PHY to low power mode?

>  	/* We don't support manual MDI control */
>  	phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
>  
> @@ -201,6 +239,15 @@ static int bcm84881_read_status(struct phy_device *phydev)
>  		return 0;
>  	}
>  
> +	/* For BCM84891/92 in USXGMII mode, the host-side register 0x4011
> +	 * reports 10GBASER (the USXGMII SerDes line rate) regardless of the
> +	 * negotiated copper speed. Skip it; phy_resolve_aneg_linkmode()
> +	 * above already set the correct speed from standard AN resolution.
> +	 */

Is the PHY really using 10GBASE-R or is it using USXGMII. The two are
different. They don't just define the SerDes rate, but the protocol
that operates over it. 10GBASE-R has no inband signalling whereas
USXGMII does and supports symbol replication for slower speeds.

If the PHY is actually using 10GBASE-R, but with rate adaption (so
converting between the media speed and 10GBASE-R which only runs at 10G
rate) then you need more in this driver (see rate adaption stuff.)

If the PHY is really using USXGMII, where it will do symbol replication,
for the slower speeds over the 10G host-side link then the rate
adaption stuff is unnecessary.

Is there a code in the 0x4011 register which indicates if it uses
USXGMII as opposed to 10GBASE-R ? Please clarify.

> +	if (bcm84881_is_bcm8489x(phydev) &&
> +	    phydev->interface == PHY_INTERFACE_MODE_USXGMII)
> +		return genphy_c45_read_mdix(phydev);
> +
>  	/* Set the host link mode - we set the phy interface mode and
>  	 * the speed according to this register so that downshift works.
>  	 * We leave the duplex setting as per the resolution from the
> @@ -235,6 +282,26 @@ static int bcm84881_read_status(struct phy_device *phydev)
>  	return genphy_c45_read_mdix(phydev);
>  }
>  
> +/* BCM8489x speed LED control.
> + * Dev1:a83b bit 1 (0x0002) = green, bit 4 (0x0010) = amber.
> + * Dev1:a82f = 0x0020 gates amber on; green ignores the gate.
> + * Writes are best-effort (LED is cosmetic; this callback is void).
> + */
> +static void bcm84881_link_change_notify(struct phy_device *phydev)

While the driver is called bcm84881, I'd prefer if we didn't name
functions with the same prefix when they have nothing to do with the
bcm84881 PHY. bcm8489x_link_change_notify() would be more suitable.

> +{
> +	if (phydev->link) {
> +		/* 10G = green, else amber; matches vendor firmware */
> +		u16 color = (phydev->speed == SPEED_10000) ? 0x0002 : 0x0010;
> +
> +		phy_write_mmd(phydev, MDIO_MMD_PMAPMD, BCM8489X_LED_GATE, 0x0020);
> +		phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, BCM8489X_LED_COLOR,
> +			       0x0012, color);
> +	} else {
> +		phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD,
> +				   BCM8489X_LED_COLOR, 0x0012);
> +	}

... however, this is not how we support LEDs. Please look at the PHY
LEDs support using the generic LEDs support.

> +}
> +
>  /* The Broadcom BCM84881 in the Methode DM7052 is unable to provide a SGMII
>   * or 802.3z control word, so inband will not work.
>   */
> @@ -257,6 +324,32 @@ static struct phy_driver bcm84881_drivers[] = {
>  		.aneg_done	= bcm84881_aneg_done,
>  		.read_status	= bcm84881_read_status,
>  	},
> +	{
> +		.phy_id		= 0x35905081,
> +		.phy_id_mask	= 0xfffffff0,

Why set the revision in .phy_id when you mask it off? Please use
PHY_ID_MATCH_MODEL().

> +		.name		= "Broadcom BCM84891",
> +		.inband_caps	= bcm84881_inband_caps,
> +		.config_init	= bcm84881_config_init,
> +		.probe		= bcm84881_probe,
> +		.get_features	= bcm84881_get_features,
> +		.config_aneg	= bcm84881_config_aneg,
> +		.aneg_done	= bcm84881_aneg_done,
> +		.read_status	= bcm84881_read_status,
> +		.link_change_notify = bcm84881_link_change_notify,
> +	},
> +	{
> +		.phy_id		= 0x359050a1,
> +		.phy_id_mask	= 0xfffffff0,

Same here.

Thanks.

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

Re: [PATCH net-next] net: phy: bcm84881: add BCM84891/BCM84892 support

[Andrew Lunn]

> +static bool bcm84881_is_bcm8489x(struct phy_device *phydev)
> +{
> +	/* For C45 PHYs, phydev->phy_id is 0; match via the driver entry's
> +	 * declared ID, which is what phy_bus_match() used to bind us.
> +	 */
> +	u32 id = phydev->drv->phy_id;
> +
> +	return (id & 0xfffffff0) == 0x35905080 ||
> +	       (id & 0xfffffff0) == 0x359050a0;

phy_id_compare_model(). 

> +/* BCM8489x speed LED control.
> + * Dev1:a83b bit 1 (0x0002) = green, bit 4 (0x0010) = amber.
> + * Dev1:a82f = 0x0020 gates amber on; green ignores the gate.
> + * Writes are best-effort (LED is cosmetic; this callback is void).
> + */
> +static void bcm84881_link_change_notify(struct phy_device *phydev)
> +{
> +	if (phydev->link) {
> +		/* 10G = green, else amber; matches vendor firmware */
> +		u16 color = (phydev->speed == SPEED_10000) ? 0x0002 : 0x0010;
> +
> +		phy_write_mmd(phydev, MDIO_MMD_PMAPMD, BCM8489X_LED_GATE, 0x0020);
> +		phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, BCM8489X_LED_COLOR,
> +			       0x0012, color);
> +	} else {
> +		phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD,
> +				   BCM8489X_LED_COLOR, 0x0012);
> +	}
> +}

What are the capabilities of the LEDs?

This is going to cause problems in the future if somebody implements
proper control of the LEDs via /sys/class/leds and the netdev trigger.

> +	{
> +		.phy_id		= 0x35905081,
> +		.phy_id_mask	= 0xfffffff0,

PHY_ID_MATCH_MODEL()

> +		.name		= "Broadcom BCM84891",
> +		.inband_caps	= bcm84881_inband_caps,
> +		.config_init	= bcm84881_config_init,
> +		.probe		= bcm84881_probe,
> +		.get_features	= bcm84881_get_features,
> +		.config_aneg	= bcm84881_config_aneg,
> +		.aneg_done	= bcm84881_aneg_done,
> +		.read_status	= bcm84881_read_status,
> +		.link_change_notify = bcm84881_link_change_notify,
> +	},
> +	{
> +		.phy_id		= 0x359050a1,
> +		.phy_id_mask	= 0xfffffff0,

PHY_ID_MATCH_MODEL()


    Andrew

---
pw-bot: cr

Re: [PATCH net-next] net: phy: bcm84881: add BCM84891/BCM84892 support

[Daniel Wagner]

Thanks both for the quick review.

On Tue, Mar 24, 2026 at 3:50 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
> > +     case PHY_INTERFACE_MODE_USXGMII:
>
> This also allows USXGMII for BCM84881, which it doesn't support. This is
> wrong. Does the bcm8489x support these other modes? If not, this is more
> wrong - it should be a separate function - bcm8489x_config_init().

Good point. v2 will have a separate bcm8489x_config_init() that only allows
USXGMII -- the only mode available on the hardware I have access to. I don't
know whether these PHYs support other modes.

This means that since only bcm8489x_config_init() allows USXGMII, we can use
the interface mode to discriminate in read_status, eliminating is_bcm8489x().

> This function only gets called when the PHY is being started, resumed,
> or when the user changes the advertisement. If firmware switches to low
> power when e.g. the results of the media negotiation change, then this
> isn't the correct place to do this. When exactly does firmware set the
> PHY to low power mode?

I went back to characterize this more thoroughly and found my v1 claim about
SerDes reconfiguration was overly broad. I can substantiate:

- At boot: comes up in low power mode. Requires clearing for link.
- ifdown/ifup: does not reappear. Tested with the clear moved to
  config_init (so config_aneg no longer touches it)
- Cable unplug/replug: does not reappear.
- Link-partner advertisement changes: I used ethtool on the link partner to
  force renegotiation to different speeds. All links negotiated fine under
  config_init-only.

v2 moves the clear to bcm8489x_config_init().

> Is the PHY really using 10GBASE-R or is it using USXGMII. The two are
> different.
[...]
> Is there a code in the 0x4011 register which indicates if it uses
> USXGMII as opposed to 10GBASE-R ? Please clarify.

It's USXGMII with symbol replication. The MAC receives the negotiated copper
speed, not 10G -- phylink's mac_link_up() is called with the copper speed.
Link works at 100M/1G/2.5G with no rate_matching in the driver, which I think
wouldn't be the case for 10GBASE-R with rate adaptation.

On 0x4011: it reads the same value (0x3107, mode=3) at 1G and at 2.5G copper,
not tracking copper speed. There's no separate USXGMII encoding I can find --
mode=3 appears to mean "10G SerDes line rate" regardless of protocol, but
admittedly I don't have access to data sheets so this is inference.

> While the driver is called bcm84881, I'd prefer if we didn't name
> functions with the same prefix when they have nothing to do with the
> bcm84881 PHY. bcm8489x_link_change_notify() would be more suitable.
>
> ... however, this is not how we support LEDs. Please look at the PHY
> LEDs support using the generic LEDs support.

In that case let me drop LEDs from v2 for now. The hardware is a single
bicolor LED (green/amber) with speed-based color selection (vendor firmware
does 10G=green, else amber). I don't immediately see an obvious way to express
that in the LED framework. If there's a good example to follow I'd appreciate
a pointer; otherwise I'll figure it out as a follow-up.

> Why set the revision in .phy_id when you mask it off? Please use
> PHY_ID_MATCH_MODEL().

Done.

v2 follows.

Thanks,
Daniel

[PATCH net-next v2] net: phy: bcm84881: add BCM84891/BCM84892 support

[Daniel Wagner]

The BCM84891 and BCM84892 are 10GBASE-T PHYs in the same family as the
BCM84881, sharing the register map and most callbacks. They add USXGMII
as a host interface mode.

bcm8489x_config_init() is separate from bcm84881_config_init(): it
allows only USXGMII (the only host mode available on the tested
hardware) and clears MDIO_CTRL1_LPOWER, which is set at boot on the
tested platform. Does not recur on ifdown/ifup, cable events, or
link-partner advertisement changes, so config_init is sufficient.

For USXGMII, read_status() skips the 0x4011 host-mode register: it
returns the same value regardless of negotiated copper speed (USXGMII
symbol replication). Speed comes from phy_resolve_aneg_linkmode() via
standard C45 AN resolution.

Tested on TRENDnet TEG-S750 (RTL9303 + 1x BCM84891 + 4x BCM84892)
running OpenWrt, where the MDIO controller driver is currently
OpenWrt-specific. Link verified at 100M, 1G, 2.5G, 10G.

Signed-off-by: Daniel Wagner <wagner.daniel.t@gmail.com>
---
Changes in v2:
- Separate bcm8489x_config_init() that allows only USXGMII. v1 also
  allowed USXGMII for BCM84881 (which doesn't support it), and put
  SGMII/2500BASEX/10GBASER in the 8489x possible_interfaces which I
  can't substantiate on this hardware. (Russell)
- LPOWER clear moved from config_aneg to config_init. Characterized on
  hardware: boot-time only, does not recur on ifdown/ifup, cable
  events, or link-partner advertisement changes. (Russell)
- Dropped LED support; will figure out the PHY LED framework approach
  for bicolor speed-mapped LEDs as a follow-up. (Russell, Andrew)
- PHY_ID_MATCH_MODEL(). (Russell, Andrew)
- is_bcm8489x() helper removed; the interface mode now suffices as a
  discriminator in read_status since only the 8489x config_init allows
  USXGMII.

v1: https://lore.kernel.org/netdev/20260324152503.1522071-2-wagner.daniel.t@gmail.com/

 drivers/net/phy/bcm84881.c | 48 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/bcm84881.c b/drivers/net/phy/bcm84881.c
index d7f7cc44c5..f114212dd3 100644
--- a/drivers/net/phy/bcm84881.c
+++ b/drivers/net/phy/bcm84881.c
@@ -54,6 +54,21 @@ static int bcm84881_config_init(struct phy_device *phydev)
 	return 0;
 }
 
+static int bcm8489x_config_init(struct phy_device *phydev)
+{
+	__set_bit(PHY_INTERFACE_MODE_USXGMII, phydev->possible_interfaces);
+
+	if (phydev->interface != PHY_INTERFACE_MODE_USXGMII)
+		return -ENODEV;
+
+	/* MDIO_CTRL1_LPOWER is set at boot on the tested platform. Does not
+	 * recur on ifdown/ifup, cable events, or link-partner advertisement
+	 * changes; clear it once.
+	 */
+	return phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1,
+				  MDIO_CTRL1_LPOWER);
+}
+
 static int bcm84881_probe(struct phy_device *phydev)
 {
 	/* This driver requires PMAPMD and AN blocks */
@@ -201,6 +216,15 @@ static int bcm84881_read_status(struct phy_device *phydev)
 		return 0;
 	}
 
+	/* BCM84891/92 on USXGMII: the host interface mode doesn't change
+	 * with copper speed (USXGMII symbol replication; the MAC receives
+	 * the negotiated copper speed, not 10G, so no rate adaptation).
+	 * Skip 0x4011; phy_resolve_aneg_linkmode() above already set the
+	 * speed. Only bcm8489x_config_init() allows USXGMII.
+	 */
+	if (phydev->interface == PHY_INTERFACE_MODE_USXGMII)
+		return genphy_c45_read_mdix(phydev);
+
 	/* Set the host link mode - we set the phy interface mode and
 	 * the speed according to this register so that downshift works.
 	 * We leave the duplex setting as per the resolution from the
@@ -256,6 +280,26 @@ static struct phy_driver bcm84881_drivers[] = {
 		.config_aneg	= bcm84881_config_aneg,
 		.aneg_done	= bcm84881_aneg_done,
 		.read_status	= bcm84881_read_status,
+	}, {
+		PHY_ID_MATCH_MODEL(0x35905080),
+		.name		= "Broadcom BCM84891",
+		.inband_caps	= bcm84881_inband_caps,
+		.config_init	= bcm8489x_config_init,
+		.probe		= bcm84881_probe,
+		.get_features	= bcm84881_get_features,
+		.config_aneg	= bcm84881_config_aneg,
+		.aneg_done	= bcm84881_aneg_done,
+		.read_status	= bcm84881_read_status,
+	}, {
+		PHY_ID_MATCH_MODEL(0x359050a0),
+		.name		= "Broadcom BCM84892",
+		.inband_caps	= bcm84881_inband_caps,
+		.config_init	= bcm8489x_config_init,
+		.probe		= bcm84881_probe,
+		.get_features	= bcm84881_get_features,
+		.config_aneg	= bcm84881_config_aneg,
+		.aneg_done	= bcm84881_aneg_done,
+		.read_status	= bcm84881_read_status,
 	},
 };
 
@@ -264,9 +308,11 @@ module_phy_driver(bcm84881_drivers);
 /* FIXME: module auto-loading for Clause 45 PHYs seems non-functional */
 static const struct mdio_device_id __maybe_unused bcm84881_tbl[] = {
 	{ 0xae025150, 0xfffffff0 },
+	{ PHY_ID_MATCH_MODEL(0x35905080) },
+	{ PHY_ID_MATCH_MODEL(0x359050a0) },
 	{ },
 };
 MODULE_AUTHOR("Russell King");
-MODULE_DESCRIPTION("Broadcom BCM84881 PHY driver");
+MODULE_DESCRIPTION("Broadcom BCM84881/BCM84891/BCM84892 PHY driver");
 MODULE_DEVICE_TABLE(mdio, bcm84881_tbl);
 MODULE_LICENSE("GPL");
-- 
2.47.3

Re: [PATCH net-next] net: phy: bcm84881: add BCM84891/BCM84892 support

[Andrew Lunn]

> In that case let me drop LEDs from v2 for now. The hardware is a single
> bicolor LED (green/amber) with speed-based color selection (vendor firmware
> does 10G=green, else amber). I don't immediately see an obvious way to express
> that in the LED framework. If there's a good example to follow I'd appreciate
> a pointer; otherwise I'll figure it out as a follow-up.

That is why asked about its capabilities.

The minimum you need for /sys/class/leds is to be able to turn the LED
on and off.

What sort of bicolor LED is it? 2 legs or three? Are green and amber
mutually exclusive, or can you control them independently? If they are
independent, then you can represent it as two LEDs. If the colours are
mutually exclusive its a different story.

Oh, unusual, there is a datasheet:

https://datasheet4u.com/pdf/1561522/BCM84891L.pdf

That is not like Broadcom.

The LED controller has what is needed to follow the normal way of
doing it. So please do drop this part of the patch. For examples, look
at any PHY driver which implements .led_brightness_set and the other
ops in struct phy_driver.

	Andrew

Re: [PATCH net-next] net: phy: bcm84881: add BCM84891/BCM84892 support

[Russell King (Oracle)]

On Tue, Mar 24, 2026 at 06:54:29PM +0000, Daniel Wagner wrote:
> Thanks both for the quick review.
> 
> On Tue, Mar 24, 2026 at 3:50 PM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> > Is the PHY really using 10GBASE-R or is it using USXGMII. The two are
> > different.
> [...]
> > Is there a code in the 0x4011 register which indicates if it uses
> > USXGMII as opposed to 10GBASE-R ? Please clarify.
> 
> It's USXGMII with symbol replication. The MAC receives the negotiated copper
> speed, not 10G -- phylink's mac_link_up() is called with the copper speed.
> Link works at 100M/1G/2.5G with no rate_matching in the driver, which I think
> wouldn't be the case for 10GBASE-R with rate adaptation.

One would expect so.

> On 0x4011: it reads the same value (0x3107, mode=3) at 1G and at 2.5G copper,
> not tracking copper speed. There's no separate USXGMII encoding I can find --
> mode=3 appears to mean "10G SerDes line rate" regardless of protocol, but
> admittedly I don't have access to data sheets so this is inference.

Looking at the information I have for this register:
bit 0 should be link bit (0 = link down, 1 = link up)
bits 4:1 indicates the interface and speed over interface:
	3  = 10GBASE-R
	...
	9, 10, 12 and 13 indicate USXGMII at various link speeds.

So, as you're seeing 3 for this field, that makes me somewhat
suspicious whether this register is implemented the same on these
devices.

Can you confirm whether bit 0 changes depending whether the PHY has
link. I don't know whether that's media link or host-side link, so
I suggest checking both.

Thanks.

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

Re: [PATCH net-next] net: phy: bcm84881: add BCM84891/BCM84892 support

[Daniel Wagner]

On Tue, Mar 24, 2026 at 7:18 PM Andrew Lunn <andrew@lunn.ch> wrote:
> What sort of bicolor LED is it? 2 legs or three? Are green and amber
> mutually exclusive, or can you control them independently? If they are
> independent, then you can represent it as two LEDs. If the colours are
> mutually exclusive its a different story.

Two legs, mutually exclusive.

> Oh, unusual, there is a datasheet:
>
> https://datasheet4u.com/pdf/1561522/BCM84891L.pdf
>
> That is not like Broadcom.

Thank you, this is a great find! I hadn't spotted this when I started
looking into the platform. I will digest a bit more but so far it
looks like the pins are independent but setting both wouldn't work
with a 2-pin LED.

Daniel

Re: [PATCH net-next] net: phy: bcm84881: add BCM84891/BCM84892 support

[Russell King (Oracle)]

On Tue, Mar 24, 2026 at 08:18:21PM +0100, Andrew Lunn wrote:
> > In that case let me drop LEDs from v2 for now. The hardware is a single
> > bicolor LED (green/amber) with speed-based color selection (vendor firmware
> > does 10G=green, else amber). I don't immediately see an obvious way to express
> > that in the LED framework. If there's a good example to follow I'd appreciate
> > a pointer; otherwise I'll figure it out as a follow-up.
> 
> That is why asked about its capabilities.
> 
> The minimum you need for /sys/class/leds is to be able to turn the LED
> on and off.
> 
> What sort of bicolor LED is it? 2 legs or three? Are green and amber
> mutually exclusive, or can you control them independently? If they are
> independent, then you can represent it as two LEDs. If the colours are
> mutually exclusive its a different story.
> 
> Oh, unusual, there is a datasheet:
> 
> https://datasheet4u.com/pdf/1561522/BCM84891L.pdf
> 
> That is not like Broadcom.

That looks like 0x4011 isn't documented on this PHY.

There's also a bunch of MDIO commands that can be used to enquire the
status of USXGMII enable/AN enable, and what mode is used for 2.5G
and 5G.

It also states:

XFI/10GBASE-KR, USXGMII, 5000BASE-X, 2500BASE-X, 5000BASE-R, 2500BASE-R,
and 1000BASE-X (SGMII) MAC Interface

5G rate over USXGMII/XFI/5000BASE-R/5000BASE-X MAC interface

2.5G rate over USXGMII/XFI/2500BASE-R/2500BASE-X MAC interface

It doesn't say when SGMII mode is used (assuming it does support SGMII,
section 1.11 makes it very vague what is actually going on there - I
think someone was having a game of ethernet protocol bingo, finding out
how many different ethernet protocols they can state one after each
other in a single sentence, while making the sentence meaningless!)

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

Re: [PATCH net-next] net: phy: bcm84881: add BCM84891/BCM84892 support

[Daniel Wagner]

> Can you confirm whether bit 0 changes depending whether the PHY has
> link. I don't know whether that's media link or host-side link, so
> I suggest checking both.

Bit 12 does seem to track copper link state in my test:
  [0s] 0x3107  bit12=1   carrier=1   plugged
  [4s] 0x2107  bit12=0   carrier=0   unplugged
  [19s] 0x3107  bit12=1   carrier=0   replugged, PHY sees it
  [20s] 0x3107  bit12=1   carrier=1   phylib poll catches up

Bit 0 stayed at 1 through all of that, and through ifdown/ifup too.
I'm not sure what it is, but it doesn't appear to track either link
under the conditions I can test.

Bits[4:1] stayed at 3 through all of it: copper transitions, different
copper speeds (1G, 2.5G), ifdown/ifup, on both BCM84891 and BCM84892.
No USXGMII codes.

> That looks like 0x4011 isn't documented on this PHY.

Indeed, it jumps from 0x4010 to 0x401A

> There's also a bunch of MDIO commands that can be used to enquire the
> status of USXGMII enable/AN enable, and what mode is used for 2.5G
> and 5G.

U-Boot on my device uses SET_USXGMII during PHY init. I didn't plumb
the mailbox into the driver since phydev->interface from DT already
tells us the mode.

Daniel

Re: [PATCH net-next] net: phy: bcm84881: add BCM84891/BCM84892 support

[Russell King (Oracle)]

On Tue, Mar 24, 2026 at 09:59:02PM +0000, Daniel Wagner wrote:
> > Can you confirm whether bit 0 changes depending whether the PHY has
> > link. I don't know whether that's media link or host-side link, so
> > I suggest checking both.
> 
> Bit 12 does seem to track copper link state in my test:
>   [0s] 0x3107  bit12=1   carrier=1   plugged
>   [4s] 0x2107  bit12=0   carrier=0   unplugged
>   [19s] 0x3107  bit12=1   carrier=0   replugged, PHY sees it
>   [20s] 0x3107  bit12=1   carrier=1   phylib poll catches up
> 
> Bit 0 stayed at 1 through all of that, and through ifdown/ifup too.
> I'm not sure what it is, but it doesn't appear to track either link
> under the conditions I can test.
> 
> Bits[4:1] stayed at 3 through all of it: copper transitions, different
> copper speeds (1G, 2.5G), ifdown/ifup, on both BCM84891 and BCM84892.
> No USXGMII codes.

I suggest this is not the same register bit allocation as on
the bcm84881 in that case.

Also note that phylib's poll "catches up" because the link bit in
BMSR is intentionally sticky-down to ensure that link-down events are
not missed - if the link was down and then comes up, it intentionally
takes two reads to indicate link up to ensure that the link down state
doesn't get lost.

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