Block at end of LPI2C embedded-hal transactions

Wait for the controller to become idle before returning to the
transaction caller. This is a stronger guarantee than waiting until the
stop bit is sent, and it meets the EH1 interface requirements.

This commit still keeps the busy check at the start of the transactions.
If the bus is busy due to another controller's activity, I don't think
we should block the caller while the other controller uses the bus. The
caller can implement a retry policy given the characteristics of their
system. EH1 lets users handle this condition with a dedicated error
kind.

Additionally, this commit keeps the FIFO flushes at the start of
transactions, after the busy checks. This is intended to avoid surprises
in case a user inserts a high-level transaction somewhere in their
low-level command sequence.

I updated one of the I2C examples to blast a device through the eh02
interfaces. It behaved reasonably on a Teensy 4 setup.

CHANGELOG.md

@@ -5,6 +5,9 @@
 - Add LPSPI low-level clock configuration APIs.
 - Add LPSPI `set_peripheral_enable` to configure the driver as a SPI peripheral.
 
+When the LPI2C driver concludes a transaction, it ensures that the controller
+is idle before returning.
+
 ## [0.5.5] 2024-05-27
 
 Add embedded-hal 1 implementations for the following drivers:

examples/hal_i2c_mpu9250.rs

@@ -3,22 +3,22 @@
 //! Requires an MPU9250. The board queries the sensor's WHO_AM_I
 //! register using various types of I2C write-read sequences. The
 //! clock should be 400KHz.
+//!
+//! The example tries to pipeline LPI2C transactions. Success is indicated
+//! by not panicking and constant activity on your scope. To understand loop
+//! start and end points, probe your LED / another output on your scope;
+//! the loop flips the output every iteration.
 
 #![no_main]
 #![no_std]
 
-use imxrt_hal as hal;
-
-use eh02::{blocking::i2c, blocking::serial::Write as _};
+use eh02::blocking::i2c;
 
 /// MPU9250 I2C address
 const MPU9250_ADDRESS: u8 = 0x68;
 const WHO_AM_I_REG: u8 = 0x75;
 const WHO_AM_I_RESP: u8 = 0x71;
 
-const GPT1_DELAY_MS: u32 = board::GPT1_FREQUENCY / 1_000 * 500;
-const GPT1_OCR: hal::gpt::OutputCompareRegister = hal::gpt::OutputCompareRegister::OCR1;
-
 /// This should show a repeated start.
 fn who_am_i_write_read<I>(i2c: &mut I) -> Result<bool, I::Error>
 where
@@ -68,76 +68,30 @@ where
     Ok(out[0] == WHO_AM_I_RESP)
 }
 
-fn write_error(console: &mut board::Console, _: hal::lpi2c::ControllerStatus) {
-    // TODO more helpful error reporting...
-    console.bwrite_all(b"I2C error\r\n").ok();
-}
-
-fn query_mpu(
-    ctx: &[u8],
-    console: &mut board::Console,
-    func: impl FnOnce() -> Result<bool, hal::lpi2c::ControllerStatus>,
-) {
-    console.bwrite_all(ctx).ok();
-    match func() {
-        Ok(true) => {
-            console.bwrite_all(b"OK\r\n").ok();
-        }
-        Ok(false) => {
-            console.bwrite_all(b"Wrong response\r\n").ok();
-        }
-        Err(err) => {
-            write_error(console, err);
-        }
-    }
-}
-
 #[imxrt_rt::entry]
 fn main() -> ! {
     let (
-        board::Common { mut gpt1, .. },
-        board::Specifics {
-            led,
-            mut console,
-            mut i2c,
+        board::Common {
+            pit: (mut pit, _, _, _),
             ..
         },
+        board::Specifics { led, mut i2c, .. },
     ) = board::new();
 
-    gpt1.set_output_compare_count(GPT1_OCR, GPT1_DELAY_MS);
-    gpt1.set_mode(hal::gpt::Mode::Restart);
-    gpt1.enable();
+    // Delay for scope set up / pin settle time.
+    pit.set_load_timer_value(board::PIT_FREQUENCY);
+    pit.enable();
+    while !pit.is_elapsed() {}
+    pit.clear_elapsed();
+    pit.disable();
 
     led.set();
     loop {
-        while !gpt1.is_elapsed(GPT1_OCR) {}
-        gpt1.clear_elapsed(GPT1_OCR);
-
-        query_mpu(
-            b"Querying WHO_AM_I with write-read... ",
-            &mut console,
-            || who_am_i_write_read(&mut i2c),
-        );
-
-        query_mpu(
-            b"Querying WHO_AM_I with write-then-read... ",
-            &mut console,
-            || who_am_i_write_then_read(&mut i2c),
-        );
-
-        query_mpu(
-            b"Querying WHO_AM_I with transactional write-read... ",
-            &mut console,
-            || who_am_i_transaction_write_read(&mut i2c),
-        );
-
-        query_mpu(
-            b"Querying WHO_AM_I with transactional write-then-read... ",
-            &mut console,
-            || who_am_i_transaction_write_then_read(&mut i2c),
-        );
+        assert!(who_am_i_write_read(&mut i2c).unwrap());
+        assert!(who_am_i_write_then_read(&mut i2c).unwrap());
+        assert!(who_am_i_transaction_write_read(&mut i2c).unwrap());
+        assert!(who_am_i_transaction_write_then_read(&mut i2c).unwrap());
 
         led.toggle();
-        console.bwrite_all(b"\r\n\r\n").ok();
     }
 }

src/common/lpi2c.rs

@@ -229,7 +229,6 @@ impl<P, const N: u8> Lpi2c<P, N> {
         let mut disabled = Disabled::new(&mut self.lpi2c);
         func(&mut disabled)
     }
-
     /// If the bus is busy, return the status flags in the error
     /// position.
     fn check_busy(&self) -> Result<(), ControllerStatus> {
@@ -241,32 +240,39 @@ impl<P, const N: u8> Lpi2c<P, N> {
         }
     }
 
-    /// Block until there's space in the transmit FIFO..
-    ///
-    /// Return errors if detected.
-    fn wait_for_transmit(&self) -> Result<(), ControllerStatus> {
+    /// Keep checking for errors until `what` produces a value.
+    fn wait_for<T>(
+        &self,
+        what: impl Fn(ControllerStatus) -> Option<T>,
+    ) -> Result<T, ControllerStatus> {
         loop {
             let status = self.controller_status();
             if status.has_error() {
                 return Err(status);
-            } else if status.contains(ControllerStatus::TRANSMIT_DATA) {
-                return Ok(());
+            }
+            if let Some(val) = what(status) {
+                return Ok(val);
             }
         }
     }
 
+    /// Wait for the controller to become idle.
+    fn wait_for_controller_idle(&self) -> Result<(), ControllerStatus> {
+        self.wait_for(ControllerStatus::break_controller_idle)
+    }
+
+    /// Block until there's space in the transmit FIFO..
+    ///
+    /// Return errors if detected.
+    fn wait_for_transmit(&self) -> Result<(), ControllerStatus> {
+        self.wait_for(ControllerStatus::break_transmit_space)
+    }
+
     /// Block until receiving a byte of data.
     ///
     /// Returns errors if detected.
     fn wait_for_data(&self) -> Result<u8, ControllerStatus> {
-        loop {
-            let status = self.controller_status();
-            if status.has_error() {
-                return Err(status);
-            } else if let Some(data) = self.read_data_register() {
-                return Ok(data);
-            }
-        }
+        self.wait_for(|_| self.read_data_register())
     }
 
     /// Wait for the end of packet, which happens for STOP or repeated
@@ -275,14 +281,7 @@ impl<P, const N: u8> Lpi2c<P, N> {
     /// Returns errors if detected. This will not unblock for a non-repeated
     /// START.
     fn wait_for_end_of_packet(&self) -> Result<(), ControllerStatus> {
-        loop {
-            let status = self.controller_status();
-            if status.has_error() {
-                return Err(status);
-            } else if status.contains(ControllerStatus::END_PACKET) {
-                return Ok(());
-            }
-        }
+        self.wait_for(ControllerStatus::break_end_of_packet)
     }
 
     /// Borrow the pins.
@@ -487,7 +486,22 @@ impl ControllerStatus {
         self.contains(Self::BUS_BUSY) && !self.contains(Self::CONTROLLER_BUSY)
     }
 
-    /// Indicates if this tatus has any error bits set.
+    /// Break when the controller is idle.
+    fn break_controller_idle(self) -> Option<()> {
+        (!self.contains(Self::CONTROLLER_BUSY)).then_some(())
+    }
+
+    /// Break when there's an end of packet.
+    fn break_end_of_packet(self) -> Option<()> {
+        self.contains(Self::END_PACKET).then_some(())
+    }
+
+    /// Break when there's space in the TX FIFO.
+    fn break_transmit_space(self) -> Option<()> {
+        self.contains(Self::TRANSMIT_DATA).then_some(())
+    }
+
+    /// Indicates if any error bit is set.
     const fn has_error(self) -> bool {
         self.intersects(Self::ERRORS)
     }
@@ -664,7 +678,6 @@ impl<P, const N: u8> blocking::WriteIterRead for Lpi2c<P, N> {
         self.check_busy()?;
         self.clear_fifo();
         self.clear_controller_status(ControllerStatus::W1C);
-
         self.wait_for_transmit()?;
         self.enqueue_controller_command(ControllerCommand::write(address));
 
@@ -693,6 +706,7 @@ impl<P, const N: u8> blocking::WriteIterRead for Lpi2c<P, N> {
         self.wait_for_transmit()?;
         self.enqueue_controller_command(ControllerCommand::Stop);
         self.wait_for_end_of_packet()?;
+        self.wait_for_controller_idle()?;
 
         Ok(())
     }
@@ -882,6 +896,7 @@ mod transaction {
             self.lpi2c
                 .enqueue_controller_command(ControllerCommand::Stop);
             self.lpi2c.wait_for_end_of_packet()?;
+            self.lpi2c.wait_for_controller_idle()?;
             Ok(())
         }
     }