nrf/usb: fix silent OUT packet loss from double-arming, and apply Erratum 199 - #6688
Open
gabelerner-kernel wants to merge 3 commits into
Open
Conversation
A bulk/interrupt OUT endpoint resumes accepting packets when the EasyDMA transfer completes *or* when SIZE.EPOUT[n] is written. Either event on its own re-arms it. read_dma() waits for ENDEPOUT, which has already re-armed the endpoint, and then writes SIZE.EPOUT as well. That second write arms it again, so the endpoint can accept a further packet on top of one it has not yet handed to software. The unread packet is overwritten in the endpoint's internal buffer and lost, and the following packet is read in its place. This only shows up when the host has another packet ready during the window between ENDEPOUT and the SIZE write, so it needs sustained OUT traffic and an executor that is slow to call read_packet again. It presents as silent data loss rather than an error: no event is missed, no register reads back an unexpected value, and the driver reports a full-size packet. Nordic's nrfx never writes SIZE.EPOUT on the completion path; its only write is nrf_usbd_epout_clear(), called from nrfx_usbd_transfer_out_drop(), which deliberately discards a packet without a DMA transfer. The write in endpoint_set_enabled() stays: before any DMA has run, that path is the only way to arm the endpoint initially. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
👋 Welcome, @gabelerner-kernel, and thanks for opening your first pull request here! If you haven't already, please give the contributor guide a read. |
leftger
requested changes
Aug 1, 2026
leftger
left a comment
Contributor
There was a problem hiding this comment.
This is not in my domain but I still have reservations about calling DMA registers directly without some HAL abstraction.
nRF52840 Erratum 199, "USBD cannot receive tasks during DMA": while an EasyDMA transfer is in progress, a USBD task is silently dropped rather than performed. The documented workaround is to write 0x00000082 to 0x40027C1C when starting a transfer and 0x00000000 once it completes. dma_start() and dma_end() already bracket every USBD EasyDMA transfer and are exactly the hooks this calls for, but they were bare compiler fences, so the workaround was absent. The sibling errata 187 and 171 are already implemented in the same module. This matches nrfx, which performs the same two writes in usbd_dma_pending_set() and usbd_dma_pending_clear() under nrfx_usbd_errata_199(). Applied unconditionally: Nordic's nrf52_errata_199() returns true for every nRF52840 revision, including its default arm, so there is no revision to check against. Gated on nrf52840, the part the erratum is documented for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gabelerner-kernel
force-pushed
the
nrf-usb-fix-out-endpoint-double-arm
branch
from
August 2, 2026 00:07
2cc26ac to
b6637ac
Compare
Co-authored-by: leftger <leftger@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AI Disclosure
This took a day of debugging with Opus 5 against real hardware. Our nRF streams 500Hz data over USB serial and at the same time we were trying to firmware update the device (sending many chunks) over another ACM channel. The firmware update consistently failed due to corruption - we had a hash check at the end of it all. We added a bunch of logs including hashing each packet on host and device, DMA logs, and raw bytes so we can see what was corrupted. The end result was tracked down to the one line removed which silently dropped packets under high load. I retested and the firmware updates started passing.
Opus also noticed Errata 199 was missing from the implementation and added it in this PR, but I'm happy to move that to another PR if desired.
Summary
Two independent fixes to the nRF USB driver, found while debugging silent data loss on a bulk OUT endpoint under sustained traffic.
read_dmare-arms the OUT endpoint twice per packet, which can silently drop a received packet. This is the actual bug.dma_start/dma_endare exactly the hooks it calls for but were bare compiler fences.1. OUT endpoint double-arming (silent packet loss)
A bulk/interrupt OUT endpoint resumes accepting packets when the EasyDMA transfer completes or when
SIZE.EPOUT[n]is written — either event on its own re-arms it. Nordic's pre-1.0 datasheet stated this as an AND, which is what the current code appears to be written against; it was corrected to an OR.read_dma()waits forENDEPOUT— which has already re-armed the endpoint — and then writesSIZE.EPOUTas well. That second write arms it again, so the endpoint accepts a further packet on top of one it hasn't yet handed to software. The unread packet is overwritten in the endpoint's internal buffer and lost; the following packet is read in its place.Failure sequence:
read_dmaruns the DMA;ENDEPOUTfires and the endpoint auto-re-arms.read_dmawritesSIZE.EPOUT— arming the endpoint a second time.It needs sustained OUT traffic plus a window between
ENDEPOUTand theSIZEwrite, so it's load-dependent and rare — and entirely silent. No event is missed, no register reads back an unexpected value,SIZE.EPOUTreports a full 64 bytes, andread_packetreturnsOk(64). You only find out downstream when the data is wrong.nrfxnever writesSIZE.EPOUTon the completion path. Its only write isnrf_usbd_epout_clear(), called solely fromnrfx_usbd_transfer_out_drop()— deliberately discarding a packet without a DMA transfer. The write inendpoint_set_enabled()is kept: before any DMA has run, that's the only way to arm the endpoint initially.How it was confirmed. An nRF52840 dongle running three CDC-ACM interfaces (command, telemetry at ~400 frames/s, log) corrupted roughly one command frame in several hundred. Logging an FNV-1a hash of every 64-byte USB packet on both host and device and comparing them entry for entry:
cea27b8b79d87cde505027c70f73fc85cea27b8b505027c7e13461310f73fc85The device's packet 1 is byte-identical to the host's packet 2; the host's packet 1 was never delivered. Corroborating measurements on the same frame: no overlapping EasyDMA, destination buffer stable after
ENDEPOUT,ENDEPOUTnever set on entry,SIZE.EPOUTnever zero, and the downstream pipe byte-conserved with no gaps — ruling out the alternatives. Reproduced from two independent hosts (macOS and Linux), ruling out both host stacks. Reducing unrelated USB TX load cut the rate sharply without eliminating it, consistent with load governing how long the endpoint sits unread rather than whether the extra credit exists. With this patch the failure no longer reproduces.2. Erratum 199 workaround
dma_start/dma_endalready bracket every USBD EasyDMA transfer, so the hooks existed but did nothing. Sibling errata 187 and 171 are already implemented in the same module, which suggests 199 was overlooked. This matches nrfx'susbd_dma_pending_set/usbd_dma_pending_clearundernrfx_usbd_errata_199().Applied unconditionally rather than behind a revision check, because the erratum applies to every nRF52840 revision — Nordic's
nrf52_errata_199()returnstruefrom everycasearm and itsdefault, so it is not fixed in any current or anticipated revision:This differs from the neighbouring errata 187 and 171, which are genuinely revision-limited. Gated on
nrf52840, the part the erratum is documented for.Note: this did not fix the packet loss above — that was still reproducing with 199 applied. It's included because it's a real documented omission, and it's a separate commit for that reason.
Testing
Builds for
nrf52840,nrf52833,nrf52820,nrf5340-app-s. Verified on nRF52840 hardware: a ~272 KB, ~1240-chunk firmware transfer over a CDC-ACM bulk OUT endpoint failed reliably before the fix (SHA-256 mismatch, or a malformed frame mid-transfer) and completes cleanly after.