Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 9 Aug 2023 15:41:21 -0700
From:   Jakub Kicinski <kuba@kernel.org>
To:     <linux@armlinux.org.uk>, Andrew Lunn <andrew@lunn.ch>,
        Heiner Kallweit <hkallweit1@gmail.com>
Cc:     Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>,
        <nicolas.ferre@microchip.com>, <claudiu.beznea@microchip.com>,
        <davem@davemloft.net>, <edumazet@google.com>, <pabeni@redhat.com>,
        <robert.hancock@calian.com>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <git@amd.com>
Subject: Re: [PATCH net] net: macb: In ZynqMP resume always configure PS GTR
 for non-wakeup source
Message-ID: <20230809154121.673ec04b@kernel.org>
In-Reply-To: <1691414091-2260697-1-git-send-email-radhey.shyam.pandey@amd.com>
References: <1691414091-2260697-1-git-send-email-radhey.shyam.pandey@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

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();