Skip to content

Commit

Permalink
Refactor LPSPI with word packing, futures
Browse files Browse the repository at this point in the history
Instead of using LPSPI continuous transactions, we pack the user's data
into u32 words. The primitives for translating the user's data to / from
the data FIFOs should help us as we consider async LPSPI drivers. And,
in order to implement embedded-hal 1.0 traits, we'll need something like
the dummy transmit / receive helpers, since we need to handle differing
transmit / receive buffer sizes. The included unit tests don't trigger
an error in Miri, and they try to simulate how we'd use the primitives
in firmware. (This is an "it's not obviously wrong" test, not an "it's
correct" test; help me review here.)

The commit introduces spinning futures into the LPSPI driver. By
combining and spinning on these futures, we can realize in-place
transfers, read-only transactions, write-only transactions, etc. These
implementations flush the FIFOs, allowing users to synchronize external
components with LPSPI I/O.

We no longer return the Busy error; we'll wait for transmit FIFO space.
We also never return the NoData error, instead returning success when
there's no I/O to do. Since this commit is a non-breaking change, the
two errors are still available in the error enum. I'll remove them
later.

I'm moving the blocking SPI example into RTIC and rewriting the driver
test. The tests demonstrate overlapping writes, writes with flushes, and
in-place transfers with a physical loopback. There's also tests that
show how word sizes and bit orders interact. I'd appreciate if folks
could test these changes in their system, since it affects how the
embedded-hal implementations behave. I'm only testing this commit on a
1170EVK with the new example.
  • Loading branch information
mciantyre committed May 25, 2024
1 parent 8063398 commit f4a369e
Show file tree
Hide file tree
Showing 7 changed files with 750 additions and 194 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ jobs:
- --examples --features=board/imxrt1010evk,board/lcd1602
- --examples --features=board/imxrt1060evk,board/lcd1602
# SPI examples (might break other examples)
- --example=hal_spi --example=rtic_spi --example=async_dma_spi --features=board/teensy4,board/spi
- --example=hal_spi --example=rtic_spi --example=async_dma_spi --features=board/imxrt1010evk,board/spi
- --example=hal_spi --example=rtic_spi --example=async_dma_spi --features=board/imxrt1060evk,board/spi
- --example=hal_spi --example=rtic_spi --example=async_dma_spi --features=board/imxrt1170evk-cm7,board/spi
- --example=rtic_spi_blocking --example=rtic_spi --example=async_dma_spi --features=board/teensy4,board/spi
- --example=rtic_spi_blocking --example=rtic_spi --example=async_dma_spi --features=board/imxrt1010evk,board/spi
- --example=rtic_spi_blocking --example=rtic_spi --example=async_dma_spi --features=board/imxrt1060evk,board/spi
- --example=rtic_spi_blocking --example=rtic_spi --example=async_dma_spi --features=board/imxrt1170evk-cm7,board/spi
# The i.MX RT 1170 EVK (CM7) target is WIP. The list below describes the working examples.
- --features=board/imxrt1170evk-cm7,board/lcd1602 --example=hal_led
--example=hal_gpio_input --example=rtic_gpio_input
Expand Down
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ Introduce LPSPI improvements:
- Allow users to change the watermark while enabled. Deprecate the corresponding
method on the `Disabled` helper.

Change how the LPSPI driver manages the FIFOs. As a result of this change, the
driver never returns the `Busy` or `NoData` errors through the embedded-hal
interfaces. Instead of returning `Busy`, the driver blocks until there's space in
the FIFO. If the caller provides an empty buffer, then the result is OK.

The LPSPI embedded-hal (0.2) implementations will implicitly flush after blocking
I/O. Users can rely on this behavior to synchronize external components.

## [0.5.4] 2023-11-26

Add CCM APIs for configuring FlexIO clocks on 1000 targets.
Expand Down
7 changes: 6 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ default-features = false
[dependencies.nb]
version = "1"

[dependencies.futures]
version = "0.3.30"
default-features = false
features = ["async-await"]

[dependencies.eh02]
package = "embedded-hal"
version = "0.2"
Expand Down Expand Up @@ -155,7 +160,7 @@ name = "async_dma_spi"
required-features = ["board/spi"]

[[example]]
name = "hal_spi"
name = "rtic_spi_blocking"
required-features = ["board/spi"]

[[example]]
Expand Down
92 changes: 0 additions & 92 deletions examples/hal_spi.rs

This file was deleted.

168 changes: 168 additions & 0 deletions examples/rtic_spi_blocking.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
//! Demonstrates a SPI device with blocking I/O.
//!
//! Connect SDI to SDO. The example uses the LPSPI interrupt to
//! schedule transfers, and to receive data. You can observe the
//! I/O with a scope / logic analyzer. The SPI CLK runs at 1MHz.
//!
//! Keep an eye on the defmt log to see if tests fail.
#![no_std]
#![no_main]

#[rtic::app(device = board, peripherals = false)]
mod app {

use imxrt_hal as hal;

const PIT_DELAY_MS: u32 = board::PIT_FREQUENCY / 1_000 * 250;

#[local]
struct Local {
spi: board::Spi,
pit: hal::pit::Pit<2>,
}

#[shared]
struct Shared {}

#[init]
fn init(_: init::Context) -> (Shared, Local, init::Monotonics) {
let (
board::Common {
pit: (_, _, pit, _),
..
},
board::Specifics { spi, .. },
) = board::new();
(Shared {}, Local { spi, pit }, init::Monotonics())
}

#[idle(local = [spi, pit])]
fn idle(cx: idle::Context) -> ! {
let idle::LocalResources { spi, pit, .. } = cx.local;
pit.set_load_timer_value(PIT_DELAY_MS);

let mut delay = move || {
pit.enable();
while !pit.is_elapsed() {}
pit.clear_elapsed();
pit.disable();
};

loop {
for _ in 0..3 {
delay();
}

// For studying the effects of bit order and word size.
//
// If you have a logic analyzer that can change its word
// size and bit order, use this sequence to evaluate how
// the driver packs your transfer elements.
{
use eh02::blocking::spi::Write;
use hal::lpspi::BitOrder::{self, *};

const BIT_ORDERS: [BitOrder; 2] = [Msb, Lsb];

const U32_WORDS: [u32; 2] = [0xDEADBEEFu32, 0xAD1CAC1D];
for bit_order in BIT_ORDERS {
spi.set_bit_order(bit_order);
spi.write(&U32_WORDS).unwrap();
}

const U8_WORDS: [u8; 7] = [0xDEu8, 0xAD, 0xBE, 0xEF, 0xA5, 0x00, 0x1D];
for bit_order in BIT_ORDERS {
spi.set_bit_order(bit_order);
spi.write(&U8_WORDS).unwrap();
}

const U16_WORDS: [u16; 3] = [0xDEADu16, 0xBEEF, 0xA5A5];
for bit_order in BIT_ORDERS {
spi.set_bit_order(bit_order);
spi.write(&U16_WORDS).unwrap();
}

delay();
}

// Change me to explore bit order behavors in the
// remaining write / loopback transfer tests.
spi.set_bit_order(hal::lpspi::BitOrder::Msb);

// Make sure concatenated elements look correct on the wire.
{
use eh02::blocking::spi::Write;

spi.write(&[1u8, 2, 3]).unwrap();
spi.write(&[1u8, 2, 3, 4]).unwrap();
spi.write(&[1u8, 2, 3, 4, 5]).unwrap();
spi.write(&[1u8, 2, 3, 4, 5, 6]).unwrap();
spi.write(&[1u8, 2, 3, 4, 5, 6, 7]).unwrap();

spi.write(&[0x0102u16, 0x0304, 0x0506]).unwrap();
spi.write(&[0x0102u16, 0x0304, 0x0506, 0x0708]).unwrap();
spi.write(&[0x0102u16, 0x0304, 0x0506, 0x0708, 0x090A])
.unwrap();

spi.write(&[0x01020304u32, 0x05060708, 0x090A0B0C]).unwrap();

delay();
}

{
use eh02::blocking::spi::{Transfer, Write};

// Change me to test different Elem sizes, buffer sizes,
// bit patterns.
type Elem = u8;
const SENTINEL: Elem = 0x0F;
const BUFFER: [Elem; 13] = [SENTINEL; 13];

// Simple loopback transfer. Easy to find with your
// scope.
let mut buffer = BUFFER;
spi.transfer(&mut buffer).unwrap();
if buffer != BUFFER {
defmt::error!("Simple transfer buffer mismatch!");
}

delay();

// Adjacent loopback transfer. Look for the big
// burst of data on your scope.
let mut buffer = BUFFER;
let mut error = false;
for idx in 0u32..16 {
buffer.fill(SENTINEL.rotate_right(idx));
let expected = buffer;
spi.transfer(&mut buffer).unwrap();
error |= buffer != expected;
}
if error {
defmt::error!("At least one of the bursted transfers didn't match!");
}

delay();

// Simple write.
let buffer = BUFFER;
spi.write(&buffer).unwrap();

delay();

// Pipelined writes. Look for the burst of data
// on your scope. Internally, the writes will flush,
// so the delay between transfers should be about
// the same as they are for the transfers.
let mut buffer = BUFFER;
for idx in 0..16 {
buffer.fill(SENTINEL.rotate_right(idx));
spi.write(&buffer).unwrap();
}

delay();
}
}
}
}
Loading

0 comments on commit f4a369e

Please sign in to comment.