[PATCH] net: phy: air_en8811h: select LED GPIO pins based on AN8811HB package variant

[PATCH] net: phy: air_en8811h: select LED GPIO pins based on AN8811HB package variant

[Weiting Lee]

The AN8811HB comes in two package variants, AN8811HBCN and AN8811HBN,
which use different GPIO pins to drive LED outputs. AN8811HBCN uses
GPIOs 0, 1, and 15, while AN8811HBN uses GPIOs 3, 4, and 5. Using a
fixed GPIO assignment causes incorrect LED behavior on one of the
variants.

Read the package variant from hardware strap register AN8811HB_HWTRAP2
at probe time and store it in priv->pkg_sel. Add an8811hb_led_gpio_setup()
to configure the correct GPIO output pins and select lines based on the
detected variant, and call it from config_init.

Signed-off-by: Weiting Lee <weiting.lee@airoha.com>
---
 drivers/net/phy/air_en8811h.c | 72 +++++++++++++++++++++++++++++++----
 1 file changed, 65 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
index edd49c193e47..299163501140 100644
--- a/drivers/net/phy/air_en8811h.c
+++ b/drivers/net/phy/air_en8811h.c
@@ -145,10 +145,22 @@
 
 #define AN8811HB_GPIO_OUTPUT		0x5cf8b8
 #define   AN8811HB_GPIO_OUTPUT_345		(BIT(3) | BIT(4) | BIT(5))
+#define   AN8811HB_GPIO_OUTPUT_0115		(BIT(0) | BIT(1) | BIT(15))
+
+#define AN8811HB_GPIO_SEL1		0x5cf8bc
+#define   AN8811HB_GPIO_SEL1_0_MASK		GENMASK(3, 0)
+#define   AN8811HB_GPIO_SEL1_1_MASK		GENMASK(7, 4)
+#define   AN8811HB_GPIO_SEL1_0			BIT(0)
+#define   AN8811HB_GPIO_SEL1_1			0
+
+#define AN8811HB_GPIO_SEL2		0x5cf8c0
+#define   AN8811HB_GPIO_SEL2_15_MASK		GENMASK(31, 28)
+#define   AN8811HB_GPIO_SEL2_15			BIT(29)
 
 #define AN8811HB_HWTRAP1		0x5cf910
 #define AN8811HB_HWTRAP2		0x5cf914
 #define   AN8811HB_HWTRAP2_CKO			BIT(28)
+#define   AN8811HB_HWTRAP2_PKG			GENMASK(14, 12)
 
 #define AN8811HB_CLK_DRV		0x5cf9e4
 #define AN8811HB_CLK_DRV_CKO_MASK		GENMASK(14, 12)
@@ -202,6 +214,7 @@ struct en8811h_priv {
 	struct phy_device	*phydev;
 	unsigned int		cko_is_enabled;
 	struct mdio_device	*pbusdev;
+	unsigned int		pkg_sel;
 };
 
 enum {
@@ -1071,10 +1084,49 @@ static int en8811h_leds_setup(struct phy_device *phydev)
 	return ret;
 }
 
+static int an8811hb_led_gpio_setup(struct phy_device *phydev)
+{
+	struct en8811h_priv *priv = phydev->priv;
+	int ret;
+
+	if (priv->pkg_sel) {
+		/* AN8811HBCN: LED GPIOs are 0, 1, 15 */
+		ret = air_phy_buckpbus_reg_modify(phydev, AN8811HB_GPIO_OUTPUT,
+						  AN8811HB_GPIO_OUTPUT_0115,
+						  AN8811HB_GPIO_OUTPUT_0115);
+		if (ret < 0)
+			return ret;
+
+		ret = air_phy_buckpbus_reg_modify(phydev, AN8811HB_GPIO_SEL1,
+						  AN8811HB_GPIO_SEL1_0_MASK |
+						  AN8811HB_GPIO_SEL1_1_MASK,
+						  AN8811HB_GPIO_SEL1_0 |
+						  AN8811HB_GPIO_SEL1_1);
+		if (ret < 0)
+			return ret;
+
+		ret = air_phy_buckpbus_reg_modify(phydev, AN8811HB_GPIO_SEL2,
+						  AN8811HB_GPIO_SEL2_15_MASK,
+						  AN8811HB_GPIO_SEL2_15);
+		if (ret < 0)
+			return ret;
+	} else {
+		/* AN8811HBN: LED GPIOs are 3, 4, 5 */
+		ret = air_phy_buckpbus_reg_modify(phydev, AN8811HB_GPIO_OUTPUT,
+						  AN8811HB_GPIO_OUTPUT_345,
+						  AN8811HB_GPIO_OUTPUT_345);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int an8811hb_probe(struct phy_device *phydev)
 {
 	struct mdio_device *mdiodev;
 	struct en8811h_priv *priv;
+	u32 reg_val;
 	int ret;
 
 	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(struct en8811h_priv),
@@ -1115,6 +1167,15 @@ static int an8811hb_probe(struct phy_device *phydev)
 	/* MDIO_DEVS1/2 empty, so set mmds_present bits here */
 	phydev->c45_ids.mmds_present |= MDIO_DEVS_PMAPMD | MDIO_DEVS_AN;
 
+	/* Detect package variant */
+	ret = air_phy_buckpbus_reg_read(phydev, AN8811HB_HWTRAP2, &reg_val);
+	if (ret < 0)
+		goto err_dev_create;
+	priv->pkg_sel = FIELD_GET(AN8811HB_HWTRAP2_PKG, reg_val);
+
+	phydev_info(phydev, "%s detected\n",
+		    priv->pkg_sel ? "AN8811HBCN" : "AN8811HBN");
+
 	ret = en8811h_leds_setup(phydev);
 	if (ret < 0)
 		goto err_dev_create;
@@ -1125,13 +1186,6 @@ static int an8811hb_probe(struct phy_device *phydev)
 	if (ret)
 		goto err_dev_create;
 
-	/* Configure led gpio pins as output */
-	ret = air_phy_buckpbus_reg_modify(phydev, AN8811HB_GPIO_OUTPUT,
-					  AN8811HB_GPIO_OUTPUT_345,
-					  AN8811HB_GPIO_OUTPUT_345);
-	if (ret < 0)
-		goto err_dev_create;
-
 	return 0;
 
 err_dev_create:
@@ -1270,6 +1324,10 @@ static int an8811hb_config_init(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
+	ret = an8811hb_led_gpio_setup(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)
-- 
2.43.0

Re: [PATCH] net: phy: air_en8811h: select LED GPIO pins based on AN8811HB package variant

[Andrew Lunn]

On Thu, Aug 27, 2026 at 01:41:55PM +0800, Weiting Lee wrote:
> The AN8811HB comes in two package variants, AN8811HBCN and AN8811HBN,
> which use different GPIO pins to drive LED outputs. AN8811HBCN uses
> GPIOs 0, 1, and 15, while AN8811HBN uses GPIOs 3, 4, and 5. Using a
> fixed GPIO assignment causes incorrect LED behavior on one of the
> variants.
> 
> Read the package variant from hardware strap register AN8811HB_HWTRAP2

Just for conformation, this is not an actually strap pin you connect a
resistor to, pull high/low, but a read only silicon bit.

Also, you called it a strap registers, yet the #define is TRAP?

> @@ -202,6 +214,7 @@ struct en8811h_priv {
>  	struct phy_device	*phydev;
>  	unsigned int		cko_is_enabled;
>  	struct mdio_device	*pbusdev;
> +	unsigned int		pkg_sel;

bool?

Also, is_an8811hbcn would be a better name. It is hard to know what
true/false means for pkg_sel.

> +	phydev_info(phydev, "%s detected\n",
> +		    priv->pkg_sel ? "AN8811HBCN" : "AN8811HBN");

No need to spam the log.

    Andrew

---
pw-bot: cr

[PATCH v2 net-next] net: phy: air_en8811h: select LED GPIO pins based on AN8811HB package variant

[Weiting Lee]

The AN8811HB comes in two package variants, AN8811HBCN and AN8811HBN,
which use different GPIO pins to drive LED outputs. AN8811HBCN uses
GPIOs 0, 1, and 15, while AN8811HBN uses GPIOs 3, 4, and 5. Using a
fixed GPIO assignment causes incorrect LED behavior on one of the
variants.

Read the package variant from the read-only silicon identification bits
in AN8811HB_HWTRAP2 at probe time and store it in priv->is_an8811hbcn.
Add an8811hb_led_gpio_setup() to configure the correct GPIO output pins
and select lines based on the detected variant, and call it from
config_init.

Signed-off-by: Weiting Lee <weiting.lee@airoha.com>
---
v1 -> v2:
- Rename pkg_sel to is_an8811hbcn (bool) for clarity
- Remove phydev_info log to avoid spamming the kernel log
- Fix commit message: HWTRAP2 bits are read-only silicon
  identification, not a hardware strap pin

v1: https://lore.kernel.org/netdev/20260827054156.2681908-1-weiting.lee@airoha.com/
 drivers/net/phy/air_en8811h.c | 69 +++++++++++++++++++++++++++++++----
 1 file changed, 62 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
index 0eeb7b9a4e26..7b69bb79b77f 100644
--- a/drivers/net/phy/air_en8811h.c
+++ b/drivers/net/phy/air_en8811h.c
@@ -145,10 +145,22 @@
 
 #define AN8811HB_GPIO_OUTPUT		0x5cf8b8
 #define   AN8811HB_GPIO_OUTPUT_345		(BIT(3) | BIT(4) | BIT(5))
+#define   AN8811HB_GPIO_OUTPUT_0115		(BIT(0) | BIT(1) | BIT(15))
+
+#define AN8811HB_GPIO_SEL1		0x5cf8bc
+#define   AN8811HB_GPIO_SEL1_0_MASK		GENMASK(3, 0)
+#define   AN8811HB_GPIO_SEL1_1_MASK		GENMASK(7, 4)
+#define   AN8811HB_GPIO_SEL1_0			BIT(0)
+#define   AN8811HB_GPIO_SEL1_1			0
+
+#define AN8811HB_GPIO_SEL2		0x5cf8c0
+#define   AN8811HB_GPIO_SEL2_15_MASK		GENMASK(31, 28)
+#define   AN8811HB_GPIO_SEL2_15			BIT(29)
 
 #define AN8811HB_HWTRAP1		0x5cf910
 #define AN8811HB_HWTRAP2		0x5cf914
 #define   AN8811HB_HWTRAP2_CKO			BIT(28)
+#define   AN8811HB_HWTRAP2_PKG			GENMASK(14, 12)
 
 #define AN8811HB_CLK_DRV		0x5cf9e4
 #define AN8811HB_CLK_DRV_CKO_MASK		GENMASK(14, 12)
@@ -202,6 +214,7 @@ struct en8811h_priv {
 	struct phy_device	*phydev;
 	unsigned int		cko_is_enabled;
 	struct mdio_device	*pbusdev;
+	bool			is_an8811hbcn;
 };
 
 enum {
@@ -1071,10 +1084,49 @@ static int en8811h_leds_setup(struct phy_device *phydev)
 	return ret;
 }
 
+static int an8811hb_led_gpio_setup(struct phy_device *phydev)
+{
+	struct en8811h_priv *priv = phydev->priv;
+	int ret;
+
+	if (priv->is_an8811hbcn) {
+		/* AN8811HBCN: LED GPIOs are 0, 1, 15 */
+		ret = air_phy_buckpbus_reg_modify(phydev, AN8811HB_GPIO_OUTPUT,
+						  AN8811HB_GPIO_OUTPUT_0115,
+						  AN8811HB_GPIO_OUTPUT_0115);
+		if (ret < 0)
+			return ret;
+
+		ret = air_phy_buckpbus_reg_modify(phydev, AN8811HB_GPIO_SEL1,
+						  AN8811HB_GPIO_SEL1_0_MASK |
+						  AN8811HB_GPIO_SEL1_1_MASK,
+						  AN8811HB_GPIO_SEL1_0 |
+						  AN8811HB_GPIO_SEL1_1);
+		if (ret < 0)
+			return ret;
+
+		ret = air_phy_buckpbus_reg_modify(phydev, AN8811HB_GPIO_SEL2,
+						  AN8811HB_GPIO_SEL2_15_MASK,
+						  AN8811HB_GPIO_SEL2_15);
+		if (ret < 0)
+			return ret;
+	} else {
+		/* AN8811HBN: LED GPIOs are 3, 4, 5 */
+		ret = air_phy_buckpbus_reg_modify(phydev, AN8811HB_GPIO_OUTPUT,
+						  AN8811HB_GPIO_OUTPUT_345,
+						  AN8811HB_GPIO_OUTPUT_345);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int an8811hb_probe(struct phy_device *phydev)
 {
 	struct mdio_device *mdiodev;
 	struct en8811h_priv *priv;
+	u32 reg_val;
 	int ret;
 
 	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(struct en8811h_priv),
@@ -1115,6 +1167,12 @@ static int an8811hb_probe(struct phy_device *phydev)
 	/* MDIO_DEVS1/2 empty, so set mmds_present bits here */
 	phydev->c45_ids.mmds_present |= MDIO_DEVS_PMAPMD | MDIO_DEVS_AN;
 
+	/* Detect package variant */
+	ret = air_phy_buckpbus_reg_read(phydev, AN8811HB_HWTRAP2, &reg_val);
+	if (ret < 0)
+		goto err_dev_create;
+	priv->is_an8811hbcn = FIELD_GET(AN8811HB_HWTRAP2_PKG, reg_val);
+
 	ret = en8811h_leds_setup(phydev);
 	if (ret < 0)
 		goto err_dev_create;
@@ -1125,13 +1183,6 @@ static int an8811hb_probe(struct phy_device *phydev)
 	if (ret)
 		goto err_dev_create;
 
-	/* Configure led gpio pins as output */
-	ret = air_phy_buckpbus_reg_modify(phydev, AN8811HB_GPIO_OUTPUT,
-					  AN8811HB_GPIO_OUTPUT_345,
-					  AN8811HB_GPIO_OUTPUT_345);
-	if (ret < 0)
-		goto err_dev_create;
-
 	return 0;
 
 err_dev_create:
@@ -1263,6 +1314,10 @@ static int an8811hb_config_init(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
+	ret = an8811hb_led_gpio_setup(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)
-- 
2.43.0

Re: [PATCH v2 net-next] net: phy: air_en8811h: select LED GPIO pins based on AN8811HB package variant

[Andrew Lunn]

On Fri, Aug 28, 2026 at 01:05:36PM +0800, Weiting Lee wrote:
> The AN8811HB comes in two package variants, AN8811HBCN and AN8811HBN,
> which use different GPIO pins to drive LED outputs. AN8811HBCN uses
> GPIOs 0, 1, and 15, while AN8811HBN uses GPIOs 3, 4, and 5. Using a
> fixed GPIO assignment causes incorrect LED behavior on one of the
> variants.
> 
> Read the package variant from the read-only silicon identification bits
> in AN8811HB_HWTRAP2 at probe time and store it in priv->is_an8811hbcn.
> Add an8811hb_led_gpio_setup() to configure the correct GPIO output pins
> and select lines based on the detected variant, and call it from
> config_init.

Have yo seen the patch:

Ziyou Xu [PATCH net v2] net: phy: air_en8811h: restore AN8811HB LED GPIO after MCU

There is likely to be a merge conflict with it. We probably should get
that patch merged first.

     Andrew