Messages in this thread | | | Subject | Re: [PATCH net-next v3 7/7] net: phy: use phy_read_poll_timeout() to simplify the code | From | Florian Fainelli <> | Date | Sat, 21 Mar 2020 19:57:11 -0700 |
| |
On 3/21/2020 7:48 PM, Dejin Zheng wrote: > use phy_read_poll_timeout() to replace the poll codes for > simplify the code in phy_poll_reset() function. > > Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com> > --- > v2 -> v3: > - adapt to it after modifying the parameter order of the > newly added function > v1 -> v2: > - remove the handle of phy_read()'s return error. > > drivers/net/phy/phy_device.c | 16 ++++------------ > 1 file changed, 4 insertions(+), 12 deletions(-) > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > index a585faf8b844..cfe7aae35084 100644 > --- a/drivers/net/phy/phy_device.c > +++ b/drivers/net/phy/phy_device.c > @@ -1059,23 +1059,15 @@ EXPORT_SYMBOL(phy_disconnect); > static int phy_poll_reset(struct phy_device *phydev) > { > /* Poll until the reset bit clears (50ms per retry == 0.6 sec) */ > - unsigned int retries = 12; > - int ret; > - > - do { > - msleep(50); > - ret = phy_read(phydev, MII_BMCR);
You are doing some subtle changes here, the sleep used to be *before* the read of BMCR and now it will be *after*. If there were PHY devices that required 50ms at least, but would incorrectly return that BMCR_RESET is cleared *before* 50ms, then we would be breaking those.
I would recommend we drop this patch for now, the rest looks good to me though. -- Florian
| |