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

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

[Lucien.Jheng]

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 <lucienzx159@gmail.com>
---
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;
+}
+
 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

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

[Eric Woudstra]

On 3/26/26 4:35 PM, Lucien.Jheng wrote:
> 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.

Perhapse you can register this extra device in the probe() function:

struct mdio_device *mdiodev;

mdiodev = mdio_device_create(phydev->mdio.bus,
			     phydev->mdio.addr + EN8811H_PBUS_ADDR_OFFS);
if (IS_ERR(mdiodev))
	return PTR_ERR(mdiodev);

ret = mdio_device_register(mdiodev);
if (ret) {
	mdio_device_free(mdiodev);
	return ret;
}

priv->pbusdev = mdiodev;

In the define part add:

#define EN8811H_PBUS_ADDR_OFFS	8

Add to struct en8811h_priv:

struct mdio_device	*pbusdev;

And in the remove function:

mdio_device_remove(priv->pbusdev);
mdio_device_free(priv->pbusdev);

Then you can use the device:

ret = en8811hb_some_neat_function(priv->pbusdev);

> 
> 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 <lucienzx159@gmail.com>
> ---
> 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;
> +}
> +
>  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
> 

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

[Lucien.Jheng]

Eric Woudstra 於 2026/3/27 上午 04:00 寫道:
>
> On 3/26/26 4:35 PM, Lucien.Jheng wrote:
>> 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.
> Perhapse you can register this extra device in the probe() function:
>
> struct mdio_device *mdiodev;
>
> mdiodev = mdio_device_create(phydev->mdio.bus,
> 			     phydev->mdio.addr + EN8811H_PBUS_ADDR_OFFS);
> if (IS_ERR(mdiodev))
> 	return PTR_ERR(mdiodev);
>
> ret = mdio_device_register(mdiodev);
> if (ret) {
> 	mdio_device_free(mdiodev);
> 	return ret;
> }
>
> priv->pbusdev = mdiodev;
>
> In the define part add:
>
> #define EN8811H_PBUS_ADDR_OFFS	8
>
> Add to struct en8811h_priv:
>
> struct mdio_device	*pbusdev;
>
> And in the remove function:
>
> mdio_device_remove(priv->pbusdev);
> mdio_device_free(priv->pbusdev);
>
> Then you can use the device:
>
> ret = en8811hb_some_neat_function(priv->pbusdev);

Thank you for the feedback

I will handle the registration of this extra device within the probe 
function in the next version.


>> 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 <lucienzx159@gmail.com>
>> ---
>> 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;
>> +}
>> +
>>   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
>>

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

[Eric Woudstra]

On 3/26/26 4:35 PM, Lucien.Jheng wrote:
> 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 <lucienzx159@gmail.com>
> ---
> 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


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

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

[Eric Woudstra]

On 3/27/26 5:41 AM, Eric Woudstra wrote:
> 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

Small correction using upper_16_bits() and lower_16_bits():

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,
			      lower_16_bits(pbus_data));
	if (ret < 0)
		return ret;

	ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_PBUS_DATA_HIGH,
			      upper_16_bits(pbus_data));
	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;
}

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

[Russell King (Oracle)]

On Sat, Mar 28, 2026 at 11:21:59AM +0100, Eric Woudstra wrote:
> static int air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
> 			      u32 pbus_data)
> {
> 	int ret;
> 
> 	mutex_lock(&mdio->bus->mdio_lock);

I wonder why we have phy_lock_mdio_bus() but not mdio_bus_lock()...
Should a helper be provided at mdio bus level to complement the PHY
level helper if we're going to have MDIO drivers directly manipulating
the lock?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

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

[Andrew Lunn]

On Sat, Mar 28, 2026 at 10:25:06AM +0000, Russell King (Oracle) wrote:
> On Sat, Mar 28, 2026 at 11:21:59AM +0100, Eric Woudstra wrote:
> > static int air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
> > 			      u32 pbus_data)
> > {
> > 	int ret;
> > 
> > 	mutex_lock(&mdio->bus->mdio_lock);
> 
> I wonder why we have phy_lock_mdio_bus() but not mdio_bus_lock()...
> Should a helper be provided at mdio bus level to complement the PHY
> level helper if we're going to have MDIO drivers directly manipulating
> the lock?

DSA is pretty much the only class of mdio driver. And they mostly need
to use:

mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);

Not the plain mutex_lock(). 

Hence it has not been added up until now.

But yes, such a helper would make sense for a PHY driver having to do
odd things.

	Andrew

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

[Lucien.Jheng]

Andrew Lunn 於 2026/3/28 下午 10:15 寫道:
> On Sat, Mar 28, 2026 at 10:25:06AM +0000, Russell King (Oracle) wrote:
>> On Sat, Mar 28, 2026 at 11:21:59AM +0100, Eric Woudstra wrote:
>>> static int air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
>>> 			      u32 pbus_data)
>>> {
>>> 	int ret;
>>>
>>> 	mutex_lock(&mdio->bus->mdio_lock);
>> I wonder why we have phy_lock_mdio_bus() but not mdio_bus_lock()...
>> Should a helper be provided at mdio bus level to complement the PHY
>> level helper if we're going to have MDIO drivers directly manipulating
>> the lock?
> DSA is pretty much the only class of mdio driver. And they mostly need
> to use:
>
> mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
>
> Not the plain mutex_lock().
>
> Hence it has not been added up until now.
>
> But yes, such a helper would make sense for a PHY driver having to do
> odd things.
>
> 	Andrew

Thank you for your feedback

I will implement the helper you suggested for this in the next iteration.

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

[Lucien.Jheng]

Eric Woudstra 於 2026/3/28 下午 06:21 寫道:
>
> On 3/27/26 5:41 AM, Eric Woudstra wrote:
>> 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
> Small correction using upper_16_bits() and lower_16_bits():
>
> 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,
> 			      lower_16_bits(pbus_data));
> 	if (ret < 0)
> 		return ret;
>
> 	ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_PBUS_DATA_HIGH,
> 			      upper_16_bits(pbus_data));
> 	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;
> }

Thank you for your feedback.

I will include this in the next version.

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

[Lucien.Jheng]

Eric Woudstra 於 2026/3/27 下午 12:41 寫道:
>
> On 3/26/26 4:35 PM, Lucien.Jheng wrote:
>> 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 <lucienzx159@gmail.com>
>> ---
>> 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.

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