From e6b42677f1c4aad3659573ca221d9f14096c880a Mon Sep 17 00:00:00 2001 From: bjoernQ Date: Wed, 26 Oct 2022 16:47:43 +0200 Subject: [PATCH 1/2] I2C Driver Refactoring --- esp-hal-common/src/i2c.rs | 632 ++++++++++++------ .../examples/i2c_bmp180_calibration_data.rs | 59 ++ .../examples/i2c_bmp180_calibration_data.rs | 60 ++ esp32c3-hal/build.rs | 2 +- .../examples/i2c_bmp180_calibration_data.rs | 63 ++ esp32c3-hal/src/lib.rs | 7 +- .../examples/i2c_bmp180_calibration_data.rs | 59 ++ .../examples/i2c_bmp180_calibration_data.rs | 59 ++ 8 files changed, 733 insertions(+), 208 deletions(-) create mode 100644 esp32-hal/examples/i2c_bmp180_calibration_data.rs create mode 100644 esp32c2-hal/examples/i2c_bmp180_calibration_data.rs create mode 100644 esp32c3-hal/examples/i2c_bmp180_calibration_data.rs create mode 100644 esp32s2-hal/examples/i2c_bmp180_calibration_data.rs create mode 100644 esp32s3-hal/examples/i2c_bmp180_calibration_data.rs diff --git a/esp-hal-common/src/i2c.rs b/esp-hal-common/src/i2c.rs index e71e0a5d122..e2320b08709 100644 --- a/esp-hal-common/src/i2c.rs +++ b/esp-hal-common/src/i2c.rs @@ -482,8 +482,8 @@ pub trait Instance { } let scl_low = half_cycle; - let setup = half_cycle; - let hold = half_cycle; + let setup = half_cycle - 1; + let hold = half_cycle - 1; #[cfg(not(esp32))] let scl_low = scl_low as u16 - 1; @@ -600,28 +600,280 @@ pub trait Instance { Ok(()) } - /// Start the actual transmission on a previously configured command set - /// - /// This includes the monitoring of the execution in the peripheral and the - /// return of the operation outcome, including error states - fn execute_transmission(&mut self) -> Result<(), Error> { + fn perform_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); + } + // Clear all I2C interrupts - self.register_block() - .int_clr - .write(|w| unsafe { w.bits(I2C_LL_INTR_MASK) }); + self.clear_all_interrupts(); - // Ensure that the configuration of the peripheral is correctly propagated - // (only necessary for C3 and S3 variant) - #[cfg(any(esp32c2, esp32c3, esp32s3))] - self.register_block() - .ctr - .modify(|_, w| w.conf_upgate().set_bit()); + // RSTART command + add_cmd(cmd_iterator, Command::Start)?; - // Start transmission + // WRITE command + add_cmd( + cmd_iterator, + Command::Write { + ack_exp: Ack::Ack, + ack_check_en: true, + length: 1 + bytes.len() as u8, + }, + )?; + + add_cmd(cmd_iterator, Command::Stop)?; + + self.upgate_config(); + + // Load address and R/W bit into FIFO + write_fifo( + self.register_block(), + addr << 1 | OperationType::Write as u8, + ); + + let index = self.fill_tx_fifo(bytes); + + self.start_transmission(); + + // fill FIFO with remaining bytes + self.write_remaining_tx_fifo(index, bytes); + + self.wait_for_completion()?; + + Ok(()) + } + + fn perform_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); + } + + // Clear all I2C interrupts + self.clear_all_interrupts(); + + // RSTART command + add_cmd(cmd_iterator, Command::Start)?; + + // 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, + }, + )?; + + add_cmd(cmd_iterator, Command::Stop)?; + + self.upgate_config(); + + // Load address and R/W bit into FIFO + write_fifo(self.register_block(), addr << 1 | OperationType::Read as u8); + + self.start_transmission(); + + self.read_all_from_fifo(buffer)?; + + self.wait_for_completion()?; + + Ok(()) + } + + #[cfg(not(debug_assertions))] + fn perform_write_read<'a, I>( + &self, + addr: u8, + bytes: &[u8], + buffer: &mut [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); + } + + // Clear all I2C interrupts + self.clear_all_interrupts(); + + // RSTART command + add_cmd(cmd_iterator, Command::Start)?; + + // WRITE command + add_cmd( + cmd_iterator, + Command::Write { + ack_exp: Ack::Ack, + ack_check_en: true, + length: 1 + bytes.len() as u8, + }, + )?; + + add_cmd(cmd_iterator, Command::Start)?; + + 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, + }, + )?; + + add_cmd(cmd_iterator, Command::Stop)?; + + self.upgate_config(); + + // Load address and R/W bit into FIFO + write_fifo( + self.register_block(), + addr << 1 | OperationType::Write as u8, + ); + + let index = self.fill_tx_fifo(bytes); + + self.start_transmission(); + + // fill FIFO with remaining bytes + self.write_remaining_tx_fifo(index, bytes); + + // writing to the FIFO here is too slow in debug mode apparently and the + // address isn't loaded fast enough ... therefore in debug mode we do separate + // write and reads + self.write_remaining_tx_fifo(0, &[addr << 1 | OperationType::Read as u8]); + + 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 + // FIXME: Handle case where less data has been provided by the slave than + // requested? Or is this prevented from a protocol perspective? + for byte in buffer.iter_mut() { + loop { + #[cfg(not(esp32))] + { + let reg = self.register_block().fifo_st.read(); + + if reg.rxfifo_raddr().bits() != reg.rxfifo_waddr().bits() { + break; + } + } + + #[cfg(esp32)] + { + let reg = self.register_block().rxfifo_st.read(); + + if reg.rxfifo_start_addr().bits() != reg.rxfifo_end_addr().bits() { + break; + } + } + } + + *byte = read_fifo(self.register_block()); + } + + Ok(()) + } + + #[cfg(any(esp32, esp32s2))] + fn read_all_from_fifo(&self, buffer: &mut [u8]) -> Result<(), Error> { + // on ESP32/ESP32-S2 we currently don't support I2C transactions larger than the + // FIFO apparently it would be possible by using non-fifo mode + // see https://github.com/espressif/arduino-esp32/blob/7e9afe8c5ed7b5bf29624a5cd6e07d431c027b97/cores/esp32/esp32-hal-i2c.c#L615 + + if buffer.len() > 32 { + panic!("On ESP32 and ESP32-S2 the max I2C read is limited to 32 bytes"); + } + + // 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()?; + + // Read bytes from FIFO + // FIXME: Handle case where less data has been provided by the slave than + // requested? Or is this prevented from a protocol perspective? + for byte in buffer.iter_mut() { + *byte = read_fifo(self.register_block()); + } + + Ok(()) + } + + fn clear_all_interrupts(&self) { self.register_block() - .ctr - .modify(|_, w| w.trans_start().set_bit()); + .int_clr + .write(|w| unsafe { w.bits(I2C_LL_INTR_MASK) }); + } + fn wait_for_completion(&self) -> Result<(), Error> { loop { let interrupts = self.register_block().int_raw.read(); @@ -658,8 +910,6 @@ pub trait Instance { break; } } - - // Confirm that all commands that were configured were actually executed for cmd in self.register_block().comd.iter() { if cmd.read().command().bits() != 0x0 && cmd.read().command_done().bit_is_clear() { return Err(Error::ExecIncomplete); @@ -669,144 +919,162 @@ pub trait Instance { Ok(()) } - fn add_write_operation<'a, I>( - &self, - addr: u8, - bytes: &[u8], - cmd_iterator: &mut I, - include_stop: bool, - ) -> Result<(), Error> - where - I: Iterator, - { - // Check if we have another cmd register ready, otherwise return appropriate - // error - let cmd_start = cmd_iterator.next().ok_or(Error::CommandNrExceeded)?; - // RSTART command - cmd_start.write(|w| unsafe { w.command().bits(Command::Start.into()) }); + fn upgate_config(&self) { + // Ensure that the configuration of the peripheral is correctly propagated + // (only necessary for C3 and S3 variant) + #[cfg(any(esp32c2, esp32c3, esp32s3))] + self.register_block() + .ctr + .modify(|_, w| w.conf_upgate().set_bit()); + } - // Load address and R/W bit into FIFO - write_fifo( - self.register_block(), - addr << 1 | OperationType::Write as u8, - ); - // Load actual data bytes - for byte in bytes { - write_fifo(self.register_block(), *byte); + fn start_transmission(&self) { + // Start transmission + self.register_block() + .ctr + .modify(|_, w| w.trans_start().set_bit()); + } + + #[cfg(not(any(esp32, esp32s2)))] + fn fill_tx_fifo(&self, bytes: &[u8]) -> usize { + let mut index = 0; + while index < bytes.len() + && !self + .register_block() + .int_raw + .read() + .txfifo_ovf_int_raw() + .bit_is_set() + { + write_fifo(self.register_block(), bytes[index]); + index += 1; } + if self + .register_block() + .int_raw + .read() + .txfifo_ovf_int_raw() + .bit_is_set() + { + index -= 1; + self.register_block() + .int_clr + .write(|w| w.txfifo_ovf_int_clr().set_bit()); + } + index + } - // Check if we have another cmd register ready, otherwise return appropriate - // error - let cmd_write = cmd_iterator.next().ok_or(Error::CommandNrExceeded)?; - // WRITE command - cmd_write.write(|w| unsafe { - w.command().bits( - Command::Write { - ack_exp: Ack::Ack, - ack_check_en: true, - length: 1 + bytes.len() as u8, - } - .into(), - ) - }); + #[cfg(not(any(esp32, esp32s2)))] + fn write_remaining_tx_fifo(&self, start_index: usize, bytes: &[u8]) { + let mut index = start_index; + loop { + while !self + .register_block() + .int_raw + .read() + .txfifo_wm_int_raw() + .bit_is_set() + {} + self.register_block() + .int_clr + .write(|w| w.txfifo_wm_int_clr().set_bit()); - if include_stop { - // Check if we have another cmd register ready, otherwise return appropriate - // error - let cmd_stop = cmd_iterator.next().ok_or(Error::CommandNrExceeded)?; - // STOP command - cmd_stop.write(|w| unsafe { w.command().bits(Command::Stop.into()) }); - } + if index >= bytes.len() { + break; + } - Ok(()) + write_fifo(self.register_block(), bytes[index]); + index += 1; + } } - fn add_read_operation<'a, I>( - &self, - addr: u8, - buffer: &[u8], - cmd_iterator: &mut I, - ) -> Result<(), Error> - where - I: Iterator, - { - // Check if we have another cmd register ready, otherwise return appropriate - // error - cmd_iterator - .next() - .ok_or(Error::CommandNrExceeded)? - .write(|w| unsafe { w.command().bits(Command::Start.into()) }); - - // Load address and R/W bit into FIFO - write_fifo(self.register_block(), addr << 1 | OperationType::Read as u8); + #[cfg(any(esp32, esp32s2))] + fn fill_tx_fifo(&self, bytes: &[u8]) -> usize { + // on ESP32/ESP32-S2 we currently don't support I2C transactions larger than the + // FIFO apparently it would be possible by using non-fifo mode + // see https://github.com/espressif/arduino-esp32/blob/7e9afe8c5ed7b5bf29624a5cd6e07d431c027b97/cores/esp32/esp32-hal-i2c.c#L615 - // Check if we have another cmd register ready, otherwise return appropriate - // error - cmd_iterator - .next() - .ok_or(Error::CommandNrExceeded)? - .write(|w| unsafe { - w.command().bits( - Command::Write { - ack_exp: Ack::Ack, - ack_check_en: true, - length: 1, - } - .into(), - ) - }); + if bytes.len() > 31 { + panic!("On ESP32 and ESP32-S2 the max I2C transfer is limited to 31 bytes"); + } - // For reading bytes prior to the last read byte we need to - // configure the ack - if buffer.len() > 1 { - // READ command for first n - 1 bytes - cmd_iterator - .next() - .ok_or(Error::CommandNrExceeded)? - .write(|w| unsafe { - w.command().bits( - Command::Read { - ack_value: Ack::Ack, - length: buffer.len() as u8 - 1, - } - .into(), - ) - }); + for b in bytes { + write_fifo(self.register_block(), *b); } - // READ command for (last or only) byte - cmd_iterator - .next() - .ok_or(Error::CommandNrExceeded)? - .write(|w| unsafe { - w.command().bits( - Command::Read { - ack_value: Ack::Nack, - length: 1, - } - .into(), - ) - }); + bytes.len() + } - // Check if we have another cmd register ready, otherwise return appropriate - // error - cmd_iterator - .next() - .ok_or(Error::CommandNrExceeded)? - .write(|w| unsafe { w.command().bits(Command::Stop.into()) }); + #[cfg(any(esp32, esp32s2))] + fn write_remaining_tx_fifo(&self, start_index: usize, bytes: &[u8]) { + // on ESP32/ESP32-S2 we currently don't support I2C transactions larger than the + // FIFO apparently it would be possible by using non-fifo mode + // see https://github.com/espressif/arduino-esp32/blob/7e9afe8c5ed7b5bf29624a5cd6e07d431c027b97/cores/esp32/esp32-hal-i2c.c#L615 - Ok(()) + if start_index >= bytes.len() { + return; + } + + // this is only possible when writing the I2C address in release mode + // from [perform_write_read] + for b in bytes { + write_fifo(self.register_block(), *b); + } } /// Resets the transmit and receive FIFO buffers + #[cfg(not(esp32))] fn reset_fifo(&mut self) { // First, reset the fifo buffers + self.register_block().fifo_conf.modify(|_, w| { + w.tx_fifo_rst() + .set_bit() + .rx_fifo_rst() + .set_bit() + .nonfifo_en() + .clear_bit() + .fifo_prt_en() + .set_bit() + .tx_fifo_rst() + .clear_bit() + .rx_fifo_rst() + .clear_bit() + .rxfifo_wm_thrhd() + .variant(1) + .txfifo_wm_thrhd() + .variant(8) + }); + self.register_block().int_clr.write(|w| { + w.rxfifo_wm_int_clr() + .set_bit() + .txfifo_wm_int_clr() + .set_bit() + }); + } + + /// Resets the transmit and receive FIFO buffers + #[cfg(esp32)] + fn reset_fifo(&mut self) { + // First, reset the fifo buffers + self.register_block().fifo_conf.modify(|_, w| { + w.tx_fifo_rst() + .set_bit() + .rx_fifo_rst() + .set_bit() + .nonfifo_en() + .clear_bit() + .tx_fifo_rst() + .clear_bit() + .rx_fifo_rst() + .clear_bit() + .nonfifo_rx_thres() + .variant(1) + .nonfifo_tx_thres() + .variant(32) + }); self.register_block() - .fifo_conf - .modify(|_, w| w.tx_fifo_rst().set_bit().rx_fifo_rst().set_bit()); - self.register_block() - .fifo_conf - .modify(|_, w| w.tx_fifo_rst().clear_bit().rx_fifo_rst().clear_bit()); + .int_clr + .write(|w| w.rxfifo_full_int_clr().set_bit()); } /// Send data bytes from the `bytes` array to a target slave with the @@ -815,19 +1083,7 @@ pub trait Instance { // Reset FIFO and command list self.reset_fifo(); self.reset_command_list(); - - // Split the potentially larger `bytes` array into chunks of (at most) 31 - // entries Together with the addr/access byte at the beginning of every - // transmission, this is the maximum size that we can store in the - // (default config) TX FIFO - for chunk in bytes.chunks(31) { - // Add write operation - self.add_write_operation(addr, chunk, &mut self.register_block().comd.iter(), true)?; - - // Start transmission - self.execute_transmission()? - } - + self.perform_write(addr, bytes, &mut self.register_block().comd.iter())?; Ok(()) } @@ -835,32 +1091,10 @@ pub trait Instance { /// 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> { - // If the buffer size is > 32 bytes, this signals the - // intent to read more than that number of bytes, which we - // cannot achieve at this point in time - // TODO: Handle the case where we transfer an amount of data that is exceeding - // the FIFO size (i.e. > 32 bytes?) - if buffer.len() > 31 { - return Err(Error::ExceedingFifo); - } - // Reset FIFO and command list self.reset_fifo(); self.reset_command_list(); - - // Add write operation - self.add_read_operation(addr, buffer, &mut self.register_block().comd.iter())?; - - // Start transmission - self.execute_transmission()?; - - // Read bytes from FIFO - // FIXME: Handle case where less data has been provided by the slave than - // requested? Or is this prevented from a protocol perspective? - for byte in buffer.iter_mut() { - *byte = read_fifo(self.register_block()); - } - + self.perform_read(addr, buffer, &mut self.register_block().comd.iter())?; Ok(()) } @@ -872,39 +1106,33 @@ pub trait Instance { bytes: &[u8], buffer: &mut [u8], ) -> Result<(), Error> { - // If the buffer or bytes size is > 32 bytes, this signals the - // intent to read/write more than that number of bytes, which we - // cannot achieve at this point in time - // TODO: Handle the case where we transfer an amount of data that is exceeding - // the FIFO size (i.e. > 32 bytes?) - if buffer.len() > 31 || bytes.len() > 31 { - return Err(Error::ExceedingFifo); - } + #[cfg(not(debug_assertions))] + { + // Reset FIFO and command list + self.reset_fifo(); + self.reset_command_list(); - // Reset FIFO and command list - self.reset_fifo(); - self.reset_command_list(); - - let mut cmd_iterator = self.register_block().comd.iter(); - - // Add write operation - self.add_write_operation(addr, bytes, &mut cmd_iterator, false)?; - - // Add read operation (which appends commands to the existing command list) - self.add_read_operation(addr, buffer, &mut cmd_iterator)?; - - // Start transmission - self.execute_transmission()?; - - // Read bytes from FIFO - for byte in buffer.iter_mut() { - *byte = read_fifo(self.register_block()); + self.perform_write_read(addr, bytes, buffer, &mut self.register_block().comd.iter())?; } + #[cfg(debug_assertions)] + { + self.master_write(addr, bytes)?; + self.master_read(addr, buffer)?; + } Ok(()) } } +fn add_cmd<'a, I>(cmd_iterator: &mut I, command: Command) -> Result<(), Error> +where + I: Iterator, +{ + let cmd = cmd_iterator.next().ok_or(Error::CommandNrExceeded)?; + cmd.write(|w| unsafe { w.command().bits(command.into()) }); + Ok(()) +} + #[cfg(not(any(esp32, esp32s2)))] fn read_fifo(register_block: &RegisterBlock) -> u8 { register_block.data.read().fifo_rdata().bits() diff --git a/esp32-hal/examples/i2c_bmp180_calibration_data.rs b/esp32-hal/examples/i2c_bmp180_calibration_data.rs new file mode 100644 index 00000000000..44a18881aa5 --- /dev/null +++ b/esp32-hal/examples/i2c_bmp180_calibration_data.rs @@ -0,0 +1,59 @@ +//! Read calibration data from BMP180 sensor +//! +//! This example dumps the calibration data from a BMP180 sensor +//! +//! The following wiring is assumed: +//! - SDA => GPIO32 +//! - SCL => GPIO33 + +#![no_std] +#![no_main] + +use esp32_hal::{ + clock::ClockControl, + gpio::IO, + i2c::I2C, + pac::Peripherals, + prelude::*, + timer::TimerGroup, + Rtc, +}; +use esp_backtrace as _; +use esp_println::println; +use xtensa_lx_rt::entry; + +#[entry] +fn main() -> ! { + let peripherals = Peripherals::take().unwrap(); + let mut system = peripherals.DPORT.split(); + let clocks = ClockControl::boot_defaults(system.clock_control).freeze(); + + let timer_group0 = TimerGroup::new(peripherals.TIMG0, &clocks); + let mut wdt = timer_group0.wdt; + let mut rtc = Rtc::new(peripherals.RTC_CNTL); + + // Disable watchdog timer + wdt.disable(); + rtc.rwdt.disable(); + + let io = IO::new(peripherals.GPIO, peripherals.IO_MUX); + + // Create a new peripheral object with the described wiring + // and standard I2C clock speed + let mut i2c = I2C::new( + peripherals.I2C0, + io.pins.gpio32, + io.pins.gpio33, + 100u32.kHz(), + &mut system.peripheral_clock_control, + &clocks, + ) + .unwrap(); + + loop { + let mut data = [0u8; 22]; + i2c.write_read(0x77, &[0xaa], &mut data).ok(); + + println!("{:02x?}", data); + } +} diff --git a/esp32c2-hal/examples/i2c_bmp180_calibration_data.rs b/esp32c2-hal/examples/i2c_bmp180_calibration_data.rs new file mode 100644 index 00000000000..66be768569d --- /dev/null +++ b/esp32c2-hal/examples/i2c_bmp180_calibration_data.rs @@ -0,0 +1,60 @@ +//! Read calibration data from BMP180 sensor +//! +//! This example dumps the calibration data from a BMP180 sensor +//! +//! The following wiring is assumed: +//! - SDA => GPIO1 +//! - SCL => GPIO2 + +#![no_std] +#![no_main] + +use esp32c2_hal::{ + clock::ClockControl, + gpio::IO, + i2c::I2C, + pac::Peripherals, + prelude::*, + timer::TimerGroup, + Rtc, +}; +use esp_backtrace as _; +use esp_println::println; +use riscv_rt::entry; + +#[entry] +fn main() -> ! { + let peripherals = Peripherals::take().unwrap(); + let mut system = peripherals.SYSTEM.split(); + let clocks = ClockControl::boot_defaults(system.clock_control).freeze(); + + let mut rtc = Rtc::new(peripherals.RTC_CNTL); + let timer_group0 = TimerGroup::new(peripherals.TIMG0, &clocks); + let mut wdt0 = timer_group0.wdt; + + // Disable watchdog timers + rtc.swd.disable(); + rtc.rwdt.disable(); + wdt0.disable(); + + let io = IO::new(peripherals.GPIO, peripherals.IO_MUX); + + // Create a new peripheral object with the described wiring + // and standard I2C clock speed + let mut i2c = I2C::new( + peripherals.I2C0, + io.pins.gpio1, + io.pins.gpio2, + 100u32.kHz(), + &mut system.peripheral_clock_control, + &clocks, + ) + .unwrap(); + + loop { + let mut data = [0u8; 22]; + i2c.write_read(0x77, &[0xaa], &mut data).ok(); + + println!("{:02x?}", data); + } +} diff --git a/esp32c3-hal/build.rs b/esp32c3-hal/build.rs index 8bfe0d5ee00..f0f3ad133c9 100644 --- a/esp32c3-hal/build.rs +++ b/esp32c3-hal/build.rs @@ -52,7 +52,7 @@ fn main() { add_defaults(); } -#[cfg(not(any(feature = "mcu-boot",feature = "direct-boot")))] +#[cfg(not(any(feature = "mcu-boot", feature = "direct-boot")))] fn main() { check_opt_level(); diff --git a/esp32c3-hal/examples/i2c_bmp180_calibration_data.rs b/esp32c3-hal/examples/i2c_bmp180_calibration_data.rs new file mode 100644 index 00000000000..a0d451e4162 --- /dev/null +++ b/esp32c3-hal/examples/i2c_bmp180_calibration_data.rs @@ -0,0 +1,63 @@ +//! Read calibration data from BMP180 sensor +//! +//! This example dumps the calibration data from a BMP180 sensor +//! +//! The following wiring is assumed: +//! - SDA => GPIO1 +//! - SCL => GPIO2 + +#![no_std] +#![no_main] + +use esp32c3_hal::{ + clock::ClockControl, + gpio::IO, + i2c::I2C, + pac::Peripherals, + prelude::*, + timer::TimerGroup, + Rtc, +}; +use esp_backtrace as _; +use esp_println::println; +use riscv_rt::entry; + +#[entry] +fn main() -> ! { + let peripherals = Peripherals::take().unwrap(); + let mut system = peripherals.SYSTEM.split(); + let clocks = ClockControl::boot_defaults(system.clock_control).freeze(); + + let mut rtc = Rtc::new(peripherals.RTC_CNTL); + let timer_group0 = TimerGroup::new(peripherals.TIMG0, &clocks); + let mut wdt0 = timer_group0.wdt; + let timer_group1 = TimerGroup::new(peripherals.TIMG1, &clocks); + let mut wdt1 = timer_group1.wdt; + + // Disable watchdog timers + rtc.swd.disable(); + rtc.rwdt.disable(); + wdt0.disable(); + wdt1.disable(); + + let io = IO::new(peripherals.GPIO, peripherals.IO_MUX); + + // Create a new peripheral object with the described wiring + // and standard I2C clock speed + let mut i2c = I2C::new( + peripherals.I2C0, + io.pins.gpio1, + io.pins.gpio2, + 100u32.kHz(), + &mut system.peripheral_clock_control, + &clocks, + ) + .unwrap(); + + loop { + let mut data = [0u8; 22]; + i2c.write_read(0x77, &[0xaa], &mut data).ok(); + + println!("{:02x?}", data); + } +} diff --git a/esp32c3-hal/src/lib.rs b/esp32c3-hal/src/lib.rs index 53a2b118038..cecb0a025c9 100644 --- a/esp32c3-hal/src/lib.rs +++ b/esp32c3-hal/src/lib.rs @@ -1,7 +1,6 @@ #![no_std] use core::arch::{asm, global_asm}; - #[cfg(feature = "mcu-boot")] use core::mem::size_of; @@ -34,7 +33,6 @@ pub use esp_hal_common::{ Serial, UsbSerialJtag, }; - #[cfg(feature = "direct-boot")] use riscv_rt::pre_init; @@ -361,9 +359,8 @@ unsafe fn configure_mmu() { let autoload = cache_suspend_icache(); cache_invalidate_icache_all(); - /* Clear the MMU entries that are already set up, so the new app only has - * the mappings it creates. - */ + // Clear the MMU entries that are already set up, so the new app only has + // the mappings it creates. const FLASH_MMU_TABLE: *mut u32 = 0x600c_5000 as *mut u32; const ICACHE_MMU_SIZE: usize = 0x200; diff --git a/esp32s2-hal/examples/i2c_bmp180_calibration_data.rs b/esp32s2-hal/examples/i2c_bmp180_calibration_data.rs new file mode 100644 index 00000000000..94afdc7f33e --- /dev/null +++ b/esp32s2-hal/examples/i2c_bmp180_calibration_data.rs @@ -0,0 +1,59 @@ +//! Read calibration data from BMP180 sensor +//! +//! This example dumps the calibration data from a BMP180 sensor +//! +//! The following wiring is assumed: +//! - SDA => GPIO35 +//! - SCL => GPIO36 + +#![no_std] +#![no_main] + +use esp32s2_hal::{ + clock::ClockControl, + gpio::IO, + i2c::I2C, + pac::Peripherals, + prelude::*, + timer::TimerGroup, + Rtc, +}; +use esp_backtrace as _; +use esp_println::println; +use xtensa_lx_rt::entry; + +#[entry] +fn main() -> ! { + let peripherals = Peripherals::take().unwrap(); + let mut system = peripherals.SYSTEM.split(); + let clocks = ClockControl::boot_defaults(system.clock_control).freeze(); + + let timer_group0 = TimerGroup::new(peripherals.TIMG0, &clocks); + let mut wdt = timer_group0.wdt; + let mut rtc = Rtc::new(peripherals.RTC_CNTL); + + // Disable watchdog timer + wdt.disable(); + rtc.rwdt.disable(); + + let io = IO::new(peripherals.GPIO, peripherals.IO_MUX); + + // Create a new peripheral object with the described wiring + // and standard I2C clock speed + let mut i2c = I2C::new( + peripherals.I2C0, + io.pins.gpio35, + io.pins.gpio36, + 100u32.kHz(), + &mut system.peripheral_clock_control, + &clocks, + ) + .unwrap(); + + loop { + let mut data = [0u8; 22]; + i2c.write_read(0x77, &[0xaa], &mut data).ok(); + + println!("{:02x?}", data); + } +} diff --git a/esp32s3-hal/examples/i2c_bmp180_calibration_data.rs b/esp32s3-hal/examples/i2c_bmp180_calibration_data.rs new file mode 100644 index 00000000000..4fb5d44ebdb --- /dev/null +++ b/esp32s3-hal/examples/i2c_bmp180_calibration_data.rs @@ -0,0 +1,59 @@ +//! Read calibration data from BMP180 sensor +//! +//! This example dumps the calibration data from a BMP180 sensor +//! +//! The following wiring is assumed: +//! - SDA => GPIO1 +//! - SCL => GPIO2 + +#![no_std] +#![no_main] + +use esp32s3_hal::{ + clock::ClockControl, + gpio::IO, + i2c::I2C, + pac::Peripherals, + prelude::*, + timer::TimerGroup, + Rtc, +}; +use esp_backtrace as _; +use esp_println::println; +use xtensa_lx_rt::entry; + +#[entry] +fn main() -> ! { + let peripherals = Peripherals::take().unwrap(); + let mut system = peripherals.SYSTEM.split(); + let clocks = ClockControl::boot_defaults(system.clock_control).freeze(); + + let timer_group0 = TimerGroup::new(peripherals.TIMG0, &clocks); + let mut wdt = timer_group0.wdt; + let mut rtc = Rtc::new(peripherals.RTC_CNTL); + + // Disable watchdog timer + wdt.disable(); + rtc.rwdt.disable(); + + let io = IO::new(peripherals.GPIO, peripherals.IO_MUX); + + // Create a new peripheral object with the described wiring + // and standard I2C clock speed + let mut i2c = I2C::new( + peripherals.I2C0, + io.pins.gpio1, + io.pins.gpio2, + 100u32.kHz(), + &mut system.peripheral_clock_control, + &clocks, + ) + .unwrap(); + + loop { + let mut data = [0u8; 22]; + i2c.write_read(0x77, &[0xaa], &mut data).ok(); + + println!("{:02x?}", data); + } +} From 09bcf455bbb79676f842fd1f52d33ec4fd84a7e4 Mon Sep 17 00:00:00 2001 From: bjoernQ Date: Fri, 28 Oct 2022 11:25:49 +0200 Subject: [PATCH 2/2] Improve I2C error handling and robustness --- esp-hal-common/src/i2c.rs | 243 ++++++++++++-------------------------- 1 file changed, 78 insertions(+), 165 deletions(-) diff --git a/esp-hal-common/src/i2c.rs b/esp-hal-common/src/i2c.rs index e2320b08709..dcf3060b141 100644 --- a/esp-hal-common/src/i2c.rs +++ b/esp-hal-common/src/i2c.rs @@ -365,11 +365,13 @@ pub trait Instance { .ctr .modify(|_, w| w.conf_upgate().set_bit()); + self.reset(); + Ok(()) } /// Resets the I2C controller (FIFO + FSM + command list) - fn reset(&mut self) { + fn reset(&self) { // Reset interrupts // Disable all I2C interrupts self.register_block() @@ -396,7 +398,7 @@ pub trait Instance { } /// Resets the I2C peripheral's command registers - fn reset_command_list(&mut self) { + fn reset_command_list(&self) { // Confirm that all commands that were configured were actually executed for cmd in self.register_block().comd.iter() { cmd.reset(); @@ -632,7 +634,7 @@ pub trait Instance { add_cmd(cmd_iterator, Command::Stop)?; - self.upgate_config(); + self.update_config(); // Load address and R/W bit into FIFO write_fifo( @@ -645,7 +647,7 @@ pub trait Instance { self.start_transmission(); // fill FIFO with remaining bytes - self.write_remaining_tx_fifo(index, bytes); + self.write_remaining_tx_fifo(index, bytes)?; self.wait_for_completion()?; @@ -704,7 +706,7 @@ pub trait Instance { add_cmd(cmd_iterator, Command::Stop)?; - self.upgate_config(); + self.update_config(); // Load address and R/W bit into FIFO write_fifo(self.register_block(), addr << 1 | OperationType::Read as u8); @@ -718,98 +720,6 @@ pub trait Instance { Ok(()) } - #[cfg(not(debug_assertions))] - fn perform_write_read<'a, I>( - &self, - addr: u8, - bytes: &[u8], - buffer: &mut [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); - } - - // Clear all I2C interrupts - self.clear_all_interrupts(); - - // RSTART command - add_cmd(cmd_iterator, Command::Start)?; - - // WRITE command - add_cmd( - cmd_iterator, - Command::Write { - ack_exp: Ack::Ack, - ack_check_en: true, - length: 1 + bytes.len() as u8, - }, - )?; - - add_cmd(cmd_iterator, Command::Start)?; - - 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, - }, - )?; - - add_cmd(cmd_iterator, Command::Stop)?; - - self.upgate_config(); - - // Load address and R/W bit into FIFO - write_fifo( - self.register_block(), - addr << 1 | OperationType::Write as u8, - ); - - let index = self.fill_tx_fifo(bytes); - - self.start_transmission(); - - // fill FIFO with remaining bytes - self.write_remaining_tx_fifo(index, bytes); - - // writing to the FIFO here is too slow in debug mode apparently and the - // address isn't loaded fast enough ... therefore in debug mode we do separate - // write and reads - self.write_remaining_tx_fifo(0, &[addr << 1 | OperationType::Read as u8]); - - 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 @@ -817,22 +727,11 @@ pub trait Instance { // requested? Or is this prevented from a protocol perspective? for byte in buffer.iter_mut() { loop { - #[cfg(not(esp32))] - { - let reg = self.register_block().fifo_st.read(); - - if reg.rxfifo_raddr().bits() != reg.rxfifo_waddr().bits() { - break; - } - } - - #[cfg(esp32)] - { - let reg = self.register_block().rxfifo_st.read(); + self.check_errors()?; - if reg.rxfifo_start_addr().bits() != reg.rxfifo_end_addr().bits() { - break; - } + let reg = self.register_block().fifo_st.read(); + if reg.rxfifo_raddr().bits() != reg.rxfifo_waddr().bits() { + break; } } @@ -877,30 +776,7 @@ pub trait Instance { loop { let interrupts = self.register_block().int_raw.read(); - // The ESP32 variant has a slightly different interrupt naming - // scheme! - cfg_if::cfg_if! { - if #[cfg(esp32)] { - // Handle error cases - if interrupts.time_out_int_raw().bit_is_set() { - return Err(Error::TimeOut); - } else if interrupts.ack_err_int_raw().bit_is_set() { - return Err(Error::AckCheckFailed); - } else if interrupts.arbitration_lost_int_raw().bit_is_set() { - return Err(Error::ArbitrationLost); - } - } - else { - // Handle error cases - if interrupts.time_out_int_raw().bit_is_set() { - return Err(Error::TimeOut); - } else if interrupts.nack_int_raw().bit_is_set() { - return Err(Error::AckCheckFailed); - } else if interrupts.arbitration_lost_int_raw().bit_is_set() { - return Err(Error::ArbitrationLost); - } - } - } + self.check_errors()?; // Handle completion cases // A full transmission was completed @@ -919,7 +795,43 @@ pub trait Instance { Ok(()) } - fn upgate_config(&self) { + fn check_errors(&self) -> Result<(), Error> { + let interrupts = self.register_block().int_raw.read(); + + // The ESP32 variant has a slightly different interrupt naming + // scheme! + cfg_if::cfg_if! { + if #[cfg(esp32)] { + // Handle error cases + if interrupts.time_out_int_raw().bit_is_set() { + self.reset(); + return Err(Error::TimeOut); + } else if interrupts.ack_err_int_raw().bit_is_set() { + self.reset(); + return Err(Error::AckCheckFailed); + } else if interrupts.arbitration_lost_int_raw().bit_is_set() { + self.reset(); + return Err(Error::ArbitrationLost); + } + } + else { + // Handle error cases + if interrupts.time_out_int_raw().bit_is_set() { + self.reset(); + return Err(Error::TimeOut); + } else if interrupts.nack_int_raw().bit_is_set() { + self.reset(); + return Err(Error::AckCheckFailed); + } else if interrupts.arbitration_lost_int_raw().bit_is_set() { + return Err(Error::ArbitrationLost); + } + } + } + + Ok(()) + } + + fn update_config(&self) { // Ensure that the configuration of the peripheral is correctly propagated // (only necessary for C3 and S3 variant) #[cfg(any(esp32c2, esp32c3, esp32s3))] @@ -965,9 +877,11 @@ pub trait Instance { } #[cfg(not(any(esp32, esp32s2)))] - fn write_remaining_tx_fifo(&self, start_index: usize, bytes: &[u8]) { + fn write_remaining_tx_fifo(&self, start_index: usize, bytes: &[u8]) -> Result<(), Error> { let mut index = start_index; loop { + self.check_errors()?; + while !self .register_block() .int_raw @@ -980,7 +894,7 @@ pub trait Instance { .write(|w| w.txfifo_wm_int_clr().set_bit()); if index >= bytes.len() { - break; + break Ok(()); } write_fifo(self.register_block(), bytes[index]); @@ -1006,25 +920,28 @@ pub trait Instance { } #[cfg(any(esp32, esp32s2))] - fn write_remaining_tx_fifo(&self, start_index: usize, bytes: &[u8]) { + fn write_remaining_tx_fifo(&self, start_index: usize, bytes: &[u8]) -> Result<(), Error> { // on ESP32/ESP32-S2 we currently don't support I2C transactions larger than the // FIFO apparently it would be possible by using non-fifo mode // see https://github.com/espressif/arduino-esp32/blob/7e9afe8c5ed7b5bf29624a5cd6e07d431c027b97/cores/esp32/esp32-hal-i2c.c#L615 if start_index >= bytes.len() { - return; + return Ok(()); } // this is only possible when writing the I2C address in release mode // from [perform_write_read] for b in bytes { write_fifo(self.register_block(), *b); + self.check_errors()?; } + + Ok(()) } /// Resets the transmit and receive FIFO buffers #[cfg(not(esp32))] - fn reset_fifo(&mut self) { + fn reset_fifo(&self) { // First, reset the fifo buffers self.register_block().fifo_conf.modify(|_, w| { w.tx_fifo_rst() @@ -1035,26 +952,29 @@ pub trait Instance { .clear_bit() .fifo_prt_en() .set_bit() - .tx_fifo_rst() - .clear_bit() - .rx_fifo_rst() - .clear_bit() .rxfifo_wm_thrhd() .variant(1) .txfifo_wm_thrhd() .variant(8) }); + + self.register_block() + .fifo_conf + .modify(|_, w| w.tx_fifo_rst().clear_bit().rx_fifo_rst().clear_bit()); + self.register_block().int_clr.write(|w| { w.rxfifo_wm_int_clr() .set_bit() .txfifo_wm_int_clr() .set_bit() }); + + self.update_config(); } /// Resets the transmit and receive FIFO buffers #[cfg(esp32)] - fn reset_fifo(&mut self) { + fn reset_fifo(&self) { // First, reset the fifo buffers self.register_block().fifo_conf.modify(|_, w| { w.tx_fifo_rst() @@ -1063,15 +983,16 @@ pub trait Instance { .set_bit() .nonfifo_en() .clear_bit() - .tx_fifo_rst() - .clear_bit() - .rx_fifo_rst() - .clear_bit() .nonfifo_rx_thres() .variant(1) .nonfifo_tx_thres() .variant(32) }); + + self.register_block() + .fifo_conf + .modify(|_, w| w.tx_fifo_rst().clear_bit().rx_fifo_rst().clear_bit()); + self.register_block() .int_clr .write(|w| w.rxfifo_full_int_clr().set_bit()); @@ -1106,20 +1027,12 @@ pub trait Instance { bytes: &[u8], buffer: &mut [u8], ) -> Result<(), Error> { - #[cfg(not(debug_assertions))] - { - // Reset FIFO and command list - self.reset_fifo(); - self.reset_command_list(); - - self.perform_write_read(addr, bytes, buffer, &mut self.register_block().comd.iter())?; - } - - #[cfg(debug_assertions)] - { - self.master_write(addr, bytes)?; - self.master_read(addr, buffer)?; - } + // 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 + self.master_write(addr, bytes)?; + self.master_read(addr, buffer)?; Ok(()) } }