Adding support for the ESP32C3 #733

onsdagens · 2023-04-14T11:24:03Z

As discussed, this adds support for the ESP32C3.

It relies on forks of both the runtime and the hal for interrupt preemption, my next goal is possibly getting those upstreamed as well.

onsdagens · 2023-04-14T12:22:20Z

As a sidenote, this should generalize well to the other ESP RISC-V MCUs but i don't have any to play around with.

onsdagens · 2023-04-15T16:32:35Z

@romancardenas has suggested splitting up export like here https://github.com/greenlsi/rtic/tree/master/rtic%2Fsrc%2Fexport for better modularity. I think for now it might be overkill, but depending on how many unique backends we expect to support, it could be a good idea.

romancardenas · 2023-04-15T16:57:12Z

If you don't like all those submodule folders, we could also leave it as export::riscv_common, export::riscv_esp32c3... And forget about things like export::riscv_backend::esp32c3.

In any case, I do believe that we should move the feature-gated definitions of functions like pend from rtic::export.rs to a backend-specific module for easier maintenance and better separation of concerns.

onsdagens · 2023-04-19T13:15:51Z

I've been able to move things around a bit, now this only relies on changes to the hal, the rt remains untouched. I've also fixed some weird errors regarding feature selection, which i'm not sure how they made it in there to begin with.

romancardenas · 2023-04-19T17:02:07Z

@steewbsd and I have re-structured the export module of our fork so it has less folders. Have a look, maybe you prefer it that way.

romancardenas

Nice piece of work! I see we more or less reached the same point. Let's try to clarify the structure stuff and then I will adapt our fork to include yours :D

romancardenas · 2023-04-20T08:49:59Z

rtic-macros/src/codegen/bindings/cortex.rs

@@ -323,6 +323,14 @@ pub fn interrupt_exit(_app: &App, _analysis: &CodegenAnalysis) -> Vec<TokenStrea
    vec![]
 }

+pub fn async_entry(


I guess this new hook is required by ESP32C3 for un-pending the dispatchers' interrupt, right?

romancardenas · 2023-04-20T08:51:48Z

rtic-macros/src/codegen/main.rs

@@ -20,7 +20,7 @@ pub fn codegen(app: &App, analysis: &Analysis) -> TokenStream2 {
        quote!(#dispatcher();)
    } else {
        quote!(loop {
-            rtic::export::nop()
+            continue;


Just curious: why do you avoid the nop function?

It feels like it achieves the same end result (this is just an empty idle generated when you don't give one), without the need for exporting nop. I guess there is no specific reason, just the solution i came up with.

If this is effectively equivalent, then I'm in

romancardenas · 2023-04-20T08:54:12Z

rtic-macros/src/codegen/util.rs

@@ -6,11 +6,20 @@ use syn::{Ident, PatType};

 const RTIC_INTERNAL: &str = "__rtic_internal";

+//this is named something else in the esp32c3 pac
+#[cfg(not(feature = "riscv-esp32c3"))]


Check this. We had a similar issue and tackled this way. The idea is to avoid feature gates out of codegen::bindings.

romancardenas · 2023-04-20T08:55:26Z

rtic/Cargo.toml

+esp32c3 = {version = "0.13.0", optional = true}
+#next goal is getting the neccessary changes to the hal upstreamed
+#esp32c3-hal = { git = "https://github.com/onsdagens/esp-hal", branch="scaled_back", optional = true}
+esp32c3-hal = { git = "https://github.com/onsdagens/esp-hal", branch="interrupt_preemption", optional = true}


I see you depend both on the PAC and the HAL. Could you explain to me why? Maybe we need this as well for the SLIC implementation

romancardenas · 2023-04-20T08:57:31Z

rtic/src/export.rs

+#[cfg(feature = "riscv-esp32c3")]
+pub use riscv_esp32c3::*;
+
+///I think all of these "pends" and "unpends"  should be moved to /export/<device>.rs


Same. We propose this structure.

Yep, i'll align this with your proposal

romancardenas · 2023-04-20T09:01:18Z

rtic/src/export/riscv_esp32c3.rs

@@ -0,0 +1,105 @@
+use esp32c3::INTERRUPT_CORE0; //priority threshold control
+pub use esp32c3::{Interrupt, Peripherals};


I see you set your core peripherals as the PAC's peripherals. So this is basically what we have done. Thus, the core peripherals and device peripherals will be exactly the same. This is why I wonder if the core peripherals are needed for RISC-V targets, as there is no such thing. Maybe we can make it optional and include it only for Cortex. However, I think @korken89 would rather not modify struct fields depending on the backend.

I don't think that API is set in stone - if there is a good reason to deviate I see no reason to strictly follow the old one :)

romancardenas

LGTM.

As a side note, it would be worth discussing removing from the API rtic::export::peripherals and rtic::export::nop at the next meeting. The prior looks too Cortex-M-driven, and the latter can just be continue.

onsdagens · 2023-08-03T20:12:58Z

Some breaking changes if you've been using this fork. I've moved it to use the new upcoming direct-vectoring feature in the esp-riscv-rt. This changes the syntax of the interrupt handlers slightly, and takes advantage of the interrupt vector table, improving task dispatch latency dramatically.

If you're using the esp32c3-hal, you must enable the interrupt-preemption and direct-vectoring features, and make sure you're using the latest version of the crate from the esp-hal repo. Similarly, if you're only running esp-riscv-rt, you must enable those same features.

For some minimal examples check out https://github.com/perlindgren/esp32c3-test/tree/rtic_sw_tasks

onsdagens · 2023-09-26T23:02:51Z

Rebased on upstream now with Per adding a drop-in replacement of atomics in rtic-common and rtic-sync with portable-atomic to move us away from the imac target and the shaky emulation it requires. There is also a new, less PoC, example in https://github.com/perlindgren/esp32c3-test/tree/rtic_sw_tasks using the UsbSerialJtag peripheral to communicate with the host, and a more advanced example here https://github.com/perlindgren/esp32c3-rtic-tau , with more things to come.

perlindgren · 2023-09-27T18:37:57Z

Manually testing for toxic effects of portable_atomic.

extern "C" fn test_atomics(offset: u8) -> AtomicU8 {
    (84 + offset).into()
}

42000174 <test_atomics>:
42000174: 13 05 45 05  	addi	a0, a0, 84
42000178: 13 75 f5 0f  	andi	a0, a0, 255
4200017c: 82 80            	ret

It's not a complete test of course, but it looks as expected (compiled in release mode, without overflow checks).

There is no reason to believe any other natively supported atomic data structures should cause problems.

korken89

LGTM, thank you for the effort!

The merge-base changed after approval.

esp32c3 support

onsdagens marked this pull request as ready for review April 15, 2023 16:32

romancardenas reviewed Apr 20, 2023

View reviewed changes

romancardenas mentioned this pull request Apr 27, 2023

(partially) removing core peripherals from RTIC context #746

Closed

romancardenas previously approved these changes May 3, 2023

View reviewed changes

romancardenas mentioned this pull request May 3, 2023

Removing nop from rtic::export #747

Closed

onsdagens dismissed romancardenas’s stale review via 381d2a1 May 5, 2023 13:09

korken89 previously approved these changes Sep 27, 2023

View reviewed changes

onsdagens force-pushed the master branch from 874dc52 to a86eb86 Compare September 27, 2023 19:11

onsdagens force-pushed the master branch from a86eb86 to 874dc52 Compare September 27, 2023 19:18

esp32c3 support

b7f3d49

onsdagens closed this Sep 27, 2023

onsdagens force-pushed the master branch from 874dc52 to 3b8d787 Compare September 27, 2023 19:41

Merge pull request #10 from onsdagens/squash

f1c4c71

esp32c3 support

onsdagens reopened this Sep 27, 2023

korken89 approved these changes Sep 27, 2023

View reviewed changes

korken89 added this pull request to the merge queue Sep 27, 2023

Merged via the queue into rtic-rs:master with commit 2b2208e Sep 27, 2023
88 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for the ESP32C3 #733

Adding support for the ESP32C3 #733

onsdagens commented Apr 14, 2023

onsdagens commented Apr 14, 2023

onsdagens commented Apr 15, 2023 •

edited

Loading

romancardenas commented Apr 15, 2023 •

edited

Loading

onsdagens commented Apr 19, 2023

romancardenas commented Apr 19, 2023

romancardenas left a comment

romancardenas Apr 20, 2023

onsdagens Apr 20, 2023

romancardenas Apr 20, 2023

onsdagens Apr 20, 2023

romancardenas Apr 20, 2023

romancardenas Apr 20, 2023

romancardenas Apr 20, 2023

romancardenas Apr 20, 2023

onsdagens Apr 20, 2023

romancardenas Apr 20, 2023

korken89 Sep 27, 2023

romancardenas left a comment

onsdagens commented Aug 3, 2023

onsdagens commented Sep 26, 2023 •

edited

Loading

perlindgren commented Sep 27, 2023

korken89 left a comment

		@@ -0,0 +1,105 @@
		use esp32c3::INTERRUPT_CORE0; //priority threshold control
		pub use esp32c3::{Interrupt, Peripherals};

Adding support for the ESP32C3 #733

Adding support for the ESP32C3 #733

Conversation

onsdagens commented Apr 14, 2023

onsdagens commented Apr 14, 2023

onsdagens commented Apr 15, 2023 • edited Loading

romancardenas commented Apr 15, 2023 • edited Loading

onsdagens commented Apr 19, 2023

romancardenas commented Apr 19, 2023

romancardenas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romancardenas left a comment

Choose a reason for hiding this comment

onsdagens commented Aug 3, 2023

onsdagens commented Sep 26, 2023 • edited Loading

perlindgren commented Sep 27, 2023

korken89 left a comment

Choose a reason for hiding this comment

onsdagens commented Apr 15, 2023 •

edited

Loading

romancardenas commented Apr 15, 2023 •

edited

Loading

onsdagens commented Sep 26, 2023 •

edited

Loading