Skip to content

Avoid InternalFileSystem corruption caused by simultaneous BLE operation #838

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

Merged
merged 4 commits into from
Feb 13, 2025
Merged

Avoid InternalFileSystem corruption caused by simultaneous BLE operation #838

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3ea7855
Reattempt failed flash operations
todd-herbert Jan 17, 2025
630668a
Refactor for maintainability
todd-herbert Jan 20, 2025
facf938
braces
todd-herbert Jan 21, 2025
275da60
Apply patch from hathach
todd-herbert Feb 13, 2025
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
110 changes: 59 additions & 51 deletions libraries/InternalFileSytem/src/flash/flash_nrf5x.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,15 @@
#include "delay.h"
#include "rtos.h"


#ifdef NRF52840_XXAA
#define BOOTLOADER_ADDR 0xF4000
#else
#define BOOTLOADER_ADDR 0x74000
#endif

// How many retry attempts when performing flash operations
#define MAX_RETRY 20

// defined in linker script
extern uint32_t __flash_arduino_start[];
//extern uint32_t __flash_arduino_end[];
Expand All @@ -44,12 +46,7 @@ extern uint32_t __flash_arduino_start[];
// MACRO TYPEDEF CONSTANT ENUM DECLARATION
//--------------------------------------------------------------------+
static SemaphoreHandle_t _sem = NULL;

void flash_nrf5x_event_cb (uint32_t event)
{
// if (event != NRF_EVT_FLASH_OPERATION_SUCCESS) LOG_LV1("IFLASH", "Flash op Failed");
if ( _sem ) xSemaphoreGive(_sem);
}
static uint32_t _flash_op_result = NRF_EVT_FLASH_OPERATION_SUCCESS;

// Flash Abstraction Layer
static bool fal_erase (uint32_t addr);
Expand All @@ -70,6 +67,31 @@ static flash_cache_t _cache =
.cache_buf = _cache_buffer
};

void flash_nrf5x_event_cb (uint32_t event) {
if ( _sem ) {
// Record the result, for consumption by fal_erase or fal_program
// Used to reattempt failed operations
_flash_op_result = event;

// Signal to fal_erase or fal_program that our async flash op is now complete
xSemaphoreGive(_sem);
}
}

// When soft device is enabled, flash ops are async
// Eventual success is reported via callback, which we await
static uint32_t wait_for_async_flash_op_completion(void) {
uint8_t sd_en = 0;
(void) sd_softdevice_is_enabled(&sd_en);

if (sd_en) {
xSemaphoreTake(_sem, portMAX_DELAY);
return (_flash_op_result == NRF_EVT_FLASH_OPERATION_SUCCESS) ? NRF_SUCCESS : NRF_ERROR_TIMEOUT;
} else {
return NRF_SUCCESS;
}
}

//--------------------------------------------------------------------+
// Application API
//--------------------------------------------------------------------+
Expand Down Expand Up @@ -105,62 +127,48 @@ bool flash_nrf5x_erase(uint32_t addr)
static bool fal_erase (uint32_t addr)
{
// Init semaphore for first call
if ( _sem == NULL )
{
_sem = xSemaphoreCreateCounting(10, 0);
if ( _sem == NULL ) {
_sem = xSemaphoreCreateBinary();
Copy link
Member

@hathach hathach Feb 7, 2025

Choose a reason for hiding this comment

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

kind of forgot, but I think sd_flash has an internal FIFO that we can queue flashing API. But I agree using binary would be better since we wait and retry each call

VERIFY(_sem);
}

// retry if busy
uint32_t err;
while ( NRF_ERROR_BUSY == (err = sd_flash_page_erase(addr / FLASH_NRF52_PAGE_SIZE)) )
{
// Erase the page: Multiple attempts if needed
for (uint8_t attempt = 0; attempt < MAX_RETRY; ++attempt) {
if (NRF_SUCCESS == sd_flash_page_erase(addr / FLASH_NRF52_PAGE_SIZE)) {
if (NRF_SUCCESS == wait_for_async_flash_op_completion()) {
return true;
}
}
delay(1);
}
VERIFY_STATUS(err, false);

// wait for async event if SD is enabled
uint8_t sd_en = 0;
(void) sd_softdevice_is_enabled(&sd_en);

if ( sd_en ) xSemaphoreTake(_sem, portMAX_DELAY);

return true;
return false;
}

static uint32_t fal_program (uint32_t dst, void const * src, uint32_t len)
{
// wait for async event if SD is enabled
uint8_t sd_en = 0;
(void) sd_softdevice_is_enabled(&sd_en);

uint32_t err;

// Somehow S140 v6.1.1 assert an error when writing a whole page
// https://devzone.nordicsemi.com/f/nordic-q-a/40088/sd_flash_write-cause-nrf_fault_id_sd_assert
// Workaround: write half page at a time.
#if NRF52832_XXAA
while ( NRF_ERROR_BUSY == (err = sd_flash_write((uint32_t*) dst, (uint32_t const *) src, len/4)) )
{
// helper for fal_program()
static bool fal_sub_program(uint32_t dst, void const * src, uint32_t len) {
for (uint8_t attempt = 0; attempt < MAX_RETRY; ++attempt) {
if (NRF_SUCCESS == sd_flash_write((uint32_t*) dst, (uint32_t const *) src, len/4)) {
if (NRF_SUCCESS == wait_for_async_flash_op_completion()) {
return true;
}
}
delay(1);
}
VERIFY_STATUS(err, 0);
return false;
}

if ( sd_en ) xSemaphoreTake(_sem, portMAX_DELAY);
static uint32_t fal_program (uint32_t dst, void const * src, uint32_t len) {
#if NRF52832_XXAA
VERIFY(fal_sub_program(dst, src, len), 0);
#else
while ( NRF_ERROR_BUSY == (err = sd_flash_write((uint32_t*) dst, (uint32_t const *) src, len/8)) )
{
delay(1);
}
VERIFY_STATUS(err, 0);
if ( sd_en ) xSemaphoreTake(_sem, portMAX_DELAY);
// Somehow S140 v6.1.1 assert an error when writing a whole page
// https://devzone.nordicsemi.com/f/nordic-q-a/40088/sd_flash_write-cause-nrf_fault_id_sd_assert
// Workaround: write half page at a time.
VERIFY(fal_sub_program(dst, src, len/2), 0); // 1st half

while ( NRF_ERROR_BUSY == (err = sd_flash_write((uint32_t*) (dst+ len/2), (uint32_t const *) (src + len/2), len/8)) )
{
delay(1);
}
VERIFY_STATUS(err, 0);
if ( sd_en ) xSemaphoreTake(_sem, portMAX_DELAY);
dst += len/2;
src += len/2;
VERIFY(fal_sub_program(dst, src, len/2), 0); // 2nd half
#endif

return len;
Expand Down