From 7081050310865bf88b87a420d870b58dd89b503e Mon Sep 17 00:00:00 2001 From: liebman Date: Tue, 23 Apr 2024 14:57:43 -0700 Subject: [PATCH 01/12] i2c: --- esp-hal/CHANGELOG.md | 2 + esp-hal/src/i2c.rs | 315 ++++++++++++++++++++++++++++--------------- 2 files changed, 209 insertions(+), 108 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 56a5607e73a..a027869e10d 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- i2c: implement `I2C:transaction` for `embedded-hal` and `embedded-hal-async` + ### Fixed - i2c: i2c1_handler used I2C0 register block by mistake (#1487) diff --git a/esp-hal/src/i2c.rs b/esp-hal/src/i2c.rs index 9eb407a2c5d..45734909589 100644 --- a/esp-hal/src/i2c.rs +++ b/esp-hal/src/i2c.rs @@ -83,6 +83,7 @@ impl embedded_hal::i2c::Error for Error { enum Command { Start, Stop, + End, Write { /// This bit is to set an expected ACK value for the transmitter. ack_exp: Ack, @@ -109,27 +110,28 @@ impl From for u16 { Command::Stop => Opcode::Stop, Command::Write { .. } => Opcode::Write, Command::Read { .. } => Opcode::Read, + Command::End => Opcode::End, }; let length = match c { - Command::Start | Command::Stop => 0, + Command::Start | Command::End | Command::Stop => 0, Command::Write { length: l, .. } | Command::Read { length: l, .. } => l, }; let ack_exp = match c { - Command::Start | Command::Stop | Command::Read { .. } => Ack::Nack, + Command::Start | Command::End | Command::Stop | Command::Read { .. } => Ack::Nack, Command::Write { ack_exp: exp, .. } => exp, }; let ack_check_en = match c { - Command::Start | Command::Stop | Command::Read { .. } => false, + Command::Start | Command::End | Command::Stop | Command::Read { .. } => false, Command::Write { ack_check_en: en, .. } => en, }; let ack_value = match c { - Command::Start | Command::Stop | Command::Write { .. } => Ack::Nack, + Command::Start | Command::End | Command::Stop | Command::Write { .. } => Ack::Nack, Command::Read { ack_value: ack, .. } => ack, }; @@ -176,6 +178,7 @@ enum Opcode { Write = 1, Read = 3, Stop = 2, + End = 4, // Check this for above chips!!! } #[cfg(any(esp32, esp32s2))] @@ -184,6 +187,7 @@ enum Opcode { Write = 1, Read = 2, Stop = 3, + End = 4, // Check this for above chips!!! } /// I2C peripheral container (I2C) @@ -270,29 +274,69 @@ impl embedded_hal::i2c::I2c for I2C<'_, T, DM> where T: Instance, { - fn read(&mut self, address: u8, buffer: &mut [u8]) -> Result<(), Self::Error> { - self.peripheral.master_read(address, buffer) - } - - fn write(&mut self, address: u8, bytes: &[u8]) -> Result<(), Self::Error> { - self.peripheral.master_write(address, bytes) - } - - fn write_read( + fn transaction( &mut self, address: u8, - bytes: &[u8], - buffer: &mut [u8], + operations: &mut [embedded_hal::i2c::Operation<'_>], ) -> Result<(), Self::Error> { - self.peripheral.master_write_read(address, bytes, buffer) - } + use embedded_hal::i2c::Operation; + #[derive(PartialEq)] + enum LastOp { + WasWrite, + WasRead, + WasNone, + } + let mut last_op = LastOp::WasNone; + let mut op_iter = operations.iter_mut().peekable(); + while let Some(op) = op_iter.next() { + // Clear all I2C interrupts + self.peripheral.clear_all_interrupts(); + // Reset FIFO and command list + self.peripheral.reset_fifo(); + self.peripheral.reset_command_list(); - fn transaction( - &mut self, - _address: u8, - _operations: &mut [embedded_hal::i2c::Operation<'_>], - ) -> Result<(), Self::Error> { - todo!() + // TODO somehow know that we can combine a write and a read into one transaction + let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); + match op { + Operation::Write(bytes) => { + // issue START/RSTART if op is different from previous + if last_op != LastOp::WasWrite { + add_cmd(cmd_iterator, Command::Start)?; + } + last_op = LastOp::WasWrite; + + self.peripheral.add_write(address, bytes, cmd_iterator)?; + if op_iter.peek().is_none() { + add_cmd(cmd_iterator, Command::Stop)?; + } else { + add_cmd(cmd_iterator, Command::End)?; + } + let index = self.peripheral.fill_tx_fifo(bytes); + self.peripheral.start_transmission(); + + // Fill the FIFO with the remaining bytes: + self.peripheral.write_remaining_tx_fifo(index, bytes)?; + self.peripheral.wait_for_completion()?; + } + Operation::Read(buffer) => { + // issue START/RSTART if op is different from previous + if last_op != LastOp::WasRead { + add_cmd(cmd_iterator, Command::Start)?; + } + last_op = LastOp::WasRead; + self.peripheral.add_read(address, buffer, cmd_iterator)?; + if op_iter.peek().is_none() { + add_cmd(cmd_iterator, Command::Stop)?; + } else { + add_cmd(cmd_iterator, Command::End)?; + } + self.peripheral.start_transmission(); + self.peripheral.read_all_from_fifo(buffer)?; + self.peripheral.wait_for_completion()?; + } + } + } + Ok(()) } } @@ -521,36 +565,6 @@ mod asynch { where T: Instance, { - async fn master_read(&mut self, addr: u8, buffer: &mut [u8]) -> Result<(), Error> { - // Reset FIFO and command list - self.peripheral.reset_fifo(); - self.peripheral.reset_command_list(); - - self.perform_read( - addr, - buffer, - &mut self.peripheral.register_block().comd_iter(), - ) - .await - } - - async fn perform_read<'a, I>( - &self, - addr: u8, - buffer: &mut [u8], - cmd_iterator: &mut I, - ) -> Result<(), Error> - where - I: Iterator, - { - self.peripheral.setup_read(addr, buffer, cmd_iterator)?; - self.peripheral.start_transmission(); - - self.read_all_from_fifo(buffer).await?; - self.wait_for_completion().await?; - - Ok(()) - } #[cfg(any(esp32, esp32s2))] async fn read_all_from_fifo(&self, buffer: &mut [u8]) -> Result<(), Error> { @@ -572,39 +586,6 @@ mod asynch { self.peripheral.read_all_from_fifo(buffer) } - async fn master_write(&mut self, addr: u8, bytes: &[u8]) -> Result<(), Error> { - // Reset FIFO and command list - self.peripheral.reset_fifo(); - self.peripheral.reset_command_list(); - - self.perform_write( - addr, - bytes, - &mut self.peripheral.register_block().comd_iter(), - ) - .await - } - - async fn perform_write<'a, I>( - &self, - addr: u8, - bytes: &[u8], - cmd_iterator: &mut I, - ) -> Result<(), Error> - where - I: Iterator, - { - self.peripheral.setup_write(addr, bytes, cmd_iterator)?; - let index = self.peripheral.fill_tx_fifo(bytes); - self.peripheral.start_transmission(); - - // Fill the FIFO with the remaining bytes: - self.write_remaining_tx_fifo(index, bytes).await?; - self.wait_for_completion().await?; - - Ok(()) - } - #[cfg(any(esp32, esp32s2))] async fn write_remaining_tx_fifo( &self, @@ -674,33 +655,69 @@ mod asynch { where T: Instance, { - async fn read(&mut self, address: u8, read: &mut [u8]) -> Result<(), Self::Error> { - self.master_read(address, read).await - } - - async fn write(&mut self, address: u8, write: &[u8]) -> Result<(), Self::Error> { - self.master_write(address, write).await - } - - async fn write_read( + async fn transaction( &mut self, address: u8, - write: &[u8], - read: &mut [u8], + operations: &mut [Operation<'_>], ) -> Result<(), Self::Error> { - self.master_write(address, write).await?; - self.master_read(address, read).await?; - + #[derive(PartialEq)] + enum LastOp { + WasWrite, + WasRead, + WasNone, + } + let mut last_op = LastOp::WasNone; + let mut op_iter = operations.iter_mut().peekable(); + while let Some(op) = op_iter.next() { + // Clear all I2C interrupts + self.peripheral.clear_all_interrupts(); + // Reset FIFO and command list + self.peripheral.reset_fifo(); + self.peripheral.reset_command_list(); + + // TODO somehow know that we can combine a write and a read into one transaction + let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); + match op { + Operation::Write(bytes) => { + // issue START/RSTART if op is different from previous + if last_op != LastOp::WasWrite { + add_cmd(cmd_iterator, Command::Start)?; + } + last_op = LastOp::WasWrite; + + self.peripheral.add_write(address, bytes, cmd_iterator)?; + if op_iter.peek().is_none() { + add_cmd(cmd_iterator, Command::Stop)?; + } else { + add_cmd(cmd_iterator, Command::End)?; + } + let index = self.peripheral.fill_tx_fifo(bytes); + self.peripheral.start_transmission(); + + // Fill the FIFO with the remaining bytes: + self.write_remaining_tx_fifo(index, bytes).await?; + self.wait_for_completion().await?; + } + Operation::Read(buffer) => { + // issue START/RSTART if op is different from previous + if last_op != LastOp::WasRead { + add_cmd(cmd_iterator, Command::Start)?; + } + last_op = LastOp::WasRead; + self.peripheral.add_read(address, buffer, cmd_iterator)?; + if op_iter.peek().is_none() { + add_cmd(cmd_iterator, Command::Stop)?; + } else { + add_cmd(cmd_iterator, Command::End)?; + } + self.peripheral.start_transmission(); + self.read_all_from_fifo(buffer).await?; + self.wait_for_completion().await?; + } + } + } Ok(()) } - - async fn transaction( - &mut self, - _address: u8, - _operations: &mut [Operation<'_>], - ) -> Result<(), Self::Error> { - todo!() - } } #[handler] @@ -1166,6 +1183,36 @@ pub trait Instance: crate::private::Sealed { } } + fn add_write<'a, I>(&self, addr: u8, bytes: &[u8], cmd_iterator: &mut I) -> Result<(), Error> + where + I: Iterator, + { + if bytes.len() > 254 { + // we could support more by adding multiple write operations + return Err(Error::ExceedingFifo); + } + + // WRITE command + add_cmd( + cmd_iterator, + Command::Write { + ack_exp: Ack::Ack, + ack_check_en: true, + length: 1 + bytes.len() as u8, + }, + )?; + + self.update_config(); + + // Load address and R/W bit into FIFO + write_fifo( + self.register_block(), + addr << 1 | OperationType::Write as u8, + ); + + Ok(()) + } + fn setup_write<'a, I>(&self, addr: u8, bytes: &[u8], cmd_iterator: &mut I) -> Result<(), Error> where I: Iterator, @@ -1224,6 +1271,58 @@ pub trait Instance: crate::private::Sealed { Ok(()) } + fn add_read<'a, I>( + &self, + addr: u8, + buffer: &mut [u8], + cmd_iterator: &mut I, + ) -> Result<(), Error> + where + I: Iterator, + { + if buffer.len() > 254 { + // we could support more by adding multiple read operations + return Err(Error::ExceedingFifo); + } + + // WRITE command + add_cmd( + cmd_iterator, + Command::Write { + ack_exp: Ack::Ack, + ack_check_en: true, + length: 1, + }, + )?; + + if buffer.len() > 1 { + // READ command (N - 1) + add_cmd( + cmd_iterator, + Command::Read { + ack_value: Ack::Ack, + length: buffer.len() as u8 - 1, + }, + )?; + } + + // READ w/o ACK + add_cmd( + cmd_iterator, + Command::Read { + ack_value: Ack::Nack, + length: 1, + }, + )?; + + self.update_config(); + + // Load address and R/W bit into FIFO + write_fifo(self.register_block(), addr << 1 | OperationType::Read as u8); + + Ok(()) + } + fn setup_read<'a, I>( &self, addr: u8, From af0b3022ce37ae6611f11204a4fb1a47f4f0aa46 Mon Sep 17 00:00:00 2001 From: liebman Date: Wed, 24 Apr 2024 13:57:44 -0700 Subject: [PATCH 02/12] i2c: refactor transaction() and reuse for master_read, master_write, and master_write_read --- esp-hal/src/i2c.rs | 409 +++++++++++++++++++++------------------------ 1 file changed, 191 insertions(+), 218 deletions(-) diff --git a/esp-hal/src/i2c.rs b/esp-hal/src/i2c.rs index 45734909589..f8dd5b00dfa 100644 --- a/esp-hal/src/i2c.rs +++ b/esp-hal/src/i2c.rs @@ -65,6 +65,17 @@ pub enum Error { CommandNrExceeded, } +#[cfg(any(feature = "embedded-hal", feature = "async"))] +#[derive(PartialEq)] +// This enum is used to keep track of the last operation that was performed +// in an embedded-hal(-async) I2C::transaction. It used to determine whether +// a START condition should be issued at the start of the current operation. +enum LastOpWas { + Write, + Read, + None, +} + #[cfg(feature = "embedded-hal")] impl embedded_hal::i2c::Error for Error { fn kind(&self) -> embedded_hal::i2c::ErrorKind { @@ -280,13 +291,7 @@ where operations: &mut [embedded_hal::i2c::Operation<'_>], ) -> Result<(), Self::Error> { use embedded_hal::i2c::Operation; - #[derive(PartialEq)] - enum LastOp { - WasWrite, - WasRead, - WasNone, - } - let mut last_op = LastOp::WasNone; + let mut last_op = LastOpWas::None; let mut op_iter = operations.iter_mut().peekable(); while let Some(op) = op_iter.next() { // Clear all I2C interrupts @@ -299,40 +304,30 @@ where let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); match op { Operation::Write(bytes) => { - // issue START/RSTART if op is different from previous - if last_op != LastOp::WasWrite { - add_cmd(cmd_iterator, Command::Start)?; - } - last_op = LastOp::WasWrite; - - self.peripheral.add_write(address, bytes, cmd_iterator)?; - if op_iter.peek().is_none() { - add_cmd(cmd_iterator, Command::Stop)?; - } else { - add_cmd(cmd_iterator, Command::End)?; - } - let index = self.peripheral.fill_tx_fifo(bytes); - self.peripheral.start_transmission(); - - // Fill the FIFO with the remaining bytes: - self.peripheral.write_remaining_tx_fifo(index, bytes)?; - self.peripheral.wait_for_completion()?; + // execute a write operation: + // - issue START/RSTART if op is different from previous + // - issue STOP if op is the last one + self.peripheral.write_operation( + address, + bytes, + last_op != LastOpWas::Write, + op_iter.peek().is_none(), + cmd_iterator, + )?; + last_op = LastOpWas::Write; } Operation::Read(buffer) => { - // issue START/RSTART if op is different from previous - if last_op != LastOp::WasRead { - add_cmd(cmd_iterator, Command::Start)?; - } - last_op = LastOp::WasRead; - self.peripheral.add_read(address, buffer, cmd_iterator)?; - if op_iter.peek().is_none() { - add_cmd(cmd_iterator, Command::Stop)?; - } else { - add_cmd(cmd_iterator, Command::End)?; - } - self.peripheral.start_transmission(); - self.peripheral.read_all_from_fifo(buffer)?; - self.peripheral.wait_for_completion()?; + // execute a read operation: + // - issue START/RSTART if op is different from previous + // - issue STOP if op is the last one + self.peripheral.read_operation( + address, + buffer, + last_op != LastOpWas::Read, + op_iter.peek().is_none(), + cmd_iterator, + )?; + last_op = LastOpWas::Read; } } } @@ -565,7 +560,6 @@ mod asynch { where T: Instance, { - #[cfg(any(esp32, esp32s2))] async fn read_all_from_fifo(&self, buffer: &mut [u8]) -> Result<(), Error> { if buffer.len() > 32 { @@ -649,6 +643,60 @@ mod asynch { Ok(()) } + + async fn write_operation<'a, I>( + &self, + address: u8, + bytes: &[u8], + start: bool, + stop: bool, + cmd_iterator: &mut I, + ) -> Result<(), Error> + where + I: Iterator, + { + if start { + add_cmd(cmd_iterator, Command::Start)?; + } + self.peripheral.setup_write(address, bytes, cmd_iterator)?; + add_cmd( + cmd_iterator, + if stop { Command::Stop } else { Command::End }, + )?; + let index = self.peripheral.fill_tx_fifo(bytes); + self.peripheral.start_transmission(); + + // Fill the FIFO with the remaining bytes: + self.write_remaining_tx_fifo(index, bytes).await?; + self.wait_for_completion().await?; + Ok(()) + } + + async fn read_operation<'a, I>( + &self, + address: u8, + buffer: &mut [u8], + start: bool, + stop: bool, + cmd_iterator: &mut I, + ) -> Result<(), Error> + where + I: Iterator, + { + if start { + add_cmd(cmd_iterator, Command::Start)?; + } + self.peripheral.setup_read(address, buffer, cmd_iterator)?; + add_cmd( + cmd_iterator, + if stop { Command::Stop } else { Command::End }, + )?; + self.peripheral.start_transmission(); + self.read_all_from_fifo(buffer).await?; + self.wait_for_completion().await?; + Ok(()) + } + } impl<'d, T> embedded_hal_async::i2c::I2c for I2C<'d, T, crate::Async> @@ -660,13 +708,7 @@ mod asynch { address: u8, operations: &mut [Operation<'_>], ) -> Result<(), Self::Error> { - #[derive(PartialEq)] - enum LastOp { - WasWrite, - WasRead, - WasNone, - } - let mut last_op = LastOp::WasNone; + let mut last_op = LastOpWas::None; let mut op_iter = operations.iter_mut().peekable(); while let Some(op) = op_iter.next() { // Clear all I2C interrupts @@ -679,40 +721,29 @@ mod asynch { let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); match op { Operation::Write(bytes) => { - // issue START/RSTART if op is different from previous - if last_op != LastOp::WasWrite { - add_cmd(cmd_iterator, Command::Start)?; - } - last_op = LastOp::WasWrite; - - self.peripheral.add_write(address, bytes, cmd_iterator)?; - if op_iter.peek().is_none() { - add_cmd(cmd_iterator, Command::Stop)?; - } else { - add_cmd(cmd_iterator, Command::End)?; - } - let index = self.peripheral.fill_tx_fifo(bytes); - self.peripheral.start_transmission(); - - // Fill the FIFO with the remaining bytes: - self.write_remaining_tx_fifo(index, bytes).await?; - self.wait_for_completion().await?; + self.write_operation( + address, + bytes, + last_op != LastOpWas::Write, + op_iter.peek().is_none(), + cmd_iterator, + ) + .await?; + last_op = LastOpWas::Write; } Operation::Read(buffer) => { - // issue START/RSTART if op is different from previous - if last_op != LastOp::WasRead { - add_cmd(cmd_iterator, Command::Start)?; - } - last_op = LastOp::WasRead; - self.peripheral.add_read(address, buffer, cmd_iterator)?; - if op_iter.peek().is_none() { - add_cmd(cmd_iterator, Command::Stop)?; - } else { - add_cmd(cmd_iterator, Command::End)?; - } - self.peripheral.start_transmission(); - self.read_all_from_fifo(buffer).await?; - self.wait_for_completion().await?; + // execute a read operation: + // - issue START/RSTART if op is different from previous + // - issue STOP if op is the last one + self.read_operation( + address, + buffer, + last_op != LastOpWas::Read, + op_iter.peek().is_none(), + cmd_iterator, + ) + .await?; + last_op = LastOpWas::Read; } } } @@ -1183,36 +1214,6 @@ pub trait Instance: crate::private::Sealed { } } - fn add_write<'a, I>(&self, addr: u8, bytes: &[u8], cmd_iterator: &mut I) -> Result<(), Error> - where - I: Iterator, - { - if bytes.len() > 254 { - // we could support more by adding multiple write operations - return Err(Error::ExceedingFifo); - } - - // WRITE command - add_cmd( - cmd_iterator, - Command::Write { - ack_exp: Ack::Ack, - ack_check_en: true, - length: 1 + bytes.len() as u8, - }, - )?; - - self.update_config(); - - // Load address and R/W bit into FIFO - write_fifo( - self.register_block(), - addr << 1 | OperationType::Write as u8, - ); - - Ok(()) - } - fn setup_write<'a, I>(&self, addr: u8, bytes: &[u8], cmd_iterator: &mut I) -> Result<(), Error> where I: Iterator, @@ -1222,12 +1223,6 @@ pub trait Instance: crate::private::Sealed { return Err(Error::ExceedingFifo); } - // Clear all I2C interrupts - self.clear_all_interrupts(); - - // RSTART command - add_cmd(cmd_iterator, Command::Start)?; - // WRITE command add_cmd( cmd_iterator, @@ -1238,8 +1233,6 @@ pub trait Instance: crate::private::Sealed { }, )?; - add_cmd(cmd_iterator, Command::Stop)?; - self.update_config(); // Load address and R/W bit into FIFO @@ -1251,78 +1244,6 @@ pub trait Instance: crate::private::Sealed { Ok(()) } - fn perform_write<'a, I>( - &self, - addr: u8, - bytes: &[u8], - cmd_iterator: &mut I, - ) -> Result<(), Error> - where - I: Iterator, - { - self.setup_write(addr, bytes, cmd_iterator)?; - let index = self.fill_tx_fifo(bytes); - self.start_transmission(); - - // Fill the FIFO with the remaining bytes: - self.write_remaining_tx_fifo(index, bytes)?; - self.wait_for_completion()?; - - Ok(()) - } - - fn add_read<'a, I>( - &self, - addr: u8, - buffer: &mut [u8], - cmd_iterator: &mut I, - ) -> Result<(), Error> - where - I: Iterator, - { - if buffer.len() > 254 { - // we could support more by adding multiple read operations - return Err(Error::ExceedingFifo); - } - - // WRITE command - add_cmd( - cmd_iterator, - Command::Write { - ack_exp: Ack::Ack, - ack_check_en: true, - length: 1, - }, - )?; - - if buffer.len() > 1 { - // READ command (N - 1) - add_cmd( - cmd_iterator, - Command::Read { - ack_value: Ack::Ack, - length: buffer.len() as u8 - 1, - }, - )?; - } - - // READ w/o ACK - add_cmd( - cmd_iterator, - Command::Read { - ack_value: Ack::Nack, - length: 1, - }, - )?; - - self.update_config(); - - // Load address and R/W bit into FIFO - write_fifo(self.register_block(), addr << 1 | OperationType::Read as u8); - - Ok(()) - } - fn setup_read<'a, I>( &self, addr: u8, @@ -1337,12 +1258,6 @@ pub trait Instance: crate::private::Sealed { return Err(Error::ExceedingFifo); } - // Clear all I2C interrupts - self.clear_all_interrupts(); - - // RSTART command - add_cmd(cmd_iterator, Command::Start)?; - // WRITE command add_cmd( cmd_iterator, @@ -1373,8 +1288,6 @@ pub trait Instance: crate::private::Sealed { }, )?; - add_cmd(cmd_iterator, Command::Stop)?; - self.update_config(); // Load address and R/W bit into FIFO @@ -1383,23 +1296,6 @@ pub trait Instance: crate::private::Sealed { Ok(()) } - fn perform_read<'a, I>( - &self, - addr: u8, - buffer: &mut [u8], - cmd_iterator: &mut I, - ) -> Result<(), Error> - where - I: Iterator, - { - self.setup_read(addr, buffer, cmd_iterator)?; - self.start_transmission(); - self.read_all_from_fifo(buffer)?; - self.wait_for_completion()?; - - Ok(()) - } - #[cfg(not(any(esp32, esp32s2)))] fn read_all_from_fifo(&self, buffer: &mut [u8]) -> Result<(), Error> { // Read bytes from FIFO @@ -1689,13 +1585,72 @@ pub trait Instance: crate::private::Sealed { .write(|w| w.rxfifo_full().clear_bit_by_one()); } + fn write_operation<'a, I>( + &self, + address: u8, + bytes: &[u8], + start: bool, + stop: bool, + cmd_iterator: &mut I, + ) -> Result<(), Error> + where + I: Iterator, + { + if start { + add_cmd(cmd_iterator, Command::Start)?; + } + self.setup_write(address, bytes, cmd_iterator)?; + add_cmd( + cmd_iterator, + if stop { Command::Stop } else { Command::End }, + )?; + let index = self.fill_tx_fifo(bytes); + self.start_transmission(); + + // Fill the FIFO with the remaining bytes: + self.write_remaining_tx_fifo(index, bytes)?; + self.wait_for_completion()?; + Ok(()) + } + + fn read_operation<'a, I>( + &self, + address: u8, + buffer: &mut [u8], + start: bool, + stop: bool, + cmd_iterator: &mut I, + ) -> Result<(), Error> + where + I: Iterator, + { + if start { + add_cmd(cmd_iterator, Command::Start)?; + } + self.setup_read(address, buffer, cmd_iterator)?; + add_cmd( + cmd_iterator, + if stop { Command::Stop } else { Command::End }, + )?; + self.start_transmission(); + self.read_all_from_fifo(buffer)?; + self.wait_for_completion()?; + Ok(()) + } + /// Send data bytes from the `bytes` array to a target slave with the /// address `addr` fn master_write(&mut self, addr: u8, bytes: &[u8]) -> Result<(), Error> { // Reset FIFO and command list self.reset_fifo(); self.reset_command_list(); - self.perform_write(addr, bytes, &mut self.register_block().comd_iter())?; + self.write_operation( + addr, + bytes, + true, + true, + &mut self.register_block().comd_iter(), + )?; Ok(()) } @@ -1706,7 +1661,13 @@ pub trait Instance: crate::private::Sealed { // Reset FIFO and command list self.reset_fifo(); self.reset_command_list(); - self.perform_read(addr, buffer, &mut self.register_block().comd_iter())?; + self.read_operation( + addr, + buffer, + true, + true, + &mut self.register_block().comd_iter(), + )?; Ok(()) } @@ -1722,8 +1683,20 @@ pub trait Instance: crate::private::Sealed { // in one transaction but filling the tx fifo with // the current code is somewhat slow even in release mode // which can cause issues - self.master_write(addr, bytes)?; - self.master_read(addr, buffer)?; + self.write_operation( + addr, + bytes, + true, + false, + &mut self.register_block().comd_iter(), + )?; + self.read_operation( + addr, + buffer, + true, + true, + &mut self.register_block().comd_iter(), + )?; Ok(()) } } From d4d3c32471eb5ad1f11ad9b45579e6c02d8443ef Mon Sep 17 00:00:00 2001 From: liebman Date: Wed, 24 Apr 2024 14:06:43 -0700 Subject: [PATCH 03/12] i2c: cargo fmt --- esp-hal/src/i2c.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/esp-hal/src/i2c.rs b/esp-hal/src/i2c.rs index f8dd5b00dfa..04ac85e9ba7 100644 --- a/esp-hal/src/i2c.rs +++ b/esp-hal/src/i2c.rs @@ -696,7 +696,6 @@ mod asynch { self.wait_for_completion().await?; Ok(()) } - } impl<'d, T> embedded_hal_async::i2c::I2c for I2C<'d, T, crate::Async> From aa97399c074920d6279ae5b04ed8ce5d45d8849b Mon Sep 17 00:00:00 2001 From: liebman Date: Fri, 26 Apr 2024 08:53:57 -0700 Subject: [PATCH 04/12] i2c: fix an issue with not clearing interrupt bits & move where we reset fifo and command_list --- esp-hal/src/i2c.rs | 44 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/esp-hal/src/i2c.rs b/esp-hal/src/i2c.rs index 04ac85e9ba7..8c016468989 100644 --- a/esp-hal/src/i2c.rs +++ b/esp-hal/src/i2c.rs @@ -296,9 +296,6 @@ where while let Some(op) = op_iter.next() { // Clear all I2C interrupts self.peripheral.clear_all_interrupts(); - // Reset FIFO and command list - self.peripheral.reset_fifo(); - self.peripheral.reset_command_list(); // TODO somehow know that we can combine a write and a read into one transaction let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); @@ -655,6 +652,9 @@ mod asynch { where I: Iterator, { + // Reset FIFO and command list + self.reset_fifo(); + self.reset_command_list(); if start { add_cmd(cmd_iterator, Command::Start)?; } @@ -683,6 +683,9 @@ mod asynch { where I: Iterator, { + // Reset FIFO and command list + self.reset_fifo(); + self.reset_command_list(); if start { add_cmd(cmd_iterator, Command::Start)?; } @@ -712,9 +715,6 @@ mod asynch { while let Some(op) = op_iter.next() { // Clear all I2C interrupts self.peripheral.clear_all_interrupts(); - // Reset FIFO and command list - self.peripheral.reset_fifo(); - self.peripheral.reset_command_list(); // TODO somehow know that we can combine a write and a read into one transaction let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); @@ -1354,8 +1354,17 @@ pub trait Instance: crate::private::Sealed { self.check_errors()?; // Handle completion cases - // A full transmission was completed + // A full transmission was completed (either a STOP condition or END was + // processed) if interrupts.trans_complete().bit_is_set() || interrupts.end_detect().bit_is_set() { + // clear the interrupt bits that were set + if interrupts.trans_complete().bit_is_set() { + #[rustfmt::skip] + self.register_block().int_clr().write(|w| w + .trans_complete().clear_bit_by_one() + .end_detect().clear_bit_by_one() + ); + } break; } } @@ -1595,6 +1604,10 @@ pub trait Instance: crate::private::Sealed { where I: Iterator, { + // Reset FIFO and command list + self.reset_fifo(); + self.reset_command_list(); + if start { add_cmd(cmd_iterator, Command::Start)?; } @@ -1623,6 +1636,10 @@ pub trait Instance: crate::private::Sealed { where I: Iterator, { + // Reset FIFO and command list + self.reset_fifo(); + self.reset_command_list(); + if start { add_cmd(cmd_iterator, Command::Start)?; } @@ -1640,9 +1657,8 @@ pub trait Instance: crate::private::Sealed { /// Send data bytes from the `bytes` array to a target slave with the /// address `addr` fn master_write(&mut self, addr: u8, bytes: &[u8]) -> Result<(), Error> { - // Reset FIFO and command list - self.reset_fifo(); - self.reset_command_list(); + // Clear all I2C interrupts + self.clear_all_interrupts(); self.write_operation( addr, bytes, @@ -1657,9 +1673,8 @@ pub trait Instance: crate::private::Sealed { /// The number of read bytes is deterimed by the size of the `buffer` /// argument fn master_read(&mut self, addr: u8, buffer: &mut [u8]) -> Result<(), Error> { - // Reset FIFO and command list - self.reset_fifo(); - self.reset_command_list(); + // Clear all I2C interrupts + self.clear_all_interrupts(); self.read_operation( addr, buffer, @@ -1682,6 +1697,9 @@ pub trait Instance: crate::private::Sealed { // in one transaction but filling the tx fifo with // the current code is somewhat slow even in release mode // which can cause issues + + // Clear all I2C interrupts + self.clear_all_interrupts(); self.write_operation( addr, bytes, From bd0fd8bd75e07dc1debf7073b6d56f4ace1a2c26 Mon Sep 17 00:00:00 2001 From: liebman Date: Fri, 26 Apr 2024 10:13:14 -0700 Subject: [PATCH 05/12] i2c: fix async compile error --- esp-hal/src/i2c.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/esp-hal/src/i2c.rs b/esp-hal/src/i2c.rs index 8c016468989..98fcd9884a4 100644 --- a/esp-hal/src/i2c.rs +++ b/esp-hal/src/i2c.rs @@ -653,8 +653,8 @@ mod asynch { I: Iterator, { // Reset FIFO and command list - self.reset_fifo(); - self.reset_command_list(); + self.peripheral.reset_fifo(); + self.peripheral.reset_command_list(); if start { add_cmd(cmd_iterator, Command::Start)?; } @@ -684,8 +684,8 @@ mod asynch { I: Iterator, { // Reset FIFO and command list - self.reset_fifo(); - self.reset_command_list(); + self.peripheral.reset_fifo(); + self.peripheral.reset_command_list(); if start { add_cmd(cmd_iterator, Command::Start)?; } From 2664f10574415de34f80c846bc5dce507c572ceb Mon Sep 17 00:00:00 2001 From: liebman Date: Fri, 26 Apr 2024 13:59:47 -0700 Subject: [PATCH 06/12] i2c: fix for esp32 & esp32s2 --- esp-hal/src/i2c.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/esp-hal/src/i2c.rs b/esp-hal/src/i2c.rs index 98fcd9884a4..db32b259e67 100644 --- a/esp-hal/src/i2c.rs +++ b/esp-hal/src/i2c.rs @@ -1357,14 +1357,6 @@ pub trait Instance: crate::private::Sealed { // A full transmission was completed (either a STOP condition or END was // processed) if interrupts.trans_complete().bit_is_set() || interrupts.end_detect().bit_is_set() { - // clear the interrupt bits that were set - if interrupts.trans_complete().bit_is_set() { - #[rustfmt::skip] - self.register_block().int_clr().write(|w| w - .trans_complete().clear_bit_by_one() - .end_detect().clear_bit_by_one() - ); - } break; } } @@ -1707,6 +1699,7 @@ pub trait Instance: crate::private::Sealed { false, &mut self.register_block().comd_iter(), )?; + self.clear_all_interrupts(); self.read_operation( addr, buffer, From 45d4b784866df6ff8cb74ab343fff1f187a0f86f Mon Sep 17 00:00:00 2001 From: liebman Date: Sat, 27 Apr 2024 12:03:19 -0700 Subject: [PATCH 07/12] i2c: real fix for esp32 (End command never gets cmd_done bit set!) --- esp-hal/src/i2c.rs | 163 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 151 insertions(+), 12 deletions(-) diff --git a/esp-hal/src/i2c.rs b/esp-hal/src/i2c.rs index db32b259e67..65b51b3eedf 100644 --- a/esp-hal/src/i2c.rs +++ b/esp-hal/src/i2c.rs @@ -90,7 +90,34 @@ impl embedded_hal::i2c::Error for Error { } } +// This should really be defined in the PAC, but the PAC only +// defines the "command" field as a 16-bit field :-( +bitfield::bitfield! { + struct CommandReg(u32); + cmd_done, _: 31; + from into Opcode, opcode, set_opcode: 13, 11; + from into Ack, ack_value, set_ack_value: 10, 10; + from into Ack, ack_exp, set_ack_exp: 9, 9; + ack_check_en, set_ack_check_en: 8, 8; + length, set_length: 7, 0; +} + +#[cfg(feature = "debug")] +impl core::fmt::Debug for CommandReg { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("CommandReg") + .field("cmd_done", &self.cmd_done()) + .field("opcode", &self.opcode()) + .field("ack_value", &self.ack_value()) + .field("ack_exp", &self.ack_exp()) + .field("ack_check_en", &self.ack_check_en()) + .field("length", &self.length()) + .finish() + } +} + /// A generic I2C Command +#[cfg_attr(feature = "debug", derive(Debug))] enum Command { Start, Stop, @@ -118,10 +145,10 @@ impl From for u16 { fn from(c: Command) -> u16 { let opcode = match c { Command::Start => Opcode::RStart, + Command::End => Opcode::End, Command::Stop => Opcode::Stop, Command::Write { .. } => Opcode::Write, Command::Read { .. } => Opcode::Read, - Command::End => Opcode::End, }; let length = match c { @@ -145,7 +172,6 @@ impl From for u16 { Command::Start | Command::End | Command::Stop | Command::Write { .. } => Ack::Nack, Command::Read { ack_value: ack, .. } => ack, }; - let mut cmd: u16 = length.into(); if ack_check_en { @@ -178,12 +204,29 @@ enum OperationType { } #[derive(Eq, PartialEq, Copy, Clone)] +#[cfg_attr(feature = "debug", derive(Debug))] enum Ack { - Ack, - Nack, + Ack = 0, + Nack = 1, +} +impl From for Ack { + fn from(ack: u32) -> Self { + match ack { + 0 => Ack::Ack, + 1 => Ack::Nack, + _ => unreachable!(), + } + } +} +impl From for u32 { + fn from(ack: Ack) -> u32 { + ack as u32 + } } #[cfg(any(esp32c2, esp32c3, esp32c6, esp32h2, esp32s3))] +#[cfg_attr(feature = "debug", derive(Debug))] +#[derive(PartialEq)] enum Opcode { RStart = 6, Write = 1, @@ -192,7 +235,36 @@ enum Opcode { End = 4, // Check this for above chips!!! } +#[cfg(all(feature = "debug", any(esp32c2, esp32c3, esp32c6, esp32h2, esp32s3)))] +impl From for Opcode { + fn from(opcode: u8) -> Self { + match opcode { + 6 => Opcode::RStart, + 1 => Opcode::Write, + 3 => Opcode::Read, + 2 => Opcode::Stop, + 4 => Opcode::End, + _ => unreachable!(), + } + } +} +#[cfg(any(esp32c2, esp32c3, esp32c6, esp32h2, esp32s3))] +impl From for Opcode { + fn from(opcode: u32) -> Self { + match opcode { + 6 => Opcode::RStart, + 1 => Opcode::Write, + 3 => Opcode::Read, + 2 => Opcode::Stop, + 4 => Opcode::End, + _ => unreachable!(), + } + } +} + #[cfg(any(esp32, esp32s2))] +#[cfg_attr(feature = "debug", derive(Debug))] +#[derive(PartialEq)] enum Opcode { RStart = 0, Write = 1, @@ -201,6 +273,38 @@ enum Opcode { End = 4, // Check this for above chips!!! } +#[cfg(all(feature = "debug", any(esp32, esp32s2)))] +impl From for Opcode { + fn from(opcode: u8) -> Self { + match opcode { + 0 => Opcode::RStart, + 1 => Opcode::Write, + 2 => Opcode::Read, + 3 => Opcode::Stop, + 4 => Opcode::End, + _ => unreachable!(), + } + } +} + +#[cfg(any(esp32, esp32s2))] +impl From for Opcode { + fn from(opcode: u32) -> Self { + match opcode { + 0 => Opcode::RStart, + 1 => Opcode::Write, + 2 => Opcode::Read, + 3 => Opcode::Stop, + 4 => Opcode::End, + _ => unreachable!(), + } + } +} +impl From for u32 { + fn from(opcode: Opcode) -> u32 { + opcode as u32 + } +} /// I2C peripheral container (I2C) pub struct I2C<'d, T, DM: crate::Mode> { peripheral: PeripheralRef<'d, T>, @@ -1329,7 +1433,7 @@ pub trait Instance: crate::private::Sealed { // wait for completion - then we can just read the data from FIFO // once we change to non-fifo mode to support larger transfers that // won't work anymore - self.wait_for_completion()?; + self.wait_for_completion(false)?; // Read bytes from FIFO // FIXME: Handle case where less data has been provided by the slave than @@ -1347,7 +1451,9 @@ pub trait Instance: crate::private::Sealed { .write(|w| unsafe { w.bits(I2C_LL_INTR_MASK) }); } - fn wait_for_completion(&self) -> Result<(), Error> { + fn wait_for_completion(&self, end_only: bool) -> Result<(), Error> { + #[cfg(feature = "log")] + log::trace!("wait_for_completion: end_only={}", end_only); loop { let interrupts = self.register_block().int_raw().read(); @@ -1356,12 +1462,20 @@ pub trait Instance: crate::private::Sealed { // Handle completion cases // A full transmission was completed (either a STOP condition or END was // processed) - if interrupts.trans_complete().bit_is_set() || interrupts.end_detect().bit_is_set() { + if (!end_only && interrupts.trans_complete().bit_is_set()) || interrupts.end_detect().bit_is_set() { + #[cfg(all(feature = "log", feature = "debug"))] + log::trace!("wait_for_completion: interrupts: {:?}", interrupts); break; } } - for cmd in self.register_block().comd_iter() { - if cmd.read().command().bits() != 0x0 && cmd.read().command_done().bit_is_clear() { + // NOTE: on esp32 executing the end command generates the end_detect interrupt + // but does not seem to clear the done bit! So we don't the done status + // of an end command + for cmd_reg in self.register_block().comd_iter() { + let cmd = CommandReg(cmd_reg.read().bits()); + if cmd.0 != 0x0 && cmd.opcode() != Opcode::End && !cmd.cmd_done() { + #[cfg(all(feature = "log", feature = "debug"))] + self.log_comd("wait_for_completion"); return Err(Error::ExecIncomplete); } } @@ -1585,6 +1699,24 @@ pub trait Instance: crate::private::Sealed { .write(|w| w.rxfifo_full().clear_bit_by_one()); } + #[cfg(all(feature = "log", feature = "debug"))] + fn log_comd(&self, msg: &str) { + log::info!("{}: {:?}", msg, self.register_block().int_raw().read()); + for cmd in self.register_block().comd_iter() { + let command = CommandReg(cmd.read().bits()); + log::info!("{}: {:?}", msg, command); + match command.opcode() { + Opcode::Stop => { + break; + } + Opcode::End => { + break; + } + _ => {} + } + } + } + fn write_operation<'a, I>( &self, address: u8, @@ -1596,6 +1728,8 @@ pub trait Instance: crate::private::Sealed { where I: Iterator, { + #[cfg(feature = "log")] + log::trace!("write_operation: addr: {:0x}, bytes: {:0x?}, start: {:?}, stop: {:?}", address, bytes, start, stop); // Reset FIFO and command list self.reset_fifo(); self.reset_command_list(); @@ -1608,12 +1742,13 @@ pub trait Instance: crate::private::Sealed { cmd_iterator, if stop { Command::Stop } else { Command::End }, )?; + //self.log_comd("write_operation"); let index = self.fill_tx_fifo(bytes); self.start_transmission(); // Fill the FIFO with the remaining bytes: self.write_remaining_tx_fifo(index, bytes)?; - self.wait_for_completion()?; + self.wait_for_completion(!stop)?; Ok(()) } @@ -1628,6 +1763,8 @@ pub trait Instance: crate::private::Sealed { where I: Iterator, { + #[cfg(feature = "log")] + log::trace!("read_operation: addr: {:0x}, read_len: {:?}, start: {:?}, stop: {:?}", address, buffer.len(), start, stop); // Reset FIFO and command list self.reset_fifo(); self.reset_command_list(); @@ -1642,7 +1779,7 @@ pub trait Instance: crate::private::Sealed { )?; self.start_transmission(); self.read_all_from_fifo(buffer)?; - self.wait_for_completion()?; + self.wait_for_completion(!stop)?; Ok(()) } @@ -1685,6 +1822,8 @@ pub trait Instance: crate::private::Sealed { bytes: &[u8], buffer: &mut [u8], ) -> Result<(), Error> { + #[cfg(feature = "log")] + log::trace!("master_write_read: addr: {:0x}, bytes: {:0x?}, read_len: {:?}", addr, bytes, buffer.len()); // it would be possible to combine the write and read // in one transaction but filling the tx fifo with // the current code is somewhat slow even in release mode @@ -1698,7 +1837,7 @@ pub trait Instance: crate::private::Sealed { true, false, &mut self.register_block().comd_iter(), - )?; + ).unwrap(); self.clear_all_interrupts(); self.read_operation( addr, From cb6bb64c5f02bc32d9ae5bec6e30d35261b74420 Mon Sep 17 00:00:00 2001 From: liebman Date: Sat, 27 Apr 2024 12:07:55 -0700 Subject: [PATCH 08/12] i2c: fmt and removal of an unwrap() that I was using while debugging --- esp-hal/src/i2c.rs | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/esp-hal/src/i2c.rs b/esp-hal/src/i2c.rs index 65b51b3eedf..86d7e34f52f 100644 --- a/esp-hal/src/i2c.rs +++ b/esp-hal/src/i2c.rs @@ -206,7 +206,7 @@ enum OperationType { #[derive(Eq, PartialEq, Copy, Clone)] #[cfg_attr(feature = "debug", derive(Debug))] enum Ack { - Ack = 0, + Ack = 0, Nack = 1, } impl From for Ack { @@ -1462,7 +1462,9 @@ pub trait Instance: crate::private::Sealed { // Handle completion cases // A full transmission was completed (either a STOP condition or END was // processed) - if (!end_only && interrupts.trans_complete().bit_is_set()) || interrupts.end_detect().bit_is_set() { + if (!end_only && interrupts.trans_complete().bit_is_set()) + || interrupts.end_detect().bit_is_set() + { #[cfg(all(feature = "log", feature = "debug"))] log::trace!("wait_for_completion: interrupts: {:?}", interrupts); break; @@ -1729,7 +1731,13 @@ pub trait Instance: crate::private::Sealed { I: Iterator, { #[cfg(feature = "log")] - log::trace!("write_operation: addr: {:0x}, bytes: {:0x?}, start: {:?}, stop: {:?}", address, bytes, start, stop); + log::trace!( + "write_operation: addr: {:0x}, bytes: {:0x?}, start: {:?}, stop: {:?}", + address, + bytes, + start, + stop + ); // Reset FIFO and command list self.reset_fifo(); self.reset_command_list(); @@ -1742,7 +1750,7 @@ pub trait Instance: crate::private::Sealed { cmd_iterator, if stop { Command::Stop } else { Command::End }, )?; - //self.log_comd("write_operation"); + // self.log_comd("write_operation"); let index = self.fill_tx_fifo(bytes); self.start_transmission(); @@ -1764,7 +1772,13 @@ pub trait Instance: crate::private::Sealed { I: Iterator, { #[cfg(feature = "log")] - log::trace!("read_operation: addr: {:0x}, read_len: {:?}, start: {:?}, stop: {:?}", address, buffer.len(), start, stop); + log::trace!( + "read_operation: addr: {:0x}, read_len: {:?}, start: {:?}, stop: {:?}", + address, + buffer.len(), + start, + stop + ); // Reset FIFO and command list self.reset_fifo(); self.reset_command_list(); @@ -1823,7 +1837,12 @@ pub trait Instance: crate::private::Sealed { buffer: &mut [u8], ) -> Result<(), Error> { #[cfg(feature = "log")] - log::trace!("master_write_read: addr: {:0x}, bytes: {:0x?}, read_len: {:?}", addr, bytes, buffer.len()); + log::trace!( + "master_write_read: addr: {:0x}, bytes: {:0x?}, read_len: {:?}", + addr, + bytes, + buffer.len() + ); // it would be possible to combine the write and read // in one transaction but filling the tx fifo with // the current code is somewhat slow even in release mode @@ -1837,7 +1856,7 @@ pub trait Instance: crate::private::Sealed { true, false, &mut self.register_block().comd_iter(), - ).unwrap(); + )?; self.clear_all_interrupts(); self.read_operation( addr, From 6cb4ca775dfe3a5eb51ec9a396035506fcefd81c Mon Sep 17 00:00:00 2001 From: liebman Date: Sat, 27 Apr 2024 13:29:12 -0700 Subject: [PATCH 09/12] i2c: only define opcode values in one place i2c: use CommandReg in add_cmd --- esp-hal/src/i2c.rs | 206 +++++++++++++++++++-------------------------- 1 file changed, 88 insertions(+), 118 deletions(-) diff --git a/esp-hal/src/i2c.rs b/esp-hal/src/i2c.rs index 86d7e34f52f..eb2fe634c60 100644 --- a/esp-hal/src/i2c.rs +++ b/esp-hal/src/i2c.rs @@ -23,6 +23,7 @@ //! io.pins.gpio2, //! 100.kHz(), //! &clocks, +//! None, //! ); //! loop { //! let mut data = [0u8; 22]; @@ -98,10 +99,51 @@ bitfield::bitfield! { from into Opcode, opcode, set_opcode: 13, 11; from into Ack, ack_value, set_ack_value: 10, 10; from into Ack, ack_exp, set_ack_exp: 9, 9; - ack_check_en, set_ack_check_en: 8, 8; + ack_check_en, set_ack_check_en: 8; length, set_length: 7, 0; } +impl CommandReg { + fn bits(&self) -> u32 { + self.0 + } + + fn new_start() -> Self { + let mut cmd = Self(0); + cmd.set_opcode(Opcode::RStart); + cmd + } + + fn new_end() -> Self { + let mut cmd = Self(0); + cmd.set_opcode(Opcode::End); + cmd + } + + fn new_stop() -> Self { + let mut cmd = Self(0); + cmd.set_opcode(Opcode::Stop); + cmd + } + + fn new_write(ack_exp: Ack, ack_check_en: bool, length: u8) -> Self { + let mut cmd = Self(0); + cmd.set_opcode(Opcode::Write); + cmd.set_ack_exp(ack_exp); + cmd.set_ack_check_en(ack_check_en); + cmd.set_length(length as u32); + cmd + } + + fn new_read(ack_value: Ack, length: u8) -> Self { + let mut cmd = Self(0); + cmd.set_opcode(Opcode::Read); + cmd.set_ack_value(ack_value); + cmd.set_length(length as u32); + cmd + } +} + #[cfg(feature = "debug")] impl core::fmt::Debug for CommandReg { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { @@ -141,60 +183,19 @@ enum Command { }, } -impl From for u16 { - fn from(c: Command) -> u16 { - let opcode = match c { - Command::Start => Opcode::RStart, - Command::End => Opcode::End, - Command::Stop => Opcode::Stop, - Command::Write { .. } => Opcode::Write, - Command::Read { .. } => Opcode::Read, - }; - - let length = match c { - Command::Start | Command::End | Command::Stop => 0, - Command::Write { length: l, .. } | Command::Read { length: l, .. } => l, - }; - - let ack_exp = match c { - Command::Start | Command::End | Command::Stop | Command::Read { .. } => Ack::Nack, - Command::Write { ack_exp: exp, .. } => exp, - }; - - let ack_check_en = match c { - Command::Start | Command::End | Command::Stop | Command::Read { .. } => false, +impl From for CommandReg { + fn from(c: Command) -> Self { + match c { + Command::Start => CommandReg::new_start(), + Command::End => CommandReg::new_end(), + Command::Stop => CommandReg::new_stop(), Command::Write { - ack_check_en: en, .. - } => en, - }; - - let ack_value = match c { - Command::Start | Command::End | Command::Stop | Command::Write { .. } => Ack::Nack, - Command::Read { ack_value: ack, .. } => ack, - }; - let mut cmd: u16 = length.into(); - - if ack_check_en { - cmd |= 1 << 8; - } else { - cmd &= !(1 << 8); - } - - if ack_exp == Ack::Nack { - cmd |= 1 << 9; - } else { - cmd &= !(1 << 9); - } - - if ack_value == Ack::Nack { - cmd |= 1 << 10; - } else { - cmd &= !(1 << 10); + ack_exp, + ack_check_en, + length, + } => CommandReg::new_write(ack_exp, ack_check_en, length), + Command::Read { ack_value, length } => CommandReg::new_read(ack_value, length), } - - cmd |= (opcode as u16) << 11; - - cmd } } @@ -224,87 +225,55 @@ impl From for u32 { } } -#[cfg(any(esp32c2, esp32c3, esp32c6, esp32h2, esp32s3))] -#[cfg_attr(feature = "debug", derive(Debug))] -#[derive(PartialEq)] -enum Opcode { - RStart = 6, - Write = 1, - Read = 3, - Stop = 2, - End = 4, // Check this for above chips!!! -} - -#[cfg(all(feature = "debug", any(esp32c2, esp32c3, esp32c6, esp32h2, esp32s3)))] -impl From for Opcode { - fn from(opcode: u8) -> Self { - match opcode { - 6 => Opcode::RStart, - 1 => Opcode::Write, - 3 => Opcode::Read, - 2 => Opcode::Stop, - 4 => Opcode::End, - _ => unreachable!(), - } - } -} -#[cfg(any(esp32c2, esp32c3, esp32c6, esp32h2, esp32s3))] -impl From for Opcode { - fn from(opcode: u32) -> Self { - match opcode { - 6 => Opcode::RStart, - 1 => Opcode::Write, - 3 => Opcode::Read, - 2 => Opcode::Stop, - 4 => Opcode::End, - _ => unreachable!(), - } +cfg_if::cfg_if! { + if #[cfg(any(esp32, esp32s2))] { + const OPCODE_RSTART: u32 = 0; + const OPCODE_WRITE: u32 = 1; + const OPCODE_READ: u32 = 2; + const OPCODE_STOP: u32 = 3; + const OPCODE_END: u32 = 4; + } else if #[cfg(any(esp32c2, esp32c3, esp32c6, esp32h2, esp32s3))] { + const OPCODE_RSTART: u32 = 6; + const OPCODE_WRITE: u32 = 1; + const OPCODE_READ: u32 = 3; + const OPCODE_STOP: u32 = 2; + const OPCODE_END: u32 = 4; } } - -#[cfg(any(esp32, esp32s2))] #[cfg_attr(feature = "debug", derive(Debug))] #[derive(PartialEq)] enum Opcode { - RStart = 0, - Write = 1, - Read = 2, - Stop = 3, - End = 4, // Check this for above chips!!! + RStart, + Write, + Read, + Stop, + End, } -#[cfg(all(feature = "debug", any(esp32, esp32s2)))] -impl From for Opcode { - fn from(opcode: u8) -> Self { +impl From for u32 { + fn from(opcode: Opcode) -> u32 { match opcode { - 0 => Opcode::RStart, - 1 => Opcode::Write, - 2 => Opcode::Read, - 3 => Opcode::Stop, - 4 => Opcode::End, - _ => unreachable!(), + Opcode::RStart => OPCODE_RSTART, + Opcode::Write => OPCODE_WRITE, + Opcode::Read => OPCODE_READ, + Opcode::Stop => OPCODE_STOP, + Opcode::End => OPCODE_END, } } } - -#[cfg(any(esp32, esp32s2))] impl From for Opcode { fn from(opcode: u32) -> Self { match opcode { - 0 => Opcode::RStart, - 1 => Opcode::Write, - 2 => Opcode::Read, - 3 => Opcode::Stop, - 4 => Opcode::End, + OPCODE_RSTART => Opcode::RStart, + OPCODE_WRITE => Opcode::Write, + OPCODE_READ => Opcode::Read, + OPCODE_STOP => Opcode::Stop, + OPCODE_END => Opcode::End, _ => unreachable!(), } } } -impl From for u32 { - fn from(opcode: Opcode) -> u32 { - opcode as u32 - } -} + /// I2C peripheral container (I2C) pub struct I2C<'d, T, DM: crate::Mode> { peripheral: PeripheralRef<'d, T>, @@ -1475,7 +1444,7 @@ pub trait Instance: crate::private::Sealed { // of an end command for cmd_reg in self.register_block().comd_iter() { let cmd = CommandReg(cmd_reg.read().bits()); - if cmd.0 != 0x0 && cmd.opcode() != Opcode::End && !cmd.cmd_done() { + if cmd.bits() != 0x0 && cmd.opcode() != Opcode::End && !cmd.cmd_done() { #[cfg(all(feature = "log", feature = "debug"))] self.log_comd("wait_for_completion"); return Err(Error::ExecIncomplete); @@ -1874,7 +1843,8 @@ where I: Iterator, { let cmd = cmd_iterator.next().ok_or(Error::CommandNrExceeded)?; - cmd.write(|w| unsafe { w.command().bits(command.into()) }); + let cmd_reg: CommandReg = command.into(); + cmd.write(|w| unsafe { w.bits(cmd_reg.bits()) }); Ok(()) } From 08dbf671a2e99be253b1ffb7c12315845705a721 Mon Sep 17 00:00:00 2001 From: liebman Date: Sun, 28 Apr 2024 08:37:50 -0700 Subject: [PATCH 10/12] i2c: async direct & embedded_hal support working --- esp-hal/src/i2c.rs | 190 +++++++++++------- .../embassy_i2c_bmp180_calibration_data.rs | 71 +++++++ 2 files changed, 187 insertions(+), 74 deletions(-) create mode 100644 examples/src/bin/embassy_i2c_bmp180_calibration_data.rs diff --git a/esp-hal/src/i2c.rs b/esp-hal/src/i2c.rs index eb2fe634c60..8d055d5626b 100644 --- a/esp-hal/src/i2c.rs +++ b/esp-hal/src/i2c.rs @@ -280,10 +280,9 @@ pub struct I2C<'d, T, DM: crate::Mode> { phantom: PhantomData, } -impl I2C<'_, T, DM> +impl I2C<'_, T, crate::Blocking> where T: Instance, - DM: crate::Mode, { /// Reads enough bytes from slave with `address` to fill `buffer` pub fn read(&mut self, address: u8, buffer: &mut [u8]) -> Result<(), Error> { @@ -308,31 +307,31 @@ where } #[cfg(feature = "embedded-hal-02")] -impl embedded_hal_02::blocking::i2c::Read for I2C<'_, T, DM> +impl embedded_hal_02::blocking::i2c::Read for I2C<'_, T, crate::Blocking> where T: Instance, { type Error = Error; fn read(&mut self, address: u8, buffer: &mut [u8]) -> Result<(), Self::Error> { - self.read(address, buffer) + self.peripheral.master_read(address, buffer) } } #[cfg(feature = "embedded-hal-02")] -impl embedded_hal_02::blocking::i2c::Write for I2C<'_, T, DM> +impl embedded_hal_02::blocking::i2c::Write for I2C<'_, T, crate::Blocking> where T: Instance, { type Error = Error; fn write(&mut self, addr: u8, bytes: &[u8]) -> Result<(), Self::Error> { - self.write(addr, bytes) + self.peripheral.master_write(addr, bytes) } } #[cfg(feature = "embedded-hal-02")] -impl embedded_hal_02::blocking::i2c::WriteRead for I2C<'_, T, DM> +impl embedded_hal_02::blocking::i2c::WriteRead for I2C<'_, T, crate::Blocking> where T: Instance, { @@ -344,7 +343,7 @@ where bytes: &[u8], buffer: &mut [u8], ) -> Result<(), Self::Error> { - self.write_read(address, bytes, buffer) + self.peripheral.master_write_read(address, bytes, buffer) } } @@ -636,7 +635,7 @@ mod asynch { panic!("On ESP32 and ESP32-S2 the max I2C read is limited to 32 bytes"); } - self.wait_for_completion().await?; + self.wait_for_completion(false).await?; for byte in buffer.iter_mut() { *byte = read_fifo(self.peripheral.register_block()); @@ -696,20 +695,19 @@ mod asynch { } } - async fn wait_for_completion(&self) -> Result<(), Error> { + async fn wait_for_completion(&self, end_only: bool) -> Result<(), Error> { self.peripheral.check_errors()?; - select( - I2cFuture::new(Event::TxComplete, self.inner()), - I2cFuture::new(Event::EndDetect, self.inner()), - ) - .await; - - for cmd in self.peripheral.register_block().comd_iter() { - if cmd.read().command().bits() != 0x0 && cmd.read().command_done().bit_is_clear() { - return Err(Error::ExecIncomplete); - } + if end_only { + I2cFuture::new(Event::EndDetect, self.inner()).await; + } else { + select( + I2cFuture::new(Event::TxComplete, self.inner()), + I2cFuture::new(Event::EndDetect, self.inner()), + ) + .await; } + self.peripheral.check_all_commands_done()?; Ok(()) } @@ -741,7 +739,7 @@ mod asynch { // Fill the FIFO with the remaining bytes: self.write_remaining_tx_fifo(index, bytes).await?; - self.wait_for_completion().await?; + self.wait_for_completion(!stop).await?; Ok(()) } @@ -769,11 +767,100 @@ mod asynch { )?; self.peripheral.start_transmission(); self.read_all_from_fifo(buffer).await?; - self.wait_for_completion().await?; + self.wait_for_completion(!stop).await?; + Ok(()) + } + + /// Send data bytes from the `bytes` array to a target slave with the + /// address `addr` + async fn master_write(&mut self, addr: u8, bytes: &[u8]) -> Result<(), Error> { + // Clear all I2C interrupts + self.peripheral.clear_all_interrupts(); + self.write_operation( + addr, + bytes, + true, + true, + &mut self.peripheral.register_block().comd_iter(), + ).await?; + Ok(()) + } + + /// Read bytes from a target slave with the address `addr` + /// The number of read bytes is deterimed by the size of the `buffer` + /// argument + async fn master_read(&mut self, addr: u8, buffer: &mut [u8]) -> Result<(), Error> { + // Clear all I2C interrupts + self.peripheral.clear_all_interrupts(); + self.read_operation( + addr, + buffer, + true, + true, + &mut self.peripheral.register_block().comd_iter(), + ).await?; + Ok(()) + } + + /// Write bytes from the `bytes` array first and then read n bytes into + /// the `buffer` array with n being the size of the array. + async fn master_write_read( + &mut self, + addr: u8, + bytes: &[u8], + buffer: &mut [u8], + ) -> Result<(), Error> { + // it would be possible to combine the write and read + // in one transaction but filling the tx fifo with + // the current code is somewhat slow even in release mode + // which can cause issues + + // Clear all I2C interrupts + self.peripheral.clear_all_interrupts(); + self.write_operation( + addr, + bytes, + true, + false, + &mut self.peripheral.register_block().comd_iter(), + ).await?; + self.peripheral.clear_all_interrupts(); + self.read_operation( + addr, + buffer, + true, + true, + &mut self.peripheral.register_block().comd_iter(), + ).await?; + Ok(()) + } + + /// Writes bytes to slave with address `address` + pub async fn write(&mut self, addr: u8, bytes: &[u8]) -> Result<(), Error> { + self.master_write(addr, bytes).await?; + Ok(()) + } + + /// Reads enough bytes from slave with `address` to fill `buffer` + pub async fn read(&mut self, addr: u8, buffer: &mut [u8]) -> Result<(), Error> { + self.master_read(addr, buffer).await?; + Ok(()) + } + + /// Writes bytes to slave with address `address` and then reads enough bytes + /// to fill `buffer` *in a single transaction* + pub async fn write_read( + &mut self, + addr: u8, + bytes: &[u8], + buffer: &mut [u8], + ) -> Result<(), Error> { + self.master_write_read(addr, bytes, buffer).await?; Ok(()) } } + #[cfg(feature = "embedded-hal")] impl<'d, T> embedded_hal_async::i2c::I2c for I2C<'d, T, crate::Async> where T: Instance, @@ -789,7 +876,6 @@ mod asynch { // Clear all I2C interrupts self.peripheral.clear_all_interrupts(); - // TODO somehow know that we can combine a write and a read into one transaction let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); match op { Operation::Write(bytes) => { @@ -1421,8 +1507,6 @@ pub trait Instance: crate::private::Sealed { } fn wait_for_completion(&self, end_only: bool) -> Result<(), Error> { - #[cfg(feature = "log")] - log::trace!("wait_for_completion: end_only={}", end_only); loop { let interrupts = self.register_block().int_raw().read(); @@ -1434,26 +1518,26 @@ pub trait Instance: crate::private::Sealed { if (!end_only && interrupts.trans_complete().bit_is_set()) || interrupts.end_detect().bit_is_set() { - #[cfg(all(feature = "log", feature = "debug"))] - log::trace!("wait_for_completion: interrupts: {:?}", interrupts); break; } } + self.check_all_commands_done()?; + Ok(()) + } + + fn check_all_commands_done(&self) -> Result<(), Error> { // NOTE: on esp32 executing the end command generates the end_detect interrupt - // but does not seem to clear the done bit! So we don't the done status - // of an end command + // but does not seem to clear the done bit! So we don't check the done + // status of an end command for cmd_reg in self.register_block().comd_iter() { let cmd = CommandReg(cmd_reg.read().bits()); if cmd.bits() != 0x0 && cmd.opcode() != Opcode::End && !cmd.cmd_done() { - #[cfg(all(feature = "log", feature = "debug"))] - self.log_comd("wait_for_completion"); return Err(Error::ExecIncomplete); } } Ok(()) } - fn check_errors(&self) -> Result<(), Error> { let interrupts = self.register_block().int_raw().read(); @@ -1670,24 +1754,6 @@ pub trait Instance: crate::private::Sealed { .write(|w| w.rxfifo_full().clear_bit_by_one()); } - #[cfg(all(feature = "log", feature = "debug"))] - fn log_comd(&self, msg: &str) { - log::info!("{}: {:?}", msg, self.register_block().int_raw().read()); - for cmd in self.register_block().comd_iter() { - let command = CommandReg(cmd.read().bits()); - log::info!("{}: {:?}", msg, command); - match command.opcode() { - Opcode::Stop => { - break; - } - Opcode::End => { - break; - } - _ => {} - } - } - } - fn write_operation<'a, I>( &self, address: u8, @@ -1699,14 +1765,6 @@ pub trait Instance: crate::private::Sealed { where I: Iterator, { - #[cfg(feature = "log")] - log::trace!( - "write_operation: addr: {:0x}, bytes: {:0x?}, start: {:?}, stop: {:?}", - address, - bytes, - start, - stop - ); // Reset FIFO and command list self.reset_fifo(); self.reset_command_list(); @@ -1719,7 +1777,6 @@ pub trait Instance: crate::private::Sealed { cmd_iterator, if stop { Command::Stop } else { Command::End }, )?; - // self.log_comd("write_operation"); let index = self.fill_tx_fifo(bytes); self.start_transmission(); @@ -1740,14 +1797,6 @@ pub trait Instance: crate::private::Sealed { where I: Iterator, { - #[cfg(feature = "log")] - log::trace!( - "read_operation: addr: {:0x}, read_len: {:?}, start: {:?}, stop: {:?}", - address, - buffer.len(), - start, - stop - ); // Reset FIFO and command list self.reset_fifo(); self.reset_command_list(); @@ -1805,13 +1854,6 @@ pub trait Instance: crate::private::Sealed { bytes: &[u8], buffer: &mut [u8], ) -> Result<(), Error> { - #[cfg(feature = "log")] - log::trace!( - "master_write_read: addr: {:0x}, bytes: {:0x?}, read_len: {:?}", - addr, - bytes, - buffer.len() - ); // it would be possible to combine the write and read // in one transaction but filling the tx fifo with // the current code is somewhat slow even in release mode diff --git a/examples/src/bin/embassy_i2c_bmp180_calibration_data.rs b/examples/src/bin/embassy_i2c_bmp180_calibration_data.rs new file mode 100644 index 00000000000..4464bc26a44 --- /dev/null +++ b/examples/src/bin/embassy_i2c_bmp180_calibration_data.rs @@ -0,0 +1,71 @@ +//! Embassy "async" vesrion of ead calibration data from BMP180 sensor +//! +//! This example dumps the calibration data from a BMP180 sensor by reading by reading +//! with the direct I2C API and the embedded-hal-async I2C API. +//! +//! //! Folowing pins are used: +//! SDA GPIO4 +//! SCL GPIO5 +//! +//! Depending on your target and the board you are using you have to change the +//! pins. +//! + +//% CHIPS: esp32 esp32c2 esp32c3 esp32c6 esp32h2 esp32s2 esp32s3 +//% FEATURES: async embassy embassy-executor-thread embassy-time-timg0 embassy-generic-timers + +#![no_std] +#![no_main] +#![feature(type_alias_impl_trait)] + +use embassy_executor::Spawner; +use embassy_time::{Duration, Timer}; +use esp_backtrace as _; +use esp_hal::{ + clock::ClockControl, + embassy::{self}, + gpio::Io, + i2c::I2C, + peripherals::Peripherals, + prelude::*, + system::SystemControl, + timer::TimerGroup, +}; + +#[main] +async fn main(_spawner: Spawner) { + let peripherals = Peripherals::take(); + let system = SystemControl::new(peripherals.SYSTEM); + let clocks = ClockControl::boot_defaults(system.clock_control).freeze(); + + let timg0 = TimerGroup::new_async(peripherals.TIMG0, &clocks); + embassy::init(&clocks, timg0); + + let io = Io::new(peripherals.GPIO, peripherals.IO_MUX); + + let mut i2c = I2C::new_async( + peripherals.I2C0, + io.pins.gpio4, + io.pins.gpio5, + 400.kHz(), + &clocks, + ); + + loop { + let mut data = [0u8; 22]; + i2c.write_read(0x77, &[0xaa], &mut data).await.unwrap(); + esp_println::println!("direct: {:02x?}", data); + read_data(&mut i2c).await; + Timer::after(Duration::from_millis(1000)).await; + } +} + +async fn read_data(i2c: &mut I2C) +where + I2C: embedded_hal_async::i2c::I2c, +{ + let mut data = [0u8; 22]; + i2c.write_read(0x77, &[0xaa], &mut data).await.unwrap(); + + esp_println::println!("embedded_hal: {:02x?}", data); +} \ No newline at end of file From ed8468824a591fbb7d7c037e075c999cc0c4abc3 Mon Sep 17 00:00:00 2001 From: liebman Date: Sun, 28 Apr 2024 08:47:29 -0700 Subject: [PATCH 11/12] i2c: cargo fmt --- esp-hal/src/i2c.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/esp-hal/src/i2c.rs b/esp-hal/src/i2c.rs index 8d055d5626b..97a2fc75b71 100644 --- a/esp-hal/src/i2c.rs +++ b/esp-hal/src/i2c.rs @@ -782,7 +782,8 @@ mod asynch { true, true, &mut self.peripheral.register_block().comd_iter(), - ).await?; + ) + .await?; Ok(()) } @@ -798,7 +799,8 @@ mod asynch { true, true, &mut self.peripheral.register_block().comd_iter(), - ).await?; + ) + .await?; Ok(()) } @@ -823,7 +825,8 @@ mod asynch { true, false, &mut self.peripheral.register_block().comd_iter(), - ).await?; + ) + .await?; self.peripheral.clear_all_interrupts(); self.read_operation( addr, @@ -831,7 +834,8 @@ mod asynch { true, true, &mut self.peripheral.register_block().comd_iter(), - ).await?; + ) + .await?; Ok(()) } @@ -847,8 +851,8 @@ mod asynch { Ok(()) } - /// Writes bytes to slave with address `address` and then reads enough bytes - /// to fill `buffer` *in a single transaction* + /// Writes bytes to slave with address `address` and then reads enough + /// bytes to fill `buffer` *in a single transaction* pub async fn write_read( &mut self, addr: u8, From 398435b28de4528a94cf94cd28d934631e06677a Mon Sep 17 00:00:00 2001 From: liebman Date: Sun, 28 Apr 2024 08:55:58 -0700 Subject: [PATCH 12/12] examples: cargo fmt --- examples/src/bin/embassy_i2c_bmp180_calibration_data.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/src/bin/embassy_i2c_bmp180_calibration_data.rs b/examples/src/bin/embassy_i2c_bmp180_calibration_data.rs index 4464bc26a44..83a208216f7 100644 --- a/examples/src/bin/embassy_i2c_bmp180_calibration_data.rs +++ b/examples/src/bin/embassy_i2c_bmp180_calibration_data.rs @@ -68,4 +68,4 @@ where i2c.write_read(0x77, &[0xaa], &mut data).await.unwrap(); esp_println::println!("embedded_hal: {:02x?}", data); -} \ No newline at end of file +}