Skip to content

Commit

Permalink
SPI/ITM fixes (#351)
Browse files Browse the repository at this point in the history
This fixes the following bugs:

- Spi.exchange() hangs if RX bytes exceeds TX bytes (#341)
- ITM broken on Nucleo H743/H753 boards (#346)
- SPI overruns are not handled, resulting in infinite loop (#349)
- SPI can suffer from overruns due to TX/RX asymmetry (#350)
    
In addition to fixing these bugs, this work removes the Gimlet hostflash
task from the H743/H753 boards -- and replaces it with the venerable
"ping" task (which demonstrates a panicking task).
  • Loading branch information
bcantrill committed Jan 10, 2022
1 parent 6f7998a commit 1af71c3
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 30 deletions.
26 changes: 12 additions & 14 deletions app/demo-stm32h7-nucleo/app-h743.toml
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,16 @@ requires = {flash = 8192, ram = 1024}
start = true
task-slots = ["gpio_driver"]

[tasks.ping]
path = "../../task/ping"
name = "task-ping"
features = ["uart"]
priority = 4
requires = {flash = 8192, ram = 512}
stacksize = 512
start = true
task-slots = [{peer = "pong"}, "usart_driver"]

[tasks.pong]
path = "../../task/pong"
name = "task-pong"
Expand All @@ -131,24 +141,12 @@ task-slots = ["user_leds"]
[tasks.hiffy]
path = "../../task/hiffy"
name = "task-hiffy"
features = ["h743", "stm32h7", "itm", "i2c", "gpio", "qspi"]
features = ["h743", "stm32h7", "itm", "i2c", "gpio", "spi"]
priority = 3
requires = {flash = 32768, ram = 32768 }
stacksize = 2048
start = true
task-slots = ["gpio_driver", "hf", "i2c_driver"]

[tasks.hf]
path = "../../drv/gimlet-hf-server"
name = "drv-gimlet-hf-server"
features = ["h743"]
priority = 3
requires = {flash = 16384, ram = 8192 }
stacksize = 2048
start = true
uses = ["quadspi"]
interrupts = {92 = 1}
task-slots = ["gpio_driver", "rcc_driver"]
task-slots = ["gpio_driver", "i2c_driver"]

[tasks.idle]
path = "../../task/idle"
Expand Down
14 changes: 12 additions & 2 deletions app/demo-stm32h7-nucleo/app-h753.toml
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,16 @@ requires = {flash = 8192, ram = 1024}
start = true
task-slots = ["gpio_driver"]

[tasks.ping]
path = "../../task/ping"
name = "task-ping"
features = ["uart"]
priority = 4
requires = {flash = 8192, ram = 512}
stacksize = 512
start = true
task-slots = [{peer = "pong"}, "usart_driver"]

[tasks.pong]
path = "../../task/pong"
name = "task-pong"
Expand All @@ -131,12 +141,12 @@ task-slots = ["user_leds"]
[tasks.hiffy]
path = "../../task/hiffy"
name = "task-hiffy"
features = ["h753", "stm32h7", "itm", "i2c", "gpio", "qspi"]
features = ["h753", "stm32h7", "itm", "i2c", "gpio", "spi"]
priority = 3
requires = {flash = 32768, ram = 32768 }
stacksize = 2048
start = true
task-slots = ["gpio_driver", "hf", "i2c_driver"]
task-slots = ["gpio_driver", "i2c_driver"]

[tasks.hf]
path = "../../drv/gimlet-hf-server"
Expand Down
3 changes: 3 additions & 0 deletions drv/spi-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ pub enum SpiError {
///
/// This is almost certainly a programming error on the client side.
BadDevice = 4,

/// Receive FIFO overflow
DataOverrun = 5,
}

impl From<SpiError> for u16 {
Expand Down
49 changes: 38 additions & 11 deletions drv/stm32h7-spi-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ enum Trace {
Start(SpiOperation, (u16, u16)),
Tx(u16, u8),
Rx(u16, u8),
WaitISR(u32),
None,
}

Expand Down Expand Up @@ -251,6 +252,7 @@ impl ServerImpl {
data_dest: Option<LenLimit<Leased<W, [u8]>, 65535>>,
) -> Result<(), RequestError<SpiError>> {
let device_index = usize::from(device_index);
let mut rval = Ok(());

// If we are locked, check that the caller isn't mistakenly
// addressing the wrong device.
Expand Down Expand Up @@ -381,21 +383,22 @@ impl ServerImpl {
*tx_pos += 1;
made_progress = true;

// If we have _just_ finished...
if *tx_pos == src_len {
// We will finish transmitting well before
// we're done receiving, so stop getting
// interrupt notifications for transmit
// space available during that time.
self.spi.disable_can_tx_interrupt();
if *tx_pos == overall_len {
// We continue to transmit beyond our src_len
// (transmitting dummy bytes, above), but once we
// hit overall_len we are done transmitting.
tx = None;
break;
}
}
}

// Just as we keep transmitting until the TX FIFO is filled, we
// keep receiving now until the RX FIFO is empty, assuring that
// we are (roughly) balanced with respect to TX and RX and reducing
// our chances of hitting an overrun.
if let Some((rx_data, rx_pos)) = &mut rx {
if self.spi.can_rx_byte() {
while self.spi.can_rx_byte() {
// Transfer byte from RX FIFO to caller.
let b = self.spi.recv8();
rx_data
Expand All @@ -406,13 +409,27 @@ impl ServerImpl {

if *rx_pos == dest_len {
rx = None;
break;
}

made_progress = true;
}
}

if !made_progress {
ringbuf_entry!(Trace::WaitISR(self.spi.read_status()));

if self.spi.check_overrun() {
// If we've had an overrun, there's little to be done other
// than cleanup and return a failure to our caller.
rval = Err(SpiError::DataOverrun.into());
break;
}

if self.spi.check_eot() {
break;
}

// Allow the controller interrupt to post to our
// notification set.
sys_irq_control(IRQ_MASK, true);
Expand Down Expand Up @@ -446,7 +463,7 @@ impl ServerImpl {
.unwrap();
}

Ok(())
rval
}
}

Expand Down Expand Up @@ -921,9 +938,19 @@ cfg_if::cfg_if! {
SpiMuxOption {
outputs: &[
(
//
// It may seem reasonable -- especially given the
// groupings and silkscreen on CN7 -- to use PB3
// for SPI3_SCK. However, this pin is TRACESWO on
// AF0: if we configure this pin for SPI, ITM
// will no longer work (and with a failure mode
// that can be difficult to debug). We instead
// opt to use PC10 for SPI3_SCK, which is also
// easily accessible: it's pin 6 on CN8.
//
PinSet {
port: gpio_api::Port::B,
pin_mask: 1 << 3,
port: gpio_api::Port::C,
pin_mask: 1 << 10,
},
gpio_api::Alternate::AF6,
),
Expand Down
8 changes: 8 additions & 0 deletions drv/stm32h7-spi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,4 +257,12 @@ impl Spi {
pub fn clear_eot(&mut self) {
self.reg.ifcr.write(|w| w.eotc().set_bit());
}

pub fn read_status(&self) -> u32 {
self.reg.sr.read().bits()
}

pub fn check_overrun(&self) -> bool {
self.reg.sr.read().ovr().is_overrun()
}
}
3 changes: 0 additions & 3 deletions task/pong/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ pub fn main() -> ! {
Err(drv_user_leds_api::LedError::NotPresent) => {
current = 0;
}
_ => {
panic!("unhandled Led error");
}
};
}
}
Expand Down

0 comments on commit 1af71c3

Please sign in to comment.