Hi Joel,

-----Original Message-----
From: Joel Stanley [mailto:joel@jms.id.au]
Sent: Monday, October 19, 2020 4:57 PM
To: Dylan Hung <redacted>; Benjamin Herrenschmidt
[off-list ref]
Cc: David S . Miller <davem@davemloft.net>; Jakub Kicinski
[off-list ref]; netdev@vger.kernel.org; Linux Kernel Mailing List
[off-list ref]; Po-Yu Chuang [off-list ref];
linux-aspeed [off-list ref]; OpenBMC Maillist
[off-list ref]; BMC-SW [off-list ref]
Subject: Re: [PATCH] net: ftgmac100: Fix missing TX-poll issue

On Mon, 19 Oct 2020 at 07:39, Dylan Hung [off-list ref]
wrote:

quoted

The cpu accesses the register and the memory via different bus/path on
aspeed soc.  So we can not guarantee that the tx-poll command

Just the 2600, or other versions too?

Just the 2600.  And this issue only occurred on Ethernet mac.

quoted

(register access) is always behind the tx descriptor (memory).  In
other words, the HW may start working even the data is not yet ready.
By

even if the

quoted

adding a dummy read after the last data write, we can ensure the data
are pushed to the memory, then guarantee the processing sequence

Signed-off-by: Dylan Hung <redacted>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c

b/drivers/net/ethernet/faraday/ftgmac100.c
index 00024dd41147..9a99a87f29f3 100644

--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c

@@ -804,7 +804,8 @@ static netdev_tx_t

ftgmac100_hard_start_xmit(struct sk_buff *skb,

quoted

         * before setting the OWN bit on the first descriptor.
         */
        dma_wmb();
-       first->txdes0 = cpu_to_le32(f_ctl_stat);
+       WRITE_ONCE(first->txdes0, cpu_to_le32(f_ctl_stat));
+       READ_ONCE(first->txdes0);

I understand what you're trying to do here, but I'm not sure that this is the
correct way to go about it.

It does cause the compiler to produce a store and then a load.

quoted

        /* Update next TX pointer */
        priv->tx_pointer = pointer;
--
2.17.1