esp32: Add support for board variants. #12088
Conversation
|
Code size report: |
Codecov Report
@@ Coverage Diff @@
## master #12088 +/- ##
=======================================
Coverage 98.38% 98.38%
=======================================
Files 158 158
Lines 20895 20900 +5
=======================================
+ Hits 20558 20563 +5
Misses 337 337
|
|
Added the change to autobuild.sh to build all variants for all boards. |
|
@dpgeorge This will require a small fixup on the build server to rename exisiting firmware files to the new names. But otherwise it should allow us to keep providing the full history for these "new" esp32 variants. |
|
Hi Jimmo, respectfully: the statement "This was recently added to the rp2 port, and used on the WEACTSTUDIO board for different flash sizes." may have been implemented as an approach but it to date not followed through on, as the WEACTSTUDIO download page shows. |
hi @Flipje1955 -- this is exactly the purpose of the final commit in this PR. All variants for all boards in all ports will now be published to the downloads page. This includes the WEACTSTUDIO variants. |
|
This is looking great @jimmo! It's a very handy feature. However, don't we need additional variants defined for the GENERIC boards, in particular the S3? 4, 8, 16 and 32MB are all common configurations, I think... |
@mattytrentini Yep for sure. I just wanted to migrate the existing boards over as a first step. |
|
After discussing with @dpgeorge we've decided to get rid of the So I've removed the definition of Also rebased after #12081 |
|
First build I've tried appears to work: > docker run -ti --rm -v $(pwd):/code -w /code espressif/idf:v5.0.2 bash -c "make -C mpy-cross && make -C ports/esp32 submodules all BOARD=GENERIC_S3 BOARD_VARIANT=spiram-oct"
...[snip]...
Project build complete. To flash, run this command:
/opt/esp/python_env/idf5.0_py3.8_env/bin/python ../../../opt/esp/idf/components/esptool_py/esptool/esptool.py -p (PORT) -b 460800 --before default_reset --after no_reset --chip esp32s3 write_flash --flash_mode dio --flash_size 8MB --flash_freq 80m 0x0 build-GENERIC_S3/bootloader/bootloader.bin 0x8000 build-GENERIC_S3/partition_table/partition-table.bin 0x10000 build-GENERIC_S3/micropython.bin
or run 'idf.py -p (PORT) flash'
bootloader @0x000000 18288 ( 14480 remaining)
partitions @0x008000 3072 ( 1024 remaining)
application @0x010000 1492224 ( 539392 remaining)
total 1557760
make: Leaving directory '/code/ports/esp32'Looks very promising! |
@dpgeorge For the for f in esp32spiram-*.*; do echo mv $f ${f/esp32spiram/esp32-spiram}; doneSame for the For for f in ESP32_S2_WROVER-*.*; do echo mv $f ${f/ESP32_S2_WROVER/GENERIC_S2-wrover}; doneFor for f in GENERIC_S3_SPIRAM_OCT-*.*; do echo mv $f ${f/GENERIC_S3_SPIRAM_OCT/GENERIC_S3-spiram-oct}; done
for f in GENERIC_S3_SPIRAM-*.*; do echo mv $f ${f/GENERIC_S3_SPIRAM/GENERIC_S3-spiram}; done |
| // Can be set by mpconfigboard.cmake. | ||
| #define MICROPY_HW_BOARD_NAME "Generic ESP32 module" | ||
| #endif | ||
|
|
||
| #define MICROPY_HW_MCU_NAME "ESP32" |
There was a problem hiding this comment.
Should this be configured per-variant? It previously did have different values for the variants, but arguably it should be the same for all of them, so probably OK to keep it at just a single value like you've done.
There was a problem hiding this comment.
Yeah, so the d2wd and unicore boards did have different names before.
I think you could make the case that d2wd and unicore are attributes of the MCU whereas spiram and ota are attributes of the board/configuration. I think we should fix it.
In theory you could have e.g. a spiram+unicore variant, in which case I think setting the MCU to "esp32-unicore" and the board to "Generic ESP32 module with SPIRAM" makes sense.
Updated.
| @@ -1,4 +1,3 @@ | |||
| CONFIG_ESP32C3_REV_MIN_3=y | |||
There was a problem hiding this comment.
The simple answer is because it's the default anyway (without a bit of stuffing around you cannot compile firmware for revisions before 3).
I tried to piece the history together but I think what happened was that the engineering samples (rev 1 & 2) didn't support the USB peripheral, but our original GENERIC_C3 board definition was added then, and then later the GENERIC_C3_USB board was added to support the production boards.
Then later (IDF 5.0 I think, but haven't confirmed) the IDF made revision 3 the default minimum. i.e. neither GENERIC_C3 and GENERIC_C3_USB builds work by default any more on engineering samples. (I know this because I only have engineering sample c3 boards).
So the longer answer is that I think we should actually switch this around and make usb the default (or possibly just not have a variant at all).
There was a problem hiding this comment.
Hmm, OK. So maybe we should follow IDF 5 and have the default GENERIC_C3 variant be minimum rev 3 and enable USB. Then we could have a variant which runs on < rev 3 without USB. Is that possible/easy? If not easy then we can just have no variants for GENERIC_C3 and enable USB by default (until someone asks for < rev 3 support).
Said differently: if we only support rev 3, then we should enable USB by default.
There was a problem hiding this comment.
Yes, I agree.
Will update later tonight.
|
|
||
| list(APPEND MICROPY_DEF_BOARD | ||
| MICROPY_HW_I2C0_SCL=7 | ||
| MICROPY_HW_I2C0_SDA=6 |
There was a problem hiding this comment.
Did you consider putting settings like this in mpconfigboard.h? Eg:
#if MICROPY_BOARD_VARIANT_WROVER
#define MICROPY_HW_I2C0_SCL 7
#endif<
9E88
/span>Probably it's cleaner to keep all variant config in the .cmake file (and .mk for make-based ports)...
There was a problem hiding this comment.
I did but there was no existing precedent for having the variant as a preprocessor macro, and exactly like you say it's neater putting it in one place.
There was a problem hiding this comment.
I'm pretty confused by this "wrover" variant anyway and why it needs to define the pins?
There was a problem hiding this comment.
Maybe because USB uses GPIO19 which is the default for I2C0 SDA? But that would be a problem for all S2 boards.
There was a problem hiding this comment.
How do you feel about replacing this variant with a "spiram" variant, which is consistent with what we've done for OG and S3?
There was a problem hiding this comment.
I've pushed the branch with an extra commit that does this rename/replace. See the commit description for notes.
b211353 to
96296f9
Compare
|
Updated with review comments fixed. |
7a32dad to
ed6f298
Compare
|
Updated based on further discussion. For everything except OG, there is no longer a "spiram" variant. For simplicity, we just support spiram on the default configuration and it auto-detects at boot. (The OG needs to maintain a spiram variant because enabling spiram comes at additional costs to work around silicon bugs that you don't want to pay for if you don't have spiram). Added a board.md for all the generic boards that explains the different variants and how they map to the Espressif module names. Squashed the S2_WROVER commit into the main S2 commit. Renamed the default |
| #define MICROPY_HW_ENABLE_SDCARD (0) | ||
|
|
||
| // Enable UART REPL for modules that have an external USB-UART and don't use native USB. | ||
| #define MICROPY_HW_ENABLE_UART_REPL (1) |
There was a problem hiding this comment.
I see you removed this... what was the reason?
Related to this, it seems that CONFIG_USB_AND_UART is not a real thing, at least I could not find it in the IDF or TinyUSB component...
There was a problem hiding this comment.
Yep, that should be still there. Updated
Also removed CONFIG_USB_AND_UART from all S2 boards.
Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
This is difficult to implement on cmake-based ports, and having the list
of variants in mpconfigboard.{cmake,mk} duplicates information that's
already in board.json.
This removes the existing query-variants make target from stm32 & rp2
and the definition of BOARD_VARIANTS from the various board files.
Also renames the cmake variable to MICROPY_BOARD_VARIANT to match other
variables such as MICROPY_BOARD. The make variable stays as
BOARD_VARIANT.
This work was funded through GitHub Sponsors.
Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
These are now variants of the GENERIC board. This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
As the IDF no longer supports earlier revisions of the C3 by default, we now just explicitly support rev 3+ and enable USB (which wasn't supported in earlier revisions). This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
Unsure of the history of the ESP32_S2_WROVER board (and why it wasn't named GENERIC_S2_...) but now it's a variant of the generic S2 board. Also removes the non-existent CONFIG_USB_AND_UART from all S2 boards. This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
These are now variants of the GENERIC_S3 board. This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
To be consistent with the other partitions files (which have the "-
{2,8,16,32}MiB" suffix). Also renames partitions-ota.csv.
Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
Removes the special-case for stm32. This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
|
Merged, thanks for all the work on this one! |
|
@jimmo I have a question about using this new board variant scheme. If there a port has a basic configuration and variants, does the basic configuration has to be referred in boards.json as well? |
|
@robert-hh Nope it doesn't. |
On the STM32 port, many of the pyboard board definitions support "variants" which enable extra features, such as threading, double-float, etc. The idea is to have a single board definition that can support different configurations.
This was recently added to the rp2 port, and used on the WEACTSTUDIO board for different flash sizes.
This PR adds this for the esp32 port, and merges the GENERIC boards down to just
GENERIC,GENERIC_S2,GENERIC_S3,GENERIC_C3.This has the following benefits:
TODO: Update tools/autobuild/build-boards.sh to query for variants and build them.Done. Removed the special case for stm32/pyboard and builds all variants for all boards across all ports.This work was funded through GitHub Sponsors.