delay limited to frequencies over 1MHz

[nebelgrau77]

Current implementation of delay asserts that the clock frequency is > 1_000_000 Hz, which limits its usage with MSI as clock source to Range4..Range6.

let freq = clocks.sys_clk().0;
assert!(freq > 1_000_000_u32);
let ticks_per_us = freq / 1_000_000_u32;

I understand that this is because otherwise we'd have less than 1 tick per microsecond, e.g. with Range0 at 65 kHz.

Would it make sense to add another method, e.g. LPDelay as in LowPower, with resolution expressed in ticks_per_ms and only DelayMs implemented?

[hannobraun]

I don't think that adding a new API for low-power mode delays would be a good solution, as the current API is implementing standard APIs from the embedded-hal project. It would be unfortunate if these were not available in low-power modes.

I think the most practical solution is to remove that assert and make sure the rest of the code is smart enough to handle low frequencies. I think it's fine to lose precision in that case, as long as that's clearly documented.

[nebelgrau77]

First of all I need to say that I'm not enough of an embedded Rustacean to code it myself, in fact I might be getting it all wrong :) But this line:

let ticks_per_us = freq / 1_000_000_u32;

that's integer division, right? The result will be u32, so if the frequency drops below 1MHz it will be just 0 ticks per microsecond, unless I'm getting it all wrong?

[hannobraun]

Yes, I believe that's correct. That's why I said we need to make sure the code is smart enough to handle it, because the line you quoted certainly isn't :-)

I think we need to store freq in the struct instead of ticks_per_us, then compute the number of ticks in delay_us.

If we calculated total_rvr like this:

let total_rvr = self.freq * us / 1_000_000;

Then it should work for low frequencies. There's a danger of an overflow from the multiplication though. Not sure what the best way to handle that is. Maybe cast to u64 for the calculation? Or maybe the u32 is already big enough, since the SysTick doesn't use the full range of u32 anyway?

I don't know. This requires some more thinking.