On Mon, 7 Dec 2020 19:38:49 +0800 Joakim Zhang wrote:
>
> commit 1c35cc9cf6a0 ("net: stmmac: remove redundant null check before
> clk_disable_unprepare()"),
> have not clean up check NULL clock parameter completely, this patch did it.
>
> commit e8377e7a29efb ("net: stmmac: only call pmt() during suspend/resume if
> HW enables PMT"),
> after this patch, we use
> if (device_may_wakeup(priv->device) && priv->plat->pmt) check MAC wakeup
> if (device_may_wakeup(priv->device)) check PHY wakeup
> Add oneline comment for readability.
>
> commit 77b2898394e3b ("net: stmmac: Speed down the PHY if WoL to save
> energy"),
> slow down phy speed when release net device under any condition.
>
> Slightly adjust the order of the codes so that suspend/resume look more
> symmetrical, generally speaking they should appear symmetrically.
>
> Signed-off-by: Joakim Zhang <qiangqing.zh...@nxp.com>
> ---
> .../net/ethernet/stmicro/stmmac/stmmac_main.c | 22 +++++++++----------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index c33db79cdd0a..a46e865c4acc 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2908,8 +2908,7 @@ static int stmmac_release(struct net_device *dev)
> struct stmmac_priv *priv = netdev_priv(dev);
> u32 chan;
>
> - if (device_may_wakeup(priv->device))
This check is to prevent link speed down if the stmmac isn't a wakeup device.
> - phylink_speed_down(priv->phylink, false);
> + phylink_speed_down(priv->phylink, false);
> /* Stop and disconnect the PHY */
> phylink_stop(priv->phylink);
> phylink_disconnect_phy(priv->phylink);
> @@ -5183,6 +5182,7 @@ int stmmac_suspend(struct device *dev)
> } else {
> mutex_unlock(&priv->lock);
> rtnl_lock();
> + /* For PHY wakeup case */
> if (device_may_wakeup(priv->device))
> phylink_speed_down(priv->phylink, false);
> phylink_stop(priv->phylink);
> @@ -5260,11 +5260,17 @@ int stmmac_resume(struct device *dev)
> /* enable the clk previously disabled */
> clk_prepare_enable(priv->plat->stmmac_clk);
> clk_prepare_enable(priv->plat->pclk);
> - if (priv->plat->clk_ptp_ref)
> - clk_prepare_enable(priv->plat->clk_ptp_ref);
> + clk_prepare_enable(priv->plat->clk_ptp_ref);
I think this 3 line modifications can be a separated patch.
> /* reset the phy so that it's ready */
> if (priv->mii)
> stmmac_mdio_reset(priv->mii);
> +
> + rtnl_lock();
> + phylink_start(priv->phylink);
> + /* We may have called phylink_speed_down before */
> + if (device_may_wakeup(priv->device))
> + phylink_speed_up(priv->phylink);
> + rtnl_unlock();
This is moving phylink op before mac setup, I'm not sure whether this is safe.
> }
>
> if (priv->plat->serdes_powerup) {
> @@ -5275,14 +5281,6 @@ int stmmac_resume(struct device *dev)
> return ret;
> }
>
> - if (!device_may_wakeup(priv->device) || !priv->plat->pmt) {
> - rtnl_lock();
> - phylink_start(priv->phylink);
> - /* We may have called phylink_speed_down before */
> - phylink_speed_up(priv->phylink);
> - rtnl_unlock();
> - }
> -
> rtnl_lock();
> mutex_lock(&priv->lock);
>
> --
> 2.17.1
>
