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/esp32: platform independent code for thread_arch, irq_arch and exception #18247

Closed

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Jun 23, 2022

Contribution description

This PR is a splitt-off from PR #17841. It provides a platform independent implementation of thread_arch.c, irq_arch.c and exception.c so that it can be used for Xtensa-based and RISC-V based ESP32x SoC variants.

For that purpose the platform dependent code is moved from thread_arch.c, irq_arch.c, exception.c to thread_arch_xtensa.c, irq_arch_xtensa.c, exception_xtensa.c and thread_arch_riscv.c, irq_arch_riscv.c, exception.c_riscv, respectively. Furthermore, some DEBUG messages are fixed to be platform independent.

Background:

  • The file thread_arch_xtensa.c is not really independent of the used ESP SoC. Although ESP8266 and ESP32 share most of the code, that's why thread_arch_xtensa.c is defined in cpu/esp_common, they require some SoC specific code, especially in thread_yield_* functions. These functions are something very special to ESP SoCs and not common for Xtensa cores.
  • We have to use vectors.S for for RISC-V interrupt handling and portasm.S for RISC-V context switching from ESP-IDF, because they are needed by other ESP-IDF functions, e.g. by the startup function, which we also use directly from ESP-IDF. Unfortunately, they are not compatible with the RISC-V implementation in cpu/riscv_common. They use a different context frame structure, a different interrupt context structure, a different interrupt/exception handling and a different startup function. Even if we could reuse some parts of riscv_common, including a common folder in the compilation always means all or nothing. Therefore we can't use cpu/riscv_common at all.
  • Another problem is that the implementation of irq_disable/irq_restore in riscv_common only reset/set the MIE bit in mstatus register while the implementation for RISC-V based ESPs requires to change the value of the SoC specific interrupt level register INTERRUPT_CORE0_CPU_INT_THRESH_REG.

Thus, neither is the Xtensa code general enough to justify a cpu/xtensa_common folder, nor are the thread or interrupt handling for RISC-V based ESP compatible with the implementation in riscv_common.
Therefore, thread_arch.c, irq_arch.c, exception.c to thread_arch_xtensa.c, irq_arch_xtensa.c, exception_xtensa.c and thread_arch_riscv.c, irq_arch_riscv.c, exception.c_riscv, respectively.

Testing procedure

  1. Green CI
  2. Compile and check any simple test app, for example:
    BOARD=esp32-wroom-32 make -j8 -C tests/shell flash term
    

Issues/PRs references

Split-off from PR #17841

Xtensa-specific code was moved from thread_arch.c to thread_arch_xtensa.c. RISC-V code has been added to thread_arch_riscv.c. This allows to handle both Xtensa and RISC-V based ESP SoCs.
Xtensa-specific code was moved from irq_arch.c to irq_arch_xtensa.c. RISC-V code has been added to irq_arch_riscv.c. This allows to handle both Xtensa and RISC-V based ESP SoCs.
@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Platform: ESP Platform: This PR/issue effects ESP-based platforms labels Jun 23, 2022
@gschorcht
Copy link
Contributor Author

If necessary, the PR could be split into seperate PRs.

@gschorcht gschorcht added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation and removed Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Jun 23, 2022
@gschorcht gschorcht changed the title Cpu/esp32/platform independent code cpu/esp32: platform independent code for thread_arch, irq_arch and exception Jun 23, 2022
@gschorcht
Copy link
Contributor Author

gschorcht commented Jun 25, 2022

@benpicco The PR still seems to have a lot of changes, but

  • thread_arch_xtensa.c is just the code that has been removed from thread_arch.c.
  • irq_arch_xtensa.c is just the code that has been removed from irq_arch.c.
  • exception_xtensa.c is just the former exception.c.

The files thread_arch_riscv.c, irq_arch_riscv.c and exception_riscv.c are new.

If it helps, I could further split this PR into two PRs, one with the changes that move platform dependent code to the platform dependent files *_xtensa.c and one which adds the platform dependent files *_riscv.c.

The changes in cpu/esp32/* are the changes to use ESP-IDF HAL for interrupt handling and could also be split-off into a separate PR.

@gschorcht gschorcht added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Jun 25, 2022
@gschorcht
Copy link
Contributor Author

Closed in favor of PRs #18259, #18260 and #18261

@gschorcht gschorcht closed this Jun 25, 2022
@gschorcht gschorcht deleted the cpu/esp32/platform_independent_code branch July 18, 2022 04:53
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 Platform: ESP Platform: This PR/issue effects ESP-based platforms State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet 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.

1 participant