Merge pull request #2742 from sgoll/i2c-async-transaction

stm32/i2c(v1): Implement asynchronous transactions

embassy-stm32/src/i2c/mod.rs

@@ -6,6 +6,7 @@
 mod _version;
 
 use core::future::Future;
+use core::iter;
 use core::marker::PhantomData;
 
 use embassy_hal_internal::{into_ref, Peripheral, PeripheralRef};
@@ -332,8 +333,142 @@ impl<'d, T: Instance, TXDMA: TxDma<T>, RXDMA: RxDma<T>> embedded_hal_async::i2c:
         address: u8,
         operations: &mut [embedded_hal_1::i2c::Operation<'_>],
     ) -> Result<(), Self::Error> {
-        let _ = address;
+        self.transaction(address, operations).await
-        let _ = operations;
-        todo!()
     }
 }
+
+/// Frame type in I2C transaction.
+///
+/// This tells each method what kind of framing to use, to generate a (repeated) start condition (ST
+/// or SR), and/or a stop condition (SP). For read operations, this also controls whether to send an
+/// ACK or NACK after the last byte received.
+///
+/// For write operations, the following options are identical because they differ only in the (N)ACK
+/// treatment relevant for read operations:
+///
+/// - `FirstFrame` and `FirstAndNextFrame`
+/// - `NextFrame` and `LastFrameNoStop`
+///
+/// Abbreviations used below:
+///
+/// - `ST` = start condition
+/// - `SR` = repeated start condition
+/// - `SP` = stop condition
+/// - `ACK`/`NACK` = last byte in read operation
+#[derive(Copy, Clone)]
+#[allow(dead_code)]
+enum FrameOptions {
+    /// `[ST/SR]+[NACK]+[SP]` First frame (of this type) in transaction and also last frame overall.
+    FirstAndLastFrame,
+    /// `[ST/SR]+[NACK]` First frame of this type in transaction, last frame in a read operation but
+    /// not the last frame overall.
+    FirstFrame,
+    /// `[ST/SR]+[ACK]` First frame of this type in transaction, neither last frame overall nor last
+    /// frame in a read operation.
+    FirstAndNextFrame,
+    /// `[ACK]` Middle frame in a read operation (neither first nor last).
+    NextFrame,
+    /// `[NACK]+[SP]` Last frame overall in this transaction but not the first frame.
+    LastFrame,
+    /// `[NACK]` Last frame in a read operation but not last frame overall in this transaction.
+    LastFrameNoStop,
+}
+
+#[allow(dead_code)]
+impl FrameOptions {
+    /// Sends start or repeated start condition before transfer.
+    fn send_start(self) -> bool {
+        match self {
+            Self::FirstAndLastFrame | Self::FirstFrame | Self::FirstAndNextFrame => true,
+            Self::NextFrame | Self::LastFrame | Self::LastFrameNoStop => false,
+        }
+    }
+
+    /// Sends stop condition after transfer.
+    fn send_stop(self) -> bool {
+        match self {
+            Self::FirstAndLastFrame | Self::LastFrame => true,
+            Self::FirstFrame | Self::FirstAndNextFrame | Self::NextFrame | Self::LastFrameNoStop => false,
+        }
+    }
+
+    /// Sends NACK after last byte received, indicating end of read operation.
+    fn send_nack(self) -> bool {
+        match self {
+            Self::FirstAndLastFrame | Self::FirstFrame | Self::LastFrame | Self::LastFrameNoStop => true,
+            Self::FirstAndNextFrame | Self::NextFrame => false,
+        }
+    }
+}
+
+/// Iterates over operations in transaction.
+///
+/// Returns necessary frame options for each operation to uphold the [transaction contract] and have
+/// the right start/stop/(N)ACK conditions on the wire.
+///
+/// [transaction contract]: embedded_hal_1::i2c::I2c::transaction
+#[allow(dead_code)]
+fn operation_frames<'a, 'b: 'a>(
+    operations: &'a mut [embedded_hal_1::i2c::Operation<'b>],
+) -> Result<impl IntoIterator<Item = (&'a mut embedded_hal_1::i2c::Operation<'b>, FrameOptions)>, Error> {
+    use embedded_hal_1::i2c::Operation::{Read, Write};
+
+    // Check empty read buffer before starting transaction. Otherwise, we would risk halting with an
+    // error in the middle of the transaction.
+    //
+    // In principle, we could allow empty read frames within consecutive read operations, as long as
+    // at least one byte remains in the final (merged) read operation, but that makes the logic more
+    // complicated and error-prone.
+    if operations.iter().any(|op| match op {
+        Read(read) => read.is_empty(),
+        Write(_) => false,
+    }) {
+        return Err(Error::Overrun);
+    }
+
+    let mut operations = operations.iter_mut().peekable();
+
+    let mut next_first_frame = true;
+
+    Ok(iter::from_fn(move || {
+        let Some(op) = operations.next() else {
+            return None;
+        };
+
+        // Is `op` first frame of its type?
+        let first_frame = next_first_frame;
+        let next_op = operations.peek();
+
+        // Get appropriate frame options as combination of the following properties:
+        //
+        // - For each first operation of its type, generate a (repeated) start condition.
+        // - For the last operation overall in the entire transaction, generate a stop condition.
+        // - For read operations, check the next operation: if it is also a read operation, we merge
+        //   these and send ACK for all bytes in the current operation; send NACK only for the final
+        //   read operation's last byte (before write or end of entire transaction) to indicate last
+        //   byte read and release the bus for transmission of the bus master's next byte (or stop).
+        //
+        // We check the third property unconditionally, i.e. even for write opeartions. This is okay
+        // because the resulting frame options are identical for write operations.
+        let frame = match (first_frame, next_op) {
+            (true, None) => FrameOptions::FirstAndLastFrame,
+            (true, Some(Read(_))) => FrameOptions::FirstAndNextFrame,
+            (true, Some(Write(_))) => FrameOptions::FirstFrame,
+            //
+            (false, None) => FrameOptions::LastFrame,
+            (false, Some(Read(_))) => FrameOptions::NextFrame,
+            (false, Some(Write(_))) => FrameOptions::LastFrameNoStop,
+        };
+
+        // Pre-calculate if `next_op` is the first operation of its type. We do this here and not at
+        // the beginning of the loop because we hand out `op` as iterator value and cannot access it
+        // anymore in the next iteration.
+        next_first_frame = match (&op, next_op) {
+            (_, None) => false,
+            (Read(_), Some(Write(_))) | (Write(_), Some(Read(_))) => true,
+            (Read(_), Some(Read(_))) | (Write(_), Some(Write(_))) => false,
+        };
+
+        Some((op, frame))
+    }))
+}

embassy-stm32/src/i2c/v1.rs

@@ -41,68 +41,6 @@ pub unsafe fn on_interrupt<T: Instance>() {
     });
 }
 
-/// Frame type in I2C transaction.
-///
-/// This tells each method what kind of framing to use, to generate a (repeated) start condition (ST
-/// or SR), and/or a stop condition (SP). For read operations, this also controls whether to send an
-/// ACK or NACK after the last byte received.
-///
-/// For write operations, the following options are identical because they differ only in the (N)ACK
-/// treatment relevant for read operations:
-///
-/// - `FirstFrame` and `FirstAndNextFrame`
-/// - `NextFrame` and `LastFrameNoStop`
-///
-/// Abbreviations used below:
-///
-/// - `ST` = start condition
-/// - `SR` = repeated start condition
-/// - `SP` = stop condition
-#[derive(Copy, Clone)]
-enum FrameOptions {
-    /// `[ST/SR]+[NACK]+[SP]` First frame (of this type) in operation and last frame overall in this
-    /// transaction.
-    FirstAndLastFrame,
-    /// `[ST/SR]+[NACK]` First frame of this type in transaction, last frame in a read operation but
-    /// not the last frame overall.
-    FirstFrame,
-    /// `[ST/SR]+[ACK]` First frame of this type in transaction, neither last frame overall nor last
-    /// frame in a read operation.
-    FirstAndNextFrame,
-    /// `[ACK]` Middle frame in a read operation (neither first nor last).
-    NextFrame,
-    /// `[NACK]+[SP]` Last frame overall in this transaction but not the first frame.
-    LastFrame,
-    /// `[NACK]` Last frame in a read operation but not last frame overall in this transaction.
-    LastFrameNoStop,
-}
-
-impl FrameOptions {
-    /// Sends start or repeated start condition before transfer.
-    fn send_start(self) -> bool {
-        match self {
-            Self::FirstAndLastFrame | Self::FirstFrame | Self::FirstAndNextFrame => true,
-            Self::NextFrame | Self::LastFrame | Self::LastFrameNoStop => false,
-        }
-    }
-
-    /// Sends stop condition after transfer.
-    fn send_stop(self) -> bool {
-        match self {
-            Self::FirstAndLastFrame | Self::LastFrame => true,
-            Self::FirstFrame | Self::FirstAndNextFrame | Self::NextFrame | Self::LastFrameNoStop => false,
-        }
-    }
-
-    /// Sends NACK after last byte received, indicating end of read operation.
-    fn send_nack(self) -> bool {
-        match self {
-            Self::FirstAndLastFrame | Self::FirstFrame | Self::LastFrame | Self::LastFrameNoStop => true,
-            Self::FirstAndNextFrame | Self::NextFrame => false,
-        }
-    }
-}
-
 impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
     pub(crate) fn init(&mut self, freq: Hertz, _config: Config) {
         T::regs().cr1().modify(|reg| {
@@ -199,17 +137,12 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
                 timeout.check()?;
             }
 
-            // Also wait until signalled we're master and everything is waiting for us
+            // Check if we were the ones to generate START
-            while {
+            if T::regs().cr1().read().start() || !T::regs().sr2().read().msl() {
-                Self::check_and_clear_error_flags()?;
+                return Err(Error::Arbitration);
-
-                let sr2 = T::regs().sr2().read();
-                !sr2.msl() && !sr2.busy()
-            } {
-                timeout.check()?;
             }
 
-            // Set up current address, we're trying to talk to
+            // Set up current address we're trying to talk to
             T::regs().dr().write(|reg| reg.set_dr(addr << 1));
 
             // Wait until address was sent
@@ -231,10 +164,6 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
         if frame.send_stop() {
             // Send a STOP condition
             T::regs().cr1().modify(|reg| reg.set_stop(true));
-            // Wait for STOP condition to transmit.
-            while T::regs().cr1().read().stop() {
-                timeout.check()?;
-            }
         }
 
         // Fallthrough is success
@@ -301,15 +230,12 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
                 timeout.check()?;
             }
 
-            // Also wait until signalled we're master and everything is waiting for us
+            // Check if we were the ones to generate START
-            while {
+            if T::regs().cr1().read().start() || !T::regs().sr2().read().msl() {
-                let sr2 = T::regs().sr2().read();
+                return Err(Error::Arbitration);
-                !sr2.msl() && !sr2.busy()
-            } {
-                timeout.check()?;
             }
 
-            // Set up current address, we're trying to talk to
+            // Set up current address we're trying to talk to
             T::regs().dr().write(|reg| reg.set_dr((addr << 1) + 1));
 
             // Wait until address was sent
@@ -340,13 +266,6 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
         // Receive last byte
         *last = self.recv_byte(timeout)?;
 
-        if frame.send_stop() {
-            // Wait for the STOP to be sent.
-            while T::regs().cr1().read().stop() {
-                timeout.check()?;
-            }
-        }
-
         // Fallthrough is success
         Ok(())
     }
@@ -386,64 +305,13 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
     ///
     /// [transaction contract]: embedded_hal_1::i2c::I2c::transaction
     pub fn blocking_transaction(&mut self, addr: u8, operations: &mut [Operation<'_>]) -> Result<(), Error> {
-        // Check empty read buffer before starting transaction. Otherwise, we would not generate the
-        // stop condition below.
-        if operations.iter().any(|op| match op {
-            Operation::Read(read) => read.is_empty(),
-            Operation::Write(_) => false,
-        }) {
-            return Err(Error::Overrun);
-        }
-
         let timeout = self.timeout();
 
-        let mut operations = operations.iter_mut();
+        for (op, frame) in operation_frames(operations)? {
-
-        let mut prev_op: Option<&mut Operation<'_>> = None;
-        let mut next_op = operations.next();
-
-        while let Some(op) = next_op {
-            next_op = operations.next();
-
-            // Check if this is the first frame of this type. This is the case for the first overall
-            // frame in the transaction and whenever the type of operation changes.
-            let first_frame =
-                match (prev_op.as_ref(), &op) {
-                    (None, _) => true,
-                    (Some(Operation::Read(_)), Operation::Write(_))
-                    | (Some(Operation::Write(_)), Operation::Read(_)) => true,
-                    (Some(Operation::Read(_)), Operation::Read(_))
-                    | (Some(Operation::Write(_)), Operation::Write(_)) => false,
-                };
-
-            let frame = match (first_frame, next_op.as_ref()) {
-                // If this is the first frame of this type, we generate a (repeated) start condition
-                // but have to consider the next operation: if it is the last, we generate the final
-                // stop condition. Otherwise, we branch on the operation: with read operations, only
-                // the last byte overall (before a write operation or the end of the transaction) is
-                // to be NACK'd, i.e. if another read operation follows, we must ACK this last byte.
-                (true, None) => FrameOptions::FirstAndLastFrame,
-                // Make sure to keep sending ACK for last byte in read operation when it is followed
-                // by another consecutive read operation. If the current operation is write, this is
-                // identical to `FirstFrame`.
-                (true, Some(Operation::Read(_))) => FrameOptions::FirstAndNextFrame,
-                // Otherwise, send NACK for last byte (in read operation). (For write, this does not
-                // matter and could also be `FirstAndNextFrame`.)
-                (true, Some(Operation::Write(_))) => FrameOptions::FirstFrame,
-
-                // If this is not the first frame of its type, we do not generate a (repeated) start
-                // condition. Otherwise, we branch the same way as above.
-                (false, None) => FrameOptions::LastFrame,
-                (false, Some(Operation::Read(_))) => FrameOptions::NextFrame,
-                (false, Some(Operation::Write(_))) => FrameOptions::LastFrameNoStop,
-            };
-
             match op {
                 Operation::Read(read) => self.blocking_read_timeout(addr, read, timeout, frame)?,
                 Operation::Write(write) => self.write_bytes(addr, write, timeout, frame)?,
             }
-
-            prev_op = Some(op);
         }
 
         Ok(())
@@ -459,111 +327,110 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
         });
     }
 
-    async fn write_with_stop(&mut self, address: u8, write: &[u8], send_stop: bool) -> Result<(), Error>
+    async fn write_frame(&mut self, address: u8, write: &[u8], frame: FrameOptions) -> Result<(), Error>
     where
         TXDMA: crate::i2c::TxDma<T>,
     {
-        let dma_transfer = unsafe {
+        T::regs().cr2().modify(|w| {
-            let regs = T::regs();
+            // Note: Do not enable the ITBUFEN bit in the I2C_CR2 register if DMA is used for
-            regs.cr2().modify(|w| {
+            // reception.
-                // DMA mode can be enabled for transmission by setting the DMAEN bit in the I2C_CR2 register.
+            w.set_itbufen(false);
-                w.set_dmaen(true);
+            // DMA mode can be enabled for transmission by setting the DMAEN bit in the I2C_CR2
-                w.set_itbufen(false);
+            // register.
-            });
+            w.set_dmaen(true);
-            // Set the I2C_DR register address in the DMA_SxPAR register. The data will be moved to this address from the memory after each TxE event.
+            // Sending NACK is not necessary (nor possible) for write transfer.
-            let dst = regs.dr().as_ptr() as *mut u8;
+            w.set_last(false);
-
+        });
-            let ch = &mut self.tx_dma;
-            let request = ch.request();
-            Transfer::new_write(ch, request, write, dst, Default::default())
-        };
 
+        // Sentinel to disable transfer when an error occurs or future is canceled.
+        // TODO: Generate STOP condition on cancel?
         let on_drop = OnDrop::new(|| {
-            let regs = T::regs();
+            T::regs().cr2().modify(|w| {
-            regs.cr2().modify(|w| {
                 w.set_dmaen(false);
                 w.set_iterren(false);
                 w.set_itevten(false);
             })
         });
 
-        Self::enable_interrupts();
-
-        // Send a START condition
-        T::regs().cr1().modify(|reg| {
-            reg.set_start(true);
-        });
-
         let state = T::state();
 
-        // Wait until START condition was generated
+        if frame.send_start() {
-        poll_fn(|cx| {
+            // Send a START condition
-            state.waker.register(cx.waker());
+            T::regs().cr1().modify(|reg| {
+                reg.set_start(true);
+            });
 
-            match Self::check_and_clear_error_flags() {
+            // Wait until START condition was generated
-                Err(e) => Poll::Ready(Err(e)),
+            poll_fn(|cx| {
-                Ok(sr1) => {
+                state.waker.register(cx.waker());
-                    if sr1.start() {
+
-                        Poll::Ready(Ok(()))
+                match Self::check_and_clear_error_flags() {
-                    } else {
+                    Err(e) => Poll::Ready(Err(e)),
-                        Poll::Pending
+                    Ok(sr1) => {
+                        if sr1.start() {
+                            Poll::Ready(Ok(()))
+                        } else {
+                            // When pending, (re-)enable interrupts to wake us up.
+                            Self::enable_interrupts();
+                            Poll::Pending
+                        }
                     }
                 }
+            })
+            .await?;
+
+            // Check if we were the ones to generate START
+            if T::regs().cr1().read().start() || !T::regs().sr2().read().msl() {
+                return Err(Error::Arbitration);
             }
-        })
-        .await?;
 
-        // Also wait until signalled we're master and everything is waiting for us
+            // Set up current address we're trying to talk to
-        Self::enable_interrupts();
+            T::regs().dr().write(|reg| reg.set_dr(address << 1));
-        poll_fn(|cx| {
-            state.waker.register(cx.waker());
 
-            match Self::check_and_clear_error_flags() {
+            // Wait for the address to be acknowledged
-                Err(e) => Poll::Ready(Err(e)),
+            poll_fn(|cx| {
-                Ok(_) => {
+                state.waker.register(cx.waker());
-                    let sr2 = T::regs().sr2().read();
+
-                    if !sr2.msl() && !sr2.busy() {
+                match Self::check_and_clear_error_flags() {
-                        Poll::Pending
+                    Err(e) => Poll::Ready(Err(e)),
-                    } else {
+                    Ok(sr1) => {
-                        Poll::Ready(Ok(()))
+                        if sr1.addr() {
+                            Poll::Ready(Ok(()))
+                        } else {
+                            // When pending, (re-)enable interrupts to wake us up.
+                            Self::enable_interrupts();
+                            Poll::Pending
+                        }
                     }
                 }
-            }
+            })
-        })
+            .await?;
-        .await?;
 
-        // Set up current address, we're trying to talk to
+            // Clear condition by reading SR2
-        Self::enable_interrupts();
+            T::regs().sr2().read();
-        T::regs().dr().write(|reg| reg.set_dr(address << 1));
+        }
 
-        poll_fn(|cx| {
+        let dma_transfer = unsafe {
-            state.waker.register(cx.waker());
+            // Set the I2C_DR register address in the DMA_SxPAR register. The data will be moved to
-            match Self::check_and_clear_error_flags() {
+            // this address from the memory after each TxE event.
-                Err(e) => Poll::Ready(Err(e)),
+            let dst = T::regs().dr().as_ptr() as *mut u8;
-                Ok(sr1) => {
+
-                    if sr1.addr() {
+            let ch = &mut self.tx_dma;
-                        // Clear the ADDR condition by reading SR2.
+            let request = ch.request();
-                        T::regs().sr2().read();
+            Transfer::new_write(ch, request, write, dst, Default::default())
-                        Poll::Ready(Ok(()))
+        };
-                    } else {
+
-                        // If we need to go around, then re-enable the interrupts, otherwise nothing
+        // Wait for bytes to be sent, or an error to occur.
-                        // can wake us up and we'll hang.
-                        Self::enable_interrupts();
-                        Poll::Pending
-                    }
-                }
-            }
-        })
-        .await?;
-        Self::enable_interrupts();
         let poll_error = poll_fn(|cx| {
             state.waker.register(cx.waker());
 
             match Self::check_and_clear_error_flags() {
-                // Unclear why the Err turbofish is necessary here? The compiler didn’t require it in the other
+                Err(e) => Poll::Ready(Err::<(), Error>(e)),
-                // identical poll_fn check_and_clear matches.
+                Ok(_) => {
-                Err(e) => Poll::Ready(Err::<T, Error>(e)),
+                    // When pending, (re-)enable interrupts to wake us up.
-                Ok(_) => Poll::Pending,
+                    Self::enable_interrupts();
+                    Poll::Pending
+                }
             }
         });
 
@@ -573,38 +440,37 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
             _ => Ok(()),
         }?;
 
-        // The I2C transfer itself will take longer than the DMA transfer, so wait for that to finish too.
-
-        // 18.3.8 “Master transmitter: In the interrupt routine after the EOT interrupt, disable DMA
-        // requests then wait for a BTF event before programming the Stop condition.”
-
-        // TODO: If this has to be done “in the interrupt routine after the EOT interrupt”, where to put it?
         T::regs().cr2().modify(|w| {
             w.set_dmaen(false);
         });
 
-        Self::enable_interrupts();
+        if frame.send_stop() {
-        poll_fn(|cx| {
+            // The I2C transfer itself will take longer than the DMA transfer, so wait for that to finish too.
-            state.waker.register(cx.waker());
 
-            match Self::check_and_clear_error_flags() {
+            // 18.3.8 “Master transmitter: In the interrupt routine after the EOT interrupt, disable DMA
-                Err(e) => Poll::Ready(Err(e)),
+            // requests then wait for a BTF event before programming the Stop condition.”
-                Ok(sr1) => {
+            poll_fn(|cx| {
-                    if sr1.btf() {
+                state.waker.register(cx.waker());
-                        if send_stop {
+
-                            T::regs().cr1().modify(|w| {
+                match Self::check_and_clear_error_flags() {
-                                w.set_stop(true);
+                    Err(e) => Poll::Ready(Err(e)),
-                            });
+                    Ok(sr1) => {
+                        if sr1.btf() {
+                            Poll::Ready(Ok(()))
+                        } else {
+                            // When pending, (re-)enable interrupts to wake us up.
+                            Self::enable_interrupts();
+                            Poll::Pending
                         }
-
-                        Poll::Ready(Ok(()))
-                    } else {
-                        Poll::Pending
                     }
                 }
-            }
+            })
-        })
+            .await?;
-        .await?;
+
+            T::regs().cr1().modify(|w| {
+                w.set_stop(true);
+            });
+        }
 
         drop(on_drop);
 
@@ -617,20 +483,8 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
     where
         TXDMA: crate::i2c::TxDma<T>,
     {
-        self.write_with_stop(address, write, true).await?;
+        self.write_frame(address, write, FrameOptions::FirstAndLastFrame)
-
+            .await?;
-        // Wait for STOP condition to transmit.
-        Self::enable_interrupts();
-        poll_fn(|cx| {
-            T::state().waker.register(cx.waker());
-            // TODO: error interrupts are enabled here, should we additional check for and return errors?
-            if T::regs().cr1().read().stop() {
-                Poll::Pending
-            } else {
-                Poll::Ready(Ok(()))
-            }
-        })
-        .await?;
 
         Ok(())
     }
@@ -640,135 +494,151 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
     where
         RXDMA: crate::i2c::RxDma<T>,
     {
-        let state = T::state();
+        self.read_frame(address, buffer, FrameOptions::FirstAndLastFrame)
-        let buffer_len = buffer.len();
+            .await?;
 
-        let dma_transfer = unsafe {
+        Ok(())
-            let regs = T::regs();
+    }
-            regs.cr2().modify(|w| {
-                // DMA mode can be enabled for transmission by setting the DMAEN bit in the I2C_CR2 register.
-                w.set_itbufen(false);
-                w.set_dmaen(true);
-            });
-            // Set the I2C_DR register address in the DMA_SxPAR register. The data will be moved to this address from the memory after each TxE event.
-            let src = regs.dr().as_ptr() as *mut u8;
 
-            let ch = &mut self.rx_dma;
+    async fn read_frame(&mut self, address: u8, buffer: &mut [u8], frame: FrameOptions) -> Result<(), Error>
-            let request = ch.request();
+    where
-            Transfer::new_read(ch, request, src, buffer, Default::default())
+        RXDMA: crate::i2c::RxDma<T>,
-        };
+    {
+        if buffer.is_empty() {
+            return Err(Error::Overrun);
+        }
 
+        // Some branches below depend on whether the buffer contains only a single byte.
+        let single_byte = buffer.len() == 1;
+
+        T::regs().cr2().modify(|w| {
+            // Note: Do not enable the ITBUFEN bit in the I2C_CR2 register if DMA is used for
+            // reception.
+            w.set_itbufen(false);
+            // DMA mode can be enabled for transmission by setting the DMAEN bit in the I2C_CR2
+            // register.
+            w.set_dmaen(true);
+            // If, in the I2C_CR2 register, the LAST bit is set, I2C automatically sends a NACK
+            // after the next byte following EOT_1. The user can generate a Stop condition in
+            // the DMA Transfer Complete interrupt routine if enabled.
+            w.set_last(frame.send_nack() && !single_byte);
+        });
+
+        // Sentinel to disable transfer when an error occurs or future is canceled.
+        // TODO: Generate STOP condition on cancel?
         let on_drop = OnDrop::new(|| {
-            let regs = T::regs();
+            T::regs().cr2().modify(|w| {
-            regs.cr2().modify(|w| {
                 w.set_dmaen(false);
                 w.set_iterren(false);
                 w.set_itevten(false);
             })
         });
 
-        Self::enable_interrupts();
+        let state = T::state();
 
-        // Send a START condition and set ACK bit
+        if frame.send_start() {
-        T::regs().cr1().modify(|reg| {
+            // Send a START condition and set ACK bit
-            reg.set_start(true);
+            T::regs().cr1().modify(|reg| {
-            reg.set_ack(true);
+                reg.set_start(true);
-        });
+                reg.set_ack(true);
+            });
 
-        // Wait until START condition was generated
+            // Wait until START condition was generated
-        poll_fn(|cx| {
+            poll_fn(|cx| {
-            state.waker.register(cx.waker());
+                state.waker.register(cx.waker());
 
-            match Self::check_and_clear_error_flags() {
+                match Self::check_and_clear_error_flags() {
-                Err(e) => Poll::Ready(Err(e)),
+                    Err(e) => Poll::Ready(Err(e)),
-                Ok(sr1) => {
+                    Ok(sr1) => {
-                    if sr1.start() {
+                        if sr1.start() {
-                        Poll::Ready(Ok(()))
+                            Poll::Ready(Ok(()))
-                    } else {
+                        } else {
-                        Poll::Pending
+                            // When pending, (re-)enable interrupts to wake us up.
-                    }
+                            Self::enable_interrupts();
-                }
+                            Poll::Pending
-            }
-        })
-        .await?;
-
-        // Also wait until signalled we're master and everything is waiting for us
-        Self::enable_interrupts();
-        poll_fn(|cx| {
-            state.waker.register(cx.waker());
-
-            // blocking read didn’t have a check_and_clear call here, but blocking write did so
-            // I’m adding it here in case that was an oversight.
-            match Self::check_and_clear_error_flags() {
-                Err(e) => Poll::Ready(Err(e)),
-                Ok(_) => {
-                    let sr2 = T::regs().sr2().read();
-                    if !sr2.msl() && !sr2.busy() {
-                        Poll::Pending
-                    } else {
-                        Poll::Ready(Ok(()))
-                    }
-                }
-            }
-        })
-        .await?;
-
-        // Set up current address, we're trying to talk to
-        T::regs().dr().write(|reg| reg.set_dr((address << 1) + 1));
-
-        // Wait for the address to be acknowledged
-
-        Self::enable_interrupts();
-        poll_fn(|cx| {
-            state.waker.register(cx.waker());
-
-            match Self::check_and_clear_error_flags() {
-                Err(e) => Poll::Ready(Err(e)),
-                Ok(sr1) => {
-                    if sr1.addr() {
-                        // 18.3.8: When a single byte must be received: the NACK must be programmed during EV6
-                        // event, i.e. program ACK=0 when ADDR=1, before clearing ADDR flag.
-                        if buffer_len == 1 {
-                            T::regs().cr1().modify(|w| {
-                                w.set_ack(false);
-                            });
                         }
-                        Poll::Ready(Ok(()))
-                    } else {
-                        Poll::Pending
                     }
                 }
+            })
+            .await?;
+
+            // Check if we were the ones to generate START
+            if T::regs().cr1().read().start() || !T::regs().sr2().read().msl() {
+                return Err(Error::Arbitration);
             }
-        })
-        .await?;
 
-        // Clear ADDR condition by reading SR2
+            // Set up current address we're trying to talk to
-        T::regs().sr2().read();
+            T::regs().dr().write(|reg| reg.set_dr((address << 1) + 1));
 
-        // 18.3.8: When a single byte must be received: [snip] Then the
+            // Wait for the address to be acknowledged
-        // user can program the STOP condition either after clearing ADDR flag, or in the
+            poll_fn(|cx| {
-        // DMA Transfer Complete interrupt routine.
+                state.waker.register(cx.waker());
-        if buffer_len == 1 {
+
+                match Self::check_and_clear_error_flags() {
+                    Err(e) => Poll::Ready(Err(e)),
+                    Ok(sr1) => {
+                        if sr1.addr() {
+                            Poll::Ready(Ok(()))
+                        } else {
+                            // When pending, (re-)enable interrupts to wake us up.
+                            Self::enable_interrupts();
+                            Poll::Pending
+                        }
+                    }
+                }
+            })
+            .await?;
+
+            // 18.3.8: When a single byte must be received: the NACK must be programmed during EV6
+            // event, i.e. program ACK=0 when ADDR=1, before clearing ADDR flag.
+            if frame.send_nack() && single_byte {
+                T::regs().cr1().modify(|w| {
+                    w.set_ack(false);
+                });
+            }
+
+            // Clear condition by reading SR2
+            T::regs().sr2().read();
+        } else {
+            // Before starting reception of single byte (but without START condition, i.e. in case
+            // of continued frame), program NACK to emit at end of this byte.
+            if frame.send_nack() && single_byte {
+                T::regs().cr1().modify(|w| {
+                    w.set_ack(false);
+                });
+            }
+        }
+
+        // 18.3.8: When a single byte must be received: [snip] Then the user can program the STOP
+        // condition either after clearing ADDR flag, or in the DMA Transfer Complete interrupt
+        // routine.
+        if frame.send_stop() && single_byte {
             T::regs().cr1().modify(|w| {
                 w.set_stop(true);
             });
-        } else {
-            // If, in the I2C_CR2 register, the LAST bit is set, I2C
-            // automatically sends a NACK after the next byte following EOT_1. The user can
-            // generate a Stop condition in the DMA Transfer Complete interrupt routine if enabled.
-            T::regs().cr2().modify(|w| {
-                w.set_last(true);
-            })
         }
 
+        let dma_transfer = unsafe {
+            // Set the I2C_DR register address in the DMA_SxPAR register. The data will be moved
+            // from this address from the memory after each RxE event.
+            let src = T::regs().dr().as_ptr() as *mut u8;
+
+            let ch = &mut self.rx_dma;
+            let request = ch.request();
+            Transfer::new_read(ch, request, src, buffer, Default::default())
+        };
+
         // Wait for bytes to be received, or an error to occur.
-        Self::enable_interrupts();
         let poll_error = poll_fn(|cx| {
             state.waker.register(cx.waker());
 
             match Self::check_and_clear_error_flags() {
-                Err(e) => Poll::Ready(Err::<T, Error>(e)),
+                Err(e) => Poll::Ready(Err::<(), Error>(e)),
-                _ => Poll::Pending,
+                _ => {
+                    // When pending, (re-)enable interrupts to wake us up.
+                    Self::enable_interrupts();
+                    Poll::Pending
+                }
             }
         });
 
@@ -777,18 +647,16 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
             _ => Ok(()),
         }?;
 
-        // Wait for the STOP to be sent (STOP bit cleared).
+        T::regs().cr2().modify(|w| {
-        Self::enable_interrupts();
+            w.set_dmaen(false);
-        poll_fn(|cx| {
+        });
-            state.waker.register(cx.waker());
+
-            // TODO: error interrupts are enabled here, should we additional check for and return errors?
+        if frame.send_stop() && !single_byte {
-            if T::regs().cr1().read().stop() {
+            T::regs().cr1().modify(|w| {
-                Poll::Pending
+                w.set_stop(true);
-            } else {
+            });
-                Poll::Ready(Ok(()))
+        }
-            }
+
-        })
-        .await?;
         drop(on_drop);
 
         // Fallthrough is success
@@ -801,8 +669,34 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
         RXDMA: crate::i2c::RxDma<T>,
         TXDMA: crate::i2c::TxDma<T>,
     {
-        self.write_with_stop(address, write, false).await?;
+        // Check empty read buffer before starting transaction. Otherwise, we would not generate the
-        self.read(address, read).await
+        // stop condition below.
+        if read.is_empty() {
+            return Err(Error::Overrun);
+        }
+
+        self.write_frame(address, write, FrameOptions::FirstFrame).await?;
+        self.read_frame(address, read, FrameOptions::FirstAndLastFrame).await
+    }
+
+    /// Transaction with operations.
+    ///
+    /// Consecutive operations of same type are merged. See [transaction contract] for details.
+    ///
+    /// [transaction contract]: embedded_hal_1::i2c::I2c::transaction
+    pub async fn transaction(&mut self, addr: u8, operations: &mut [Operation<'_>]) -> Result<(), Error>
+    where
+        RXDMA: crate::i2c::RxDma<T>,
+        TXDMA: crate::i2c::TxDma<T>,
+    {
+        for (op, frame) in operation_frames(operations)? {
+            match op {
+                Operation::Read(read) => self.read_frame(addr, read, frame).await?,
+                Operation::Write(write) => self.write_frame(addr, write, frame).await?,
+            }
+        }
+
+        Ok(())
     }
 }
 

embassy-stm32/src/i2c/v2.rs

@@ -557,6 +557,21 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
         Ok(())
     }
 
+    /// Transaction with operations.
+    ///
+    /// Consecutive operations of same type are merged. See [transaction contract] for details.
+    ///
+    /// [transaction contract]: embedded_hal_1::i2c::I2c::transaction
+    pub async fn transaction(&mut self, addr: u8, operations: &mut [Operation<'_>]) -> Result<(), Error>
+    where
+        RXDMA: crate::i2c::RxDma<T>,
+        TXDMA: crate::i2c::TxDma<T>,
+    {
+        let _ = addr;
+        let _ = operations;
+        todo!()
+    }
+
     // =========================
     //  Blocking public API