-
Notifications
You must be signed in to change notification settings - Fork 539
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 2 commits
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 |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ | |
| #include "nrf_soc.h" | ||
| #include "delay.h" | ||
| #include "rtos.h" | ||
| #include "assert.h" | ||
|
|
||
|
|
||
| #ifdef NRF52840_XXAA | ||
|
|
@@ -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) | ||
| 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.
Outdated
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.
Outdated
Show resolved
Hide resolved
|
||
| assert(false); | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| // Flash Abstraction Layer | ||
|
|
@@ -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(); | ||
|
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); | ||
| // 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) | ||
|
|
@@ -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.
Outdated
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; | ||
|
|
||
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.