public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] net: phy: airoha: add AN8811HB MCU assert/deassert support
@ 2026-03-15 13:41 Lucien.Jheng
  2026-03-17 18:18 ` Andrew Lunn
  0 siblings, 1 reply; 3+ messages in thread
From: Lucien.Jheng @ 2026-03-15 13:41 UTC (permalink / raw)
  To: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni, netdev,
	linux-kernel
  Cc: ericwouds, frank-w, daniel, lucien.jheng, Lucien.Jheng

AN8811HB requires the MCU to be held in reset before firmware
loading and released afterwards via a dedicated PBUS register
pair (0x5cf9f8 / 0x5cf9fc), accessed through the PHY-addr+8
MDIO bus node rather than the BUCKPBUS indirect path.

Add __air_pbus_reg_write() as a low-level helper for this
access, then implement an8811hb_mcu_assert() / _deassert()
on top of it.  Wire both into an8811hb_load_firmware() and
en8811h_restart_mcu() so every firmware load or MCU restart
on AN8811HB correctly sequences the reset control registers.

Signed-off-by: Lucien Jheng <lucienzx159@gmail.com>
---
 drivers/net/phy/air_en8811h.c | 105 ++++++++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)

diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
index 29ae73e65caa..d36e946aa224 100644
--- a/drivers/net/phy/air_en8811h.c
+++ b/drivers/net/phy/air_en8811h.c
@@ -170,6 +170,16 @@
 #define   AN8811HB_CLK_DRV_CKO_LDPWD		BIT(13)
 #define   AN8811HB_CLK_DRV_CKO_LPPWD		BIT(14)
 
+#define AN8811HB_MCU_SW_RST		0x5cf9f8
+#define   AN8811HB_MCU_SW_RST_HOLD		BIT(16)
+#define   AN8811HB_MCU_SW_RST_RUN		(BIT(16) | BIT(0))
+#define AN8811HB_MCU_SW_START		0x5cf9fc
+#define   AN8811HB_MCU_SW_START_EN		BIT(16)
+
+/* MII register constants for PBUS access (PHY addr + 8) */
+#define AIR_PBUS_ADDR_HIGH		0x1c
+#define AIR_PBUS_DATA_HIGH		0x10
+
 /* Led definitions */
 #define EN8811H_LED_COUNT	3
 
@@ -254,6 +264,36 @@ static int air_phy_write_page(struct phy_device *phydev, int page)
 	return __phy_write(phydev, AIR_EXT_PAGE_ACCESS, page);
 }
 
+static int __air_pbus_reg_write(struct phy_device *phydev,
+				u32 pbus_reg, u32 pbus_data)
+{
+	struct mii_bus *bus = phydev->mdio.bus;
+	int pbus_addr = phydev->mdio.addr + 8;
+	int ret;
+
+	ret = __mdiobus_write(bus, pbus_addr, AIR_EXT_PAGE_ACCESS,
+			      upper_16_bits(pbus_reg));
+	if (ret < 0)
+		return ret;
+
+	ret = __mdiobus_write(bus, pbus_addr, AIR_PBUS_ADDR_HIGH,
+			      (pbus_reg & GENMASK(15, 6)) >> 6);
+	if (ret < 0)
+		return ret;
+
+	ret = __mdiobus_write(bus, pbus_addr, (pbus_reg & GENMASK(5, 2)) >> 2,
+			      lower_16_bits(pbus_data));
+	if (ret < 0)
+		return ret;
+
+	ret = __mdiobus_write(bus, pbus_addr, AIR_PBUS_DATA_HIGH,
+			      upper_16_bits(pbus_data));
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 static int __air_buckpbus_reg_write(struct phy_device *phydev,
 				    u32 pbus_address, u32 pbus_data)
 {
@@ -570,10 +610,65 @@ static int an8811hb_load_file(struct phy_device *phydev, const char *name,
 	return ret;
 }
 
+static int an8811hb_mcu_assert(struct phy_device *phydev)
+{
+	int ret;
+
+	phy_lock_mdio_bus(phydev);
+
+	ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_RST,
+				   AN8811HB_MCU_SW_RST_HOLD);
+	if (ret < 0)
+		goto unlock;
+
+	ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_START, 0);
+	if (ret < 0)
+		goto unlock;
+
+	msleep(50);
+	phydev_info(phydev, "MCU asserted\n");
+
+unlock:
+	phy_unlock_mdio_bus(phydev);
+	return ret;
+}
+
+static int an8811hb_mcu_deassert(struct phy_device *phydev)
+{
+	int ret;
+
+	phy_lock_mdio_bus(phydev);
+
+	ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_START,
+				   AN8811HB_MCU_SW_START_EN);
+	if (ret < 0)
+		goto unlock;
+
+	ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_RST,
+				   AN8811HB_MCU_SW_RST_RUN);
+	if (ret < 0)
+		goto unlock;
+
+	msleep(50);
+	phydev_info(phydev, "MCU deasserted\n");
+
+unlock:
+	phy_unlock_mdio_bus(phydev);
+	return ret;
+}
+
 static int an8811hb_load_firmware(struct phy_device *phydev)
 {
 	int ret;
 
+	ret = an8811hb_mcu_assert(phydev);
+	if (ret < 0)
+		return ret;
+
+	ret = an8811hb_mcu_deassert(phydev);
+	if (ret < 0)
+		return ret;
+
 	ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
 				     EN8811H_FW_CTRL_1_START);
 	if (ret < 0)
@@ -662,6 +757,16 @@ static int en8811h_restart_mcu(struct phy_device *phydev)
 {
 	int ret;
 
+	if (phy_id_compare_model(phydev->phy_id, AN8811HB_PHY_ID)) {
+		ret = an8811hb_mcu_assert(phydev);
+		if (ret < 0)
+			return ret;
+
+		ret = an8811hb_mcu_deassert(phydev);
+		if (ret < 0)
+			return ret;
+	}
+
 	ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
 				     EN8811H_FW_CTRL_1_START);
 	if (ret < 0)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] net: phy: airoha: add AN8811HB MCU assert/deassert support
  2026-03-15 13:41 [PATCH v1] net: phy: airoha: add AN8811HB MCU assert/deassert support Lucien.Jheng
@ 2026-03-17 18:18 ` Andrew Lunn
  2026-03-23 14:11   ` Lucien.Jheng
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Lunn @ 2026-03-17 18:18 UTC (permalink / raw)
  To: Lucien.Jheng
  Cc: hkallweit1, linux, davem, edumazet, kuba, pabeni, netdev,
	linux-kernel, ericwouds, frank-w, daniel, lucien.jheng

On Sun, Mar 15, 2026 at 09:41:55PM +0800, Lucien.Jheng wrote:
> AN8811HB requires the MCU to be held in reset before firmware
> loading and released afterwards via a dedicated PBUS register
> pair (0x5cf9f8 / 0x5cf9fc), accessed through the PHY-addr+8
> MDIO bus node rather than the BUCKPBUS indirect path.
> 
> Add __air_pbus_reg_write() as a low-level helper for this
> access, then implement an8811hb_mcu_assert() / _deassert()
> on top of it.  Wire both into an8811hb_load_firmware() and
> en8811h_restart_mcu() so every firmware load or MCU restart
> on AN8811HB correctly sequences the reset control registers.

What happens if the MCU is not held in reset? Should this be
considered a fix?

What are the valid PHY addresses for this device? Do we need to worry
about the PHY is at address 31, is the dedicated PBUS register is at
7?

> +static int an8811hb_mcu_assert(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	phy_lock_mdio_bus(phydev);
> +
> +	ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_RST,
> +				   AN8811HB_MCU_SW_RST_HOLD);
> +	if (ret < 0)
> +		goto unlock;
> +
> +	ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_START, 0);
> +	if (ret < 0)
> +		goto unlock;
> +
> +	msleep(50);
> +	phydev_info(phydev, "MCU asserted\n");

Please don't spam the log. phydev_dbg() or not at all.

>  static int an8811hb_load_firmware(struct phy_device *phydev)
>  {
>  	int ret;
>  
> +	ret = an8811hb_mcu_assert(phydev);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = an8811hb_mcu_deassert(phydev);
> +	if (ret < 0)
> +		return ret;
> +

Assert and then deassert, but no firmware download between? That is
not what the commit message says.

	Andrew

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] net: phy: airoha: add AN8811HB MCU assert/deassert support
  2026-03-17 18:18 ` Andrew Lunn
@ 2026-03-23 14:11   ` Lucien.Jheng
  0 siblings, 0 replies; 3+ messages in thread
From: Lucien.Jheng @ 2026-03-23 14:11 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: hkallweit1, linux, davem, edumazet, kuba, pabeni, netdev,
	linux-kernel, ericwouds, frank-w, daniel, lucien.jheng

Hi Andrew

Sorry for the late reply.

Andrew Lunn 於 2026/3/18 上午 02:18 寫道:
> On Sun, Mar 15, 2026 at 09:41:55PM +0800, Lucien.Jheng wrote:
>> AN8811HB requires the MCU to be held in reset before firmware
>> loading and released afterwards via a dedicated PBUS register
>> pair (0x5cf9f8 / 0x5cf9fc), accessed through the PHY-addr+8
>> MDIO bus node rather than the BUCKPBUS indirect path.
>>
>> Add __air_pbus_reg_write() as a low-level helper for this
>> access, then implement an8811hb_mcu_assert() / _deassert()
>> on top of it.  Wire both into an8811hb_load_firmware() and
>> en8811h_restart_mcu() so every firmware load or MCU restart
>> on AN8811HB correctly sequences the reset control registers.
> What happens if the MCU is not held in reset? Should this be
> considered a fix?
If the MCU does not perform the assert/deassert sequence before loading 
the firmware, the AN8811HB may not function correctly.

Yes, this should be considered a fix.
>
> What are the valid PHY addresses for this device? Do we need to worry
> about the PHY is at address 31, is the dedicated PBUS register is at
> 7?
The AN8811HB PHY address is restricted to 8–15 (decimal); therefore, the 
PBUS address will only be within the range of 16–21 (decimal).

Therefore, we don't need to worry about the PHY is at address 31.
>> +static int an8811hb_mcu_assert(struct phy_device *phydev)
>> +{
>> +	int ret;
>> +
>> +	phy_lock_mdio_bus(phydev);
>> +
>> +	ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_RST,
>> +				   AN8811HB_MCU_SW_RST_HOLD);
>> +	if (ret < 0)
>> +		goto unlock;
>> +
>> +	ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_START, 0);
>> +	if (ret < 0)
>> +		goto unlock;
>> +
>> +	msleep(50);
>> +	phydev_info(phydev, "MCU asserted\n");
> Please don't spam the log. phydev_dbg() or not at all.
Got it. I’ll use phydev_dbg() instead to keep the log clean.
>
>>   static int an8811hb_load_firmware(struct phy_device *phydev)
>>   {
>>   	int ret;
>>   
>> +	ret = an8811hb_mcu_assert(phydev);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = an8811hb_mcu_deassert(phydev);
>> +	if (ret < 0)
>> +		return ret;
>> +
> Assert and then deassert, but no firmware download between? That is
> not what the commit message says.
The intended sequence is: asserting the MCU, deasserting it, and then 
downloading the firmware."

I will update the commit message.
>
> 	Andrew

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-03-23 14:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-15 13:41 [PATCH v1] net: phy: airoha: add AN8811HB MCU assert/deassert support Lucien.Jheng
2026-03-17 18:18 ` Andrew Lunn
2026-03-23 14:11   ` Lucien.Jheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox