-
Notifications
You must be signed in to change notification settings - Fork 2.1k
sam0_common flash write not working at least for SAML21J18B #10068
Description
Description
The flashpage_write function doesn't seem to work correctly for the sam0/SAML21J18B (on the reference saml21-xpro board).
Basically when testing the OTA branch I needed to write to flash and the function would always write wrong data, as the flashpage_write_and_verify memory comparison would fail.
I did quite some investigation and I think I understood the issue and I have a working patch now, but I'm not sure how this could be solved better and generically.
What I see is:
/* a flashpage in RIOT is mapped to a flash row on the SAM0s */
#define FLASHPAGE_SIZE (256U)
/* one SAM0 row contains 4 SAM0 pages -> 4x the amount of RIOT flashpages */
#define FLASHPAGE_NUMOF (FLASH_NB_OF_PAGES / 4)
and then the flashpage_write:
flashpage_write_raw(page_addr, data, FLASHPAGE_SIZE);
And flashpage_write_raw does a write:
NVMCTRL->CTRLA.reg = (NVMCTRL_CTRLA_CMDEX_KEY | NVMCTRL_CTRLA_CMD_PBC);
for (unsigned i = 0; i < len; i++) {
*dst++ = *data_addr++;
}
NVMCTRL->CTRLA.reg = (NVMCTRL_CTRLA_CMDEX_KEY | NVMCTRL_CTRLA_CMD_WP);
So tries to write 256 bytes in a single page buffer. But the page of the CPU is actually 64 bytes as by datasheet (document 60001477A, table 11-2) or as the CPU register itself states if we print it:
NVMCTRL.PARAM
psz = 3
nvmp = 4096
Where psz = 3 means 64 (datasheet page 444). Also if you look at NVMCTRL_PAGE_SIZE in the Atmel supplied header, you can see that it is indeed 64.
This attempt fails and just the last 64 bytes are written.
What I did is change the single flash write to a loop. Knowing that a RIOT page is 4 CPU pages (a row) I changed the single write I mention before to:
for (int curpage=0;curpage<4;curpage++) {
flashpage_write_raw(page_addr+(curpage*NVMCTRL_PAGE_SIZE/4), data+(curpage*NVMCTRL_PAGE_SIZE), NVMCTRL_PAGE_SIZE);
}
Basically a "RIOT page" write (long 256) becomes a loop of 4 CPU pages written. The erase always erases a row so that is fine as it was.
With this modification the code works very well. But, apart for purely esthetic things, I'm not sure how generic this solution is, so would appreciate some inputs from you.
If deemed fine I'd be glad do a PR of course.
Steps to reproduce the issue
Try flashing a page with some data and verify on the saml21-xpro board.
Expected results
Data correctly written.
Actual results
Data is just partially written, most of the page being 0xff from the erase.