* [PATCH 0/4] fix ftgmac100 issues on aspeed soc
@ 2020-10-19 8:57 Dylan Hung
2020-10-19 8:57 ` [PATCH 1/4] ftgmac100: Fix race issue on TX descriptor[0] Dylan Hung
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Dylan Hung @ 2020-10-19 8:57 UTC (permalink / raw)
To: davem, kuba, netdev, linux-kernel, ratbert, linux-aspeed, openbmc; +Cc: BMC-SW
This patch series fixes the ftgmac100 mac hw issues on aspeed soc.
Fixes: 52c0cae ("ftgmac100: Remove tx descriptor accessors")
Fixes: 39bfab8 ("net: ftgmac100: Add support for DT phy-handle property")
Fixes: 10cbd64 ("ftgmac100: Rework NAPI & interrupts handling")
Dylan Hung (4):
ftgmac100: Fix race issue on TX descriptor[0]
ftgmac100: Fix missing-poll issue
ftgmac100: Add a dummy read to ensure running sequence
ftgmac100: Restart MAC HW once
drivers/net/ethernet/faraday/ftgmac100.c | 53 ++++++++++++++----------
1 file changed, 32 insertions(+), 21 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/4] ftgmac100: Fix race issue on TX descriptor[0] 2020-10-19 8:57 [PATCH 0/4] fix ftgmac100 issues on aspeed soc Dylan Hung @ 2020-10-19 8:57 ` Dylan Hung 2020-10-19 23:19 ` Benjamin Herrenschmidt 2020-10-19 8:57 ` [PATCH 2/4] ftgmac100: Fix missing-poll issue Dylan Hung ` (2 subsequent siblings) 3 siblings, 1 reply; 14+ messages in thread From: Dylan Hung @ 2020-10-19 8:57 UTC (permalink / raw) To: davem, kuba, netdev, linux-kernel, ratbert, linux-aspeed, openbmc; +Cc: BMC-SW These rules must be followed when accessing the TX descriptor: 1. A TX descriptor is "cleanable" only when its value is non-zero and the owner bit is set to "software" 2. A TX descriptor is "writable" only when its value is zero regardless the edotr mask. Fixes: 52c0cae87465 ("ftgmac100: Remove tx descriptor accessors") Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com> Signed-off-by: Joel Stanley <joel@jms.id.au> --- drivers/net/ethernet/faraday/ftgmac100.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c index 00024dd41147..7cacbe4aecb7 100644 --- a/drivers/net/ethernet/faraday/ftgmac100.c +++ b/drivers/net/ethernet/faraday/ftgmac100.c @@ -647,6 +647,9 @@ static bool ftgmac100_tx_complete_packet(struct ftgmac100 *priv) if (ctl_stat & FTGMAC100_TXDES0_TXDMA_OWN) return false; + if ((ctl_stat & ~(priv->txdes0_edotr_mask)) == 0) + return false; + skb = priv->tx_skbs[pointer]; netdev->stats.tx_packets++; netdev->stats.tx_bytes += skb->len; @@ -756,6 +759,9 @@ static netdev_tx_t ftgmac100_hard_start_xmit(struct sk_buff *skb, pointer = priv->tx_pointer; txdes = first = &priv->txdes[pointer]; + if (le32_to_cpu(txdes->txdes0) & ~priv->txdes0_edotr_mask) + goto drop; + /* Setup it up with the packet head. Don't write the head to the * ring just yet */ @@ -787,6 +793,10 @@ static netdev_tx_t ftgmac100_hard_start_xmit(struct sk_buff *skb, /* Setup descriptor */ priv->tx_skbs[pointer] = skb; txdes = &priv->txdes[pointer]; + + if (le32_to_cpu(txdes->txdes0) & ~priv->txdes0_edotr_mask) + goto dma_err; + ctl_stat = ftgmac100_base_tx_ctlstat(priv, pointer); ctl_stat |= FTGMAC100_TXDES0_TXDMA_OWN; ctl_stat |= FTGMAC100_TXDES0_TXBUF_SIZE(len); -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] ftgmac100: Fix race issue on TX descriptor[0] 2020-10-19 8:57 ` [PATCH 1/4] ftgmac100: Fix race issue on TX descriptor[0] Dylan Hung @ 2020-10-19 23:19 ` Benjamin Herrenschmidt 2020-10-20 4:13 ` Joel Stanley 0 siblings, 1 reply; 14+ messages in thread From: Benjamin Herrenschmidt @ 2020-10-19 23:19 UTC (permalink / raw) To: Dylan Hung, davem, kuba, netdev, linux-kernel, ratbert, linux-aspeed, openbmc Cc: BMC-SW On Mon, 2020-10-19 at 16:57 +0800, Dylan Hung wrote: > These rules must be followed when accessing the TX descriptor: > > 1. A TX descriptor is "cleanable" only when its value is non-zero > and the owner bit is set to "software" Can you elaborate ? What is the point of that change ? The owner bit should be sufficient, why do we need to check other fields ? > 2. A TX descriptor is "writable" only when its value is zero > regardless the edotr mask. Again, why is that ? Can you elaborate ? What race are you trying to address here ? Cheers, Ben. > Fixes: 52c0cae87465 ("ftgmac100: Remove tx descriptor accessors") > Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com> > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > drivers/net/ethernet/faraday/ftgmac100.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c > b/drivers/net/ethernet/faraday/ftgmac100.c > index 00024dd41147..7cacbe4aecb7 100644 > --- a/drivers/net/ethernet/faraday/ftgmac100.c > +++ b/drivers/net/ethernet/faraday/ftgmac100.c > @@ -647,6 +647,9 @@ static bool ftgmac100_tx_complete_packet(struct > ftgmac100 *priv) > if (ctl_stat & FTGMAC100_TXDES0_TXDMA_OWN) > return false; > > + if ((ctl_stat & ~(priv->txdes0_edotr_mask)) == 0) > + return false; > + > skb = priv->tx_skbs[pointer]; > netdev->stats.tx_packets++; > netdev->stats.tx_bytes += skb->len; > @@ -756,6 +759,9 @@ static netdev_tx_t > ftgmac100_hard_start_xmit(struct sk_buff *skb, > pointer = priv->tx_pointer; > txdes = first = &priv->txdes[pointer]; > > + if (le32_to_cpu(txdes->txdes0) & ~priv->txdes0_edotr_mask) > + goto drop; > + > /* Setup it up with the packet head. Don't write the head to > the > * ring just yet > */ > @@ -787,6 +793,10 @@ static netdev_tx_t > ftgmac100_hard_start_xmit(struct sk_buff *skb, > /* Setup descriptor */ > priv->tx_skbs[pointer] = skb; > txdes = &priv->txdes[pointer]; > + > + if (le32_to_cpu(txdes->txdes0) & ~priv- > >txdes0_edotr_mask) > + goto dma_err; > + > ctl_stat = ftgmac100_base_tx_ctlstat(priv, pointer); > ctl_stat |= FTGMAC100_TXDES0_TXDMA_OWN; > ctl_stat |= FTGMAC100_TXDES0_TXBUF_SIZE(len); ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] ftgmac100: Fix race issue on TX descriptor[0] 2020-10-19 23:19 ` Benjamin Herrenschmidt @ 2020-10-20 4:13 ` Joel Stanley 2020-10-20 6:23 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 14+ messages in thread From: Joel Stanley @ 2020-10-20 4:13 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: BMC-SW, linux-aspeed, Po-Yu Chuang, netdev, OpenBMC Maillist, Linux Kernel Mailing List, Jakub Kicinski, Dylan Hung, David S . Miller On Mon, 19 Oct 2020 at 23:20, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > > On Mon, 2020-10-19 at 16:57 +0800, Dylan Hung wrote: > > These rules must be followed when accessing the TX descriptor: > > > > 1. A TX descriptor is "cleanable" only when its value is non-zero > > and the owner bit is set to "software" > > Can you elaborate ? What is the point of that change ? The owner bit > should be sufficient, why do we need to check other fields ? I would like Dylan to clarify too. The datasheet has a footnote below the descriptor layout: - TXDES#0: Bits 27 ~ 14 are valid only when FTS = 1 - TXDES#1: Bits 31 ~ 0 are valid only when FTS = 1 So the ownership bit (31) is not valid unless FTS is set. However, this isn't what his patch does. It adds checks for EDOTR. > > > 2. A TX descriptor is "writable" only when its value is zero > > regardless the edotr mask. > > Again, why is that ? Can you elaborate ? What race are you trying to > address here ? > > Cheers, > Ben. > > > Fixes: 52c0cae87465 ("ftgmac100: Remove tx descriptor accessors") > > Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com> > > Signed-off-by: Joel Stanley <joel@jms.id.au> > > --- > > drivers/net/ethernet/faraday/ftgmac100.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c > > b/drivers/net/ethernet/faraday/ftgmac100.c > > index 00024dd41147..7cacbe4aecb7 100644 > > --- a/drivers/net/ethernet/faraday/ftgmac100.c > > +++ b/drivers/net/ethernet/faraday/ftgmac100.c > > @@ -647,6 +647,9 @@ static bool ftgmac100_tx_complete_packet(struct > > ftgmac100 *priv) > > if (ctl_stat & FTGMAC100_TXDES0_TXDMA_OWN) > > return false; > > > > + if ((ctl_stat & ~(priv->txdes0_edotr_mask)) == 0) > > + return false; > > + > > skb = priv->tx_skbs[pointer]; > > netdev->stats.tx_packets++; > > netdev->stats.tx_bytes += skb->len; > > @@ -756,6 +759,9 @@ static netdev_tx_t > > ftgmac100_hard_start_xmit(struct sk_buff *skb, > > pointer = priv->tx_pointer; > > txdes = first = &priv->txdes[pointer]; > > > > + if (le32_to_cpu(txdes->txdes0) & ~priv->txdes0_edotr_mask) > > + goto drop; > > + > > /* Setup it up with the packet head. Don't write the head to > > the > > * ring just yet > > */ > > @@ -787,6 +793,10 @@ static netdev_tx_t > > ftgmac100_hard_start_xmit(struct sk_buff *skb, > > /* Setup descriptor */ > > priv->tx_skbs[pointer] = skb; > > txdes = &priv->txdes[pointer]; > > + > > + if (le32_to_cpu(txdes->txdes0) & ~priv- > > >txdes0_edotr_mask) > > + goto dma_err; > > + > > ctl_stat = ftgmac100_base_tx_ctlstat(priv, pointer); > > ctl_stat |= FTGMAC100_TXDES0_TXDMA_OWN; > > ctl_stat |= FTGMAC100_TXDES0_TXBUF_SIZE(len); > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] ftgmac100: Fix race issue on TX descriptor[0] 2020-10-20 4:13 ` Joel Stanley @ 2020-10-20 6:23 ` Benjamin Herrenschmidt 2020-10-20 7:13 ` Joel Stanley 0 siblings, 1 reply; 14+ messages in thread From: Benjamin Herrenschmidt @ 2020-10-20 6:23 UTC (permalink / raw) To: Joel Stanley Cc: BMC-SW, linux-aspeed, Po-Yu Chuang, netdev, OpenBMC Maillist, Linux Kernel Mailing List, Jakub Kicinski, Dylan Hung, David S . Miller On Tue, 2020-10-20 at 04:13 +0000, Joel Stanley wrote: > On Mon, 19 Oct 2020 at 23:20, Benjamin Herrenschmidt > <benh@kernel.crashing.org> wrote: > > > > On Mon, 2020-10-19 at 16:57 +0800, Dylan Hung wrote: > > > These rules must be followed when accessing the TX descriptor: > > > > > > 1. A TX descriptor is "cleanable" only when its value is non-zero > > > and the owner bit is set to "software" > > > > Can you elaborate ? What is the point of that change ? The owner > > bit > > should be sufficient, why do we need to check other fields ? > > I would like Dylan to clarify too. The datasheet has a footnote below > the descriptor layout: > > - TXDES#0: Bits 27 ~ 14 are valid only when FTS = 1 > - TXDES#1: Bits 31 ~ 0 are valid only when FTS = 1 > > So the ownership bit (31) is not valid unless FTS is set. However, > this isn't what his patch does. It adds checks for EDOTR. No I think it adds a check for everything except EDOTR which just marks the end of ring and needs to be ignored in the comparison. That said, we do need a better explanation. One potential bug I did find by looking at my code however is: static bool ftgmac100_tx_complete_packet(struct ftgmac100 *priv) { struct net_device *netdev = priv->netdev; struct ftgmac100_txdes *txdes; struct sk_buff *skb; unsigned int pointer; u32 ctl_stat; pointer = priv->tx_clean_pointer; txdes = &priv->txdes[pointer]; ctl_stat = le32_to_cpu(txdes->txdes0); if (ctl_stat & FTGMAC100_TXDES0_TXDMA_OWN) return false; skb = priv->tx_skbs[pointer]; netdev->stats.tx_packets++; netdev->stats.tx_bytes += skb->len; ftgmac100_free_tx_packet(priv, pointer, skb, txdes, ctl_stat); txdes->txdes0 = cpu_to_le32(ctl_stat & priv->txdes0_edotr_mask); ^^^^ There should probably be an smp_wmb() here to ensure that all the above stores are visible before the tx clean pointer is updated. priv->tx_clean_pointer = ftgmac100_next_tx_pointer(priv, pointer); return true; } Similarly we probablu should have one before setting tx_pointer in start_xmit(). As for the read side of this, I'm not 100% sure, I'll have to think more about it, it *think* the existing barriers are sufficient at first sight. Cheers, Ben. > > > > > 2. A TX descriptor is "writable" only when its value is zero > > > regardless the edotr mask. > > > > Again, why is that ? Can you elaborate ? What race are you trying > > to > > address here ? > > > > Cheers, > > Ben. > > > > > Fixes: 52c0cae87465 ("ftgmac100: Remove tx descriptor accessors") > > > Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com> > > > Signed-off-by: Joel Stanley <joel@jms.id.au> > > > --- > > > drivers/net/ethernet/faraday/ftgmac100.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c > > > b/drivers/net/ethernet/faraday/ftgmac100.c > > > index 00024dd41147..7cacbe4aecb7 100644 > > > --- a/drivers/net/ethernet/faraday/ftgmac100.c > > > +++ b/drivers/net/ethernet/faraday/ftgmac100.c > > > @@ -647,6 +647,9 @@ static bool > > > ftgmac100_tx_complete_packet(struct > > > ftgmac100 *priv) > > > if (ctl_stat & FTGMAC100_TXDES0_TXDMA_OWN) > > > return false; > > > > > > + if ((ctl_stat & ~(priv->txdes0_edotr_mask)) == 0) > > > + return false; > > > + > > > skb = priv->tx_skbs[pointer]; > > > netdev->stats.tx_packets++; > > > netdev->stats.tx_bytes += skb->len; > > > @@ -756,6 +759,9 @@ static netdev_tx_t > > > ftgmac100_hard_start_xmit(struct sk_buff *skb, > > > pointer = priv->tx_pointer; > > > txdes = first = &priv->txdes[pointer]; > > > > > > + if (le32_to_cpu(txdes->txdes0) & ~priv->txdes0_edotr_mask) > > > + goto drop; > > > + > > > /* Setup it up with the packet head. Don't write the head > > > to > > > the > > > * ring just yet > > > */ > > > @@ -787,6 +793,10 @@ static netdev_tx_t > > > ftgmac100_hard_start_xmit(struct sk_buff *skb, > > > /* Setup descriptor */ > > > priv->tx_skbs[pointer] = skb; > > > txdes = &priv->txdes[pointer]; > > > + > > > + if (le32_to_cpu(txdes->txdes0) & ~priv- > > > > txdes0_edotr_mask) > > > > > > + goto dma_err; > > > + > > > ctl_stat = ftgmac100_base_tx_ctlstat(priv, > > > pointer); > > > ctl_stat |= FTGMAC100_TXDES0_TXDMA_OWN; > > > ctl_stat |= FTGMAC100_TXDES0_TXBUF_SIZE(len); ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] ftgmac100: Fix race issue on TX descriptor[0] 2020-10-20 6:23 ` Benjamin Herrenschmidt @ 2020-10-20 7:13 ` Joel Stanley 0 siblings, 0 replies; 14+ messages in thread From: Joel Stanley @ 2020-10-20 7:13 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: BMC-SW, linux-aspeed, Po-Yu Chuang, netdev, OpenBMC Maillist, Linux Kernel Mailing List, Jakub Kicinski, Dylan Hung, David S . Miller On Tue, 20 Oct 2020 at 06:23, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > > On Tue, 2020-10-20 at 04:13 +0000, Joel Stanley wrote: > > On Mon, 19 Oct 2020 at 23:20, Benjamin Herrenschmidt > > <benh@kernel.crashing.org> wrote: > > > > > > On Mon, 2020-10-19 at 16:57 +0800, Dylan Hung wrote: > > > > These rules must be followed when accessing the TX descriptor: > > > > > > > > 1. A TX descriptor is "cleanable" only when its value is non-zero > > > > and the owner bit is set to "software" > > > > > > Can you elaborate ? What is the point of that change ? The owner > > > bit > > > should be sufficient, why do we need to check other fields ? > > > > I would like Dylan to clarify too. The datasheet has a footnote below > > the descriptor layout: > > > > - TXDES#0: Bits 27 ~ 14 are valid only when FTS = 1 > > - TXDES#1: Bits 31 ~ 0 are valid only when FTS = 1 > > > > So the ownership bit (31) is not valid unless FTS is set. However, > > this isn't what his patch does. It adds checks for EDOTR. > > No I think it adds a check for everything except EDOTR which just marks > the end of ring and needs to be ignored in the comparison. Of course. I missed the invert. I did some testing with just this patch (and "[4/4] ftgmac100: Restart MAC HW once") from Dylan. It seemed to resolve the hang, but there were occasional retries. Putting in some tracing I only hit the condition in ftgmac100_tx_complete_packet, never in ftgmac100_hard_start_xmit. > That said, we do need a better explanation. > > One potential bug I did find by looking at my code however is: > > static bool ftgmac100_tx_complete_packet(struct ftgmac100 *priv) > { > struct net_device *netdev = priv->netdev; > struct ftgmac100_txdes *txdes; > struct sk_buff *skb; > unsigned int pointer; > u32 ctl_stat; > > pointer = priv->tx_clean_pointer; > txdes = &priv->txdes[pointer]; > > ctl_stat = le32_to_cpu(txdes->txdes0); > if (ctl_stat & FTGMAC100_TXDES0_TXDMA_OWN) > return false; > > skb = priv->tx_skbs[pointer]; > netdev->stats.tx_packets++; > netdev->stats.tx_bytes += skb->len; > ftgmac100_free_tx_packet(priv, pointer, skb, txdes, ctl_stat); > txdes->txdes0 = cpu_to_le32(ctl_stat & priv->txdes0_edotr_mask); > > ^^^^ There should probably be an smp_wmb() here to ensure that all the above > stores are visible before the tx clean pointer is updated. > > priv->tx_clean_pointer = ftgmac100_next_tx_pointer(priv, pointer); > > return true; > } > > Similarly we probablu should have one before setting tx_pointer in start_xmit(). I added the two smp_wmb you suggested (with only 4/4 applied). This did the trick; iperf on a gigabit link is running well with no retries. diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c index 331d4bdd4a67..15cdfeb135b0 100644 --- a/drivers/net/ethernet/faraday/ftgmac100.c +++ b/drivers/net/ethernet/faraday/ftgmac100.c @@ -653,6 +653,11 @@ static bool ftgmac100_tx_complete_packet(struct ftgmac100 *priv) ftgmac100_free_tx_packet(priv, pointer, skb, txdes, ctl_stat); txdes->txdes0 = cpu_to_le32(ctl_stat & priv->txdes0_edotr_mask); + /* Ensure the descriptor config is visible before setting the tx + * pointer. + */ + smp_wmb(); + priv->tx_clean_pointer = ftgmac100_next_tx_pointer(priv, pointer); return true; @@ -806,6 +811,11 @@ static netdev_tx_t ftgmac100_hard_start_xmit(struct sk_buff *skb, dma_wmb(); first->txdes0 = cpu_to_le32(f_ctl_stat); + /* Ensure the descriptor config is visible before setting the tx + * pointer. + */ + smp_wmb(); + /* Update next TX pointer */ priv->tx_pointer = pointer; I left the test running while writing this email and I did start to see some retries. I'm not sure if that's because my laptop is one of the test machines, or if we have another issue. I will do some further testing over night. Cheers, Joel > > As for the read side of this, I'm not 100% sure, I'll have to think more about > it, it *think* the existing barriers are sufficient at first sight. > > Cheers, > Ben. > > > > > > > > 2. A TX descriptor is "writable" only when its value is zero > > > > regardless the edotr mask. > > > > > > Again, why is that ? Can you elaborate ? What race are you trying > > > to > > > address here ? > > > > > > Cheers, > > > Ben. > > > > > > > Fixes: 52c0cae87465 ("ftgmac100: Remove tx descriptor accessors") > > > > Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com> > > > > Signed-off-by: Joel Stanley <joel@jms.id.au> > > > > --- > > > > drivers/net/ethernet/faraday/ftgmac100.c | 10 ++++++++++ > > > > 1 file changed, 10 insertions(+) > > > > > > > > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c > > > > b/drivers/net/ethernet/faraday/ftgmac100.c > > > > index 00024dd41147..7cacbe4aecb7 100644 > > > > --- a/drivers/net/ethernet/faraday/ftgmac100.c > > > > +++ b/drivers/net/ethernet/faraday/ftgmac100.c > > > > @@ -647,6 +647,9 @@ static bool > > > > ftgmac100_tx_complete_packet(struct > > > > ftgmac100 *priv) > > > > if (ctl_stat & FTGMAC100_TXDES0_TXDMA_OWN) > > > > return false; > > > > > > > > + if ((ctl_stat & ~(priv->txdes0_edotr_mask)) == 0) > > > > + return false; > > > > + > > > > skb = priv->tx_skbs[pointer]; > > > > netdev->stats.tx_packets++; > > > > netdev->stats.tx_bytes += skb->len; > > > > @@ -756,6 +759,9 @@ static netdev_tx_t > > > > ftgmac100_hard_start_xmit(struct sk_buff *skb, > > > > pointer = priv->tx_pointer; > > > > txdes = first = &priv->txdes[pointer]; > > > > > > > > + if (le32_to_cpu(txdes->txdes0) & ~priv->txdes0_edotr_mask) > > > > + goto drop; > > > > + > > > > /* Setup it up with the packet head. Don't write the head > > > > to > > > > the > > > > * ring just yet > > > > */ > > > > @@ -787,6 +793,10 @@ static netdev_tx_t > > > > ftgmac100_hard_start_xmit(struct sk_buff *skb, > > > > /* Setup descriptor */ > > > > priv->tx_skbs[pointer] = skb; > > > > txdes = &priv->txdes[pointer]; > > > > + > > > > + if (le32_to_cpu(txdes->txdes0) & ~priv- > > > > > txdes0_edotr_mask) > > > > > > > > + goto dma_err; > > > > + > > > > ctl_stat = ftgmac100_base_tx_ctlstat(priv, > > > > pointer); > > > > ctl_stat |= FTGMAC100_TXDES0_TXDMA_OWN; > > > > ctl_stat |= FTGMAC100_TXDES0_TXBUF_SIZE(len); > ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/4] ftgmac100: Fix missing-poll issue 2020-10-19 8:57 [PATCH 0/4] fix ftgmac100 issues on aspeed soc Dylan Hung 2020-10-19 8:57 ` [PATCH 1/4] ftgmac100: Fix race issue on TX descriptor[0] Dylan Hung @ 2020-10-19 8:57 ` Dylan Hung 2020-10-19 8:57 ` [PATCH 3/4] ftgmac100: Add a dummy read to ensure running sequence Dylan Hung 2020-10-19 8:57 ` [PATCH 4/4] ftgmac100: Restart MAC HW once Dylan Hung 3 siblings, 0 replies; 14+ messages in thread From: Dylan Hung @ 2020-10-19 8:57 UTC (permalink / raw) To: davem, kuba, netdev, linux-kernel, ratbert, linux-aspeed, openbmc; +Cc: BMC-SW the tx-poll command may advance the tx descriptor due the HW design. By adding a pseudo read and proper memory barrier, we can ensure all the data are ready before TX poll command. Fixes: 52c0cae87465 ("ftgmac100: Remove tx descriptor accessors") Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com> Signed-off-by: Joel Stanley <joel@jms.id.au> --- drivers/net/ethernet/faraday/ftgmac100.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c index 7cacbe4aecb7..810bda80f138 100644 --- a/drivers/net/ethernet/faraday/ftgmac100.c +++ b/drivers/net/ethernet/faraday/ftgmac100.c @@ -814,8 +814,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); /* Update next TX pointer */ priv->tx_pointer = pointer; -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/4] ftgmac100: Add a dummy read to ensure running sequence 2020-10-19 8:57 [PATCH 0/4] fix ftgmac100 issues on aspeed soc Dylan Hung 2020-10-19 8:57 ` [PATCH 1/4] ftgmac100: Fix race issue on TX descriptor[0] Dylan Hung 2020-10-19 8:57 ` [PATCH 2/4] ftgmac100: Fix missing-poll issue Dylan Hung @ 2020-10-19 8:57 ` Dylan Hung 2020-10-19 23:25 ` Benjamin Herrenschmidt 2020-10-19 8:57 ` [PATCH 4/4] ftgmac100: Restart MAC HW once Dylan Hung 3 siblings, 1 reply; 14+ messages in thread From: Dylan Hung @ 2020-10-19 8:57 UTC (permalink / raw) To: davem, kuba, netdev, linux-kernel, ratbert, linux-aspeed, openbmc; +Cc: BMC-SW On the AST2600 care must be taken to ensure writes appear correctly when modifying the interrupt reglated registers. Add a function to perform a read after all writes to the IER and ISR registers. Fixes: 39bfab8844a0 ("net: ftgmac100: Add support for DT phy-handle property") Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com> Signed-off-by: Joel Stanley <joel@jms.id.au> --- drivers/net/ethernet/faraday/ftgmac100.c | 38 ++++++++++++------------ 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c index 810bda80f138..0c67fc3e27df 100644 --- a/drivers/net/ethernet/faraday/ftgmac100.c +++ b/drivers/net/ethernet/faraday/ftgmac100.c @@ -111,6 +111,14 @@ struct ftgmac100 { bool is_aspeed; }; +/* Helper to ensure writes are observed with the correct ordering. Use only + * for IER and ISR accesses. */ +static void ftgmac100_write(u32 val, void __iomem *addr) +{ + iowrite32(val, addr); + ioread32(addr); +} + static int ftgmac100_reset_mac(struct ftgmac100 *priv, u32 maccr) { struct net_device *netdev = priv->netdev; @@ -1048,7 +1056,7 @@ static void ftgmac100_adjust_link(struct net_device *netdev) return; /* Disable all interrupts */ - iowrite32(0, priv->base + FTGMAC100_OFFSET_IER); + ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER); /* Reset the adapter asynchronously */ schedule_work(&priv->reset_task); @@ -1246,7 +1254,7 @@ static irqreturn_t ftgmac100_interrupt(int irq, void *dev_id) /* Fetch and clear interrupt bits, process abnormal ones */ status = ioread32(priv->base + FTGMAC100_OFFSET_ISR); - iowrite32(status, priv->base + FTGMAC100_OFFSET_ISR); + ftgmac100_write(status, priv->base + FTGMAC100_OFFSET_ISR); if (unlikely(status & FTGMAC100_INT_BAD)) { /* RX buffer unavailable */ @@ -1266,7 +1274,7 @@ static irqreturn_t ftgmac100_interrupt(int irq, void *dev_id) if (net_ratelimit()) netdev_warn(netdev, "AHB bus error ! Resetting chip.\n"); - iowrite32(0, priv->base + FTGMAC100_OFFSET_IER); + ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER); schedule_work(&priv->reset_task); return IRQ_HANDLED; } @@ -1281,7 +1289,7 @@ static irqreturn_t ftgmac100_interrupt(int irq, void *dev_id) } /* Only enable "bad" interrupts while NAPI is on */ - iowrite32(new_mask, priv->base + FTGMAC100_OFFSET_IER); + ftgmac100_write(new_mask, priv->base + FTGMAC100_OFFSET_IER); /* Schedule NAPI bh */ napi_schedule_irqoff(&priv->napi); @@ -1320,8 +1328,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget) ftgmac100_start_hw(priv); /* Re-enable "bad" interrupts */ - iowrite32(FTGMAC100_INT_BAD, - priv->base + FTGMAC100_OFFSET_IER); + ftgmac100_write(FTGMAC100_INT_BAD, priv->base + FTGMAC100_OFFSET_IER); } /* As long as we are waiting for transmit packets to be @@ -1336,13 +1343,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget) * they were masked. So we clear them first, then we need * to re-check if there's something to process */ - iowrite32(FTGMAC100_INT_RXTX, - priv->base + FTGMAC100_OFFSET_ISR); - - /* Push the above (and provides a barrier vs. subsequent - * reads of the descriptor). - */ - ioread32(priv->base + FTGMAC100_OFFSET_ISR); + ftgmac100_write(FTGMAC100_INT_RXTX, priv->base + FTGMAC100_OFFSET_ISR); /* Check RX and TX descriptors for more work to do */ if (ftgmac100_check_rx(priv) || @@ -1353,8 +1354,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget) napi_complete(napi); /* enable all interrupts */ - iowrite32(FTGMAC100_INT_ALL, - priv->base + FTGMAC100_OFFSET_IER); + ftgmac100_write(FTGMAC100_INT_ALL, priv->base + FTGMAC100_OFFSET_IER); } return work_done; @@ -1382,7 +1382,7 @@ static int ftgmac100_init_all(struct ftgmac100 *priv, bool ignore_alloc_err) netif_start_queue(priv->netdev); /* Enable all interrupts */ - iowrite32(FTGMAC100_INT_ALL, priv->base + FTGMAC100_OFFSET_IER); + ftgmac100_write(FTGMAC100_INT_ALL, priv->base + FTGMAC100_OFFSET_IER); return err; } @@ -1508,7 +1508,7 @@ static int ftgmac100_open(struct net_device *netdev) err_irq: netif_napi_del(&priv->napi); err_hw: - iowrite32(0, priv->base + FTGMAC100_OFFSET_IER); + ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER); ftgmac100_free_rings(priv); return err; } @@ -1526,7 +1526,7 @@ static int ftgmac100_stop(struct net_device *netdev) */ /* disable all interrupts */ - iowrite32(0, priv->base + FTGMAC100_OFFSET_IER); + ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER); netif_stop_queue(netdev); napi_disable(&priv->napi); @@ -1549,7 +1549,7 @@ static void ftgmac100_tx_timeout(struct net_device *netdev, unsigned int txqueue struct ftgmac100 *priv = netdev_priv(netdev); /* Disable all interrupts */ - iowrite32(0, priv->base + FTGMAC100_OFFSET_IER); + ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER); /* Do the reset outside of interrupt context */ schedule_work(&priv->reset_task); -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] ftgmac100: Add a dummy read to ensure running sequence 2020-10-19 8:57 ` [PATCH 3/4] ftgmac100: Add a dummy read to ensure running sequence Dylan Hung @ 2020-10-19 23:25 ` Benjamin Herrenschmidt 0 siblings, 0 replies; 14+ messages in thread From: Benjamin Herrenschmidt @ 2020-10-19 23:25 UTC (permalink / raw) To: Dylan Hung, davem, kuba, netdev, linux-kernel, ratbert, linux-aspeed, openbmc Cc: BMC-SW On Mon, 2020-10-19 at 16:57 +0800, Dylan Hung wrote: > On the AST2600 care must be taken to ensure writes appear correctly when > modifying the interrupt reglated registers. > > Add a function to perform a read after all writes to the IER and ISR registers. You need to elaborate here. MMIO writes shouldn't get out of order, though they can get posted. I thus object to that "blanket" ftgmac100_write(). Instead, specifically add reads to flush posted writes where they are necessary and document it with a comment. Unless there's a deeper problem in the HW, in which case you need a better explanation. Cheers, Ben. > Fixes: 39bfab8844a0 ("net: ftgmac100: Add support for DT phy-handle property") > Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com> > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > drivers/net/ethernet/faraday/ftgmac100.c | 38 ++++++++++++------------ > 1 file changed, 19 insertions(+), 19 deletions(-) > > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c > index 810bda80f138..0c67fc3e27df 100644 > --- a/drivers/net/ethernet/faraday/ftgmac100.c > +++ b/drivers/net/ethernet/faraday/ftgmac100.c > @@ -111,6 +111,14 @@ struct ftgmac100 { > bool is_aspeed; > }; > > +/* Helper to ensure writes are observed with the correct ordering. Use only > + * for IER and ISR accesses. */ > +static void ftgmac100_write(u32 val, void __iomem *addr) > +{ > + iowrite32(val, addr); > + ioread32(addr); > +} > + > static int ftgmac100_reset_mac(struct ftgmac100 *priv, u32 maccr) > { > struct net_device *netdev = priv->netdev; > @@ -1048,7 +1056,7 @@ static void ftgmac100_adjust_link(struct net_device *netdev) > return; > > /* Disable all interrupts */ > - iowrite32(0, priv->base + FTGMAC100_OFFSET_IER); > + ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER); > > /* Reset the adapter asynchronously */ > schedule_work(&priv->reset_task); > @@ -1246,7 +1254,7 @@ static irqreturn_t ftgmac100_interrupt(int irq, void *dev_id) > > /* Fetch and clear interrupt bits, process abnormal ones */ > status = ioread32(priv->base + FTGMAC100_OFFSET_ISR); > - iowrite32(status, priv->base + FTGMAC100_OFFSET_ISR); > + ftgmac100_write(status, priv->base + FTGMAC100_OFFSET_ISR); > if (unlikely(status & FTGMAC100_INT_BAD)) { > > /* RX buffer unavailable */ > @@ -1266,7 +1274,7 @@ static irqreturn_t ftgmac100_interrupt(int irq, void *dev_id) > if (net_ratelimit()) > netdev_warn(netdev, > "AHB bus error ! Resetting chip.\n"); > - iowrite32(0, priv->base + FTGMAC100_OFFSET_IER); > + ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER); > schedule_work(&priv->reset_task); > return IRQ_HANDLED; > } > @@ -1281,7 +1289,7 @@ static irqreturn_t ftgmac100_interrupt(int irq, void *dev_id) > } > > /* Only enable "bad" interrupts while NAPI is on */ > - iowrite32(new_mask, priv->base + FTGMAC100_OFFSET_IER); > + ftgmac100_write(new_mask, priv->base + FTGMAC100_OFFSET_IER); > > /* Schedule NAPI bh */ > napi_schedule_irqoff(&priv->napi); > @@ -1320,8 +1328,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget) > ftgmac100_start_hw(priv); > > /* Re-enable "bad" interrupts */ > - iowrite32(FTGMAC100_INT_BAD, > - priv->base + FTGMAC100_OFFSET_IER); > + ftgmac100_write(FTGMAC100_INT_BAD, priv->base + FTGMAC100_OFFSET_IER); > } > > /* As long as we are waiting for transmit packets to be > @@ -1336,13 +1343,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget) > * they were masked. So we clear them first, then we need > * to re-check if there's something to process > */ > - iowrite32(FTGMAC100_INT_RXTX, > - priv->base + FTGMAC100_OFFSET_ISR); > - > - /* Push the above (and provides a barrier vs. subsequent > - * reads of the descriptor). > - */ > - ioread32(priv->base + FTGMAC100_OFFSET_ISR); > + ftgmac100_write(FTGMAC100_INT_RXTX, priv->base + FTGMAC100_OFFSET_ISR); > > /* Check RX and TX descriptors for more work to do */ > if (ftgmac100_check_rx(priv) || > @@ -1353,8 +1354,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget) > napi_complete(napi); > > /* enable all interrupts */ > - iowrite32(FTGMAC100_INT_ALL, > - priv->base + FTGMAC100_OFFSET_IER); > + ftgmac100_write(FTGMAC100_INT_ALL, priv->base + FTGMAC100_OFFSET_IER); > } > > return work_done; > @@ -1382,7 +1382,7 @@ static int ftgmac100_init_all(struct ftgmac100 *priv, bool ignore_alloc_err) > netif_start_queue(priv->netdev); > > /* Enable all interrupts */ > - iowrite32(FTGMAC100_INT_ALL, priv->base + FTGMAC100_OFFSET_IER); > + ftgmac100_write(FTGMAC100_INT_ALL, priv->base + FTGMAC100_OFFSET_IER); > > return err; > } > @@ -1508,7 +1508,7 @@ static int ftgmac100_open(struct net_device *netdev) > err_irq: > netif_napi_del(&priv->napi); > err_hw: > - iowrite32(0, priv->base + FTGMAC100_OFFSET_IER); > + ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER); > ftgmac100_free_rings(priv); > return err; > } > @@ -1526,7 +1526,7 @@ static int ftgmac100_stop(struct net_device *netdev) > */ > > /* disable all interrupts */ > - iowrite32(0, priv->base + FTGMAC100_OFFSET_IER); > + ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER); > > netif_stop_queue(netdev); > napi_disable(&priv->napi); > @@ -1549,7 +1549,7 @@ static void ftgmac100_tx_timeout(struct net_device *netdev, unsigned int txqueue > struct ftgmac100 *priv = netdev_priv(netdev); > > /* Disable all interrupts */ > - iowrite32(0, priv->base + FTGMAC100_OFFSET_IER); > + ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER); > > /* Do the reset outside of interrupt context */ > schedule_work(&priv->reset_task); ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/4] ftgmac100: Restart MAC HW once 2020-10-19 8:57 [PATCH 0/4] fix ftgmac100 issues on aspeed soc Dylan Hung ` (2 preceding siblings ...) 2020-10-19 8:57 ` [PATCH 3/4] ftgmac100: Add a dummy read to ensure running sequence Dylan Hung @ 2020-10-19 8:57 ` Dylan Hung 2020-10-19 23:26 ` Benjamin Herrenschmidt 2020-10-20 4:14 ` Joel Stanley 3 siblings, 2 replies; 14+ messages in thread From: Dylan Hung @ 2020-10-19 8:57 UTC (permalink / raw) To: davem, kuba, netdev, linux-kernel, ratbert, linux-aspeed, openbmc; +Cc: BMC-SW The interrupt handler may set the flag to reset the mac in the future, but that flag is not cleared once the reset has occured. Fixes: 10cbd6407609 ("ftgmac100: Rework NAPI & interrupts handling") Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com> Signed-off-by: Joel Stanley <joel@jms.id.au> --- drivers/net/ethernet/faraday/ftgmac100.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c index 0c67fc3e27df..57736b049de3 100644 --- a/drivers/net/ethernet/faraday/ftgmac100.c +++ b/drivers/net/ethernet/faraday/ftgmac100.c @@ -1326,6 +1326,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget) */ if (unlikely(priv->need_mac_restart)) { ftgmac100_start_hw(priv); + priv->need_mac_restart = false; /* Re-enable "bad" interrupts */ ftgmac100_write(FTGMAC100_INT_BAD, priv->base + FTGMAC100_OFFSET_IER); -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] ftgmac100: Restart MAC HW once 2020-10-19 8:57 ` [PATCH 4/4] ftgmac100: Restart MAC HW once Dylan Hung @ 2020-10-19 23:26 ` Benjamin Herrenschmidt 2020-10-20 4:14 ` Joel Stanley 1 sibling, 0 replies; 14+ messages in thread From: Benjamin Herrenschmidt @ 2020-10-19 23:26 UTC (permalink / raw) To: Dylan Hung, davem, kuba, netdev, linux-kernel, ratbert, linux-aspeed, openbmc Cc: BMC-SW On Mon, 2020-10-19 at 16:57 +0800, Dylan Hung wrote: > The interrupt handler may set the flag to reset the mac in the > future, > but that flag is not cleared once the reset has occured. > > Fixes: 10cbd6407609 ("ftgmac100: Rework NAPI & interrupts handling") > Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com> > Signed-off-by: Joel Stanley <joel@jms.id.au> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > drivers/net/ethernet/faraday/ftgmac100.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c > b/drivers/net/ethernet/faraday/ftgmac100.c > index 0c67fc3e27df..57736b049de3 100644 > --- a/drivers/net/ethernet/faraday/ftgmac100.c > +++ b/drivers/net/ethernet/faraday/ftgmac100.c > @@ -1326,6 +1326,7 @@ static int ftgmac100_poll(struct napi_struct > *napi, int budget) > */ > if (unlikely(priv->need_mac_restart)) { > ftgmac100_start_hw(priv); > + priv->need_mac_restart = false; > > /* Re-enable "bad" interrupts */ > ftgmac100_write(FTGMAC100_INT_BAD, priv->base + > FTGMAC100_OFFSET_IER); ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] ftgmac100: Restart MAC HW once 2020-10-19 8:57 ` [PATCH 4/4] ftgmac100: Restart MAC HW once Dylan Hung 2020-10-19 23:26 ` Benjamin Herrenschmidt @ 2020-10-20 4:14 ` Joel Stanley 2021-03-12 0:26 ` Joel Stanley 1 sibling, 1 reply; 14+ messages in thread From: Joel Stanley @ 2020-10-20 4:14 UTC (permalink / raw) To: Dylan Hung Cc: BMC-SW, Po-Yu Chuang, linux-aspeed, netdev, OpenBMC Maillist, Linux Kernel Mailing List, Jakub Kicinski, David S . Miller On Mon, 19 Oct 2020 at 08:57, Dylan Hung <dylan_hung@aspeedtech.com> wrote: > > The interrupt handler may set the flag to reset the mac in the future, > but that flag is not cleared once the reset has occured. > > Fixes: 10cbd6407609 ("ftgmac100: Rework NAPI & interrupts handling") > Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com> > Signed-off-by: Joel Stanley <joel@jms.id.au> Reviewed-by: Joel Stanley <joel@jms.id.au> > --- > drivers/net/ethernet/faraday/ftgmac100.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c > index 0c67fc3e27df..57736b049de3 100644 > --- a/drivers/net/ethernet/faraday/ftgmac100.c > +++ b/drivers/net/ethernet/faraday/ftgmac100.c > @@ -1326,6 +1326,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget) > */ > if (unlikely(priv->need_mac_restart)) { > ftgmac100_start_hw(priv); > + priv->need_mac_restart = false; > > /* Re-enable "bad" interrupts */ > ftgmac100_write(FTGMAC100_INT_BAD, priv->base + FTGMAC100_OFFSET_IER); > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] ftgmac100: Restart MAC HW once 2020-10-20 4:14 ` Joel Stanley @ 2021-03-12 0:26 ` Joel Stanley 2021-03-12 0:28 ` David Miller 0 siblings, 1 reply; 14+ messages in thread From: Joel Stanley @ 2021-03-12 0:26 UTC (permalink / raw) To: Dylan Hung Cc: BMC-SW, Po-Yu Chuang, linux-aspeed, Networking, OpenBMC Maillist, Linux Kernel Mailing List, Jakub Kicinski, David S . Miller On Tue, 20 Oct 2020 at 04:14, Joel Stanley <joel@jms.id.au> wrote: > > On Mon, 19 Oct 2020 at 08:57, Dylan Hung <dylan_hung@aspeedtech.com> wrote: > > > > The interrupt handler may set the flag to reset the mac in the future, > > but that flag is not cleared once the reset has occured. > > > > Fixes: 10cbd6407609 ("ftgmac100: Rework NAPI & interrupts handling") > > Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com> > > Signed-off-by: Joel Stanley <joel@jms.id.au> > > Reviewed-by: Joel Stanley <joel@jms.id.au> net maintainers, this one never made it into the tree. Do you need me to re-send it? Cheers, Joel > > > --- > > drivers/net/ethernet/faraday/ftgmac100.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c > > index 0c67fc3e27df..57736b049de3 100644 > > --- a/drivers/net/ethernet/faraday/ftgmac100.c > > +++ b/drivers/net/ethernet/faraday/ftgmac100.c > > @@ -1326,6 +1326,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget) > > */ > > if (unlikely(priv->need_mac_restart)) { > > ftgmac100_start_hw(priv); > > + priv->need_mac_restart = false; > > > > /* Re-enable "bad" interrupts */ > > ftgmac100_write(FTGMAC100_INT_BAD, priv->base + FTGMAC100_OFFSET_IER); > > -- > > 2.17.1 > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] ftgmac100: Restart MAC HW once 2021-03-12 0:26 ` Joel Stanley @ 2021-03-12 0:28 ` David Miller 0 siblings, 0 replies; 14+ messages in thread From: David Miller @ 2021-03-12 0:28 UTC (permalink / raw) To: joel Cc: BMC-SW, ratbert, linux-aspeed, netdev, openbmc, linux-kernel, kuba, dylan_hung From: Joel Stanley <joel@jms.id.au> Date: Fri, 12 Mar 2021 00:26:43 +0000 > On Tue, 20 Oct 2020 at 04:14, Joel Stanley <joel@jms.id.au> wrote: >> >> On Mon, 19 Oct 2020 at 08:57, Dylan Hung <dylan_hung@aspeedtech.com> wrote: >> > >> > The interrupt handler may set the flag to reset the mac in the future, >> > but that flag is not cleared once the reset has occured. >> > >> > Fixes: 10cbd6407609 ("ftgmac100: Rework NAPI & interrupts handling") >> > Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com> >> > Signed-off-by: Joel Stanley <joel@jms.id.au> >> >> Reviewed-by: Joel Stanley <joel@jms.id.au> > > net maintainers, this one never made it into the tree. Do you need me > to re-send it? If it has been this long, definitely you need to resend. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-03-12 0:36 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-19 8:57 [PATCH 0/4] fix ftgmac100 issues on aspeed soc Dylan Hung 2020-10-19 8:57 ` [PATCH 1/4] ftgmac100: Fix race issue on TX descriptor[0] Dylan Hung 2020-10-19 23:19 ` Benjamin Herrenschmidt 2020-10-20 4:13 ` Joel Stanley 2020-10-20 6:23 ` Benjamin Herrenschmidt 2020-10-20 7:13 ` Joel Stanley 2020-10-19 8:57 ` [PATCH 2/4] ftgmac100: Fix missing-poll issue Dylan Hung 2020-10-19 8:57 ` [PATCH 3/4] ftgmac100: Add a dummy read to ensure running sequence Dylan Hung 2020-10-19 23:25 ` Benjamin Herrenschmidt 2020-10-19 8:57 ` [PATCH 4/4] ftgmac100: Restart MAC HW once Dylan Hung 2020-10-19 23:26 ` Benjamin Herrenschmidt 2020-10-20 4:14 ` Joel Stanley 2021-03-12 0:26 ` Joel Stanley 2021-03-12 0:28 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).