Messages in this thread | | | From | Clark Wang <> | Subject | RE: [PATCH 1/2] net: phylink: add sync flag mac_ready to fix resume issue with WoL enabled | Date | Wed, 30 Nov 2022 11:40:54 +0000 |
| |
> -----Original Message----- > From: Russell King <linux@armlinux.org.uk> > Sent: 2022年11月30日 19:33 > To: Clark Wang <xiaoning.wang@nxp.com> > Cc: peppe.cavallaro@st.com; alexandre.torgue@foss.st.com; > joabreu@synopsys.com; davem@davemloft.net; edumazet@google.com; > kuba@kernel.org; pabeni@redhat.com; mcoquelin.stm32@gmail.com; > andrew@lunn.ch; hkallweit1@gmail.com; netdev@vger.kernel.org; > linux-stm32@st-md-mailman.stormreply.com; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 1/2] net: phylink: add sync flag mac_ready to fix resume > issue with WoL enabled > > On Wed, Nov 30, 2022 at 11:23:42AM +0000, Russell King (Oracle) wrote: > > On Wed, Nov 30, 2022 at 07:11:47PM +0800, Clark Wang wrote: > > > Issue we met: > > > On some platforms, mac cannot work after resumed from the suspend > > > with WoL enabled. > > > > > > The cause of the issue: > > > 1. phylink_resolve() is in a workqueue which will not be executed > immediately. > > > This is the call sequence: > > > phylink_resolve()->phylink_link_up()->pl->mac_ops->mac_link_up() > > > For stmmac driver, mac_link_up() will set the correct speed/duplex... > > > values which are from link_state. > > > 2. In stmmac_resume(), it will call stmmac_hw_setup() after called the > > > phylink_resume(). stmmac_core_init() is called in function > > > stmmac_hw_setup(), > > > > ... and that is where the problem is. Don't call phylink_resume() > > before your hardware is ready to see a link-up event. > > ... and while that is being fixed, maybe the stupid code in > stmmac_resume() can also be fixed: > > 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(); > > 1. phylink_resume() is always called after that first rtnl_lock(), so there's no > point it being stupidly in each side of the if(). > > 2. the rtnl_unlock() followed by rtnl_lock() is completely unnecessary. > > Thanks. >
Thanks for your reminder, I will send another patch to clean this. Sorry, the last empty email was sent by mistake.
Best Regards, Clark Wang
> -- > RMK's Patch system: > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.ar > mlinux.org.uk%2Fdeveloper%2Fpatches%2F&data=05%7C01%7Cxiaoning. > wang%40nxp.com%7Cc0961fa17589464a742308dad2c69502%7C686ea1d3bc2 > b4c6fa92cd99c5c301635%7C0%7C0%7C638054047593066949%7CUnknown% > 7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi > LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=wURkTxhK2IlqktOKmJzx > Nr8E4KCzd1gjDgHK3iSy6EA%3D&reserved=0 > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
| |