usb: musb: gadget: don't flush a waiting RX packet
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v4 Latest2 patches 1 replyIn review
- v32 patches 1 replySuperseded
- v21 patch 1 replySuperseded
Earlier versions · 1
| Version | Posted | Patches | Replies | Status |
|---|---|---|---|---|
| v1 | 1 patch | 0 replies | Superseded |
Changes in v4 from the cover letter
- Move musb_gadget_enable_rx_kicks() in musb_resume() to be after musb_enable_interrupts() and under musb->lock (addresses sashiko's "premature and unlocked rx_kick queueing in musb_resume()")
Patches in v4 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/2 | usb: musb: gadget: count givebacks in busydrivers/ drivers/ | +6 | 1 reply | |
| 2/2 | usb: musb: gadget: don't flush a waiting RX packet when queueing a requestdrivers/ drivers/ drivers/ | +127 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v4 yet.
Cover letter [PATCH v4 0/2] usb: musb: gadget: don't flush a waiting RX packet
Since commit 3fc137386c46 ("usb: musb: Fix hardware lockup on first Rx endpoint request"), musb_ep_restart() flushes the RX FIFO instead of calling rxstate().
When using some functions with musb, for example, f_mass_storage or f_acm, this code path will be used mid-communication, and it will cause data loss.
Patch 2 fixes this in musb_ep_restart() and schedules a bh work item instead of flushing and losing the data in the RX FIFO.
Patch 1 modifies the semantics of musb_ep.busy from a flag to a counter. If usb_ep_dequeue() runs at the same time as another musb_g_giveback(), busy could end up with a false 1; this would stall an endpoint. I added this patch in the series as prep for patch 2, because patch 2 would otherwise add one more case where such a race is possible -- a musb_g_giveback() of a work item with a musb_g_giveback() of nuke().
v3 -> v4:
- Move musb_gadget_enable_rx_kicks() in musb_resume() to be after musb_enable_interrupts() and under musb->lock (addresses sashiko's "premature and unlocked rx_kick queueing in musb_resume()")
v2 -> v3:
- Added patch 1: busy is a counter now (addresses sashiko's "overlapping musb_g_giveback() calls wedge ep->busy" and the nuke() part of "race and ordering bug in musb_gadget_disable() for rx_kick")
- Schedule a kick on every OUT ep when resuming to avoid stalls when the USB session is preserved across suspend/resume (addresses sashiko's "lost rx_kick across system suspend and resume")
v1 -> v2:
- musb_g_rx_kick() does not re-queue itself when busy is set. Instead, musb_g_giveback() re-queues it when rx_kick_deferred. This new flag is set in three cases:
1) busy is set inside musb_g_rx_kick()
2) musb_gadget_queue() adds an RX request to an empty request queue and busy is set
3) busy is set in the beginning of musb_g_rx() (addresses sashiko's "RX endpoint lockup after giveback in musb_g_rx_kick()", "softirq busy-loop and livelock in musb_g_rx_kick()", and "race and ordering bug in musb_gadget_disable() for rx_kick")
- musb_g_rx_kick checks musb->is_runtime_suspended now and exits early if the controller is suspended, and the work items are disabled during system suspend (addresses sashiko's
"unushed rx_kick work in musb_suspend() accesses disabled MMIO")
- This time tested also on a Pine A64+ (Allwinner A64), which has both MUSB and 4 cores.
v3: https://lore.kernel.org/linux-usb/[email protected]/
v2: https://lore.kernel.org/linux-usb/[email protected]/
v1: https://lore.kernel.org/linux-usb/[email protected]/
Elvira Khabirova (2):
usb: musb: gadget: count givebacks in busy
usb: musb: gadget: don't flush a waiting RX packet when queueing a
request
drivers/usb/musb/musb_core.c | 3 + drivers/usb/musb/musb_gadget.c | 128 ++++++++++++++++++++++++++++++--- drivers/usb/musb/musb_gadget.h | 11 ++- 3 files changed, 133 insertions(+), 9 deletions(-)
base-commit: c339f4010dea8b0d754f84ace70cd1b089878ef8
Signature · 1 line
2.53.0