Fix read-modify-write race condition

imxrt-rs · Dec 21, 2023 · 64c3af9 · 64c3af9
1 parent 956dbc5
commit 64c3af9
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 22 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -45,6 +45,9 @@ version = "0.5"
 default-features = false
 optional = true
 
+[dependencies.cortex-m]
+version = "0.7"
+
 #######################
 # imxrt-rs dependencies
 #######################
@@ -131,7 +134,6 @@ codegen-units = 256
 ######################################
 
 [dev-dependencies]
-cortex-m = "0.7"
 imxrt-rt = { workspace = true }
 menu = "0.3.2"
 cortex-m-rtic = "1.0"

diff --git a/src/common/gpio.rs b/src/common/gpio.rs
@@ -37,6 +37,8 @@
 
 use crate::{iomuxc, ral};
 
+use cortex_m::interrupt;
+
 /// GPIO ports.
 pub struct Port<const N: u8> {
     gpio: ral::gpio::Instance<N>,
@@ -91,31 +93,36 @@ impl<const N: u8> Port<N> {
 
     /// Set the GPIO input interrupt trigger for the provided input pin.
     fn set_interrupt_trigger<P>(&mut self, pin: &Input<P>, trigger: Trigger) {
-        if Trigger::EitherEdge == trigger {
-            ral::modify_reg!(ral::gpio, self.gpio, EDGE_SEL, |edge_sel| {
-                edge_sel | pin.mask()
-            });
-        } else {
-            ral::modify_reg!(ral::gpio, self.gpio, EDGE_SEL, |edge_sel| {
-                edge_sel & !pin.mask()
-            });
-            let icr = trigger as u32;
-            let icr_modify = |reg| reg & !(0b11 << pin.icr_offset()) | (icr << pin.icr_offset());
-            if pin.offset < 16 {
-                ral::modify_reg!(ral::gpio, self.gpio, ICR1, icr_modify);
+        interrupt::free(|_| {
+            if Trigger::EitherEdge == trigger {
+                ral::modify_reg!(ral::gpio, self.gpio, EDGE_SEL, |edge_sel| {
+                    edge_sel | pin.mask()
+                });
             } else {
-                ral::modify_reg!(ral::gpio, self.gpio, ICR2, icr_modify);
+                ral::modify_reg!(ral::gpio, self.gpio, EDGE_SEL, |edge_sel| {
+                    edge_sel & !pin.mask()
+                });
+                let icr = trigger as u32;
+                let icr_modify =
+                    |reg| reg & !(0b11 << pin.icr_offset()) | (icr << pin.icr_offset());
+                if pin.offset < 16 {
+                    ral::modify_reg!(ral::gpio, self.gpio, ICR1, icr_modify);
+                } else {
+                    ral::modify_reg!(ral::gpio, self.gpio, ICR2, icr_modify);
+                }
             }
-        }
+        })
     }
 
     /// Enable (`true`) or disable (`false`) interrupt generation.
     fn set_interrupt_enable<P>(&mut self, pin: &Input<P>, enable: bool) {
-        if enable {
-            ral::modify_reg!(ral::gpio, self.gpio, IMR, |imr| imr | pin.mask());
-        } else {
-            ral::modify_reg!(ral::gpio, self.gpio, IMR, |imr| imr & !pin.mask());
-        }
+        interrupt::free(|_| {
+            if enable {
+                ral::modify_reg!(ral::gpio, self.gpio, IMR, |imr| imr | pin.mask());
+            } else {
+                ral::modify_reg!(ral::gpio, self.gpio, IMR, |imr| imr & !pin.mask());
+            }
+        })
     }
 }
 
@@ -136,7 +143,9 @@ unsafe impl<P: Send> Send for Output<P> {}
 impl<P> Output<P> {
     fn new(pin: P, gpio: &'static ral::gpio::RegisterBlock, offset: u32) -> Self {
         let output = Self { pin, gpio, offset };
-        ral::modify_reg!(ral::gpio, gpio, GDIR, |gdir| gdir | output.mask());
+        interrupt::free(|_| {
+            ral::modify_reg!(ral::gpio, gpio, GDIR, |gdir| gdir | output.mask());
+        });
         output
     }
 
@@ -236,7 +245,9 @@ pub enum Trigger {
 impl<P> Input<P> {
     fn new(pin: P, gpio: &'static ral::gpio::RegisterBlock, offset: u32) -> Self {
         let input = Self { pin, gpio, offset };
-        ral::modify_reg!(ral::gpio, gpio, GDIR, |gdir| gdir & !input.mask());
+        interrupt::free(|_| {
+            ral::modify_reg!(ral::gpio, gpio, GDIR, |gdir| gdir & !input.mask());
+        });
         input
     }