embassy icon indicating copy to clipboard operation
embassy copied to clipboard

`embassy-time` `std` driver has fixed tick rate of 1 MHz, which conflicts at the workspace level with other time drivers (particularly for Rust Analyzer)

Open reivilibre opened this issue 7 months ago • 8 comments
trafficstars

If we have 3 crates in a workspace, one of them a simulator (which uses the std feature flag of embassy-time), one of them a firmware implementation for our target chip (assuming it doesn't use 1 MHz as its tick rate, but e.g. 32768 Hz) and one of them a common library used for both, it becomes awkward to work on the project because if you cargo check the entire workspace, it fails to compile with a conflicting TICK_HZ definition.

You can work on an individual crate, e.g. by cding to it first or cargo check -p myproject_simulator, but I haven't found a similar workaround for Rust Analyser, which means the IDE is almost useless in such a project.

I'm not sure about a general-case fix for this, but one thing that might soften it would be to allow the std time driver to use an arbitrary tick rate.

reivilibre avatar Apr 14 '25 09:04 reivilibre

Do not create a Cargo workspace mixing embedded and non-embedded, it simply doesn't work. Keep it as separate crates.

For IDE support you can use linkedProjects to decide which crate becomes "active", this makes rust-analyzer work in it and all its deps. See how this repo does it.. If you want to work on both at the same time you can create two .code-workspace files with different linkedProjects settings and open them in two separate windows.

Dirbaio avatar Apr 14 '25 09:04 Dirbaio

What Dirbaio said seems like the best solution.

However a hack to get around the issue is to add the following (replacing the target with yours) to your IDE settings configuration:

"rust-analyzer.check.allTargets": false,
"rust-analyzer.cargo.target": "thumbv7em-none-eabihf"

Only issue I’ve found it rust analyzer doesn’t work on host based tests, but that might be fixable. I haven’t looked into it yet.

ArchieAtkinson avatar Apr 17 '25 21:04 ArchieAtkinson

I'm also running into this issue with a pure-embedded-crate workspace.

It supports 3 "platforms" (actually 2 platforms, one with 2 ISA's)--how should I specify the tick rate (the tick rate in my project is the clock rate, none of which are 1MHz):

[dependencies]
cortex-m-rt = { version = "0.7.5", optional = true }
defmt = "1.0.1"
defmt-rtt = "1.0.0"
embassy-executor = { version = "0.7.0", optional = true, features = [
    "defmt",
    "executor-interrupt",
    "executor-thread"
] }
embassy-rp = { version = "0.4.0", optional = true, default-features = false, features = [
    "critical-section-impl",
    "defmt",
    "rt",
    "time-driver",
    "unstable-pac",
] }
embassy-time = { version = "0.4.0", optional = true, features = ["defmt"] }
panic-probe = { version = "1.0.0", features = ["print-defmt"] }
riscv-rt = { version = "0.14.0", optional = true }

[features]
board_rpi_pico_w = [
    "cortex-m-rt",
    "embassy-executor/arch-cortex-m",
    "embassy-rp/rp2040",
    "embassy-time/tick-hz-133_000_000",
]
board_rpi_pico_2_w_cortex_m = [
    "cortex-m-rt",
    "embassy-executor/arch-cortex-m",
    "embassy-rp/rp235xa",
    "embassy-time/tick-hz-150_000_000",
]
board_rpi_pico_2_w_risc_v = ["riscv-rt",
    "embassy-executor/arch-riscv32",
    "embassy-rp/rp235xa",
    "embassy-time/tick-hz-150_000_000",
]
# ...

The error message I'm getting when compiling e.g. --features board_rpi_pico_2_w_cortex_m:

error[E0428]: the name `TICK_HZ` is defined multiple times
   --> /Users/sergibli/.cargo/git/checkouts/embassy-312bdb1c9e0ee5b8/34b6a51/embassy-time-driver/src/tick.rs:192:1
    |
84  | pub const TICK_HZ: u64 = 1_000_000;
    | ----------------------------------- previous definition of the value `TICK_HZ` here
...
192 | pub const TICK_HZ: u64 = 150_000_000;
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `TICK_HZ` redefined here
    |
    = note: `TICK_HZ` must be defined only once in the value namespace of this module

I'm thinking about submitting another MR, but want to be careful not break existing workflows.

Is there any supported way to enable alternative tick speeds via features? Hopefully, yes.

If not, I'd need to make changes to the supported features of embassy-rp by augmenting the gen_tick.py script to also generate for embassy-rp.

Lmk if you can think of a smarter way--this idea seems pretty heavyweight to just have embassy-rp transitively pass through the selected embassy-time tick-* feature to embassy-time.

U007D avatar Apr 30 '25 23:04 U007D

@U007D you're running into a different issue: embassy-rp fixes the tick rate to 1Mhz because that's hardcoded in hardware, you can't make the rp2040 timer work at a different freq. https://github.com/embassy-rs/embassy/blob/52e4c7c30c5f59d10afbef2447b96da68f4be0bc/embassy-rp/Cargo.toml#L45

I'm not sure if rp235x is more flexible in this regard, if it is then the solution would be to make embassy-rp not set the tick rate and let the user set it instead.

Dirbaio avatar May 01 '25 12:05 Dirbaio

Hi, @Dirbaio, that's interesting.

I've got some time blocked off tomorrow to get to the bottom of this. My understanding is the RP2350 can set the clock source to count clock cycles (and maybe more, I don't know--I was only looking to count clock cycles). I'll verify this for the RP2350 and will confirm for the RP2040 tomorrow as well and will write back here.

Based on those findings, your advice on what should change in embassy and how it should change will be invaluble.

I'm happy to put together an MR making those changes.

U007D avatar May 02 '25 05:05 U007D

Hi, @Dirbaio,

Update: [RESOLVED] I've made the change you suggested and added a feature for this mode on the RP2350. Feel free to skip this response.

Summary

Based on the research below,

  • The PR2040 TIMER frequency 1MHz (nominal) from a fixed source.
  • The RP2350's TIMERx can be set to either: i) a 1MHz (nominal) source or ii) the system clock--a 150MHz (nominal)--source.

I am using ii), this latter feature.

How would you like to see this capability reflected in embassy-rp's API?

Note the RP2350 provides two timers (TIMER0 and TIMER1). For my needs, a single embassy-rp feature which configures both to the same SOURCE will suffice, but would you like the timers to be independently configurable as the hardware supports this?

As a user, I would like to express (via embassy-rp's Cargo features) that I would like 150MHz ticks. That should only even be an option when using an RP2350, and would drive settings into embassy-timer and embassy-timer-driver's respective tick-related Cargo features.

--

Research

I've been able to confirm the following:

RP2040: TIMER is a virtual 64-bit register (two latched 32-bit registers) incrementing at 1MHz (nominal) frequency. Its source cannot be changed. Source: RP2040 Datasheet §4.6.1, p.534:

4.6.1. Overview The system timer peripheral on RP2040 provides a global microsecond timebase for the system, and generates interrupts based on this timebase. It supports the following features: • A single 64-bit counter, incrementing once per microsecond • This counter can be read from a pair of latching registers, for race-free reads over a 32-bit bus. • Four alarms: match on the lower 32 bits of counter, IRQ on match. The timer uses a one microsecond reference that is generated in the Watchdog (see Section 4.7.2), and derived from the reference clock (Figure 28), which itself is usually connected directly to the crystal oscillator (Section 2.16). The 64-bit counter effectively can not overflow (thousands of years at 1MHz), so the system timer is completely monotonic in practice.

RP2350: TIMER0 and TIMER1 are independent virtual 64-bit registers (two latched 32-bit registers each) incrementing at either the traditional 1MHz (nominal) frequency or at the full system clock speed, currently 150MHz (nominal), selectable via setting TIMERx's new source register. Source RP2350 Datasheet §12.8.1.1, p.1179:

12.8.1.1. Changes from RP2040 • RP2350 now has two timer instances: TIMER0 and TIMER1 • On RP2350, the tick source for each timer comes from the system-level tick generators (see Section 8.5) • RP2350 added two new registers: LOCKED is used to disable write access to the timer, and SOURCE allows the timer to count system clock cycles rather than a 1 μs tick

U007D avatar May 02 '25 14:05 U007D

I think I have a solution which is much lighter touch than I was expecting. I'm testing it now, but wanted to let you know to save you time thinking up a design if this one works for you. If it works as hoped, I'll submit it to embassy-rp as an MR.

U007D avatar May 02 '25 17:05 U007D

@Dirbaio,

The changes seem to work. But I am concerned that removing the tick-hz-1_000_000 from the time-driver feature may break existing users. Is there a default I should add back in somewhere or does its removal not break existing users?

I'll submit the MR, with this concern expressed in the description.

U007D avatar May 02 '25 19:05 U007D