Pass --effort max to the claude CLI in the auto-review workflow so PR
reviews run at maximum reasoning effort. Switch claude_args to a
multi-line block scalar for readability, keeping --max-turns 50 and
--model claude-opus-4-8 unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Enabling the audio test fleet-wide surfaced failures on esp32-p4/s3 and
metro_m4_express: the UAC mic enumerates but arecord fails the iso IN read
with EIO, while 18 other boards pass strict=1.000.
esp32: root cause is the FreeRTOS tick rate. ESP-IDF defaults
CONFIG_FREERTOS_HZ to 100, so the audio task wakes only every 10 ms and
can't service the 1 ms UAC iso frames -> underrun -> arecord EIO. (The same
dwc2 driver passes on STM32, whose FreeRTOSConfig is 1000 Hz.) Set
CONFIG_FREERTOS_HZ=1000 in the example sdkconfig.defaults; the example
defaults are honored in the generated sdkconfig alongside the BSP's, so this
takes effect.
metro_m4_express (samd51): not tick-rate -- its FreeRTOSConfig is already
1000 Hz like the passing boards -- so it's a separate iso-IN issue, skipped
for now.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The classic-USB STM32 fsdev driver enabled the EXTI-line USB wakeup interrupt
(USBWakeUp_IRQn, and USBWakeUp_RMP_IRQn on the F3 remap path) in the NVIC, but
never uses it: resume is serviced in-band via ISTR.WKUP in the USB_LP/HP ISR,
and the driver never arms or clears that EXTI line. The wakeup EXTI interrupt is
only needed to wake the core from STOP mode, which TinyUSB does not implement.
Leaving its NVIC vector enabled lets it fire spuriously into an unhandled or
looping vector -- the freeze reported in #3696 on STM32G473.
USBWakeUp_IRQn is a valid, dedicated USB-wakeup-via-EXTI interrupt (e.g.
stm32g473xx.h: =42 "USB Wakeup through EXTI line"), not an "unrelated
interrupt"; it is simply unused here.
- Comment out USBWakeUp_IRQn for F1/F3/G4/L1 and USBWakeUp_RMP_IRQn on the F3
remap path, kept in place so STOP-mode wakeup is a one-line re-enable.
- Keep the STM32L1 USBWakeUp_IRQn -> USB_FS_WKUP_IRQn alias for that re-enable.
- Document the rationale in fsdev_stm32.h with a TODO.
- Comment out the matching USBWakeUp(_RMP)_IRQHandler in the F1/F3/G4 BSPs, and
the FreeRTOS NVIC_SetPriority(USBWakeUp_IRQn) on F1/G4.
Fixes#3696
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace PR #3515's separate dcd_ch58x_usbfs.c / hcd_ch58x_usbfs.c with the
shared WCH USBFS device driver (combined per-endpoint control, like CH32V103),
adding two CH58x-specific behaviors guarded so CH32V103/V20x/V307 are unchanged:
- CH32_USBFS_EP_MANUAL_TOG: CH58x's hardware AUTO_TOG does not stay in sync, so
the ISR toggles DATA0/DATA1 manually and discards toggle-mismatched OUT
packets. Fixes multi-packet bulk-IN (e.g. MSC READ10) that otherwise hung.
- CH32_USBFS_EP4_SHARES_EP0: EP4 has no DMA register and overlays EP0's region
(EP0[0:63] + EP4 OUT[64:127] + EP4 IN[128:191]); add a 192-byte shared buffer
and buffer-pointer helpers (transparent for the other parts). Fixes
cdc_dual_ports (Port1 is on EP4).
Add the ch582m_evt board. Device only on USB0 (rhport 0): the shared
hcd_ch32_usbfs.c is CH32V20x-specific and cannot drive CH58x, so host / USB2
(rhport 1) is left commented out in the BSP for easy re-add.
Verified on ch582m_evt via local HIL: all device examples pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The audio20 get/set entity dispatchers already extract entity_id from
wIndex and route to the matching clock / feature-unit helper, so each
helper's own entity_id re-derivation and TU_ASSERT(entity_id == ...) was
dead: the helper is only ever reached for its one entity. Unknown
entities are still rejected by the dispatcher's "not handled" path.
Remove the redundant local, the dead assert, and the constant "entity"
field from each helper's not-supported log (the message text already
identifies the entity). The local is dropped entirely rather than kept
for the log, since TU_LOG1 compiles out in release and would leave it
unused.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When no base metrics are available to compare against, the PR comment
falls back to the combined "TinyUSB Average Code Size Metrics" report
(build.yml copies metrics.md to metrics_compare.md). That posted the
full per-example size table inline, cluttering the comment.
Wrap the table in a <details><summary>Size table</summary> block so the
heading stays visible but the detail is collapsed by default, matching
the Size Difference Report's collapsible sections.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
arecord (alsa-utils) is a documented HIL host requirement, like
mtools/libmtp9/iperf — none of which have a skip-if-missing guard. The
audio test was the exception: it silently returned 'skipped' when arecord
was absent, masking host misconfiguration. The ci.lan rig had been
silently skipping device/audio_test_freertos on every board because
alsa-utils was never installed.
Remove the shutil.which('arecord') guard so a missing package surfaces as
a failure, consistent with the other tool-dependent tests, and drop the
now-unused shutil import. Note in the host-setup comment that these
packages are required (a missing tool fails its test rather than skipping).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
After the UAC2 examples switched to tusb_control_request_t with
TU_U16_HIGH/LOW() extraction, audio20_control_request_t is no longer
referenced anywhere in the tree. It is a byte-overlay of the setup
packet whose bChannelNumber/bControlSelector/bInterface/bEntityID
sub-byte fields silently misread on big-endian once wValue/wIndex are
converted to host order (tu_le16toh in dcd.h), so leaving it in the
public header is a latent BE trap; the BE bitfield guard previously
added to its bmRequestType_bit only masked that by guarding byte 0.
Drop the struct entirely. Callers should use tusb_control_request_t and
TU_U16_LOW/HIGH(wValue|wIndex), matching audio_device.c and the examples.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The CH32 workaround only restored EP0 to CONTROL in edpt_xfer(). When
the stack rejects a control write with data (handle_ctr_setup having
already switched EP0 to BULK) it stalls EP0 via dcd_edpt_stall() without
ever calling dcd_edpt_xfer(), leaving EP0 typed BULK+STALL. As SETUP
recognition is tied to CONTROL-typed endpoints, the host's recovery
SETUP (which should auto-clear the stall) would be ignored, wedging EP0
until a bus reset.
Restore CONTROL for EP0 in the same exclusive write that sets the STALL
status. DFU download-in-wrong-state is a concrete trigger.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
edpt_xfer() restored EP0 to CONTROL at the top of the function, before
the OUT stage was armed (rx bufsize + STAT_RX=VALID written further
down, with interrupts enabled in between). That re-enabled the CH32
blind OUT ACK while STAT_RX was still NAK and the buffer size stale, so
a host-retried DATA OUT / status ZLP could be ACKed into the wrong
buffer in the gap.
Fold the CONTROL restore into the single exclusive write that programs
STAT_RX=VALID for the OUT direction, after the buffer size is set, so
type and arming go live together. The IN direction keeps the (now
atomic) early restore, where no pending OUT exists to be blind-ACKed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Parse the setup packet via tusb_control_request_t instead of hand-rolled byte
indexing; drop a stray blank line and fix indentation. No functional change.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ep0_set_type() read the EP0 register outside any critical section and
only ep_write() masked the USB IRQ around the store. A USB interrupt
landing between the read and the write (e.g. a new SETUP whose handler
installs the BULK gate) was silently undone when the task resumed and
wrote back its pre-interrupt snapshot with the type forced to CONTROL,
re-exposing the unsolicited EP0 OUT ACK the workaround blocks.
Bracket the whole read-modify-write with fsdev_int_disable/enable when
called with need_exclusive (task context). ISR-context callers pass
false and are unaffected (the ISR cannot preempt itself).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Now that CH32V103 USB device works, add the board to the active HIL pool.
It is a WCH RISC-V USBFS part, so it builds under the riscv-gcc bucket;
single config (USBFS only, no fsdev variant).
cdc_msc_throughput is skipped for this board: its device->host CDC bulk-IN
read hard-fails here (a known, pre-existing dcd_ch32_usbfs throughput
limitation, not specific to CH32V103). All other device tests pass on
ci.lan (verified green, 0 failures).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CH32V103 uses the older USBFS IP: a single combined UEPn_CTRL register per
endpoint (IN response in bits [1:0], OUT response in [3:2], shared auto-toggle,
separate IN/OUT toggles) instead of the separate UEPn_TX_CTRL/UEPn_RX_CTRL
bytes of the newer IP (CH32V20x/V307). The shared driver was written for the
newer IP, so EP0 control transfers never worked on V103: the OUT response was
written to a reserved byte and the IN write clobbered the OUT bits.
- ch32_usbfs_reg.h: annotate the V103 register struct with byte offsets and add
a union exposing the combined UEPn_CTRL at the UEPn_TX_CTRL offset; define
CH32_USBFS_EP_CTRL_COMBINED and the combined-register bit positions.
- dcd_ch32_usbfs.c: abstract EP control access behind ep_tx/rx_ctrl_set() (full
write) and ep_tx/rx_set_response() (response-only RMW). The newer-IP path is
unchanged; the combined path read-modify-writes the single register and arms
the post-SETUP data stage at DATA1.
- bsp/ch32v10x: implement board_get_unique_id() (real chip UID) and drop the
CSR 0x800 writes that corrupted the QingKe V3 interrupt config and left all
interrupts disabled (the USB ISR never ran).
Verified on ch32v103r_r1_1v0: enumerates and passes HIL for cdc_msc, hid,
msc, midi, mtp, dfu, etc.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
netd_init resets link_is_up to a compile-time default, which is
incorrect when the host reboots without power-cycling the device.
Add tud_network_default_link_state_cb() so applications can return
the actual physical link state. The weak default preserves existing
CFG_TUD_NCM_DEFAULT_LINK_UP behaviour.
* hil: enable nanoch32v203 in CI with fsdev + usbfs variants
nanoch32v203 was parked in boards-skip; move it into the active pool now
that the board is wired to the ci.lan rig. Cover both USB device IPs as
build variants:
- nanoch32v203-fsdev: RHPORT_DEVICE=0 (USBD / stm32 FSDev IP)
- nanoch32v203-usbfs: RHPORT_DEVICE=1 (WCH USBFS IP)
* device: clamp EP0 OUT data copy to the control transfer buffer
usbd_control_xfer_cb() copied xferred_bytes from the EP0 bounce buffer
into the requester's buffer with no bound. A non-compliant host that
sends an OUT data packet larger than the control transfer's data_len
(= min(len, wLength), the buffer capacity) would overflow that buffer
and over-count total_xferred. Clamp xferred_bytes to the remaining
buffer space before the memcpy and accounting.
Cleanup from a code-review pass, no behavior change:
- Replace the open-coded "last DATA packet" test (remain_wlength == 0 ||
len < CFG_TUD_ENDPOINT0_SIZE), duplicated in the edpt0_xfer DATA IN
arm, pipe0_process_xfer_state_isr, and the DATA OUT drain, with one
inline pipe0_data_stage_done() so IN and OUT can't drift.
- Correct the xact_len comment (only the IN path reports it; OUT reports
count0) and the dcd_edpt_stall comment (a deferred SETUP means the old
transfer ended on the wire, not that its status stage was "seen").
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The helper advances the whole EP0 control state machine on a
completion/confirmation IRQ — it dispatches on pipe0->state and also
fires the DATA_IN completion, not just the status stage — so
"process_status" undersold it. Matches the process_*_isr family.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mirror the IN-side short-packet fix on the OUT drain: end the data
stage (-> STATUS_IN) when wLength is received OR a short OUT packet
(count0 < CFG_TUD_ENDPOINT0_SIZE) signals the host's end-of-data, not
only when remain_wlength hits exactly 0. Also clamp the
remain_wlength subtraction so a host that overruns wLength can't
underflow it and strand the transfer.
Without this, a control-OUT whose host sends fewer bytes than wLength
left pipe0 in DATA_OUT; usbd then armed STATUS IN and tripped the
split's TU_ASSERT(!dir_in). Found by /code-review; conformant hosts
send exactly wLength so HIL was already green.
Verified: HIL pass on ek_tm4c123gxl and max32666fthr (13/13 each).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pipe0_read_setup() copied the FIFO into a local union, then copied that
into the caller's struct. Read the two FIFO words straight into the
caller's uint32_t[2] (one copy) and cast to tusb_control_request_t* in
pipe0_start_setup(). pipe0.deferred_setup becomes uint32_t[2] so the
deferral path reads directly into it as well.
Verified: HIL pass on ek_tm4c123gxl and max32666fthr (13/13 each).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The deferral (RXRDY-combined) and csrl==0 tail paths in process_ep0_isr
ran the same per-state status-stage logic. Move all of it into one
pipe0_process_status_isr() helper covering every state including
DATA_IN, which picks STATUS_OUT vs STATUS_OUT_PENDING_IRQ from
deferred_setup_valid (a deferred SETUP means the status confirm was
coalesced with it). Both callers now just invoke the helper; the
deferral path saves the SETUP and sets deferred_setup_valid first.
Also drops the deferral path's TU_ASSERT(remain_wlength == 0), which
was wrong for a short last DATA-IN packet, and renames
pipe0_process_deferred_setup -> pipe0_try_deferred_setup (it no-ops
when nothing is deferred).
Verified: HIL pass on ek_tm4c123gxl and max32666fthr (13/13 each),
including the #3643 high-CPU-load IRQ-toggle coalescing stress.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rename process_ep0/process_epin/process_epout/process_bus_reset (all
invoked only from dcd_int_handler) to *_isr, making their ISR context
explicit at every call site. pipe0_process_deferred_setup is left
as-is since it also runs from task context (dcd_edpt_stall).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A short IN control response (device sends fewer bytes than wLength —
e.g. the 18-byte device descriptor answering a 64-byte GET_DESCRIPTOR)
left remain_wlength != 0, so the DATA_IN -> STATUS_OUT transition never
fired and pipe0 stayed in DATA_IN through the status stage. usbd then
armed the status-OUT while state was still DATA_IN. Set DATAEND and
transition on the last packet: remain_wlength == 0, or a short packet
(incl. a terminating ZLP) which ends the data stage.
With state now tracking the stage, split edpt0_xfer's DATA handling
into separate DATA_IN / DATA_OUT cases dispatching on state (asserting
state == call direction) instead of the combined dir_in branch.
Verified: HIL pass on ek_tm4c123gxl and max32666fthr (13/13 each),
including the #3643 high-CPU-load IRQ-toggle coalescing stress.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pure cleanup, no behavior change:
- Extract the EP0 control-transfer state into a named pipe0_state_t
typedef instead of an anonymous nested struct, and access it through
a local pipe0_state_t* in the functions that touch it repeatedly.
- Group the pipe0 fields so the two bools sit together and the larger
tusb_control_request_t deferred_setup is last.
- Reword the deferral comments: "coalesced" -> "combined".
Note: separating the edpt0_xfer DATA_IN/DATA_OUT case (dispatch on
state instead of dir_in) was attempted and reverted — it breaks ADI
MUSB enumeration. usbd can arm the opposite-direction status while
pipe0 is still in a DATA state, and only dir-dispatch routes that
correctly; a comment on the combined case records this.
Verified: HIL pass on ek_tm4c123gxl and max32666fthr (13/13 each).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pyserial's posix write() raises SerialTimeoutException after partial
progress with the byte count lost, so the retry loop re-sent from the
same offset and could duplicate bytes on the wire — surfacing as bogus
data-mismatch failures that look like device firmware bugs.
write_timeout is already a total per-call deadline, so the loop added
duplication risk without extending the budget: write once and treat a
timeout as fatal. Default bumped 2 -> 10 s to keep the old overall
bound; HIL_SERIAL_WRITE_DEADLINE removed.
The per-character CLI loops keep their existing pacing (the 2 ms sleep
between single-byte writes already spaces them on the wire); no
unbounded ser.flush()/tcdrain is added.
Review follow-up for #3643 (hil_test.py l.257/264 findings).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The actual-STALL path (no deferred SETUP) forced EP0 to IDLE but left
rxrdy_consumed set if the aborted transfer had parked RXRDY via NAK flow
control (e.g. a rejected OUT-data request in DATA_OUT). A subsequent
SETUP IRQ would then hit the parked-gate early return and be ignored,
relying on SentStall/SetupEnd to clear the flag first. Clear it here
so recovery never depends on that ordering.
Addresses Copilot review on #3699.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dcd_edpt_stall(EP0 OUT) discarded the deferred SETUP and armed
SendStall. A deferred SETUP can only exist once the old transfer's
status stage was seen on the wire, so the request usbd is rejecting
(class callback failing at CONTROL_STAGE_DATA) already succeeded
host-side and the hardware already ACKed the next SETUP - the STALL
would land on that innocent request, which then fails host-side
without any tud callback ever seeing it. Skip the stall and replay
the deferred SETUP; the rejected transfer needs no wire-level stall
since it is already over.
Review follow-up for #3643 (dcd_musb.c l.860 finding).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The deferral path drains the SETUP but leaves RxPktRdy set, and the
SETUP's IRQ latches after the ISR's clear-on-read intr_tx read - so a
second process_ep0 pass (same ISR, via the intr_tx re-read merge) is
guaranteed and misreads the leftovers: count0==0 fires a spurious
DATA OUT completion, the replay's RXRDYC write turns the second pass
into a phantom csrl==0 DATA IN completion, and a zero-length replay
re-enters the deferral case on a drained FIFO (count0 assert or
garbage saved as a SETUP). The registers cannot expose the staleness:
RxPktRdy and count0 read unchanged until ServicedRxPktRdy is written.
Track it in software: rxrdy_consumed means "RxPktRdy is set in hw but
its packet was already consumed". Set wherever a drained packet's
RXRDY is intentionally left set (OUT/zero-length flow-control parks,
every DATA OUT drain awaiting the next arm, the deferral path);
cleared at every RXRDYC write site (edpt0_xfer arms, dcd_set_address,
STALLED/SETEND recovery, bus reset). The RXRDY block returns early
while parked. Replayed IN requests skip the RXRDYC in
pipe0_start_setup and keep the packet parked until the
edpt0_xfer(DATA IN) arm acks it (before loading the shared FIFO), so
the stale pass sees RXRDY+parked instead of csrl==0. The normal IDLE
path is unchanged - master never re-entered these windows because the
single SETUP edge was always consumed by the pass that parked it; the
deferral is what introduced a pending second pass.
Review follow-up for #3643 (dcd_musb.c l.516 finding).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
STATUS_OUT_PENDING conflated "edpt0_xfer(STATUS OUT) called, awaiting
confirm IRQ" with "confirm IRQ seen, awaiting edpt0_xfer". The
deferral path completed the status and replayed the saved SETUP from
the ISR in both flavors; in the IRQ-first one, usbd's still-
outstanding edpt0_xfer(STATUS OUT) for the old transfer (queued via
status_stage_xact) then landed in the replayed transfer's state and
corrupted it: NULL pipe0.buf armed plus RXRDYC, so the host's next
DATA OUT drained through a NULL pointer. usbd processes EP0
XFER_COMPLETE events unconditionally, so nothing downstream defuses
it.
Split the state into STATUS_OUT_PENDING_XFER / _IRQ. The deferral
completes and replays only in PENDING_XFER (old transfer already
retired); in PENDING_IRQ it only holds the SETUP and the usbd-driven
edpt0_xfer fires the completion and replays. The DATA_IN deferral now
synthesizes PENDING_IRQ (its remain==0 invariant asserted: a SETUP
before DataEnd raises SetupEnd instead), which also makes the old
deferred-promotion in the csrl==0 DATA_IN case unreachable - dropped.
Assert the drain buffer before the DATA OUT FIFO read as a cheap
backstop for this corruption class.
Review follow-up for #3643 (dcd_musb.c l.503 finding).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MUSBMHDRC 21.1.5 requires the EP0 service routine to check SentStall
and SetupEnd first; the early DATAEND return ran before both, and
SentStall is most likely to fire exactly while DataEnd may still read
back set (auto-STALL after DataEnd, 21.1.7), which would skip the
recovery. The guard also moves below the RXRDY block so a coalesced
DATAEND|RXRDY read cannot swallow a SETUP on cores where the
CPU-set-only DataEnd bit reads back 1; the comment documents the
vendor-dependent read-back.
Review follow-up for #3643 (dcd_musb.c l.445 finding).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The goto jumped into the csrl==0 completion switch with RXRDY still
set, making its "When CSRL0 is zero" guard comment untrue on that
path. Handle each deferral state in a self-contained switch instead;
the csrl==0 switch is now only reached with csrl==0 and its comment
is truthful again. Behavior unchanged.
Review follow-up for #3643 (dcd_musb.c l.523 finding).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The pre-existing comment explaining why the OUT branch does not ack
RxPktRdy was dropped when the SETUP handling moved into
pipe0_start_setup(). It is load-bearing: acking before edpt0_xfer()
arms the drain buffer would let the host send data with nowhere to
put it. Restore it with the databook-deviation rationale so the
branches don't get "unified" later.
Review follow-up for #3643 (dcd_musb.c l.116 finding).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 8-byte EP0 SETUP drain (count0 assert + two FIFO word reads via a
union) was duplicated verbatim between the IDLE case and the deferral
case; a future fix applied to one copy but not the other would only
show up on the rare deferred-race path. Share one helper. count0 is
now read inside the only remaining user (DATA OUT drain).
Review follow-up for #3643 (dcd_musb.c l.507 finding).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wrap the "Changes <1% in size" section in a <details> block like the
"No changes" section, so the report comment only expands changes >1%.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
