Messages in this thread | | | Date | Wed, 9 Aug 2023 15:41:21 -0700 | From | Jakub Kicinski <> | Subject | Re: [PATCH net] net: macb: In ZynqMP resume always configure PS GTR for non-wakeup source |
| |
On Mon, 7 Aug 2023 18:44:51 +0530 Radhey Shyam Pandey wrote: > On Zynq UltraScale+ MPSoC ubuntu platform when systemctl issues suspend, > network manager bring down the interface and goes into suspend. When it > wakes up it again enables the interface. > > This leads to xilinx-psgtr "PLL lock timeout" on interface bringup, as > the power management controller power down the entire FPD (including > SERDES) if none of the FPD devices are in use and serdes is not > initialized on resume. > > $ sudo rtcwake -m no -s 120 -v > $ sudo systemctl suspend <this does ifconfig eth1 down> > $ ifconfig eth1 up > xilinx-psgtr fd400000.phy: lane 0 (type 10, protocol 5): PLL lock timeout > phy phy-fd400000.phy.0: phy poweron failed --> -110 > > macb driver is called in this way: > 1. macb_close: Stop network interface. In this function, it > reset MACB IP and disables PHY and network interface. > > 2. macb_suspend: It is called in kernel suspend flow. But because > network interface has been disabled(netif_running(ndev) is > false), it does nothing and returns directly; > > 3. System goes into suspend state. Some time later, system is > waken up by RTC wakeup device; > > 4. macb_resume: It does nothing because network interface has > been disabled; > > 5. macb_open: It is called to enable network interface again. ethernet > interface is initialized in this API but serdes which is power-off > by PMUFW during FPD-off suspend is not initialized again and so > we hit GT PLL lock issue on open. > > To resolve this PLL timeout issue always do PS GTR initialization > when ethernet device is configured as non-wakeup source. > > Fixes: f22bd29ba19a ("net: macb: Fix ZynqMP SGMII non-wakeup source resume failure") > Fixes: 8b73fa3ae02b ("net: macb: Added ZynqMP-specific initialization") > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Could be more of a PHY than MAC question. Adding remaining PHY maintainers to CC.
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c > index f6a0f12a6d52..82929ee76739 100644 > --- a/drivers/net/ethernet/cadence/macb_main.c > +++ b/drivers/net/ethernet/cadence/macb_main.c > @@ -5194,6 +5194,9 @@ static int __maybe_unused macb_suspend(struct device *dev) > unsigned int q; > int err; > > + if (!device_may_wakeup(&bp->dev->dev)) > + phy_exit(bp->sgmii_phy); > + > if (!netif_running(netdev)) > return 0; > > @@ -5254,7 +5257,6 @@ static int __maybe_unused macb_suspend(struct device *dev) > if (!(bp->wol & MACB_WOL_ENABLED)) { > rtnl_lock(); > phylink_stop(bp->phylink); > - phy_exit(bp->sgmii_phy); > rtnl_unlock(); > spin_lock_irqsave(&bp->lock, flags); > macb_reset_hw(bp); > @@ -5284,6 +5286,9 @@ static int __maybe_unused macb_resume(struct device *dev) > unsigned int q; > int err; > > + if (!device_may_wakeup(&bp->dev->dev)) > + phy_init(bp->sgmii_phy); > + > if (!netif_running(netdev)) > return 0; > > @@ -5344,8 +5349,6 @@ static int __maybe_unused macb_resume(struct device *dev) > macb_set_rx_mode(netdev); > macb_restore_features(bp); > rtnl_lock(); > - if (!device_may_wakeup(&bp->dev->dev)) > - phy_init(bp->sgmii_phy); > > phylink_start(bp->phylink); > rtnl_unlock();
| |