Re: [PATCH] net: macb: add TX stall timeout callback to recover from lost TSTART write
From: Théo Lebrun <theo.lebrun@bootlin.com>
Date: 2026-06-12 09:45:38
Also in:
linux-arm-kernel, lkml
Hello Andrea, On Fri Jun 12, 2026 at 11:01 AM CEST, Andrea della Porta wrote:
quoted hunk ↗ jump to hunk
From: Lukasz Raczylo <redacted> 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/ (local) Fixes: dc110d1b23564 ("net: cadence: macb: Add support for Raspberry Pi RP1 ethernet controller") Signed-off-by: Lukasz Raczylo <redacted> Co-developed-by: Steffen Jaeckel <redacted> Signed-off-by: Steffen Jaeckel <redacted> Co-developed-by: Andrea della Porta <andrea.porta@suse.com> Signed-off-by: Andrea della Porta <andrea.porta@suse.com> --- drivers/net/ethernet/cadence/macb_main.c | 11 +++++++++++ 1 file changed, 11 insertions(+)diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index a12aa21244e83..615da65d5d68d 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c@@ -4522,6 +4522,16 @@ 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); + + if (net_ratelimit()) + netdev_err(dev, "TX stall detected, re-kicking TSTART\n");
Is this standard? It looks odd.
+ dev->stats.tx_errors++;
I am surprised by this. `tx_errors` would ideally be one per packet that didn't get sent. Here we increment it once per queue that stalled. I have a series to address stats issue (and use netdev_stat_ops API). It is a follow-up to this: https://lore.kernel.org/netdev/20260428-macb-drop-tx-v2-0-647f5199d8df@bootlin.com/ (local) Also this is per-device shared data and we access it without synchronisation. Let's drop this increment.
+ macb_tx_restart(&bp->queues[q]); +}
Regards, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com