Re: [RFC net-next] net: stmmac: should not modify RX descriptor when STMMAC resume

From: Florian Fainelli <f.fainelli@gmail.com>
To: Jon Hunter <jonathanh@nvidia.com>,
	Joakim Zhang <qiangqing.zhang@nxp.com>,
	"peppe.cavallaro@st.com" <peppe.cavallaro@st.com>,
	"alexandre.torgue@foss.st.com" <alexandre.torgue@foss.st.com>,
	"joabreu@synopsys.com" <joabreu@synopsys.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"mcoquelin.stm32@gmail.com" <mcoquelin.stm32@gmail.com>,
	"andrew@lunn.ch" <andrew@lunn.ch>
Cc: dl-linux-imx <linux-imx@nxp.com>,
	"treding@nvidia.com" <treding@nvidia.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [RFC net-next] net: stmmac: should not modify RX descriptor when STMMAC resume
Date: Thu, 22 Apr 2021 10:32:32 -0700	[thread overview]
Message-ID: <22bf351b-3f01-db62-8185-7a925f19998e@gmail.com> (raw)
In-Reply-To: <9abe58c4-a788-e07d-f281-847ee5b9fcf3@nvidia.com>

On 4/22/2021 10:00 AM, Jon Hunter wrote:
> 
> On 22/04/2021 17:12, Florian Fainelli wrote:
> 
> ...
> 
>> What does the resumption failure looks like? Does the stmmac driver
>> successfully resume from your suspend state, but there is no network
>> traffic? Do you have a log by any chance?
> 
> The board fails to resume and appears to hang. With regard to the
> original patch I did find that moving the code to re-init the RX buffers
> to before the PHY is enabled did work [0].

You indicated that you are using a Broadcom PHY, which specific PHY are
you using?

I suspect that the stmmac is somehow relying on the PHY to provide its
125MHz RXC clock back to you in order to have its RX logic work correctly.

One difference between using the Broadcom PHY and the Generic PHY
drivers could be whether your Broadcom PHY driver entry has a
.suspend/.resume callback implemented or not.

> 
>> Is power to the Ethernet MAC turned off in this suspend state, in which
>> case could we be missing an essential register programming stage?
> 
> It seems to be more of a sequencing issue rather than a power issue.
> 
> I have also ran 2000 suspend cycles on our Tegra platform without
> Joakim's patch to see how stable suspend is on this platform. I did not
> see any failures in 2000 cycles and so it is not evident to me that the
> problem that Joakim is trying to fix is seen on devices such as Tegra.
> Admittedly, if it is hard to reproduce, then it is possible we have not
> seen it yet.
> 
> Jon
> 
> [0]
> https://lore.kernel.org/netdev/e4864046-e52f-63b6-a490-74c3cd8045f4@nvidia.com/
> 

-- 
Florian