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

> +#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

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

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

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