NaxdyOrg/embassy
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

gp/gpio: fix InputFuture edge waits

Browse source 
InputFuture did not use and check edge interrupts correctly.
InterruptTrigger should've checked for not 1,2,3,4 but 1,2,4,8 since the
inte fields are bitmasks, and not clearing INTR would have repeatedly
triggered edge interrupts early.
This commit is contained in:
pennae 2023-05-02 08:29:32 +02:00
parent 05c36e05f9
commit c6424fdc11
1 changed files with 43 additions and 83 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

126
embassy-rp/src/gpio.rs
View file

@ -136,18 +136,6 @@ pub enum InterruptTrigger {
AnyEdge,
}
impl InterruptTrigger {
fn from_u32(value: u32) -> Option<InterruptTrigger> {
match value {
1 => Some(InterruptTrigger::LevelLow),
2 => Some(InterruptTrigger::LevelHigh),
3 => Some(InterruptTrigger::EdgeLow),
4 => Some(InterruptTrigger::EdgeHigh),
_ => None,
}
}
}
#[interrupt]
unsafe fn IO_IRQ_BANK0() {
let cpu = SIO.cpuid().read() as usize;
@ -166,26 +154,16 @@ unsafe fn IO_IRQ_BANK0() {
let pin_group = (pin % 8) as usize;
let event = (intsx.read().0 >> pin_group * 4) & 0xf as u32;
if let Some(trigger) = InterruptTrigger::from_u32(event) {
// no more than one event can be awaited per pin at any given time, so
// we can just clear all interrupt enables for that pin without having
// to check which event was signalled.
if event != 0 {
critical_section::with(|_| {
proc_intx.inte(pin / 8).modify(|w| match trigger {
InterruptTrigger::AnyEdge => {
w.set_edge_high(pin_group, false);
w.set_edge_low(pin_group, false);
}
InterruptTrigger::LevelHigh => {
trace!("IO_IRQ_BANK0 pin {} LevelHigh triggered", pin);
w.set_level_high(pin_group, false);
}
InterruptTrigger::LevelLow => {
w.set_level_low(pin_group, false);
}
InterruptTrigger::EdgeHigh => {
w.set_edge_high(pin_group, false);
}
InterruptTrigger::EdgeLow => {
w.set_edge_low(pin_group, false);
}
proc_intx.inte(pin / 8).modify(|w| {
w.set_edge_high(pin_group, false);
w.set_edge_low(pin_group, false);
w.set_level_high(pin_group, false);
w.set_level_low(pin_group, false);
});
});
INTERRUPT_WAKERS[pin as usize].wake();
@ -207,10 +185,23 @@ impl<'d, T: Pin> InputFuture<'d, T> {
irq.disable();
irq.set_priority(interrupt::Priority::P3);
let pin_group = (pin.pin() % 8) as usize;
// first, clear the INTR register bits. without this INTR will still
// contain reports of previous edges, causing the IRQ to fire early
// on stale state. clearing these means that we can only detect edges
// that occur *after* the clear happened, but since both this and the
// alternative are fundamentally racy it's probably fine.
// (the alternative being checking the current level and waiting for
// its inverse, but that requires reading the current level and thus
// missing anything that happened before the level was read.)
pac::IO_BANK0.intr(pin.pin() as usize / 8).write(|w| {
w.set_edge_high(pin_group, true);
w.set_edge_low(pin_group, true);
});
// Each INTR register is divided into 8 groups, one group for each
// pin, and each group consists of LEVEL_LOW, LEVEL_HIGH, EDGE_LOW,
// and EGDE_HIGH.
let pin_group = (pin.pin() % 8) as usize;
critical_section::with(|_| {
pin.int_proc().inte((pin.pin() / 8) as usize).modify(|w| match level {
InterruptTrigger::LevelHigh => {
@ -227,7 +218,8 @@ impl<'d, T: Pin> InputFuture<'d, T> {
w.set_edge_low(pin_group, true);
}
InterruptTrigger::AnyEdge => {
// noop
w.set_edge_high(pin_group, true);
w.set_edge_low(pin_group, true);
}
});
});
@ -257,47 +249,21 @@ impl<'d, T: Pin> Future for InputFuture<'d, T> {
// LEVEL_HIGH, EDGE_LOW, and EDGE_HIGH for each pin.
let pin_group = (self.pin.pin() % 8) as usize;
// This should check the the level of the interrupt trigger level of
// the pin and if it has been disabled that means it was done by the
// interrupt service routine, so we then know that the event/trigger
// happened and Poll::Ready will be returned.
trace!("{:?} for pin {}", self.level, self.pin.pin());
match self.level {
InterruptTrigger::AnyEdge => {
if !inte.edge_high(pin_group) && !inte.edge_low(pin_group) {
#[rustfmt::skip]
trace!("{:?} for pin {} was cleared, return Poll::Ready", self.level, self.pin.pin());
return Poll::Ready(());
}
}
InterruptTrigger::LevelHigh => {
if !inte.level_high(pin_group) {
#[rustfmt::skip]
trace!("{:?} for pin {} was cleared, return Poll::Ready", self.level, self.pin.pin());
return Poll::Ready(());
}
}
InterruptTrigger::LevelLow => {
if !inte.level_low(pin_group) {
#[rustfmt::skip]
trace!("{:?} for pin {} was cleared, return Poll::Ready", self.level, self.pin.pin());
return Poll::Ready(());
}
}
InterruptTrigger::EdgeHigh => {
if !inte.edge_high(pin_group) {
#[rustfmt::skip]
trace!("{:?} for pin {} was cleared, return Poll::Ready", self.level, self.pin.pin());
return Poll::Ready(());
}
}
InterruptTrigger::EdgeLow => {
if !inte.edge_low(pin_group) {
#[rustfmt::skip]
trace!("{:?} for pin {} was cleared, return Poll::Ready", self.level, self.pin.pin());
return Poll::Ready(());
}
}
// since the interrupt handler clears all INTE flags we'll check that
// all have been cleared and unconditionally return Ready(()) if so.
// we don't need further handshaking since only a single event wait
// is possible for any given pin at any given time.
if !inte.edge_high(pin_group)
&& !inte.edge_low(pin_group)
&& !inte.level_high(pin_group)
&& !inte.level_low(pin_group)
{
trace!(
"{:?} for pin {} was cleared, return Poll::Ready",
self.level,
self.pin.pin()
);
return Poll::Ready(());
}
trace!("InputFuture::poll return Poll::Pending");
Poll::Pending
@ -644,23 +610,17 @@ impl<'d, T: Pin> Flex<'d, T> {
#[inline]
pub async fn wait_for_rising_edge(&mut self) {
self.wait_for_low().await;
self.wait_for_high().await;
InputFuture::new(&mut self.pin, InterruptTrigger::EdgeHigh).await;
}
#[inline]
pub async fn wait_for_falling_edge(&mut self) {
self.wait_for_high().await;
self.wait_for_low().await;
InputFuture::new(&mut self.pin, InterruptTrigger::EdgeLow).await;
}
#[inline]
pub async fn wait_for_any_edge(&mut self) {
if self.is_high() {
self.wait_for_low().await;
} else {
self.wait_for_high().await;
}
InputFuture::new(&mut self.pin, InterruptTrigger::AnyEdge).await;
}
}