RIOT icon indicating copy to clipboard operation
RIOT copied to clipboard

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

Open gschorcht opened this issue 2 years ago • 6 comments

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 904d5c3e719252e23c42aab79e42487939c9d6e2.

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 avatar Jun 09 '22 12:06 gschorcht

@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: https://github.com/RIOT-OS/RIOT/blob/c6b890cc809f4634847118d59e3be85aca406865/boards/esp32s3-devkit/Makefile.features#L1-L28

I tried the same in Kconfig https://github.com/RIOT-OS/RIOT/blob/c6b890cc809f4634847118d59e3be85aca406865/boards/esp32s3-devkit/Kconfig#L8-L56 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 avatar Jun 26 '22 08:06 gschorcht

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?

leandrolanzieri avatar Jun 27 '22 07:06 leandrolanzieri

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.

gschorcht avatar Jun 27 '22 14:06 gschorcht

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?

leandrolanzieri avatar Jun 28 '22 07:06 leandrolanzieri

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 avatar Jun 28 '22 07:06 gschorcht

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.

gschorcht avatar Jun 28 '22 07:06 gschorcht

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

gschorcht avatar Aug 27 '22 16:08 gschorcht