Skip to content

esp32: Have each boards' mpconfigboard.cmake include a common file.#18873

Merged
projectgus merged 2 commits into
micropython:masterfrom
projectgus:refactor/esp32_mpconfigboard_cmake
May 28, 2026
Merged

esp32: Have each boards' mpconfigboard.cmake include a common file.#18873
projectgus merged 2 commits into
micropython:masterfrom
projectgus:refactor/esp32_mpconfigboard_cmake

Conversation

@projectgus
Copy link
Copy Markdown
Contributor

@projectgus projectgus commented Feb 26, 2026 •
edited
Loading

Summary

This PR reduces copy-paste in the mpconfigboard.cmake files, which makes it easier to apply config fixes/changes to all boards with a particular chip family.

Also:

  • Renames the sdkconfig.c3usb config file to sdkconfig.c3 as this file no longer contains any USB-specific config settings. This allowed deleting some other sdkconfig.board files which were copies of sdkconfig.c3usb.
  • Refactors out the common pattern of setting 80MHz flash speed and QIO flash mode to a dedicated sdkconfig.flash_qio_80m snippet file.
  • Took the opportunity to replace using set() to extend a list in CMake with list(APPEND ....) instead.

This work was funded through GitHub Sponsors.

Testing

Ran a script to build all boards on this branch and master, and verified that the sdkconfig files were identical and the micropython.bin binary size was identical.

They're all identical except for LOLIN_S2_MINI and LOLIN_S2_PICO, because until now their sdkconfig.board files were not actually being applied. That's fixed now in this PR.

Trade-offs and Alternatives

  • Main downside is a bit of extra complexity, and had to use some CMake code to opt-out of including sdkconfig.spiram_sx on S3 boards without SPIRAM.
  • It's also no longer as simple to see all of the config files included for a single boards' build, although as the "include" is only one layer deep it's also not particularly complex.

Possible future work

  • There are other common patterns in the board files that could be refactored out in the future. I did this one now because I was making another change to the C3 configs, and realised I'd have to touch multiple board config files for it to apply properly.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 26, 2026 •
edited
Loading

Code size report:

Reference:  drivers/esp-hosted: Refactor driver to support full-duplex SPI. [8c57717]
Comparison: esp32: Make a generic sdkconfig.flash_qio_80m config snippet. [merge of d8c25b2]
  mpy-cross:    +0 +0.000% 
   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
      esp32:    +0 +0.000% ESP32_GENERIC
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

dpgeorge
dpgeorge reviewed Mar 3, 2026
View reviewed changes
Comment thread ports/esp32/boards/UM_TINYWATCHS3/mpconfigboard.cmake
@dpgeorge
Copy link
Copy Markdown
Member

dpgeorge commented Mar 3, 2026

This is a nice clean up / code factoring, but I feel that doing things like:

include(boards/ESP32_GENERIC/mpconfigboard.cmake)

and making most boards depend on an ESP32_GENERIC_xxx board may be too tight a dependency. Eg it's now hard to make changes to ESP32_GENERIC boards that are specific to those boards.

How about factoring the common code to files like boards/mpconfigboard_esp32s3_common.cmake and then having all esp32s3 boards include that instead? Then you only need to follow the trail of includes down one level, rather than up into a board.

@projectgus projectgus marked this pull request as draft April 15, 2026 00:10
@projectgus projectgus force-pushed the refactor/esp32_mpconfigboard_cmake branch 2 times, most recently from 8685126 to 276b99b Compare May 27, 2026 08:15
@projectgus
Copy link
Copy Markdown
Contributor Author

projectgus commented May 27, 2026

Have updated to apply suggestions, this way is much more flexible! Checked the other sdkconfig.board entries and they are all unique now.

Re-ran script to build all boards, and built and flashed a few to confirm they were working as expected.

@projectgus projectgus requested a review from dpgeorge May 27, 2026 08:34
@projectgus projectgus marked this pull request as ready for review May 27, 2026 08:34
@projectgus
Copy link
Copy Markdown
Contributor Author

projectgus commented May 27, 2026

If this merges then I'll go around the open PRs which add esp32 board profiles and request them to update.

@projectgus projectgus changed the title esp32: Have boards' mpconfigboard.cmake include the generic file. esp32: Have each boards mpconfigboard.cmake include a common file. May 27, 2026
@projectgus projectgus changed the title esp32: Have each boards mpconfigboard.cmake include a common file. esp32: Have each board mpconfigboard.cmake include a common file. May 27, 2026
@projectgus projectgus changed the title esp32: Have each board mpconfigboard.cmake include a common file. esp32: Have each boards' mpconfigboard.cmake include a common file. May 27, 2026
dpgeorge
dpgeorge reviewed May 27, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@dpgeorge dpgeorge left a comment

Choose a reason for hiding this comment

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

Thanks for updating. It looks good overall (and nice little improvement to use list(APPEND SDKCONFIG_DEFAULTS ...).

Just a few comments.

Comment thread ports/esp32/boards/mpconfigboard_esp32c3_common.cmake Outdated
Comment thread ports/esp32/boards/mpconfigboard_esp32c2_common.cmake Outdated
Comment thread ports/esp32/boards/M5STACK_ATOMS3_LITE/mpconfigboard.cmake
Comment thread ports/esp32/boards/SPARKFUN_THINGPLUS_ESP32C5/mpconfigboard.cmake
@projectgus
esp32: Add generic mpconfigboard_xyz_common.cmake files.
951c45c 
- Move the "generic" mpconfigboard.cmake files to per-chip
  files in the top-level board directory.

- Replace all the copy-paste in other boards' mpconfigboard.cmake
  files to directly include those ones.

- Rename sdkconfig.c3usb config file to sdkconfig.c3 as this file no longer
  contains any USB-specific config settings. This allowed deleting some
  other sdkconfig.board files which were copies of sdkconfig.c3usb.

- Replace using set() to extend a list with CMake list(APPEND ...)

The downside is a bit of extra complexity, and had to use a CMake function
to opt-out of sdkconfig.spiram_sx on S2/S3 boards without SPIRAM.

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
Signed-off-by: Angus Gratton <angus@redyak.com.au>
@projectgus projectgus force-pushed the refactor/esp32_mpconfigboard_cmake branch from 276b99b to cf80773 Compare May 28, 2026 04:31
@projectgus
Copy link
Copy Markdown
Contributor Author

projectgus commented May 28, 2026

Thanks @dpgeorge. Have applied your suggestions, including a new commit to make a sdkconfig.flash_qio_80m.

After you found some mistakes I ran a script to build all boards on this branch and master, and verified that the sdkconfig files were identical and the micropython.bin binary size was identical.

They're all identical except for LOLIN_S2_MINI and LOLIN_S2_PICO, because until now their sdkconfig.board files were not actually being applied. That's fixed now in this PR.

@projectgus
esp32: Make a generic sdkconfig.flash_qio_80m config snippet.
d8c25b2 
Replaces the common practice of setting 80MHz & QIO flash in
individual sdkconfig.board files.

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
@projectgus projectgus force-pushed the refactor/esp32_mpconfigboard_cmake branch from cf80773 to d8c25b2 Compare May 28, 2026 04:41
@projectgus
Copy link
Copy Markdown
Contributor Author

projectgus commented May 28, 2026

(Last push is only a trailing whitespace cleanup)

dpgeorge
dpgeorge approved these changes May 28, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@dpgeorge dpgeorge left a comment

Choose a reason for hiding this comment

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

Thanks for updating, and for running a check on the sdkconfig being identical.

And good that the LOLIN_S2's are fixed.

Feel free to merge it.

@projectgus projectgus merged commit 4fd7295 into micropython:master May 28, 2026
13 checks passed
@projectgus projectgus deleted the refactor/esp32_mpconfigboard_cmake branch May 28, 2026 06:34
@projectgus projectgus mentioned this pull request May 28, 2026
Open
2 tasks
This was referenced Jun 4, 2026
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dpgeorge dpgeorge dpgeorge approved these changes

Assignees

No one assigned

Labels

port-esp32

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@projectgus @dpgeorge