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

Avoid InternalFileSystem corruption caused by simultaneous BLE operation #838

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from 2 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
115 changes: 86 additions & 29 deletions libraries/InternalFileSytem/src/flash/flash_nrf5x.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "nrf_soc.h"
#include "delay.h"
#include "rtos.h"
#include "assert.h"


#ifdef NRF52840_XXAA
Expand All @@ -44,11 +45,52 @@ extern uint32_t __flash_arduino_start[];
// MACRO TYPEDEF CONSTANT ENUM DECLARATION
//--------------------------------------------------------------------+
static SemaphoreHandle_t _sem = NULL;
static uint32_t _flash_op_result = NRF_EVT_FLASH_OPERATION_SUCCESS;

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);
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);
}
}

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

// 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(uint32_t initial_result) {
// If initial result not NRF_SUCCESS, no need to await callback
// We will pass the initial result (failure) straight through
int32_t result = initial_result;

// Operation was queued successfully
if (initial_result == NRF_SUCCESS) {

// Wait for result via callback
xSemaphoreTake(_sem, portMAX_DELAY);

// If completed successfully
if (_flash_op_result == NRF_EVT_FLASH_OPERATION_SUCCESS)
todd-herbert marked this conversation as resolved.
Show resolved Hide resolved
result = NRF_SUCCESS;

// If general failure.
// The comment on NRF_EVT_FLASH_OPERATION_ERROR describes it as a timeout,
// so we're using a similar error when translating from NRF_SOC_EVTS type to the global NRF52 error defines
else if (_flash_op_result == NRF_EVT_FLASH_OPERATION_ERROR)
todd-herbert marked this conversation as resolved.
Show resolved Hide resolved
result = NRF_ERROR_TIMEOUT;

// If this assert triggers, we need to implement a new NRF_SOC_EVTS value
else
todd-herbert marked this conversation as resolved.
Show resolved Hide resolved
assert(false);
}

return result;
}

// Flash Abstraction Layer
Expand Down Expand Up @@ -107,25 +149,28 @@ static bool fal_erase (uint32_t addr)
// Init semaphore for first call
if ( _sem == NULL )
{
_sem = xSemaphoreCreateCounting(10, 0);
_sem = xSemaphoreCreateBinary();
Copy link
Member

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)) )
{
delay(1);
}
VERIFY_STATUS(err, false);

// wait for async event if SD is enabled
// Check if soft device is enabled
// If yes, flash operations are async, so we need to wait for the callback to give the semaphore
uint8_t sd_en = 0;
(void) sd_softdevice_is_enabled(&sd_en);

if ( sd_en ) xSemaphoreTake(_sem, portMAX_DELAY);
// Erase the page
// Multiple attempts if needed
uint32_t err;
for (uint8_t attempt = 0; attempt < MAX_RETRY; ++attempt) {
err = sd_flash_page_erase(addr / FLASH_NRF52_PAGE_SIZE);

if(sd_en) err = wait_for_async_flash_op_completion(err); // Only async if soft device enabled
if (err == NRF_SUCCESS) break;
if (err == NRF_ERROR_BUSY) delay(1);
}
VERIFY_STATUS(err, false); // Return false if all retries fail

return true;
return true; // Successfully erased
}

static uint32_t fal_program (uint32_t dst, void const * src, uint32_t len)
Expand All @@ -140,27 +185,39 @@ static uint32_t fal_program (uint32_t dst, void const * src, uint32_t len)
// 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)) )
{
delay(1);
// Write the page
// Multiple attempts, if needed
for (uint8_t attempt = 0; attempt < MAX_RETRY; ++attempt) {
err = sd_flash_write((uint32_t*) dst, (uint32_t const *) src, len/4);

if(sd_en) err = wait_for_async_flash_op_completion(err); // Only async if soft device enabled
todd-herbert marked this conversation as resolved.
Show resolved Hide resolved
if (err == NRF_SUCCESS) break;
if (err == NRF_ERROR_BUSY) delay(1);
}
VERIFY_STATUS(err, 0);
VERIFY_STATUS(err, 0); // Return 0 if all retries fail

if ( sd_en ) xSemaphoreTake(_sem, portMAX_DELAY);
#else
while ( NRF_ERROR_BUSY == (err = sd_flash_write((uint32_t*) dst, (uint32_t const *) src, len/8)) )
{
delay(1);
// Write first part of page
// Multiple attempts, if needed
for (uint8_t attempt = 0; attempt < MAX_RETRY; ++attempt) {
err = sd_flash_write((uint32_t*) dst, (uint32_t const *) src, len/8);

if(sd_en) err = wait_for_async_flash_op_completion(err); // Only async if soft device enabled
if (err == NRF_SUCCESS) break;
if (err == NRF_ERROR_BUSY) delay(1);
}
VERIFY_STATUS(err, 0);
if ( sd_en ) xSemaphoreTake(_sem, portMAX_DELAY);
VERIFY_STATUS(err, 0); // Return 0 if all retries fail

while ( NRF_ERROR_BUSY == (err = sd_flash_write((uint32_t*) (dst+ len/2), (uint32_t const *) (src + len/2), len/8)) )
{
delay(1);
// Write second part of page
// Multiple attempts, if needed
for (uint8_t attempt = 0; attempt < MAX_RETRY; ++attempt) {
err = sd_flash_write((uint32_t*) (dst+ len/2), (uint32_t const *) (src + len/2), len/8);

if(sd_en) err = wait_for_async_flash_op_completion(err); // Only async if soft device enabled
if (err == NRF_SUCCESS) break;
if (err == NRF_ERROR_BUSY) delay(1);
}
VERIFY_STATUS(err, 0);
if ( sd_en ) xSemaphoreTake(_sem, portMAX_DELAY);
VERIFY_STATUS(err, 0); // Return 0 if all retries fail
#endif

return len;
Expand Down
Loading