search

sipeed / bl602-hal

Hardware Abstract Layer for BL602 RISC-V WiFi + BLE SoC in embedded Rust
Other
74 stars 14 forks source link

Timers should be updated to allow any Duration instead of just Milliseconds #36

Closed kiyoshigawa closed 2 years ago

kiyoshigawa commented 2 years ago

The current implementation of timer.rs only allows you to set times in units of integer numbers of Milliseconds() in the set_match#() functions. Since you can configure the timer clocks to tick as quickly as 160MHz, the millisecond resolution is inadequate for using the timers as faster time scales.

I propose to instead implement time value passed into these functions as a generic embedded-time::Duration, which will allow for time scales from nanoseconds all the way up to hours, instead of explicitly using Milliseconds for these values. The set_match#() functions would then need to verify that the specified time works out to be evenly divisible by a number of the timer's currently configured clocks.

For instance, if you set the clock value to be 1000ns (or 1us), with the clock configured to its maximum 160MHz (6.5ns per clock), it would not be allowed because 1000ns isn't evenly divisible by the clock period, however if you set it to 650ns it would be allowed because that time value evenly divides into 100 clock cycles. This setup would be similar to how the set_clock_source() function checks that the target clock value is evenly divisible by the configured divider value, and panics when it is not.

kiyoshigawa commented 2 years ago

It turns out in my proposed solution, I just used the previous integer math with a couple modifications, so everything truncates to the nearest whole clock tick. No need for the clock alignment checks I mentioned earlier, and no panics either.

kiyoshigawa commented 2 years ago

While working on the Watchdog timer, I noticed a couple places I missed adding generics time support in the timers.rs file. Please leave this issue open until I can submit a second PR to add generic support to ConfiguredTimerChannel#::current_time(), and to CountDown::start().

kiyoshigawa commented 2 years ago

The fixes for the missing functions that still used Milliseconds were included in the PR for the watchdog timer. This issue is now resolved, and all timer functions now support generic time unit inputs.