From c2048800dae8175c71d43881ba214fec003b63a8 Mon Sep 17 00:00:00 2001 From: Wilfried Chauveau Date: Sun, 18 Feb 2024 23:36:05 +0000 Subject: [PATCH 1/5] Add on-target-tests and prepare for async implementation --- .cargo/config.toml | 39 + Cargo.toml | 14 +- memory.x | 36 + on-target-tests/.gitignore | 14 + on-target-tests/Cargo.toml | 40 ++ on-target-tests/README.md | 37 + on-target-tests/memory.x | 15 + on-target-tests/tests/i2c_loopback.rs | 123 ++++ on-target-tests/tests/i2c_tests/blocking.rs | 549 ++++++++++++++ on-target-tests/tests/i2c_tests/mod.rs | 131 ++++ run_tests.bat | 4 + run_tests.sh | 5 + src/eh0_2.rs | 171 +++++ src/eh1_0.rs | 121 ++++ src/i2c_cmd.rs | 139 ++++ src/lib.rs | 749 ++++++-------------- 16 files changed, 1633 insertions(+), 554 deletions(-) create mode 100644 .cargo/config.toml create mode 100644 memory.x create mode 100644 on-target-tests/.gitignore create mode 100644 on-target-tests/Cargo.toml create mode 100644 on-target-tests/README.md create mode 100644 on-target-tests/memory.x create mode 100644 on-target-tests/tests/i2c_loopback.rs create mode 100644 on-target-tests/tests/i2c_tests/blocking.rs create mode 100644 on-target-tests/tests/i2c_tests/mod.rs create mode 100755 run_tests.bat create mode 100755 run_tests.sh create mode 100644 src/eh0_2.rs create mode 100644 src/eh1_0.rs create mode 100644 src/i2c_cmd.rs diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 0000000..5826bd0 --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,39 @@ +# +# Cargo Configuration for the https://github.com/rp-rs/rp-hal.git repository. +# +# Copyright (c) The RP-RS Developers, 2021 +# +# You might want to make a similar file in your own repository if you are +# writing programs for Raspberry Silicon microcontrollers. +# +# This file is MIT or Apache-2.0 as per the repository README.md file +# + +[build] +# Set the default target to match the Cortex-M0+ in the RP2040 +target = "thumbv6m-none-eabi" + +# Target specific options +[target.thumbv6m-none-eabi] +# Pass some extra options to rustc, some of which get passed on to the linker. +# +# * linker argument --nmagic turns off page alignment of sections (which saves +# flash space) +# * linker argument -Tlink.x tells the linker to use link.x as the linker +# script. This is usually provided by the cortex-m-rt crate, and by default +# the version in that crate will include a file called `memory.x` which +# describes the particular memory layout for your specific chip. +# * inline-threshold=5 makes the compiler more aggressive and inlining functions +# * no-vectorize-loops turns off the loop vectorizer (seeing as the M0+ doesn't +# have SIMD) +rustflags = [ + "-C", "link-arg=--nmagic", + "-C", "link-arg=-Tlink.x", + "-C", "link-arg=-Tdefmt.x", + "-C", "inline-threshold=5", + "-C", "no-vectorize-loops", +] + +# This runner will find a supported SWD debug probe and flash your RP2040 over +# SWD: +runner = "probe-rs run --chip RP2040" diff --git a/Cargo.toml b/Cargo.toml index f9e2295..a1fb1ee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,15 +7,21 @@ description = "I2C driver implementation using the RP2040's PIO peripheral." documentation = "https://docs.rs/i2c-pio" repository = "https://github.com/rp-rs/i2c-pio-rs" -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html +[workspace] +members = ["on-target-tests"] [dependencies] cortex-m = "0.7.3" -eh1_0_alpha = { version = "=1.0.0-alpha.11", package = "embedded-hal", optional = true } -embedded-hal = "0.2.6" +embedded-hal = { version = "1.0.0" } +embedded_hal_0_2 = { version = "0.2.6", package = "embedded-hal" } nb = "1.0.0" pio = "0.2.0" pio-proc = "0.2.0" -rp2040-hal = "0.9.0" +rp2040-hal = "0.9.1" fugit = "0.3.5" defmt = { version = "0.3.0", optional = true } +either = { version = "1.10.0", default-features = false } +heapless = { version = "0.8.0", default-features = false } + +[patch.crates-io] +rp2040-hal = { git = "https://github.com/rp-rs/rp-hal" } diff --git a/memory.x b/memory.x new file mode 100644 index 0000000..e6b80c4 --- /dev/null +++ b/memory.x @@ -0,0 +1,36 @@ +MEMORY { + BOOT2 : ORIGIN = 0x10000000, LENGTH = 0x100 + FLASH : ORIGIN = 0x10000100, LENGTH = 2048K - 0x100 + /* + * RAM consists of 4 banks, SRAM0-SRAM3, with a striped mapping. + * This is usually good for performance, as it distributes load on + * those banks evenly. + */ + RAM : ORIGIN = 0x20000000, LENGTH = 256K + /* + * RAM banks 4 and 5 use a direct mapping. They can be used to have + * memory areas dedicated for some specific job, improving predictability + * of access times. + * Example: Separate stacks for core0 and core1. + */ + SRAM4 : ORIGIN = 0x20040000, LENGTH = 4k + SRAM5 : ORIGIN = 0x20041000, LENGTH = 4k + + /* SRAM banks 0-3 can also be accessed directly. However, those ranges + alias with the RAM mapping, above. So don't use them at the same time! + SRAM0 : ORIGIN = 0x21000000, LENGTH = 64k + SRAM1 : ORIGIN = 0x21010000, LENGTH = 64k + SRAM2 : ORIGIN = 0x21020000, LENGTH = 64k + SRAM3 : ORIGIN = 0x21030000, LENGTH = 64k + */ +} + +EXTERN(BOOT2_FIRMWARE) + +SECTIONS { + /* ### Boot loader */ + .boot2 ORIGIN(BOOT2) : + { + KEEP(*(.boot2)); + } > BOOT2 +} INSERT BEFORE .text; diff --git a/on-target-tests/.gitignore b/on-target-tests/.gitignore new file mode 100644 index 0000000..6ccfefb --- /dev/null +++ b/on-target-tests/.gitignore @@ -0,0 +1,14 @@ +**/*.rs.bk +.#* +.gdb_history +Cargo.lock +target/ + +# editor files +.vscode/* +!.vscode/*.md +!.vscode/*.svd +!.vscode/launch.json +!.vscode/tasks.json +!.vscode/extensions.json +!.vscode/settings.json diff --git a/on-target-tests/Cargo.toml b/on-target-tests/Cargo.toml new file mode 100644 index 0000000..2fb63ef --- /dev/null +++ b/on-target-tests/Cargo.toml @@ -0,0 +1,40 @@ +[package] +edition = "2021" +name = "on-target-tests" +version = "0.1.0" +publish = false + +[[test]] +name = "i2c_loopback" +harness = false + +[dependencies] +i2c-pio = { path = "../", features = ["defmt"] } + +cortex-m = "0.7" +cortex-m-rt = "0.7" +embedded_hal_0_2 = { package = "embedded-hal", version = "0.2.5", features = [ + "unproven", +] } +embedded-hal = "1.0.0" +embedded-hal-async = "1.0.0" +fugit = "0.3.5" + +defmt = "0.3" +defmt-rtt = "0.4" +defmt-test = "0.3.1" +panic-probe = { version = "0.3", features = ["print-defmt"] } + +rp2040-hal = { version = "0.9.1", features = [ + "critical-section-impl", + "defmt", + "rt", +] } + +rp2040-boot2 = "0.3.0" +critical-section = "1.0.0" +heapless = { version = "0.8.0", features = [ + "portable-atomic-critical-section", + "defmt-03", +] } +itertools = { version = "0.12.0", default-features = false } diff --git a/on-target-tests/README.md b/on-target-tests/README.md new file mode 100644 index 0000000..90d693e --- /dev/null +++ b/on-target-tests/README.md @@ -0,0 +1,37 @@ +# Target tests for i2c-pio + +This project is for running tests of i2c-pio against real hardware via knurling-rs tools. +It is based or rp2040-hal own on-target-tests. + +Adding a test: +- Add a new Rust program to tests (eg tests/my_new_test.rs) +- Add a new [[test]] to the Cargo.toml + +Running all tests: +Linux (and any other Unix-likes where probe-rs are supported): +```system +./run_tests.sh +``` +Windows +```system +run_tests.bat +``` + +To run a specific test (to make developing tests faster) + +```system +cargo test -p on-target-tests --test my_new_test +``` + +## Prerequisites + +Some of the tests need connections between specific pins. + +Currently, the following connections are required: + +- Connect GPIO 0 to GPIO 2 (pins 1 and 4 on a Pico) and + connect GPIO 1 to GPIO 3 (pins 2 and 5 on a Pico) for the I2C loopback tests + +If you add tests that need some hardware setup, make sure that they are +compatible to the existing on-target tests, so all tests can be run with +a single configuration. diff --git a/on-target-tests/memory.x b/on-target-tests/memory.x new file mode 100644 index 0000000..4077aab --- /dev/null +++ b/on-target-tests/memory.x @@ -0,0 +1,15 @@ +MEMORY { + BOOT2 : ORIGIN = 0x10000000, LENGTH = 0x100 + FLASH : ORIGIN = 0x10000100, LENGTH = 2048K - 0x100 + RAM : ORIGIN = 0x20000000, LENGTH = 256K +} + +EXTERN(BOOT2_FIRMWARE) + +SECTIONS { + /* ### Boot loader */ + .boot2 ORIGIN(BOOT2) : + { + KEEP(*(.boot2)); + } > BOOT2 +} INSERT BEFORE .text; diff --git a/on-target-tests/tests/i2c_loopback.rs b/on-target-tests/tests/i2c_loopback.rs new file mode 100644 index 0000000..f411b7a --- /dev/null +++ b/on-target-tests/tests/i2c_loopback.rs @@ -0,0 +1,123 @@ +//! This test needs a connection between: +//! +//! | from GPIO (pico Pin) | to GPIO (pico Pin) | +//! | -------------------- | ------------------ | +//! | 0 (1) | 2 (4) | +//! | 1 (2) | 3 (5) | + +#![no_std] +#![no_main] +#![cfg(test)] + +use defmt_rtt as _; // defmt transport +use defmt_test as _; +use panic_probe as _; +use rp2040_hal as hal; // memory layout // panic handler + +use hal::pac::interrupt; + +/// The linker will place this boot block at the start of our program image. We +/// need this to help the ROM bootloader get our code up and running. +/// Note: This boot block is not necessary when using a rp-hal based BSP +/// as the BSPs already perform this step. +#[link_section = ".boot2"] +#[used] +pub static BOOT2: [u8; 256] = rp2040_boot2::BOOT_LOADER_GENERIC_03H; + +/// External high-speed crystal on the Raspberry Pi Pico board is 12 MHz. Adjust +/// if your board has a different frequency +const XTAL_FREQ_HZ: u32 = 12_000_000u32; + +pub mod i2c_tests; + +#[interrupt] +unsafe fn I2C1_IRQ() { + i2c_tests::blocking::peripheral_handler(); +} + +#[defmt_test::tests] +mod tests { + use crate::i2c_tests::{self, blocking::State, ADDR_10BIT, ADDR_7BIT}; + + #[init] + fn setup() -> State { + i2c_tests::blocking::system_setup(super::XTAL_FREQ_HZ, ADDR_7BIT) + } + + #[test] + fn write(state: &mut State) { + i2c_tests::blocking::write(state, ADDR_7BIT); + i2c_tests::blocking::write(state, ADDR_10BIT); + } + + #[test] + fn write_iter(state: &mut State) { + i2c_tests::blocking::write_iter(state, ADDR_7BIT); + i2c_tests::blocking::write_iter(state, ADDR_10BIT); + } + + #[test] + fn write_iter_read(state: &mut State) { + i2c_tests::blocking::write_iter_read(state, ADDR_7BIT, 1..=1); + i2c_tests::blocking::write_iter_read(state, ADDR_10BIT, 2..=2); + } + + #[test] + fn write_read(state: &mut State) { + i2c_tests::blocking::write_read(state, ADDR_7BIT, 1..=1); + i2c_tests::blocking::write_read(state, ADDR_10BIT, 2..=2); + } + + #[test] + fn read(state: &mut State) { + i2c_tests::blocking::read(state, ADDR_7BIT, 0..=0); + i2c_tests::blocking::read(state, ADDR_10BIT, 1..=1); + } + + #[test] + fn transactions_read(state: &mut State) { + i2c_tests::blocking::transactions_read(state, ADDR_7BIT, 0..=0); + i2c_tests::blocking::transactions_read(state, ADDR_10BIT, 1..=1); + } + + #[test] + fn transactions_write(state: &mut State) { + i2c_tests::blocking::transactions_write(state, ADDR_7BIT); + i2c_tests::blocking::transactions_write(state, ADDR_10BIT); + } + + #[test] + fn transactions_read_write(state: &mut State) { + i2c_tests::blocking::transactions_read_write(state, ADDR_7BIT, 1..=1); + i2c_tests::blocking::transactions_read_write(state, ADDR_10BIT, 2..=2); + } + + #[test] + fn transactions_write_read(state: &mut State) { + i2c_tests::blocking::transactions_write_read(state, ADDR_7BIT, 1..=1); + i2c_tests::blocking::transactions_write_read(state, ADDR_10BIT, 2..=2); + } + + #[test] + fn transaction(state: &mut State) { + i2c_tests::blocking::transaction(state, ADDR_7BIT, 7..=9); + i2c_tests::blocking::transaction(state, ADDR_10BIT, 7..=14); + } + + #[test] + fn transactions_iter(state: &mut State) { + i2c_tests::blocking::transactions_iter(state, ADDR_7BIT, 1..=1); + i2c_tests::blocking::transactions_iter(state, ADDR_10BIT, 2..=2); + } + + #[test] + fn embedded_hal(state: &mut State) { + i2c_tests::blocking::embedded_hal(state, ADDR_7BIT, 2..=2); + i2c_tests::blocking::embedded_hal(state, ADDR_10BIT, 2..=7); + } + + // Sad paths: + // Peripheral Nack + // + // Arbritration conflict +} diff --git a/on-target-tests/tests/i2c_tests/blocking.rs b/on-target-tests/tests/i2c_tests/blocking.rs new file mode 100644 index 0000000..261c833 --- /dev/null +++ b/on-target-tests/tests/i2c_tests/blocking.rs @@ -0,0 +1,549 @@ +use core::{cell::RefCell, ops::RangeInclusive}; + +use critical_section::Mutex; +use fugit::{HertzU32, RateExtU32}; + +use hal::{ + gpio::FunctionNull, + pac::PIO0, + pio::{PIOExt, UninitStateMachine, PIO, PIO0SM0}, +}; +use rp2040_hal::{ + self as hal, + clocks::init_clocks_and_plls, + gpio::{FunctionI2C, Pin, PullUp}, + pac, + watchdog::Watchdog, + Clock, Timer, +}; + +use super::{ + Controller, CtrlPinScl, CtrlPinSda, FIFOBuffer, Generator, MutexCell, Target, TargetState, + ValidAddress, +}; + +pub struct State { + pio: PIO, + i2c_components: Option<((CtrlPinSda, CtrlPinScl), UninitStateMachine)>, + + timer: hal::Timer, + resets: hal::pac::RESETS, + ref_clock_freq: HertzU32, +} + +static TARGET: MutexCell> = Mutex::new(RefCell::new(None)); +static PAYLOAD: MutexCell = MutexCell::new(RefCell::new(TargetState::new())); +static TIMER: MutexCell> = MutexCell::new(RefCell::new(None)); + +macro_rules! assert_vec_eq { + ($e:expr) => { + critical_section::with(|cs| { + let v = &mut PAYLOAD.borrow_ref_mut(cs).vec; + assert_eq!(*v, $e, "FIFO"); + v.clear(); + }); + }; +} +macro_rules! assert_restart_count { + ($e:expr) => {{ + let restart_cnt: u32 = critical_section::with(|cs| PAYLOAD.borrow_ref(cs).restart_cnt); + defmt::assert!( + $e.contains(&restart_cnt), + "restart count out of range {} ∉ {}", + restart_cnt, + $e + ); + }}; +} + +pub fn system_setup(xtal_freq_hz: u32, addr: T) -> State { + unsafe { + hal::sio::spinlock_reset(); + } + let mut pac = pac::Peripherals::take().unwrap(); + let mut watchdog = Watchdog::new(pac.WATCHDOG); + + let clocks = init_clocks_and_plls( + xtal_freq_hz, + pac.XOSC, + pac.CLOCKS, + pac.PLL_SYS, + pac.PLL_USB, + &mut pac.RESETS, + &mut watchdog, + ) + .ok() + .unwrap(); + + let timer = hal::Timer::new(pac.TIMER, &mut pac.RESETS, &clocks); + + // The single-cycle I/O block controls our GPIO pins + let mut sio = hal::Sio::new(pac.SIO); + let pins = hal::gpio::Pins::new( + pac.IO_BANK0, + pac.PADS_BANK0, + sio.gpio_bank0, + &mut pac.RESETS, + ); + + // Configure two pins as being I²C, not GPIO + let ctrl_sda_pin: Pin<_, FunctionNull, PullUp> = pins.gpio0.reconfigure(); + let ctrl_scl_pin: Pin<_, FunctionNull, PullUp> = pins.gpio1.reconfigure(); + + let trg_sda_pin: Pin<_, FunctionI2C, PullUp> = pins.gpio2.reconfigure(); + let trg_scl_pin: Pin<_, FunctionI2C, PullUp> = pins.gpio3.reconfigure(); + + let (pio, sm, ..) = pac.PIO0.split(&mut pac.RESETS); + let i2c_target = hal::I2C::new_peripheral_event_iterator( + pac.I2C1, + trg_sda_pin, + trg_scl_pin, + &mut pac.RESETS, + addr, + ); + + critical_section::with(|cs| TARGET.replace(cs, Some(i2c_target))); + + static mut STACK: rp2040_hal::multicore::Stack<10240> = rp2040_hal::multicore::Stack::new(); + unsafe { + // delegate I2C1 irqs to core 1 + // If the IRQ is not defined, core 1 will just spin in the default IRQ without doing anything + hal::multicore::Multicore::new(&mut pac.PSM, &mut pac.PPB, &mut sio.fifo) + .cores() + .get_mut(1) + .expect("core 1 is not available") + .spawn(&mut STACK.mem, || { + pac::NVIC::unpend(hal::pac::Interrupt::I2C1_IRQ); + pac::NVIC::unmask(hal::pac::Interrupt::I2C1_IRQ); + + loop { + cortex_m::asm::wfi() + } + }) + .expect("failed to start second core."); + } + + State { + pio, + i2c_components: Some(((ctrl_sda_pin, ctrl_scl_pin), sm)), + timer, + resets: pac.RESETS, + ref_clock_freq: clocks.system_clock.freq(), + } +} + +pub fn test_setup<'s, T: ValidAddress>( + state: &'s mut State, + addr: T, + throttling: bool, +) -> Controller<'s> { + let ((sda, scl), sm) = state + .i2c_components + .take() + .expect("I2C components are available"); + + // TARGET is shared with core1. Therefore this needs to happen in a cross-core + // critical-section. + critical_section::with(|cs| { + // reset peripheral + let (i2c, (sda, scl)) = TARGET + .replace(cs, None) + .expect("State contains a target") + .free(&mut state.resets); + + // reset payload storage + PAYLOAD.replace_with(cs, |_| TargetState::new()); + + // remove timer/disable throttling + TIMER.replace(cs, throttling.then_some(state.timer)); + + // + TARGET.replace( + cs, + Some(hal::I2C::new_peripheral_event_iterator( + i2c, + sda, + scl, + &mut state.resets, + addr, + )), + ); + }); + + i2c_pio::I2C::new( + &mut state.pio, + sda, + scl, + sm, + 200.kHz(), + state.ref_clock_freq.clone(), + ) +} + +macro_rules! test_teardown { + ($state:expr, $controller:expr) => { + $state.i2c_components = Some($controller.free()); + }; +} +/// Wait for the expected count of Stop event to ensure the target side has finished processing +/// requests. +/// +/// If a test ends with a write command, there is a risk that the test will check the content of +/// the shared buffer while the target handler hasn't finished processing its fifo. +pub fn wait_stop_count(stop_cnt: u32) { + while critical_section::with(|cs| PAYLOAD.borrow_ref(cs).stop_cnt) < stop_cnt { + cortex_m::asm::wfe(); + } + defmt::flush(); +} + +pub fn peripheral_handler() { + critical_section::with(|cs| { + let Some(ref mut target) = *TARGET.borrow_ref_mut(cs) else { + return; + }; + + let mut timer = TIMER.borrow_ref_mut(cs); + + while let Some(evt) = target.next_event() { + if let Some(t) = timer.as_mut() { + use embedded_hal_0_2::blocking::delay::DelayUs; + t.delay_us(50); + } + + super::target_handler( + target, + evt, + &mut *PAYLOAD.borrow_ref_mut(cs), + timer.is_some(), + ); + } + }) +} + +pub fn write(state: &mut State, addr: T) { + use embedded_hal_0_2::blocking::i2c::Write; + let mut controller = test_setup(state, addr, false); + + let samples: FIFOBuffer = Generator::seq().take(25).collect(); + assert_eq!(controller.write(addr, &samples).is_ok(), true); + wait_stop_count(1); + + assert_restart_count!((0..=0)); + assert_vec_eq!(samples); + + test_teardown!(state, controller); +} +pub fn write_iter(state: &mut State, addr: T) { + let mut controller = test_setup(state, addr, false); + + let samples: FIFOBuffer = Generator::seq().take(25).collect(); + controller + .write_iter(addr, samples.iter().cloned()) + .expect("Successful write_iter"); + wait_stop_count(1); + + assert_restart_count!((0..=0)); + assert_vec_eq!(samples); + test_teardown!(state, controller); +} + +pub fn write_iter_read( + state: &mut State, + addr: T, + restart_count: RangeInclusive, +) { + let mut controller = test_setup(state, addr, false); + + let samples_seq: FIFOBuffer = Generator::seq().take(25).collect(); + let samples_fib: FIFOBuffer = Generator::fib().take(25).collect(); + let mut v = [0u8; 25]; + controller + .write_iter_read(addr, samples_fib.iter().cloned(), &mut v) + .expect("Successful write_iter_read"); + wait_stop_count(1); + + assert_restart_count!(restart_count); + assert_eq!(v, samples_seq); + assert_vec_eq!(samples_fib); + test_teardown!(state, controller); +} + +pub fn write_read(state: &mut State, addr: T, restart_count: RangeInclusive) { + use embedded_hal_0_2::blocking::i2c::WriteRead; + let mut controller = test_setup(state, addr, false); + + let samples_seq: FIFOBuffer = Generator::seq().take(25).collect(); + let samples_fib: FIFOBuffer = Generator::fib().take(25).collect(); + let mut v = [0u8; 25]; + controller + .write_read(addr, &samples_fib, &mut v) + .expect("successfully write_read"); + wait_stop_count(1); + + assert_restart_count!(restart_count); + assert_eq!(v, samples_seq); + assert_vec_eq!(samples_fib); + test_teardown!(state, controller); +} + +pub fn read(state: &mut State, addr: T, restart_count: RangeInclusive) { + use embedded_hal_0_2::blocking::i2c::Read; + let mut controller = test_setup(state, addr, false); + + let mut v = [0u8; 25]; + controller.read(addr, &mut v).expect("successfully read"); + wait_stop_count(1); + + let samples: FIFOBuffer = Generator::fib().take(25).collect(); + assert_restart_count!(restart_count); + assert_eq!(v, samples); + assert_vec_eq!([]); + test_teardown!(state, controller); +} + +pub fn transactions_read( + state: &mut State, + addr: T, + restart_count: RangeInclusive, +) { + use embedded_hal::i2c::{I2c, Operation}; + let mut controller = test_setup(state, addr, false); + + let mut v = [0u8; 25]; + controller + .transaction(addr, &mut [Operation::Read(&mut v)]) + .expect("successfully write_read"); + wait_stop_count(1); + + let samples: FIFOBuffer = Generator::fib().take(25).collect(); + assert_restart_count!(restart_count); + assert_eq!(v, samples); + assert_vec_eq!([]); + test_teardown!(state, controller); +} + +pub fn transactions_write(state: &mut State, addr: T) { + use embedded_hal::i2c::{I2c, Operation}; + let mut controller = test_setup(state, addr, false); + + let samples: FIFOBuffer = Generator::seq().take(25).collect(); + controller + .transaction(addr, &mut [Operation::Write(&samples)]) + .expect("successfully write_read"); + wait_stop_count(1); + + assert_restart_count!((0..=0)); + assert_vec_eq!(samples); + test_teardown!(state, controller); +} + +pub fn transactions_read_write( + state: &mut State, + addr: T, + restart_count: RangeInclusive, +) { + use embedded_hal::i2c::{I2c, Operation}; + let mut controller = test_setup(state, addr, true); + + let samples_seq: FIFOBuffer = Generator::seq().take(25).collect(); + let samples_fib: FIFOBuffer = Generator::fib().take(25).collect(); + let mut v = [0u8; 25]; + controller + .transaction( + addr, + &mut [Operation::Read(&mut v), Operation::Write(&samples_seq)], + ) + .expect("successfully write_read"); + wait_stop_count(1); + + assert_restart_count!(restart_count); + assert_eq!(v, samples_fib); + assert_vec_eq!(samples_seq); + test_teardown!(state, controller); +} + +pub fn transactions_write_read( + state: &mut State, + addr: T, + restart_count: RangeInclusive, +) { + use embedded_hal::i2c::{I2c, Operation}; + let mut controller = test_setup(state, addr, false); + + let samples_seq: FIFOBuffer = Generator::seq().take(25).collect(); + let mut v = [0u8; 25]; + + controller + .transaction( + addr, + &mut [Operation::Write(&samples_seq), Operation::Read(&mut v)], + ) + .expect("successfully write_read"); + wait_stop_count(1); + + assert_restart_count!(restart_count); + assert_eq!(v, samples_seq); + assert_vec_eq!(samples_seq); + test_teardown!(state, controller); +} + +pub fn transaction( + state: &mut State, + addr: T, + restart_count: RangeInclusive, +) { + // Throttling is important for this test as it also ensures that the Target implementation + // does not "waste" bytes that would be discarded otherwise. + // + // One down side of this is that the Target implementation is unable to detect restarts + // between consicutive write operations + use embedded_hal::i2c::{I2c, Operation}; + let mut controller = test_setup(state, addr, true); + + let mut v = ([0u8; 14], [0u8; 25], [0u8; 25], [0u8; 14], [0u8; 14]); + let samples: FIFOBuffer = Generator::seq().take(25).collect(); + controller + .transaction( + addr, + &mut [ + Operation::Write(&samples), // goes to v2 + Operation::Read(&mut v.0), + Operation::Read(&mut v.1), + Operation::Read(&mut v.2), + Operation::Write(&samples), // goes to v3 + Operation::Read(&mut v.3), + Operation::Write(&samples), // goes to v4 + Operation::Write(&samples), // remains in buffer + Operation::Write(&samples), // remains in buffer + Operation::Read(&mut v.4), + ], + ) + .expect("successfully write_read"); + wait_stop_count(1); + + // There are 14restarts in this sequence but because of latency in the target handling, it + // may only detect 7. + assert_restart_count!(restart_count); + + // assert writes + let e: FIFOBuffer = itertools::chain!( + samples.iter(), + samples.iter(), + samples.iter(), + samples.iter(), + samples.iter(), + ) + .cloned() + .collect(); + assert_vec_eq!(e); + + // assert reads + let g: FIFOBuffer = Generator::seq().take(92).collect(); + let h: FIFOBuffer = itertools::chain!( + v.0.into_iter(), + v.1.into_iter(), + v.2.into_iter(), + v.3.into_iter(), + v.4.into_iter() + ) + .collect(); + assert_eq!(g, h); + test_teardown!(state, controller); +} + +pub fn transactions_iter( + state: &mut State, + addr: T, + restart_count: RangeInclusive, +) { + use embedded_hal::i2c::{I2c, Operation}; + let mut controller = test_setup(state, addr, false); + + let samples: FIFOBuffer = Generator::seq().take(25).collect(); + let mut v = [0u8; 25]; + controller + .transaction( + addr, + &mut [Operation::Write(&samples), Operation::Read(&mut v)], + ) + .expect("successfully write_read"); + wait_stop_count(1); + + assert_restart_count!(restart_count); + assert_eq!(v, samples); + assert_vec_eq!(samples); + test_teardown!(state, controller); +} + +pub fn embedded_hal( + state: &mut State, + addr: T, + restart_count: RangeInclusive, +) { + // Throttling is important for this test as it also ensures that the Target implementation + // does not "waste" bytes that would be discarded otherwise. + // + // One down side of this is that the Target implementation is unable to detect restarts + // between consicutive write operations + use embedded_hal::i2c::I2c; + let mut controller = test_setup(state, addr, true); + + let samples1: FIFOBuffer = Generator::seq().take(25).collect(); + let samples2: FIFOBuffer = Generator::fib().take(14).collect(); + let mut v = ([0; 14], [0; 25], [0; 25], [0; 14], [0; 14]); + + let mut case = || { + controller.write(addr, &samples1)?; + wait_stop_count(1); + controller.read(addr, &mut v.0)?; + wait_stop_count(2); + controller.read(addr, &mut v.1)?; + wait_stop_count(3); + controller.read(addr, &mut v.2)?; + wait_stop_count(4); + controller.write_read(addr, &samples2, &mut v.3)?; + wait_stop_count(5); + controller.write(addr, &samples2)?; + wait_stop_count(6); + controller.write(addr, &samples1)?; + wait_stop_count(7); + controller.write_read(addr, &samples1, &mut v.4)?; + wait_stop_count(8); + Ok::<(), i2c_pio::Error>(()) + }; + case().expect("Successful test"); + + // There are 14restarts in this sequence but because of latency in the target handling, it + // may only detect 7. + assert_restart_count!(restart_count); + + // assert writes + let e: FIFOBuffer = itertools::chain!( + Generator::seq().take(25), + Generator::fib().take(14), + Generator::fib().take(14), + Generator::seq().take(25), + Generator::seq().take(25), + ) + .collect(); + assert_vec_eq!(e); + + // assert reads + let g: FIFOBuffer = itertools::chain!( + Generator::seq().take(64), + Generator::fib().take(14), + Generator::seq().take(14) + ) + .collect(); + let h: FIFOBuffer = itertools::chain!( + v.0.into_iter(), + v.1.into_iter(), + v.2.into_iter(), + v.3.into_iter(), + v.4.into_iter() + ) + .collect(); + assert_eq!(g, h); + test_teardown!(state, controller); +} diff --git a/on-target-tests/tests/i2c_tests/mod.rs b/on-target-tests/tests/i2c_tests/mod.rs new file mode 100644 index 0000000..5fa930d --- /dev/null +++ b/on-target-tests/tests/i2c_tests/mod.rs @@ -0,0 +1,131 @@ +use core::cell::RefCell; + +use critical_section::Mutex; +use rp2040_hal::{ + self as hal, + gpio::{ + bank0::{Gpio0, Gpio1, Gpio2, Gpio3}, + FunctionI2C, FunctionNull, Pin, PullUp, + }, + i2c::peripheral::Event, +}; + +pub mod blocking; +//pub mod non_blocking; + +pub const ADDR_7BIT: u8 = 0x2c; +pub const ADDR_10BIT: u16 = 0x12c; + +type P = (Pin, Pin); +pub type Target = hal::I2C, hal::i2c::Peripheral>; +pub type Controller<'a> = i2c_pio::I2C<'a, hal::pac::PIO0, hal::pio::SM0, CtrlPinSda, CtrlPinScl>; +pub type CtrlPinSda = Pin; +pub type CtrlPinScl = Pin; +type MutexCell = Mutex>; +type FIFOBuffer = heapless::Vec; + +#[derive(Debug, defmt::Format, Default)] +pub struct TargetState { + first: bool, + gen: Generator, + vec: FIFOBuffer, + restart_cnt: u32, + stop_cnt: u32, +} +impl TargetState { + pub const fn new() -> TargetState { + TargetState { + first: true, + gen: Generator::fib(), + vec: FIFOBuffer::new(), + restart_cnt: 0, + stop_cnt: 0, + } + } +} + +#[derive(Debug, defmt::Format, Clone, Copy)] +pub enum Generator { + Sequence(u8), + Fibonacci(u8, u8), +} +impl Generator { + const fn fib() -> Generator { + Generator::Fibonacci(0, 1) + } + const fn seq() -> Generator { + Generator::Sequence(0) + } + fn switch(&mut self) { + *self = if matches!(self, Generator::Sequence(_)) { + Generator::Fibonacci(0, 1) + } else { + Generator::Sequence(0) + }; + } +} +impl Default for Generator { + fn default() -> Self { + Generator::Sequence(0) + } +} +impl Iterator for Generator { + type Item = u8; + + fn next(&mut self) -> Option { + let out; + match self { + Generator::Sequence(prev) => { + (out, *prev) = (*prev, prev.wrapping_add(1)); + } + Generator::Fibonacci(a, b) => { + out = *a; + (*a, *b) = (*b, a.wrapping_add(*b)); + } + } + Some(out) + } +} + +fn target_handler( + target: &mut Target, + evt: rp2040_hal::i2c::peripheral::Event, + payload: &mut TargetState, + throttle: bool, +) { + let TargetState { + first, + gen, + vec, + restart_cnt, + stop_cnt, + } = payload; + match evt { + Event::Start => *first = true, + Event::Restart => *restart_cnt += 1, + Event::TransferRead => { + let n = throttle.then_some(1).unwrap_or(target.tx_fifo_available()); + let v: FIFOBuffer = gen.take(n.into()).collect(); + target.write(&v); + } + Event::TransferWrite => { + if *first { + gen.switch(); + *first = false; + } + // when throttling, treat 1 byte at a time + let max = throttle + .then_some(1) + .unwrap_or_else(|| target.rx_fifo_used().into()); + vec.extend(target.take(max)); + } + Event::Stop => { + *stop_cnt += 1; + // notify the other core a stop was detected + cortex_m::asm::sev(); + } + } +} + +pub trait ValidAddress: i2c_pio::ValidAddressMode + rp2040_hal::i2c::ValidAddress {} +impl ValidAddress for T where T: rp2040_hal::i2c::ValidAddress + i2c_pio::ValidAddressMode {} diff --git a/run_tests.bat b/run_tests.bat new file mode 100755 index 0000000..e68c7d2 --- /dev/null +++ b/run_tests.bat @@ -0,0 +1,4 @@ +@rem Keep running tests even if one of them fails +@rem We need to specify environment variables here to control build since we aren't able to override them in Cargo.toml + +cargo test -p on-target-tests --no-fail-fast diff --git a/run_tests.sh b/run_tests.sh new file mode 100755 index 0000000..fef5039 --- /dev/null +++ b/run_tests.sh @@ -0,0 +1,5 @@ +#!/bin/sh + +# Keep running tests even if one of them fails +# "$@" passes any extra args given to ran +cargo test -p on-target-tests --no-fail-fast "$@" diff --git a/src/eh0_2.rs b/src/eh0_2.rs new file mode 100644 index 0000000..7d18827 --- /dev/null +++ b/src/eh0_2.rs @@ -0,0 +1,171 @@ +use crate::*; +use embedded_hal_0_2::blocking::i2c::{self, Operation}; +impl i2c::Read for I2C<'_, P, SMI, SDA, SCL> +where + A: ValidAddressMode, + P: PIOExt, + SMI: StateMachineIndex, + SDA: AnyPin, + SCL: AnyPin, +{ + type Error = Error; + + fn read(&mut self, address: A, buffer: &mut [u8]) -> Result<(), Self::Error> { + let iter = super::setup(address, true, false).chain(buffer.iter_mut().map(CmdWord::read)); + self.process_queue(iter)?; + self.generate_stop(); + Ok(()) + } +} + +impl i2c::WriteIter for I2C<'_, P, SMI, SDA, SCL> +where + A: ValidAddressMode, + P: PIOExt, + SMI: StateMachineIndex, + SDA: AnyPin, + SCL: AnyPin, +{ + type Error = Error; + + fn write(&mut self, address: A, bytes: B) -> Result<(), Self::Error> + where + B: IntoIterator, + { + let iter = super::setup(address, false, false).chain(bytes.into_iter().map(CmdWord::write)); + self.process_queue(iter)?; + self.generate_stop(); + Ok(()) + } +} +impl i2c::Write for I2C<'_, P, SMI, SDA, SCL> +where + A: ValidAddressMode, + P: PIOExt, + SMI: StateMachineIndex, + SDA: AnyPin, + SCL: AnyPin, +{ + type Error = Error; + + fn write(&mut self, address: A, buffer: &[u8]) -> Result<(), Self::Error> { + >::write(self, address, buffer.iter().cloned()) + } +} + +impl i2c::WriteIterRead for I2C<'_, P, SMI, SDA, SCL> +where + A: ValidAddressMode, + P: PIOExt, + SMI: StateMachineIndex, + SDA: AnyPin, + SCL: AnyPin, +{ + type Error = Error; + + fn write_iter_read( + &mut self, + address: A, + bytes: B, + buffer: &mut [u8], + ) -> Result<(), Self::Error> + where + B: IntoIterator, + { + self.process_queue( + super::setup(address, false, false) + .chain(bytes.into_iter().map(CmdWord::write)) + .chain(super::setup(address, true, true)) + .chain(buffer.iter_mut().map(CmdWord::read)), + )?; + self.generate_stop(); + Ok(()) + } +} +impl i2c::WriteRead for I2C<'_, P, SMI, SDA, SCL> +where + A: ValidAddressMode, + P: PIOExt, + SMI: StateMachineIndex, + SDA: AnyPin, + SCL: AnyPin, +{ + type Error = Error; + + fn write_read( + &mut self, + address: A, + bytes: &[u8], + buffer: &mut [u8], + ) -> Result<(), Self::Error> { + >::write_iter_read( + self, + address, + bytes.iter().cloned(), + buffer, + ) + } +} + +impl i2c::TransactionalIter for I2C<'_, P, SMI, SDA, SCL> +where + A: ValidAddressMode, + P: PIOExt, + SMI: StateMachineIndex, + SDA: AnyPin, + SCL: AnyPin, +{ + type Error = Error; + + fn exec_iter<'a, O>(&mut self, address: A, operations: O) -> Result<(), Self::Error> + where + O: IntoIterator>, + { + let mut first = true; + for op in operations { + let iter = match op { + Operation::Read(buf) => Left( + super::setup(address, true, !first).chain(buf.iter_mut().map(CmdWord::read)), + ), + Operation::Write(buf) => Right( + super::setup(address, false, !first) + .chain(buf.iter().cloned().map(CmdWord::write)), + ), + }; + self.process_queue(iter)?; + first = false; + } + self.generate_stop(); + Ok(()) + } +} + +impl i2c::Transactional for I2C<'_, P, SMI, SDA, SCL> +where + A: ValidAddressMode, + P: PIOExt, + SMI: StateMachineIndex, + SDA: AnyPin, + SCL: AnyPin, +{ + type Error = Error; + + fn exec(&mut self, address: A, operations: &mut [Operation<'_>]) -> Result<(), Self::Error> { + let mut first = true; + for op in operations { + let iter = match op { + Operation::Read(buf) => Left( + super::setup(address, true, !first).chain(buf.iter_mut().map(CmdWord::read)), + ), + Operation::Write(buf) => Right( + super::setup(address, false, !first) + .chain(buf.iter().cloned().map(CmdWord::write)), + ), + }; + self.process_queue(iter)?; + first = false; + } + self.generate_stop(); + Ok(()) + } +} diff --git a/src/eh1_0.rs b/src/eh1_0.rs new file mode 100644 index 0000000..1bb8b12 --- /dev/null +++ b/src/eh1_0.rs @@ -0,0 +1,121 @@ +use either::Either::{Left, Right}; +use embedded_hal::i2c::{ErrorKind, NoAcknowledgeSource, Operation}; + +use crate::{CmdWord, Error, ValidAddressMode}; + +use super::{AnyPin, PIOExt, StateMachineIndex, I2C}; + +impl I2C<'_, P, SMI, SDA, SCL> +where + P: PIOExt, + SMI: StateMachineIndex, + SDA: AnyPin, + SCL: AnyPin, +{ + pub fn write_iter(&mut self, address: A, bytes: B) -> Result<(), Error> + where + A: ValidAddressMode, + B: IntoIterator + Clone, + { + self.process_queue( + super::setup(address, false, false).chain(bytes.into_iter().map(CmdWord::write)), + )?; + self.generate_stop(); + Ok(()) + } + + pub fn write_iter_read( + &mut self, + address: A, + bytes: B, + buffer: &mut [u8], + ) -> Result<(), Error> + where + A: ValidAddressMode, + B: IntoIterator, + { + self.process_queue( + super::setup(address, false, false) + .chain(bytes.into_iter().map(CmdWord::write)) + .chain(super::setup(address, true, true)) + .chain(buffer.iter_mut().map(CmdWord::read)), + )?; + self.generate_stop(); + Ok(()) + } + + pub fn transaction_iter<'a, A, O>(&mut self, address: A, operations: O) -> Result<(), Error> + where + A: ValidAddressMode, + O: IntoIterator>, + { + let mut first = true; + for op in operations { + let iter = match op { + Operation::Read(buf) => Left( + super::setup(address, true, !first).chain(buf.iter_mut().map(CmdWord::read)), + ), + Operation::Write(buf) => Right( + super::setup(address, false, !first) + .chain(buf.iter().cloned().map(CmdWord::write)), + ), + }; + self.process_queue(iter)?; + first = false; + } + self.generate_stop(); + Ok(()) + } +} + +impl embedded_hal::i2c::Error for super::Error { + fn kind(&self) -> ErrorKind { + match self { + Error::NoAcknowledgeAddress => ErrorKind::NoAcknowledge(NoAcknowledgeSource::Address), + Error::NoAcknowledgeData => ErrorKind::NoAcknowledge(NoAcknowledgeSource::Data), + Error::BusContention => ErrorKind::ArbitrationLoss, + } + } +} + +impl embedded_hal::i2c::ErrorType for I2C<'_, P, SMI, SDA, SCL> +where + P: PIOExt, + SMI: StateMachineIndex, + SDA: AnyPin, + SCL: AnyPin, +{ + type Error = super::Error; +} + +impl embedded_hal::i2c::I2c for I2C<'_, P, SMI, SDA, SCL> +where + A: ValidAddressMode, + P: PIOExt, + SMI: StateMachineIndex, + SDA: AnyPin, + SCL: AnyPin, +{ + fn transaction( + &mut self, + address: A, + operations: &mut [Operation<'_>], + ) -> Result<(), Self::Error> { + let mut first = true; + for op in operations { + let iter = match op { + Operation::Read(buf) => Left( + super::setup(address, true, !first).chain(buf.iter_mut().map(CmdWord::read)), + ), + Operation::Write(buf) => Right( + super::setup(address, false, !first) + .chain(buf.iter().cloned().map(CmdWord::write)), + ), + }; + self.process_queue(iter)?; + first = false; + } + self.generate_stop(); + Ok(()) + } +} diff --git a/src/i2c_cmd.rs b/src/i2c_cmd.rs new file mode 100644 index 0000000..28b8ece --- /dev/null +++ b/src/i2c_cmd.rs @@ -0,0 +1,139 @@ +use core::iter::once; + +use either::Either::{Left, Right}; +use pio::{Instruction, SideSet}; + +const SIDESET: SideSet = SideSet::new(true, 1, true); +const SC0SD0: Instruction = Instruction { + operands: pio::InstructionOperands::SET { + destination: pio::SetDestination::PINDIRS, + data: 0, + }, + delay: 7, + side_set: Some(0), +}; +const SC0SD1: Instruction = Instruction { + operands: pio::InstructionOperands::SET { + destination: pio::SetDestination::PINDIRS, + data: 1, + }, + delay: 7, + side_set: Some(0), +}; +const SC1SD0: Instruction = Instruction { + operands: pio::InstructionOperands::SET { + destination: pio::SetDestination::PINDIRS, + data: 0, + }, + delay: 7, + side_set: Some(1), +}; +const SC1SD1: Instruction = Instruction { + operands: pio::InstructionOperands::SET { + destination: pio::SetDestination::PINDIRS, + data: 1, + }, + delay: 7, + side_set: Some(1), +}; + +const START: [Instruction; 2] = [SC1SD0, SC0SD0]; +const STOP: [Instruction; 3] = [SC0SD0, SC1SD0, SC1SD1]; +const RESTART: [Instruction; 4] = [SC0SD1, SC1SD1, SC1SD0, SC0SD0]; + +const NAK_BIT: u16 = 0b0000_0000_0000_0001; +const FINAL_BIT: u16 = 0b0000_0010_0000_0000; +const INSTR_OFFSET: usize = 10; +const DATA_OFFSET: usize = 1; + +#[derive(Debug)] +pub struct Data<'b> { + pub byte: either::Either, + pub is_address: bool, +} +impl<'b> Data<'b> { + pub fn encode(&self, last: bool) -> u16 { + match self.byte.as_ref().left() { + // if write: send data & let the target handle the nak/ack + Some(b) => (u16::from(*b) << DATA_OFFSET) | NAK_BIT, + // if read: clock out and idle state & ack only if not last + None => (0xFF << DATA_OFFSET) | if last { NAK_BIT | FINAL_BIT } else { 0 }, + } + } + pub fn address(v: u8) -> Self { + Self { + byte: Left(v), + is_address: true, + } + } + pub fn write(v: u8) -> Self { + Self { + byte: Left(v), + is_address: false, + } + } + pub fn read(v: &'b mut u8) -> Self { + Self { + byte: Right(v), + is_address: false, + } + } +} +#[cfg(feature = "defmt")] +impl defmt::Format for Data<'_> { + fn format(&self, fmt: defmt::Formatter) { + defmt::write!(fmt, "Data {{ byte:"); + match self.byte { + Left(b) => defmt::write!(fmt, "{:x}", b), + Right(_) => defmt::write!(fmt, "…"), + } + defmt::write!(fmt, " }}"); + } +} +#[derive(Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum CmdWord<'b> { + Raw(u16), + Data(Data<'b>), +} +impl From for CmdWord<'_> { + fn from(value: Instruction) -> Self { + CmdWord::Raw(value.encode(SIDESET)) + } +} +impl<'b> CmdWord<'b> { + pub fn encode(&self, last: bool) -> u16 { + match self { + CmdWord::Raw(r) => *r, + CmdWord::Data(d) => d.encode(last), + } + } + pub fn address(v: u8) -> Self { + CmdWord::Data(Data::address(v)) + } + pub fn write(v: u8) -> Self { + CmdWord::Data(Data::write(v)) + } + pub fn read(v: &'b mut u8) -> Self { + CmdWord::Data(Data::read(v)) + } +} +fn cmd_len(cmd: &[Instruction]) -> impl Iterator { + let encoded_len = (cmd.len() - 1) << INSTR_OFFSET; + once(encoded_len as u16) +} +pub fn stop() -> impl Iterator { + cmd_len(&STOP).chain(STOP.into_iter().map(|i| i.encode(SIDESET))) +} + +pub fn start<'b>() -> impl Iterator> { + cmd_len(&START) + .map(CmdWord::Raw) + .chain(START.into_iter().map(CmdWord::from)) +} + +pub fn restart<'b>() -> impl Iterator> { + cmd_len(&RESTART) + .map(CmdWord::Raw) + .chain(RESTART.into_iter().map(CmdWord::from)) +} diff --git a/src/lib.rs b/src/lib.rs index 4141416..b8f8ab6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -40,63 +40,108 @@ //! (It's possible for the inversion to be done in this program, //! but costs 2 instructions: 1 for inversion, and one to cope //! with the side effect of the MOV on TX shift counter.) +use core::iter::once; -use embedded_hal::blocking::i2c::{self, AddressMode, Operation, SevenBitAddress, TenBitAddress}; +use either::Either::{Left, Right}; use fugit::HertzU32; -use pio::{Instruction, InstructionOperands, SideSet}; +use heapless::Deque; +use i2c_cmd::{restart, start, CmdWord, Data}; use rp2040_hal::{ - gpio::{AnyPin, FunctionNull, Pin, PullUp, ValidFunction}, + gpio::{ + AnyPin, Function, FunctionNull, OutputEnableOverride, Pin, PinId, PullType, PullUp, + ValidFunction, + }, pio::{ PIOExt, PinDir, PinState, Rx, ShiftDirection, StateMachine, StateMachineIndex, Tx, UninitStateMachine, PIO, }, }; -const SC0SD0: Instruction = Instruction { - operands: pio::InstructionOperands::SET { - destination: pio::SetDestination::PINDIRS, - data: 0, - }, - delay: 7, - side_set: Some(0), -}; -const SC0SD1: Instruction = Instruction { - operands: pio::InstructionOperands::SET { - destination: pio::SetDestination::PINDIRS, - data: 1, - }, - delay: 7, - side_set: Some(0), -}; -const SC1SD0: Instruction = Instruction { - operands: pio::InstructionOperands::SET { - destination: pio::SetDestination::PINDIRS, - data: 0, - }, - delay: 7, - side_set: Some(1), -}; -const SC1SD1: Instruction = Instruction { - operands: pio::InstructionOperands::SET { - destination: pio::SetDestination::PINDIRS, - data: 1, - }, - delay: 7, - side_set: Some(1), -}; +use crate::i2c_cmd::stop; + +mod eh0_2; +mod eh1_0; +mod i2c_cmd; -const SIDESET: SideSet = SideSet::new(true, 1, true); +/// Length of an address. +#[derive(Debug, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum AddressLength { + _7, + _10, +} +pub trait ValidAddressMode: + Copy + Into + embedded_hal::i2c::AddressMode + embedded_hal_0_2::blocking::i2c::AddressMode +{ + fn address_len() -> AddressLength; +} +macro_rules! impl_valid_addr { + ($t:path => $e:expr) => { + impl ValidAddressMode for $t { + fn address_len() -> AddressLength { + $e + } + } + }; +} +impl_valid_addr!(u8 => AddressLength::_7); +impl_valid_addr!(u16 => AddressLength::_10); +// `embedded_hal`s’ SevenBitAddress and TenBitAddress are aliases to u8 and u16 respectively. +//impl_valid_addr!(embedded_hal::i2c::SevenBitAddress => AddressLength::_7); +//impl_valid_addr!(embedded_hal::i2c::TenBitAddress => AddressLength::_10); +//impl_valid_addr!(embedded_hal_0_2::blocking::i2c::SevenBitAddress => AddressLength::_7); +//impl_valid_addr!(embedded_hal_0_2::blocking::i2c::TenBitAddress => AddressLength::_10); + +fn setup<'b, A: ValidAddressMode>( + address: A, + read: bool, + do_restart: bool, +) -> impl Iterator> { + let read_flag = if read { 1 } else { 0 }; + let address: u16 = address.into(); + let address = match A::address_len() { + AddressLength::_7 => { + let address_and_flag = ((address as u8) << 1) | read_flag; + Left([address_and_flag].into_iter().map(CmdWord::address)) + } + AddressLength::_10 => { + let addr_hi = 0xF0 | ((address >> 7) as u8) & 0xFE; + let addr_lo = (address & 0xFF) as u8; + + Right(if read { + let full_addr = [addr_hi, addr_lo] + .into_iter() + .map(Data::address) + .map(CmdWord::Data); + let read_addr = + restart().chain(once(CmdWord::Data(Data::address(addr_hi | read_flag)))); + + Left(full_addr.chain(read_addr)) + } else { + Right( + [addr_hi | read_flag, addr_lo] + .into_iter() + .map(Data::address) + .map(CmdWord::Data), + ) + }) + } + }; -const NAK_BIT: u16 = 0b0000_0000_0000_0001; -const FINAL_BIT: u16 = 0b0000_0010_0000_0000; -const INSTR_OFFSET: usize = 10; -const DATA_OFFSET: usize = 1; + if do_restart { + Left(restart()) + } else { + Right(start()) + } + .chain(address) +} #[derive(Debug)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum Error { NoAcknowledgeAddress, NoAcknowledgeData, + BusContention, } /// Instance of I2C Controller. @@ -111,8 +156,8 @@ where sm: StateMachine<(P, SMI), rp2040_hal::pio::Running>, tx: Tx<(P, SMI)>, rx: Rx<(P, SMI)>, - _sda: Pin, - _scl: Pin, + sda: (Pin, OutputEnableOverride), + scl: (Pin, OutputEnableOverride), } impl<'pio, P, SMI, SDA, SCL> I2C<'pio, P, SMI, SDA, SCL> @@ -210,7 +255,7 @@ where let frac: u8 = frac as u8; // init - let (mut sm, rx, tx) = rp2040_hal::pio::PIOBuilder::from_program(installed) + let (mut sm, rx, tx) = rp2040_hal::pio::PIOBuilder::from_installed_program(installed) // use both RX & TX FIFO .buffers(rp2040_hal::pio::Buffers::RxTx) // Pin configuration @@ -231,8 +276,10 @@ where .build(sm); // enable pull up on SDA & SCL: idle bus - let sda = sda.into_pull_type(); - let scl = scl.into_pull_type(); + let sda: Pin<_, _, PullUp> = sda.into_pull_type(); + let scl: Pin<_, _, PullUp> = scl.into_pull_type(); + let sda_override = sda.get_output_enable_override(); + let scl_override = scl.get_output_enable_override(); // This will pull the bus high for a little bit of time sm.set_pins([ @@ -247,19 +294,19 @@ where // attach SDA pin to pio let mut sda: Pin = sda.into_function(); // configure SDA pin as inverted - sda.set_output_enable_override(rp2040_hal::gpio::OutputEnableOverride::Invert); + sda.set_output_enable_override(OutputEnableOverride::Invert); // attach SCL pin to pio let mut scl: Pin = scl.into_function(); // configure SCL pin as inverted - scl.set_output_enable_override(rp2040_hal::gpio::OutputEnableOverride::Invert); + scl.set_output_enable_override(OutputEnableOverride::Invert); // the PIO now keeps the pin as Input, we can set the pin state to Low. sm.set_pins([(sda.id().num, PinState::Low), (scl.id().num, PinState::Low)]); // Set the state machine on the entry point. - sm.exec_instruction(Instruction { - operands: InstructionOperands::JMP { + sm.exec_instruction(pio::Instruction { + operands: pio::InstructionOperands::JMP { condition: pio::JmpCondition::Always, address: wrap_target, }, @@ -275,8 +322,8 @@ where sm, tx, rx, - _sda: sda.into(), - _scl: scl.into(), + sda: (sda, sda_override), + scl: (scl, scl_override), } } @@ -285,534 +332,136 @@ where self.pio.get_irq_raw() & mask != 0 } - fn resume_after_error(&mut self) { - self.sm.drain_tx_fifo(); + fn err_with(&mut self, err: Error) -> Result<(), Error> { + // clear fifos + self.sm.clear_fifos(); + // resume pio driver self.pio.clear_irq(1 << SMI::id()); - while !self.sm.stalled() { - let _ = self.rx.read(); - } - } - - fn put(&mut self, data: u16) { - while !self.tx.write_u16_replicated(data) {} + // generate stop condition + self.generate_stop(); + Err(err) } - fn put_data(&mut self, data: u8, read_ack: bool, last: bool) { - let final_field = if last { FINAL_BIT } else { 0 }; - let nak_field = if read_ack { NAK_BIT } else { 0 }; - let data_field = u16::from(data) << DATA_OFFSET; - - let word = final_field | data_field | nak_field; - self.put(word); - } - - fn put_instr_sequence(&mut self, seq: T) - where - T: IntoIterator, - U: Iterator + ExactSizeIterator, - { - let seq = seq.into_iter(); - assert!(seq.len() < 64); - let n = seq.len() as u16; - - self.put((n - 1) << INSTR_OFFSET); - for instr in seq { - self.put(instr.encode(SIDESET)); + fn generate_stop(&mut self) { + // this driver checks for acknoledge error and/or expects data back, so by the time a stop + // is generated, the tx fifo should be empty. + assert!(self.tx.is_empty(), "TX FIFO is empty"); + for encoded in stop() { + self.tx.write_u16_replicated(encoded); } + self.tx.clear_stalled_flag(); + while !self.tx.has_stalled() {} } - fn start(&mut self) { - self.put_instr_sequence([SC1SD0, SC0SD0]) - } - - fn stop(&mut self) { - if self.has_errored() { - self.resume_after_error(); - } - self.put_instr_sequence([SC0SD0, SC1SD0, SC1SD1]) - } - - fn restart(&mut self) { - self.put_instr_sequence([SC0SD1, SC1SD1, SC1SD0, SC0SD0]) - } - - fn setup(&mut self, address: A, read: bool, do_restart: bool) -> Result<(), Error> - where - A: Into + 'static, - { - // TODO: validate addr - let address: u16 = address.into(); - - // flush read fifo - assert!(self.rx.read().is_none(), "rx FIFO shall be empty"); - - // send start condition - if !do_restart { - self.start(); - } else { - self.restart(); - } - - let read_flag = if read { 1 } else { 0 }; - - // send address - use core::any::TypeId; - let a_tid = TypeId::of::(); - let mut address_len: u32 = if TypeId::of::() == a_tid { - let addr = (address << 1) | read_flag; - self.put_data(addr as u8, true, false); - 1 - } else if TypeId::of::() == a_tid { - let addr_hi = 0xF0 | ((address >> 7) & 0x6) | read_flag; - let addr_lo = address & 0xFF; - self.put_data(addr_hi as u8, true, false); - self.put_data(addr_lo as u8, true, false); - 2 - } else { - panic!("Unsupported address type."); - }; - - while !(self.has_errored() || address_len == 0) { - while address_len > 0 && self.rx.read().is_some() { - address_len -= 1; - } - } - - if self.has_errored() { - Err(Error::NoAcknowledgeAddress) - } else { - Ok(()) - } - } - - fn read(&mut self, buffer: &mut [u8]) -> Result<(), Error> { - assert!( - !self.has_errored() && self.rx.is_empty(), - "Invalid state in entering read: has_errored:{} rx.is_empty:{}", - self.has_errored(), - self.rx.is_empty() - ); - - let mut queued = 0; - let mut iter = buffer.iter_mut(); - - // while there are still bytes to queue - while iter.len() != 0 && !self.has_errored() { - if queued < iter.len() && !self.tx.is_full() { - queued += 1; - let last = queued == iter.len(); - self.put_data(0xFF, last, last); - } - - if let Some(byte) = self.rx.read() { - queued -= 1; - if let Some(data) = iter.next() { - *data = (byte & 0xFF) as u8; + fn process_queue<'b>( + &mut self, + queue: impl IntoIterator>, + ) -> Result<(), Error> { + let mut output = queue.into_iter().peekable(); + // P::TX_FIFO_DEPTH + let mut input: Deque, 16> = Deque::new(); + + // while we’re not does sending/receiving + while output.peek().is_some() || !input.is_empty() { + // if there is room in the tx fifo + if !self.tx.is_full() { + if let Some(mut word) = output.next() { + let last = matches!( + (&mut word, output.peek()), + (CmdWord::Data(_), None) | (CmdWord::Data(_), Some(CmdWord::Raw(_))) + ); + let word_u16 = word.encode(last); + self.tx.write_u16_replicated(word_u16); + input.push_back(word).expect("`input` is not full"); } } - } - - if self.has_errored() { - Err(Error::NoAcknowledgeData) - } else { - Ok(()) - } - } - - fn write(&mut self, buffer: B) -> Result<(), Error> - where - B: IntoIterator, - { - assert!( - !self.has_errored() && self.rx.is_empty(), - "Invalid state in entering write: has_errored:{} rx.is_empty:{}", - self.has_errored(), - self.rx.is_empty() - ); - - let mut queued = 0; - let mut iter = buffer.into_iter().peekable(); - while let (Some(byte), false) = (iter.next(), self.has_errored()) { - // ignore any received bytes - if self.rx.read().is_some() { - queued -= 1; - } - self.put_data(byte, true, iter.peek().is_none()); - queued += 1; - } - - while !(queued == 0 || self.has_errored()) { - if self.rx.read().is_some() { - queued -= 1; - } - } - - if self.has_errored() { - Err(Error::NoAcknowledgeData) - } else { - Ok(()) - } - } -} - -impl i2c::Read for I2C<'_, P, SMI, SDA, SCL> -where - A: AddressMode + Into + 'static, - P: PIOExt, - SMI: StateMachineIndex, - SDA: AnyPin, - SCL: AnyPin, -{ - type Error = Error; - - fn read(&mut self, address: A, buffer: &mut [u8]) -> Result<(), Self::Error> { - let mut res = self.setup(address, true, false); - if res.is_ok() { - res = self.read(buffer); - } - self.stop(); - res - } -} - -impl i2c::WriteIter for I2C<'_, P, SMI, SDA, SCL> -where - A: AddressMode + Into + 'static, - P: PIOExt, - SMI: StateMachineIndex, - SDA: AnyPin, - SCL: AnyPin, -{ - type Error = Error; - - fn write(&mut self, address: A, bytes: B) -> Result<(), Self::Error> - where - B: IntoIterator, - { - let mut res = self.setup(address, false, false); - if res.is_ok() { - res = self.write(bytes); - } - self.stop(); - res - } -} -impl i2c::Write for I2C<'_, P, SMI, SDA, SCL> -where - A: AddressMode + Into + 'static, - P: PIOExt, - SMI: StateMachineIndex, - SDA: AnyPin, - SCL: AnyPin, -{ - type Error = Error; - - fn write(&mut self, address: A, buffer: &[u8]) -> Result<(), Self::Error> { - >::write(self, address, buffer.iter().cloned()) - } -} - -impl i2c::WriteIterRead for I2C<'_, P, SMI, SDA, SCL> -where - A: AddressMode + Into + Clone + 'static, - P: PIOExt, - SMI: StateMachineIndex, - SDA: AnyPin, - SCL: AnyPin, -{ - type Error = Error; - fn write_iter_read( - &mut self, - address: A, - bytes: B, - buffer: &mut [u8], - ) -> Result<(), Self::Error> - where - B: IntoIterator, - { - let mut res = self.setup(address.clone(), false, false); - if res.is_ok() { - res = self.write(bytes); - } - if res.is_ok() { - res = self.setup(address, true, true); - } - if res.is_ok() { - res = self.read(buffer); - } - self.stop(); - res - } -} -impl i2c::WriteRead for I2C<'_, P, SMI, SDA, SCL> -where - A: AddressMode + Into + Clone + 'static, - P: PIOExt, - SMI: StateMachineIndex, - SDA: AnyPin, - SCL: AnyPin, -{ - type Error = Error; - - fn write_read( - &mut self, - address: A, - bytes: &[u8], - buffer: &mut [u8], - ) -> Result<(), Self::Error> { - >::write_iter_read( - self, - address, - bytes.iter().cloned(), - buffer, - ) - } -} - -impl i2c::TransactionalIter for I2C<'_, P, SMI, SDA, SCL> -where - A: AddressMode + Into + Clone + 'static, - P: PIOExt, - SMI: StateMachineIndex, - SDA: AnyPin, - SCL: AnyPin, -{ - type Error = Error; - - fn exec_iter<'a, O>(&mut self, address: A, operations: O) -> Result<(), Self::Error> - where - O: IntoIterator>, - { - let mut res = Ok(()); - let mut first = true; - for op in operations { - match op { - Operation::Read(buf) => { - res = self.setup(address.clone(), true, !first); - if res.is_ok() { - res = self.read(buf); + if let Some(word) = self.rx.read() { + let word = (word & 0xFF) as u8; + loop { + match input.pop_front() { + Some(CmdWord::Raw(_)) => continue, + Some(CmdWord::Data(d)) => match d.byte { + Left(exp) if word != exp => { + return self.err_with(Error::BusContention); + } + Right(inp) => *inp = word, + _ => {} + }, + None => {} } + break; } - Operation::Write(buf) => { - res = self.setup(address.clone(), false, !first); - if res.is_ok() { - res = self.write(buf.iter().cloned()); + } else if self.has_errored() { + // the byte that err’ed isn’t in the rx fifo. Once we’re done clearing them, we + // know the head of the queue is the byte that failed. + loop { + match input.pop_front() { + // Raw cmd cannot fail + Some(CmdWord::Raw(_)) => continue, + Some(CmdWord::Data(d)) => { + return self.err_with(if d.is_address { + Error::NoAcknowledgeAddress + } else { + Error::NoAcknowledgeData + }); + } + None => { + unreachable!("There cannot be a failure without a transmition") + } } } - }; - if res.is_err() { - break; } - first = false; } - self.stop(); - res + Ok(()) } } -impl i2c::Transactional for I2C<'_, P, SMI, SDA, SCL> +impl<'pio, P, SMI, SDA, SCL> I2C<'pio, P, SMI, SDA, SCL> where - A: AddressMode + Into + Clone + 'static, P: PIOExt, SMI: StateMachineIndex, SDA: AnyPin, + SDA::Id: ValidFunction, SCL: AnyPin, + SCL::Id: ValidFunction, { - type Error = Error; - - fn exec(&mut self, address: A, operations: &mut [Operation<'_>]) -> Result<(), Self::Error> { - let mut res = Ok(()); - let mut first = true; - for op in operations { - match op { - Operation::Read(buf) => { - res = self.setup(address.clone(), true, !first); - if res.is_ok() { - res = self.read(buf); - } - } - Operation::Write(buf) => { - res = self.setup(address.clone(), false, !first); - if res.is_ok() { - res = self.write(buf.iter().cloned()); - } - } - }; - if res.is_err() { - break; - } - first = false; - } - self.stop(); - res - } -} - -#[cfg(feature = "eh1_0_alpha")] -mod eh1_0_alpha { - use eh1_0_alpha::i2c::{AddressMode, ErrorKind, NoAcknowledgeSource, Operation}; - - use crate::Error; - - use super::{AnyPin, PIOExt, StateMachineIndex, I2C}; - - impl I2C<'_, P, SMI, SDA, SCL> + fn reset_pin( + (mut pin, override_): (Pin, OutputEnableOverride), + ) -> Pin where - P: PIOExt, - SMI: StateMachineIndex, - SDA: AnyPin, - SCL: AnyPin, + I: PinId, + F: Function, + T: PullType, + I: ValidFunction, { - pub fn write_iter(&mut self, address: A, bytes: B) -> Result<(), Error> - where - A: AddressMode + Into + Clone + 'static, - B: IntoIterator, - { - let mut res = self.setup(address, false, false); - if res.is_ok() { - res = self.write(bytes); - } - self.stop(); - res - } - - pub fn write_iter_read( - &mut self, - address: A, - bytes: B, - buffer: &mut [u8], - ) -> Result<(), Error> - where - A: AddressMode + Into + Clone + 'static, - B: IntoIterator, - { - let mut res = self.setup(address.clone(), false, false); - if res.is_ok() { - res = self.write(bytes); - } - if res.is_ok() { - res = self.setup(address, true, true); - } - if res.is_ok() { - res = self.read(buffer); - } - self.stop(); - res - } - - pub fn transaction_iter<'a, A, O>(&mut self, address: A, operations: O) -> Result<(), Error> - where - A: AddressMode + Into + Clone + 'static, - O: IntoIterator>, - { - let mut res = Ok(()); - let mut first = true; - for op in operations { - match op { - Operation::Read(buf) => { - res = self.setup(address.clone(), true, !first); - if res.is_ok() { - res = self.read(buf); - } - } - Operation::Write(buf) => { - res = self.setup(address.clone(), false, !first); - if res.is_ok() { - res = self.write(buf.iter().cloned()); - } - } - }; - if res.is_err() { - break; - } - first = false; - } - self.stop(); - res - } + // Prevent glitches during reconfiguration + pin.set_output_enable_override(OutputEnableOverride::Disable); + // reconfigure the pin + let mut pin = pin.reconfigure(); + // revert to normal operation + pin.set_output_enable_override(override_); + pin } - impl eh1_0_alpha::i2c::Error for super::Error { - fn kind(&self) -> ErrorKind { - match self { - Error::NoAcknowledgeAddress => { - ErrorKind::NoAcknowledge(NoAcknowledgeSource::Address) - } - Error::NoAcknowledgeData => ErrorKind::NoAcknowledge(NoAcknowledgeSource::Data), - } - } - } - - impl eh1_0_alpha::i2c::ErrorType for I2C<'_, P, SMI, SDA, SCL> - where - P: PIOExt, - SMI: StateMachineIndex, - SDA: AnyPin, - SCL: AnyPin, - { - type Error = super::Error; - } - - impl eh1_0_alpha::i2c::I2c for I2C<'_, P, SMI, SDA, SCL> - where - A: AddressMode + Into + Clone + 'static, - P: PIOExt, - SMI: StateMachineIndex, - SDA: AnyPin, - SCL: AnyPin, - { - fn read(&mut self, address: A, buffer: &mut [u8]) -> Result<(), Self::Error> { - let mut res = self.setup(address, true, false); - if res.is_ok() { - res = self.read(buffer); - } - self.stop(); - res - } - - fn write(&mut self, address: A, bytes: &[u8]) -> Result<(), Self::Error> { - self.write_iter(address, bytes.into_iter().cloned()) - } + /// Frees the state machine and pins. + #[allow(clippy::type_complexity)] + pub fn free(self) -> ((SDA::Type, SCL::Type), UninitStateMachine<(P, SMI)>) { + let Self { + pio, + sm, + tx, + rx, + sda, + scl, + .. + } = self; + let (uninit, program) = sm.uninit(rx, tx); + pio.uninstall(program); - fn write_read( - &mut self, - address: A, - bytes: &[u8], - buffer: &mut [u8], - ) -> Result<(), Self::Error> { - self.write_iter_read(address, bytes.into_iter().cloned(), buffer) - } + let scl = Self::reset_pin(scl); + let sda = Self::reset_pin(sda); - fn transaction<'a>( - &mut self, - address: A, - operations: &mut [Operation<'a>], - ) -> Result<(), Self::Error> { - let mut res = Ok(()); - let mut first = true; - for op in operations { - match op { - Operation::Read(buf) => { - res = self.setup(address.clone(), true, !first); - if res.is_ok() { - res = self.read(buf); - } - } - Operation::Write(buf) => { - res = self.setup(address.clone(), false, !first); - if res.is_ok() { - res = self.write(buf.iter().cloned()); - } - } - }; - if res.is_err() { - break; - } - first = false; - } - self.stop(); - res - } + ((sda, scl), uninit) } } From e5c87b9e70effd36e1f7c2686e871aa2b3a67fe7 Mon Sep 17 00:00:00 2001 From: Wilfried Chauveau Date: Mon, 19 Feb 2024 14:00:06 +0000 Subject: [PATCH 2/5] impl nak on address handling --- on-target-tests/tests/i2c_loopback.rs | 9 +- on-target-tests/tests/i2c_tests/blocking.rs | 16 ++++ src/lib.rs | 92 ++++++++++++--------- 3 files changed, 75 insertions(+), 42 deletions(-) diff --git a/on-target-tests/tests/i2c_loopback.rs b/on-target-tests/tests/i2c_loopback.rs index f411b7a..a946b37 100644 --- a/on-target-tests/tests/i2c_loopback.rs +++ b/on-target-tests/tests/i2c_loopback.rs @@ -117,7 +117,14 @@ mod tests { } // Sad paths: - // Peripheral Nack + // Peripheral Nack on Addr + #[test] + fn nak_on_addr(state: &mut State) { + i2c_tests::blocking::nak_on_addr(state, ADDR_7BIT, ADDR_7BIT + 1); + i2c_tests::blocking::nak_on_addr(state, ADDR_10BIT, ADDR_10BIT + 1); + i2c_tests::blocking::nak_on_addr(state, ADDR_10BIT, ADDR_10BIT + 0x100); + } + // Peripheral Nack on Data // // Arbritration conflict } diff --git a/on-target-tests/tests/i2c_tests/blocking.rs b/on-target-tests/tests/i2c_tests/blocking.rs index 261c833..652dde9 100644 --- a/on-target-tests/tests/i2c_tests/blocking.rs +++ b/on-target-tests/tests/i2c_tests/blocking.rs @@ -8,6 +8,7 @@ use hal::{ pac::PIO0, pio::{PIOExt, UninitStateMachine, PIO, PIO0SM0}, }; +use i2c_pio::Error; use rp2040_hal::{ self as hal, clocks::init_clocks_and_plls, @@ -547,3 +548,18 @@ pub fn embedded_hal( assert_eq!(g, h); test_teardown!(state, controller); } + +/// the address given must be different from each other. +pub fn nak_on_addr(state: &mut State, addr_target: A, addr_ctrl: A) { + use embedded_hal::i2c::I2c; + let mut controller = test_setup(state, addr_target, false); + + let samples1: FIFOBuffer = Generator::seq().take(25).collect(); + assert_eq!( + controller.write(addr_ctrl, &samples1), + Err(Error::NoAcknowledgeAddress) + ); + + assert_vec_eq!([]); + test_teardown!(state, controller); +} diff --git a/src/lib.rs b/src/lib.rs index b8f8ab6..bd4227c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -46,6 +46,7 @@ use either::Either::{Left, Right}; use fugit::HertzU32; use heapless::Deque; use i2c_cmd::{restart, start, CmdWord, Data}; +use pio::Instruction; use rp2040_hal::{ gpio::{ AnyPin, Function, FunctionNull, OutputEnableOverride, Pin, PinId, PullType, PullUp, @@ -102,7 +103,7 @@ fn setup<'b, A: ValidAddressMode>( let address = match A::address_len() { AddressLength::_7 => { let address_and_flag = ((address as u8) << 1) | read_flag; - Left([address_and_flag].into_iter().map(CmdWord::address)) + Left(once(address_and_flag).into_iter().map(CmdWord::address)) } AddressLength::_10 => { let addr_hi = 0xF0 | ((address >> 7) as u8) & 0xFE; @@ -136,7 +137,7 @@ fn setup<'b, A: ValidAddressMode>( .chain(address) } -#[derive(Debug)] +#[derive(Debug, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum Error { NoAcknowledgeAddress, @@ -327,16 +328,33 @@ where } } - fn has_errored(&mut self) -> bool { + fn has_irq(&mut self) -> bool { let mask = 1 << SMI::id(); self.pio.get_irq_raw() & mask != 0 } fn err_with(&mut self, err: Error) -> Result<(), Error> { - // clear fifos - self.sm.clear_fifos(); - // resume pio driver - self.pio.clear_irq(1 << SMI::id()); + // clear RX FiFo + while self.rx.read().is_some() {} + // Clear Tx FiFo + self.sm.drain_tx_fifo(); + // wait for the state machine to either stall on pull or block on irq + self.tx.clear_stalled_flag(); + while !(self.tx.has_stalled() || self.has_irq()) {} + + // Clear OSR + if self.has_irq() { + self.sm.exec_instruction(Instruction { + operands: pio::InstructionOperands::OUT { + destination: pio::OutDestination::NULL, + bit_count: 16, + }, + delay: 0, + side_set: None, + }); + // resume pio driver + self.pio.clear_irq(1 << SMI::id()); + } // generate stop condition self.generate_stop(); Err(err) @@ -346,9 +364,10 @@ where // this driver checks for acknoledge error and/or expects data back, so by the time a stop // is generated, the tx fifo should be empty. assert!(self.tx.is_empty(), "TX FIFO is empty"); - for encoded in stop() { + + stop().for_each(|encoded| { self.tx.write_u16_replicated(encoded); - } + }); self.tx.clear_stalled_flag(); while !self.tx.has_stalled() {} } @@ -358,8 +377,10 @@ where queue: impl IntoIterator>, ) -> Result<(), Error> { let mut output = queue.into_iter().peekable(); - // P::TX_FIFO_DEPTH - let mut input: Deque, 16> = Deque::new(); + // - TX FIFO depth (cmd waiting to be sent) + // - OSR + // - RX FIFO input waiting to be processed + let mut input: Deque, 9> = Deque::new(); // while we’re not does sending/receiving while output.peek().is_some() || !input.is_empty() { @@ -372,45 +393,34 @@ where ); let word_u16 = word.encode(last); self.tx.write_u16_replicated(word_u16); - input.push_back(word).expect("`input` is not full"); + if let CmdWord::Data(d) = word { + input.push_back(d).expect("`input` is not full"); + } } } if let Some(word) = self.rx.read() { let word = (word & 0xFF) as u8; - loop { - match input.pop_front() { - Some(CmdWord::Raw(_)) => continue, - Some(CmdWord::Data(d)) => match d.byte { - Left(exp) if word != exp => { - return self.err_with(Error::BusContention); - } - Right(inp) => *inp = word, - _ => {} - }, - None => {} + if let Some(d) = input.pop_front() { + match d.byte { + Left(exp) if word != exp => { + return self.err_with(Error::BusContention); + } + Right(inp) => *inp = word, + _ => {} } - break; } - } else if self.has_errored() { + } else if self.has_irq() { // the byte that err’ed isn’t in the rx fifo. Once we’re done clearing them, we // know the head of the queue is the byte that failed. - loop { - match input.pop_front() { - // Raw cmd cannot fail - Some(CmdWord::Raw(_)) => continue, - Some(CmdWord::Data(d)) => { - return self.err_with(if d.is_address { - Error::NoAcknowledgeAddress - } else { - Error::NoAcknowledgeData - }); - } - None => { - unreachable!("There cannot be a failure without a transmition") - } - } - } + let Some(d) = input.pop_front() else { + unreachable!("There cannot be a failure without a transmition") + }; + return self.err_with(if d.is_address { + Error::NoAcknowledgeAddress + } else { + Error::NoAcknowledgeData + }); } } Ok(()) From b0e0eca438e2be1f20664aefcfb80023ffcbe416 Mon Sep 17 00:00:00 2001 From: Wilfried Chauveau Date: Tue, 20 Feb 2024 19:56:57 +0000 Subject: [PATCH 3/5] remove duplicated memory.x --- on-target-tests/memory.x | 15 --------------- 1 file changed, 15 deletions(-) delete mode 100644 on-target-tests/memory.x diff --git a/on-target-tests/memory.x b/on-target-tests/memory.x deleted file mode 100644 index 4077aab..0000000 --- a/on-target-tests/memory.x +++ /dev/null @@ -1,15 +0,0 @@ -MEMORY { - BOOT2 : ORIGIN = 0x10000000, LENGTH = 0x100 - FLASH : ORIGIN = 0x10000100, LENGTH = 2048K - 0x100 - RAM : ORIGIN = 0x20000000, LENGTH = 256K -} - -EXTERN(BOOT2_FIRMWARE) - -SECTIONS { - /* ### Boot loader */ - .boot2 ORIGIN(BOOT2) : - { - KEEP(*(.boot2)); - } > BOOT2 -} INSERT BEFORE .text; From 02e51fa040460633557e5f6089e39f38003f3e38 Mon Sep 17 00:00:00 2001 From: Jan Niehusmann Date: Sun, 10 Mar 2024 19:55:40 +0000 Subject: [PATCH 4/5] Use released version 0.10.0 of rp2040-hal --- Cargo.toml | 5 +---- on-target-tests/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a1fb1ee..ddc78ce 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,11 +17,8 @@ embedded_hal_0_2 = { version = "0.2.6", package = "embedded-hal" } nb = "1.0.0" pio = "0.2.0" pio-proc = "0.2.0" -rp2040-hal = "0.9.1" +rp2040-hal = "0.10.0" fugit = "0.3.5" defmt = { version = "0.3.0", optional = true } either = { version = "1.10.0", default-features = false } heapless = { version = "0.8.0", default-features = false } - -[patch.crates-io] -rp2040-hal = { git = "https://github.com/rp-rs/rp-hal" } diff --git a/on-target-tests/Cargo.toml b/on-target-tests/Cargo.toml index 2fb63ef..968260a 100644 --- a/on-target-tests/Cargo.toml +++ b/on-target-tests/Cargo.toml @@ -25,7 +25,7 @@ defmt-rtt = "0.4" defmt-test = "0.3.1" panic-probe = { version = "0.3", features = ["print-defmt"] } -rp2040-hal = { version = "0.9.1", features = [ +rp2040-hal = { version = "0.10.0", features = [ "critical-section-impl", "defmt", "rt", From dcf69d3f6ca319132b34368f6a96351e9b7a8cd5 Mon Sep 17 00:00:00 2001 From: Jan Niehusmann Date: Sun, 10 Mar 2024 20:10:04 +0000 Subject: [PATCH 5/5] Fix clippy warning --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index bd4227c..544204c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -103,7 +103,7 @@ fn setup<'b, A: ValidAddressMode>( let address = match A::address_len() { AddressLength::_7 => { let address_and_flag = ((address as u8) << 1) | read_flag; - Left(once(address_and_flag).into_iter().map(CmdWord::address)) + Left(once(address_and_flag).map(CmdWord::address)) } AddressLength::_10 => { let addr_hi = 0xF0 | ((address >> 7) as u8) & 0xFE;