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*: Xtensa vendor code moved to esp_common #10883

Merged
merged 13 commits into from
Apr 15, 2019

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Jan 27, 2019

Contribution description

This PR contains all the changes required to use the same vendor code for context, exception, and interrupt handling for ESP8266 and ESP32.

Testing procedure

tests/thread_basic should still work for an ESP8266 board as well as an ESP32 board.

Issues/PRs references

Depends on #10981
This PR is part of refactoring ESP8266 and ESP32 code in issue #10658.

@gschorcht gschorcht added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Platform: ESP Platform: This PR/issue effects ESP-based platforms Area: cpu Area: CPU/MCU ports labels Jan 27, 2019
Copy link
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

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

nice consolidation

cpu/esp32/thread_arch.c Show resolved Hide resolved
@smlng smlng added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 4, 2019
Copy link
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

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

mostly doc-related, i.e. use @name for groups instead of @brief ... though this was overseen when merging the original code.

cpu/esp32/include/irq_arch.h Outdated Show resolved Hide resolved
cpu/esp32/include/irq_arch.h Outdated Show resolved Hide resolved
cpu/esp32/include/irq_arch.h Outdated Show resolved Hide resolved
@gschorcht
Copy link
Contributor Author

@smlng Changed.

@gschorcht
Copy link
Contributor Author

@smlng Too late for the 2019.04 release but not too late for next release 😄 I would like to make some progress with this PR (as well as with #10929) since these PRs provide important improvements that are prerequisites for the reimplementation of ESP8266 port in PR #11108.

Sorry that I'm pushing a bit 😎 but the conflicts with current master in PR #11108 are already increasing a lot. It will become more and more difficult to resolve all these conflicts while guaranteeing that the reimplementation PR #11108 keeps as stable as it already was. It was already tested completely.

Copy link
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

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

tested ACK!

@smlng smlng added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Apr 15, 2019
@smlng
Copy link
Member

smlng commented Apr 15, 2019

please rebase and squash as needed

@gschorcht
Copy link
Contributor Author

@sming Thanks for reviewing and testing it again. Rebased and squashed.

@smlng
Copy link
Member

smlng commented Apr 15, 2019

Side note: careful with my GitHub name, its smlng with lower case L not i, i.e. no vowels. The other guy (sming) already got annoyed a couple times 😬

@smlng
Copy link
Member

smlng commented Apr 15, 2019

CI is not happy, some whitespace stuff, please amend fix directly

@gschorcht
Copy link
Contributor Author

Side note: careful with my GitHub name, its smlng with lower case L not i, i.e. no vowels. The other guy (sming) already got annoyed a couple times

Ups 😎 Usually, I only type in 'sm' and use the completion which is usually 'smlng'. I didn't take much care. I thought Github is intelligent enough to know from the context of the PR that I didn't want to refer 'sming'.

Usually, the access to the IROM (flash) memory requires 32-bit word aligned reads. Attempts to access data in the IROM (flash) memory less than 32 bits in size triggers a LoadStoreError exception. With the exception handler from esp-open-rtos it becomes possible to access data in IROM (flash) with a size of less than 32 bits and thus to place .rodata sections in the IROM (flash).
@gschorcht
Copy link
Contributor Author

@smlng Fixed and all light are green.

@smlng
Copy link
Member

smlng commented Apr 15, 2019

retested, all good!

@smlng smlng merged commit 35c617e into RIOT-OS:master Apr 15, 2019
@gschorcht gschorcht deleted the esp_common_xtensa branch April 15, 2019 11:30
@gschorcht
Copy link
Contributor Author

@smlng Thanks again for your fast support.

@smlng
Copy link
Member

smlng commented Apr 15, 2019

@smlng Thanks again for your fast support.

Sure thing. I now have a couple of ESP lying around at work and home, so I'm happy to help where ever I can to keep the RIOT support for ESP up and running (and improving)!

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 Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants