Hi Joakim,
On 19/04/2021 12:59, Joakim Zhang wrote:
> When system resume back, STMMAC will clear RX descriptors:
> stmmac_resume()
> ->stmmac_clear_descriptors()
> ->stmmac_clear_rx_descriptors()
> ->stmmac_init_rx_desc()
> ->dwmac4_set_rx_owner()
> //p->des3 |= cpu_to_le32(RDES3_OWN |
> RDES3_BUFFER1_VALID_ADDR);
> It only assets OWN and BUF1V bits in desc3 field, doesn't clear desc0/1/2
> fields.
>
> Let's take a case into account, when system suspend, it is possible that
> there are packets have not received yet, so the RX descriptors are wrote
> back by DMA, e.g.
> 008 [0x00000000c4310080]: 0x0 0x40 0x0 0x34010040
>
> When system resume back, after above process, it became a broken
> descriptor:
> 008 [0x00000000c4310080]: 0x0 0x40 0x0 0xb5010040
>
> The issue is that it only changes the owner of this descriptor, but do nothing
> about desc0/1/2 fields. The descriptor of STMMAC a bit special, applicaton
> prepares RX descriptors for DMA, after DMA recevie the packets, it will write
> back the descriptors, so the same field of a descriptor have different
> meanings to application and DMA. It should be a software bug there, and may
> not easy to reproduce, but there is a certain probability that it will
> occur.
>
> Commit 9c63faaa931e ("net: stmmac: re-init rx buffers when mac resume back")
> tried
> to re-init desc0/desc1 (buffer address fields) to fix this issue, but it
> is not a proper solution, and made regression on Jetson TX2 boards.
>
> It is unreasonable to modify RX descriptors outside of stmmac_rx_refill()
> function,
> where it will clear all desc0/desc1/desc2/desc3 fields together.
>
> This patch removes RX descriptors modification when STMMAC resume.
>
> Signed-off-by: Joakim Zhang <qiangqing.zh...@nxp.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 9f396648d76f..b784304a22e8 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -7186,6 +7186,8 @@ static void stmmac_reset_queues_param(struct
> stmmac_priv *priv)
> tx_q->mss = 0;
>
> netdev_tx_reset_queue(netdev_get_tx_queue(priv->dev, queue));
> +
> + stmmac_clear_tx_descriptors(priv, queue);
> }
> }
>
> @@ -7250,7 +7252,6 @@ int stmmac_resume(struct device *dev)
> stmmac_reset_queues_param(priv);
>
> stmmac_free_tx_skbufs(priv);
> - stmmac_clear_descriptors(priv);
>
> stmmac_hw_setup(ndev, false);
> stmmac_init_coalesce(priv);
>
I have tested this patch, but unfortunately the board still fails to
resume correctly. So it appears to suffer with the same issue we saw on
the previous implementation.
Jon
--
nvpublic
