qemu_arm_mps2: fix UART receive stall and other pre-merge review findings - #21
Closed
ppannuto-claude wants to merge 1 commit into
Closed
qemu_arm_mps2: fix UART receive stall and other pre-merge review findings#21ppannuto-claude wants to merge 1 commit into
ppannuto-claude wants to merge 1 commit into
Conversation
ppannuto-claude
force-pushed
the
qemu-arm-mps2-review2
branch
from
September 2, 2026 15:56
f6783b6 to
683100e
Compare
ppannuto-claude
force-pushed
the
qemu-arm-mps2-review2
branch
2 times, most recently
from
September 2, 2026 19:12
683100e to
587218b
Compare
"The MPS2 images share peripherals and differ in their core" was stated in eight places across four READMEs and three module docs, and the "purely virtual target, not a real vendor chip" framing in four. State each once, where a reader looking for it would land. The two board READMEs are peers, but an386's was written as a footnote to an385's: it deferred run-app to "same as mps2-an385's" and so never mentioned APP_ADDRESS or the largest-first .tbf packing rule, both of which apply to it identically. Move the shared material -- the QEMU invocation, APP_ADDRESS, packing -- into mps2_base's README, which both boards already point at, and make those pointers actual links; nothing in boards/README.md could reach that file otherwise. What is left per board is what differs: machine, core, target triple, the AN386 FPU note. Also: - an385's sample `make run` output quoted a size line that no longer matches the build, while an386's did; drop the block rather than carry a number that goes stale. - The chips README listed mps2-an505 and mps2-an521 as "not yet implemented here". They are not part of this machine family at all -- they live in QEMU's hw/arm/mps2-tz.c on the IoTKit/SSE-200, with a different peripheral layout. Say so, drop the "as of Aug 2026" stamp, note that an500 also differs in its PSRAM base, and use the same heading style as every other README here. - boards/README.md advertised "Yes (10.2.1)", which is the version this was developed against, not a minimum, in a column whose other entries read as minimums. Neither the true minimum nor CI's pinned build has been tested here, so claim no version. - semihosting.rs said a `bkpt` with no host "traps with no host to service it"; with C_DEBUGEN clear it raises a debug exception that escalates to a HardFault, which terminate()'s own doc already said. Fix that, the reason given for omitting `noreturn`, a broken doc link, a BPKT/BKPT typo, and a 2022 copyright year on a new file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L56it8rfV2vkZF9YRN5fa8
ppannuto-claude
force-pushed
the
qemu-arm-mps2-review2
branch
from
September 2, 2026 19:15
587218b to
f175d81
Compare
Author
|
All five commits are integrated into Closing. #22 is rebased on top and now carries just the closure commit. |
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.
Review findings from a pre-merge pass over the branch behind tock#5125, split so the behavioural fixes can be read apart from the documentation trim. Based on
qemu-arm-mps2at d164be4.The one blocker is the first commit: the CMSDK UART latches
INTSTATUS.RXonly at the instant a byte arrives and only ifCTRL.RxIntEnis set right then, so clearingRxIntEnacross the client callback loses any byte that lands in that window — and, because the device stops accepting input while its holding register is full, every byte after it. Driving the AN385 process console over a TCP serial, the console stops answering after five short bursts and never recovers.Verified, not just read:
process_consoleline-buffer overflow at burst 65 — that one reproduces identically onqemu_rv32_virt, so it is a pre-existing capsule bug, not this board's, and is left alone here).main.rsatqemu_arm_mps2_an385::CortexM3now fails the link with "AN386 vector table missing", where before it linked cleanly with Cortex-M3 handlers in a Cortex-M4 image.c_helloandspi_loopback(SPI PASS) run on an385,c_helloon an386;blinkstill cycles LED0 through multiple states, so the timer change did not break the alarm path.make format-check,make ci-job-syntax,make ci-job-clippy,make licensecheck,tools/ci/check-boards-readme.pyall pass.Not verified: no real MPS2 hardware; register semantics are checked against QEMU 10.2.1's device models, not ARM DDI0479C. Alarm accuracy is not measured — these boards do not use
-icount, so timing claims under QEMU would be meaningless. No QEMU other than 10.2.1 was exercised, which is whyboards/README.mdnow claims no minimum version rather than a new one.Left out deliberately, as design questions rather than fixes: the duplication between the two per-image vector-table crates, the
early_init/finish_starttwo-call seam, package naming, and adding an385 toci-job-clippy's per-arch list.