mirror/tinyusb
3
0
Fork 0
mirror of https://github.com/hathach/tinyusb.git synced 2026-08-18 11:02:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

dcd_lpc17_40: mask USB IRQ around non-reentrant SIE and realization sequences

Browse Source 
The SIE command protocol (CmdCode + CCEMPTY/CDFULL handshake), the
slave-mode Ctrl/RxData/TxData window, the EpIntEn read-modify-writes, and
set_ep_size's ReEp/EP_RLZED handshake are all shared between thread-mode
API calls and dcd_int_handler, and none are reentrant: an ISR preempting a
thread-mode sequence consumes its handshake flags or, in set_ep_size's
case, a bus reset's DevIntClr = 0xFFFFFFFF eats the EP_RLZED flag the
spin waits on, hanging it forever. Guard them by masking only the USB IRQ
(nestable, ISR-safe; CMSIS NVIC_DisableIRQ already ends with DSB+ISB).
control_xact keeps the mask across its in_isr=true event push, since
osal_none skips queue locking for in_isr.

Hardening, not a fix for an observed failure: the ea4088 usbtest 30/30 +
HIL 14/14 results were reproduced with and without it. The windows are a
few instructions wide and most exposed on RTOS builds where class drivers
queue transfers from tasks concurrent with the USB IRQ.
This commit is contained in:
hathach
2026-07-16 14:11:04 +07:00
parent 36cd9f9f46
commit a3ee0b4ff1
1 changed files with 55 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

56
src/portable/nxp/lpc17_40/dcd_lpc17_40.c
View File

@ -114,6 +114,28 @@ CFG_TUD_MEM_SECTION TU_ATTR_ALIGNED(128) static dcd_data_t _dcd;
// SIE Command
//--------------------------------------------------------------------+
// The SIE command protocol (CmdCode + CCEMPTY/CDFULL handshake) and the
// slave-mode Ctrl/RxData/TxData registers are shared between thread-mode API
// calls and dcd_int_handler, and are not reentrant: an ISR preempting a
// thread-mode SIE sequence consumes its handshake flags and overwrites
// CmdCode (symptom: EP0 wedges/answers stale data right after SET_INTERFACE
// stall/clear-stall bursts overlapping bulk EOT interrupts). Mask only the
// USB interrupt around those sequences; safe to nest, including from the ISR.
static inline bool usb_irq_lock(void)
{
bool const enabled = NVIC_GetEnableIRQ(USB_IRQn) != 0;
if (enabled)
{
NVIC_DisableIRQ(USB_IRQn); // CMSIS already ends this with DSB+ISB
}
return enabled;
}
static inline void usb_irq_unlock(bool enabled)
{
if (enabled) NVIC_EnableIRQ(USB_IRQn);
}
static void sie_cmd_code (sie_cmdphase_t phase, uint8_t code_data)
{
LPC_USB->DevIntClr = (DEV_INT_COMMAND_CODE_EMPTY_MASK | DEV_INT_COMMAND_DATA_FULL_MASK);
@ -127,19 +149,28 @@ static void sie_cmd_code (sie_cmdphase_t phase, uint8_t code_data)
static void sie_write (uint8_t cmd_code, uint8_t data_len, uint8_t data)
{
bool const lock = usb_irq_lock();
sie_cmd_code(SIE_CMDPHASE_COMMAND, cmd_code);
if (data_len)
{
sie_cmd_code(SIE_CMDPHASE_WRITE, data);
}
usb_irq_unlock(lock);
}
static uint8_t sie_read (uint8_t cmd_code)
{
bool const lock = usb_irq_lock();
sie_cmd_code(SIE_CMDPHASE_COMMAND , cmd_code);
sie_cmd_code(SIE_CMDPHASE_READ , cmd_code);
return (uint8_t) LPC_USB->CmdData;
uint8_t const data = (uint8_t) LPC_USB->CmdData;
usb_irq_unlock(lock);
return data;
}
//--------------------------------------------------------------------+
@ -152,6 +183,11 @@ static inline uint8_t ep_addr2idx(uint8_t ep_addr)
static void set_ep_size(uint8_t ep_id, uint16_t max_packet_size)
{
// ReEp RMW + the EP_RLZED handshake share DevIntSt with the ISR: a bus reset
// from dcd_int_handler writes DevIntClr = 0xFFFFFFFF and would consume the
// flag this spin waits on, hanging it forever -> same lock as the SIE paths.
bool const lock = usb_irq_lock();
// follows example in 11.10.4.2
LPC_USB->ReEp |= TU_BIT(ep_id);
LPC_USB->EpInd = ep_id; // select index before setting packet size
@ -159,6 +195,8 @@ static void set_ep_size(uint8_t ep_id, uint16_t max_packet_size)
while ((LPC_USB->DevIntSt & DEV_INT_ENDPOINT_REALIZED_MASK) == 0) {}
LPC_USB->DevIntClr = DEV_INT_ENDPOINT_REALIZED_MASK;
usb_irq_unlock(lock);
}
@ -265,6 +303,7 @@ static inline uint8_t byte2dword(uint8_t bytes)
static void control_ep_write(void const * buffer, uint8_t len)
{
uint32_t const * buf32 = (uint32_t const *) buffer;
bool const lock = usb_irq_lock(); // Ctrl/TxData + SIE sequence must not interleave with the ISR
LPC_USB->Ctrl = USBCTRL_WRITE_ENABLE_MASK; // logical endpoint = 0
LPC_USB->TxPLen = (uint32_t) len;
@ -280,10 +319,14 @@ static void control_ep_write(void const * buffer, uint8_t len)
// select control IN & validate the endpoint
sie_write(SIE_CMDCODE_ENDPOINT_SELECT+1, 0, 0);
sie_write(SIE_CMDCODE_BUFFER_VALIDATE , 0, 0);
usb_irq_unlock(lock);
}
static uint8_t control_ep_read(void * buffer, uint8_t len)
{
bool const lock = usb_irq_lock(); // Ctrl/RxData + SIE sequence must not interleave with the ISR
LPC_USB->Ctrl = USBCTRL_READ_ENABLE_MASK; // logical endpoint = 0
while ((LPC_USB->RxPLen & USBRXPLEN_PACKET_READY_MASK) == 0) {} // TODO blocking, should have timeout
@ -302,6 +345,7 @@ static uint8_t control_ep_read(void * buffer, uint8_t len)
sie_write(SIE_CMDCODE_ENDPOINT_SELECT+0, 0, 0);
sie_write(SIE_CMDCODE_BUFFER_CLEAR , 0, 0);
usb_irq_unlock(lock);
return len;
}
@ -443,13 +487,19 @@ static bool control_xact(uint8_t rhport, uint8_t dir, uint8_t * buffer, uint8_t
control_ep_write(buffer, len);
}else
{
// guard the out_received/out_buffer handshake against the EP0 OUT ISR
bool const lock = usb_irq_lock();
if ( _dcd.control.out_received )
{
// Already received the DATA OUT packet
_dcd.control.out_received = false;
uint8_t received = control_ep_read(buffer, len);
// event queued with in_isr=true, which skips the queue's own locking: keep the
// USB IRQ masked across it, or a real ISR completion could interleave the write
dcd_event_xfer_complete(0, 0, received, XFER_RESULT_SUCCESS, true);
usb_irq_unlock(lock);
}else
{
// buffer is NULL for a status-stage ZLP: signal the pending xfer explicitly,
@ -458,6 +508,7 @@ static bool control_xact(uint8_t rhport, uint8_t dir, uint8_t * buffer, uint8_t
_dcd.control.out_buffer = buffer;
_dcd.control.out_bytes = len;
_dcd.control.out_queued = true;
usb_irq_unlock(lock);
}
}
@ -531,8 +582,11 @@ bool dcd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t t
if ( ep_id % 2 )
{
// Clear EP interrupt before Enable DMA
// EpIntEn read-modify-write races the ISR's own RMWs -> lock
bool const lock = usb_irq_lock();
LPC_USB->EpIntEn &= ~TU_BIT(ep_id);
LPC_USB->EpDMAEn = TU_BIT(ep_id);
usb_irq_unlock(lock);
// endpoint IN need to actively raise DMA request
LPC_USB->DMARSet = TU_BIT(ep_id);