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/{esp8266,esp32}: cleanup of SPI Flash configuration #18387

Merged
merged 6 commits into from
Aug 5, 2022

Conversation

gschorcht
Copy link
Contributor

Contribution description

This PR fixes some inconsistencies in the definition of the correct SPI flash parameters from the FLASH_* variables. In detail it contains the following changes:

  • For ESP32x SoC, esptool.py is has to be called with --flash_mode dio when any Quad SPI mode qout or qio is used to let the first stage bootloader always start in Dual SPI mode. That is, dio has to be set as FLASH_MODE variable in makefile/tools/esptool.py if the FLASH_MODE variable is set to qio or dio.
  • FLASH_MODE_* defines are replaced by CONFIG_FLASHMODE_* and CONFIG_ESPTOOLPY_FLASHMODE_* defines to be compatible with ESP-IDF. Furthermore, this will also needed for the migration of defining the flash mode in Kconfig. Defining them CFLAGS in the makefile make them visible in whole make system.
  • The default flash mode in bootloader has to be dio instead of dout.
  • The default flash configuration defined by the FLASH_* variables has to be exported for ESP32 to be also visible in cpu/esp32/bootloader/Makefile.
  • The comments in flash configuration defined by the FLASH_* in cpu/esp8266/Makefile.include have to be removed so that the FLASH_* variables do not contain trailing white spaces.
  • Flash mode 'qout' is no longer required for SPI-RAM.

Testing procedure

  1. Use a esp32-wroom-32 and compile and flash with following command.

    FLASH_MODE=dout FLASH_FREQ=80m FLASH_SIZE=4 USEMODULE='esp_log_startup' \
    BOARD=esp32-wrover-kit make -j8 -C tests/malloc flash term PORT=/dev/ttyUSB0
    

    The output should be something like the following:

    rst:0x1 (POWERON_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
    configsip: 0, SPIWP:0xee
    clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
    mode:DOUT, clock div:1
    load:0x3fff0030,len:6120
    load:0x40078000,len:13732
    load:0x40080400,len:3492
    entry 0x4008041c
    I (27) boot: ESP-IDF v4.4.1 2nd stage bootloader
    I (27) boot: chip revision: 1
    I (27) boot_comm: chip revision: 1, min. bootloader chip revision: 0
    I (30) boot.esp32: SPI Speed      : 80MHz
    I (33) boot.esp32: SPI Mode       : DOUT
    I (37) boot.esp32: SPI Flash Size : 4MB
    

    That is, DOUT is used for the first stage bootloader and the second stage bootloader as well.

  2. Use a esp32-wroom-32 and compile and flash with following command.

    FLASH_MODE=qio FLASH_FREQ=80m FLASH_SIZE=4 USEMODULE='esp_log_startup' \
    BOARD=esp32-wrover-kit make -j8 -C tests/malloc flash term PORT=/dev/ttyUSB0
    

    The output should be something like the following:

    rst:0x1 (POWERON_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
    configsip: 0, SPIWP:0xee
    clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
    mode:DIO, clock div:1
    load:0x3fff0030,len:6436
    load:0x40078000,len:14436
    load:0x40080400,len:4192
    entry 0x4008041c
    I (27) boot: ESP-IDF v4.4.1 2nd stage bootloader
    I (27) boot: chip revision: 1
    I (27) boot_comm: chip revision: 1, min. bootloader chip revision: 0
    I (30) qio_mode: Enabling default flash chip QIO
    I (34) boot.esp32: SPI Speed      : 80MHz
    I (38) boot.esp32: SPI Mode       : QIO
    I (41) boot.esp32: SPI Flash Size : 4MB
    

    That is, DIO is used for the first stage bootloader and QIO for the second stage bootloader.

Issues/PRs references

If Quad SPI modes qout or qio are set by variable FLASH_MODE, esptool.py has to be called with parameter `--flash_mode dio` so that the first stage bootloader is always using Dual SPI mode.
The former FLASH_MODE_{DOUT,DIO,QOUT,QIO} defines are replaced by the corresponding CONFIG_FLASHMODE_{DOUT,DIO,QOUT,QIO} and CONFIG_ESPTOOLPY_FLASHMODE_{DOUT,DIO,QOUT,QIO} as used by the ESP-IDF. This is also needed for the migration of defining flash mode in Kconfig.
@github-actions github-actions bot added Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: tools Area: Supplementary tools Platform: ESP Platform: This PR/issue effects ESP-based platforms labels Aug 1, 2022
@gschorcht gschorcht added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 1, 2022
@gschorcht
Copy link
Contributor Author

@benpicco I found some inconsistencies in using FLASH_MODE variables that have to be fixed. This PR should be merged before all subsequent PRs for ESP2-C3, ESP2-S3 and ESP32-S2 because it affects all of them.

@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 1, 2022
@miri64
Copy link
Member

miri64 commented Aug 1, 2022

Requeued in Murdock queue to give 2022.07 release backport #18354 precedence.

@gschorcht gschorcht force-pushed the cpu/esp32/cleanup_spi_flash_mode branch from d3ec4bc to a4869c1 Compare August 1, 2022 16:10
The former FLASH_MODE_{DOUT,DIO,QOUT,QIO} defines are replaced by the corresponding CONFIG_FLASHMODE_{DOUT,DIO,QOUT,QIO} and CONFIG_ESPTOOLPY_FLASHMODE_{DOUT,DIO,QOUT,QIO} as used by the ESP-IDF. This is also needed for the migration of defining flash mode in Kconfig.
This commit includes the following changes:
- the default flash mode in bootloader should be dio and not dout
- the default flash configuration for ESP32 has to be exported to be also visible in cpu/esp32/bootloader/Makefile
- the comments in  cpu/esp8266/Makefile.include have to be removed so that the flash configuration do not contain trailing white spaces
CONFIG_ESPTOOL_FLASH_FREQ_* defines are used in ESP-IDF to select the right SPI clock speed for flash.
@gschorcht gschorcht force-pushed the cpu/esp32/cleanup_spi_flash_mode branch from a4869c1 to da39354 Compare August 2, 2022 05:25
@github-actions github-actions bot added the Area: boards Area: Board ports label Aug 2, 2022
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

esp8266 also still works

@benpicco benpicco enabled auto-merge August 5, 2022 10:24
@benpicco benpicco merged commit cf745e9 into RIOT-OS:master Aug 5, 2022
@gschorcht
Copy link
Contributor Author

@benpicco Thanks for reviewing and merging.

@gschorcht gschorcht deleted the cpu/esp32/cleanup_spi_flash_mode branch August 8, 2022 07:34
@maribu maribu added this to the Release 2022.10 milestone Oct 14, 2022
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: cpu Area: CPU/MCU ports Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants