diff --git a/hal/src/sercom/i2c.rs b/hal/src/sercom/i2c.rs index c33878abe05..68c030c3b3c 100644 --- a/hal/src/sercom/i2c.rs +++ b/hal/src/sercom/i2c.rs @@ -199,10 +199,10 @@ //! use atsamd_hal::dmac::channel::{AnyChannel, Ready}; //! use atsand_hal::sercom::i2c::{I2c, AnyConfig, Error}; //! use atsamd_hal::embedded_hal::i2c::I2c; -//! fn i2c_send_with_dma>(i2c: I2c, channel: C, bytes: &[u8]) -> Result<(), Error>{ +//! fn i2c_write_with_dma>(i2c: I2c, channel: C, bytes: &[u8]) -> Result<(), Error>{ //! // Attach a DMA channel //! let i2c = i2c.with_dma_channel(channel); -//! i2c.send(0x54, bytes)?; +//! i2c.write(0x54, bytes)?; //! } //! ``` //! @@ -215,6 +215,13 @@ //! across all adjacent operations must not exceed 256. If you need continuous //! transfers of 256 bytes or more, use the non-DMA [`I2c`] implementations. //! +//! * When using [`I2c::transaction`] or [`I2c::write_read`], the +//! [`embedded_hal::i2c::I2c`] specification mandates that a REPEATED START +//! (instead of a STOP+START) is sent between transactions of a different type +//! (read/write). Unfortunately, in DMA mode, the hardware is only capable of +//! sending STOP+START. If you absolutely need repeated starts, the only +//! workaround is to use the I2C without DMA. +//! //! * Using [`I2c::transaction`] consumes significantly more memory than the //! other methods provided by [`embedded_hal::i2c::I2c`] (at least 256 bytes //! extra). @@ -239,6 +246,7 @@ //! [`PinMode`]: crate::gpio::pin::PinMode //! [`embedded_hal::i2c::I2c`]: crate::ehal::i2c::I2c //! [`I2c::transaction`]: crate::ehal::i2c::I2c::transaction +//! [`I2c::write_read`]: crate::ehal::i2c::I2c::write_read use atsamd_hal_macros::hal_module; diff --git a/hal/src/sercom/i2c/impl_ehal.rs b/hal/src/sercom/i2c/impl_ehal.rs index 6c7838d7804..3a811b30d77 100644 --- a/hal/src/sercom/i2c/impl_ehal.rs +++ b/hal/src/sercom/i2c/impl_ehal.rs @@ -1,7 +1,7 @@ //! [`embedded-hal`] trait implementations for [`I2c`]s use super::{config::AnyConfig, flags::Error, I2c}; -use crate::ehal::i2c::{self, ErrorKind, ErrorType, NoAcknowledgeSource}; +use crate::ehal::i2c::{self, ErrorKind, ErrorType, NoAcknowledgeSource, Operation}; impl i2c::Error for Error { #[allow(unreachable_patterns)] @@ -26,42 +26,42 @@ impl I2c { fn transaction_byte_by_byte( &mut self, address: u8, - operations: &mut [i2c::Operation<'_>], + operations: &mut [Operation<'_>], ) -> Result<(), Error> { - /// Helper type for keeping track of the type of operation that was - /// executed last - #[derive(Clone, Copy)] - enum Operation { - Read, - Write, - } + let mut op_groups = chunk_operations(operations).peekable(); + + while let Some(group) = op_groups.next() { + let mut group = group.iter_mut(); + // Unwrapping is OK here because chunk_operations will never give us a 0-length + // chunk. + let op = group.next().unwrap(); - // Keep track of the last executed operation type. The method - // specification demands, that no repeated start condition is sent - // between adjacent operations of the same type. - let mut last_op = None; - for op in operations { + // First operation in the group - send a START with the address, and the first + // operation. match op { - i2c::Operation::Read(buf) => { - if let Some(Operation::Read) = last_op { - self.continue_read(buf)?; - } else { - self.do_read(address, buf)?; - last_op = Some(Operation::Read); - } - } - i2c::Operation::Write(bytes) => { - if let Some(Operation::Write) = last_op { - self.continue_write(bytes)?; - } else { - self.do_write(address, bytes)?; - last_op = Some(Operation::Write); - } + Operation::Read(buf) => self.do_read(address, buf)?, + Operation::Write(buf) => self.do_write(address, buf)?, + } + + // For all subsequent operations, just send/read more bytes without any more + // ceremony. + for op in group { + match op { + Operation::Read(buf) => self.continue_read(buf)?, + Operation::Write(buf) => self.continue_write(buf)?, } } + + let regs = &mut self.config.as_mut().registers; + if op_groups.peek().is_some() { + // If we still have more groups to go, send a repeated start + regs.cmd_repeated_start(); + } else { + // Otherwise, send a stop + regs.cmd_stop(); + } } - self.cmd_stop(); Ok(()) } } @@ -70,7 +70,7 @@ impl i2c::I2c for I2c { fn transaction( &mut self, address: u8, - operations: &mut [i2c::Operation<'_>], + operations: &mut [Operation<'_>], ) -> Result<(), Self::Error> { self.transaction_byte_by_byte(address, operations)?; Ok(()) @@ -290,10 +290,7 @@ mod dma { // the same type, we must revert to the byte-by-byte I2C implementations. let mut descriptors = heapless::Vec::::new(); - // Arrange operations in groups of contiguous types (R/W) - let op_groups = operations.chunk_by_mut(|this, next| { - matches!((this, next), (Write(_), Write(_)) | (Read(_), Read(_))) - }); + let op_groups = chunk_operations(operations); for group in op_groups { descriptors.clear(); @@ -435,3 +432,14 @@ impl crate::ehal_02::blocking::i2c::WriteRead for I2c { Ok(()) } } + +/// Arrange all operations in contiguous chunks of the same R/W type +pub(super) fn chunk_operations<'a, 'op>( + operations: &'a mut [Operation<'op>], +) -> impl Iterator]> { + use i2c::Operation::{Read, Write}; + + operations.chunk_by_mut(|this, next| { + matches!((this, next), (Write(_), Write(_)) | (Read(_), Read(_))) + }) +} diff --git a/hal/src/sercom/i2c/reg.rs b/hal/src/sercom/i2c/reg.rs index 90e53929ad3..0c5379913c3 100644 --- a/hal/src/sercom/i2c/reg.rs +++ b/hal/src/sercom/i2c/reg.rs @@ -10,6 +10,7 @@ use atsamd_hal_macros::hal_cfg; const MASTER_ACT_READ: u8 = 2; const MASTER_ACT_STOP: u8 = 3; +const MASTER_ACT_REPEATED_START: u8 = 1; #[hal_cfg(any("sercom0-d11", "sercom0-d21"))] type DataReg = u8; @@ -330,18 +331,32 @@ impl Registers { self.sync_sysop(); } + /// Send a STOP condition. If the I2C is performing a read, will also send a + /// NACK to the slave. #[inline] - pub(super) fn issue_command(&mut self, cmd: u8) { - self.i2c_master() - .ctrlb() - .modify(|_, w| unsafe { w.cmd().bits(cmd) }); - + pub(super) fn cmd_stop(&mut self) { + unsafe { + self.i2c_master().ctrlb().modify(|_, w| { + // set bit means send NACK + w.ackact().set_bit(); + w.cmd().bits(MASTER_ACT_STOP) + }); + } self.sync_sysop(); } + /// Send a REPEATED START condition. If the I2C is performing a read, will + /// also send a NACK to the slave. #[inline] - pub(super) fn cmd_stop(&mut self) { - self.issue_command(MASTER_ACT_STOP) + pub(super) fn cmd_repeated_start(&mut self) { + unsafe { + self.i2c_master().ctrlb().modify(|_, w| { + // set bit means send NACK + w.ackact().set_bit(); + w.cmd().bits(MASTER_ACT_REPEATED_START) + }); + } + self.sync_sysop(); } #[inline]