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>
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>
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>
* 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>
- Parse PVS_STUDIO_CREDENTIALS into two quoted fields (no glob/word-split).
- AGENTS.md: examples build sets CMAKE_EXPORT_COMPILE_COMMANDS ON already.
Addresses Copilot review on #3695.
- Add --security-related-issues to run_pvs.sh and AGENTS.md analyze commands
so local runs reproduce the CI SAST classification (static_analysis.yml).
- Ignore *.sarif so a successful run leaves the worktree clean.
Addresses Codex review on #3695.
The review action currently runs on the default Sonnet 4.6. On PR #3643
(musb EP0 race) it posted "No issues found" while an Opus pass on the
same diff surfaced substantive questions (ISR-boundary RXRDY lifetime,
regression scope of the DATA-state split). Pin the reviewer to
claude-opus-4-8 for higher-signal reviews; subagents keep their cheaper
default models.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Claude Code Review action runs /code-review:code-review with a hard
--max-turns cap. On large PRs (e.g. #3636 "add stm32c5 support", 29 files
/ +1689), the agent exhausts 20 turns exploring the diff before it can
produce and post its review, so the SDK returns an error and the
claude-review check fails red with:
Reached maximum number of turns (20)
Raise the cap to 50 so port-sized PRs complete and post their review.
Cost scales with tokens, not the cap: a finished review pays the same
whether the ceiling is 25 or 50 — the cap only bites when the agent
would otherwise be force-stopped mid-run.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the ✔/⚠/✖ status symbols in the Supported CPUs table and its
legend with ✅ (Supported), 🟡 (Partial support) and ❌ (Not supported by
hardware) for clearer at-a-glance scanning.
Fix stm32f723disco host HIL: UART RX starvation + DWC2 split bulk NAK/XactErr handling (#3677)
stm32f7 BSP — UART RX starvation
- The host console USART shared interrupt priority with the USB OTG ISR, so a long
OTG interrupt could starve RXNE and drop received bytes. Raise the USART RX IRQ
above OTG_FS/OTG_HS in both the bare-metal and FreeRTOS init paths, guarded by
#ifdef UART_ID so boards without a UART console keep the default OTG priority.
dwc2 host — split NAK/XactErr handling
- Slave mode: a persistently-NAKing split bulk/control IN poll re-armed the
start-split immediately, storming the ISR and starving task context. Throttle by
disabling the channel and re-arming on the resulting halt (no frame deferral).
- Buffer-DMA mode: a pure split bulk-OUT NAK was unhandled, leaving the channel
halted and stalling the transfer — the dominant cause of CDC echo truncation.
Handle it by rewinding the buffer pointers and retrying the start-split
(Programming Guide v4.20a 5.1.4.2).
- Buffer-DMA mode: a split bulk-OUT XactErr was retried immediately, exhausting
HCD_XFER_ERROR_MAX before the transient cleared. Throttle via channel_disable +
re-arm to give the hub TT a recovery gap, mirroring slave mode.
- All three are scoped to split transfers (hcsplt.split_en); non-split NAK/XactErr
keep the core-handled / immediate-retry behavior. The OUT XactErr throttle also
excludes periodic split, where channel_disable() is a no-op and would wedge the
channel. The nak_disabled flag is generalized to retry_disabled and honors
xfer->closing so an endpoint close during a throttled retry tears down cleanly.
Verified on stm32f723disco HIL (slave + CFG_TUH_DWC2_DMA_ENABLE): host/cdc_msc_hid,
msc_file_explorer, and device_info all pass on both variants; DMA CDC echo went
from ~15-25% raw failure to 10/10 clean.
* test/hil: replace build.flags_on with named variant schema
Boards declare build variants as `variant: [{name, flags}]` instead of
`build.flags_on`. The variant `name` is the build dir (cmake-build-<name>) and
the HIL report row; `flags` is the raw CFLAGS string (-D...=1) injected via
CFLAGS_CLI. No `variant` => a single build named after the board.
- build.py: --build-name <name> (dir) + --cflag=<token> (raw CFLAGS, repeatable,
=form survives the matrix's shell word-splitting); drop -f1/CFLAGS wrapping.
- hil_ci_set_matrix.py: emit one build arg per variant.
- hil_test.py: iterate variants; report row + build dir = variant name.
- hil_ci.sh: copy all cmake-build-<board>* dirs for -b runs.
- get_deps.py: accept (ignore) --build-name/--cflag from matrix args.
- tinyusb.json: migrate all 6 flags_on boards to variant.
* board_test: park CI build with busy spin instead of wfe
Teardown: instead of flashing device/board_test (a USB-less blink loop that
keeps the MCU busy-looping), erase the first flash sector (vector table) so the
board faults to idle after its tests — no USB, lower power, faster. Per-flasher
erase_<name>: openocd/openocd_adi `flash erase_sector 0 0 0`; stlink `--erase
0`; jlink erases the sector at the flash origin read from the ELF (pure-Python,
new elf_flash_origin); esptool `erase_region 0x0 0x4000`; lm4flash writes a 4 KB
all-0xFF blank image (lm4flash erases before programming, so the first sector
ends up blank). device/board_test flash remains a fallback for flashers with no
erase_ function. The teardown is no longer a report column (it's cleanup).
Report: cdc_msc_throughput and msc_file_explorer[_freertos] now return a compact
read/write speed shown in their report cell instead of the pass tick (e.g.
"C 652k/422k M 1.1M/783k", "rd 1.2MB/s"). test_example returns an optional
metric; render_matrix shows it verbatim. Firmware lookup factored into
find_firmware (reused by the erase teardown).
Verified on the rig (stm32f723disco, jlink): erase disables the board in 0.8 s
and it disappears from the bus; the throughput cell shows live speeds.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Colored emoji render green/red in the GitHub PR comment, far more visible than
the monochrome ✔/✖ dingbats. Skip stays ➖.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
metrics.md (write_combine_markdown) is also used as the PR size comment when
there is no base-metrics baseline; use h2 for its title too so the sticky
comment heading is consistent (and not oversized) in that fallback case.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
