Make this library compatible for ESP8266 RTOS SDK. #215
Conversation
Only require `sdmmc` when `CONFIG_LITTLEFS_SDMMC_SUPPORT` is set.
Avoid referring to `portMUX_INITIALIZER_UNLOCKED` and `portENTER_CRITICAL` / `portEXIT_CRITICAL`, which are not available for ESP8266 RTOS SDK.
ESP8266 RTOS SDK default enables VFS DIR support.
Use a more generic condition for ESP8266 RTOS SDK compatibility.
src/esp_littlefs.c
Outdated
| @@ -899,13 +899,17 @@ static int esp_littlefs_flags_conv(int m) { | |||
|
|
|||
| static void esp_littlefs_take_efs_lock(void) { | |||
| if( _efs_lock == NULL ){ | |||
| #if portNUM_PROCESSORS > 1 // SMP protection needed for multi-core | |||
There was a problem hiding this comment.
Last issue for this PR:
Is this portENTER_CRITICAL still required for a single core processor? The race condition is if 2 threads attempt to initialize the global _efs_lock, at the same time, then without this look we could result in 2 calls of xSemaphoreCreateMutex. This is avoided by the seemingly-redundant _efs_lock == NULL check.
There was a problem hiding this comment.
Ah, good point. I found taskENTER_CRITICAL() and taskEXIT_CRITICAL() which seem to be what is needed.
CMakeLists.txt
Outdated
| @@ -5,14 +5,15 @@ list(APPEND SOURCES src/esp_littlefs.c src/littlefs_esp_part.c src/lfs_config.c) | |||
|
|
|||
| if(CONFIG_LITTLEFS_SDMMC_SUPPORT) | |||
| list(APPEND SOURCES src/littlefs_sdmmc.c) | |||
| list(APPEND priv_requires sdmmc) | |||
There was a problem hiding this comment.
I'm pretty sure sdmmc has to be in pub_requires because it is referenced in the public include/esp_littlefs.h
There was a problem hiding this comment.
if it's in pub_requires, then we don't have to add it to priv_requires, so we should be able to delete this line (line 8).
There was a problem hiding this comment.
Ah, OK. I am not very familar with the conventions. :P
Add `sdmmc` to `pub_requires` since it is referred to in include/esp_littlefs.h
Use `taskENTER_CRITICAL()` and `taskEXIT_CRITICAL()` for single core concurrency protection.
src/esp_littlefs.c
Outdated
| @@ -899,13 +899,21 @@ static int esp_littlefs_flags_conv(int m) { | |||
|
|
|||
| static void esp_littlefs_take_efs_lock(void) { | |||
| if( _efs_lock == NULL ){ | |||
| #if portNUM_PROCESSORS > 1 | |||
There was a problem hiding this comment.
we're doing this because the 8266 doesn't have a portENTER_CRITICAL macro, correct? If so, I guess we should change this back to #ifdef ESP8266 since this is an 8266-specific change, not dependent on the number of cores of the target device. Lets add a comment about it as well.
Sorry about the thrash!
There was a problem hiding this comment.
just searching the sdk makes it seem like portENTER_CRITICAL is available, however.
There was a problem hiding this comment.
Yes, just that ESP8266's portENTER_CRITICAL doesn't take any arguments -- probably just an outdated ESP-IDF convention, similar to the dir support. So, sure I guess we can just do #ifdef ESP8266 -- so a future clean up (if the ESP8266 SDK ever get refreshed) would be easier. :D
`priv_requires` not needed if `pub_requires` is used.
Use `ESP8266` to guard the concurrency protection API patch instead.
|
Thank you! |
No description provided.