rp2040/rp23xx: order the USB BUFF_STATUS clear before the AVAILABLE re-arm

rp2040_update_buffer_control()/rp23xx_update_buffer_control() re-arm an
endpoint buffer by setting the AVAILABLE bit in the buffer-control word,
which lives in DPSRAM.  When a buffer is re-armed from the completion
path (rp2040_usbintr_buffstat/rp23xx_usbintr_buffstat), that runs after
the handler has cleared the endpoint's bit in BUFF_STATUS, which lives in
the USB controller register block -- a different peripheral region.

The bus fabric may reorder those two stores.  If the controller observes
the AVAILABLE re-arm before the BUFF_STATUS clear lands, it can transmit
the next IN packet and latch its completion in BUFF_STATUS before the
clear arrives; the late clear then wipes that just-set completion bit.
The lost completion edge stops all further buffer interrupts for the
endpoint, so the class driver's write-complete callback never runs and
TX wedges permanently.

This is most visible on RP2350 (Cortex-M33) under dense/bursty IN traffic
such as CDC-NCM with TCP write buffers.  Add a UP_DMB() at the top of the
AVAILABLE re-arm so the preceding BUFF_STATUS clear is ordered ahead of
it.  (Non-SMP builds reduce spin_lock_irqsave to a barrier-free
up_irq_save, so nothing else orders these two stores.)

Assisted-by: Claude (Anthropic Claude Code)
Signed-off-by: Ricard Rosson <ricard@groundbits.com>
This commit is contained in:
Ricard Rosson 2026-07-29 11:30:15 +01:00 committed by Xiang Xiao
parent a9e4af4f0d
commit a85b28bc2c
2 changed files with 26 additions and 0 deletions

View file

@ -38,6 +38,7 @@
#include <nuttx/arch.h>
#include <nuttx/spinlock.h>
#include <nuttx/kmalloc.h>
#include <arch/barriers.h>
#include <nuttx/usb/usb.h>
#include <nuttx/usb/usbdev.h>
#include <nuttx/usb/usbdev_trace.h>
@ -483,6 +484,18 @@ static void rp2040_update_buffer_control(struct rp2040_ep_s *privep,
if (or_mask & RP2040_USBCTRL_DPSRAM_EP_BUFF_CTRL_AVAIL)
{
/* Order the preceding BUFF_STATUS clear (in the USB controller
* register block) ahead of the AVAILABLE re-arm (in DPSRAM):
* these are two different peripheral regions whose stores the
* bus fabric may reorder. Without the barrier the controller
* can observe the re-arm before the clear, transmit the next IN
* packet and latch its completion in BUFF_STATUS, then have that
* bit wiped by the late-landing clear -- the lost completion
* stops all further buffer interrupts and TX wedges.
*/
UP_DMB();
/* RP2040 datasheet 4.1.2.5.1: the AVAILABLE bit must be set
* after the rest of the buffer control register has been
* written and had time to settle across the clock domain

View file

@ -38,6 +38,7 @@
#include <nuttx/arch.h>
#include <nuttx/spinlock.h>
#include <nuttx/kmalloc.h>
#include <arch/barriers.h>
#include <nuttx/usb/usb.h>
#include <nuttx/usb/usbdev.h>
#include <nuttx/usb/usbdev_trace.h>
@ -483,6 +484,18 @@ static void rp23xx_update_buffer_control(struct rp23xx_ep_s *privep,
if (or_mask & RP23XX_USBCTRL_DPSRAM_EP_BUFF_CTRL_AVAIL)
{
/* Order the preceding BUFF_STATUS clear (in the USB controller
* register block) ahead of the AVAILABLE re-arm (in DPSRAM):
* these are two different peripheral regions whose stores the
* Cortex-M33 bus fabric may reorder. Without the barrier the
* controller can observe the re-arm before the clear, transmit
* the next IN packet and latch its completion in BUFF_STATUS,
* then have that bit wiped by the late-landing clear -- the lost
* completion stops all further buffer interrupts and TX wedges.
*/
UP_DMB();
/* The AVAILABLE bit must be set after the rest of the buffer
* control register has been written and had time to settle
* across the clock domain crossing, or the controller may act