RE: [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@xxxxxxxxxx>
> Sent: Thursday, November 9, 2023 8:14 PM
> To: Russell King (Oracle) <linux@xxxxxxxxxxxxxxx>; Gan, Yi Fang
> <yi.fang.gan@xxxxxxxxx>
> Cc: Alexandre Torgue <alexandre.torgue@xxxxxxxxxxx>; Jose Abreu
> <joabreu@xxxxxxxxxxxx>; David S . Miller <davem@xxxxxxxxxxxxx>; Eric
> Dumazet <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>;
> Maxime Coquelin <mcoquelin.stm32@xxxxxxxxx>; Joakim Zhang
> <qiangqing.zhang@xxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux-stm32@st-md-
> mailman.stormreply.com; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Looi, Hong Aun <hong.aun.looi@xxxxxxxxx>; Voon,
> Weifeng <weifeng.voon@xxxxxxxxx>; Song, Yoong Siang
> <yoong.siang.song@xxxxxxxxx>
> 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@xxxxxxxxx>
> > >
> > > 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
>