FLASH_EepromWrite in MCUXpresso MKE02Z64 SDK

[justusandjustic]

In the fsl_flash.c library in the MKE02z64 SDK, why does the FLASH_EepromWrite function throw an InvalidArgument status when given more than 4 bytes?  Some of the code is written as if it assumes you must only give it 4 bytes, but it's almost written such that it can accept any number of bytes (within a valid range). 

It starts by checking that there are not more than 4 bytes given

if ((lengthInBytes > 4UL) || (0UL == lengthInBytes))

{

    return kStatus_FLASH_InvalidArgument;

}

then enters a while loop if there are more than 3 bytes. But why is this a "while" loop, instead of an "if" statement, if we KNOW we're never going to his this loop more than once (because we know  we will only ever have either 4 bytes, or fewer than 4 bytes)?

If we simply remove the check for more than 4 bytes, and change this:

for (i = 0UL; i < lengthInBytes; i++

{

    flash_set_command(0x02UL + i, *src++, 0UL);

}

to this:

for (i = 0UL; i < 4; i++

{

    flash_set_command(0x02UL + i, *src++, 0UL);

}

we could send any number of bytes, and it would loop through internally, instead of having to call this function in a loop.

EXCEPT that, there is seems to be another error.  As we loop through each byte in the above "for" loop, we increment src.  By the time we are done, src will have incremented by 4.  However,  after we finish doing that (and checking that there are no other errors), we hit this code:

/* update start address for next iteration */

start += 4U;

src += 4U           

/* update lengthInBytes for next iteration */

lengthInBytes -= 4UL;

Which ends up incrementing src by an additional 4.

So if we remove the first check from more than 4 bytes, make the change to the bounds of the "for" loop as suggested above, and get rid of the redundant incrementation of src at the end, we should be able to send any number of bytes (within a valid range) and the code will handle it until all bytes are written.  I have, in fact, made this changes myself and it seems to work just fine.  The code even states that it is incrementing/decrementing variables for "next iteration", which (if we only accept 4 bytes), we know is never going to happen.

Am I missing something?  Is there a reason this was not done?

-Josiah

[nxf56274]

Hi,

This function has a mistake. The "if ((lengthInBytes > 4UL) || (0UL == lengthInBytes))" should be modified as "if ((lengthInBytes&0x03 != 0UL) || (0UL == lengthInBytes))". We shouldn't limit the lengthInBytes. But the lengthInBytes should be aligned with 4 bytes.

Have a great day,
TIC

-------------------------------------------------------------------------------
Note:
- If this post answers your question, please click the "Mark Correct" button. Thank you!

- We are following threads for 7 weeks after the last post, later replies are ignored
Please open a new thread and refer to the closed one, if you have a related question at a later point in time.
-------------------------------------------------------------------------------

[justusandjustic]

Da Li,

Thanks for the response.

If we can only ever write in increments of 4, why is there an if statement after the while statement that seems to be for writing any bytes after the lengthInBytes is no longer greater than 3:

    while (lengthInBytes > 3UL)

    {

       …

    }   

    if ((lengthInBytes > 0UL) && (kStatus_FLASH_Success == returnCode))

    {

        /* pass paramters to FTMRx */

        FTMRx->FSTAT = FTMRx_FSTAT_ACCERR_MASK | FTMRx_FSTAT_FPVIOL_MASK;

        /* Write index to specify the command code to be loaded */

        flash_set_command(0UL, start >> 16UL, FTMRx_PROGRAM_EEPROM);

        flash_set_command(1UL, start, start >> 8UL);

        /* Write index to specify the byte (MSB word) to be programmed */

        for (i = 0UL; i < lengthInBytes; i++)

        {

            flash_set_command(0x02UL + i, *src++, 0UL);

        }

        /* calling flash command sequence function to execute the command */

        returnCode = flash_command_sequence(config);

    }

And what about the other two mistakes I mentioned:

1. The redundant incrementation of src  

2. The for loop that goes from 0 to lengthInBytes, instead of 0 to 4.

  Are these not also mistakes?

-Josiah

[nxf56274 · Accepted solution]

Hi,

I think you are right. This function made many mistakes. And you solution is right.

Have a great day,
TIC

-------------------------------------------------------------------------------
Note:
- If this post answers your question, please click the "Mark Correct" button. Thank you!

- We are following threads for 7 weeks after the last post, later replies are ignored
Please open a new thread and refer to the closed one, if you have a related question at a later point in time.
-------------------------------------------------------------------------------

[justusandjustic]

Thanks for confirming!

Is there somewhere else I should post this as a bug-fix?

-Josiah