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

feat: introduce compile error if compiling with atomic on #8

Merged
merged 1 commit into from
Nov 3, 2024

Conversation

Dummyc0m
Copy link
Member

@Dummyc0m Dummyc0m commented Nov 3, 2024

This change introduce a compile error if the toolchain is using a target with atomics. It looks like at least on the the QingKe V4 atomics are suspiciously broken.

There is also a feature you can enable unsafe-trust-wch-atomics you can enable to remove this compile error

See issue: ch32-rs/ch32-hal#59

@Dummyc0m Dummyc0m requested a review from andelf November 3, 2024 00:50
Codetector1374 added a commit to vapor-keeb/ch32-hal that referenced this pull request Nov 3, 2024
This introduce a breaking change where the atomic is no longer availiable
DMA can no longer be using atomic, so critical section is used instead.

As discovered in ch32-rs#59, the QingKeV4 atomic implementation is likely
broken. As a result we added a compiler check to make sure the atomic
exetnsion is disabled in ch32-rs/qingke#8. This change updates the
dependency to use the new qingke as well as remove any reference to
`core::atomic` in ch32-hal.
Codetector1374 added a commit to vapor-keeb/ch32-hal that referenced this pull request Nov 3, 2024
This change remvoes all use of atomic from ch32-hal

As discovered in ch32-rs#59, the QingKeV4 atomic implementation is likely
broken. As a result we added a compiler check to make sure the atomic
exetnsion is disabled in ch32-rs/qingke#8. This change updates the
dependency to use the new qingke as well as remove any reference to
`core::atomic` in ch32-hal.
Codetector1374 added a commit to vapor-keeb/ch32-hal that referenced this pull request Nov 3, 2024
This change remvoes all use of atomic from ch32-hal

As discovered in ch32-rs#59, the QingKeV4 atomic implementation is likely
broken. As a result we added a compiler check to make sure the atomic
exetnsion is disabled in ch32-rs/qingke#8. This change updates the
dependency to use the new qingke as well as remove any reference to
`core::atomic` in ch32-hal.
@Codetector1374 Codetector1374 force-pushed the main branch 2 times, most recently from bd48f68 to bdf5198 Compare November 3, 2024 03:26
This change introduce a compile error if the toolchain is using a target
with atomics. It looks like at least on the the QingKe V4 atomics are
suspiciously broken.

There is also a feature you can enable
`unsafe-trust-wch-atomics` you can enable to remove
this compile error

See issue: ch32-rs/ch32-hal#59
Copy link
Contributor

@andelf andelf left a comment

Choose a reason for hiding this comment

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

LGTM.
Agree that "unsafe-x" warning is the right signal here.

@andelf andelf merged commit 75dbd95 into ch32-rs:main Nov 3, 2024
1 check passed
@andelf
Copy link
Contributor

andelf commented Nov 3, 2024

Published as v0.5.0

Codetector1374 added a commit to vapor-keeb/ch32-hal that referenced this pull request Nov 3, 2024
This change remvoes all use of atomic (CAS) from ch32-hal

As discovered in ch32-rs#59, the QingKeV4 atomic implementation is likely
broken. As a result we added a compiler check to make sure the atomic
exetnsion is disabled in ch32-rs/qingke#8. This change updates the
dependency to use the new qingke as well as remove any reference to
`core::atomic` in ch32-hal.
Codetector1374 added a commit to vapor-keeb/ch32-hal that referenced this pull request Nov 3, 2024
This change remvoes all use of atomic (CAS) from ch32-hal

As discovered in ch32-rs#59, the QingKeV4 atomic implementation is likely
broken. As a result we added a compiler check to make sure the atomic
exetnsion is disabled in ch32-rs/qingke#8. This change updates the
dependency to use the new qingke as well as remove any reference to
`core::atomic` in ch32-hal.
Dummyc0m pushed a commit to vapor-keeb/ch32-hal that referenced this pull request Nov 4, 2024
This change remvoes all use of atomic (CAS) from ch32-hal

As discovered in ch32-rs#59, the QingKeV4 atomic implementation is likely
broken. As a result we added a compiler check to make sure the atomic
exetnsion is disabled in ch32-rs/qingke#8. This change updates the
dependency to use the new qingke as well as remove any reference to
`core::atomic` in ch32-hal.
andelf pushed a commit to ch32-rs/ch32-hal that referenced this pull request Nov 4, 2024
This change remvoes all use of atomic (CAS) from ch32-hal

As discovered in #59, the QingKeV4 atomic implementation is likely
broken. As a result we added a compiler check to make sure the atomic
exetnsion is disabled in ch32-rs/qingke#8. This change updates the
dependency to use the new qingke as well as remove any reference to
`core::atomic` in ch32-hal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants