Merge #688

688: Fix generated code for MSP430 atomics r=burrbull a=YuhanLiin

Rebased from #685


Co-authored-by: YuhanLiin <[email protected]>

Author: bors[bot]

.github/workflows/ci.yml

@@ -74,7 +74,8 @@ jobs:
           # Test MSRV
           - { rust: 1.60.0, vendor: Nordic, options: "" }
           # Use nightly for architectures which don't support stable
-          - { rust: nightly, vendor: OTHER, options: "" }
+          - { rust: nightly, vendor: MSP430, options: "--nightly" }
+          - { rust: nightly, vendor: MSP430, options: "" }
           - { rust: nightly, vendor: Espressif, options: "" }
 
     steps:

CHANGELOG.md

@@ -12,8 +12,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [v0.27.1] - 2022-10-25
 
-- fix cli error with --help/version
+- Fix cli error with --help/version
 - Don't cast fields with width 17-31 and non-zero offset.
+- Fix generated code for MSP430 atomics
 
 ## [v0.27.0] - 2022-10-24
 

ci/script.sh

@@ -460,13 +460,17 @@ main() {
             # test_svd LPC5410x_v0.4
         ;;
 
-        # test other targets (architectures)
-        OTHER)
+        # MSP430
+        MSP430)
             echo '[dependencies.msp430]' >> $td/Cargo.toml
-            echo 'version = "0.3.0"' >> $td/Cargo.toml
+            echo 'version = "0.4.0"' >> $td/Cargo.toml
+
+            echo '[dependencies.portable-atomic]' >> $td/Cargo.toml
+            echo 'version = "0.3.15"' >> $td/Cargo.toml
 
             # Test MSP430
             test_svd_for_target msp430 https://raw.githubusercontent.com/pftbest/msp430g2553/v0.3.0-svd/msp430g2553.svd
+            test_svd_for_target msp430 https://raw.githubusercontent.com/YuhanLiin/msp430fr2355/master/msp430fr2355.svd
         ;;
 
         # Community-provided RISC-V SVDs

ci/svd2rust-regress/src/svd_test.rs

@@ -6,8 +6,8 @@ use std::path::PathBuf;
 use std::process::{Command, Output};
 
 const CRATES_ALL: &[&str] = &["critical-section = \"1.0\"", "vcell = \"0.1.2\""];
-const CRATES_MSP430: &[&str] = &["msp430 = \"0.2.2\"", "msp430-rt = \"0.2.0\""];
-const CRATES_MSP430_NIGHTLY: &[&str] = &["msp430-atomic = \"0.1.2\""];
+const CRATES_MSP430: &[&str] = &["msp430 = \"0.4.0\"", "msp430-rt = \"0.4.0\""];
+const CRATES_MSP430_NIGHTLY: &[&str] = &["portable-atomic = \"0.3.15\""];
 const CRATES_CORTEX_M: &[&str] = &["cortex-m = \"0.7.6\"", "cortex-m-rt = \"0.6.13\""];
 const CRATES_RISCV: &[&str] = &["riscv = \"0.9.0\"", "riscv-rt = \"0.9.0\""];
 const CRATES_XTENSALX: &[&str] = &["xtensa-lx-rt = \"0.9.0\"", "xtensa-lx = \"0.6.0\""];

src/generate/generic_msp430_atomic.rs

@@ -1,51 +1,91 @@
-use msp430_atomic::AtomicOperations;
+mod atomic {
+    use portable_atomic::{AtomicU16, AtomicU8, Ordering};
 
-impl<REG: Writable> Reg<REG>
+    pub trait AtomicOperations {
+        unsafe fn atomic_or(ptr: *mut Self, val: Self);
+        unsafe fn atomic_and(ptr: *mut Self, val: Self);
+        unsafe fn atomic_xor(ptr: *mut Self, val: Self);
+    }
+
+    macro_rules! impl_atomics {
+        ($U:ty, $Atomic:ty) => {
+            impl AtomicOperations for $U {
+                unsafe fn atomic_or(ptr: *mut Self, val: Self) {
+                    (*(ptr as *const $Atomic)).fetch_or(val, Ordering::SeqCst);
+                }
+
+                unsafe fn atomic_and(ptr: *mut Self, val: Self) {
+                    (*(ptr as *const $Atomic)).fetch_and(val, Ordering::SeqCst);
+                }
+
+                unsafe fn atomic_xor(ptr: *mut Self, val: Self) {
+                    (*(ptr as *const $Atomic)).fetch_xor(val, Ordering::SeqCst);
+                }
+            }
+        };
+    }
+    impl_atomics!(u8, AtomicU8);
+    impl_atomics!(u16, AtomicU16);
+}
+use atomic::AtomicOperations;
+
+impl<REG: Readable + Writable> Reg<REG>
 where
-    Self: Readable + Writable,
     REG::Ux: AtomicOperations + Default + core::ops::Not<Output = REG::Ux>,
 {
     /// Set high every bit in the register that was set in the write proxy. Leave other bits
     /// untouched. The write is done in a single atomic instruction.
+    ///
+    /// # Safety
+    ///
+    /// The resultant bit pattern may not be valid for the register.
     #[inline(always)]
     pub unsafe fn set_bits<F>(&self, f: F)
     where
-        F: FnOnce(&mut W<REG>) -> &mut W<REG>,
+        F: FnOnce(&mut REG::Writer) -> &mut W<REG>,
     {
-        let bits = f(&mut W {
+        let bits = f(&mut REG::Writer::from(W {
             bits: Default::default(),
             _reg: marker::PhantomData,
-        })
+        }))
         .bits;
         REG::Ux::atomic_or(self.register.as_ptr(), bits);
     }
 
     /// Clear every bit in the register that was cleared in the write proxy. Leave other bits
     /// untouched. The write is done in a single atomic instruction.
+    ///
+    /// # Safety
+    ///
+    /// The resultant bit pattern may not be valid for the register.
     #[inline(always)]
     pub unsafe fn clear_bits<F>(&self, f: F)
     where
-        F: FnOnce(&mut W<REG>) -> &mut W<REG>,
+        F: FnOnce(&mut REG::Writer) -> &mut W<REG>,
     {
-        let bits = f(&mut W {
+        let bits = f(&mut REG::Writer::from(W {
             bits: !REG::Ux::default(),
             _reg: marker::PhantomData,
-        })
+        }))
         .bits;
         REG::Ux::atomic_and(self.register.as_ptr(), bits);
     }
 
     /// Toggle every bit in the register that was set in the write proxy. Leave other bits
     /// untouched. The write is done in a single atomic instruction.
+    ///
+    /// # Safety
+    ///
+    /// The resultant bit pattern may not be valid for the register.
     #[inline(always)]
     pub unsafe fn toggle_bits<F>(&self, f: F)
     where
-        F: FnOnce(&mut W<REG>) -> &mut W<REG>,
+        F: FnOnce(&mut REG::Writer) -> &mut W<REG>,
     {
-        let bits = f(&mut W {
+        let bits = f(&mut REG::Writer::from(W {
             bits: Default::default(),
             _reg: marker::PhantomData,
-        })
+        }))
         .bits;
         REG::Ux::atomic_xor(self.register.as_ptr(), bits);
     }

src/lib.rs

@@ -122,21 +122,21 @@
 //! - [`msp430-rt`](https://crates.io/crates/msp430-rt) v0.4.x
 //! - [`vcell`](https://crates.io/crates/vcell) v0.1.x
 //!
-//! If the `--nightly` flag is provided to `svd2rust`, then `msp430-atomic` v0.1.4 is also needed.
+//! If the `--nightly` flag is provided to `svd2rust`, then `portable-atomic` v0.3.15 is also needed.
 //! Furthermore the "device" feature of `msp430-rt` must be enabled when the `rt` feature is
 //! enabled. The `Cargo.toml` of the device crate will look like this:
 //!
 //! ``` toml
 //! [dependencies]
 //! critical-section = { version = "1.0", optional = true }
 //! msp430 = "0.4.0"
-//! msp430-atomic = "0.1.4" # Only when using the --nightly flag
+//! portable-atomic = "0.3.15" # Only when using the --nightly flag
 //! msp430-rt = { version = "0.4.0", optional = true }
 //! vcell = "0.1.0"
 //!
 //! [features]
 //! rt = ["msp430-rt/device"]
-//! unstable = ["msp430-atomic"]
+//! unstable = ["portable-atomic"]
 //! ```
 //!
 //! ## Other targets
@@ -504,8 +504,8 @@
 //! ```ignore
 //! // These can be called from different contexts even though they are modifying the same register
 //! P1.p1out.set_bits(|w| unsafe { w.bits(1 << 1) });
-//! P1.p1out.clear(|w| unsafe { w.bits(!(1 << 2)) });
-//! P1.p1out.toggle(|w| unsafe { w.bits(1 << 4) });
+//! P1.p1out.clear_bits(|w| unsafe { w.bits(!(1 << 2)) });
+//! P1.p1out.toggle_bits(|w| unsafe { w.bits(1 << 4) });
 //! ```
 #![recursion_limit = "128"]