2022-12-21 08:08:27

by Clark Wang

Subject: [PATCH 2/2] net: stmmac: resume phy separately before calling stmmac_hw_setup()

On some platforms, mac cannot work after resumed from the suspend with WoL
enabled.

We found the stmmac_hw_setup() when system resumes will called after the
stmmac_mac_link_up(). So the correct values set in stmmac_mac_link_up()
are overwritten by stmmac_core_init() in phylink_resume().

So call the new added function in phylink to resume phy fristly.
Then can call the stmmac_hw_setup() before calling phy_resume().

Signed-off-by: Clark Wang <[email protected]>
---
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index c6951c976f5d..d0bdc9b6dbe8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7532,16 +7532,9 @@ int stmmac_resume(struct device *dev)
}

rtnl_lock();
- if (device_may_wakeup(priv->device) && priv->plat->pmt) {
- phylink_resume(priv->phylink);
- } else {
- phylink_resume(priv->phylink);
- if (device_may_wakeup(priv->device))
- phylink_speed_up(priv->phylink);
- }
- rtnl_unlock();

- rtnl_lock();
+ phylink_phy_resume(priv->phylink);
+
mutex_lock(&priv->lock);

stmmac_reset_queues_param(priv);
@@ -7559,6 +7552,11 @@ int stmmac_resume(struct device *dev)
stmmac_enable_all_dma_irq(priv);

mutex_unlock(&priv->lock);
+
+ phylink_resume(priv->phylink);
+ if (device_may_wakeup(priv->device) && !priv->plat->pmt)
+ phylink_speed_up(priv->phylink);
+
rtnl_unlock();

netif_device_attach(ndev);
--
2.34.1

2022-12-22 20:28:33

by Piotr Raczynski

Subject: Re: [PATCH 2/2] net: stmmac: resume phy separately before calling stmmac_hw_setup()

On Wed, Dec 21, 2022 at 04:01:44PM +0800, Clark Wang wrote:
> On some platforms, mac cannot work after resumed from the suspend with WoL
> enabled.
>
> We found the stmmac_hw_setup() when system resumes will called after the
> stmmac_mac_link_up(). So the correct values set in stmmac_mac_link_up()
> are overwritten by stmmac_core_init() in phylink_resume().
>
> So call the new added function in phylink to resume phy fristly.
first.
> Then can call the stmmac_hw_setup() before calling phy_resume().

It'd be nice to add Fixes tag with appropriate commit id.
Other than that looks fine.

Reviewed-by: Piotr Raczynski <[email protected]>

>
> Signed-off-by: Clark Wang <[email protected]>
> ---
> .../net/ethernet/stmicro/stmmac/stmmac_main.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index c6951c976f5d..d0bdc9b6dbe8 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -7532,16 +7532,9 @@ int stmmac_resume(struct device *dev)
> }
>
> rtnl_lock();
> - if (device_may_wakeup(priv->device) && priv->plat->pmt) {
> - phylink_resume(priv->phylink);
> - } else {
> - phylink_resume(priv->phylink);
> - if (device_may_wakeup(priv->device))
> - phylink_speed_up(priv->phylink);
> - }
> - rtnl_unlock();
>
> - rtnl_lock();
> + phylink_phy_resume(priv->phylink);
> +
> mutex_lock(&priv->lock);
>
> stmmac_reset_queues_param(priv);
> @@ -7559,6 +7552,11 @@ int stmmac_resume(struct device *dev)
> stmmac_enable_all_dma_irq(priv);
>
> mutex_unlock(&priv->lock);
> +
> + phylink_resume(priv->phylink);
> + if (device_may_wakeup(priv->device) && !priv->plat->pmt)
> + phylink_speed_up(priv->phylink);
> +
> rtnl_unlock();
>
> netif_device_attach(ndev);
> --
> 2.34.1
>