AnsweredAssumed Answered

Bug in KBOOT code

Question asked by Fred Roeber on Sep 14, 2014
Latest reply on Sep 17, 2014 by Michael Minnick

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.

Outcomes