Re: [PATCH net-next v4 3/3] net: phy: add a PHY write barrier when disabling interrupts

[Andrew Lunn <andrew@lunn.ch>]

> static void phy_write_barrier(struct phy_device *phydev)
> {
>        int err;
>  
>        err = mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, MII_PHYSID1);
>        if (err == -EOPNOTSUPP)
>                mdiobus_c45_read(phydev->mdio.bus, phydev->mdio.addr,
>                                 __ffs(phydev->c45_ids.mmds_present),
>                                 MII_PHYSID1);
> }

Using __ffs() is maybe more complex than needed.

All you are trying to do is ensure the last write happened, by doing a
read. Any read should work, even if the device does not respond. We
just need to be careful not to read a register which might clear on
read, such as an interrupt status register, or the link status, which
latches. MII_PHYID1 is safe. Since we throw away the value, we don't
care if the MMD is not present, the read will still flush the previous
write. So i would replace the _ffs() with a hard coded value. KISS.

> Do you think there's any way I can test this on my VSC8574 or VSC8541? It
> supports some C45 registers for EEE but not the device discovery part.

It is not so easy to do. You need to hack the C22 read so that is
returns EOPNOTSUPP, but you also need the first few reads to return a
valid value otherwise the probe will fail.

I think this is one of the cases that if the reviewers thinks its
looks O.K, we can accept it without extensive testing.

      Andrew