Skip to content
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

Add ESP32-S3 support #4363

Closed
tannewt opened this issue Mar 8, 2021 · 14 comments · Fixed by #5615
Closed

Add ESP32-S3 support #4363

tannewt opened this issue Mar 8, 2021 · 14 comments · Fixed by #5615
Labels
enhancement espressif applies to multiple Espressif chips
Milestone

Comments

@tannewt
Copy link
Member

tannewt commented Mar 8, 2021

We should evolve the ESP32-S2 port to support ESP32-S3 as well. @UnexpectedMaker has started looking at this.

@tannewt tannewt added enhancement espressif applies to multiple Espressif chips labels Mar 8, 2021
@tannewt tannewt added this to the Long term milestone Mar 8, 2021
@UnexpectedMaker
Copy link

It would be super helpful if we could move to IDF4.3 as the base for all of the esp32 sooner than later as that is the minimum I need to work on the S3 integration.

@tannewt
Copy link
Member Author

tannewt commented Mar 8, 2021

There is a pending PR for 4.3 here: #4195

@askpatrickw
Copy link

askpatrickw commented Mar 9, 2021

I had a great chat with @UnexpectedMaker about this update to the IDF, he's going to post more detailed notes, but a couple of things:

1.The paths updates I provided (now ~ a month old) are incorrect in many instances, wherever possible the path should have an esp32s2 folder in the path.
2. In order to support S3, ESP32 as well as the S2, from the same port, the build system will need to have a way to indicate the target IDF microcontroller. @tannewt should advise on that, and we should probably break that out as a separate issue. q: Maybe this can go in the board definition?
3. When the second item is worked on, the conditional paths should also be fixed so either the esp32s2 folder is conditionally esp32s3, or esp32 or that there are conditional sections for each target micro-controller's includes

Seon is going to provide a sample of the path changes and if my day allows, I'll give a swing at those changes tomorrow.

@microdev1 @skieast, I'm also happy to bow out if someone else has time and would like to step-in.

Keep in mind, this also fixes building on my the M1 Mac Mini...

@microdev1
Copy link
Collaborator

Yes, we can have the board definitions contain the IDF_TARGET definition.
I think we should have something similar to what other ports like stm have for building different micros which have shared apis.
The nice thing here is that apis are pretty similar for esp32, S2, S3 and the C3 as well.

@askpatrickw
Copy link

Do you mean how $(MCU_SERIES_LOWER) is used in the include paths (and other places)?
https://github.com/adafruit/circuitpython/blob/main/ports/stm/Makefile#L223

@microdev1
Copy link
Collaborator

Do you mean how $(MCU_SERIES_LOWER) is used in the include paths (and other places)?

Yup... so something like the following in Makefile:

INC += -isystem esp-idf/components/esp32s2/include
INC += -isystem esp-idf/components/$(IDF_TARGET)/include

@askpatrickw
Copy link

askpatrickw commented Mar 9, 2021

I'm just curious if I understand this..

So if we added IDF_TARGET = esp32s2 to each boards/<boardname>/mpconfigboard.mk
we could then use the INC as you show above anywhere there is a mcu specific include and then the S3 or any other idf support mcu could be supported.

There was this thread about INC paths in the PR which I still think is relevant. A standard way the includes are handled in this port will now need to be chosen as well (I think)

@tannewt
Copy link
Member Author

tannewt commented Mar 9, 2021

I'm just curious if I understand this..

So if we added IDF_TARGET = esp32s2 to each boards/<boardname>/mpconfigboard.mk
we could then use the INC as you show above anywhere there is a mcu specific include and then the S3 or any other idf support mcu could be supported.

There was this thread about INC paths in the PR which I still think is relevant. A standard way the includes are handled in this port will now need to be chosen as well (I think)

This sounds good to me. You can use it in the long include paths I prefer too. Though I'm open to changing how we do includes for ESP.

@askpatrickw
Copy link

askpatrickw commented Mar 9, 2021

Please see the PR #4195 I made some of these changes to head us down this path. We discussed and Microdev removed those changes so we could get in his fixes which built the as-is branch on idf release/4.3

@tannewt if you show me an example of how to specify a variable in an include, I'm willing to update all the includes to the long paths an remove the length INC section.

@UnexpectedMaker
Copy link

Great stuff @askpatrickw - I was going to mention you need to remove the ..../include/... stuff now, but you did that already!

@tannewt
Copy link
Member Author

tannewt commented Mar 10, 2021

Please see the PR #4195 I made some of these changes to head us down this path. We discussed and Microdev removed those changes so we could get in his fixes which built the as-is branch on idf release/4.3

@tannewt if you show me an example of how to specify a variable in an include, I'm willing to update all the includes to the long paths an remove the length INC section.

We don't actually do it in #include with SAMD. Looks like we always do it in -I: https://github.com/adafruit/circuitpython/blob/main/ports/atmel-samd/Makefile#L66 We can do it similarly for ESP.

@askpatrickw
Copy link

@UnexpectedMaker, ESP-IDF4.3 was merged so you should be clear to look at this again.

@UnexpectedMaker
Copy link

@UnexpectedMaker, ESP-IDF4.3 was merged so you should be clear to look at this again.

I need to wait fore the next S3 silicon to get into my hands... the latest stuff Espressif are working on for S3 is in IDF4.4 and they have not back ported everything to 4.3 and there was a breaking firmware change that makes my S3 dev kit board incompatible :(

I have the initial beta silicon, they are on beta 3 internally but not releasing that and so whatever the next one is, is what I am waiting for.

I'm in the S3 silicon queue - no ETA yet, but I believe it's close.

tannewt added a commit that referenced this issue Sep 14, 2021
This is in preparation for ESP32-S3 support and potentially others.

Related to #4363
@skieast
Copy link

skieast commented Oct 13, 2021

I've gotten USB PIDs from Espressif for use on the DevkitC. Just in case. VID is 0x303A //Espressif USB VID
0x7002 | ESP32-S3-DevKitC-1 - UF2 Bootloader
0x7003 | ESP32-S3-DevKitC-1 - CircuitPython

@microdev1 microdev1 linked a pull request Nov 24, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement espressif applies to multiple Espressif chips
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants