From 7c5b8cab9a1804d4fc6143fccc74419d2e4643c2 Mon Sep 17 00:00:00 2001 From: Nicolai Buchwitz Date: Fri, 3 Jul 2026 13:03:14 +0200 Subject: [PATCH 1/5] Revert "net: macb: gate TX stall watchdog on netif_carrier_ok, use warn_ratelimited" This reverts commit b2f7eec2bfff1a7f03e9c2ad81cb4d654079ba79. Signed-off-by: Nicolai Buchwitz --- drivers/net/ethernet/cadence/macb_main.c | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index ce4f3ff1ca4cc4..eb4cb37bf116d7 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -2075,16 +2075,6 @@ static void macb_tx_stall_watchdog(struct work_struct *work) if (!netif_running(bp->dev)) return; - /* No carrier => no completion is possible. Skip the stall - * check (otherwise queue->tx_head can advance from kernel- - * queued packets between macb_open() and link autoneg - * completion while tx_tail stays unchanged, tripping a false - * positive), but keep the watchdog ticking so it picks up - * once carrier comes up. - */ - if (!netif_carrier_ok(bp->dev)) - goto reschedule; - spin_lock_irqsave(&queue->tx_ptr_lock, flags); cur_tail = queue->tx_tail; cur_head = queue->tx_head; @@ -2094,14 +2084,13 @@ static void macb_tx_stall_watchdog(struct work_struct *work) spin_unlock_irqrestore(&queue->tx_ptr_lock, flags); if (stalled) { - pr_warn_ratelimited("%s: TX stall detected on queue %u (tail=%u head=%u); re-kicking TSTART\n", - netdev_name(bp->dev), - (unsigned int)(queue - bp->queues), - cur_tail, cur_head); + netdev_warn_once(bp->dev, + "TX stall detected on queue %u (tail=%u head=%u); re-kicking TSTART\n", + (unsigned int)(queue - bp->queues), + cur_tail, cur_head); macb_tx_restart(queue); } -reschedule: schedule_delayed_work(&queue->tx_stall_watchdog_work, msecs_to_jiffies(MACB_TX_STALL_INTERVAL_MS)); } From 064f33c5aae95ab3d2cdc223d29c82f26b1d24c5 Mon Sep 17 00:00:00 2001 From: Nicolai Buchwitz Date: Fri, 3 Jul 2026 13:03:14 +0200 Subject: [PATCH 2/5] Revert "net: macb: drop destructive ISR read, use IMR barrier in macb_tx_poll" This reverts commit 60fc80b89d134d720fbab7ef504f80a4ae7a998c. Signed-off-by: Nicolai Buchwitz --- drivers/net/ethernet/cadence/macb_main.c | 37 ++++++++++-------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index eb4cb37bf116d7..cea3ecdb4bb73d 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -2002,31 +2002,24 @@ static int macb_tx_poll(struct napi_struct *napi, int budget) if (work_done < budget && napi_complete_done(napi, work_done)) { queue_writel(queue, IER, MACB_BIT(TCOMP)); - /* TCOMP events that fire while masked don't re-fire when - * IER is re-enabled (HW errata), so check in software. - * macb_tx_complete_pending() inspects the descriptor at - * tx_tail; the rmb() in there orders prior CPU writes but - * does not retire in-flight peripheral DMA writes that - * may still be racing back to memory on PCIe-attached - * parts. + /* TCOMP events that fire while the interrupt is masked do + * not re-fire when IER is re-enabled. Catch this two ways + * to avoid losing a wakeup: * - * Read a side-effect-free MMIO register (IMR, the - * read-only mask mirror) to act as a PCIe read barrier - * for prior peripheral DMA writes. After this read, any - * in-flight TX_USED descriptor update has retired and - * macb_tx_complete_pending() will observe it. + * (1) Read ISR -- catches completions the hardware flagged + * but that we did not see as an interrupt. The MMIO + * read doubles as a PCIe read barrier, flushing any + * in-flight descriptor TX_USED DMA writes into memory. + * (2) macb_tx_complete_pending() inspects the ring after + * that flush, catching a descriptor whose TX_USED is + * now visible as a result of the barrier. * - * Note: an earlier form of this block read ISR directly - * to also sample a latched TCOMP bit, but that is - * destructive on silicon where MACB_CAPS_ISR_CLEAR_ON_WRITE - * is not set (raspberrypi_rp1_config among others): the - * read clears every set bit, and a masked check silently - * consumes RCOMP / ROVR / TXUBR bits the IRQ handler is - * expected to process in one pass. IMR is non-destructive - * on both read-clear and W1C silicon. + * This can race with the interrupt handler taking the same + * path if an interrupt fires just after the IER write; + * rescheduling NAPI in that case is harmless. */ - (void)queue_readl(queue, IMR); - if (macb_tx_complete_pending(queue)) { + if ((queue_readl(queue, ISR) & MACB_BIT(TCOMP)) || + macb_tx_complete_pending(queue)) { queue_writel(queue, IDR, MACB_BIT(TCOMP)); if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE) queue_writel(queue, ISR, MACB_BIT(TCOMP)); From 0fb25b7167aee5f644fba300d795fb8776b7ec63 Mon Sep 17 00:00:00 2001 From: Nicolai Buchwitz Date: Fri, 3 Jul 2026 13:03:14 +0200 Subject: [PATCH 3/5] Revert "net: macb: add TX stall watchdog as defence-in-depth safety net" This reverts commit 79dc190b12f9504d6a1bda1e497f912f24cf54d8. Signed-off-by: Nicolai Buchwitz --- drivers/net/ethernet/cadence/macb.h | 11 ---- drivers/net/ethernet/cadence/macb_main.c | 65 ------------------------ 2 files changed, 76 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h index d62cfc19f2e570..8ef7995db60c63 100644 --- a/drivers/net/ethernet/cadence/macb.h +++ b/drivers/net/ethernet/cadence/macb.h @@ -1298,17 +1298,6 @@ struct macb_queue { struct work_struct tx_error_task; bool txubr_pending; bool tx_pending; - - /* TX stall watchdog -- see macb_tx_stall_watchdog() in macb_main.c. - * tx_stall_tail_moved is set by macb_tx_complete() under tx_ptr_lock - * whenever tx_tail advances, and cleared by the watchdog tick on the - * same lock. A bool avoids the index-aliasing false-positive that a - * snapshot-of-tx_tail comparison would have when the ring index space - * happens to wrap to the same value between two ticks. - */ - struct delayed_work tx_stall_watchdog_work; - bool tx_stall_tail_moved; - struct napi_struct napi_tx; dma_addr_t rx_ring_dma; diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index cea3ecdb4bb73d..87b6e25ce5f839 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -1505,8 +1505,6 @@ static int macb_tx_complete(struct macb_queue *queue, int budget) packets, bytes); queue->tx_tail = tail; - if (packets) - queue->tx_stall_tail_moved = true; if (__netif_subqueue_stopped(bp->dev, queue_index) && CIRC_CNT(queue->tx_head, queue->tx_tail, bp->tx_ring_size) <= MACB_TX_WAKEUP_THRESH(bp)) @@ -2031,63 +2029,6 @@ static int macb_tx_poll(struct napi_struct *napi, int budget) return work_done; } -#define MACB_TX_STALL_INTERVAL_MS 1000 - -/* TX stall watchdog. - * - * Defence-in-depth against lost TCOMP interrupts. macb already has a - * recovery chain (tx_pending -> txubr_pending -> macb_tx_restart()) - * that fires on TCOMP; if TCOMP itself is lost the TX ring stalls - * silently until something else kicks TSTART. This watchdog runs - * once per second per queue and calls macb_tx_restart() if the ring - * is non-empty and tx_tail has not advanced since the previous tick. - * - * Movement is tracked via the tx_stall_tail_moved boolean rather - * than by snapshotting tx_tail. Per-queue ring indices are bounded - * (and reused), so a snapshot comparison can false-positive when the - * index happens to land on the same value between two ticks under - * sustained load. The boolean is set by macb_tx_complete() whenever - * tx_tail advances and cleared by this watchdog after each tick; - * both writes are under tx_ptr_lock, so no atomic is required. - * - * macb_tx_restart() already checks the hardware's TBQP against the - * driver's head index before re-asserting TSTART, so on a healthy - * ring this is a no-op at the hardware level. The watchdog only - * adds the missing trigger. - */ -static void macb_tx_stall_watchdog(struct work_struct *work) -{ - struct macb_queue *queue = container_of(to_delayed_work(work), - struct macb_queue, - tx_stall_watchdog_work); - struct macb *bp = queue->bp; - unsigned int cur_tail, cur_head; - bool stalled = false; - unsigned long flags; - - if (!netif_running(bp->dev)) - return; - - spin_lock_irqsave(&queue->tx_ptr_lock, flags); - cur_tail = queue->tx_tail; - cur_head = queue->tx_head; - if (cur_head != cur_tail && !queue->tx_stall_tail_moved) - stalled = true; - queue->tx_stall_tail_moved = false; - spin_unlock_irqrestore(&queue->tx_ptr_lock, flags); - - if (stalled) { - netdev_warn_once(bp->dev, - "TX stall detected on queue %u (tail=%u head=%u); re-kicking TSTART\n", - (unsigned int)(queue - bp->queues), - cur_tail, cur_head); - macb_tx_restart(queue); - } - - schedule_delayed_work(&queue->tx_stall_watchdog_work, - msecs_to_jiffies(MACB_TX_STALL_INTERVAL_MS)); -} - static void macb_hresp_error_task(struct work_struct *work) { struct macb *bp = from_work(bp, work, hresp_err_bh_work); @@ -3356,9 +3297,6 @@ static int macb_open(struct net_device *dev) for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) { napi_enable(&queue->napi_rx); napi_enable(&queue->napi_tx); - queue->tx_stall_tail_moved = true; - schedule_delayed_work(&queue->tx_stall_watchdog_work, - msecs_to_jiffies(MACB_TX_STALL_INTERVAL_MS)); } macb_init_hw(bp); @@ -3405,7 +3343,6 @@ static int macb_close(struct net_device *dev) for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) { napi_disable(&queue->napi_rx); napi_disable(&queue->napi_tx); - cancel_delayed_work_sync(&queue->tx_stall_watchdog_work); netdev_tx_reset_queue(netdev_get_tx_queue(dev, q)); } @@ -5004,8 +4941,6 @@ static int macb_init(struct platform_device *pdev) } INIT_WORK(&queue->tx_error_task, macb_tx_error_task); - INIT_DELAYED_WORK(&queue->tx_stall_watchdog_work, - macb_tx_stall_watchdog); q++; } From b29e2b1a55782bb7ec767376ab1b7271ea0fed84 Mon Sep 17 00:00:00 2001 From: Nicolai Buchwitz Date: Fri, 3 Jul 2026 13:03:14 +0200 Subject: [PATCH 4/5] Revert "net: macb: re-check ISR after IER re-enable in macb_tx_poll" This reverts commit ff6914e97386f9a7d37468c79996dc6c34172cfb. Signed-off-by: Nicolai Buchwitz --- drivers/net/ethernet/cadence/macb_main.c | 27 +++++++++--------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index 87b6e25ce5f839..d2731ae24b8e6f 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -2000,24 +2000,17 @@ static int macb_tx_poll(struct napi_struct *napi, int budget) if (work_done < budget && napi_complete_done(napi, work_done)) { queue_writel(queue, IER, MACB_BIT(TCOMP)); - /* TCOMP events that fire while the interrupt is masked do - * not re-fire when IER is re-enabled. Catch this two ways - * to avoid losing a wakeup: - * - * (1) Read ISR -- catches completions the hardware flagged - * but that we did not see as an interrupt. The MMIO - * read doubles as a PCIe read barrier, flushing any - * in-flight descriptor TX_USED DMA writes into memory. - * (2) macb_tx_complete_pending() inspects the ring after - * that flush, catching a descriptor whose TX_USED is - * now visible as a result of the barrier. - * - * This can race with the interrupt handler taking the same - * path if an interrupt fires just after the IER write; - * rescheduling NAPI in that case is harmless. + /* Packet completions only seem to propagate to raise + * interrupts when interrupts are enabled at the time, so if + * packets were sent while interrupts were disabled, + * they will not cause another interrupt to be generated when + * interrupts are re-enabled. + * Check for this case here to avoid losing a wakeup. This can + * potentially race with the interrupt handler doing the same + * actions if an interrupt is raised just after enabling them, + * but this should be harmless. */ - if ((queue_readl(queue, ISR) & MACB_BIT(TCOMP)) || - macb_tx_complete_pending(queue)) { + if (macb_tx_complete_pending(queue)) { queue_writel(queue, IDR, MACB_BIT(TCOMP)); if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE) queue_writel(queue, ISR, MACB_BIT(TCOMP)); From 871cb497bbf7d4d5654a8c9f70dd994bf01138bd Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Tue, 16 Jun 2026 15:23:03 +0200 Subject: [PATCH 5/5] net: macb: add TX stall timeout callback to recover from lost TSTART write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MACB found in the Raspberry Pi RP1 suffers from sporadic stalls on the TX queue. While the exact root cause is not yet fully understood, it is likely related to a hardware issue where a TSTART write to the NCR register is missed, preventing the transmission from being kicked off. Implement a timeout callback to handle TX queue stalls, triggering the existing restart mechanism to recover. Link: https://lore.kernel.org/all/20260514215459.36109-1-lukasz@raczylo.com/ Fixes: dc110d1b23564 ("net: cadence: macb: Add support for Raspberry Pi RP1 ethernet controller") Signed-off-by: Lukasz Raczylo Co-developed-by: Steffen Jaeckel Signed-off-by: Steffen Jaeckel Co-developed-by: Andrea della Porta Signed-off-by: Andrea della Porta Reviewed-by: Nicolai Buchwitz Reviewed-by: Théo Lebrun Link: https://patch.msgid.link/468f480454a314303bac6a54780b153f689f2267.1781598350.git.andrea.porta@suse.com Signed-off-by: Jakub Kicinski (cherry picked from commit e438ec3e9e95cd3f49a8120e5f63ae3f9606e6fa) --- drivers/net/ethernet/cadence/macb_main.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index d2731ae24b8e6f..48c7a9ec45d3f4 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -4682,6 +4682,13 @@ static int macb_setup_tc(struct net_device *dev, enum tc_setup_type type, } } +static void macb_tx_timeout(struct net_device *dev, unsigned int q) +{ + struct macb *bp = netdev_priv(dev); + + macb_tx_restart(&bp->queues[q]); +} + static const struct net_device_ops macb_netdev_ops = { .ndo_open = macb_open, .ndo_stop = macb_close, @@ -4700,6 +4707,7 @@ static const struct net_device_ops macb_netdev_ops = { .ndo_hwtstamp_set = macb_hwtstamp_set, .ndo_hwtstamp_get = macb_hwtstamp_get, .ndo_setup_tc = macb_setup_tc, + .ndo_tx_timeout = macb_tx_timeout, }; /* Configure peripheral capabilities according to device tree