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

[Lucien.Jheng]

Paolo Abeni 於 2026/4/23 下午 07:19 寫道:
> On 4/20/26 3:45 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 a registered mdio_device at PHY-addr+8.
> >
> > Add __air_pbus_reg_write() as a low-level helper taking a struct
> > mdio_device *, create and register the PBUS mdio_device in
> > an8811hb_probe() and store it in priv->pbusdev, then implement
> > an8811hb_mcu_assert() / _deassert() on top of it. Add
> > an8811hb_remove() to unregister the PBUS device on teardown. Wire
> > both calls 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")
> The hash is incorrect, should be 5afda1d734ed
I will fix it in the next patch.
> [...]
> > @@ -254,6 +267,31 @@ 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 mdio_device *mdiodev,
> > +				u32 pbus_reg, u32 pbus_data)
> > +{
> > +	int ret;
> > +
> > +	ret = __mdiobus_write(mdiodev->bus, mdiodev->addr, AIR_EXT_PAGE_ACCESS,
> > +			      upper_16_bits(pbus_reg));
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = __mdiobus_write(mdiodev->bus, mdiodev->addr, AIR_PBUS_ADDR_HIGH,
> > +			      (pbus_reg & GENMASK(15, 6)) >> 6);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = __mdiobus_write(mdiodev->bus, mdiodev->addr,
> > +			      (pbus_reg & GENMASK(5, 2)) >> 2,
> > +			      lower_16_bits(pbus_data));
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return __mdiobus_write(mdiodev->bus, mdiodev->addr, AIR_PBUS_DATA_HIGH,
> > +			       upper_16_bits(pbus_data));
> Sashiko says:
>
> Will writing the lower 16 bits before the upper 16 bits cause the
> hardware transaction to fire with stale upper data?
> The __air_buckpbus_reg_write() helper triggers the 32-bit transaction
> using the lower 16 bits as the execution trigger. If this hardware
> behaves similarly, should AIR_PBUS_DATA_HIGH be populated before writing
> the lower 16 bits?
This is an8811hb pbus sequence. Therefore, it doesn't need to be modified.
> [...]
> > @@ -1175,10 +1281,22 @@ static int an8811hb_probe(struct phy_device *phydev)
> >   		return -ENOMEM;
> >   	phydev->priv = priv;
> >   
> > +	mdiodev = mdio_device_create(phydev->mdio.bus,
> > +				     phydev->mdio.addr + EN8811H_PBUS_ADDR_OFFS);
> Sashiko says:
>
> Can this create an out-of-bounds array access if the base PHY address is
> high?
> The mdio_map array in struct mii_bus has a fixed size of PHY_MAX_ADDR (32).
> If phydev->mdio.addr is 24 or higher, adding EN8811H_PBUS_ADDR_OFFS (8)
> will result in an address of 32 or more.
> Neither mdio_device_create() nor mdio_device_register() validate that
> the address is within PHY_MAX_ADDR. When mdiobus_register_device()
> executes mdiodev->bus->mdio_map[mdiodev->addr] = mdiodev, could this
> write past the end of the array and corrupt adjacent memory?
>
> /P