max/embassy
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

rp/pio: don't use modify on shared registers

Browse Source 
pio control registers are notionally shared between state machines as
well. state machine operations that change these registers must use
atomic accesses (or critical sections, which would be overkill).

notably PioPin::set_input_sync_bypass was even wrong, enabling the
bypass on a pin requires the corresponding bit to be set (not cleared).
the PioCommon function got it right.
This commit is contained in:
pennae 2023-04-25 23:17:57 +02:00
parent 0d224a00e1
commit 6cec6fa09b
1 changed files with 19 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
embassy-rp/src/pio.rs
View File

@ -298,9 +298,11 @@ impl<PIO: PioInstance> PioPin<PIO> {
pub fn set_input_sync_bypass<'a>(&mut self, bypass: bool) { pub fn set_input_sync_bypass<'a>(&mut self, bypass: bool) {
let mask = 1 << self.pin(); let mask = 1 << self.pin();
unsafe { unsafe {
PIO::PIO if bypass {
.input_sync_bypass() PIO::PIO.input_sync_bypass().write_set(|w| *w = mask);
.modify(|w| *w = if bypass { *w & !mask } else { *w | mask }); } else {
PIO::PIO.input_sync_bypass().write_clear(|w| *w = mask);
}
} }
} }
@ -336,18 +338,19 @@ pub trait PioStateMachine: sealed::PioStateMachine + Sized + Unpin {
} }
fn restart(&mut self) { fn restart(&mut self) {
let mask = 1u8 << Self::Sm::SM_NO;
unsafe { unsafe {
Self::Pio::PIO Self::Pio::PIO.ctrl().write_set(|w| w.set_sm_restart(mask));
.ctrl()
.modify(|w| w.set_sm_restart(1u8 << Self::Sm::SM_NO));
} }
} }
fn set_enable(&mut self, enable: bool) { fn set_enable(&mut self, enable: bool) {
let mask = 1u8 << Self::Sm::SM_NO; let mask = 1u8 << Self::Sm::SM_NO;
unsafe { unsafe {
Self::Pio::PIO if enable {
.ctrl() Self::Pio::PIO.ctrl().write_set(|w| w.set_sm_enable(mask));
.modify(|w| w.set_sm_enable((w.sm_enable() & !mask) | (if enable { mask } else { 0 }))); } else {
Self::Pio::PIO.ctrl().write_clear(|w| w.set_sm_enable(mask));
}
} }
} }
@ -419,10 +422,9 @@ pub trait PioStateMachine: sealed::PioStateMachine + Sized + Unpin {
} }
fn clkdiv_restart(&mut self) { fn clkdiv_restart(&mut self) {
let mask = 1u8 << Self::Sm::SM_NO;
unsafe { unsafe {
Self::Pio::PIO Self::Pio::PIO.ctrl().write_set(|w| w.set_clkdiv_restart(mask));
.ctrl()
.modify(|w| w.set_clkdiv_restart(1u8 << Self::Sm::SM_NO));
} }
} }
@ -869,9 +871,11 @@ pub trait PioCommon: sealed::PioCommon + Sized {
fn set_input_sync_bypass<'a>(&'a mut self, bypass: u32, mask: u32) { fn set_input_sync_bypass<'a>(&'a mut self, bypass: u32, mask: u32) {
unsafe { unsafe {
Self::Pio::PIO // this can interfere with per-pin bypass functions. splitting the
.input_sync_bypass() // modification is going to be fine since nothing that relies on
.modify(|w| *w = (*w & !mask) | (bypass & mask)); // it can reasonably run before we finish.
Self::Pio::PIO.input_sync_bypass().write_set(|w| *w = mask & bypass);
Self::Pio::PIO.input_sync_bypass().write_clear(|w| *w = mask & !bypass);
} }
} }