Skip to content

Commit

Permalink
gpio: Fix GPIO high-speed state inconsistency
Browse files Browse the repository at this point in the history
There are two valid ways to prepare a high-speed GPIO output:

```rust
pub fn configure_led(pad: LedPadType) -> LED {
    let mut led = hal::gpio::GPIO::new(pad);
    led.set_fast(true);
    led.output()
}
```

and

```rust
pub fn configure_led(pad: LedPadType) -> LED {
    let mut led = hal::gpio::GPIO::new(pad).output();
    led.set_fast(true);
    led
}
```

the former will put the GPIO into high-speed, or 'fast,' mode before
setting 'output mode.' The latter will put the GPIO into output mode
before fast mode.

After transitioning into fast mode, the GPIO will start to reference
a different GPIO register block. The issue is that, after entering
fast mode, the output / input state of the pin is not maintained. In
the second snippet, the set_fast(true) call ends up reverting the
GPIO state back to 'input,' since the newly-referenced register block
does not maintain the same GPIO input / output configuration.

This commit updates the set_fast() method to maintain the GPIO I/O
state for the new register block. It makes it so that either of the
above patterns work.
  • Loading branch information
mciantyre committed Sep 23, 2020
1 parent bd1b2df commit 887a8e1
Showing 1 changed file with 37 additions and 11 deletions.
48 changes: 37 additions & 11 deletions imxrt-hal/src/gpio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,20 +110,54 @@ where
pub fn set_fast(&mut self, fast: bool) -> bool {
self.gpr()
.map(|gpr| {
cortex_m::interrupt::free(|_| unsafe {
cortex_m::interrupt::free(|cs| unsafe {
let v = core::ptr::read_volatile(gpr);

let was_output = self.is_output();

if fast {
core::ptr::write_volatile(gpr, v | self.offset());
} else {
core::ptr::write_volatile(gpr, v & !self.offset());
}

// At this point, calls to set_output / set_input will refer to the 'fast'
// or 'slow' GPIO register block.
if was_output {
self.set_output(cs);
} else {
self.set_input(cs);
}

true
})
})
.unwrap_or(false)
}

/// Returns `true` if the pin is configured as an output pin
fn is_output(&self) -> bool {
// Safety: atomic read
unsafe { ral::read_reg!(ral::gpio, self.register_block(), GDIR) & self.offset() != 0 }
}

/// Configure the GPIO as an output
fn set_output(&self, _: &cortex_m::interrupt::CriticalSection) {
// Safety: critical section, enforced by API, ensures consistency
unsafe {
ral::modify_reg!(ral::gpio, self.register_block(), GDIR, |gdir| gdir
| self.offset());
}
}

/// Configure the GPIO as an input
fn set_input(&self, _: &cortex_m::interrupt::CriticalSection) {
// Safety: critical section, enforced by API, ensures consistency
unsafe {
ral::modify_reg!(ral::gpio, self.register_block(), GDIR, |gdir| gdir
& !self.offset());
}
}
}

impl<P> GPIO<P, Input>
Expand All @@ -143,11 +177,7 @@ where

/// Set the GPIO as an output
pub fn output(self) -> GPIO<P, Output> {
// Safety: critical section ensures consistency
cortex_m::interrupt::free(|_| unsafe {
ral::modify_reg!(ral::gpio, self.register_block(), GDIR, |gdir| gdir
| self.offset());
});
cortex_m::interrupt::free(|cs| self.set_output(cs));
GPIO {
pin: self.pin,
dir: PhantomData,
Expand All @@ -167,11 +197,7 @@ where
{
/// Transition the pin back to an input
pub fn input(self) -> GPIO<P, Input> {
// Safety: critical section ensures consistency
cortex_m::interrupt::free(|_| unsafe {
ral::modify_reg!(ral::gpio, self.register_block(), GDIR, |gdir| gdir
& !self.offset());
});
cortex_m::interrupt::free(|cs| self.set_input(cs));
GPIO {
pin: self.pin,
dir: PhantomData,
Expand Down

0 comments on commit 887a8e1

Please sign in to comment.