Re: [PATCH net-next v5 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Mon, Apr 21, 2025 at 07:23:52PM +0000, Biju Das wrote:
> Hi Russell,
> 
> > -----Original Message-----
> > From: Biju Das
> > Sent: 21 April 2025 20:06
> > Subject: RE: [PATCH net-next v5 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH
> > 
> > Hi Russell,
> > 
> > > -----Original Message-----
> > > From: Russell King <linux@armlinux.org.uk>
> > > Sent: 21 April 2025 20:00
> > > Subject: Re: [PATCH net-next v5 3/3] net: stmmac: Add DWMAC glue layer
> > > for Renesas GBETH
> > >
> > > On Mon, Apr 21, 2025 at 01:45:50PM +0000, Biju Das wrote:
> > > > Hi All,
> > > > FYI, On RZ/G3E, for STR to work with mainline, we need to reinitialize the PHY.
> > >
> > > Which "mainline" are you using?
> > >
> > > Reading your emails, I suspect v6.14 rather than something post-dating
> > > v6.15-rc1, since your latest email suggests that the PHY driver's
> > > ->resume method is not being called early in stmmac's resume. However,
> > > commits 367f1854d442 and ef43e5132895 made this happen, which were
> > > merged during the merge window, and are thus in v6.15-rc1.
> > 
> > I am using Linux version 6.15.0-rc2-next-20250417 + renesas_defconfig with CONFIG_PROVE_LOCKING
> > enabled.
> 
> For me, it looks like issue related to timing, see[1] for details
> 
> [1] https://lore.kernel.org/all/TY3PR01MB1134690619EC6CADD07CD2DE186B82@TY3PR01MB11346.jpnprd01.prod.outlook.com/
> 
> Please let me know, if you have any patch that I can try out to fix the random timing issue.

That's the email that provoked me to reply this evening (I wouldn't have
because I'm still on vacation.)

So, this is how things are supposed to be working:
- stmmac_phy_setup() sets phylink_config.mac_managed_pm and
  phylink_config.mac_requires_rxc to be true. The former disables phylib
  based power management.

- You've hooked in stmmac_pltfr_pm_ops.
- On resume, this will call stmmac_pltfr_resume().
- stmmac_pltfr_resume() will call your ->init function followed by
  stmmac_resume().
- stmmac_resume() will call phylink_prepare_resume().
- phylink_prepare_resume() will call phy_resume() to resume the PHY
  if pl->config->mac_requires_rxc && phydev && phydev->suspended is
  true. The first and second will be true. The third... depends.

For phydev->suspended to be true, phy_suspend() needs to have been
called. Neither mdio_bus_phy_suspend() nor mdio_bus_phy_resume()
should be having any effect as phydev->mac_managed_pm should be
set (as a result of phylink_config.mac_managed_pm having been set.)

phy_suspend() also gets called from phy_detach() and
_phy_state_machine_post_work() when the work is PHY_STATE_WORK_SUSPEND.
This happens when we halt the PHY, which will happen if phy_stop() is
called.

phylink_suspend() will do this only when WoL is not active - calling
it when WoL is active will prevent WoL from working as the PHY needs
to stay awake to (1) detect WoL packets if it is programmed to do
so, or (2) pass packets to the MAC in the case where the MAC is doing
WoL.

So, phy_resume() should be getting called for the !WoL case, which will
result in the PHY driver's ->resume method being called - in your case
kszphy_resume().

This will occur synchronously, and after gbeth's ->init function has
been called, and as its all in the same thread of execution, it should
be 100% reliable.

For the WoL case, we assume that the PHY retains its settings since it
needs to remain powered up, and because it hasn't been suspended or
shutdown, it should be retaining all settings when the system wakes up.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!