Skip to content

toolchain: Add Espressif's ESP32 and ESP32S2 support #426

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.

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

Merged

Conversation

sylvioalves
Copy link
Collaborator

No description provided.

@sylvioalves sylvioalves changed the title toolchain: Add Espressif ESP32 and ESP32S2 support toolchain: Add Espressif's ESP32 and ESP32S2 support Feb 17, 2022
@sylvioalves
Copy link
Collaborator Author

@galak, not sure if the buildkite error is indeed an error. Would you check?

@stephanosio
Copy link
Member

@galak, not sure if the buildkite error is indeed an error. Would you check?

@sylvioalves I have re-triggered the CI. It should succeed.

Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

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

@sylvioalves Have you verified that this works with the ESP32? IIRC, there were some issues earlier without applying the ESP32-specific patches for gcc, newlib and others.

@sylvioalves
Copy link
Collaborator Author

sylvioalves commented Feb 18, 2022

@stephanosio @galak We analyzed all the patches in Espressif's gcc/gdb/newlib repository and concluded that we can have it as is (without patches). We will submit some patches upstream eventually, but there is no blocking patch at all to have it integrated in Zephyr.
However, this won't work in Zephyr with latest branch, of course. I will submit a PR with a bunch of changes to enable the usage of this new toolchain. Can you suggest me a way to synchronize this?
Also, I have build this toolchain locally, copied and pasted into the zephyr's sdk folder to simulate final behavior. Works good. Let me know if you want any sort of testing or info, ok?

@stephanosio
Copy link
Member

stephanosio commented Feb 18, 2022

However, this won't work in Zephyr with latest branch, of course.

Do you mean the "ESP32 toolchain" built using the changes in this PR is not able to build working binaries for the ESP32 SoC? If so, this PR should be considered a draft until it can.

Can you suggest me a way to synchronize this?

If you have ESP32-specific patches for the binutils, gcc and newlib, please create a pull request in the relevant Zephyr fork repositories and update this PR to reference them (you can set *_DEVEL_REVISION to pull/(PR NUMBER)/head).

For example, if you have a patch for gcc:

  1. Create a pull request to the zephyr-gcc-10.3.0 branch in the Zephyr gcc repository. Let's say this PR is number 10.
  2. Set CT_GCC_DEVEL_REVISION="pull/10/head" in the toolchain config files in this PR, and push to build the toolchain.
  3. Test the toolchain.
  4. Merge the GCC PR.
  5. Set CT_GCC_DEVEL_REVISION="(MERGED COMMIT SHA)" in the toolchain config files.
  6. Merge this PR.

@stephanosio stephanosio marked this pull request as draft February 18, 2022 08:04
@sylvioalves
Copy link
Collaborator Author

sylvioalves commented Feb 18, 2022

@stephanosio Yes, current Zephyr's main branch can't be build using the toolchain from this PR. I am working out a Zephyr's PR to enabled this (I have it all working locally already).
Regarding patches, I won't submit any patch at all as it works as is.

@stephanosio stephanosio added the area: Toolchain Issues related to Toolchain (Binutils+GCC+GDB+libs) label Feb 22, 2022
@sylvioalves sylvioalves force-pushed the feature/espressif_toolchain branch 4 times, most recently from 3ea778e to 595d052 Compare March 8, 2022 15:22
Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

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

Please specify the source (URL) and version of the xtensa_espressif_esp32.tar.gz and xtensa_espressif_esp32s2.tar.gz added here in the commit message.

@stephanosio
Copy link
Member

Also noting that this needs to be tested against zephyrproject-rtos/zephyr#43534.

@sylvioalves
Copy link
Collaborator Author

Please specify the source (URL) and version of the xtensa_espressif_esp32.tar.gz and xtensa_espressif_esp32s2.tar.gz added here in the commit message.

I added the github URL in the commit message. The .tar.gz file was manually created.

@sylvioalves sylvioalves force-pushed the feature/espressif_toolchain branch 2 times, most recently from 89756fb to 8769867 Compare March 8, 2022 19:37
@stephanosio stephanosio added this to the 0.14.0 milestone Mar 9, 2022
@stephanosio
Copy link
Member

I added the github URL in the commit message.

@sylvioalves Thanks. Please add the version/tag or commit (in xtensa-overlay repository) from which the tarball was created in the commit message as well. This will make identifying the current version of overlays we have in the sdk-ng repository much easier, in case we ever need to update these overlays in the future.

@stephanosio stephanosio added the DNM DO NOT MERGE label Mar 9, 2022
Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

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

Looks ok overall.

DNM until the build failures described in zephyrproject-rtos/zephyr#43534 (review) are resolved, in case they require Zephyr SDK-side changes.

@stephanosio stephanosio modified the milestones: 0.14.0, 0.14.1 Mar 11, 2022
@stephanosio
Copy link
Member

Postponing this to 0.14.1, which will be a quick follow-up to 0.14.0 (multi-platform SDK) in a month or so.

@sylvioalves sylvioalves force-pushed the feature/espressif_toolchain branch from 8769867 to 7e60eb9 Compare March 11, 2022 18:10
@sylvioalves
Copy link
Collaborator Author

Only added commit sha into commit message.

Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

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

There has been a slight change to the way the toolchain components are referenced in the configs (#444).

Could you update the PR as follows?

@sylvioalves sylvioalves force-pushed the feature/espressif_toolchain branch from 7e60eb9 to 19948b1 Compare March 20, 2022 18:19
@sylvioalves sylvioalves requested a review from stephanosio March 21, 2022 04:21
Copy link
Member

@stephanosio stephanosio 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 the update. It looks like the GDB source configs were duplicated in the xtensa-espressif_esp32_zephyr-elf.config (specified GDB instead of Binutils at the top).

Also I found a few extra changes to be made; it would be good to address them while we are at it.

@sylvioalves
Copy link
Collaborator Author

Thanks for the update. It looks like the GDB source configs were duplicated in the xtensa-espressif_esp32_zephyr-elf.config (specified GDB instead of Binutils at the top).

Also I found a few extra changes to be made; it would be good to address them while we are at it.

Thanks for pointing that out.

@sylvioalves sylvioalves force-pushed the feature/espressif_toolchain branch 2 times, most recently from 77ed6b0 to a626957 Compare March 21, 2022 11:35
Add all necessary changes to allow ESP32 toolchain integration.

Overlay URL:
https://github.com/espressif/xtensa-overlays/tree/master/xtensa_esp32
commit: 30d30463d2bd917d2c504122a887aff00b27f858

Signed-off-by: Sylvio Alves <[email protected]>
Add all necessary changes to allow ESP32S2
toolchain integration.

Overlay URL:
https://github.com/espressif/xtensa-overlays/tree/master/xtensa_esp32s2
commit: ac00ec5abf21102578d9afde22ffffd46929feaf

Signed-off-by: Sylvio Alves <[email protected]>
Add CI content.

Signed-off-by: Sylvio Alves <[email protected]>
@sylvioalves sylvioalves force-pushed the feature/espressif_toolchain branch from a626957 to 714cc8b Compare March 21, 2022 11:36
@sylvioalves sylvioalves marked this pull request as ready for review March 21, 2022 11:37
Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

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

Looks ok overall.

Pending tests.

@stephanosio
Copy link
Member

Ran full twister build on the esp32 and esp32s2_saola boards with zephyrproject-rtos/zephyr@6e952a0 (#43534), verified that all test builds succeed (build only, no hardware runs).

@stephanosio stephanosio removed the DNM DO NOT MERGE label Mar 21, 2022
@stephanosio stephanosio merged commit 798295a into zephyrproject-rtos:main Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Toolchain Issues related to Toolchain (Binutils+GCC+GDB+libs)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants