Re: [PATCH v2] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support

Eric Woudstra 於 2026/3/27 下午 12:41 寫道:

On 3/26/26 4:35 PM, Lucien.Jheng wrote:

quoted

AN8811HB needs a MCU soft-reset cycle before firmware loading begins.
Assert the MCU (hold it in reset) and immediately deassert (release)
via a dedicated PBUS register pair (0x5cf9f8 / 0x5cf9fc), accessed
through the PHY-addr+8 MDIO bus node rather than the BUCKPBUS indirect
path.  This clears the MCU state before the firmware loading sequence
starts.

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.

Fixes: 0a55766b7711 ("net: phy: air_en8811h: add Airoha AN8811HB support")
Signed-off-by: Lucien Jheng <redacted>
---
Changes in v2:
- Rewrite commit message: The previous wording was ambiguous,
   as it suggested the MCU remains asserted during the entire
   firmware loading process, rather than a quick reset cycle
   before it starts.
   Clarify that assert and deassert is an immediate soft-reset
   cycle to clear MCU state before firmware loading begins.
- Add Fixes: 0a55766b7711 ("net: phy: air_en8811h: add Airoha AN8811HB
   support").
- Change phydev_info() to phydev_dbg() in an8811hb_mcu_assert() and
   an8811hb_mcu_deassert() to avoid noise during normal boot.

  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..01fce1b93618 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;
+}
+

Maybe you can use mutex_lock() and mutex_unlock() here also, like so:

static int __air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
				u32 pbus_data)
{
	int ret;

	ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_EXT_PAGE_ACCESS,
			      (u16)(pbus_address >> 6));
	if (ret < 0)
		return ret;

	ret = __mdiobus_write(mdio->bus, mdio->addr, (pbus_address >> 2) & 0xf,
			      (u16)pbus_data);
	if (ret < 0)
		return ret;

	ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_PBUS_DATA_HIGH,
			      (u16)(pbus_data >> 16));
	if (ret < 0)
		return ret;

	return 0;
}

static int air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
			      u32 pbus_data)
{
	int ret;

	mutex_lock(&mdio->bus->mdio_lock);

	ret = __air_pbus_reg_write(mdio, pbus_address, pbus_data);
	if (ret < 0)
		dev_err(&mdio->dev, "%s 0x%08x failed: %d\n", __func__,
			pbus_address, ret);

	mutex_unlock(&mdio->bus->mdio_lock);

	return ret;
}

static int __air_pbus_reg_read(struct mdio_device *mdio, u32 pbus_address,
			       u32 *pbus_data)
{
	int ret, pbus_data_low;

	ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_EXT_PAGE_ACCESS,
			      (u16)(pbus_address >> 6));
	if (ret < 0)
		return ret;

	ret = __mdiobus_read(mdio->bus, mdio->addr, (pbus_address >> 2) & 0xf);
	if (ret < 0)
		return ret;
	pbus_data_low = ret;

	ret = __mdiobus_read(mdio->bus, mdio->addr, AIR_PBUS_DATA_HIGH);
	if (ret < 0)
		return ret;

	*pbus_data = (ret << 16) + pbus_data_low;

	return 0;
}

static int air_pbus_reg_read(struct mdio_device *mdio, u32 pbus_address,
			     u32 *pbus_data)
{
	int ret;

	mutex_lock(&mdio->bus->mdio_lock);

	ret = __air_pbus_reg_read(mdio, pbus_address, pbus_data);
	if (ret < 0)
		dev_err(&mdio->dev, "%s 0x%08x failed: %d\n", __func__,
			pbus_address, ret);

	mutex_unlock(&mdio->bus->mdio_lock);

	return ret;
}

With:

#define AIR_PBUS_DATA_HIGH		0x10

Thank you for your feedback.

I should add mutex_lock() and mutex_unlock() to protect these operations.

I will include this in the next version.

quoted

  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_dbg(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_dbg(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