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

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

Hi Jakub

Jakub Kicinski 於 2026/5/21 上午 07:54 寫道:
> On Sun, 17 May 2026 14:50:41 +0800 Lucien.Jheng wrote:
>> +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));
> Please add proper defines for these GENMASK'ed fields and use
> FIELD_GET() to access the fields?
I will update it in the next patch.
>
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return __mdiobus_write(mdiodev->bus, mdiodev->addr, AIR_PBUS_DATA_HIGH,
>> +			       upper_16_bits(pbus_data));
>> +}
>> @@ -1175,10 +1281,21 @@ 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);
> AI says:
>
> What happens when phydev->mdio.addr is in the range 24..31?
> MDIO addresses 0..31 are all valid 5-bit hardware addresses, so a
> strap-selected AN8811HB at address >= 24 makes
> phydev->mdio.addr + EN8811H_PBUS_ADDR_OFFS land in 32..39. struct
> mii_bus has mdio_map[PHY_MAX_ADDR] with PHY_MAX_ADDR == 32, and
> mdiobus_register_device() does:
> 	if (mdiodev->bus->mdio_map[mdiodev->addr])
> 		return -EBUSY;
> 	...
> 	mdiodev->bus->mdio_map[mdiodev->addr] = mdiodev;
> with no bounds check on mdiodev->addr. The OOB read from
> mdio_map[32..39] may return garbage that happens to be non-NULL
> (probe fails with -EBUSY) or NULL (the OOB store overwrites adjacent
> fields of struct mii_bus such as phy_mask, phy_ignore_ta_mask, or
> the irq[] array).
> Even without corruption, every later __air_pbus_reg_write() goes
> through __mdiobus_write(), which has:
> 	if (addr >= PHY_MAX_ADDR)
> 		return -ENXIO;
> so each MCU assert/deassert silently fails with -ENXIO, which seems
> to contradict the commit message:
>      so every firmware load or MCU restart on AN8811HB correctly
>      sequences the reset control registers.
> Should an8811hb_probe() reject configurations where
> phydev->mdio.addr + EN8811H_PBUS_ADDR_OFFS >= PHY_MAX_ADDR, or
> otherwise enforce the assumed strap-pin constraint?

As previously discussed, the fact that the AN8811HB PHY address is 
restricted to 8–15 (decimal)—meaning the PBUS address will only be 
within the range of 16–21 (decimal)—will be documented in the comments 
of the upcoming version.

Thanks