-
Notifications
You must be signed in to change notification settings - Fork 538
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.
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
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,11 +44,46 @@ extern uint32_t __flash_arduino_start[]; | |
// MACRO TYPEDEF CONSTANT ENUM DECLARATION | ||
//--------------------------------------------------------------------+ | ||
static SemaphoreHandle_t _sem = NULL; | ||
static bool _flash_op_failed = false; | ||
|
||
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_failed = (event == NRF_EVT_FLASH_OPERATION_ERROR); | ||
|
||
// 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 | ||
|
||
// Check whether a flash operation was successful, or should be repeated | ||
static bool retry_flash_op (uint32_t op_result, bool sd_enabled) { | ||
henrygab marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
// If busy | ||
if (op_result == NRF_ERROR_BUSY) { | ||
delay(1); | ||
return true; // Retry | ||
} | ||
|
||
// If unspecified error | ||
if (op_result != NRF_SUCCESS) | ||
return true; // Retry | ||
|
||
// If the soft device is enabled, flash operations run async | ||
// The callback (flash_nrf5x_event_cb) will give semaphore when the flash operation is complete | ||
// The callback also checks for NRF_EVT_FLASH_OPERATION_ERROR, which is not available to us otherwise | ||
if (sd_enabled) { | ||
xSemaphoreTake(_sem, portMAX_DELAY); | ||
if (_flash_op_failed) | ||
return true; // Retry | ||
} | ||
|
||
// Success | ||
return false; | ||
} | ||
|
||
// Flash Abstraction Layer | ||
|
@@ -107,30 +142,28 @@ static bool fal_erase (uint32_t addr) | |
// Init semaphore for first call | ||
if ( _sem == NULL ) | ||
{ | ||
_sem = xSemaphoreCreateCounting(10, 0); | ||
_sem = xSemaphoreCreateBinary(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
|
||
return true; | ||
// Make multiple attempts to erase | ||
uint8_t attempt = 0; | ||
while (retry_flash_op(sd_flash_page_erase(addr / FLASH_NRF52_PAGE_SIZE), sd_en)) { | ||
henrygab marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
if (++attempt > MAX_RETRY) | ||
return false; // Failure | ||
} | ||
return true; // Success | ||
} | ||
|
||
static uint32_t fal_program (uint32_t dst, void const * src, uint32_t len) | ||
{ | ||
// 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); | ||
|
||
|
@@ -140,27 +173,26 @@ 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); | ||
uint8_t attempt = 0; | ||
while (retry_flash_op(sd_flash_write((uint32_t*) dst, (uint32_t const *) src, len/4), sd_en)) { | ||
henrygab marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
if (++attempt > MAX_RETRY) | ||
return 0; // Failure | ||
} | ||
VERIFY_STATUS(err, 0); | ||
|
||
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); | ||
|
||
// First part of block | ||
uint8_t attempt = 0; | ||
while (retry_flash_op(sd_flash_write((uint32_t*) dst, (uint32_t const *) src, len/8), sd_en)) { | ||
if (++attempt > MAX_RETRY) | ||
return 0; // Failure | ||
} | ||
VERIFY_STATUS(err, 0); | ||
if ( sd_en ) xSemaphoreTake(_sem, portMAX_DELAY); | ||
|
||
while ( NRF_ERROR_BUSY == (err = sd_flash_write((uint32_t*) (dst+ len/2), (uint32_t const *) (src + len/2), len/8)) ) | ||
{ | ||
delay(1); | ||
// Second part of block | ||
attempt = 0; | ||
while (retry_flash_op(sd_flash_write((uint32_t*) (dst+ len/2), (uint32_t const *) (src + len/2), len/8), sd_en)) { | ||
if (++attempt > MAX_RETRY) | ||
return 0; // Failure | ||
} | ||
VERIFY_STATUS(err, 0); | ||
if ( sd_en ) xSemaphoreTake(_sem, portMAX_DELAY); | ||
#endif | ||
|
||
return len; | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.