Hi Joel, > -----Original Message----- > From: Joel Stanley [mailto:j...@jms.id.au] > Sent: Monday, October 19, 2020 4:57 PM > To: Dylan Hung <dylan_h...@aspeedtech.com>; Benjamin Herrenschmidt > <b...@kernel.crashing.org> > Cc: David S . Miller <da...@davemloft.net>; Jakub Kicinski > <k...@kernel.org>; netdev@vger.kernel.org; Linux Kernel Mailing List > <linux-ker...@vger.kernel.org>; Po-Yu Chuang <ratb...@faraday-tech.com>; > linux-aspeed <linux-asp...@lists.ozlabs.org>; OpenBMC Maillist > <open...@lists.ozlabs.org>; BMC-SW <bmc...@aspeedtech.com> > Subject: Re: [PATCH] net: ftgmac100: Fix missing TX-poll issue > > On Mon, 19 Oct 2020 at 07:39, Dylan Hung <dylan_h...@aspeedtech.com> > wrote: > > > > 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. > > > (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 > > > 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 <dylan_h...@aspeedtech.com> > > --- > > 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, > > * 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. > > > > > /* Update next TX pointer */ > > priv->tx_pointer = pointer; > > -- > > 2.17.1 > >
