Completely remove atomic emulation

[MabezDev]

This PR completely removes atomic emulation. Now that the +forced-atomics feature is enabled on -a RISCV targets, the trap is less useful. Most crates in the embedded ecosystem that need cas will opt to use portable-atomic to support targets without cas.

Must

• The code compiles without errors or warnings.
• All examples work.
• cargo fmt was run.
• Your changes were added to the CHANGELOG.md in the proper section.
• You updated existing examples or added examples (if applicable).
• Added examples are checked in CI

Nice to have

• You add a description of your work to this PR.
• [~] You added proper docs for your newly added features and code.

[bugadani]

FYI smartled is checked with the CI host as target, that causes the portable-atomic madness (can't enable unsafe-assume-single-core on x86). Though if this came up, we shouldn't enable unsafe-assume-single-core anyway as per portable-atomic recommendation.

[MabezDev]

I opened rust-lang/futures-rs#2805 to add esp32s2 target to the no cas list. This was important as embassy-sync depends on futures-util.

As @bugadani mentioned we need everything to move to heapless 0.8, this requires a release of embassy-sync at a minimum, but for esp-wifi we know we'll need more.

[jessebraham]

I opened rust-lang/futures-rs#2805 to add esp32s2 target to the no cas list. This was important as embassy-sync depends on futures-util.

Well, so much for that 😅

[MabezDev]

I found a way around it, though we should eventually fix it upstream. The last step is to remove the patched embassy_sync with a new release.

[MabezDev]

With heapless 0.7.17 released we can punt the embassy upgrades to a different PR. This is now ready for review!

[jessebraham]

LGTM, thank you for resolving this 🙏🏻

[jessebraham]

With heapless 0.7.17 released we can punt the embassy upgrades to a different PR. This is now ready for review!

Handling this in #994

[bugadani]

Apparently we shouldn't enable these in a library:

critical-section
...
Note:
...
It is usually not recommended to always enable this feature in dependencies of the library.

[MabezDev]

I read that too, however, I feel that although esp-hal is a library, its also not a platform-agnostic library, we know exactly what the intended target is capable of. If it causes problems in the future we can revert this.

[brychanrobot]

Was this tested on esp32c6? I just tried to compile a project that runs on the c6 with embassy and it failed with

       consider enabling one of the `unsafe-assume-single-core` or `critical-section` Cargo features.
       see <https://docs.rs/portable-atomic/latest/portable_atomic/#optional-features> for more.
   --> /home/bryant/.cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.6.0/src/lib.rs:434:1
    |
434 | / compile_error!(
435 | |     "dependents require atomic CAS but not available on this target by default;\n\
436 | |     consider enabling one of the `unsafe-assume-single-core` or `critical-section` Cargo features.\n\
437 | |     see <https://docs.rs/portable-atomic/latest/portable_atomic/#optional-features> for more."
438 | | );
    | |_^

I noticed that unsafe-assume-single-core is enabled on the c3, but c6 doesn't use either of these features.

[bugadani]

Yes, this works as intended. The C6 did not use the atomic emulation mechanism at all. Don't enable unsafe-assume-single-core, C6 has native support for atomic operations and portable-atomic should be able to pick that up.

Could you share your project?

[brychanrobot]

Interesting. Thanks! In case anyone else has the same problem and comes across this thread. It turns out that I had converted a project from c3 to c6 and missed updating the target string. riscv32imc-unknown-none-elf -> riscv32imac-unknown-none-elf.