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: fix problems with ESP WiFi and migration to ztimer #17427

Merged

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Dec 19, 2021

Contribution description

This PR fixes the problem with all types of ESP WiFi netdev's (esp_now | esp_wifi | esp_wifi_ap = esp_wifi_any) that arose with the migration to ztimer.

With PR #17386, ESP implementations were migrated to ztimer. If periph_rtt is provided as feature, ztimer uses by default periph_rtt whenever possible.

Unfortunately, the periph_rtt implementation for ESP8266 is quite tricky and uses a combination of the completely undocumented ESP8266 RTC module and the FRC2 counter to support deep sleep. However, this FRC2 counter also seems to be used by the binary SDK libraries for the WiFi interface once it is enabled by any ESP WiFi netdev (esp_wifi_any). I already pointed out this problem when I provided the periph_rtt implementation (#13640 (comment) and #13640 (comment)). Due to the fact that the SDK libraries for the WiFi interface are closed-source and completely undocumented, nobody knows what actually happens in these libraries.

As a result of this conflict, ztimer must not use periph_rtt when esp_wifi_any is used. Therefore, ztimer_no_periph_rtt is enabled if esp_wifi_any is used. Since xtimer always used periph_timer instead, we never ran into a problem with xtimer and esp_wifi_any.

This PR also includes a fix for the linker script. ztimer_core functions have to reside in IRAM for timing reasons and to be available also when the IROM cache is disabled. Although the module is called ztimer, its object files are generated in directory ztimer_core.

Kconfig already takes the migration of ESP WiFi netdev into account.

Testing procedure

With commit fe72971 two test applications have been provided for compilation tests. They are copies of tests/shell but enable additionally ztimer and ztimer + esp_wifi, repsectively. app.config.test files also enable these modules. It will be removed before merge.

  1. Compilation should succeed in CI.

  2. Use for example commands:

    BOARD=esp8266-esp-12x make -C tests/shell_ztimer info-modules
    BOARD=esp8266-esp-12x make -C tests/shell_ztimer_esp_wifi info-modules
    

    and compare enabled modules:

    • ztimer_periph_rtt and periph_rtt are used for tests/shell_ztimer w/o esp_wifi
    • ztimer_no periph_rtt and periph_timer are used for tests/shell_ztimer_esp_wifi with esp_wifi

Issues/PRs references

Fixes #17386

`ztimer_core` functions have to reside in IRAM for timing reasons and to be available also when the IROM cache is disabled. Although the module is called `ztimer`, its object files are generated in directory `ztimer_core`.
If the WiFi interface is enabled by module `esp_wifi_any`, binary SDK libraries use the RTT. Therefore, `ztimer` must not use `periph_rtt`as backend, if the WiFi interface is enabled by module `esp_wifi_any`.
@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Area: Kconfig Area: Kconfig integration Platform: ESP Platform: This PR/issue effects ESP-based platforms labels Dec 19, 2021
@gschorcht gschorcht added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Dec 19, 2021
@github-actions github-actions bot added Area: CI Area: Continuous Integration of RIOT components Area: doc Area: Documentation Area: tests Area: tests and testing framework labels Dec 19, 2021
@gschorcht gschorcht force-pushed the cpu/esp8266/fix_ztimer_esp_wifi_problem branch from 446053c to d6b5945 Compare December 19, 2021 16:17
@gschorcht gschorcht changed the title cpu/esp8266: fix the esp_wifi_any problem arose with migration to ztimer cpu/esp8266: fix the ESP WiFi problems arose with migration to ztimer Dec 19, 2021
@gschorcht gschorcht force-pushed the cpu/esp8266/fix_ztimer_esp_wifi_problem branch 2 times, most recently from dff483f to fe72971 Compare December 19, 2021 17:30
@gschorcht gschorcht changed the title cpu/esp8266: fix the ESP WiFi problems arose with migration to ztimer cpu/esp8266: fix the ESP WiFi problems with migration to ztimer Dec 19, 2021
@gschorcht gschorcht changed the title cpu/esp8266: fix the ESP WiFi problems with migration to ztimer cpu/esp8266: fix the ESP WiFi problems and migration to ztimer Dec 19, 2021
Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

ACK. Trusting your testing.

@benpicco benpicco added this to the Release 2022.01 milestone Dec 19, 2021
@gschorcht gschorcht force-pushed the cpu/esp8266/fix_ztimer_esp_wifi_problem branch from fe72971 to 48f59a1 Compare December 20, 2021 07:54
@github-actions github-actions bot removed the Area: CI Area: Continuous Integration of RIOT components label Dec 20, 2021
@github-actions github-actions bot removed Area: doc Area: Documentation Area: tests Area: tests and testing framework labels Dec 20, 2021
@gschorcht gschorcht changed the title cpu/esp8266: fix the ESP WiFi problems and migration to ztimer cpu/esp8266: fix problems with ESP WiFi and migration to ztimer Dec 20, 2021
@benpicco benpicco merged commit f9e3196 into RIOT-OS:master Dec 21, 2021
@gschorcht gschorcht deleted the cpu/esp8266/fix_ztimer_esp_wifi_problem branch December 21, 2021 13:27
@gschorcht
Copy link
Contributor Author

Thanks for reviewing and merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports Area: Kconfig Area: Kconfig integration 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: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants