From 96c53c0b11684dd98334e07930e724f9bf9f3d97 Mon Sep 17 00:00:00 2001 From: Wilfried Chauveau Date: Wed, 31 Jan 2024 05:03:24 +0000 Subject: [PATCH] i2c: add on-target-tests & fix discovered issues --- on-target-tests/Cargo.toml | 15 +- on-target-tests/tests/i2c_loopback.rs | 128 +++++ on-target-tests/tests/i2c_tests/blocking.rs | 530 ++++++++++++++++++++ on-target-tests/tests/i2c_tests/mod.rs | 125 +++++ rp2040-hal/examples/i2c.rs | 2 +- rp2040-hal/src/i2c.rs | 97 ++-- rp2040-hal/src/i2c/controller.rs | 383 ++++++++------ rp2040-hal/src/i2c/peripheral.rs | 223 ++++---- 8 files changed, 1227 insertions(+), 276 deletions(-) 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 diff --git a/on-target-tests/Cargo.toml b/on-target-tests/Cargo.toml index c65ed72b0..6a5975e03 100644 --- a/on-target-tests/Cargo.toml +++ b/on-target-tests/Cargo.toml @@ -28,10 +28,17 @@ harness = false name = "dma_dyn" harness = false +[[test]] +name = "i2c_loopback" +harness = false + [dependencies] cortex-m = "0.7" cortex-m-rt = "0.7" -embedded-hal = { version = "0.2.5", features = ["unproven"] } +embedded_hal_0_2 = { package = "embedded-hal", version = "0.2.5", features = [ + "unproven", +] } +embedded-hal = "1.0.0" defmt = "0.3" defmt-rtt = "0.4" @@ -41,9 +48,15 @@ panic-probe = { version = "0.3", features = ["print-defmt"] } rp2040-hal = { path = "../rp2040-hal", features = [ "defmt", "critical-section-impl", + "rt", ] } # Needed to set spi frequencies fugit = "0.3.6" 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/tests/i2c_loopback.rs b/on-target-tests/tests/i2c_loopback.rs new file mode 100644 index 000000000..bad66a8ea --- /dev/null +++ b/on-target-tests/tests/i2c_loopback.rs @@ -0,0 +1,128 @@ +//! 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::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..=9); + } + + #[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: + // invalid tx buf on write + // invalid rx buf on read + // + // invalid (rx/tx) buf in transactions + // + // 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 000000000..13d731c0c --- /dev/null +++ b/on-target-tests/tests/i2c_tests/blocking.rs @@ -0,0 +1,530 @@ +use core::{cell::RefCell, ops::RangeInclusive}; + +use critical_section::Mutex; +use fugit::{HertzU32, RateExtU32}; + +use rp2040_hal::{ + self as hal, + clocks::init_clocks_and_plls, + gpio::{FunctionI2C, Pin, PullUp}, + i2c::{Error, ValidAddress}, + pac, + watchdog::Watchdog, + Clock, Timer, +}; + +use super::{Controller, FIFOBuffer, Generator, MutexCell, Target, TargetState}; + +pub struct State { + controller: Option, + 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 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<_, FunctionI2C, PullUp> = pins.gpio0.reconfigure(); + let ctrl_scl_pin: Pin<_, FunctionI2C, 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 i2c_ctrl = hal::I2C::new_controller( + pac.I2C0, + ctrl_sda_pin, + ctrl_scl_pin, + 400.kHz(), + &mut pac.RESETS, + clocks.system_clock.freq(), + ); + 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 + 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 { + controller: Some(i2c_ctrl), + timer, + resets: pac.RESETS, + ref_clock_freq: clocks.system_clock.freq(), + } +} + +pub fn reset(state: &mut State, addr: T, throttling: bool) -> &mut Controller { + // reset controller + let (i2c, (sda, scl)) = state + .controller + .take() + .expect("State contains a controller") + .free(&mut state.resets); + + // 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, + )), + ); + }); + + state.controller = Some(hal::I2C::new_controller( + i2c, + sda, + scl, + 400.kHz(), + &mut state.resets, + state.ref_clock_freq, + )); + state + .controller + .as_mut() + .expect("State contains a controller") +} + +/// 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 controller = reset(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); +} +pub fn write_iter(state: &mut State, addr: T) { + let controller = reset(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); +} + +pub fn write_iter_read( + state: &mut State, + addr: T, + restart_count: RangeInclusive, +) { + let controller = reset(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); +} + +pub fn write_read(state: &mut State, addr: T, restart_count: RangeInclusive) { + use embedded_hal_0_2::blocking::i2c::WriteRead; + let controller = reset(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); +} + +pub fn read(state: &mut State, addr: T, restart_count: RangeInclusive) { + use embedded_hal_0_2::blocking::i2c::Read; + let controller = reset(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!([]); +} + +pub fn transactions_read( + state: &mut State, + addr: T, + restart_count: RangeInclusive, +) { + use embedded_hal::i2c::{I2c, Operation}; + let controller = reset(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!([]); +} + +pub fn transactions_write(state: &mut State, addr: T) { + use embedded_hal::i2c::{I2c, Operation}; + let controller = reset(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); +} + +pub fn transactions_read_write( + state: &mut State, + addr: T, + restart_count: RangeInclusive, +) { + use embedded_hal::i2c::{I2c, Operation}; + let controller = reset(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); +} + +pub fn transactions_write_read( + state: &mut State, + addr: T, + restart_count: RangeInclusive, +) { + use embedded_hal::i2c::{I2c, Operation}; + let controller = reset(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); +} + +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 controller = reset(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); +} + +pub fn transactions_iter( + state: &mut State, + addr: T, + restart_count: RangeInclusive, +) { + use embedded_hal::i2c::{I2c, Operation}; + let controller = reset(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); +} + +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 controller = reset(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::<(), 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); +} 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 000000000..c3cf0a10c --- /dev/null +++ b/on-target-tests/tests/i2c_tests/mod.rs @@ -0,0 +1,125 @@ +use core::cell::RefCell; + +use critical_section::Mutex; +use rp2040_hal::{ + self as hal, + gpio::{ + bank0::{Gpio0, Gpio1, Gpio2, Gpio3}, + FunctionI2C, Pin, PullUp, + }, + i2c::peripheral::Event, +}; + +pub mod 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 = hal::I2C, hal::i2c::Controller>; +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(); + } + } +} diff --git a/rp2040-hal/examples/i2c.rs b/rp2040-hal/examples/i2c.rs index ef6480912..9f325bec1 100644 --- a/rp2040-hal/examples/i2c.rs +++ b/rp2040-hal/examples/i2c.rs @@ -94,7 +94,7 @@ fn main() -> ! { ); // Write three bytes to the I²C device with 7-bit address 0x2C - i2c.write(0x2c, &[1, 2, 3]).unwrap(); + i2c.write(0x2Cu8, &[1, 2, 3]).unwrap(); // Demo finish - just loop until reset diff --git a/rp2040-hal/src/i2c.rs b/rp2040-hal/src/i2c.rs index a01fccdbb..cb225db56 100644 --- a/rp2040-hal/src/i2c.rs +++ b/rp2040-hal/src/i2c.rs @@ -21,7 +21,7 @@ //! //! // Scan for devices on the bus by attempting to read from them //! use embedded_hal_0_2::prelude::_embedded_hal_blocking_i2c_Read; -//! for i in 0..=127 { +//! for i in 0..=127u8 { //! let mut readbuf: [u8; 1] = [0; 1]; //! let result = i2c.read(i, &mut readbuf); //! if let Ok(d) = result { @@ -32,12 +32,12 @@ //! //! // Write some data to a device at 0x2c //! use embedded_hal_0_2::prelude::_embedded_hal_blocking_i2c_Write; -//! i2c.write(0x2c, &[1, 2, 3]).unwrap(); +//! i2c.write(0x2Cu8, &[1, 2, 3]).unwrap(); //! //! // Write and then read from a device at 0x3a //! use embedded_hal_0_2::prelude::_embedded_hal_blocking_i2c_WriteRead; //! let mut readbuf: [u8; 1] = [0; 1]; -//! i2c.write_read(0x2c, &[1, 2, 3], &mut readbuf).unwrap(); +//! i2c.write_read(0x2Cu8, &[1, 2, 3], &mut readbuf).unwrap(); //! ``` //! //! See [examples/i2c.rs](https://github.com/rp-rs/rp-hal/tree/main/rp2040-hal/examples/i2c.rs) @@ -45,10 +45,14 @@ use core::{marker::PhantomData, ops::Deref}; use fugit::HertzU32; +use rp2040_pac::i2c0::ic_con::IC_10BITADDR_SLAVE_A; use crate::{ gpio::{bank0::*, pin::pin_sealed::TypeLevelPinId, AnyPin, FunctionI2c, PullUp}, - pac::{self, i2c0::RegisterBlock as I2CBlock, I2C0, I2C1, RESETS}, + pac::{ + i2c0::{ic_con::IC_10BITADDR_MASTER_A, RegisterBlock}, + I2C0, I2C1, RESETS, + }, resets::SubsystemReset, typelevel::Sealed, }; @@ -57,19 +61,52 @@ mod controller; pub mod peripheral; /// Pac I2C device -pub trait I2cDevice: Deref + SubsystemReset + Sealed { +pub trait I2cDevice: Deref + SubsystemReset + Sealed { /// Index of the peripheral. const ID: usize; } -impl Sealed for pac::I2C0 {} -impl I2cDevice for pac::I2C0 { +impl Sealed for I2C0 {} +impl I2cDevice for I2C0 { const ID: usize = 0; } -impl Sealed for pac::I2C1 {} -impl I2cDevice for pac::I2C1 { +impl Sealed for I2C1 {} +impl I2cDevice for I2C1 { const ID: usize = 1; } +/// Marks valid/supported address types +pub trait ValidAddress: + Into + embedded_hal::i2c::AddressMode + embedded_hal_0_2::blocking::i2c::AddressMode + Copy +{ + /// Variant for the IC_CON.10bitaddr_master field + const BIT_ADDR_M: IC_10BITADDR_MASTER_A; + /// Variant for the IC_CON.10bitaddr_slave field + const BIT_ADDR_S: IC_10BITADDR_SLAVE_A; + + /// Validates the address against address ranges supported by the hardware. + fn is_valid(self) -> Result<(), Error>; +} +impl ValidAddress for u8 { + const BIT_ADDR_M: IC_10BITADDR_MASTER_A = IC_10BITADDR_MASTER_A::ADDR_7BITS; + const BIT_ADDR_S: IC_10BITADDR_SLAVE_A = IC_10BITADDR_SLAVE_A::ADDR_7BITS; + + fn is_valid(self) -> Result<(), Error> { + if self >= 0x80 { + Err(Error::AddressOutOfRange(self.into())) + } else { + Ok(()) + } + } +} +impl ValidAddress for u16 { + const BIT_ADDR_M: IC_10BITADDR_MASTER_A = IC_10BITADDR_MASTER_A::ADDR_10BITS; + const BIT_ADDR_S: IC_10BITADDR_SLAVE_A = IC_10BITADDR_SLAVE_A::ADDR_10BITS; + + fn is_valid(self) -> Result<(), Error> { + Ok(()) + } +} + /// I2C error #[non_exhaustive] pub enum Error { @@ -226,14 +263,18 @@ pub trait I2CMode: Sealed { /// Indicates whether this mode is Controller or Peripheral. const IS_CONTROLLER: bool; } + /// Marker for an I2C peripheral operating as a controller. -pub enum Controller {} +pub struct Controller {} impl Sealed for Controller {} impl I2CMode for Controller { const IS_CONTROLLER: bool = true; } -/// Marker for an I2C peripheral operating as a peripehral. -pub enum Peripheral {} + +/// Marker for an I2C block operating as a peripehral. +pub struct Peripheral { + state: peripheral::State, +} impl Sealed for Peripheral {} impl I2CMode for Peripheral { const IS_CONTROLLER: bool = false; @@ -243,19 +284,13 @@ impl I2CMode for Peripheral { pub struct I2C { i2c: I2C, pins: Pins, - mode: PhantomData, -} - -const TX_FIFO_SIZE: u8 = 16; -const RX_FIFO_SIZE: u8 = 16; - -fn i2c_reserved_addr(addr: u16) -> bool { - (addr & 0x78) == 0 || (addr & 0x78) == 0x78 + mode: Mode, } +impl Sealed for I2C {} impl I2C where - Block: SubsystemReset + Deref, + Block: SubsystemReset + Deref, { /// Releases the I2C peripheral and associated pins #[allow(clippy::type_complexity)] @@ -266,7 +301,13 @@ where } } -impl, PINS, Mode> I2C { +impl, PINS, Mode> I2C { + /// Depth of the TX FIFO. + pub const TX_FIFO_DEPTH: u8 = 16; + + /// Depth of the RX FIFO. + pub const RX_FIFO_DEPTH: u8 = 16; + /// Number of bytes currently in the RX FIFO #[inline] pub fn rx_fifo_used(&self) -> u8 { @@ -275,14 +316,14 @@ impl, PINS, Mode> I2C { /// Remaining capacity in the RX FIFO #[inline] - pub fn rx_fifo_free(&self) -> u8 { - RX_FIFO_SIZE - self.rx_fifo_used() + pub fn rx_fifo_available(&self) -> u8 { + Self::RX_FIFO_DEPTH - self.rx_fifo_used() } /// RX FIFO is empty #[inline] pub fn rx_fifo_empty(&self) -> bool { - self.rx_fifo_used() == 0 + self.i2c.ic_status.read().rfne().bit_is_clear() } /// Number of bytes currently in the TX FIFO @@ -293,14 +334,14 @@ impl, PINS, Mode> I2C { /// Remaining capacity in the TX FIFO #[inline] - pub fn tx_fifo_free(&self) -> u8 { - TX_FIFO_SIZE - self.tx_fifo_used() + pub fn tx_fifo_available(&self) -> u8 { + Self::TX_FIFO_DEPTH - self.tx_fifo_used() } /// TX FIFO is at capacity #[inline] pub fn tx_fifo_full(&self) -> bool { - self.tx_fifo_free() == 0 + self.i2c.ic_status.read().tfnf().bit_is_clear() } } diff --git a/rp2040-hal/src/i2c/controller.rs b/rp2040-hal/src/i2c/controller.rs index 3fef2f609..0f5c6ef1d 100644 --- a/rp2040-hal/src/i2c/controller.rs +++ b/rp2040-hal/src/i2c/controller.rs @@ -1,11 +1,11 @@ -use core::{marker::PhantomData, ops::Deref}; +use core::{ops::Deref, task::Poll}; use embedded_hal_0_2::blocking::i2c::{Read, Write, WriteIter, WriteIterRead, WriteRead}; use fugit::HertzU32; use embedded_hal::i2c as eh1; -use super::{i2c_reserved_addr, Controller, Error, ValidPinScl, ValidPinSda, I2C}; use crate::{ + i2c::{Controller, Error, ValidAddress, ValidPinScl, ValidPinSda, I2C}, pac::{i2c0::RegisterBlock as Block, RESETS}, resets::SubsystemReset, }; @@ -86,12 +86,25 @@ where .write(|w| w.ic_fs_scl_hcnt().bits(hcnt as u16)); i2c.ic_fs_scl_lcnt .write(|w| w.ic_fs_scl_lcnt().bits(lcnt as u16)); + // spike filter duration i2c.ic_fs_spklen.write(|w| { - w.ic_fs_spklen() - .bits(if lcnt < 16 { 1 } else { (lcnt / 16) as u8 }) + let a = if lcnt < 16 { 1 } else { (lcnt / 16) as u8 }; + w.ic_fs_spklen().bits(a) }); + // sda hold time i2c.ic_sda_hold .modify(|_r, w| w.ic_sda_tx_hold().bits(sda_tx_hold_count as u16)); + + // make TX_EMPTY raise when the tx fifo is not full + i2c.ic_tx_tl.write(|w| w.tx_tl().bits(Self::TX_FIFO_DEPTH)); + // make RX_FULL raise when the rx fifo contains at least 1 byte + i2c.ic_rx_tl.write(|w| w.rx_tl().bits(0)); + // Enable clock stretching. + // Will hold clock when: + // - receiving and rx fifo is full + // - writting and tx fifo is empty + i2c.ic_con + .modify(|_, w| w.rx_fifo_full_hld_ctrl().enabled()); } // Enable I2C block @@ -100,89 +113,136 @@ where Self { i2c, pins: (sda_pin, scl_pin), - mode: PhantomData, + mode: Controller {}, } } } -impl, PINS> I2C { - fn validate( - addr: u16, - opt_tx_empty: Option, - opt_rx_empty: Option, - ) -> Result<(), Error> { - // validate tx parameters if present - if opt_tx_empty.unwrap_or(false) { - return Err(Error::InvalidWriteBufferLength); - } - - // validate rx parameters if present - if opt_rx_empty.unwrap_or(false) { - return Err(Error::InvalidReadBufferLength); - } - // validate address - if addr >= 0x80 { - Err(Error::AddressOutOfRange(addr)) - } else if i2c_reserved_addr(addr) { - Err(Error::AddressReserved(addr)) - } else { +impl, PINS> I2C { + fn validate_buffer( + &mut self, + first: bool, + buf: &mut core::iter::Peekable, + err: Error, + ) -> Result<(), Error> + where + U: Iterator, + { + if buf.peek().is_some() { Ok(()) + } else { + if !first { + self.abort(); + } + Err(err) } } - fn setup(&mut self, addr: u16) { + fn setup(&mut self, addr: A) -> Result<(), Error> { + addr.is_valid()?; + self.i2c.ic_enable.write(|w| w.enable().disabled()); + self.i2c + .ic_con + .modify(|_, w| w.ic_10bitaddr_master().variant(A::BIT_ADDR_M)); + + let addr = addr.into(); self.i2c.ic_tar.write(|w| unsafe { w.ic_tar().bits(addr) }); self.i2c.ic_enable.write(|w| w.enable().enabled()); + Ok(()) } - fn read_and_clear_abort_reason(&mut self) -> Option { + #[inline] + fn read_and_clear_abort_reason(&mut self) -> Result<(), Error> { let abort_reason = self.i2c.ic_tx_abrt_source.read().bits(); if abort_reason != 0 { // Note clearing the abort flag also clears the reason, and // this instance of flag is clear-on-read! Note also the // IC_CLR_TX_ABRT register always reads as 0. self.i2c.ic_clr_tx_abrt.read(); - Some(abort_reason) + Err(Error::Abort(abort_reason)) } else { - None + Ok(()) } } + #[inline] + fn poll_tx_not_full(&mut self) -> Poll> { + self.read_and_clear_abort_reason()?; + if !self.tx_fifo_full() { + Poll::Ready(Ok(())) + } else { + Poll::Pending + } + } + + #[inline] + fn poll_tx_empty(&mut self) -> Poll<()> { + if self.i2c.ic_raw_intr_stat.read().tx_empty().is_inactive() { + Poll::Pending + } else { + Poll::Ready(()) + } + } + + #[inline] + fn poll_stop_deteced(&mut self) -> Poll<()> { + if self.i2c.ic_raw_intr_stat.read().stop_det().is_inactive() { + Poll::Pending + } else { + Poll::Ready(()) + } + } + + #[inline] + fn abort(&mut self) { + self.i2c.ic_enable.modify(|_, w| w.abort().set_bit()); + while self.i2c.ic_enable.read().abort().bit_is_set() { + cortex_m::asm::nop() + } + while self.i2c.ic_raw_intr_stat.read().tx_abrt().bit_is_clear() { + cortex_m::asm::nop() + } + // clear tx_abort interrupt flags (might have already been clear by irq) + self.i2c.ic_clr_tx_abrt.read(); + // clear tx_abrt_source by reading it + self.i2c.ic_tx_abrt_source.read(); + } + fn read_internal( &mut self, + first_transaction: bool, buffer: &mut [u8], - force_restart: bool, do_stop: bool, ) -> Result<(), Error> { + self.validate_buffer( + first_transaction, + &mut buffer.iter().peekable(), + Error::InvalidReadBufferLength, + )?; + let lastindex = buffer.len() - 1; + let mut first_byte = true; for (i, byte) in buffer.iter_mut().enumerate() { - let first = i == 0; - let last = i == lastindex; + let last_byte = i == lastindex; // wait until there is space in the FIFO to write the next byte - while self.tx_fifo_full() {} + while self.i2c.ic_status.read().tfnf().bit_is_clear() {} self.i2c.ic_data_cmd.write(|w| { - if force_restart && first { - w.restart().enable(); - } else { - w.restart().disable(); - } - - if do_stop && last { - w.stop().enable(); - } else { - w.stop().disable(); + if first_byte { + if !first_transaction { + w.restart().enable(); + } + first_byte = false; } + w.stop().bit(do_stop && last_byte); w.cmd().read() }); while self.i2c.ic_rxflr.read().bits() == 0 { - if let Some(abort_reason) = self.read_and_clear_abort_reason() { - return Err(Error::Abort(abort_reason)); - } + self.read_and_clear_abort_reason()?; } *byte = self.i2c.ic_data_cmd.read().dat().bits(); @@ -191,64 +251,82 @@ impl, PINS> I2C { Ok(()) } - fn write_internal(&mut self, bytes: &[u8], do_stop: bool) -> Result<(), Error> { - for (i, byte) in bytes.iter().enumerate() { - let last = i == bytes.len() - 1; + fn write_internal( + &mut self, + first_transaction: bool, + bytes: impl IntoIterator, + do_stop: bool, + ) -> Result<(), Error> { + let mut peekable = bytes.into_iter().peekable(); + self.validate_buffer( + first_transaction, + &mut peekable, + Error::InvalidWriteBufferLength, + )?; + + let mut abort_reason = Ok(()); + let mut first_byte = true; + 'outer: while let Some(byte) = peekable.next() { + if self.tx_fifo_full() { + // wait for more room in the fifo + loop { + match self.poll_tx_not_full() { + Poll::Pending => continue, + Poll::Ready(Ok(())) => break, + Poll::Ready(r) => { + abort_reason = r; + break 'outer; + } + } + } + } + // else enqueue + let last = peekable.peek().is_none(); self.i2c.ic_data_cmd.write(|w| { - if do_stop && last { - w.stop().enable(); - } else { - w.stop().disable(); + if first_byte { + if !first_transaction { + w.restart().enable(); + } + first_byte = false; } - unsafe { w.dat().bits(*byte) } + w.stop().bit(do_stop && last); + unsafe { w.dat().bits(byte) } }); + } + if abort_reason.is_err() { // Wait until the transmission of the address/data from the internal - // shift register has completed. For this to function correctly, the - // TX_EMPTY_CTRL flag in IC_CON must be set. The TX_EMPTY_CTRL flag - // was set in i2c_init. - while self.i2c.ic_raw_intr_stat.read().tx_empty().is_inactive() {} - - let abort_reason = self.read_and_clear_abort_reason(); - - if abort_reason.is_some() || (do_stop && last) { - // If the transaction was aborted or if it completed - // successfully wait until the STOP condition has occured. - - while self.i2c.ic_raw_intr_stat.read().stop_det().is_inactive() {} - - self.i2c.ic_clr_stop_det.read().clr_stop_det(); - } + // shift register has completed. + while self.poll_tx_empty().is_pending() {} + abort_reason = self.read_and_clear_abort_reason(); + } - // Note the hardware issues a STOP automatically on an abort condition. - // Note also the hardware clears RX FIFO as well as TX on abort, - // ecause we set hwparam IC_AVOID_RX_FIFO_FLUSH_ON_TX_ABRT to 0. - if let Some(abort_reason) = abort_reason { - return Err(Error::Abort(abort_reason)); - } + if abort_reason.is_err() || do_stop { + // If the transaction was aborted or if it completed + // successfully wait until the STOP condition has occured. + while self.poll_stop_deteced().is_pending() {} + self.i2c.ic_clr_stop_det.read().clr_stop_det(); } - Ok(()) + // Note: the hardware issues a STOP automatically on an abort condition. + // Note: the hardware also clears RX FIFO as well as TX on abort + + abort_reason } +} +impl, PINS> I2C { /// Writes bytes to slave with address `address` /// /// # I2C Events (contract) /// /// Same as the `write` method - pub fn write_iter(&mut self, address: u8, bytes: B) -> Result<(), Error> + pub fn write_iter(&mut self, address: A, bytes: B) -> Result<(), Error> where B: IntoIterator, { - let mut peekable = bytes.into_iter().peekable(); - let addr: u16 = address.into(); - Self::validate(addr, Some(peekable.peek().is_none()), None)?; - self.setup(addr); - - while let Some(tx) = peekable.next() { - self.write_internal(&[tx], peekable.peek().is_none())? - } - Ok(()) + self.setup(address)?; + self.write_internal(true, bytes, true) } /// Writes bytes to slave with address `address` and then reads enough bytes to fill `buffer` *in a @@ -257,24 +335,19 @@ impl, PINS> I2C { /// # I2C Events (contract) /// /// Same as the `write_read` method - pub fn write_iter_read( + pub fn write_iter_read( &mut self, - address: u8, + address: A, bytes: B, buffer: &mut [u8], ) -> Result<(), Error> where B: IntoIterator, { - let mut peekable = bytes.into_iter().peekable(); - let addr: u16 = address.into(); - Self::validate(addr, Some(peekable.peek().is_none()), None)?; - self.setup(addr); + self.setup(address)?; - for tx in peekable { - self.write_internal(&[tx], false)? - } - self.read_internal(buffer, true, true) + self.write_internal(true, bytes, false)?; + self.read_internal(false, buffer, true) } /// Execute the provided operations on the I2C bus (iterator version). @@ -290,66 +363,84 @@ impl, PINS> I2C { /// - `SAD+R/W` = slave address followed by bit 1 to indicate reading or 0 to indicate writing /// - `SR` = repeated start condition /// - `SP` = stop condition - pub fn transaction_iter<'a, O>(&mut self, address: u8, operations: O) -> Result<(), Error> + fn transaction<'op: 'iter, 'iter, A: ValidAddress>( + &mut self, + address: A, + operations: impl IntoIterator>, + ) -> Result<(), Error> { + self.setup(address)?; + + let mut first = true; + let mut operations = operations.into_iter().peekable(); + while let Some(operation) = operations.next() { + let last = operations.peek().is_none(); + match operation { + eh1::Operation::Read(buf) => self.read_internal(first, buf, last)?, + eh1::Operation::Write(buf) => { + self.write_internal(first, buf.iter().cloned(), last)? + } + } + first = false; + } + Ok(()) + } + + #[cfg(feature = "i2c-write-iter")] + fn transaction_iter<'op, A, O, B>(&mut self, address: A, operations: O) -> Result<(), Error> where - O: IntoIterator>, + A: ValidAddress, + O: IntoIterator>, + B: IntoIterator, { - let addr: u16 = address.into(); - self.setup(addr); - let mut peekable = operations.into_iter().peekable(); - while let Some(operation) = peekable.next() { - let last = peekable.peek().is_none(); + use i2c_write_iter::Operation; + self.setup(address)?; + + let mut first = true; + let mut operations = operations.into_iter().peekable(); + while let Some(operation) = operations.next() { + let last = operations.peek().is_none(); match operation { - eh1::Operation::Read(buf) => self.read_internal(buf, false, last)?, - eh1::Operation::Write(buf) => self.write_internal(buf, last)?, + Operation::Read(buf) => self.read_internal(first, buf, last)?, + Operation::WriteIter(buf) => self.write_internal(first, buf, last)?, } + first = false; } Ok(()) } } -impl, PINS> Read for I2C { +impl, PINS> Read for I2C { type Error = Error; - fn read(&mut self, addr: u8, buffer: &mut [u8]) -> Result<(), Error> { - let addr: u16 = addr.into(); - - Self::validate(addr, None, Some(buffer.is_empty()))?; - - self.setup(addr); - self.read_internal(buffer, true, true) + fn read(&mut self, addr: A, buffer: &mut [u8]) -> Result<(), Error> { + self.setup(addr)?; + self.read_internal(true, buffer, true) } } -impl, PINS> WriteRead for I2C { +impl, PINS> WriteRead for I2C { type Error = Error; - fn write_read(&mut self, addr: u8, tx: &[u8], rx: &mut [u8]) -> Result<(), Error> { - let addr: u16 = addr.into(); + fn write_read(&mut self, addr: A, tx: &[u8], rx: &mut [u8]) -> Result<(), Error> { + self.setup(addr)?; - Self::validate(addr, Some(tx.is_empty()), Some(rx.is_empty()))?; - self.setup(addr); - - self.write_internal(tx, false)?; - self.read_internal(rx, true, true) + self.write_internal(true, tx.iter().cloned(), false)?; + self.read_internal(false, rx, true) } } -impl, PINS> Write for I2C { +impl, PINS> Write for I2C { type Error = Error; - fn write(&mut self, addr: u8, tx: &[u8]) -> Result<(), Error> { - let addr: u16 = addr.into(); - Self::validate(addr, Some(tx.is_empty()), None)?; - self.setup(addr); - - self.write_internal(tx, true) + fn write(&mut self, addr: A, tx: &[u8]) -> Result<(), Error> { + self.setup(addr)?; + self.write_internal(true, tx.iter().cloned(), true) } } -impl, PINS> WriteIter for I2C { +impl, PINS> WriteIter for I2C { type Error = Error; - fn write(&mut self, address: u8, bytes: B) -> Result<(), Self::Error> + fn write(&mut self, address: A, bytes: B) -> Result<(), Self::Error> where B: IntoIterator, { @@ -357,12 +448,14 @@ impl, PINS> WriteIter for I2C { } } -impl, PINS> WriteIterRead for I2C { +impl, PINS> WriteIterRead + for I2C +{ type Error = Error; fn write_iter_read( &mut self, - address: u8, + address: A, bytes: B, buffer: &mut [u8], ) -> Result<(), Self::Error> @@ -377,29 +470,13 @@ impl, PINS> eh1::ErrorType for I2C type Error = Error; } -impl, PINS> eh1::I2c for I2C { - fn write(&mut self, addr: u8, bytes: &[u8]) -> Result<(), Self::Error> { - Write::write(self, addr, bytes) - } - - fn read(&mut self, addr: u8, buffer: &mut [u8]) -> Result<(), Error> { - Read::read(self, addr, buffer) - } - +impl, PINS> eh1::I2c for I2C { fn transaction( &mut self, - address: u8, + address: A, operations: &mut [eh1::Operation<'_>], ) -> Result<(), Self::Error> { - let addr: u16 = address.into(); - self.setup(addr); - for i in 0..operations.len() { - let last = i == operations.len() - 1; - match &mut operations[i] { - eh1::Operation::Read(buf) => self.read_internal(buf, false, last)?, - eh1::Operation::Write(buf) => self.write_internal(buf, last)?, - } - } - Ok(()) + self.transaction(address, operations.iter_mut()) } } + diff --git a/rp2040-hal/src/i2c/peripheral.rs b/rp2040-hal/src/i2c/peripheral.rs index 593885006..1011fbf87 100644 --- a/rp2040-hal/src/i2c/peripheral.rs +++ b/rp2040-hal/src/i2c/peripheral.rs @@ -3,36 +3,56 @@ //! The RP2040 I2C block can behave as a peripheral node on an I2C bus. //! //! In order to handle peripheral transactions this driver exposes an iterator streaming I2C event -//! that the usercode must handle to properly handle the I2C communitation. See [`I2CEvent`] for a +//! that the usercode must handle to properly handle the I2C communitation. See [`Event`] for a //! list of events to handle. //! -//! Although [`Start`](I2CEvent::Start), [`Restart`](I2CEvent::Restart) and [`Stop`](I2CEvent::Stop) -//! events may not require any action on the device, [`TransferRead`](I2CEvent::TransferRead) and -//! [`TransferWrite`](I2CEvent::TransferWrite) require some action: +//! Although [`Start`](Event::Start), [`Restart`](Event::Restart) and [`Stop`](Event::Stop) +//! events may not require any action on the device, [`TransferRead`](Event::TransferRead) and +//! [`TransferWrite`](Event::TransferWrite) require some action: //! -//! - [`TransferRead`](I2CEvent::TransferRead): A controller is attempting to read from this peripheral. +//! - [`TransferRead`](Event::TransferRead): A controller is attempting to read from this peripheral. //! The I2C block holds the SCL line low (clock stretching) until data is pushed to the transmission -//! FIFO by the user application using [`write`](I2CPeripheralEventIterator::write). +//! FIFO by the user application using [`write`](I2C::write). //! Data remaining in the FIFO when the bus constroller stops the transfer are ignored & the fifo //! is flushed. -//! - [`TransferWrite`](I2CEvent::TransferWrite): A controller is sending data to this peripheral. +//! - [`TransferWrite`](Event::TransferWrite): A controller is sending data to this peripheral. //! The I2C block holds the SCL line (clock stretching) until there is room for more data in the -//! Rx FIFO using [`read`](I2CPeripheralEventIterator::read). +//! Rx FIFO using [`read`](I2C::read). //! Data are automatically acknowledged by the I2C block and it is not possible to NACK incoming //! data comming to the rp2040. +//! +//! ## Warning +//! +//! `Start`, `Restart` and `Stop` events may not be reported before or after a write operations. +//! This is because several write operation may take place and complete before the core has time to +//! react. All the data sent will be stored in the peripheral FIFO but it will not be possible to +//! identify between which bytes should the start/restart/stop events precicely took place. +//! +//! Because a Read operation will always cause a pause waiting for the firmware's input, a `Start` +//! (or `Restart` if the peripheral is already active) will always be reported. However, this does +//! not mean no other event occured in the mean time. +//! +//! For example, let's consider the following sequence: +//! +//! `Start, Write, Stop, Start, Write, Restart, Read, Stop.` +//! +//! Depending on the firmware's and bus' speed, this driver may only report: +//! - `Start, Write, Restart, Read, Stop` + +use core::ops::Deref; -use core::{marker::PhantomData, ops::Deref}; +use embedded_hal::i2c::AddressMode; -use super::{Peripheral, ValidPinScl, ValidPinSda, I2C}; +use super::{Peripheral, ValidAddress, ValidPinScl, ValidPinSda, I2C}; use crate::{ - pac::{i2c0::RegisterBlock as I2CBlock, RESETS}, + pac::{i2c0::RegisterBlock, RESETS}, resets::SubsystemReset, }; /// I2C bus events #[derive(Debug, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub enum I2CEvent { +pub enum Event { /// Start condition has been detected. Start, /// Restart condition has been detected. @@ -45,23 +65,18 @@ pub enum I2CEvent { Stop, } -#[derive(Debug, Clone, Copy)] -enum State { +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub(crate) enum State { Idle, Active, Read, Write, } -/// Provides Async features to I2C peripheral. -pub struct I2CPeripheralEventIterator { - i2c: I2C, - state: State, -} - impl I2C where - T: SubsystemReset + Deref, + T: SubsystemReset + Deref, Sda: ValidPinSda, Scl: ValidPinScl, { @@ -69,20 +84,22 @@ where /// /// The bus *MUST* be idle when this method is called. #[allow(clippy::type_complexity)] - pub fn new_peripheral_event_iterator( + pub fn new_peripheral_event_iterator( i2c: T, sda_pin: Sda, scl_pin: Scl, resets: &mut RESETS, - addr: u16, - ) -> I2CPeripheralEventIterator { + addr: A, + ) -> Self { i2c.reset_bring_down(resets); i2c.reset_bring_up(resets); i2c.ic_enable.write(|w| w.enable().disabled()); - // TODO: rp2040 supports 10bits addressing - //i2c_reserved_addr(addr) + // TODO: Validate address? + let addr = addr.into(); + // SAFETY: Only address by this function. IC_SAR spec filters out bits 15:10. + // Any value is valid for the controller. They may not be for the bus itself though. i2c.ic_sar.write(|w| unsafe { w.ic_sar().bits(addr) }); // select peripheral mode & speed i2c.ic_con.modify(|_, w| { @@ -94,120 +111,140 @@ where // hold scl when fifo's full w.rx_fifo_full_hld_ctrl().enabled(); w.ic_restart_en().enabled(); - w.tx_empty_ctrl().enabled() + w.ic_10bitaddr_slave().variant(A::BIT_ADDR_S); + //w.tx_empty_ctrl().enabled(); + w }); // Clear FIFO threshold + // SAFETY: Only address by this function. The field is 8bit long. 0 is a valid value. i2c.ic_tx_tl.write(|w| unsafe { w.tx_tl().bits(0) }); i2c.ic_rx_tl.write(|w| unsafe { w.rx_tl().bits(0) }); + i2c.ic_clr_intr.read(); + + let mut me = Self { + i2c, + pins: (sda_pin, scl_pin), + mode: Peripheral { state: State::Idle }, + }; + me.unmask_intr(); // Enable I2C block - i2c.ic_enable.write(|w| w.enable().enabled()); - - I2CPeripheralEventIterator { - i2c: Self { - i2c, - pins: (sda_pin, scl_pin), - mode: PhantomData, - }, - state: State::Idle, - } + me.i2c.ic_enable.write(|w| w.enable().enabled()); + + me } } -impl, PINS> I2CPeripheralEventIterator { +/// SAFETY: Takes a non-mutable reference to RegisterBlock but mutates its `ic_intr_mask` register. +unsafe fn unmask_intr(i2c: &RegisterBlock) { + // SAFETY: 0 is a valid value meaning all irq masked. + i2c.ic_intr_mask.write_with_zero(|w| { + // Only keep these IRQ enabled. + w.m_start_det() + .disabled() + .m_rd_req() + .disabled() + .m_rx_full() + .disabled() + .m_stop_det() + .disabled() + }); +} + +impl, PINS> I2C { + fn unmask_intr(&mut self) { + // SAFETY: We are the only owner of this register block. + unsafe { unmask_intr(&self.i2c) } + } + /// Push up to `usize::min(TX_FIFO_SIZE, buf.len())` bytes to the TX FIFO. /// Returns the number of bytes pushed to the FIFO. Note this does *not* reflect how many bytes /// are effectively received by the controller. pub fn write(&mut self, buf: &[u8]) -> usize { // just in case, clears previous tx abort. - self.i2c.i2c.ic_clr_tx_abrt.read(); + self.i2c.ic_clr_tx_abrt.read(); let mut sent = 0; for &b in buf.iter() { - if self.i2c.tx_fifo_full() { + if self.tx_fifo_full() { break; } - self.i2c - .i2c - .ic_data_cmd - .write(|w| unsafe { w.dat().bits(b) }); + // SAFETY: dat field is 8bits long. All values are valid. + self.i2c.ic_data_cmd.write(|w| unsafe { w.dat().bits(b) }); sent += 1; } // serve a pending read request - self.i2c.i2c.ic_clr_rd_req.read(); + self.i2c.ic_clr_rd_req.read(); sent } /// Pull up to `usize::min(RX_FIFO_SIZE, buf.len())` bytes from the RX FIFO. pub fn read(&mut self, buf: &mut [u8]) -> usize { - let mut read = 0; + buf.iter_mut().zip(self).map(|(b, r)| *b = r).count() + } +} - for b in buf.iter_mut() { - if self.i2c.rx_fifo_empty() { - break; - } +/// This allows I2C to be used with `core::iter::Extend`. +impl, PINS> Iterator for I2C { + type Item = u8; - *b = self.i2c.i2c.ic_data_cmd.read().dat().bits(); - read += 1; + fn next(&mut self) -> Option { + if self.rx_fifo_empty() { + None + } else { + Some(self.i2c.ic_data_cmd.read().dat().bits()) } - read } } -impl, PINS> Iterator for I2CPeripheralEventIterator { - type Item = I2CEvent; +impl, PINS> I2C { + /// Returns the next i2c event if any. + pub fn next_event(&mut self) -> Option { + let stat = self.i2c.ic_raw_intr_stat.read(); - fn next(&mut self) -> Option { - let stat = self.i2c.i2c.ic_raw_intr_stat.read(); - self.i2c.i2c.ic_clr_activity.read(); - - match self.state { + match self.mode.state { State::Idle if stat.start_det().bit_is_set() => { - self.i2c.i2c.ic_clr_start_det.read(); - self.state = State::Active; - Some(I2CEvent::Start) - } - State::Active if !self.i2c.rx_fifo_empty() => { - self.state = State::Write; - Some(I2CEvent::TransferWrite) + self.i2c.ic_clr_start_det.read(); + self.mode.state = State::Active; + Some(Event::Start) } State::Active if stat.rd_req().bit_is_set() => { // Clearing `rd_req` is used by the hardware to detect when the I2C block can stop // stretching the clock and start process the data pushed to the FIFO (if any). // This is done in `Self::write`. - self.state = State::Read; - Some(I2CEvent::TransferRead) + self.mode.state = State::Read; + // If stop_det is set at this point we know that it is related to a previous request, + // It cannot be due the end of the current request as SCL is held low while waiting + // for user input. + if stat.stop_det().bit_is_set() { + self.i2c.ic_clr_stop_det.read(); + } + Some(Event::TransferRead) } - State::Read if stat.rd_req().bit_is_set() => Some(I2CEvent::TransferRead), - State::Read if stat.restart_det().bit_is_set() => { - self.i2c.i2c.ic_clr_restart_det.read(); - self.state = State::Active; - Some(I2CEvent::Restart) + State::Active if !self.rx_fifo_empty() => { + self.mode.state = State::Write; + Some(Event::TransferWrite) } - State::Write if !self.i2c.rx_fifo_empty() => Some(I2CEvent::TransferWrite), - State::Write if stat.restart_det().bit_is_set() => { - self.i2c.i2c.ic_clr_restart_det.read(); - self.state = State::Active; - Some(I2CEvent::Restart) + + State::Read if stat.rd_req().bit_is_set() => Some(Event::TransferRead), + State::Write if !self.rx_fifo_empty() => Some(Event::TransferWrite), + + State::Read | State::Write if stat.restart_det().bit_is_set() => { + self.i2c.ic_clr_restart_det.read(); + self.i2c.ic_clr_start_det.read(); + self.mode.state = State::Active; + Some(Event::Restart) } + _ if stat.stop_det().bit_is_set() => { - self.i2c.i2c.ic_clr_stop_det.read(); - self.state = State::Idle; - Some(I2CEvent::Stop) + self.i2c.ic_clr_stop_det.read(); + self.i2c.ic_clr_tx_abrt.read(); + self.mode.state = State::Idle; + Some(Event::Stop) } + _ => None, } } } - -impl I2CPeripheralEventIterator -where - Block: SubsystemReset + Deref, -{ - /// Releases the I2C peripheral and associated pins - #[allow(clippy::type_complexity)] - pub fn free(self, resets: &mut RESETS) -> (Block, (Sda, Scl)) { - self.i2c.free(resets) - } -}