From ae726274ddb95c048c386817ef105012dcc06dad Mon Sep 17 00:00:00 2001 From: xoviat Date: Thu, 30 Oct 2025 15:49:07 -0500 Subject: reduce diff with misc. reversions --- embassy-stm32/src/i2c/mod.rs | 97 +++++++++++++++-------------- embassy-stm32/src/i2c/v1.rs | 144 ++++++++++++++++++++++++------------------- 2 files changed, 131 insertions(+), 110 deletions(-) diff --git a/embassy-stm32/src/i2c/mod.rs b/embassy-stm32/src/i2c/mod.rs index 785ebd866..ee60c3f44 100644 --- a/embassy-stm32/src/i2c/mod.rs +++ b/embassy-stm32/src/i2c/mod.rs @@ -436,55 +436,58 @@ impl<'d, IM: MasterMode> embedded_hal_async::i2c::I2c for I2c<'d, Async, IM> { } } -/// Operation framing configuration for I2C transactions. +/// Frame type in I2C transaction. /// -/// This determines the I2C frame boundaries for each operation within a transaction, -/// controlling the generation of start conditions (ST or SR), stop conditions (SP), -/// and ACK/NACK behavior for read operations. +/// This tells each method what kind of frame 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, some framing configurations are functionally identical -/// because they differ only in ACK/NACK treatment which is relevant only for reads: +/// For write operations, the following options are identical because they differ only in the (N)ACK +/// treatment relevant for read operations: /// -/// - `First` and `FirstAndNext` behave identically for writes -/// - `Next` and `LastNoStop` behave identically for writes +/// - `FirstFrame` and `FirstAndNextFrame` behave identically for writes +/// - `NextFrame` and `LastFrameNoStop` behave identically for writes +/// +/// Abbreviations used below: /// -/// **Framing Legend:** /// - `ST` = start condition -/// - `SR` = repeated start condition +/// - `SR` = repeated start condition /// - `SP` = stop condition -/// - `ACK/NACK` = acknowledgment behavior for the final byte of read operations +/// - `ACK`/`NACK` = last byte in read operation #[derive(Copy, Clone)] #[allow(dead_code)] -enum OperationFraming { - /// `[ST/SR]+[NACK]+[SP]` - First operation of its type in the transaction and also the final operation overall. - FirstAndLast, - /// `[ST/SR]+[NACK]` - First operation of its type in the transaction, final operation in a read sequence, but not the final operation overall. - First, - /// `[ST/SR]+[ACK]` - First operation of its type in the transaction, but neither the final operation overall nor the final operation in a read sequence. - FirstAndNext, - /// `[ACK]` - Continuation operation in a read sequence (neither first nor last). - Next, - /// `[NACK]+[SP]` - Final operation overall in the transaction, but not the first operation of its type. - Last, - /// `[NACK]` - Final operation in a read sequence, but not the final operation overall in the transaction. - LastNoStop, +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 OperationFraming { +impl FrameOptions { /// Returns true if a start or repeated start condition should be generated before this operation. fn send_start(self) -> bool { match self { - Self::FirstAndLast | Self::First | Self::FirstAndNext => true, - Self::Next | Self::Last | Self::LastNoStop => false, + Self::FirstAndLastFrame | Self::FirstFrame | Self::FirstAndNextFrame => true, + Self::NextFrame | Self::LastFrame | Self::LastFrameNoStop => false, } } /// Returns true if a stop condition should be generated after this operation. fn send_stop(self) -> bool { match self { - Self::FirstAndLast | Self::Last => true, - Self::First | Self::FirstAndNext | Self::Next | Self::LastNoStop => false, + Self::FirstAndLastFrame | Self::LastFrame => true, + Self::FirstFrame | Self::FirstAndNextFrame | Self::NextFrame | Self::LastFrameNoStop => false, } } @@ -494,16 +497,16 @@ impl OperationFraming { /// next transmission (or stop condition). fn send_nack(self) -> bool { match self { - Self::FirstAndLast | Self::First | Self::Last | Self::LastNoStop => true, - Self::FirstAndNext | Self::Next => false, + Self::FirstAndLastFrame | Self::FirstFrame | Self::LastFrame | Self::LastFrameNoStop => true, + Self::FirstAndNextFrame | Self::NextFrame => false, } } } -/// Analyzes I2C transaction operations and assigns appropriate framing to each. +/// Analyzes I2C transaction operations and assigns appropriate frame to each. /// /// This function processes a sequence of I2C operations and determines the correct -/// framing configuration for each operation to ensure proper I2C protocol compliance. +/// frame configuration for each operation to ensure proper I2C protocol compliance. /// It handles the complex logic of: /// /// - Generating start conditions for the first operation of each type (read/write) @@ -512,7 +515,7 @@ impl OperationFraming { /// - Ensuring proper bus handoff between different operation types /// /// **Transaction Contract Compliance:** -/// The framing assignments ensure compliance with the embedded-hal I2C transaction contract, +/// The frame assignments ensure compliance with the embedded-hal I2C transaction contract, /// where consecutive operations of the same type are logically merged while maintaining /// proper protocol boundaries. /// @@ -524,12 +527,12 @@ impl OperationFraming { /// * `operations` - Mutable slice of I2C operations from embedded-hal /// /// # Returns -/// An iterator over (operation, framing) pairs, or an error if the transaction is invalid +/// An iterator over (operation, frame) pairs, or an error if the transaction is invalid /// #[allow(dead_code)] -fn assign_operation_framing<'a, 'b: 'a>( +fn operation_frames<'a, 'b: 'a>( operations: &'a mut [embedded_hal_1::i2c::Operation<'b>], -) -> Result, OperationFraming)>, Error> { +) -> Result, FrameOptions)>, Error> { use embedded_hal_1::i2c::Operation::{Read, Write}; // Validate that no read operations have empty buffers before starting the transaction. @@ -555,29 +558,29 @@ fn assign_operation_framing<'a, 'b: 'a>( let is_first_of_type = next_first_operation; let next_op = operations.peek(); - // Compute the appropriate framing based on three key properties: + // Compute the appropriate frame based on three key properties: // // 1. **Start Condition**: Generate (repeated) start for first operation of each type // 2. **Stop Condition**: Generate stop for the final operation in the entire transaction // 3. **ACK/NACK for Reads**: For read operations, send ACK if more reads follow in the // sequence, or NACK for the final read in a sequence (before write or transaction end) // - // The third property is checked for all operations since the resulting framing + // The third property is checked for all operations since the resulting frame // configurations are identical for write operations regardless of ACK/NACK treatment. - let framing = match (is_first_of_type, next_op) { + let frame = match (is_first_of_type, next_op) { // First operation of type, and it's also the final operation overall - (true, None) => OperationFraming::FirstAndLast, + (true, None) => FrameOptions::FirstAndLastFrame, // First operation of type, next operation is also a read (continue read sequence) - (true, Some(Read(_))) => OperationFraming::FirstAndNext, + (true, Some(Read(_))) => FrameOptions::FirstAndNextFrame, // First operation of type, next operation is write (end current sequence) - (true, Some(Write(_))) => OperationFraming::First, + (true, Some(Write(_))) => FrameOptions::FirstFrame, // Continuation operation, and it's the final operation overall - (false, None) => OperationFraming::Last, + (false, None) => FrameOptions::LastFrame, // Continuation operation, next operation is also a read (continue read sequence) - (false, Some(Read(_))) => OperationFraming::Next, + (false, Some(Read(_))) => FrameOptions::NextFrame, // Continuation operation, next operation is write (end current sequence, no stop) - (false, Some(Write(_))) => OperationFraming::LastNoStop, + (false, Some(Write(_))) => FrameOptions::LastFrameNoStop, }; // Pre-calculate whether the next operation will be the first of its type. @@ -592,6 +595,6 @@ fn assign_operation_framing<'a, 'b: 'a>( (Read(_), Some(Read(_))) | (Write(_), Some(Write(_))) => false, }; - Some((current_op, framing)) + Some((current_op, frame)) })) } diff --git a/embassy-stm32/src/i2c/v1.rs b/embassy-stm32/src/i2c/v1.rs index 42d39655f..128a58db7 100644 --- a/embassy-stm32/src/i2c/v1.rs +++ b/embassy-stm32/src/i2c/v1.rs @@ -14,7 +14,7 @@ use embedded_hal_1::i2c::Operation; use mode::Master; use super::*; -use crate::mode::Mode; +use crate::mode::Mode as PeriMode; use crate::pac::i2c; // /!\ /!\ @@ -43,7 +43,7 @@ pub unsafe fn on_interrupt() { }); } -impl<'d, M: Mode, IM: MasterMode> I2c<'d, M, IM> { +impl<'d, M: PeriMode, IM: MasterMode> I2c<'d, M, IM> { pub(crate) fn init(&mut self, config: Config) { self.info.regs.cr1().modify(|reg| { reg.set_pe(false); @@ -82,8 +82,8 @@ impl<'d, M: Mode, IM: MasterMode> I2c<'d, M, IM> { reg.set_freq(timings.freq); }); self.info.regs.ccr().modify(|reg| { - reg.set_f_s(timings.f_s); - reg.set_duty(timings.duty); + reg.set_f_s(timings.mode.f_s()); + reg.set_duty(timings.duty.duty()); reg.set_ccr(timings.ccr); }); self.info.regs.trise().modify(|reg| { @@ -158,9 +158,9 @@ impl<'d, M: Mode, IM: MasterMode> I2c<'d, M, IM> { address: u8, write_buffer: &[u8], timeout: Timeout, - framing: OperationFraming, + frame: FrameOptions, ) -> Result<(), Error> { - if framing.send_start() { + if frame.send_start() { // Send a START condition self.info.regs.cr1().modify(|reg| { @@ -196,7 +196,7 @@ impl<'d, M: Mode, IM: MasterMode> I2c<'d, M, IM> { self.send_byte(*c, timeout)?; } - if framing.send_stop() { + if frame.send_stop() { // Send a STOP condition self.info.regs.cr1().modify(|reg| reg.set_stop(true)); } @@ -247,13 +247,13 @@ impl<'d, M: Mode, IM: MasterMode> I2c<'d, M, IM> { address: u8, read_buffer: &mut [u8], timeout: Timeout, - framing: OperationFraming, + frame: FrameOptions, ) -> Result<(), Error> { let Some((last_byte, read_buffer)) = read_buffer.split_last_mut() else { return Err(Error::Overrun); }; - if framing.send_start() { + if frame.send_start() { // Send a START condition and set ACK bit self.info.regs.cr1().modify(|reg| { reg.set_start(true); @@ -290,10 +290,10 @@ impl<'d, M: Mode, IM: MasterMode> I2c<'d, M, IM> { // Prepare to send NACK then STOP after next byte self.info.regs.cr1().modify(|reg| { - if framing.send_nack() { + if frame.send_nack() { reg.set_ack(false); } - if framing.send_stop() { + if frame.send_stop() { reg.set_stop(true); } }); @@ -307,12 +307,12 @@ impl<'d, M: Mode, IM: MasterMode> I2c<'d, M, IM> { /// Blocking read. pub fn blocking_read(&mut self, address: u8, read_buffer: &mut [u8]) -> Result<(), Error> { - self.blocking_read_timeout(address, read_buffer, self.timeout(), OperationFraming::FirstAndLast) + self.blocking_read_timeout(address, read_buffer, self.timeout(), FrameOptions::FirstAndLastFrame) } /// Blocking write. pub fn blocking_write(&mut self, address: u8, write_buffer: &[u8]) -> Result<(), Error> { - self.write_bytes(address, write_buffer, self.timeout(), OperationFraming::FirstAndLast)?; + self.write_bytes(address, write_buffer, self.timeout(), FrameOptions::FirstAndLastFrame)?; // Fallthrough is success Ok(()) @@ -333,8 +333,8 @@ impl<'d, M: Mode, IM: MasterMode> I2c<'d, M, IM> { let timeout = self.timeout(); - self.write_bytes(address, write_buffer, timeout, OperationFraming::First)?; - self.blocking_read_timeout(address, read_buffer, timeout, OperationFraming::FirstAndLast)?; + self.write_bytes(address, write_buffer, timeout, FrameOptions::FirstFrame)?; + self.blocking_read_timeout(address, read_buffer, timeout, FrameOptions::FirstAndLastFrame)?; Ok(()) } @@ -347,10 +347,10 @@ impl<'d, M: Mode, IM: MasterMode> I2c<'d, M, IM> { pub fn blocking_transaction(&mut self, address: u8, operations: &mut [Operation<'_>]) -> Result<(), Error> { let timeout = self.timeout(); - for (op, framing) in assign_operation_framing(operations)? { + for (op, frame) in operation_frames(operations)? { match op { - Operation::Read(read_buffer) => self.blocking_read_timeout(address, read_buffer, timeout, framing)?, - Operation::Write(write_buffer) => self.write_bytes(address, write_buffer, timeout, framing)?, + Operation::Read(read_buffer) => self.blocking_read_timeout(address, read_buffer, timeout, frame)?, + Operation::Write(write_buffer) => self.write_bytes(address, write_buffer, timeout, frame)?, } } @@ -380,12 +380,7 @@ impl<'d, M: Mode, IM: MasterMode> I2c<'d, M, IM> { } impl<'d, IM: MasterMode> I2c<'d, Async, IM> { - async fn write_with_framing( - &mut self, - address: u8, - write_buffer: &[u8], - framing: OperationFraming, - ) -> Result<(), Error> { + async fn write_frame(&mut self, address: u8, write_buffer: &[u8], frame: FrameOptions) -> Result<(), Error> { self.info.regs.cr2().modify(|w| { // Note: Do not enable the ITBUFEN bit in the I2C_CR2 register if DMA is used for // reception. @@ -407,7 +402,7 @@ impl<'d, IM: MasterMode> I2c<'d, Async, IM> { }) }); - if framing.send_start() { + if frame.send_start() { // Send a START condition self.info.regs.cr1().modify(|reg| { reg.set_start(true); @@ -498,7 +493,7 @@ impl<'d, IM: MasterMode> I2c<'d, Async, IM> { w.set_dmaen(false); }); - if framing.send_stop() { + if frame.send_stop() { // 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 @@ -534,7 +529,7 @@ impl<'d, IM: MasterMode> I2c<'d, Async, IM> { /// Write. pub async fn write(&mut self, address: u8, write_buffer: &[u8]) -> Result<(), Error> { - self.write_with_framing(address, write_buffer, OperationFraming::FirstAndLast) + self.write_frame(address, write_buffer, FrameOptions::FirstAndLastFrame) .await?; Ok(()) @@ -542,18 +537,13 @@ impl<'d, IM: MasterMode> I2c<'d, Async, IM> { /// Read. pub async fn read(&mut self, address: u8, read_buffer: &mut [u8]) -> Result<(), Error> { - self.read_with_framing(address, read_buffer, OperationFraming::FirstAndLast) + self.read_frame(address, read_buffer, FrameOptions::FirstAndLastFrame) .await?; Ok(()) } - async fn read_with_framing( - &mut self, - address: u8, - read_buffer: &mut [u8], - framing: OperationFraming, - ) -> Result<(), Error> { + async fn read_frame(&mut self, address: u8, read_buffer: &mut [u8], frame: FrameOptions) -> Result<(), Error> { if read_buffer.is_empty() { return Err(Error::Overrun); } @@ -571,7 +561,7 @@ impl<'d, IM: MasterMode> I2c<'d, Async, IM> { // 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(framing.send_nack() && !single_byte); + w.set_last(frame.send_nack() && !single_byte); }); // Sentinel to disable transfer when an error occurs or future is canceled. @@ -584,7 +574,7 @@ impl<'d, IM: MasterMode> I2c<'d, Async, IM> { }) }); - if framing.send_start() { + if frame.send_start() { // Send a START condition and set ACK bit self.info.regs.cr1().modify(|reg| { reg.set_start(true); @@ -639,7 +629,7 @@ impl<'d, IM: MasterMode> I2c<'d, Async, IM> { // 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 framing.send_nack() && single_byte { + if frame.send_nack() && single_byte { self.info.regs.cr1().modify(|w| { w.set_ack(false); }); @@ -650,7 +640,7 @@ impl<'d, IM: MasterMode> I2c<'d, Async, IM> { } else { // Before starting reception of single byte (but without START condition, i.e. in case // of merged operations), program NACK to emit at end of this byte. - if framing.send_nack() && single_byte { + if frame.send_nack() && single_byte { self.info.regs.cr1().modify(|w| { w.set_ack(false); }); @@ -660,7 +650,7 @@ impl<'d, IM: MasterMode> I2c<'d, Async, IM> { // 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 framing.send_stop() && single_byte { + if frame.send_stop() && single_byte { self.info.regs.cr1().modify(|w| { w.set_stop(true); }); @@ -697,7 +687,7 @@ impl<'d, IM: MasterMode> I2c<'d, Async, IM> { w.set_dmaen(false); }); - if framing.send_stop() && !single_byte { + if frame.send_stop() && !single_byte { self.info.regs.cr1().modify(|w| { w.set_stop(true); }); @@ -717,9 +707,9 @@ impl<'d, IM: MasterMode> I2c<'d, Async, IM> { return Err(Error::Overrun); } - self.write_with_framing(address, write_buffer, OperationFraming::First) + self.write_frame(address, write_buffer, FrameOptions::FirstFrame) .await?; - self.read_with_framing(address, read_buffer, OperationFraming::FirstAndLast) + self.read_frame(address, read_buffer, FrameOptions::FirstAndLastFrame) .await } @@ -729,10 +719,10 @@ impl<'d, IM: MasterMode> I2c<'d, Async, IM> { /// /// [transaction contract]: embedded_hal_1::i2c::I2c::transaction pub async fn transaction(&mut self, address: u8, operations: &mut [Operation<'_>]) -> Result<(), Error> { - for (op, framing) in assign_operation_framing(operations)? { + for (op, frame) in operation_frames(operations)? { match op { - Operation::Read(read_buffer) => self.read_with_framing(address, read_buffer, framing).await?, - Operation::Write(write_buffer) => self.write_with_framing(address, write_buffer, framing).await?, + Operation::Read(read_buffer) => self.read_frame(address, read_buffer, frame).await?, + Operation::Write(write_buffer) => self.write_frame(address, write_buffer, frame).await?, } } @@ -740,6 +730,34 @@ impl<'d, IM: MasterMode> I2c<'d, Async, IM> { } } +enum Mode { + Fast, + Standard, +} + +impl Mode { + fn f_s(&self) -> i2c::vals::FS { + match self { + Mode::Fast => i2c::vals::FS::FAST, + Mode::Standard => i2c::vals::FS::STANDARD, + } + } +} + +enum Duty { + Duty2_1, + Duty16_9, +} + +impl Duty { + fn duty(&self) -> i2c::vals::Duty { + match self { + Duty::Duty2_1 => i2c::vals::Duty::DUTY2_1, + Duty::Duty16_9 => i2c::vals::Duty::DUTY16_9, + } + } +} + /// Result of attempting to send a byte in slave transmitter mode #[derive(Debug, PartialEq)] enum TransmitResult { @@ -776,7 +794,7 @@ enum SlaveTermination { Nack, } -impl<'d, M: Mode> I2c<'d, M, Master> { +impl<'d, M: PeriMode> I2c<'d, M, Master> { /// Configure the I2C driver for slave operations, allowing for the driver to be used as a slave and a master (multimaster) pub fn into_slave_multimaster(mut self, slave_addr_config: SlaveAddrConfig) -> I2c<'d, M, MultiMaster> { let mut slave = I2c { @@ -797,7 +815,7 @@ impl<'d, M: Mode> I2c<'d, M, Master> { } // Address configuration methods -impl<'d, M: Mode, IM: MasterMode> I2c<'d, M, IM> { +impl<'d, M: PeriMode, IM: MasterMode> I2c<'d, M, IM> { /// Initialize slave mode with address configuration pub(crate) fn init_slave(&mut self, config: SlaveAddrConfig) { trace!("I2C slave: initializing with config={:?}", config); @@ -888,7 +906,7 @@ impl<'d, M: Mode, IM: MasterMode> I2c<'d, M, IM> { } } -impl<'d, M: Mode> I2c<'d, M, MultiMaster> { +impl<'d, M: PeriMode> I2c<'d, M, MultiMaster> { /// Listen for incoming I2C address match and return the command type /// /// This method blocks until the slave address is matched by a master. @@ -1685,11 +1703,11 @@ impl<'d> I2c<'d, Async, MultiMaster> { /// peripherals, which use three separate registers (CR2.FREQ, CCR, TRISE) instead of /// the unified TIMINGR register found in v2 hardware. struct Timings { - freq: u8, // APB frequency in MHz for CR2.FREQ register - f_s: i2c::vals::FS, // Standard or Fast mode selection - trise: u8, // Rise time compensation value - ccr: u16, // Clock control register value - duty: i2c::vals::Duty, // Fast mode duty cycle selection + freq: u8, // APB frequency in MHz for CR2.FREQ register + mode: Mode, // Standard or Fast mode selection + trise: u8, // Rise time compensation value + ccr: u16, // Clock control register value + duty: Duty, // Fast mode duty cycle selection } impl Timings { @@ -1709,25 +1727,25 @@ impl Timings { let mut ccr; let duty; - let f_s; + let mode; // I2C clock control calculation if frequency <= 100_000 { - duty = i2c::vals::Duty::DUTY2_1; - f_s = i2c::vals::FS::STANDARD; + duty = Duty::Duty2_1; + mode = Mode::Standard; ccr = { let ccr = clock / (frequency * 2); if ccr < 4 { 4 } else { ccr } }; } else { const DUTYCYCLE: u8 = 0; - f_s = i2c::vals::FS::FAST; + mode = Mode::Fast; if DUTYCYCLE == 0 { - duty = i2c::vals::Duty::DUTY2_1; + duty = Duty::Duty2_1; ccr = clock / (frequency * 3); ccr = if ccr < 1 { 1 } else { ccr }; } else { - duty = i2c::vals::Duty::DUTY16_9; + duty = Duty::Duty16_9; ccr = clock / (frequency * 25); ccr = if ccr < 1 { 1 } else { ccr }; } @@ -1735,15 +1753,15 @@ impl Timings { Self { freq: freq as u8, - f_s, trise: trise as u8, ccr: ccr as u16, duty, + mode, } } } -impl<'d, M: Mode> SetConfig for I2c<'d, M, Master> { +impl<'d, M: PeriMode> SetConfig for I2c<'d, M, Master> { type Config = Hertz; type ConfigError = (); fn set_config(&mut self, config: &Self::Config) -> Result<(), ()> { @@ -1752,8 +1770,8 @@ impl<'d, M: Mode> SetConfig for I2c<'d, M, Master> { reg.set_freq(timings.freq); }); self.info.regs.ccr().modify(|reg| { - reg.set_f_s(timings.f_s); - reg.set_duty(timings.duty); + reg.set_f_s(timings.mode.f_s()); + reg.set_duty(timings.duty.duty()); reg.set_ccr(timings.ccr); }); self.info.regs.trise().modify(|reg| { -- cgit