arch/xtensa: unify WiFi source code for ESP32|S2|S3

[fdcavalcanti]

Summary

This PR modifies ESP32 and ESP32S3 to use WiFi source code from the common layer, which is already used by ESP32S2.
Overall there's a massive reduction in source code since WiFi support is now centered around the common layer for the Xtensa devices.

Impact

KConfig prefixes related to radio functionalities have been modified. ESP32_* and ESP32S3_* are replaced with ESPRESSIF_* (CONFIG_ESP32_WIFI -> CONFIG_ESPRESSIF_WIFI). This is applied through arch/ and also board/ and may impact users if they are running custom defconfigs.

BLE symbols are altered as well, since they share some common wireless implementations. However, source code for BLE has not been merged into the common layer.

Reduces the amount of source code required for WiFi support.
No difference in performance.

Testing

Internal CI testing.

[nuttxpr]

[Experimental Bot, please feedback here]

This PR mostly meets the NuttX requirements but is missing some crucial information. Here's a breakdown:

Strengths:

• Clear Summary: The summary explains the what and why of the change well. The mention of Kconfig prefix changes is helpful.
• Impact - Feature Change: Clearly states the reduction in code duplication.
• Impact - Performance: Explicitly states no performance impact, which is important.

Weaknesses:

• Summary - How: While the what is clear, the how is lacking detail. It mentions using the common layer, but doesn't explain the mechanism of code sharing (e.g., file moves, symbolic links, etc.).
• Missing Issue References: No related NuttX issue(s) are referenced. Even if there isn't a directly related issue, it's good practice to create one to document the motivation and discussion around the change.
• Impact - User: While stating "no difference in performance" addresses one aspect of user impact, it doesn't cover potential configuration changes. The Kconfig prefix changes will impact users, as they will need to adjust their configurations. This needs to be explicitly acknowledged and documented. The PR should explain how users should migrate their existing configurations.
• Impact - Build: The Kconfig changes definitely impact the build process. This needs to be described. At minimum, it should mention the configuration changes required.
• Impact - Hardware: Even if no architectural changes occur, explicitly stating which architectures (ESP32, ESP32S3) and boards are affected is important.
• Impact - Documentation: Given the Kconfig changes, documentation updates are almost certainly required. This needs to be acknowledged, and ideally, the PR should include the updated documentation or a separate PR for documentation should be mentioned.
• Impact - Security/Compatibility: Even if there's no impact, explicitly stating "NO" for these sections is better than leaving them blank.
• Testing - Insufficient Detail: "Internal CI testing" is too vague. Specific target architectures, boards, and configurations tested should be listed. Ideally, snippets of relevant log output demonstrating the functionality before and after the change should be included (even if just a few key lines). The build host details are also missing.

Recommendations for Improvement:

1. Elaborate on the "How": Explain the technical implementation details of using the common WiFi code.
2. Link to a NuttX Issue: Create an issue describing the motivation for this change and link it to the PR.
3. Detail User Impact: Explicitly describe the Kconfig changes and provide instructions for users to migrate their configurations.
4. Detail Build Impact: Explain the build changes due to Kconfig modifications.
5. Specify Affected Hardware: List the specific architectures (ESP32, ESP32S3) and boards impacted.
6. Address Documentation: Explain how the documentation is updated (or will be updated in a separate PR).
7. Explicitly address Security and Compatibility: Even if there's no impact, state "NO" for clarity.
8. Provide Detailed Testing Information: Specify the build hosts, target architectures, boards, configurations, and include relevant log snippets.

By addressing these weaknesses, the PR will be much stronger and better adhere to the NuttX requirements.

[acassis]

@fdcavalcanti please include the option to use the original driver without using the HAL

[fdcavalcanti]

Hey @acassis

That is not an option.

This PR is not a register based to HAL based WiFi upgrade.
In fact, there's no big change in the source code used for WiFi, we are only sharing the same source across devices.

[acassis]

Hey @acassis

That is not an option.

This PR is not a register based to HAL based WiFi upgrade. In fact, there's no big change in the source code used for WiFi, we are only sharing the same source across devices.

Hi @fdcavalcanti so after this commit the original driver will not be possible to use?

It is important to keep the original drivers that don't depend on HAL, the let the users to have alternative. As @raiden00pl pointed in the Discord channel, using the HAL could be against INVIOLABLES: https://github.com/apache/nuttx/blob/master/INVIOLABLES.md#no-short-cuts

[fdcavalcanti]

Before this commit we used HAL. After this commit we are still using HAL.
I don't know what you mean by "original driver". Are you referring to register based WiFi? Because we don't have that.

[acassis]

Before this commit we used HAL. After this commit we are still using HAL. I don't know what you mean by "original driver". Are you referring to register based WiFi? Because we don't have that.

Understood, I thought it was using the native driver like ESP32. So this driver just unify S2 and S3 to use a common driver, right?

[fdcavalcanti]

Yes. It unifies ESP32|S2|S3.

There is still one arch specific file for each of those: *_wifi_adapter.c.
*wifi_utils.c, *wireless.c and *wlan.c are the files unified.

[fdcavalcanti]

@acassis Do you have something else to suggest changes? If not please remove the change request from the PR 😃

[tmedicci]

@acassis , wasn't the answer from @eren-terzioglu satisfactory?

If so, can you please remove the "Requested changes" if there aren't other pending changes?

[acassis]

@tmedicci I just approve it. I didn't realize that the original WiFi driver was removed and replaced by the new that depends on the HAL :-(