-
Notifications
You must be signed in to change notification settings - Fork 218
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
Add basic esp32p4
support
#2466
base: main
Are you sure you want to change the base?
Conversation
1c8ee8b
to
85abc3a
Compare
I don't think we want to include this in this release, so converting to draft for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I have some initial comments, mostly around the maintainability of adding more support in the future - we don't want to spend a bunch of time working on something only to realise the function that should be doing something is stubbed out to do nothing :D.
I'll dig out my P4 rev1 board and try the examples soon.
Regarding |
Yeah let's have those errors, I'll move PeripheralMarker to a better place. Alternatively, you could define an axi_gdma and ahb_gdma symbol pair, and cfg-enable dma for those without gdma/pdma parts. |
7237709
to
55f48fe
Compare
# "systimer", | ||
# "timg0", | ||
# "timg1", | ||
"systimer", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did systimer and timg work out of the box? Can we enable the examples/tests(I know we can't run them yet) for them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can get the examples to even work, but we need to implement interrupt handling. I'm starting to do that right now.
examples/.cargo/config.toml
Outdated
esp32s2 = "run --release --features=esp32s2 --target=xtensa-esp32s2-none-elf" | ||
esp32s3 = "run --release --features=esp32s3 --target=xtensa-esp32s3-none-elf" | ||
|
||
[target.'cfg(target_arch = "riscv32")'] | ||
runner = "espflash flash --monitor" | ||
runner = "espflash flash --monitor --no-stub" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit pick: We probably still want to use the stub for other chips, can we duplicate this and specify target = "riscv32imafc-unknown-none-elf"
to only pass --no-stub
on the P4?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I'm not even sure why is it there.
It doesn't affect a program run/flash in any sense 🤷
55f48fe
to
c4f583e
Compare
Submission Checklist 📝
cargo xtask fmt-packages
command to ensure that all changed code is formatted correctly.CHANGELOG.md
in the proper section.Extra:
Pull Request Details 📖
Description
For now, there're still some
TODO
s left. They'll be incrementally removed asp4
support grows. For example, for now some of checks inCI
are off, some code sections and so on.related to #2285
Testing
advanced_serial
example, some more basic tests like: