| Message ID | 20200322174943.26332-9-zhengdejin5@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | introduce read_poll_timeout | expand |
On Mon, Mar 23, 2020 at 01:49:41AM +0800, 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> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Mon, Mar 23, 2020 at 01:49:41AM +0800, 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> > --- > v4 -> v5: > - it can add msleep before call phy_read_poll_timeout() > to keep the code more similar. so add it. > v3 -> v4: > - drop it. > 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 | 17 +++++------------ > 1 file changed, 5 insertions(+), 12 deletions(-) > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > index a585faf8b844..24297ee7f626 100644 > --- a/drivers/net/phy/phy_device.c > +++ b/drivers/net/phy/phy_device.c > @@ -1059,23 +1059,16 @@ 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); > - if (ret < 0) > - return ret; > - } while (ret & BMCR_RESET && --retries); > - if (ret & BMCR_RESET) > - return -ETIMEDOUT; > + int ret, val; > > + msleep(50); > + ret = phy_read_poll_timeout(phydev, MII_BMCR, val, !(val & BMCR_RESET), > + 50000, 550000); > /* Some chips (smsc911x) may still need up to another 1ms after the > * BMCR_RESET bit is cleared before they are usable. > */ > msleep(1); > - return 0; > + return ret; This isn't actually equivaent behaviour. If the read timed out, the old code didn't wait 1ms. Your new code does. However, since we've waited about 600ms already, it probably doesn't matter. These sorts of things should be documented in the commit log, IMHO, so it's obvious that it's been considered.
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index a585faf8b844..24297ee7f626 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1059,23 +1059,16 @@ 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); - if (ret < 0) - return ret; - } while (ret & BMCR_RESET && --retries); - if (ret & BMCR_RESET) - return -ETIMEDOUT; + int ret, val; + msleep(50); + ret = phy_read_poll_timeout(phydev, MII_BMCR, val, !(val & BMCR_RESET), + 50000, 550000); /* Some chips (smsc911x) may still need up to another 1ms after the * BMCR_RESET bit is cleared before they are usable. */ msleep(1); - return 0; + return ret; } int phy_init_hw(struct phy_device *phydev)
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> --- v4 -> v5: - it can add msleep before call phy_read_poll_timeout() to keep the code more similar. so add it. v3 -> v4: - drop it. 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 | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-)