Hi, I've been looking into the recently released KBOOT code. It installs as FSL_Kinetis_Bootloader_1_0_2. Just wanted to mention that the microseconds_delay function in src/microseconds/src/microseconds_sysclk.c is wrong. The function in question is:
//! @brief Delay specified time
//!
//! @param us Delay time in microseconds unit
void microseconds_delay(uint32_t us)
{
uint32_t currentTicks = microseconds_get_ticks();
//! The clock value in Mhz = ticks/microsecond
uint64_t ticksNeeded = (us * s_tickPerMicrosecondMul8 / 8) + currentTicks;
while(microseconds_get_ticks() > ticksNeeded)
{
;
}
}
The check "while(microseconds_get_ticks() > ticksNeeded)" is backwards I believe and unsafe too. It's unsafe because when the get_ticks value is in the "roll over" region and the check is done right (ie microseconds_get_ticks() < ticksNeeded) you could have cases where the test "misses" and the delay goes for a very long time. Correct code does a signed subtraction and checks that result.
It seems like this delay function isn't actually used in any of the loader code so the error isn't actually hurting anything. But having a broken function just begs problems in the future if someone goes to use the function. So I figured I would report it. I'm just starting to look at the loader code to adapt it to my project and figured I would just see what the code looked like. Doesn't give me a warm fuzzy feeling that one of the first things I happened to look at had a bug. But, hey, it's probably the only one ;-).
Just thought I'd alert the maintainers so they might fix it.
已解决! 转到解答。
Hi Fred,
The reversed conditional test has been fixed for the next KBOOT release. Thanks for pointing it out!
Best Regards,
Mike
Hey folks, just an update. So, I'm not used to using 64 bit values on small MCUs. The delay function uses such values in the time comparison code so I guess we wouldn't have to worry about time value roll over issues and problems with them if it used the values right. If the current_ticks value was declared uint64_t to match microseconds_get_ticks then I think the only other fix needed is to reverse the conditional test.
Fred