sam0 flashpage: wait for READY bit in INTFLAG after write command

[fedepell]

Contribution description

While debugging on an issue with RWWEE (see devel mailing list) I saw in the SAML manual:

Procedure for Manual Page Writes (CTRLB.MANW=1)
The row to be written to must be erased before the write command is given.
• Write to the page buffer by addressing the NVM main address space directly
• Write the page buffer to memory: CTRL.CMD='Write Page' and CMDEX
• The READY bit in the INTFLAG register will be low while programming is in progress, and access
through the AHB will be stalled

And noticed that the last point, waiting for the READY bit in INTFLAG, was actually missing in current code (while it is done correctly for example for the erase cycle).
While I didn't notice myself any problem until now with the code without this READY bit waiting, I believe it may be more correct to add this code and prevent possible race conditions and strange errors.

Testing procedure

On SAMx boards you can run the automated test in tests/periph_flashpage.

[fedepell]

From datasheet also:

Any commands written while INTFLAG.READY is low will be ignored.

[fedepell]

As an additional information: I've put also a counter inside that while to see if it gets indeed triggered and for how much time. On my saml21-xpro for the main flash it usually passes inside just once, for the RWWEE about 300 times. So while for main flash it doesn't seem to be somehow vital, for RWWEE it seems to be. Anyhow given it's in the specs it may be still wise to always use it probably.

[dylad]

On my saml21-xpro for the main flash it usually passes inside just once, for the RWWEE about 300 times. So while for main flash it doesn't seem to be somehow vital, for RWWEE it seems to be

This is a bit weird, isn't it ? I mean, according to the datasheet this is supposed to be the same flash but smaller.
I'm wondering if it's not related to the wait states. Changing CPU frequency or Power-level have an impact to the flash.

I'll try to test your PR on SAML21 and SAML10/SAML11 soon. I'll also check with different wait states to see if there is a difference but I'll be quite busy this week.

Anyway, I agreed with you. This check must be there even "if it works" now.

[fedepell]

I was quite surprised by the difference (almost instant vs 300) I must admit!

While I don't know how it is internally managed, I could try to explain to myself the difference by the fact that the RWWEE must have a quite more complex logic (to handle this "read while write" thing, so must somehow be allowing the main flash to be reading at the same time if needed) or also by the fact that the main flash has a cache while this one not.
Of course they are just conjectures to try to explain the measurement, will try to dig into the datasheet more (even if from what I looked so far there were no details on these things)

[keestux]

Something else I noticed in the SAMD manual.

To issue a command, the CTRLA.CMD bits must be written along with the CTRLA.CMDEX value. When a command is issued, INTFLAG.READY will be cleared until the command has completed. Any commands written while INTFLAG.READY is low will be ignored.

That raises the question whether we should worry about the INTFLAG.READY bit for any CMDEX command, including PBC.

[keestux]

Have a look at the nvm driver in ASF, sam0/drivers/nvm/nvm.c. They do a wait for NVM ready after every CMDEX command.

BTW. I suggest we create a static inline function that waits for the ready bit. That way there is only one place where we need the #ifdef and the while

[keestux]

For example

static inline bool nvm_is_ready() __attribute__((always_inline));
static inline bool nvm_is_ready()
{
#ifdef CPU_SAML1X
    return _NVMCTRL->STATUS.bit.READY;
#else
    return _NVMCTRL->INTFLAG.bit.READY;
#endif
}

And to use it:

    _NVMCTRL->CTRLA.reg = (NVMCTRL_CTRLA_CMDEX_KEY | NVMCTRL_CTRLA_CMD_WP);
    while (!nvm_is_ready()) {}

[fedepell]

Thanks for the hint, that looks good! I would even put the while then in the inline so it becomes even cleaner maybe, try to check my lastest commit.

I also added the wait after PBC as you suggest as the ASF does.

[dylad]

Tested lastest changes on SAML10-XPRO.
You have my ACK too.