Skip to content

Fix for bsp m5stack_core_s3 LCD & SD Sharing SPI Bus#754

Open
mbwilliamson1581 wants to merge 5 commits intoespressif:masterfrom
mbwilliamson1581:master
Open

Fix for bsp m5stack_core_s3 LCD & SD Sharing SPI Bus#754
mbwilliamson1581 wants to merge 5 commits intoespressif:masterfrom
mbwilliamson1581:master

Conversation

@mbwilliamson1581
Copy link
Copy Markdown

@mbwilliamson1581 mbwilliamson1581 commented Apr 2, 2026

bsp: m5stack_core_s3

  • Current Version 3.0.2

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 (when BSP_CAPS_SDCARD is set) and bsp_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 updates API.md/README.md to 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.

…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.
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 2, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 5 potential issues.

Fix All in Cursor

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 keeps spi_initialized true unless spi_bus_free() succeeds and ignores expected ESP_ERR_INVALID_STATE when the shared LCD bus is still active.

Create PR

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))
@cursor
Copy link
Copy Markdown

cursor bot commented Apr 2, 2026

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.

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 2, 2026

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.
@cursor
Copy link
Copy Markdown

cursor bot commented Apr 3, 2026

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.

@mbwilliamson1581
Copy link
Copy Markdown
Author

New commit with creation of static esp_err_t bsp_sdcard_mount_worker(void) and dummy esp_err_t bsp_sdcard_mount(void).
esp_err_t bsp_sdcard_mount(void) for bsp API compatibility.

@espzav
Copy link
Copy Markdown
Collaborator

espzav commented Apr 7, 2026

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 esp_video, create IO Expander component, and more).
I thought that sharing SPI was fixed inside IDF and it was working. If I understand right, there should be right init process:

  1. Init SPI from esp_lcd
  2. Mount SD card
  3. Continue init LCD
  • I am thinking, how to do it right in our API functions...

And aw9523 settings should be called from bsp_enable_feature. It will be public API.

@mbwilliamson1581
Copy link
Copy Markdown
Author

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 esp_video, create IO Expander component, and more). I thought that sharing SPI was fixed inside IDF and it was working. If I understand right, there should be right init process:

  1. Init SPI from esp_lcd
  2. Mount SD card
  3. Continue init LCD
  • I am thinking, how to do it right in our API functions...

And aw9523 settings should be called from bsp_enable_feature. It will be public API.

@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
For completeness (and any later reference) here's my full drilling down of m5stack_core_s3 starting from the standard API of bsp_display_start() on the user app;
[Line numbers are from the current published master]:
m5stack_core_s3.c L584 function bsp_display_start(void) L596 calls bsp_display_start_with_config(...)
m5stack_core_s3.c L599 function bsp_display_start_with_config(...) L606 calls bsp_display_lcd_init(...)
m5stack_core_s3.c L530 function bsp_display_lcd_init(...) L538 calls bsp_display_new(...)
m5stack_core_s3.c L454 function bsp_display_new(.. , esp_lcd_panel_handle_t *ret_panel, ..) L485 calls esp_lcd_new_panel_ili9341(.., ret_panel) L487, L488 & L489 call esp_lcd_panel_reset , esp_lcd_panel_init and esp_lcd_panel_invert_color

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
esp_lcd_ili9341.c L169 panel_ili9341_reset(...)
esp_lcd_ili9341.c L236 panel_ili9341_init(...)
esp_lcd_ili9341.c L324 panel_ili9341_invert_color(...)

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants