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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions cpu/cc2538/ldscripts/cc2538.ld
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@

INCLUDE cortexm_rom_offset.ld

_cca_length = 44;

/* Memory Space Definitions: */
MEMORY
{
rom (rx) : ORIGIN = _rom_start_addr + _rom_offset, LENGTH = _fw_rom_length
cca : ORIGIN = 0x0027ffd4, LENGTH = 44
rom (rx) : ORIGIN = _rom_start_addr + _rom_offset, LENGTH = _fw_rom_length - _cca_length
cca : ORIGIN = 0x0027ffd4, LENGTH = _cca_length
sram0 : ORIGIN = 0x20000000, LENGTH = 16K /* Lost in PM2 and PM3 */
sram1 : ORIGIN = 0x20004000, LENGTH = 16K
ram (w!rx) : ORIGIN = _ram_start_addr, LENGTH = _ram_length
Expand Down
8 changes: 8 additions & 0 deletions cpu/cortexm_common/ldscripts/cortexm_base.ld
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,10 @@ SECTIONS
_sram = ORIGIN(ram);
_eram = ORIGIN(ram) + LENGTH(ram);

/* Populate information about rom size */
_srom = ORIGIN(rom);
_erom = ORIGIN(rom) + LENGTH(rom);

_sbackup_data_load = LOADADDR(.backup.data);
.backup.data : ALIGN(4) {
_sbackup_data = .;
Expand All @@ -228,4 +232,8 @@ SECTIONS
_sheap1 = . ;
_eheap1 = ORIGIN(bkup_ram) + LENGTH(bkup_ram);
} > bkup_ram

.end_fw (NOLOAD) : ALIGN(4) {
_end_fw = . ;
} > rom
}
37 changes: 37 additions & 0 deletions cpu/cortexm_common/periph/flashpage.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright (C) 2021 Freie Universität Berlin
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @ingroup cpu_cortexm_common
* @ingroup drivers_periph_pm
* @{
* @file
* @brief common periph/flashpage functions
*
* @author Nils Ollrogge <[email protected]>
* @}
*/

#include "periph/flashpage.h"

/**
* @brief Memory markers, defined in the linker script
* @{
*/
extern uint32_t _end_fw;
extern uint32_t _erom;

unsigned flashpage_first_free(void)
{
return flashpage_page(&_end_fw) + 1;
}

unsigned flashpage_last_free(void)
{
return flashpage_page(&_erom) - 1;
Ollrogge marked this conversation as resolved.
Show resolved Hide resolved
}
8 changes: 8 additions & 0 deletions cpu/lpc23xx/ldscripts/lpc23xx.ld
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,10 @@ SECTIONS
_sram = ORIGIN(ram);
_eram = ORIGIN(ram) + LENGTH(ram);

/* Populate information about rom size */
_srom = ORIGIN(rom);
_erom = ORIGIN(rom) + LENGTH(rom);

.heap1 ALIGN(4) (NOLOAD) :
{
_sheap1 = ORIGIN(ram_ethernet);
Expand Down Expand Up @@ -243,4 +247,8 @@ SECTIONS
_sheap3 = . ;
_eheap3 = ORIGIN(ram_battery) + LENGTH(ram_battery);
} > ram_battery

.end_fw (NOLOAD) : ALIGN(4) {
_end_fw = . ;
} > rom
}
17 changes: 17 additions & 0 deletions cpu/lpc23xx/periph/flashpage.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@
typedef void (*IAP)(unsigned int[], unsigned int[]);
static const IAP IAP_Entry = (IAP)0x7ffffff1;

/**
* @brief Memory markers, defined in the linker script
* @{
*/
extern uint32_t _end_fw;
extern uint32_t _erom;

static uint32_t iap(uint32_t code, uint32_t p1, uint32_t p2, uint32_t p3, uint32_t p4)
{
/* contains parameters for IAP command */
Expand Down Expand Up @@ -251,3 +258,13 @@ void flashpage_write(void *target_addr, const void *data, size_t len)
DEBUG("ERROR: COPY_RAM_TO_FLASH: %u\n", err);
}
}

unsigned flashpage_first_free(void)
{
return flashpage_page(&_end_fw) + 1;
}

unsigned flashpage_last_free(void)
{
return flashpage_page(&_erom) - 1;
}
18 changes: 18 additions & 0 deletions cpu/msp430_common/ldscripts/msp430_common.ld
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright (C) 2021 Freie Universität Berlin
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

SECTIONS
{
/* Populate information about rom size */
_srom = ORIGIN(ROM);
_erom = ORIGIN(ROM) + LENGTH(ROM);

.end_fw (NOLOAD) : ALIGN(4) {
_end_fw = . ;
} > ROM
}
19 changes: 19 additions & 0 deletions cpu/msp430_common/periph/flashpage.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@
#include "irq.h"
#include "periph/flashpage.h"

/**
* @brief Memory markers, defined in the linker script
* @{
*/
extern uint32_t _end_fw;
extern uint32_t _erom;

static inline int _unlock(void)
{
int state;
Expand Down Expand Up @@ -97,3 +104,15 @@ void flashpage_write(void *target_addr, const void *data, size_t len)
/* lock flash and re-enable interrupts */
_lock(state);
}

unsigned flashpage_first_free(void)
{
return flashpage_page(&_end_fw) + 1;
}

/* MSP430 cpu's last page holds the interrupt vector, so flashpage_last_free
is the one before last */
unsigned flashpage_last_free(void)
{
return flashpage_page(&_erom) - 1;
}
10 changes: 10 additions & 0 deletions cpu/native/periph/flashpage.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,13 @@ void flashpage_write(void *target_addr, const void *data, size_t len)

_flash_write(target_addr, data, len);
}

unsigned flashpage_first_free(void)
{
return flashpage_page(&_native_flash[0]);
}

unsigned flashpage_last_free(void)
{
return flashpage_page(&_native_flash[FLASHPAGE_SIZE * FLASHPAGE_NUMOF - 1]);
}
8 changes: 8 additions & 0 deletions cpu/riscv_common/ldscripts/riscv_base.ld
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ SECTIONS
{
__stack_size = DEFINED(__stack_size) ? __stack_size : 256;

/* Populate information about rom size */
_srom = ORIGIN(rom);
_erom = ORIGIN(rom) + LENGTH(rom);

.init :
{
KEEP (*(SORT_NONE(.init)))
Expand Down Expand Up @@ -213,4 +217,8 @@ SECTIONS
. = __stack_size;
PROVIDE( _sp = . );
} >ram AT>ram :ram

.end_fw (NOLOAD) : ALIGN(4) {
_end_fw = . ;
} > rom
}
37 changes: 37 additions & 0 deletions cpu/riscv_common/periph/flashpage.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright (C) 2021 Freie Universität Berlin
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @ingroup cpu_riscv_common
* @ingroup drivers_periph_pm
* @{
* @file
* @brief common periph/flashpage functions
*
* @author Nils Ollrogge <[email protected]>
* @}
*/

#include "periph/flashpage.h"

/**
* @brief Memory markers, defined in the linker script
* @{
*/
extern uint32_t _end_fw;
extern uint32_t _erom;

unsigned flashpage_first_free(void)
{
return flashpage_page(&_end_fw) + 1;
}

unsigned flashpage_last_free(void)
{
return flashpage_page(&_erom) - 1;
}
10 changes: 10 additions & 0 deletions drivers/include/periph/flashpage.h
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
1 change: 1 addition & 0 deletions makefiles/arch/msp430.inc.mk
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ ASFLAGS += $(CFLAGS_CPU) --defsym $(CPU_MODEL)=1 $(CFLAGS_DBG)
LINKFLAGS += $(CFLAGS_CPU) $(CFLAGS_DBG) $(CFLAGS_OPT)
LINKFLAGS += -Wl,--gc-sections -Wl,-L$(MSP430_SUPPORT_FILES)/include
LINKFLAGS += -T $(MSP430_SUPPORT_FILES)/include/$(CPU_MODEL).ld
LINKFLAGS += -T $(RIOTCPU)/msp430_common/ldscripts/msp430_common.ld
LINKFLAGS += $(RIOTCPU)/msp430_common/ldscripts/xfa.ld

OPTIONAL_CFLAGS_BLACKLIST += -fdiagnostics-color
Expand Down
41 changes: 19 additions & 22 deletions tests/periph_flashpage/main.c
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
Contributor Author

@Ollrogge Ollrogge Oct 20, 2021

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
Contributor Author

@Ollrogge Ollrogge Oct 25, 2021

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
Contributor 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