[PATCH net 1/1] net: stmmac: fix MAC and phylink mismatch issue after resume with STMMAC_FLAG_USE_PHY_WOL enabled

[Gan, Yi Fang]

Hi Paolo and Russell King,

After study the information provided, it seems better to find another way to
resolve the issue. Appreciate for the details given. Will try to figure out another
solution.


Best Regards,
Gan Yi Fang

> -----Original Message-----
> From: Paolo Abeni <pabeni at redhat.com>
> Sent: Thursday, November 9, 2023 8:14 PM
> To: Russell King (Oracle) <linux at armlinux.org.uk>; Gan, Yi Fang
> <yi.fang.gan at intel.com>
> Cc: Alexandre Torgue <alexandre.torgue at foss.st.com>; Jose Abreu
> <joabreu at synopsys.com>; David S . Miller <davem at davemloft.net>; Eric
> Dumazet <edumazet at google.com>; Jakub Kicinski <kuba at kernel.org>;
> Maxime Coquelin <mcoquelin.stm32 at gmail.com>; Joakim Zhang
> <qiangqing.zhang at nxp.com>; netdev at vger.kernel.org; linux-stm32 at st-md-
> mailman.stormreply.com; linux-arm-kernel at lists.infradead.org; linux-
> kernel at vger.kernel.org; Looi, Hong Aun <hong.aun.looi at intel.com>; Voon,
> Weifeng <weifeng.voon at intel.com>; Song, Yoong Siang
> <yoong.siang.song at intel.com>
> Subject: Re: [PATCH net 1/1] net: stmmac: fix MAC and phylink mismatch issue
> after resume with STMMAC_FLAG_USE_PHY_WOL enabled
> 
> On Thu, 2023-11-09 at 09:15 +0000, Russell King (Oracle) wrote:
> > On Thu, Nov 09, 2023 at 01:00:27PM +0800, Gan Yi Fang wrote:
> > > From: "Gan, Yi Fang" <yi.fang.gan at intel.com>
> > >
> > > The issue happened when flag STMMAC_FLAG_USE_PHY_WOL is enabled.
> > > It can be reproduced with steps below:
> > > 1. Advertise only one speed on the host 2. Enable the WoL on the
> > > host 3. Suspend the host 4. Wake up the host
> > >
> > > When the WoL is disabled, both the PHY and MAC will suspend and wake
> > > up with everything configured well. When WoL is enabled, the PHY
> > > needs to be stay awake to receive the signal from remote client but
> > > MAC will enter suspend mode.
> > >
> > > When the MAC resumes from suspend, phylink_resume() will call
> > > phylink_start() to start the phylink instance which will trigger the
> > > phylink machine to invoke the mac_link_up callback function. The
> > > stmmac_mac_link_up() will configure the MAC_CTRL_REG based on the
> > > current link state. Then the stmmac_hw_setup() will be called to configure
> the MAC.
> > >
> > > This sequence might cause mismatch of the link state between MAC and
> > > phylink. This patch moves the phylink_resume() after
> > > stmamc_hw_setup() to ensure the MAC is initialized before phylink is being
> configured.
> >
> > Isn't this going to cause problems?
> >
> > stmamc_hw_setup() calls stmmac_init_dma_engine(), which then calls
> > stmmac_reset() - and stmmac_reset() can fail if the PHY clock isn't
> > running, which is why phylink_resume() gets called before this.
> 
> @Gan Yi Fang: at very least we need a solid explanation in the commit message
> why this change don't cause the above problems.
> 
> Thanks,
> 
> Paolo
>