Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

periph/flashpage: extend API #16972

Merged
merged 7 commits into from
Oct 26, 2021
Merged

periph/flashpage: extend API #16972

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
72d4701
periph/flashpage: extend API
Ollrogge Oct 19, 2021
d38f6c6
cpu/cortexm_common: extend flashpage API
Ollrogge Oct 19, 2021
741943f
cpu/msp430_common: extend flashpage API
Ollrogge Oct 25, 2021
3a11b1f
cpu/riscv_common: extend flashpage API
Ollrogge Oct 19, 2021
4fa25af
cpu/lpc23xx: extend flashpage API
Ollrogge Oct 19, 2021
cdd9f30
cpu/native: extend flashpage API
Ollrogge Oct 19, 2021
be254d4
cpu/cc2538: adjust rom size to account for cca region
Ollrogge Oct 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
periph/flashpage: extend API
  • Loading branch information
@Ollrogge
Ollrogge committed Oct 19, 2021
commit 72d47013ddf2a439440631f4e04eec6ffec603d9
10 changes: 10 additions & 0 deletions drivers/include/periph/flashpage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,16 @@ unsigned flashpage_page(const void *addr);
*/
void flashpage_erase(unsigned page);

/**
* @brief Get number of first free flashpage
*/
unsigned flashpage_first_free(void);

/**
* @brief Get number of last free flashpage
*/
unsigned flashpage_last_free(void);
Ollrogge marked this conversation as resolved.
Show resolved Hide resolved

/**
* @brief Write the given page with the given data
*
Expand Down
41 changes: 19 additions & 22 deletions tests/periph_flashpage/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,6 @@

#define LINE_LEN (16)

/* For MSP430 cpu's the last page holds the interrupt vector, although the api
should not limit erasing that page, we don't want to break when testing */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still true?

Copy link
Member Author

@Ollrogge Ollrogge Oct 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you are referring to "although the api should not limit erasing that page" ? If using flashpage_last_free() as page number, it will erase the page before the interrupt vector and therefore limit the erasure of this page.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you keep this comment in the MSP430 code you specify last_free? Otherwise, I tested that the test still works on z1, nice to have this feature~

Copy link
Member Author

@Ollrogge Ollrogge Oct 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify: you mean in cpu/msp430_common/periph/flashpage.c ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I believe that is where you specify flashpage_last_free no?, and from that you are substracting the page holding the vector table, so it would make sense to add a comment there, in the lines of:

/* MSP430 cpu's  last page holds the interrupt vector, so the last free page is the one before last */

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I just wasn't quite sure if you wanted to have the comment in the tests or in the function implementation code.
Done :)

#if defined(CPU_CC430) || defined(CPU_MSP430FXYZ)
#define TEST_LAST_AVAILABLE_PAGE (FLASHPAGE_NUMOF - 2)
#else
#define TEST_LAST_AVAILABLE_PAGE (FLASHPAGE_NUMOF - 1)
#endif

/* When writing raw bytes on flash, data must be correctly aligned. */
#define ALIGNMENT_ATTR __attribute__((aligned(FLASHPAGE_WRITE_BLOCK_ALIGNMENT)))

Expand Down Expand Up @@ -115,24 +107,27 @@ static int cmd_info(int argc, char **argv)
(void)argc;
(void)argv;

printf("Flash start addr:\t0x%08x\n", (int)CPU_FLASH_BASE);
printf("Flash start addr:\t\t0x%08x\n", (int)CPU_FLASH_BASE);
#ifdef FLASHPAGE_SIZE
printf("Page size:\t\t%i\n", (int)FLASHPAGE_SIZE);
printf("Page size:\t\t\t%i\n", (int)FLASHPAGE_SIZE);
#else
puts("Page size:\t\tvariable");
puts("Page size:\t\t\tvariable");
#endif
printf("Number of pages:\t%i\n", (int)FLASHPAGE_NUMOF);
printf("Number of pages:\t\t%i\n", (int)FLASHPAGE_NUMOF);

#ifdef FLASHPAGE_RWWEE_NUMOF
printf("RWWEE Flash start addr:\t0x%08x\n", (int)CPU_FLASH_RWWEE_BASE);
printf("RWWEE Number of pages:\t%i\n", (int)FLASHPAGE_RWWEE_NUMOF);
printf("RWWEE Flash start addr:\t\t0x%08x\n", (int)CPU_FLASH_RWWEE_BASE);
printf("RWWEE Number of pages:\t\t%i\n", (int)FLASHPAGE_RWWEE_NUMOF);
#endif

#ifdef NVMCTRL_USER
printf("AUX page size:\t%i\n", FLASH_USER_PAGE_AUX_SIZE + sizeof(nvm_user_page_t));
printf(" user area:\t%i\n", FLASH_USER_PAGE_AUX_SIZE);
printf("AUX page size:\t\t%i\n", FLASH_USER_PAGE_AUX_SIZE + sizeof(nvm_user_page_t));
printf(" user area:\t\t%i\n", FLASH_USER_PAGE_AUX_SIZE);
#endif

printf("Number of first free page: \t%u \n", flashpage_first_free());
printf("Number of last free page: \t%u \n", flashpage_last_free());

return 0;
}

Expand Down Expand Up @@ -349,6 +344,7 @@ static int cmd_test_last(int argc, char **argv)
(void) argc;
(void) argv;
char fill = 'a';
unsigned last_free = flashpage_last_free();

for (unsigned i = 0; i < sizeof(page_mem); i++) {
page_mem[i] = (uint8_t)fill++;
Expand All @@ -357,9 +353,9 @@ static int cmd_test_last(int argc, char **argv)
}
}
#if defined(CPU_CC430) || defined(CPU_MSP430FXYZ)
printf("The last page holds the ISR vector, so test page %d\n", TEST_LAST_AVAILABLE_PAGE);
printf("The last page holds the ISR vector, so test page %u\n", last_free);
#endif
if (flashpage_write_and_verify(TEST_LAST_AVAILABLE_PAGE, page_mem) != FLASHPAGE_OK) {
if (flashpage_write_and_verify(last_free, page_mem) != FLASHPAGE_OK) {
puts("error verifying the content of last page");
return 1;
}
Expand All @@ -380,22 +376,23 @@ static int cmd_test_last_raw(int argc, char **argv)
{
(void) argc;
(void) argv;
unsigned last_free = flashpage_last_free();

memset(raw_buf, 0, sizeof(raw_buf));

/* try to align */
memcpy(raw_buf, "test12344321tset", 16);
#if defined(CPU_CC430) || defined(CPU_MSP430FXYZ)
printf("The last page holds the ISR vector, so test page %d\n", TEST_LAST_AVAILABLE_PAGE);
printf("The last page holds the ISR vector, so test page %u\n", last_free);
#endif

/* erase the page first */
flashpage_erase(TEST_LAST_AVAILABLE_PAGE);
flashpage_erase(last_free);

flashpage_write(flashpage_addr(TEST_LAST_AVAILABLE_PAGE), raw_buf, sizeof(raw_buf));
flashpage_write(flashpage_addr(last_free), raw_buf, sizeof(raw_buf));

/* verify that previous write_raw effectively wrote the desired data */
if (memcmp(flashpage_addr(TEST_LAST_AVAILABLE_PAGE), raw_buf, strlen(raw_buf)) != 0) {
if (memcmp(flashpage_addr(last_free), raw_buf, strlen(raw_buf)) != 0) {
puts("error verifying the content of last page");
return 1;
}
Expand Down