I2c slave take 2

refactored to split modules
renamed to match upstream docs
slight improvement to slave error handling

embassy-rp/src/i2c/i2c.rs

@@ -11,7 +11,38 @@ use super::{
 use crate::gpio::sealed::Pin;
 use crate::gpio::AnyPin;
 use crate::interrupt::typelevel::{Binding, Interrupt};
-use crate::{pac, Peripheral};
+use crate::{interrupt, pac, peripherals, Peripheral};
+
+/// I2C error abort reason
+#[derive(Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub enum AbortReason {
+    /// A bus operation was not acknowledged, e.g. due to the addressed device
+    /// not being available on the bus or the device not being ready to process
+    /// requests at the moment
+    NoAcknowledge,
+    /// The arbitration was lost, e.g. electrical problems with the clock signal
+    ArbitrationLoss,
+    /// Transmit ended with data still in fifo
+    TxNotEmpty(u16),
+    Other(u32),
+}
+
+/// I2C error
+#[derive(Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub enum Error {
+    /// I2C abort with error
+    Abort(AbortReason),
+    /// User passed in a read buffer that was 0 length
+    InvalidReadBufferLength,
+    /// User passed in a write buffer that was 0 length
+    InvalidWriteBufferLength,
+    /// Target i2c address is out of range
+    AddressOutOfRange(u16),
+    /// Target i2c address is reserved
+    AddressReserved(u16),
+}
 
 #[non_exhaustive]
 #[derive(Copy, Clone)]
@@ -26,6 +57,8 @@ impl Default for Config {
     }
 }
 
+pub const FIFO_SIZE: u8 = 16;
+
 pub struct I2c<'d, T: Instance, M: Mode> {
     phantom: PhantomData<(&'d mut T, M)>,
 }
@@ -696,3 +729,123 @@ mod nightly {
         }
     }
 }
+<<<<<<< HEAD:embassy-rp/src/i2c/i2c.rs
+=======
+
+pub fn i2c_reserved_addr(addr: u16) -> bool {
+    ((addr & 0x78) == 0 || (addr & 0x78) == 0x78) && addr != 0
+}
+
+mod sealed {
+    use embassy_sync::waitqueue::AtomicWaker;
+
+    use crate::interrupt;
+
+    pub trait Instance {
+        const TX_DREQ: u8;
+        const RX_DREQ: u8;
+
+        type Interrupt: interrupt::typelevel::Interrupt;
+
+        fn regs() -> crate::pac::i2c::I2c;
+        fn reset() -> crate::pac::resets::regs::Peripherals;
+        fn waker() -> &'static AtomicWaker;
+    }
+
+    pub trait Mode {}
+
+    pub trait SdaPin<T: Instance> {}
+    pub trait SclPin<T: Instance> {}
+}
+
+pub trait Mode: sealed::Mode {}
+
+macro_rules! impl_mode {
+    ($name:ident) => {
+        impl sealed::Mode for $name {}
+        impl Mode for $name {}
+    };
+}
+
+pub struct Blocking;
+pub struct Async;
+
+impl_mode!(Blocking);
+impl_mode!(Async);
+
+pub trait Instance: sealed::Instance {}
+
+macro_rules! impl_instance {
+    ($type:ident, $irq:ident, $reset:ident, $tx_dreq:expr, $rx_dreq:expr) => {
+        impl sealed::Instance for peripherals::$type {
+            const TX_DREQ: u8 = $tx_dreq;
+            const RX_DREQ: u8 = $rx_dreq;
+
+            type Interrupt = crate::interrupt::typelevel::$irq;
+
+            #[inline]
+            fn regs() -> pac::i2c::I2c {
+                pac::$type
+            }
+
+            #[inline]
+            fn reset() -> pac::resets::regs::Peripherals {
+                let mut ret = pac::resets::regs::Peripherals::default();
+                ret.$reset(true);
+                ret
+            }
+
+            #[inline]
+            fn waker() -> &'static AtomicWaker {
+                static WAKER: AtomicWaker = AtomicWaker::new();
+
+                &WAKER
+            }
+        }
+        impl Instance for peripherals::$type {}
+    };
+}
+
+impl_instance!(I2C0, I2C0_IRQ, set_i2c0, 32, 33);
+impl_instance!(I2C1, I2C1_IRQ, set_i2c1, 34, 35);
+
+pub trait SdaPin<T: Instance>: sealed::SdaPin<T> + crate::gpio::Pin {}
+pub trait SclPin<T: Instance>: sealed::SclPin<T> + crate::gpio::Pin {}
+
+macro_rules! impl_pin {
+    ($pin:ident, $instance:ident, $function:ident) => {
+        impl sealed::$function<peripherals::$instance> for peripherals::$pin {}
+        impl $function<peripherals::$instance> for peripherals::$pin {}
+    };
+}
+
+impl_pin!(PIN_0, I2C0, SdaPin);
+impl_pin!(PIN_1, I2C0, SclPin);
+impl_pin!(PIN_2, I2C1, SdaPin);
+impl_pin!(PIN_3, I2C1, SclPin);
+impl_pin!(PIN_4, I2C0, SdaPin);
+impl_pin!(PIN_5, I2C0, SclPin);
+impl_pin!(PIN_6, I2C1, SdaPin);
+impl_pin!(PIN_7, I2C1, SclPin);
+impl_pin!(PIN_8, I2C0, SdaPin);
+impl_pin!(PIN_9, I2C0, SclPin);
+impl_pin!(PIN_10, I2C1, SdaPin);
+impl_pin!(PIN_11, I2C1, SclPin);
+impl_pin!(PIN_12, I2C0, SdaPin);
+impl_pin!(PIN_13, I2C0, SclPin);
+impl_pin!(PIN_14, I2C1, SdaPin);
+impl_pin!(PIN_15, I2C1, SclPin);
+impl_pin!(PIN_16, I2C0, SdaPin);
+impl_pin!(PIN_17, I2C0, SclPin);
+impl_pin!(PIN_18, I2C1, SdaPin);
+impl_pin!(PIN_19, I2C1, SclPin);
+impl_pin!(PIN_20, I2C0, SdaPin);
+impl_pin!(PIN_21, I2C0, SclPin);
+impl_pin!(PIN_22, I2C1, SdaPin);
+impl_pin!(PIN_23, I2C1, SclPin);
+impl_pin!(PIN_24, I2C0, SdaPin);
+impl_pin!(PIN_25, I2C0, SclPin);
+impl_pin!(PIN_26, I2C1, SdaPin);
+impl_pin!(PIN_27, I2C1, SclPin);
+impl_pin!(PIN_28, I2C0, SdaPin);
+impl_pin!(PIN_29, I2C0, SclPin);

embassy-rp/src/i2c_slave.rs

@@ -0,0 +1,326 @@
+use core::future;
+use core::marker::PhantomData;
+use core::task::Poll;
+
+use embassy_hal_internal::into_ref;
+use pac::i2c;
+
+use crate::i2c::{i2c_reserved_addr, AbortReason, Instance, InterruptHandler, SclPin, SdaPin, FIFO_SIZE};
+use crate::interrupt::typelevel::{Binding, Interrupt};
+use crate::{pac, Peripheral};
+
+/// I2C error
+#[derive(Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+#[non_exhaustive]
+pub enum Error {
+    /// I2C abort with error
+    Abort(AbortReason),
+    /// User passed in a response buffer that was 0 length
+    InvalidResponseBufferLength,
+}
+
+/// Received command
+#[derive(Debug, Copy, Clone, Eq, PartialEq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub enum Command {
+    /// General Call
+    GeneralCall(usize),
+    /// Read
+    Read,
+    /// Write+read
+    WriteRead(usize),
+    /// Write
+    Write(usize),
+}
+
+/// Possible responses to responding to a read
+#[derive(Debug, Copy, Clone, Eq, PartialEq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub enum ReadStatus {
+    /// Transaction Complete, controller naked our last byte
+    Done,
+    /// Transaction Incomplete, controller trying to read more bytes than were provided
+    NeedMoreBytes,
+    /// Transaction Complere, but controller stopped reading bytes before we ran out
+    LeftoverBytes(u16),
+}
+
+/// Slave Configuration
+#[non_exhaustive]
+#[derive(Copy, Clone)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub struct Config {
+    /// Target Address
+    pub addr: u16,
+}
+
+impl Default for Config {
+    fn default() -> Self {
+        Self { addr: 0x55 }
+    }
+}
+
+pub struct I2cSlave<'d, T: Instance> {
+    phantom: PhantomData<&'d mut T>,
+}
+
+impl<'d, T: Instance> I2cSlave<'d, T> {
+    pub fn new(
+        _peri: impl Peripheral<P = T> + 'd,
+        scl: impl Peripheral<P = impl SclPin<T>> + 'd,
+        sda: impl Peripheral<P = impl SdaPin<T>> + 'd,
+        _irq: impl Binding<T::Interrupt, InterruptHandler<T>>,
+        config: Config,
+    ) -> Self {
+        into_ref!(_peri, scl, sda);
+
+        assert!(!i2c_reserved_addr(config.addr));
+        assert!(config.addr != 0);
+
+        let p = T::regs();
+
+        let reset = T::reset();
+        crate::reset::reset(reset);
+        crate::reset::unreset_wait(reset);
+
+        p.ic_enable().write(|w| w.set_enable(false));
+
+        p.ic_sar().write(|w| w.set_ic_sar(config.addr));
+        p.ic_con().modify(|w| {
+            w.set_master_mode(false);
+            w.set_ic_slave_disable(false);
+            w.set_tx_empty_ctrl(true);
+        });
+
+        // Set FIFO watermarks to 1 to make things simpler. This is encoded
+        // by a register value of 0. Rx watermark should never change, but Tx watermark will be
+        // adjusted in operation.
+        p.ic_tx_tl().write(|w| w.set_tx_tl(0));
+        p.ic_rx_tl().write(|w| w.set_rx_tl(0));
+
+        // Configure SCL & SDA pins
+        scl.gpio().ctrl().write(|w| w.set_funcsel(3));
+        sda.gpio().ctrl().write(|w| w.set_funcsel(3));
+
+        scl.pad_ctrl().write(|w| {
+            w.set_schmitt(true);
+            w.set_ie(true);
+            w.set_od(false);
+            w.set_pue(true);
+            w.set_pde(false);
+        });
+        sda.pad_ctrl().write(|w| {
+            w.set_schmitt(true);
+            w.set_ie(true);
+            w.set_od(false);
+            w.set_pue(true);
+            w.set_pde(false);
+        });
+
+        // Clear interrupts
+        p.ic_clr_intr().read();
+
+        // Enable I2C block
+        p.ic_enable().write(|w| w.set_enable(true));
+
+        // mask everything initially
+        p.ic_intr_mask().write_value(i2c::regs::IcIntrMask(0));
+        T::Interrupt::unpend();
+        unsafe { T::Interrupt::enable() };
+
+        Self { phantom: PhantomData }
+    }
+
+    /// Calls `f` to check if we are ready or not.
+    /// If not, `g` is called once the waker is set (to eg enable the required interrupts).
+    #[inline(always)]
+    async fn wait_on<F, U, G>(&mut self, mut f: F, mut g: G) -> U
+    where
+        F: FnMut(&mut Self) -> Poll<U>,
+        G: FnMut(&mut Self),
+    {
+        future::poll_fn(|cx| {
+            let r = f(self);
+
+            trace!("intr p: {:013b}", T::regs().ic_raw_intr_stat().read().0);
+
+            if r.is_pending() {
+                T::waker().register(cx.waker());
+                g(self);
+            }
+
+            r
+        })
+        .await
+    }
+
+    #[inline(always)]
+    fn drain_fifo(&mut self, buffer: &mut [u8], offset: usize) -> usize {
+        let p = T::regs();
+        let len = p.ic_rxflr().read().rxflr() as usize;
+        let end = offset + len;
+        for i in offset..end {
+            buffer[i] = p.ic_data_cmd().read().dat();
+        }
+        end
+    }
+
+    #[inline(always)]
+    fn write_to_fifo(&mut self, buffer: &[u8]) {
+        let p = T::regs();
+        for byte in buffer {
+            p.ic_data_cmd().write(|w| w.set_dat(*byte));
+        }
+    }
+
+    /// Wait asynchronously for commands from an I2C master.
+    /// `buffer` is provided in case master does a 'write' and is unused for 'read'.
+    pub async fn listen(&mut self, buffer: &mut [u8]) -> Result<Command, Error> {
+        let p = T::regs();
+
+        p.ic_clr_intr().read();
+        // set rx fifo watermark to 1 byte
+        p.ic_rx_tl().write(|w| w.set_rx_tl(0));
+
+        let mut len = 0;
+        let ret = self
+            .wait_on(
+                |me| {
+                    let stat = p.ic_raw_intr_stat().read();
+                    if p.ic_rxflr().read().rxflr() > 0 {
+                        len = me.drain_fifo(buffer, len);
+                        // we're recieving data, set rx fifo watermark to 12 bytes to reduce interrupt noise
+                        p.ic_rx_tl().write(|w| w.set_rx_tl(11));
+                    }
+
+                    if stat.restart_det() && stat.rd_req() {
+                        Poll::Ready(Ok(Command::WriteRead(len)))
+                    } else if stat.gen_call() && stat.stop_det() && len > 0 {
+                        Poll::Ready(Ok(Command::GeneralCall(len)))
+                    } else if stat.stop_det() {
+                        Poll::Ready(Ok(Command::Write(len)))
+                    } else if stat.rd_req() {
+                        Poll::Ready(Ok(Command::Read))
+                    } else {
+                        Poll::Pending
+                    }
+                },
+                |_me| {
+                    p.ic_intr_mask().modify(|w| {
+                        w.set_m_stop_det(true);
+                        w.set_m_restart_det(true);
+                        w.set_m_gen_call(true);
+                        w.set_m_rd_req(true);
+                        w.set_m_rx_full(true);
+                    });
+                },
+            )
+            .await;
+
+        p.ic_clr_intr().read();
+
+        ret
+    }
+
+    /// Respond to an I2C master READ command, asynchronously.
+    pub async fn respond_to_read(&mut self, buffer: &[u8]) -> Result<ReadStatus, Error> {
+        let p = T::regs();
+
+        if buffer.len() == 0 {
+            return Err(Error::InvalidResponseBufferLength);
+        }
+
+        let mut chunks = buffer.chunks(FIFO_SIZE as usize);
+
+        let ret = self
+            .wait_on(
+                |me| {
+                    if let Err(abort_reason) = me.read_and_clear_abort_reason() {
+                        if let Error::Abort(AbortReason::TxNotEmpty(bytes)) = abort_reason {
+                            return Poll::Ready(Ok(ReadStatus::LeftoverBytes(bytes)));
+                        } else {
+                            return Poll::Ready(Err(abort_reason));
+                        }
+                    }
+
+                    if let Some(chunk) = chunks.next() {
+                        me.write_to_fifo(chunk);
+
+                        Poll::Pending
+                    } else {
+                        let stat = p.ic_raw_intr_stat().read();
+
+                        if stat.rx_done() && stat.stop_det() {
+                            Poll::Ready(Ok(ReadStatus::Done))
+                        } else if stat.rd_req() {
+                            Poll::Ready(Ok(ReadStatus::NeedMoreBytes))
+                        } else {
+                            Poll::Pending
+                        }
+                    }
+                },
+                |_me| {
+                    p.ic_intr_mask().modify(|w| {
+                        w.set_m_stop_det(true);
+                        w.set_m_rx_done(true);
+                        w.set_m_tx_empty(true);
+                        w.set_m_tx_abrt(true);
+                    })
+                },
+            )
+            .await;
+
+        p.ic_clr_intr().read();
+
+        ret
+    }
+
+    /// Respond to reads with the fill byte until the controller stops asking
+    pub async fn respond_till_stop(&mut self, fill: u8) -> Result<(), Error> {
+        loop {
+            match self.respond_to_read(&[fill]).await {
+                Ok(ReadStatus::NeedMoreBytes) => (),
+                Ok(_) => break Ok(()),
+                Err(e) => break Err(e),
+            }
+        }
+    }
+
+    #[inline(always)]
+    fn read_and_clear_abort_reason(&mut self) -> Result<(), Error> {
+        let p = T::regs();
+        let mut abort_reason = p.ic_tx_abrt_source().read();
+
+        // Mask off fifo flush count
+        let tx_flush_cnt = abort_reason.tx_flush_cnt();
+        abort_reason.set_tx_flush_cnt(0);
+
+        // Mask off master_dis
+        abort_reason.set_abrt_master_dis(false);
+
+        if abort_reason.0 != 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.
+            p.ic_clr_tx_abrt().read();
+
+            let reason = if abort_reason.abrt_7b_addr_noack()
+                | abort_reason.abrt_10addr1_noack()
+                | abort_reason.abrt_10addr2_noack()
+            {
+                AbortReason::NoAcknowledge
+            } else if abort_reason.arb_lost() {
+                AbortReason::ArbitrationLoss
+            } else if abort_reason.abrt_slvflush_txfifo() {
+                AbortReason::TxNotEmpty(tx_flush_cnt)
+            } else {
+                AbortReason::Other(abort_reason.0)
+            };
+
+            Err(Error::Abort(reason))
+        } else {
+            Ok(())
+        }
+    }
+}

embassy-rp/src/lib.rs

@@ -16,6 +16,7 @@ pub mod flash;
 mod float;
 pub mod gpio;
 pub mod i2c;
+pub mod i2c_slave;
 pub mod multicore;
 pub mod pwm;
 mod reset;

tests/rp/src/bin/i2c.rs

@@ -5,10 +5,9 @@
 use defmt::{assert_eq, info, panic, unwrap};
 use embassy_executor::Executor;
 use embassy_executor::_export::StaticCell;
-use embassy_rp::bind_interrupts;
-use embassy_rp::i2c::{self, Async, InterruptHandler};
 use embassy_rp::multicore::{spawn_core1, Stack};
 use embassy_rp::peripherals::{I2C0, I2C1};
+use embassy_rp::{bind_interrupts, i2c, i2c_slave};
 use embedded_hal_1::i2c::Operation;
 use embedded_hal_async::i2c::I2c;
 use {defmt_rtt as _, panic_probe as _, panic_probe as _, panic_probe as _};
@@ -20,14 +19,14 @@ static EXECUTOR1: StaticCell<Executor> = StaticCell::new();
 use crate::i2c::AbortReason;
 
 bind_interrupts!(struct Irqs {
-    I2C0_IRQ => InterruptHandler<I2C0>;
+    I2C0_IRQ => i2c::InterruptHandler<I2C0>;
-    I2C1_IRQ => InterruptHandler<I2C1>;
+    I2C1_IRQ => i2c::InterruptHandler<I2C1>;
 });
 
 const DEV_ADDR: u8 = 0x42;
 
 #[embassy_executor::task]
-async fn device_task(mut dev: i2c::I2cDevice<'static, I2C1>) -> ! {
+async fn device_task(mut dev: i2c_slave::I2cSlave<'static, I2C1>) -> ! {
     info!("Device start");
 
     let mut count = 0xD0;
@@ -35,33 +34,33 @@ async fn device_task(mut dev: i2c::I2cDevice<'static, I2C1>) -> ! {
     loop {
         let mut buf = [0u8; 128];
         match dev.listen(&mut buf).await {
-            Ok(i2c::Command::GeneralCall(len)) => {
+            Ok(i2c_slave::Command::GeneralCall(len)) => {
                 assert_eq!(buf[..len], [0xCA, 0x11], "recieving the general call failed");
                 info!("General Call - OK");
             }
-            Ok(i2c::Command::Read) => {
+            Ok(i2c_slave::Command::Read) => {
                 loop {
                     //info!("Responding to read, count {}", count);
                     let a = dev.respond_to_read(&[count]).await;
                     //info!("x {}", a);
                     match a {
                         Ok(x) => match x {
-                            i2c::ReadStatus::Done => break,
+                            i2c_slave::ReadStatus::Done => break,
-                            i2c::ReadStatus::NeedMoreBytes => count += 1,
+                            i2c_slave::ReadStatus::NeedMoreBytes => count += 1,
-                            i2c::ReadStatus::LeftoverBytes(x) => {
+                            i2c_slave::ReadStatus::LeftoverBytes(x) => {
                                 info!("tried to write {} extra bytes", x);
                                 break;
                             }
                         },
                         Err(e) => match e {
-                            embassy_rp::i2c::Error::Abort(AbortReason::Other(n)) => panic!("Other {:b}", n),
+                            embassy_rp::i2c_slave::Error::Abort(AbortReason::Other(n)) => panic!("Other {:b}", n),
                             _ => panic!("{}", e),
                         },
                     }
                 }
                 count += 1;
             }
-            Ok(i2c::Command::Write(len)) => match len {
+            Ok(i2c_slave::Command::Write(len)) => match len {
                 1 => {
                     assert_eq!(buf[..len], [0xAA], "recieving a single byte failed");
                     info!("Single Byte Write - OK")
@@ -83,7 +82,7 @@ async fn device_task(mut dev: i2c::I2cDevice<'static, I2C1>) -> ! {
                 }
                 _ => panic!("Invalid write length {}", len),
             },
-            Ok(i2c::Command::WriteRead(len)) => {
+            Ok(i2c_slave::Command::WriteRead(len)) => {
                 info!("device recieved write read: {:x}", buf[..len]);
                 match buf[0] {
                     0xC2 => {
@@ -101,7 +100,7 @@ async fn device_task(mut dev: i2c::I2cDevice<'static, I2C1>) -> ! {
                 }
             }
             Err(e) => match e {
-                embassy_rp::i2c::Error::Abort(AbortReason::Other(n)) => panic!("Other {:b}", n),
+                embassy_rp::i2c_slave::Error::Abort(AbortReason::Other(n)) => panic!("Other {:b}", n),
                 _ => panic!("{}", e),
             },
         }
@@ -109,7 +108,7 @@ async fn device_task(mut dev: i2c::I2cDevice<'static, I2C1>) -> ! {
 }
 
 #[embassy_executor::task]
-async fn controller_task(mut con: i2c::I2c<'static, I2C0, Async>) {
+async fn controller_task(mut con: i2c::I2c<'static, I2C0, i2c::Async>) {
     info!("Device start");
 
     {
@@ -194,9 +193,9 @@ fn main() -> ! {
 
     let d_sda = p.PIN_19;
     let d_scl = p.PIN_18;
-    let mut config = i2c::DeviceConfig::default();
+    let mut config = i2c_slave::Config::default();
     config.addr = DEV_ADDR as u16;
-    let device = i2c::I2cDevice::new(p.I2C1, d_sda, d_scl, Irqs, config);
+    let device = i2c_slave::I2cSlave::new(p.I2C1, d_sda, d_scl, Irqs, config);
 
     spawn_core1(p.CORE1, unsafe { &mut CORE1_STACK }, move || {
         let executor1 = EXECUTOR1.init(Executor::new());