[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

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

[WeiTing Lee (李威霆)]

(Resending in plain text, apologies for the duplicate.)

Hi Andrew,

Yes, I've seen Ziyou's patch, which is now at v3
(https://lore.kernel.org/netdev/20260829033844.1975-1-xuziyougm@gmail.com/).

My patch is a superset of Ziyou's: it also restores the GPIO output
pins in config_init, but additionally selects the correct set of GPIO
pins based on the AN8811HB package variant (HBCN uses GPIOs 0, 1, 15;
HBN uses GPIOs 3, 4, 5).

I'll wait for Ziyou's v3 to be merged, then rebase and send v3 of this
patch on top of it.

Thanks,
Weiting


________________________________________
寄件者: Andrew Lunn <andrew@lunn.ch>
已傳送: 星期五, 2026 年 8 月 28 日  下午 09:07
收件者: WeiTing Lee (李威霆) <weiting.lee@airoha.com>
副本: netdev@vger.kernel.org <netdev@vger.kernel.org>; hkallweit1@gmail.com <hkallweit1@gmail.com>; linux@armlinux.org.uk <linux@armlinux.org.uk>; davem@davemloft.net <davem@davemloft.net>; kuba@kernel.org <kuba@kernel.org>; edumazet@google.com <edumazet@google.com>; pabeni@redhat.com <pabeni@redhat.com>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; bjorn@mork.no <bjorn@mork.no>; ericwouds@gmail.com <ericwouds@gmail.com>; frank-w@public-files.de <frank-w@public-files.de>; Joseph Lin (林蔚煌) <Joseph.Lin@airoha.com>; Wenshin Chung (鍾問星) <wenshin.chung@airoha.com>; Lucien Jheng (鄭祥鈞) <lucien.jheng@airoha.com>; Albert-AL Lee (李家宏) <albert-al.lee@airoha.com>
主旨: Re: [PATCH v2 net-next] net: phy: air_en8811h: select LED GPIO pins based on AN8811HB package variant


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

[PATCH v3 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>
---
v2 -> v3:
- Rebase on top of net-next commit 3498acda6b68
  ("net: phy: air_en8811h: restore AN8811HB LED GPIO after MCU restart")
- an8811hb_led_gpio_setup() now covers the AN8811HBN (345) case that
  commit restored, replacing its direct GPIO_OUTPUT_345 restore call

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/
v2: https://lore.kernel.org/netdev/20260828050537.2728253-1-weiting.lee@airoha.com/
 drivers/net/phy/air_en8811h.c | 67 ++++++++++++++++++++++++++++++++---
 1 file changed, 63 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
index 38eb18f0e9bb..ae24c41f538a 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;
@@ -1263,10 +1321,11 @@ static int an8811hb_config_init(struct phy_device *phydev)
 		return ret;
 	}
 
-	/* Restore LED GPIO output enables after MCU initialization. */
-	ret = air_phy_buckpbus_reg_modify(phydev, AN8811HB_GPIO_OUTPUT,
-					  AN8811HB_GPIO_OUTPUT_345,
-					  AN8811HB_GPIO_OUTPUT_345);
+	ret = an8811hb_led_gpio_setup(phydev);
+	if (ret < 0) {
+		phydev_err(phydev, "Failed to set up LED GPIO: %d\n", ret);
+		return ret;
+	}
 	return ret;
 }
 
-- 
2.43.0

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 2

- [Medium] Multi-bit hardware field implicitly cast to a boolean
  evaluates to true for any non-zero value, potentially misidentifying…
- [Low] The commit message states that "Using a fixed GPIO assignment
  causes incorrect LED behavior on one of the variants", i.e. it…
- [Low] The in-file comment at drivers/net/phy/air_en8811h.c:188-192
  documents the default LED-to-GPIO mapping unconditionally ("GPIO5 <->…

--

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

This patch reads the AN8811HB package variant from the read-only silicon
identification bits in AN8811HB_HWTRAP2 during an8811hb_probe() and stores
it in priv->is_an8811hbcn.  A new helper an8811hb_led_gpio_setup() then
programs either GPIO 0/1/15 (AN8811HBCN) or GPIO 3/4/5 (AN8811HBN) from
an8811hb_config_init().

No Link: tags were present in the commit header.

> diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
> index ec7c44ca68e7d..b6eac8dc63d9f 100644
> --- a/drivers/net/phy/air_en8811h.c
> +++ b/drivers/net/phy/air_en8811h.c

[ ... ]

> @@ -1077,10 +1090,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;

[Severity: Low]

This isn't a bug, but should the LED mapping comment above the trigger
defaults be qualified per package now?

The only in-tree description of which LED index drives which pin sits
above AIR_DEFAULT_TRIGGER_LED0/1/2 and is written unconditionally:

  /* Default LED setup:
   * GPIO5 <-> LED0  On: Link detected, blink Rx/Tx
   * GPIO4 <-> LED1  On: Link detected at 2500 or 1000 Mbps
   * GPIO3 <-> LED2  On: Link detected at 2500 or  100 Mbps
   */

Those same defaults are applied by en8811h_leds_setup(), which is called
from both en8811h_probe() and an8811hb_probe().  After this patch the
GPIO 3/4/5 wording only holds for EN8811H and AN8811HBN, since on
AN8811HBCN the identical trigger defaults now drive pads 0, 1 and 15 via
AN8811HB_GPIO_OUTPUT_0115 plus the SEL1/SEL2 mux codes.

Could the comment mention the AN8811HBCN pin assignment as well?

> +
> +		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;
> +}
> +

[ ... ]

> @@ -1269,10 +1327,11 @@ static int an8811hb_config_init(struct phy_device *phydev)
>  		return ret;
>  	}
>  
> -	/* Restore LED GPIO output enables after MCU initialization. */
> -	ret = air_phy_buckpbus_reg_modify(phydev, AN8811HB_GPIO_OUTPUT,
> -					  AN8811HB_GPIO_OUTPUT_345,
> -					  AN8811HB_GPIO_OUTPUT_345);
> +	ret = an8811hb_led_gpio_setup(phydev);
> +	if (ret < 0) {
> +		phydev_err(phydev, "Failed to set up LED GPIO: %d\n", ret);
> +		return ret;
> +	}
>  	return ret;
>  }

[Severity: Low]

Should this carry a Fixes: tag?  The commit message says:

  "Using a fixed GPIO assignment causes incorrect LED behavior on one of
   the variants."

That describes a user-visible defect in code that already shipped, but
there is no Fixes: tag and no stable annotation.

The AN8811HB entry and .config_init = an8811hb_config_init came in with
5afda1d734ed ("net: phy: air_en8811h: add Airoha AN8811HB support"), and
the unconditional AN8811HB_GPIO_OUTPUT_345 write being replaced here was
moved into config_init by 3498acda6b68 ("net: phy: air_en8811h: restore
AN8811HB LED GPIO after MCU restart").

The driver binds with phy_id_compare_model(phydev->phy_id,
AN8811HB_PHY_ID), so AN8811HBCN parts already bind to this driver in
released kernels and get GPIO 3/4/5 forced as outputs while their LEDs
are on GPIO 0/1/15.

If this is a fix for those kernels, could a Fixes: tag be added?  If it
is instead enablement of a newly supported package, could the changelog
be reworded so it does not describe existing behaviour as incorrect?

[ ... ]

> @@ -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;

[Severity: Medium]
AN8811HB_HWTRAP2_PKG is GENMASK(14, 12), so FIELD_GET() yields a
3-bit value here, but it is stored straight into the bool
priv->is_an8811hbcn.  Any non-zero package code therefore collapses to
true, which means the whole variant decision in
an8811hb_led_gpio_setup() rests on the unstated assumption that
AN8811HBCN is the only encoding with a non-zero value and AN8811HBN
reads back exactly 0.

If the field can take any other value - a second AN8811HBN encoding, a
reserved code, or a future package - it lands in the
AN8811HB_GPIO_OUTPUT_0115 branch and the driver drives GPIO 0/1/15
plus the SEL1/SEL2 mux for a part whose LEDs are not on those pads.
That failure mode is silent: the LEDs simply do not work, and there is
nothing in the source that documents what the encoding actually is.

Could you define the expected package identifiers as named constants
and compare against them explicitly, e.g. something like

	pkg = FIELD_GET(AN8811HB_HWTRAP2_PKG, reg_val);
	priv->is_an8811hbcn = (pkg == AN8811HB_HWTRAP2_PKG_HBCN);

so that unknown codes fall into the conservative branch rather than
being treated as AN8811HBCN?  If the hardware really only ever reports
two values and 0 is guaranteed to mean AN8811HBN, please say so in a
comment next to the assignment, since the reader cannot tell that from
the mask alone.  Also, is it worth warning (or failing probe) on an
unrecognised package code rather than silently picking a GPIO mapping?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907015638.2875763-1-weiting.lee%40airoha.com

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

[Paolo Abeni]

On 9/9/26 6:57 PM, netdev-bot+sashiko@kernel.org wrote:
>> @@ -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;
> 
> [Severity: Medium]
> AN8811HB_HWTRAP2_PKG is GENMASK(14, 12), so FIELD_GET() yields a
> 3-bit value here, but it is stored straight into the bool
> priv->is_an8811hbcn.  Any non-zero package code therefore collapses to
> true, which means the whole variant decision in
> an8811hb_led_gpio_setup() rests on the unstated assumption that
> AN8811HBCN is the only encoding with a non-zero value and AN8811HBN
> reads back exactly 0.
> 
> If the field can take any other value - a second AN8811HBN encoding, a
> reserved code, or a future package - it lands in the
> AN8811HB_GPIO_OUTPUT_0115 branch and the driver drives GPIO 0/1/15
> plus the SEL1/SEL2 mux for a part whose LEDs are not on those pads.
> That failure mode is silent: the LEDs simply do not work, and there is
> nothing in the source that documents what the encoding actually is.
> 
> Could you define the expected package identifiers as named constants
> and compare against them explicitly, e.g. something like
> 
> 	pkg = FIELD_GET(AN8811HB_HWTRAP2_PKG, reg_val);
> 	priv->is_an8811hbcn = (pkg == AN8811HB_HWTRAP2_PKG_HBCN);
> 
> so that unknown codes fall into the conservative branch rather than
> being treated as AN8811HBCN?  If the hardware really only ever reports
> two values and 0 is guaranteed to mean AN8811HBN, please say so in a
> comment next to the assignment, since the reader cannot tell that from
> the mask alone.  Also, is it worth warning (or failing probe) on an
> unrecognised package code rather than silently picking a GPIO mapping?

I think we are better off without per pkg macros until there are only
2 of them around.

All others are doubtful nit-picks.

@Weiting: note that you are requested to address sashiko comments
proactively.

/P


> 

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

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon, 7 Sep 2026 09:56:37 +0800 you 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.
> 
> [...]

Here is the summary with links:
  - [v3,net-next] net: phy: air_en8811h: select LED GPIO pins based on AN8811HB package variant
    https://git.kernel.org/netdev/net-next/c/10ca508878b6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html