[PATCH net-next v2 0/3] Airoha AN8811HB 2.5 Gbps phy support

[PATCH net-next v2 0/3] Airoha AN8811HB 2.5 Gbps phy support

[Bjørn Mork]

The RFC patch posted earlier has been split into a series based on the
feedback received:

 1/3: preparing the EN8811H driver for maximum reuse
 2/3: adding support for the new AN8811HB hardware
 3/3: adding (optional) clock driver for AN8811HB

Patch 3/3 is not required for a functional device. It is included here
for full feature parity between the EN8811H and AN8811HB drivers.

The AN8811HB phy requires new firmware, which is now available with
the 20260110 release of linux-firmware,

RFC: https://lore.kernel.org/20251202102222.1681522-1-bjorn@mork.no/
v1:  https://lore.kernel.org/20260122071601.1057083-1-bjorn@mork.no/

Changes since v1:
- moved AIR_AUX_CTRL_STATUS_SPEED_10 reference from patch 1 to 2, where
  it is defined, fixing intermediate build error
- rearranged an8811hb_check_crc loop to improve readability for AI and
  other intelligent beings

Changes since RFC:
- refactoring of the existing EN8811H code is separated into a patch
  of its own
- ID conditionals are dropped, using separate functions for the two
  hardware variants instead. Some common helpers have been added to
  reduse code duplication
- deprecated serdes polarity properties are not supported for AN8811HB
- the polarity configuration code has proper error handling
- a polarity configuration bug is fixed by using the new simplified
  helper interface
- the clock driver is split out in a separate patch to reduce the size
  of the main AN8811HB driver patch

Bjørn Mork (3):
  net: phy: air_en8811h: factor out shareable code
  net: phy: air_en8811h: add Airoha AN8811HB support
  net: phy: air_en8811h: Add clk provider for an8811hb

 drivers/net/phy/air_en8811h.c | 511 ++++++++++++++++++++++++++++++----
 1 file changed, 461 insertions(+), 50 deletions(-)

-- 
2.47.3

[PATCH net-next v2 1/3] net: phy: air_en8811h: factor out shareable code

[Bjørn Mork]

Create reusable helpers in preparation for the AN8811HB support.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/phy/air_en8811h.c | 123 +++++++++++++++++++++-------------
 1 file changed, 77 insertions(+), 46 deletions(-)

diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
index e890bb2c0aa8..e617108fe469 100644
--- a/drivers/net/phy/air_en8811h.c
+++ b/drivers/net/phy/air_en8811h.c
@@ -448,6 +448,11 @@ static int en8811h_wait_mcu_ready(struct phy_device *phydev)
 {
 	int ret, reg_value;
 
+	ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
+				     EN8811H_FW_CTRL_1_FINISH);
+	if (ret)
+		return ret;
+
 	/* Because of mdio-lock, may have to wait for multiple loads */
 	ret = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND1,
 					EN8811H_PHY_FW_STATUS, reg_value,
@@ -461,9 +466,18 @@ static int en8811h_wait_mcu_ready(struct phy_device *phydev)
 	return 0;
 }
 
-static int en8811h_load_firmware(struct phy_device *phydev)
+static void en8811h_print_fw_version(struct phy_device *phydev)
 {
 	struct en8811h_priv *priv = phydev->priv;
+
+	air_buckpbus_reg_read(phydev, EN8811H_FW_VERSION,
+			      &priv->firmware_version);
+	phydev_info(phydev, "MD32 firmware version: %08x\n",
+		    priv->firmware_version);
+}
+
+static int en8811h_load_firmware(struct phy_device *phydev)
+{
 	struct device *dev = &phydev->mdio.dev;
 	const struct firmware *fw1, *fw2;
 	int ret;
@@ -500,17 +514,11 @@ static int en8811h_load_firmware(struct phy_device *phydev)
 	if (ret < 0)
 		goto en8811h_load_firmware_out;
 
-	ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
-				     EN8811H_FW_CTRL_1_FINISH);
+	ret = en8811h_wait_mcu_ready(phydev);
 	if (ret < 0)
 		goto en8811h_load_firmware_out;
 
-	ret = en8811h_wait_mcu_ready(phydev);
-
-	air_buckpbus_reg_read(phydev, EN8811H_FW_VERSION,
-			      &priv->firmware_version);
-	phydev_info(phydev, "MD32 firmware version: %08x\n",
-		    priv->firmware_version);
+	en8811h_print_fw_version(phydev);
 
 en8811h_load_firmware_out:
 	release_firmware(fw2);
@@ -533,11 +541,6 @@ static int en8811h_restart_mcu(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
-	ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
-				     EN8811H_FW_CTRL_1_FINISH);
-	if (ret < 0)
-		return ret;
-
 	return en8811h_wait_mcu_ready(phydev);
 }
 
@@ -919,6 +922,23 @@ static int en8811h_clk_provider_setup(struct device *dev, struct clk_hw *hw)
 	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, hw);
 }
 
+static int en8811h_leds_setup(struct phy_device *phydev)
+{
+	struct en8811h_priv *priv = phydev->priv;
+	int ret;
+
+	priv->led[0].rules = AIR_DEFAULT_TRIGGER_LED0;
+	priv->led[1].rules = AIR_DEFAULT_TRIGGER_LED1;
+	priv->led[2].rules = AIR_DEFAULT_TRIGGER_LED2;
+
+	ret = air_leds_init(phydev, EN8811H_LED_COUNT, AIR_PHY_LED_DUR,
+			    AIR_LED_MODE_DISABLE);
+	if (ret < 0)
+		phydev_err(phydev, "Failed to disable leds: %d\n", ret);
+
+	return ret;
+}
+
 static int en8811h_probe(struct phy_device *phydev)
 {
 	struct en8811h_priv *priv;
@@ -937,19 +957,12 @@ static int en8811h_probe(struct phy_device *phydev)
 	/* mcu has just restarted after firmware load */
 	priv->mcu_needs_restart = false;
 
-	priv->led[0].rules = AIR_DEFAULT_TRIGGER_LED0;
-	priv->led[1].rules = AIR_DEFAULT_TRIGGER_LED1;
-	priv->led[2].rules = AIR_DEFAULT_TRIGGER_LED2;
-
 	/* MDIO_DEVS1/2 empty, so set mmds_present bits here */
 	phydev->c45_ids.mmds_present |= MDIO_DEVS_PMAPMD | MDIO_DEVS_AN;
 
-	ret = air_leds_init(phydev, EN8811H_LED_COUNT, AIR_PHY_LED_DUR,
-			    AIR_LED_MODE_DISABLE);
-	if (ret < 0) {
-		phydev_err(phydev, "Failed to disable leds: %d\n", ret);
+	ret = en8811h_leds_setup(phydev);
+	if (ret < 0)
 		return ret;
-	}
 
 	priv->phydev = phydev;
 	/* Co-Clock Output */
@@ -1090,11 +1103,9 @@ static int en8811h_config_aneg(struct phy_device *phydev)
 	return __genphy_config_aneg(phydev, changed);
 }
 
-static int en8811h_read_status(struct phy_device *phydev)
+static int en8811h_get_lpa(struct phy_device *phydev)
 {
-	struct en8811h_priv *priv = phydev->priv;
-	u32 pbus_value;
-	int ret, val;
+	int ret;
 
 	ret = genphy_update_link(phydev);
 	if (ret)
@@ -1112,23 +1123,12 @@ static int en8811h_read_status(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
-	ret = genphy_read_lpa(phydev);
-	if (ret < 0)
-		return ret;
-
-	/* Get link partner 2.5GBASE-T ability from vendor register */
-	ret = air_buckpbus_reg_read(phydev, EN8811H_2P5G_LPA, &pbus_value);
-	if (ret < 0)
-		return ret;
-	linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
-			 phydev->lp_advertising,
-			 pbus_value & EN8811H_2P5G_LPA_2P5G);
-
-	if (phydev->autoneg_complete)
-		phy_resolve_aneg_pause(phydev);
+	return genphy_read_lpa(phydev);
+}
 
-	if (!phydev->link)
-		return 0;
+static int en8811h_get_speed(struct phy_device *phydev)
+{
+	int val;
 
 	/* Get real speed from vendor register */
 	val = phy_read(phydev, AIR_AUX_CTRL_STATUS);
@@ -1146,6 +1146,40 @@ static int en8811h_read_status(struct phy_device *phydev)
 		break;
 	}
 
+	/* Only supports full duplex */
+	phydev->duplex = DUPLEX_FULL;
+
+	return 0;
+}
+
+static int en8811h_read_status(struct phy_device *phydev)
+{
+	struct en8811h_priv *priv = phydev->priv;
+	u32 pbus_value;
+	int ret;
+
+	ret = en8811h_get_lpa(phydev);
+	if (ret < 0)
+		return ret;
+
+	/* Get link partner 2.5GBASE-T ability from vendor register */
+	ret = air_buckpbus_reg_read(phydev, EN8811H_2P5G_LPA, &pbus_value);
+	if (ret < 0)
+		return ret;
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
+			 phydev->lp_advertising,
+			 pbus_value & EN8811H_2P5G_LPA_2P5G);
+
+	if (phydev->autoneg_complete)
+		phy_resolve_aneg_pause(phydev);
+
+	if (!phydev->link)
+		return 0;
+
+	ret = en8811h_get_speed(phydev);
+	if (ret < 0)
+		return ret;
+
 	/* Firmware before version 24011202 has no vendor register 2P5G_LPA.
 	 * Assume link partner advertised it if connected at 2500Mbps.
 	 */
@@ -1155,9 +1189,6 @@ static int en8811h_read_status(struct phy_device *phydev)
 				 phydev->speed == SPEED_2500);
 	}
 
-	/* Only supports full duplex */
-	phydev->duplex = DUPLEX_FULL;
-
 	return 0;
 }
 
-- 
2.47.3

[PATCH net-next v2 2/3] net: phy: air_en8811h: add Airoha AN8811HB support

[Bjørn Mork]

The Airoha AN8811HB is mostly compatible with the EN8811H, adding 10Base-T
support and reducing power consumption.

This driver is based on the air_an8811hb v0.0.4 out-of-tree driver
written by "Lucien.Jheng <lucien.jheng@airoha.com>"

Firmware is available in linux-firmware. The driver has been tested with
firmware version 25110702

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
Cc: Lucien.Jheng <lucienzx159@gmail.com>
Cc: Daniel Golle <daniel@makrotopia.org>
Cc: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/phy/air_en8811h.c | 281 +++++++++++++++++++++++++++++++++-
 1 file changed, 277 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
index e617108fe469..ed6a094a4185 100644
--- a/drivers/net/phy/air_en8811h.c
+++ b/drivers/net/phy/air_en8811h.c
@@ -1,14 +1,15 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * Driver for the Airoha EN8811H 2.5 Gigabit PHY.
+ * Driver for the Airoha EN8811H and AN8811HB 2.5 Gigabit PHYs.
  *
- * Limitations of the EN8811H:
+ * Limitations:
  * - Only full duplex supported
  * - Forced speed (AN off) is not supported by hardware (100Mbps)
  *
  * Source originated from airoha's en8811h.c and en8811h.h v1.2.1
+ * with AN8811HB bits from air_an8811hb.c v0.0.4
  *
- * Copyright (C) 2023 Airoha Technology Corp.
+ * Copyright (C) 2023, 2026 Airoha Technology Corp.
  */
 
 #include <linux/clk.h>
@@ -21,9 +22,12 @@
 #include <linux/unaligned.h>
 
 #define EN8811H_PHY_ID		0x03a2a411
+#define AN8811HB_PHY_ID		0xc0ff04a0
 
 #define EN8811H_MD32_DM		"airoha/EthMD32.dm.bin"
 #define EN8811H_MD32_DSP	"airoha/EthMD32.DSP.bin"
+#define AN8811HB_MD32_DM	"airoha/an8811hb/EthMD32_CRC.DM.bin"
+#define AN8811HB_MD32_DSP	"airoha/an8811hb/EthMD32_CRC.DSP.bin"
 
 #define AIR_FW_ADDR_DM	0x00000000
 #define AIR_FW_ADDR_DSP	0x00100000
@@ -31,6 +35,7 @@
 /* MII Registers */
 #define AIR_AUX_CTRL_STATUS		0x1d
 #define   AIR_AUX_CTRL_STATUS_SPEED_MASK	GENMASK(4, 2)
+#define   AIR_AUX_CTRL_STATUS_SPEED_10		0x0
 #define   AIR_AUX_CTRL_STATUS_SPEED_100		0x4
 #define   AIR_AUX_CTRL_STATUS_SPEED_1000	0x8
 #define   AIR_AUX_CTRL_STATUS_SPEED_2500	0xc
@@ -56,6 +61,7 @@
 #define EN8811H_PHY_FW_STATUS		0x8009
 #define   EN8811H_PHY_READY			0x02
 
+#define AIR_PHY_MCU_CMD_0		0x800b
 #define AIR_PHY_MCU_CMD_1		0x800c
 #define AIR_PHY_MCU_CMD_1_MODE1			0x0
 #define AIR_PHY_MCU_CMD_2		0x800d
@@ -65,6 +71,10 @@
 #define AIR_PHY_MCU_CMD_3_DOCMD			0x1100
 #define AIR_PHY_MCU_CMD_4		0x800f
 #define AIR_PHY_MCU_CMD_4_MODE1			0x0002
+#define AIR_PHY_MCU_CMD_4_CABLE_PAIR_A		0x00d7
+#define AIR_PHY_MCU_CMD_4_CABLE_PAIR_B		0x00d8
+#define AIR_PHY_MCU_CMD_4_CABLE_PAIR_C		0x00d9
+#define AIR_PHY_MCU_CMD_4_CABLE_PAIR_D		0x00da
 #define AIR_PHY_MCU_CMD_4_INTCLR		0x00e4
 
 /* Registers on MDIO_MMD_VEND2 */
@@ -106,6 +116,9 @@
 #define   AIR_PHY_LED_BLINK_2500RX		BIT(11)
 
 /* Registers on BUCKPBUS */
+#define AIR_PHY_CONTROL			0x3a9c
+#define   AIR_PHY_CONTROL_INTERNAL		BIT(11)
+
 #define EN8811H_2P5G_LPA		0x3b30
 #define   EN8811H_2P5G_LPA_2P5G			BIT(0)
 
@@ -129,6 +142,34 @@
 #define EN8811H_FW_CTRL_2		0x800000
 #define EN8811H_FW_CTRL_2_LOADING		BIT(11)
 
+#define AN8811HB_CRC_PM_SET1		0xf020c
+#define AN8811HB_CRC_PM_MON2		0xf0218
+#define AN8811HB_CRC_PM_MON3		0xf021c
+#define AN8811HB_CRC_DM_SET1		0xf0224
+#define AN8811HB_CRC_DM_MON2		0xf0230
+#define AN8811HB_CRC_DM_MON3		0xf0234
+#define   AN8811HB_CRC_RD_EN			BIT(0)
+#define   AN8811HB_CRC_ST			(BIT(0) | BIT(1))
+#define   AN8811HB_CRC_CHECK_PASS		BIT(0)
+
+#define AN8811HB_TX_POLARITY		0x5ce004
+#define   AN8811HB_TX_POLARITY_NORMAL		BIT(7)
+#define AN8811HB_RX_POLARITY		0x5ce61c
+#define   AN8811HB_RX_POLARITY_NORMAL		BIT(7)
+
+#define AN8811HB_GPIO_OUTPUT		0x5cf8b8
+#define   AN8811HB_GPIO_OUTPUT_345		(BIT(3) | BIT(4) | BIT(5))
+
+#define AN8811HB_HWTRAP1		0x5cf910
+#define AN8811HB_HWTRAP2		0x5cf914
+#define   AN8811HB_HWTRAP2_CKO			BIT(28)
+
+#define AN8811HB_CLK_DRV		0x5cf9e4
+#define AN8811HB_CLK_DRV_CKO_MASK		GENMASK(14, 12)
+#define   AN8811HB_CLK_DRV_CKOPWD		BIT(12)
+#define   AN8811HB_CLK_DRV_CKO_LDPWD		BIT(13)
+#define   AN8811HB_CLK_DRV_CKO_LPPWD		BIT(14)
+
 /* Led definitions */
 #define EN8811H_LED_COUNT	3
 
@@ -466,6 +507,39 @@ static int en8811h_wait_mcu_ready(struct phy_device *phydev)
 	return 0;
 }
 
+static int an8811hb_check_crc(struct phy_device *phydev, u32 set1,
+			      u32 mon2, u32 mon3)
+{
+	u32 pbus_value;
+	int retry = 25;
+	int ret;
+
+	/* Configure CRC */
+	ret = air_buckpbus_reg_modify(phydev, set1,
+				      AN8811HB_CRC_RD_EN,
+				      AN8811HB_CRC_RD_EN);
+	if (ret < 0)
+		return ret;
+	air_buckpbus_reg_read(phydev, set1, &pbus_value);
+
+	do {
+		mdelay(300);
+		air_buckpbus_reg_read(phydev, mon2, &pbus_value);
+
+		if (pbus_value & AN8811HB_CRC_ST) {
+			air_buckpbus_reg_read(phydev, mon3, &pbus_value);
+			phydev_dbg(phydev, "CRC Check %s!\n",
+				   pbus_value & AN8811HB_CRC_CHECK_PASS ?
+					"PASS" : "FAIL");
+			return air_buckpbus_reg_modify(phydev, set1,
+						       AN8811HB_CRC_RD_EN, 0);
+		}
+	} while (--retry);
+
+	phydev_err(phydev, "CRC Check is not ready (%u)\n", pbus_value);
+	return -ENODEV;
+}
+
 static void en8811h_print_fw_version(struct phy_device *phydev)
 {
 	struct en8811h_priv *priv = phydev->priv;
@@ -476,6 +550,63 @@ static void en8811h_print_fw_version(struct phy_device *phydev)
 		    priv->firmware_version);
 }
 
+static int an8811hb_load_firmware(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	const struct firmware *fw1, *fw2;
+	int ret;
+
+	ret = request_firmware_direct(&fw1, AN8811HB_MD32_DM, dev);
+	if (ret < 0)
+		return ret;
+
+	ret = request_firmware_direct(&fw2, AN8811HB_MD32_DSP, dev);
+	if (ret < 0)
+		goto an8811hb_load_firmware_rel1;
+
+	ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
+				     EN8811H_FW_CTRL_1_START);
+	if (ret < 0)
+		goto an8811hb_load_firmware_out;
+
+	ret = air_write_buf(phydev, AIR_FW_ADDR_DM,  fw1);
+	if (ret < 0)
+		goto an8811hb_load_firmware_out;
+
+	ret = an8811hb_check_crc(phydev, AN8811HB_CRC_DM_SET1,
+				 AN8811HB_CRC_DM_MON2,
+				 AN8811HB_CRC_DM_MON3);
+	if (ret < 0)
+		goto an8811hb_load_firmware_out;
+
+	ret = air_write_buf(phydev, AIR_FW_ADDR_DSP, fw2);
+	if (ret < 0)
+		goto an8811hb_load_firmware_out;
+
+	ret = an8811hb_check_crc(phydev, AN8811HB_CRC_PM_SET1,
+				 AN8811HB_CRC_PM_MON2,
+				 AN8811HB_CRC_PM_MON3);
+	if (ret < 0)
+		goto an8811hb_load_firmware_out;
+
+	ret = en8811h_wait_mcu_ready(phydev);
+	if (ret < 0)
+		goto an8811hb_load_firmware_out;
+
+	en8811h_print_fw_version(phydev);
+
+an8811hb_load_firmware_out:
+	release_firmware(fw2);
+
+an8811hb_load_firmware_rel1:
+	release_firmware(fw1);
+
+	if (ret < 0)
+		phydev_err(phydev, "Load firmware failed: %d\n", ret);
+
+	return ret;
+}
+
 static int en8811h_load_firmware(struct phy_device *phydev)
 {
 	struct device *dev = &phydev->mdio.dev;
@@ -939,6 +1070,41 @@ static int en8811h_leds_setup(struct phy_device *phydev)
 	return ret;
 }
 
+static int an8811hb_probe(struct phy_device *phydev)
+{
+	struct en8811h_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(struct en8811h_priv),
+			    GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	phydev->priv = priv;
+
+	ret = an8811hb_load_firmware(phydev);
+	if (ret < 0)
+		return ret;
+
+	/* mcu has just restarted after firmware load */
+	priv->mcu_needs_restart = false;
+
+	/* MDIO_DEVS1/2 empty, so set mmds_present bits here */
+	phydev->c45_ids.mmds_present |= MDIO_DEVS_PMAPMD | MDIO_DEVS_AN;
+
+	ret = en8811h_leds_setup(phydev);
+	if (ret < 0)
+		return ret;
+
+	/* Configure led gpio pins as output */
+	ret = air_buckpbus_reg_modify(phydev, AN8811HB_GPIO_OUTPUT,
+				      AN8811HB_GPIO_OUTPUT_345,
+				      AN8811HB_GPIO_OUTPUT_345);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 static int en8811h_probe(struct phy_device *phydev)
 {
 	struct en8811h_priv *priv;
@@ -980,6 +1146,37 @@ static int en8811h_probe(struct phy_device *phydev)
 	return 0;
 }
 
+static int an8811hb_config_serdes_polarity(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	u32 pbus_value = 0;
+	unsigned int pol;
+	int ret;
+
+	ret = phy_get_manual_rx_polarity(dev_fwnode(dev),
+					 phy_modes(phydev->interface), &pol);
+	if (ret)
+		return ret;
+	if (pol == PHY_POL_NORMAL)
+		pbus_value |= AN8811HB_RX_POLARITY_NORMAL;
+	ret = air_buckpbus_reg_modify(phydev, AN8811HB_RX_POLARITY,
+				      AN8811HB_RX_POLARITY_NORMAL,
+				      pbus_value);
+	if (ret < 0)
+		return ret;
+
+	ret = phy_get_manual_tx_polarity(dev_fwnode(dev),
+					 phy_modes(phydev->interface), &pol);
+	if (ret)
+		return ret;
+	pbus_value = 0;
+	if (pol == PHY_POL_NORMAL)
+		pbus_value |= AN8811HB_TX_POLARITY_NORMAL;
+	return air_buckpbus_reg_modify(phydev, AN8811HB_TX_POLARITY,
+				       AN8811HB_TX_POLARITY_NORMAL,
+				       pbus_value);
+}
+
 static int en8811h_config_serdes_polarity(struct phy_device *phydev)
 {
 	struct device *dev = &phydev->mdio.dev;
@@ -1016,6 +1213,33 @@ static int en8811h_config_serdes_polarity(struct phy_device *phydev)
 				       EN8811H_POLARITY_TX_NORMAL, pbus_value);
 }
 
+static int an8811hb_config_init(struct phy_device *phydev)
+{
+	struct en8811h_priv *priv = phydev->priv;
+	int ret;
+
+	/* If restart happened in .probe(), no need to restart now */
+	if (priv->mcu_needs_restart) {
+		ret = en8811h_restart_mcu(phydev);
+		if (ret < 0)
+			return ret;
+	} else {
+		/* Next calls to .config_init() mcu needs to restart */
+		priv->mcu_needs_restart = true;
+	}
+
+	ret = an8811hb_config_serdes_polarity(phydev);
+	if (ret < 0)
+		return ret;
+
+	ret = air_leds_init(phydev, EN8811H_LED_COUNT, AIR_PHY_LED_DUR,
+			    AIR_LED_MODE_USER_DEFINE);
+	if (ret < 0)
+		phydev_err(phydev, "Failed to initialize leds: %d\n", ret);
+
+	return ret;
+}
+
 static int en8811h_config_init(struct phy_device *phydev)
 {
 	struct en8811h_priv *priv = phydev->priv;
@@ -1144,6 +1368,9 @@ static int en8811h_get_speed(struct phy_device *phydev)
 	case AIR_AUX_CTRL_STATUS_SPEED_100:
 		phydev->speed = SPEED_100;
 		break;
+	case AIR_AUX_CTRL_STATUS_SPEED_10:
+		phydev->speed = SPEED_10;
+		break;
 	}
 
 	/* Only supports full duplex */
@@ -1152,6 +1379,30 @@ static int en8811h_get_speed(struct phy_device *phydev)
 	return 0;
 }
 
+static int an8811hb_read_status(struct phy_device *phydev)
+{
+	int ret, val;
+
+	ret = en8811h_get_lpa(phydev);
+	if (ret < 0)
+		return ret;
+
+	if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) {
+		val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_STAT);
+		if (val < 0)
+			return val;
+		linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
+				 phydev->lp_advertising,
+				 val & MDIO_AN_10GBT_STAT_LP2_5G);
+		phy_resolve_aneg_pause(phydev);
+	}
+
+	if (!phydev->link)
+		return 0;
+
+	return en8811h_get_speed(phydev);
+}
+
 static int en8811h_read_status(struct phy_device *phydev)
 {
 	struct en8811h_priv *priv = phydev->priv;
@@ -1259,20 +1510,42 @@ static struct phy_driver en8811h_driver[] = {
 	.led_brightness_set	= air_led_brightness_set,
 	.led_hw_control_set	= air_led_hw_control_set,
 	.led_hw_control_get	= air_led_hw_control_get,
+},
+{
+	PHY_ID_MATCH_MODEL(AN8811HB_PHY_ID),
+	.name			= "Airoha AN8811HB",
+	.probe			= an8811hb_probe,
+	.get_features		= en8811h_get_features,
+	.config_init		= an8811hb_config_init,
+	.get_rate_matching	= en8811h_get_rate_matching,
+	.config_aneg		= en8811h_config_aneg,
+	.read_status		= an8811hb_read_status,
+	.config_intr		= en8811h_clear_intr,
+	.handle_interrupt	= en8811h_handle_interrupt,
+	.led_hw_is_supported	= en8811h_led_hw_is_supported,
+	.read_page		= air_phy_read_page,
+	.write_page		= air_phy_write_page,
+	.led_blink_set		= air_led_blink_set,
+	.led_brightness_set	= air_led_brightness_set,
+	.led_hw_control_set	= air_led_hw_control_set,
+	.led_hw_control_get	= air_led_hw_control_get,
 } };
 
 module_phy_driver(en8811h_driver);
 
 static const struct mdio_device_id __maybe_unused en8811h_tbl[] = {
 	{ PHY_ID_MATCH_MODEL(EN8811H_PHY_ID) },
+	{ PHY_ID_MATCH_MODEL(AN8811HB_PHY_ID) },
 	{ }
 };
 
 MODULE_DEVICE_TABLE(mdio, en8811h_tbl);
 MODULE_FIRMWARE(EN8811H_MD32_DM);
 MODULE_FIRMWARE(EN8811H_MD32_DSP);
+MODULE_FIRMWARE(AN8811HB_MD32_DM);
+MODULE_FIRMWARE(AN8811HB_MD32_DSP);
 
-MODULE_DESCRIPTION("Airoha EN8811H PHY drivers");
+MODULE_DESCRIPTION("Airoha EN8811H and AN8811HB PHY drivers");
 MODULE_AUTHOR("Airoha");
 MODULE_AUTHOR("Eric Woudstra <ericwouds@gmail.com>");
 MODULE_LICENSE("GPL");
-- 
2.47.3

[PATCH net-next v2 3/3] net: phy: air_en8811h: Add clk provider for an8811hb

[Bjørn Mork]

Implement clk provider driver so we can disable the clock output when it
isn't needed. This helps to reduce EMF noise

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/phy/air_en8811h.c | 107 ++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
index ed6a094a4185..e664cbc2f1ed 100644
--- a/drivers/net/phy/air_en8811h.c
+++ b/drivers/net/phy/air_en8811h.c
@@ -954,6 +954,105 @@ static int en8811h_led_hw_is_supported(struct phy_device *phydev, u8 index,
 	return 0;
 };
 
+static unsigned long an8811hb_clk_recalc_rate(struct clk_hw *hw,
+					      unsigned long parent)
+{
+	struct en8811h_priv *priv = clk_hw_to_en8811h_priv(hw);
+	struct phy_device *phydev = priv->phydev;
+	u32 pbus_value;
+	int ret;
+
+	ret = air_buckpbus_reg_read(phydev, AN8811HB_HWTRAP2, &pbus_value);
+	if (ret < 0)
+		return ret;
+
+	return (pbus_value & AN8811HB_HWTRAP2_CKO) ? 50000000 : 25000000;
+}
+
+static int an8811hb_clk_enable(struct clk_hw *hw)
+{
+	struct en8811h_priv *priv = clk_hw_to_en8811h_priv(hw);
+	struct phy_device *phydev = priv->phydev;
+
+	return air_buckpbus_reg_modify(phydev, AN8811HB_CLK_DRV,
+				       AN8811HB_CLK_DRV_CKO_MASK,
+				       AN8811HB_CLK_DRV_CKO_MASK);
+}
+
+static void an8811hb_clk_disable(struct clk_hw *hw)
+{
+	struct en8811h_priv *priv = clk_hw_to_en8811h_priv(hw);
+	struct phy_device *phydev = priv->phydev;
+
+	air_buckpbus_reg_modify(phydev, AN8811HB_CLK_DRV,
+				AN8811HB_CLK_DRV_CKO_MASK, 0);
+}
+
+static int an8811hb_clk_is_enabled(struct clk_hw *hw)
+{
+	struct en8811h_priv *priv = clk_hw_to_en8811h_priv(hw);
+	struct phy_device *phydev = priv->phydev;
+	u32 pbus_value;
+	int ret;
+
+	ret = air_buckpbus_reg_read(phydev, AN8811HB_CLK_DRV, &pbus_value);
+	if (ret < 0)
+		return ret;
+
+	return (pbus_value & AN8811HB_CLK_DRV_CKO_MASK);
+}
+
+static int an8811hb_clk_save_context(struct clk_hw *hw)
+{
+	struct en8811h_priv *priv = clk_hw_to_en8811h_priv(hw);
+
+	priv->cko_is_enabled = an8811hb_clk_is_enabled(hw);
+
+	return 0;
+}
+
+static void an8811hb_clk_restore_context(struct clk_hw *hw)
+{
+	struct en8811h_priv *priv = clk_hw_to_en8811h_priv(hw);
+
+	if (!priv->cko_is_enabled)
+		an8811hb_clk_disable(hw);
+}
+
+static const struct clk_ops an8811hb_clk_ops = {
+	.recalc_rate		= an8811hb_clk_recalc_rate,
+	.enable			= an8811hb_clk_enable,
+	.disable		= an8811hb_clk_disable,
+	.is_enabled		= an8811hb_clk_is_enabled,
+	.save_context		= an8811hb_clk_save_context,
+	.restore_context	= an8811hb_clk_restore_context,
+};
+
+static int an8811hb_clk_provider_setup(struct device *dev, struct clk_hw *hw)
+{
+	struct clk_init_data init;
+	int ret;
+
+	if (!IS_ENABLED(CONFIG_COMMON_CLK))
+		return 0;
+
+	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s-cko",
+				   fwnode_get_name(dev_fwnode(dev)));
+	if (!init.name)
+		return -ENOMEM;
+
+	init.ops = &an8811hb_clk_ops;
+	init.flags = 0;
+	init.num_parents = 0;
+	hw->init = &init;
+
+	ret = devm_clk_hw_register(dev, hw);
+	if (ret)
+		return ret;
+
+	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, hw);
+}
+
 static unsigned long en8811h_clk_recalc_rate(struct clk_hw *hw,
 					     unsigned long parent)
 {
@@ -1095,6 +1194,12 @@ static int an8811hb_probe(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
+	priv->phydev = phydev;
+	/* Co-Clock Output */
+	ret = an8811hb_clk_provider_setup(&phydev->mdio.dev, &priv->hw);
+	if (ret)
+		return ret;
+
 	/* Configure led gpio pins as output */
 	ret = air_buckpbus_reg_modify(phydev, AN8811HB_GPIO_OUTPUT,
 				      AN8811HB_GPIO_OUTPUT_345,
@@ -1520,6 +1625,8 @@ static struct phy_driver en8811h_driver[] = {
 	.get_rate_matching	= en8811h_get_rate_matching,
 	.config_aneg		= en8811h_config_aneg,
 	.read_status		= an8811hb_read_status,
+	.resume			= en8811h_resume,
+	.suspend		= en8811h_suspend,
 	.config_intr		= en8811h_clear_intr,
 	.handle_interrupt	= en8811h_handle_interrupt,
 	.led_hw_is_supported	= en8811h_led_hw_is_supported,
-- 
2.47.3

Re: [PATCH net-next v2 1/3] net: phy: air_en8811h: factor out shareable code

[Andrew Lunn]

> -static int en8811h_read_status(struct phy_device *phydev)
> +static int en8811h_get_lpa(struct phy_device *phydev)
>  {
> -	struct en8811h_priv *priv = phydev->priv;
> -	u32 pbus_value;
> -	int ret, val;
> +	int ret;
>  
>  	ret = genphy_update_link(phydev);
>  	if (ret)

This call to genphy_update_link() means this function is doing more
than en8811h_get_lpa() would imply.

It is hard to see from just the patch, but it also seems to set the
state in phydev back to unknown defaults, and read the master/slave
status?

So i think it needs a better name.

	Andrew

Re: [PATCH net-next v2 2/3] net: phy: air_en8811h: add Airoha AN8811HB support

[Andrew Lunn]

> +#define AN8811HB_GPIO_OUTPUT		0x5cf8b8
> +#define   AN8811HB_GPIO_OUTPUT_345		(BIT(3) | BIT(4) | BIT(5))

> +	/* Configure led gpio pins as output */
> +	ret = air_buckpbus_reg_modify(phydev, AN8811HB_GPIO_OUTPUT,
> +				      AN8811HB_GPIO_OUTPUT_345,
> +				      AN8811HB_GPIO_OUTPUT_345);

The code/comment probably does not describe what is actually happening
here. My _guess_ is you are setting a pinmux, disconnecting the pins
from the GPIO controller and connecting them to the LED controller.

I assume there is no open data sheet for this device?

	Andrew

Re: [PATCH net-next v2 2/3] net: phy: air_en8811h: add Airoha AN8811HB support

[Bjørn Mork]

Andrew Lunn <andrew@lunn.ch> writes:

>> +#define AN8811HB_GPIO_OUTPUT		0x5cf8b8
>> +#define   AN8811HB_GPIO_OUTPUT_345		(BIT(3) | BIT(4) | BIT(5))
>
>> +	/* Configure led gpio pins as output */
>> +	ret = air_buckpbus_reg_modify(phydev, AN8811HB_GPIO_OUTPUT,
>> +				      AN8811HB_GPIO_OUTPUT_345,
>> +				      AN8811HB_GPIO_OUTPUT_345);
>
> The code/comment probably does not describe what is actually happening
> here. My _guess_ is you are setting a pinmux, disconnecting the pins
> from the GPIO controller and connecting them to the LED controller.

Possibly.  This code is copied from the out-of-tree vendor driver.  We
already have similar code and comment in the en8811h probe:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/tree/drivers/net/phy/air_en8811h.c#n959

The register addresses and layouts are suspiciously similar:

#define EN8811H_GPIO_OUTPUT		0xcf8b8
#define   EN8811H_GPIO_OUTPUT_345		(BIT(3) | BIT(4) | BIT(5))

Without any docs, or a way to test this particular feature, I
believe the safest option is to assume that the vendor driver is
correct.  Can't start guessing no matter how tempting it is :-)

> I assume there is no open data sheet for this device?

Correct AFAIK.

I have no other docs either.  The code is based solely on the vendor
driver.  But trying to reuse as much as possible of the existing en8811h
driver instead of duplicating it like the vendor did.

I have two almost identical boards with this phy connected to a Mediatek
MT7988D SoC.  I can test, and have tested, the features exposed by those
boards.  But this is obviously a limited test environment.  There are
for example no port LEDs on any of the boards.

But that's what I got.


Bjørn

Re: [PATCH net-next v2 1/3] net: phy: air_en8811h: factor out shareable code

[Bjørn Mork]

Andrew Lunn <andrew@lunn.ch> writes:

>> -static int en8811h_read_status(struct phy_device *phydev)
>> +static int en8811h_get_lpa(struct phy_device *phydev)
>>  {
>> -	struct en8811h_priv *priv = phydev->priv;
>> -	u32 pbus_value;
>> -	int ret, val;
>> +	int ret;
>>  
>>  	ret = genphy_update_link(phydev);
>>  	if (ret)
>
> This call to genphy_update_link() means this function is doing more
> than en8811h_get_lpa() would imply.
>
> It is hard to see from just the patch, but it also seems to set the
> state in phydev back to unknown defaults, and read the master/slave
> status?
>
> So i think it needs a better name.

Yes, that was my feeling too. Just couldn't think of any.  So I took the
lazy route.  You wouldn't happen to have a suggestion?

OK, I know it's my job.  I'll try to come up with something better.


Bjørn

Re: [PATCH net-next v2 2/3] net: phy: air_en8811h: add Airoha AN8811HB support

[Andrew Lunn]

On Sat, Jan 24, 2026 at 05:53:03PM +0100, Bjørn Mork wrote:
> Andrew Lunn <andrew@lunn.ch> writes:
> 
> >> +#define AN8811HB_GPIO_OUTPUT		0x5cf8b8
> >> +#define   AN8811HB_GPIO_OUTPUT_345		(BIT(3) | BIT(4) | BIT(5))
> >
> >> +	/* Configure led gpio pins as output */
> >> +	ret = air_buckpbus_reg_modify(phydev, AN8811HB_GPIO_OUTPUT,
> >> +				      AN8811HB_GPIO_OUTPUT_345,
> >> +				      AN8811HB_GPIO_OUTPUT_345);
> >
> > The code/comment probably does not describe what is actually happening
> > here. My _guess_ is you are setting a pinmux, disconnecting the pins
> > from the GPIO controller and connecting them to the LED controller.
> 
> Possibly.  This code is copied from the out-of-tree vendor driver.  We
> already have similar code and comment in the en8811h probe:
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/tree/drivers/net/phy/air_en8811h.c#n959
> 
> The register addresses and layouts are suspiciously similar:
> 
> #define EN8811H_GPIO_OUTPUT		0xcf8b8
> #define   EN8811H_GPIO_OUTPUT_345		(BIT(3) | BIT(4) | BIT(5))
> 
> Without any docs, or a way to test this particular feature, I
> believe the safest option is to assume that the vendor driver is
> correct.  Can't start guessing no matter how tempting it is :-)

The writing of the value to the register is likely correct. I just
think all the names and comments are wrong.

Maybe using magic numbers in this case is actually better?

And describe the intent of the code in more general terms, allow the
pins to be used to driver LEDs.

> I have no other docs either.  The code is based solely on the vendor
> driver.  But trying to reuse as much as possible of the existing en8811h
> driver instead of duplicating it like the vendor did.

That is typical of vendors, and i agree with your strategy,.

> I have two almost identical boards with this phy connected to a Mediatek
> MT7988D SoC.  I can test, and have tested, the features exposed by those
> boards.  But this is obviously a limited test environment.  There are
> for example no port LEDs on any of the boards.

So you have no idea if the LEDs actualy blink with traffic, show link
state etc?

	Andrew

Re: [PATCH net-next v2 1/3] net: phy: air_en8811h: factor out shareable code

[Andrew Lunn]

On Sat, Jan 24, 2026 at 05:55:05PM +0100, Bjørn Mork wrote:
> Andrew Lunn <andrew@lunn.ch> writes:
> 
> >> -static int en8811h_read_status(struct phy_device *phydev)
> >> +static int en8811h_get_lpa(struct phy_device *phydev)
> >>  {
> >> -	struct en8811h_priv *priv = phydev->priv;
> >> -	u32 pbus_value;
> >> -	int ret, val;
> >> +	int ret;
> >>  
> >>  	ret = genphy_update_link(phydev);
> >>  	if (ret)
> >
> > This call to genphy_update_link() means this function is doing more
> > than en8811h_get_lpa() would imply.
> >
> > It is hard to see from just the patch, but it also seems to set the
> > state in phydev back to unknown defaults, and read the master/slave
> > status?
> >
> > So i think it needs a better name.
> 
> Yes, that was my feeling too. Just couldn't think of any.  So I took the
> lazy route.  You wouldn't happen to have a suggestion?

Maybe a different strategy. Most of read_status is generic, it appears
just reading lpa is specific to the device. So have a generic
read_status() function, and then use phy_id_compare() to call the
needed device specific function.

       Andrew

Re: [PATCH net-next v2 1/3] net: phy: air_en8811h: factor out shareable code

[Bjørn Mork]

Andrew Lunn <andrew@lunn.ch> writes:

> Maybe a different strategy. Most of read_status is generic, it appears
> just reading lpa is specific to the device. So have a generic
> read_status() function, and then use phy_id_compare() to call the
> needed device specific function.

Yes, maybe that's better.  My immediate feeling was that this
contradicted your comments on the RFC, but after rereading it I see that
it doesn't.  I was reading too much into that

I'll try and see if I can improve the readability of read_status() with
a generic function.


Bjørn

Re: [PATCH net-next v2 2/3] net: phy: air_en8811h: add Airoha AN8811HB support

[Bjørn Mork]

Andrew Lunn <andrew@lunn.ch> writes:
> On Sat, Jan 24, 2026 at 05:53:03PM +0100, Bjørn Mork wrote:
>> Andrew Lunn <andrew@lunn.ch> writes:
>> 
>> >> +#define AN8811HB_GPIO_OUTPUT		0x5cf8b8
>> >> +#define   AN8811HB_GPIO_OUTPUT_345		(BIT(3) | BIT(4) | BIT(5))
>> >
>> >> +	/* Configure led gpio pins as output */
>> >> +	ret = air_buckpbus_reg_modify(phydev, AN8811HB_GPIO_OUTPUT,
>> >> +				      AN8811HB_GPIO_OUTPUT_345,
>> >> +				      AN8811HB_GPIO_OUTPUT_345);
>> >
>> > The code/comment probably does not describe what is actually happening
>> > here. My _guess_ is you are setting a pinmux, disconnecting the pins
>> > from the GPIO controller and connecting them to the LED controller.
>> 
>> Possibly.  This code is copied from the out-of-tree vendor driver.  We
>> already have similar code and comment in the en8811h probe:
>> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/tree/drivers/net/phy/air_en8811h.c#n959
>> 
>> The register addresses and layouts are suspiciously similar:
>> 
>> #define EN8811H_GPIO_OUTPUT		0xcf8b8
>> #define   EN8811H_GPIO_OUTPUT_345		(BIT(3) | BIT(4) | BIT(5))
>> 
>> Without any docs, or a way to test this particular feature, I
>> believe the safest option is to assume that the vendor driver is
>> correct.  Can't start guessing no matter how tempting it is :-)
>
> The writing of the value to the register is likely correct. I just
> think all the names and comments are wrong.
>
> Maybe using magic numbers in this case is actually better?

I agree that it would probably be just as good here.  But I really don't
want to make unnecessary changes to the existing EN8811H code.  Trying
hard to modify as little as possible of that in this series.  And I
believe it's always best to keep the style of similar code blocks inside
as single file/driver.

So unless this is important, I'd prefer to keep the macros and comment.
It is what we have had so far for EN8811H.

> And describe the intent of the code in more general terms, allow the
> pins to be used to driver LEDs.
>
>> I have no other docs either.  The code is based solely on the vendor
>> driver.  But trying to reuse as much as possible of the existing en8811h
>> driver instead of duplicating it like the vendor did.
>
> That is typical of vendors, and i agree with your strategy,.
>
>> I have two almost identical boards with this phy connected to a Mediatek
>> MT7988D SoC.  I can test, and have tested, the features exposed by those
>> boards.  But this is obviously a limited test environment.  There are
>> for example no port LEDs on any of the boards.
>
> So you have no idea if the LEDs actualy blink with traffic, show link
> state etc?

Correct.  I can only test that this code doesn't cause any obvious harm
to other functions.

The options, as I see them, are either
a) dropping this functionality from the driver until someone is able to
   test and adds it back, or
b) keep it in the hope that it works, until someone is able to test and
   either verifies or fixes the code.  Including fixups of the macros
   and comment if/when we get docs

I believe that b) is more likely to succeed, which is why I chose to
include it.


Bjørn

Re: [PATCH net-next v2 1/3] net: phy: air_en8811h: factor out shareable code

[Andrew Lunn]

On Sun, Jan 25, 2026 at 11:51:56AM +0100, Bjørn Mork wrote:
> Andrew Lunn <andrew@lunn.ch> writes:
> 
> > Maybe a different strategy. Most of read_status is generic, it appears
> > just reading lpa is specific to the device. So have a generic
> > read_status() function, and then use phy_id_compare() to call the
> > needed device specific function.
> 
> Yes, maybe that's better.  My immediate feeling was that this
> contradicted your comments on the RFC

Reviewers sometimes get things wrong. It is not always possible to see
everything from just the patch, and then jump to the wrong conclusion,
etc. The review process should be a conversation. We can talk about
different solutions, which might contradict earlier suggestions, as we
get a better understanding of the problem to be solved.

   Andrew