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

[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