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/esp_common: use ESP-IDF random API for ESP32 #18277

Merged
merged 1 commit into from
Jul 4, 2022

Conversation

gschorcht
Copy link
Contributor

Contribution description

This PR is a split-off from PR #17841 and provides the changes to use the ESP-IDF random API for periph/hwrng. This RNG uses the noise in the RF system of the WiFi or the BT interface as entropy source. If neither WiFi nor BT are used, an internal non-RF entropy source is used, the internal reference voltage noise.

Testing procedure

Compile and test

BOARD=esp32-wroom-32 make -j8 -C tests/periph_hwrng/ flash term

Issues/PRs references

Split-off from PR #17841

@gschorcht gschorcht requested a review from benpicco June 28, 2022 17:44
@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Platform: ESP Platform: This PR/issue effects ESP-based platforms labels Jun 28, 2022
@gschorcht gschorcht added Type: enhancement The issue suggests enhanceable parts / The PR enhances 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 Jun 28, 2022
}

#if defined(MCU_ESP8266) || !defined(ESP32_USE_ESP_IDF_RANDOM)
Copy link
Contributor

@benpicco benpicco Jun 28, 2022

Choose a reason for hiding this comment

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

Suggested change
#if defined(MCU_ESP8266) || !defined(ESP32_USE_ESP_IDF_RANDOM)
#if !IS_USED(ESP32_USE_ESP_IDF_RANDOM)

Copy link
Contributor

@benpicco benpicco Jun 28, 2022

Choose a reason for hiding this comment

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

Or just split the file between esp32 and esp8266 if that's possible

Copy link
Contributor Author

@gschorcht gschorcht Jun 29, 2022

Choose a reason for hiding this comment

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

Ok, I can move old implementation to cpu/esp8266 and remove it in cpu/esp32. There is no reason to enable the old implementation for ESP32 because the new implementation is better because it works for all ESP32x SoC variants and gives better entropy. It is easy to split it. May I squash the split directly?

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.

If you provide a ESP32_USE_ESP_IDF_RANDOM define we might as well use it to allow switching to the 'old' implementation if desired. Since it's already unset for the esp8266 case we don't need to check for it twice.

Apart for the nitpick ACK, just squash directly

@gschorcht gschorcht force-pushed the cpu/esp/periph_hal_esp32_hwrng branch from 6d1b39e to 02310b1 Compare June 30, 2022 09:10
@maribu maribu 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 Jul 4, 2022
@maribu maribu enabled auto-merge July 4, 2022 12:35
@maribu maribu added CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs 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 Jul 4, 2022
@maribu
Copy link
Member

maribu commented Jul 4, 2022

The usual false positive due to mismatching checksums of an unrelated build (not even an ESP32 or ESP8266 board)

@maribu maribu merged commit d337997 into RIOT-OS:master Jul 4, 2022
@gschorcht gschorcht deleted the cpu/esp/periph_hal_esp32_hwrng branch July 15, 2022 14:14
@gschorcht gschorcht restored the cpu/esp/periph_hal_esp32_hwrng branch July 15, 2022 14:14
@gschorcht gschorcht deleted the cpu/esp/periph_hal_esp32_hwrng branch July 15, 2022 14:16
@chrysn chrysn added this to the Release 2022.07 milestone Aug 25, 2022
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 CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Platform: ESP Platform: This PR/issue effects ESP-based platforms 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