Skip to content

extra: add RP2350 RISC-V machine timer timing provider - #166

Open
tobiaguiar08 wants to merge 4 commits into
ventZl:masterfrom
tobiaguiar08:riscv_pico_timing_provider_library
Open

tobiaguiar08 wants to merge 4 commits into
ventZl:masterfrom
tobiaguiar08:riscv_pico_timing_provider_library

Conversation

@tobiaguiar08

Copy link
Copy Markdown
Contributor

Summary

Add aux_riscv_pico_timer, a ready-to-use timing provider library for RP2350 RISC-V that generates the kernel tick from the RISC-V machine timer. RISC-V applications no longer need to write their own timing provider. The PR also fixes problems found by following the RP2350 RISC-V getting-started guide end to end.

Problem

RISC-V applications on RP2350 have no timing provider library. The getting-started guide embeds an implementation that every integrator must copy into their project. That copy also enables the timer interrupt during timing_provider_setup() and ignores the kernel's stop request (delay_us == 0) in timing_provider_schedule().

Changes

extra: add RP2350 RISC-V machine timer timing provider

  • New library aux_riscv_pico_timer (src/extra/riscv_pico_timer.c, include/extra/riscv_pico_timer.h). It only builds when CMRX_ARCH=riscv and PICO_PLATFORM=rp2350-riscv; otherwise CMake prints a status message and skips it.
    • timing_provider_setup() only stores the tick period.
    • timing_provider_schedule() disables the machine timer interrupt for delay_us == 0. Otherwise it re-arms mtimecmp from the current mtime and enables the interrupt.
    • timing_provider_delay() busy-waits on mtime, with no pico_time/hardware_timer dependency.
    • cmrx_machine_timer_handler() is called by the existing quirks/pico-sdk-riscv machine timer ISR. It re-arms the timer and calls os_sched_timing_callback().
  • The getting-started guide and the library overview now use the library instead of embedding an implementation. Every target_link_libraries line in the RISC-V guide links it.

riscv: note timing provider use of HAL CSR accessors

  • Comment-only update to include/cmrx/arch/riscv/hal.h. It said the CSR accessors exist only for the thread switcher, but the new library uses the mie accessors too.

docs: fix markup rendering in RP2350 RISC-V getting-started guide

  • A warning in "Linking CMRX to the project" rendered with literal **, because Doxygen does not open bold emphasis directly before a code span.
  • A note in the step that selects the RISC-V core rendered with literal backticks, because Doxygen does not format a code span wrapped in double quotes.

docs: clarify GDB flashing steps in RP2350 RISC-V getting-started guide

  • The guide's command now names a RISC-V-capable GDB. It also lists three things a reader may see while flashing: a first-connect vMustReplyEmpty error (retry), the run confirmation prompt (answer y), and OpenOCD keep_alive() warnings during load.

Notes for review

  • The tick period is fixed at setup, so the kernel's tickless mode is not used. delay_us only distinguishes stop (0) from run.
  • cmrx_machine_timer_handler() is declared in the public header so its definition is checked against a prototype. It is meant to be called by the quirk ISR, not by applications.
  • The header redeclares timing_provider_setup/schedule/delay without doc comments, as extra/systick.h does. Doxygen merges identically named declarations across headers, so documenting them again would leak text into the aux_systick and timing API pages. The group description documents setup instead.
  • There is no host unit test for the library yet.

Testing

Host: Ubuntu 24.04, Pico SDK 2.3.1, riscv32-corev-elf-gcc 14.1, OpenOCD built from source (cd4873400), Raspberry Pi Debug Probe, Pico 2.

  • Getting-started guide, end to end: I created the project from the guide's own code listings and applied its edits in order, with cmrx pointing at this branch. It builds with cmake -B build && cmake --build build.
    • I flashed it with the guide's OpenOCD and GDB target extended-remote / load / run steps, using gdb-multiarch. The LED blinks at about 0.5 s, and blink prints on UART.
    • That run used the guide text before the last commit. The last commit documents what it hit: one connect retry and the run prompt.
    • Before the guide fix in the first commit, following the guide literally failed to link, with undefined timing_provider_* symbols.
  • Documentation: rendered with the repository Doxyfile (Doxygen 1.9.8).
    • The warning set is identical to master.
    • The text of the aux_systick and timing API group pages is unchanged.

Generated with Claude Code

RISC-V applications on RP2350 have no timing provider library and must
copy one from the getting-started guide into every project. The copy in
the guide also enables the timer interrupt during setup and ignores the
kernel's request to stop the timer when it passes a zero delay.

Add aux_riscv_pico_timer, a ready-to-use timing provider that generates
the kernel tick from the RP2350 RISC-V platform timer. It honors the
kernel's stop request. The tick period is fixed at setup, so the
kernel's tickless mode is not used. The library is only available when
building for the rp2350-riscv Pico SDK platform.

Update the getting-started guide and the library overview to use the
library instead of embedding the implementation.

Assisted-by: Claude [claude-opus-5]
Signed-off-by: Tobias Aguiar <tobi.aguiar01@gmail.com>
The HAL header states that its CSR accessors exist only for the thread
switcher. The RP2350 timing provider now also uses them to control the
machine timer interrupt, so the statement no longer holds and could
mislead anyone trimming the accessor set.

Update the scope comments to cover timing providers.

Assisted-by: Claude [claude-opus-5]
Signed-off-by: Tobias Aguiar <tobi.aguiar01@gmail.com>
Two passages in the RP2350 RISC-V guide render their markup literally.
A warning in the "Linking CMRX to the project" section shows asterisks
instead of bold text, because Doxygen does not start bold emphasis when
the opening marker is followed by a code span. A note in the step that
selects the RISC-V core shows backticks instead of code, because
Doxygen does not format a code span wrapped in double quotes.

Start the warning with a plain word and drop the quotes around the code
span so both passages render as intended.

Assisted-by: Claude [claude-opus-5]
Signed-off-by: Tobias Aguiar <tobi.aguiar01@gmail.com>
Following the RP2350 RISC-V guide's flashing steps as written hits
several problems the guide does not mention. The GDB command it shows
may not support RISC-V, the first connection to OpenOCD can fail with a
protocol error, and run stops at a confirmation prompt.

Name a RISC-V-capable GDB in the command, and list the retry, the prompt
answer and the OpenOCD warnings seen during load.

Assisted-by: Claude [claude-opus-5]
Signed-off-by: Tobias Aguiar <tobi.aguiar01@gmail.com>
Comment thread src/extra/CMakeLists.txt
hardware_riscv_platform_timer
)
else()
message(STATUS "aux_riscv_pico_timer: only available for PICO_PLATFORM=rp2350-riscv")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably isn't needed. Ignore this case silently.

Comment thread man/03_getting_started.md
run
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

While running these commands, you may see the following:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd instruct your AI to not turn this howto into tutorial of using GDB with OpenOCD :)

Comment thread man/03_getting_started.md

Create `src/timing_provider.h`:
The library implements the timing provider interface the kernel calls into -
`timing_provider_schedule()` and `timing_provider_delay()` - and `cmrx_machine_timer_handler()`,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

People reading this probably don't care what it does as long as it works. Too much detail.

uint64_t now = riscv_timer_get_mtime();
uint64_t end = now + (uint64_t)delay_us;

while (end > now) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In rare conditions, this can overflow. mtime may be shortly before overflowing and end may become number actually numerically lower than now, so the while loop below will terminate prematurely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants