Re: [PATCH v1] net: phy: airoha: add AN8811HB MCU assert/deassert support

["Lucien.Jheng" <lucienzx159@gmail.com>]

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