8000 esp32: Merge the per-SoC main components, remove linker workaround. by projectgus · Pull Request #16865 · micropython/micropython · GitHub
[go: up one dir, main page]
Skip to content

esp32: Merge the per-SoC main components, remove linker workaround. #16865

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 13, 2025
Merged

esp32: Merge the per-SoC main components, remove linker workaround. #16865

merged 2 commits into from
Mar 13, 2025

Conversation

projectgus
Copy link
Contributor
@projectgus projectgus commented Mar 5, 2025 •
edited
Loading

Summary

I noticed that IDF Component Manager has a conditional dependencies language that allows us to shrink the per-target main_espXYZ components back down to a single main component.

Split into two commits:

  1. Combines the main components but keeps the linker.lf workaround added in esp32: Workaround for ringbuf functions required by ISRs. #14145 (but with a conditional to only do anything on ESP32).
  2. Removes the < 8000 code class="notranslate">linker.lf workaround entirely, as the upstream issue apparently only existed for ESP-IDF V5.1 which MicroPython no longer supports. The support for a user-specified linker fragment (added in ports/esp32: Allow overriding linker.lf. #16658) is kept, although it may not have a clear use case any more.

Testing

  • With only commit 1 applied, built multiple chips and verified application code size remained the same (Xtensa targets varied by 32 bytes and RISC-V targets by 16 bytes, I think because the final linker script was subtly different and changed padding.)
  • Ran unit tests on ESP32 board
  • Ran a build with the Espressif IDF V5.2.2 container, to check this works OK with the older Component Manager version (it does).

Trade-offs and Alternatives

  • Keeping the individual main components is arguably more flexible and simpler, but it's also a bunch of duplicated info (that adds more duplicates each time we add a new SoC).
  • Out-of-tree builds may need to update their main component, at least the ones which use RISC-V.

dpgeorge
dpgeorge reviewed Mar 11, 2025
View reviewed changes
Copy link
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, this is a very nice simplification!

I noticed that CONFIG_RINGBUF_PLACE_FUNCTIONS_INTO_FLASH is still commented out, and I think it should now be uncommented, ie enabled. That will claw back a bit of IRAM.

@projectgus projectgus force-pushed the refactor/esp32_main_components branch from f0dfdce to f2879ce Compare March 11, 2025 06:59
@projectgus
Copy link
Contributor Author

I noticed that CONFIG_RINGBUF_PLACE_FUNCTIONS_INTO_FLASH is still commented out, and I think it should now be uncommented, ie enabled. That will claw back a bit of IRAM.

Oops, yes! It's restored now.

@projectgus @dpgeorge
esp32: Merge the per-SoC "main" components back together.
4b1c666 
Removes redundant metadata from each, shouldn't otherwise change
any build output.

Reverts the split originally added in e465012.

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
@projectgus @dpgeorge
esp32: Remove the ESP32 ringbuffer linker workaround.
4d65b4e 
Reverts workaround added in acbdbcd.

According to the linked ESP-IDF issue this was only a problem for ESP-IDF
V5.0.x, and support for versions older than V5.2 was dropped in 6e5d8d0.

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
@dpgeorge dpgeorge force-pushed the refactor/esp32_main_components branch from f2879ce to 4d65b4e Compare March 13, 2025 00:34
@dpgeorge dpgeorge merged commit 4d65b4e into micropython:master Mar 13, 2025
8 checks passed
@projectgus projectgus deleted the refactor/esp32_main_components branch March 17, 2025 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dpgeorge dpgeorge dpgeorge left review comments

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
0