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

cpu/esp32: add support for ESP32-S3 SoC family #18185

Closed
wants to merge 100 commits into from

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Jun 9, 2022

Contribution description

This PR provides the support for the ESP32-S3 SoC family and the ESP32-S3-DevKit board series.

For the moment, this PR includes PR #17841, PR #17842 and PR #17844 to be compilable. Once these PRs are merged this PR is rebased. The first additional commit of this PR is 904d5c3.

Testing procedure

Compilation in CI has to succeed. All peripheral and basic tests have to pass.

Issues/PRs references

Depends on PR #17841
Depends on PR #17842
Depends on PR #17844
Prerequisite for PR #18235

gschorcht added 30 commits June 6, 2022 08:33
Bootloader makefile that can be used for different ESP32x variants
Updates `cpu/esp_common/periph/flash` for ESP-IDF 4.4. `spi_flash_*` functions for ESP32 are removed since these functions are now used from ESP-IDF. DEBUG output are changed to be platform independent.
The MCU_* conditionals are inverted so that they can be tested for ESP8266. In all other cases the MCU is any ESP32x SoC
Implements an interface for ESP-IDF types and functions that are required by RIOT-OS but cannot be included directly due to name conflicts.
periph/spi implementation can be used for ESP8266 only from now. An implementation using the ESP-IDF spi HAL interface is required for ESP32x SoCs.
The MCU_* conditionals are inverted so that they can be tested for ESP8266. In all other cases the MCU is any ESP32x SoC
@gschorcht gschorcht force-pushed the cpu/esp32/add_esp32s3_cpu branch from d658008 to c6b890c Compare June 26, 2022 08:15
@gschorcht
Copy link
Contributor Author

@leandrolanzieri Again a question about Kconfig:

This PR adds the ESP32-S3-DevKit board for which 10 different versions are available. All have exactly the same peripheral configuration and differ only by the ESP32-S3 module version used, which in turn differs only by the size of the flash and the availability and size of the SPI RAM. However, this only affects whether the esp_spi_ram feature is available and whether the esp_spi_ram module can be used.

IMHO, it makes no sense to define a separate board for each of these board versions and unnecessarily blow up the number of boards that need to be compiled into CI. Therefore, I tried a another approach by defining variable BOARD_VERSION that is used in boards/esp32s3-devkit/Makefile.features to select the right CPU_MODEL:

# default board version if not defined
ifeq (,$(BOARD_VERSION))
BOARD_VERSION = esp32s3-devkitc-1-n8
endif
ifeq (esp32s3-devkitc-1-n8,$(BOARD_VERSION))
CPU_MODEL = esp32s3_wroom_1x_n8
else ifeq (esp32s3-devkitc-1-n8r2,$(BOARD_VERSION))
CPU_MODEL = esp32s3_wroom_1x_n8r2
else ifeq (esp32s3-devkitc-1-n8r8,$(BOARD_VERSION))
CPU_MODEL = esp32s3_wroom_1x_n8r8
else ifeq (esp32s3-devkitc-1-n16r8v,$(BOARD_VERSION))
CPU_MODEL = esp32s3_wroom_2_n16r8v
else ifeq (esp32s3-devkitc-1-n32r8v,$(BOARD_VERSION))
CPU_MODEL = esp32s3_wroom_2_n32r8v
else ifeq (esp32s3-devkitc-1u-n8,$(BOARD_VERSION))
CPU_MODEL = esp32s3_wroom_1x_n8
else ifeq (esp32s3-devkitc-1u-n8r2,$(BOARD_VERSION))
CPU_MODEL = esp32s3_wroom_1x_n8r2
else ifeq (esp32s3-devkitc-1u-n8r8,$(BOARD_VERSION))
CPU_MODEL = esp32s3_wroom_1x_n8r8
else ifeq (esp32s3-devkitm-1-n8,$(BOARD_VERSION))
CPU_MODEL = esp32s3_mini_1x_n8
else ifeq (esp32s3-devkitm-1u-n8,$(BOARD_VERSION))
CPU_MODEL = esp32s3_mini_1x_n8
else
$(error BOARD_VERSION is unknown)
endif

I tried the same in Kconfig

config BOARD
default "esp32s3-devkit" if BOARD_ESP32S3_DEVKIT
config BOARD_ESP32S3_DEVKIT
bool
default y
select BOARD_COMMON_ESP32S3
select CPU_MODEL_ESP32S3_WROOM_1X_N8 if BOARD_VERSION_ESP32S3_DEVKITC_1_N8
select CPU_MODEL_ESP32S3_WROOM_1X_N8R2 if BOARD_VERSION_ESP32S3_DEVKITC_1_N8R2
select CPU_MODEL_ESP32S3_WROOM_1X_N8R8 if BOARD_VERSION_ESP32S3_DEVKITC_1_N8R8
select CPU_MODEL_ESP32S3_WROOM_1X_N16R8V if BOARD_VERSION_ESP32S3_DEVKITC_1_N16R8V
select CPU_MODEL_ESP32S3_WROOM_1X_N32R8V if BOARD_VERSION_ESP32S3_DEVKITC_1_N32R8V
select CPU_MODEL_ESP32S3_WROOM_1X_N8 if BOARD_VERSION_ESP32S3_DEVKITC_1U_N8
select CPU_MODEL_ESP32S3_WROOM_1X_N8R2 if BOARD_VERSION_ESP32S3_DEVKITC_1U_N8R2
select CPU_MODEL_ESP32S3_WROOM_1X_N8R8 if BOARD_VERSION_ESP32S3_DEVKITC_1U_N8R8
select CPU_MODEL_ESP32S3_MINI_1X_N8 if BOARD_VERSION_ESP32S3_DEVKITM_1_N8
select CPU_MODEL_ESP32S3_MINI_1X_N8 if BOARD_VERSION_ESP32S3_DEVKITM_1U_N8
select HAS_ARDUINO
select HAS_ESP_JTAG
select HAS_PERIPH_ADC
select HAS_PERIPH_I2C
select HAS_PERIPH_PWM
select HAS_PERIPH_SPI
choice
bool "ESP32-S3-DevKit board version"
default BOARD_VERSION_ESP32S3_DEVKITC_1_N8
config BOARD_VERSION_ESP32S3_DEVKITC_1_N8
bool "ESP32-S3-DevKitC-1-N8"
config BOARD_VERSION_ESP32S3_DEVKITC_1_N8R2
bool "ESP32-S3-DevKitC-1-N8R2"
config BOARD_VERSION_ESP32S3_DEVKITC_1_N8R8
bool "ESP32-S3-DevKitC-1-N8R8"
config BOARD_VERSION_ESP32S3_DEVKITC_1_N16R8V
bool "ESP32-S3-DevKitC-1-N16R8V"
config BOARD_VERSION_ESP32S3_DEVKITC_1_N32R8V
bool "ESP32-S3-DevKitC-1-N32R8V"
config BOARD_VERSION_ESP32S3_DEVKITC_1U_N8
bool "ESP32-S3-DevKitC-1U-N8"
config BOARD_VERSION_ESP32S3_DEVKITC_1U_N8R2
bool "ESP32-S3-DevKitC-1U-N8R2"
config BOARD_VERSION_ESP32S3_DEVKITC_1U_N8R8
bool "ESP32-S3-DevKitC-1U-N8R8"
config BOARD_VERSION_ESP32S3_DEVKITM_1_N8
bool "ESP32-S3-DevKitM-1-N8"
config BOARD_VERSION_ESP32S3_DEVKITM_1U_N8
bool "ESP32-S3-DevKitM-1U-N8"
endchoice
but it doesn't work since the BOARD_VERSION variable in boards/esp32s3-devkit/Makefile.features is set to default if not defined at command line before Kconfig is used.

Do you have any idea how to realize it?

@gschorcht gschorcht force-pushed the cpu/esp32/add_esp32s3_cpu branch from c6b890c to 2cb9db2 Compare June 26, 2022 08:56
@leandrolanzieri
Copy link
Contributor

but it doesn't work since the BOARD_VERSION variable in boards/esp32s3-devkit/Makefile.features is set to default if not defined at command line before Kconfig is used.
Do you have any idea how to realize it?

@gschorcht I'm not sure I understand, what is exactly not working with the approach?


choice
bool "ESP32-S3-DevKit board version"
depends on TEST_KCONFIG
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this have to depend on TEST_KCONFIG? When TEST_KCONFIG=n there is no CPU selected, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it should only be visible if other ESP32 configurations are visible. Otherwise you would see the board version selection as the only ESP stuff which seems a little bit strange.

Copy link
Contributor

Choose a reason for hiding this comment

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

Because it should only be visible if other ESP32 configurations are visible. Otherwise you would see the board version selection as the only ESP stuff which seems a little bit strange.

But TEST_KCONFIG is meant to be a switch to indicate that dependency resolution is handled with Kconfig, thus reserved for MODULE_ Kconfig symbols.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, I didn't know that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But TEST_KCONFIG is meant to be a switch to indicate that dependency resolution is handled with Kconfig

One more question about TEST_KCONFIG that I don't know if I modeled something wrong or if it's intentional:

When I start the graphical interface with

TEST_KCONFIG=1 BOARD=esp32s3-devkit make -C tests/kconfig_features menuconfig

and change the board version from the default BOARD_VERSION_ESP32S3_DEVKITC_1_N8 to BOARD_VERSION_ESP32S3_DEVKITC_1_N8R2, the correct configuration is stored in tests/shell/bin/esp32s3-devkit/generated/out.config when I save it:

CONFIG_BOARD="esp32s3-devkit"
CONFIG_BOARD_VERSION="esp32s3-devkitc-1-n8r2"
CONFIG_CPU="esp32"
CONFIG_CPU_FAM="esp32s3"
CONFIG_CPU_MODEL="esp32s3_wroom_1x_n8r2"

But when I leave the graphical interface, tests/shell/bin/esp32s3-devkit/generated/out.config is overwritten with the defaults:

CONFIG_BOARD="esp32s3-devkit"
CONFIG_BOARD_VERSION="esp32s3-devkitc-1-n8"
CONFIG_CPU="esp32"
CONFIG_CPU_FAM="esp32s3"
CONFIG_CPU_MODEL="esp32s3_wroom_1x_n8"

So I can't compile it with the configuration defined in graphical Kconfig. Is this intentional or am I doing something wrong?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I was able to reproduce the behavior. It seems something is up when menuconfig is used with TEST_KCONFIG=1 I will look into it and see if it on master.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to note this behaviour seems to exist only in the esp boards with variants (though testing was not extensive).
We get more than 2 === [ATTENTION] Testing Kconfig dependency modelling === warning which means that there is there are multiple calls, probably due to this, the kconfig settings are being overwritten with defaults.

Further investigation required.

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, this problem exists elsewhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup turns out that one must clean before toggling the TEST_KCONFIG... Please try to reproduce the issue after cleaning.

@gschorcht
Copy link
Contributor Author

but it doesn't work since the BOARD_VERSION variable in boards/esp32s3-devkit/Makefile.features is set to default if not defined at command line before Kconfig is used.
Do you have any idea how to realize it?

@gschorcht I'm not sure I understand, what is exactly not working with the approach?

BOARD_VERSION environment variable is used in boards/esp32s3-devkit/Makefile.features to select the CPU_MODEL variable. If BOARD_VERSION is not set, it iis set to esp32s3-devkitc-1-n8 by default and the CPU_MODEL is set to esp32s3_wroom_1x_n8.

The variable CONFIG_BOARD_VERSION variable from Kconfig is not known when boards/esp32s3-devkit/Makefile.features is read. Thus, independent from what is selected as BOARD_VERSION in Kconfig, used BOARD_VERSION is esp32s3_wroom_1x_n8.

@leandrolanzieri
Copy link
Contributor

leandrolanzieri commented Jun 28, 2022

BOARD_VERSION environment variable is used in boards/esp32s3-devkit/Makefile.features to select the CPU_MODEL variable. If BOARD_VERSION is not set, it iis set to esp32s3-devkitc-1-n8 by default and the CPU_MODEL is set to esp32s3_wroom_1x_n8.

The variable CONFIG_BOARD_VERSION variable from Kconfig is not known when boards/esp32s3-devkit/Makefile.features is read. Thus, independent from what is selected as BOARD_VERSION in Kconfig, used BOARD_VERSION is esp32s3_wroom_1x_n8.

Kconfig does not require the features from Makefile.features, can we check for TEST_KCONFIG and not assign a value then? Kconfig does not use BOARD_VERSION anyway right?

@gschorcht
Copy link
Contributor Author

gschorcht commented Jun 28, 2022

Kconfig does not require the features from Makefile.features, can we check for TEST_KCONFIG and not assign a value then?

If it is OK to check for TEST_KCONFIG in Makefile.features, it could be solved probably. I didn't know that.

@gschorcht
Copy link
Contributor Author

Kconfig does not require the features from Makefile.features, can we check for TEST_KCONFIG and not assign a value then?

If it is OK to check for TEST_KCONFIG in Makefile.features, it could be solved probably. I didn't know that.

Hm, when I use TEST_KCONFIG to check whether to set the BOARD_VERSION to a default value or not

ifneq (1,$(TEST_KCONFIG))
  # default board version if not defined
  ifeq (,$(BOARD_VERSION))
    BOARD_VERSION = esp32s3-devkitc-1-n8
  endif

  ifeq (esp32s3-devkitc-1-n8,$(BOARD_VERSION))
    CPU_MODEL = esp32s3_wroom_1x_n8
  else ifeq (esp32s3-devkitc-1-n8r2,$(BOARD_VERSION))
    CPU_MODEL = esp32s3_wroom_1x_n8r2
  else ifeq (esp32s3-devkitc-1-n8r8,$(BOARD_VERSION))
    CPU_MODEL = esp32s3_wroom_1x_n8r8
  else ifeq (esp32s3-devkitc-1-n16r8v,$(BOARD_VERSION))
    CPU_MODEL = esp32s3_wroom_2_n16r8v
  else ifeq (esp32s3-devkitc-1-n32r8v,$(BOARD_VERSION))
    CPU_MODEL = esp32s3_wroom_2_n32r8v
  else ifeq (esp32s3-devkitc-1u-n8,$(BOARD_VERSION))
    CPU_MODEL = esp32s3_wroom_1x_n8
  else ifeq (esp32s3-devkitc-1u-n8r2,$(BOARD_VERSION))
    CPU_MODEL = esp32s3_wroom_1x_n8r2
  else ifeq (esp32s3-devkitc-1u-n8r8,$(BOARD_VERSION))
    CPU_MODEL = esp32s3_wroom_1x_n8r8
  else ifeq (esp32s3-devkitm-1-n8,$(BOARD_VERSION))
    CPU_MODEL = esp32s3_mini_1x_n8
  else ifeq (esp32s3-devkitm-1u-n8,$(BOARD_VERSION))
    CPU_MODEL = esp32s3_mini_1x_n8
  else
    $(error BOARD_VERSION is unknown)
  endif
endif

CPU_MODEL isn't set and tests/kconfig_features fails.

bors bot added a commit to RIOT-OS/riotdocker that referenced this pull request Aug 3, 2022
200: riotbuild: add ESP32-S3 toolchain r=benpicco a=gschorcht

Now that the 2022.07 release branch is created, it is a good time to go the next migration step and to add Espressif's vendor toolchain for ESP32-S3 to be able to compile RIOT-OS/RIOT#18185 in CI. The toolchain has a size of 292 MByte.

Co-authored-by: Gunar Schorcht <[email protected]>
@gschorcht
Copy link
Contributor Author

Now that all the split-offs have been merged, we can close this PR.

@gschorcht gschorcht closed this Aug 27, 2022
@gschorcht gschorcht deleted the cpu/esp32/add_esp32s3_cpu branch August 31, 2022 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: build system Area: Build system Area: CI Area: Continuous Integration of RIOT components Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: pkg Area: External package ports Area: tests Area: tests and testing framework Area: tools Area: Supplementary tools Platform: ESP Platform: This PR/issue effects ESP-based platforms State: waiting for CI update State: The PR requires an Update to CI to be performed first State: waiting for other PR State: The PR requires another PR to be merged first
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants