Fix for bsp m5stack_core_s3 LCD & SD Sharing SPI Bus#754
Fix for bsp m5stack_core_s3 LCD & SD Sharing SPI Bus#754mbwilliamson1581 wants to merge 5 commits intoespressif:masterfrom
Conversation
…ity with the shared SPI bus Initiation and mounting follows: https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/peripherals/sdspi_share.html 1) Initialize SPI Bus 2.1) Attach other SPI Devices which will tie those CS lines high 3) Attach SD to SPI Bus and Mount SD Card
… updates and improvements in the API and documentation. Ensure that all new features and changes Revise Version reflecting SD and LCD support.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 5 potential issues.
Bugbot Autofix prepared a fix for 1 of the 5 issues found in the latest run.
- ✅ Fixed: SD unmount corrupts SPI state when LCD shares bus
bsp_sdcard_unmount()now keepsspi_initializedtrue unlessspi_bus_free()succeeds and ignores expectedESP_ERR_INVALID_STATEwhen the shared LCD bus is still active.
Or push these changes by commenting:
@cursor push 53392295bf
Preview (53392295bf)
diff --git a/bsp/m5stack_core_s3/m5stack_core_s3.c b/bsp/m5stack_core_s3/m5stack_core_s3.c
--- a/bsp/m5stack_core_s3/m5stack_core_s3.c
+++ b/bsp/m5stack_core_s3/m5stack_core_s3.c
@@ -323,10 +323,13 @@
ret |= esp_vfs_fat_sdcard_unmount(BSP_SD_MOUNT_POINT, bsp_sdcard);
bsp_sdcard = NULL;
- //TODO: Check if LCD initialized (when LCD deinit will be covered by BSP)
if (spi_initialized) {
- ret |= spi_bus_free(BSP_SDSPI_HOST);
- spi_initialized = false;
+ esp_err_t spi_ret = spi_bus_free(BSP_SDSPI_HOST);
+ if (spi_ret == ESP_OK) {
+ spi_initialized = false;
+ } else if (spi_ret != ESP_ERR_INVALID_STATE) {
+ ret |= spi_ret;
+ }
}
return ret;This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
** #if !(defined(CONFIG_FATFS_LONG_FILENAMES) || defined(CONFIG_FATFS_LFN_STACK) || defined(CONFIG_FATFS_LFN_HEAP))
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
…ard mounting during LCD initialization. esp_err_t bsp_sdcard_mount() is now a dummy function to retain API compatibility, with a log message indicating that SD Card is initialized and mounted during LCD initialization due to shared SPI bus.
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
New commit with creation of static esp_err_t bsp_sdcard_mount_worker(void) and dummy esp_err_t bsp_sdcard_mount(void). |
|
Hi @mbwilliamson1581, thank you for this PR. I am currently working on regenerating this board by our ESP-BSP Generator. And I want to fix and update more things (like migration to
And |
@espzav thanks for the heads up re your regeneration of the bsp for the board and what would be included. I see you've done a PR for the aw9523 io_expander. I'll take a look. Re the initiation order of 1., 2., 3. Yes, that's what I understand from the substantive Application Note: https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/peripherals/sdspi_share.html esp_lcd_ili9341.c L49 function esp_lcd_new_panel_ili9341(.., esp_lcd_panel_handle_t *ret_panel) L127 etc set the callbacks for base->reset etc as called in esp_lcd\esp_lcd_panel_ops etc Now, even if an io_expander AW9523 virtual gpio_num_t was setup for the lcd reset functionality, i.e. esp_lcd_ili9341.c L176-L179 get executed, the ili9341 init and invert_color are performed with SPI commands. Therefore, as it's coded in master, bsp_display_new(...) and the subsequent calls contravene the 'sdspi_share' application note (as stated in my pull rubric). So the LCD and SD card do not concurrently operate as noted in the current bsp master README.md. My PR is a fairly simple fix to the existing code with a typical use case of the m5stack_core_s3. There could well be more elegant ways to ensure the shared SPI bus and the LCD and SD card are created and attached and comms take place in the correct order. As you see I created a static bsp_sdcard_mount_worker() function (mwilliamson1581 fork L366). This was to retain the standard API of bsp_sdcard_mount() without compile errors. In my fork we could add a library static bool flag and set it in bsp_sdcard_mount() and then && to the if condition L534 to be certain the user intended to initiate and mount the sdcard. As I coded it, if you didn't intend to use the SD then the log message would be the same as if a card is not in the slot, accepting extra memory and resources are consumed. We'd still need rubric in the API documentation for the m5stack_core_s3 stating that the standard FILE *file operations can only commence after bsp_display_start(). One could add functionality to start the SPI bus, mount the SD card and not start and initialize the display; but why would anybody use the m5stack_core_s3 bsp features for such a project? Other bsp's may have shared a shared SPI bus too, i.e. from the bsp header: BSP_LCD_SPI_NUM = BSP_SDSPI_HOST. Sorry for the long answer to your short question. Whatever I can do to help you and others just let me know. |


bsp: m5stack_core_s3
Change description
Ref: https://github.com/espressif/esp-bsp/tree/master/bsp/m5stack_core_s3
** WARNING: The SD card is not working simultaneously with the LCD screen. We are working on a fix.
This PR is a Fix.
Tested with 5.5.3 + LVGL 9.5 + MicroSD FAT32 containing a single-line text file with contents displayed as lv_label_set_text
With reference to https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/peripherals/sdspi_share.html
The existing and standard initiation for the LCD and SDCARD contravene the "sdspi_share" application note; drilling down it's found that an LCD soft-reset is performed via SPI communication during initiation and before the SD Card is added as a SPI Device and then mounted.
This PR performs the bsp_sdcard_mount() within bsp_display_new() that occurs during the bsp_display_start() sequence. Other logic is applied to ensure compliance to the "sdspi_share" application note.
Additionally this PR adds AW9523 functionality and clarity. [A future feature would be an io_expander for the AW9523B chip and application of the library within the m5stack_core_s3 library].
Note
Medium Risk
Changes initialization sequencing for the shared LCD/SD SPI bus (including stopping/resuming LVGL) and alters SD mount behavior, which can impact display and filesystem stability on boot. Risk is mitigated by mount idempotency checks and logging but touches core bring-up paths.
Overview
Fixes M5Stack CoreS3 SD/LCD coexistence on the shared SPI bus by moving SD mounting into the display bring-up path:
bsp_display_new()now mounts the SD card (whenBSP_CAPS_SDCARDis set) andbsp_display_start_with_config()temporarily stops LVGL to avoid SPI traffic during initialization.Makes SD mounting more robust by preventing repeated
esp_vfs_fat_sdspi_mount()calls (warn + no-op if already mounted) and updates the FATFS long-filename warning guard for broader config compatibility.Improves AW9523 handling during I2C init (adds register defines, enables internal I2C pullups, configures AW9523 pin directions/interrupt masks based on enabled capabilities), bumps the component version to
3.1.0, and updatesAPI.md/README.mdto reflect the new SD mount workflow and shared-bus constraint.Written by Cursor Bugbot for commit 8ff32cd. This will update automatically on new commits. Configure here.