From e367c83bde6418a650c1ce8ebeecd72506eb0634 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 23 Oct 2024 08:31:08 +0200 Subject: [PATCH] Erase i2c peripheral instances (#2361) * Clean up * Erase I2C instance type * Clean up * Implement Peripheral * Changelog * Remove with_timeout constructors --- esp-hal/CHANGELOG.md | 3 + esp-hal/MIGRATING-0.21.md | 10 + esp-hal/src/i2c.rs | 588 ++++++++++++++--------------- examples/src/bin/lcd_cam_ov2640.rs | 4 +- hil-test/tests/i2c.rs | 5 +- 5 files changed, 295 insertions(+), 315 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 813893cb19e..149b466d88b 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -16,11 +16,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added `AnySpi` and `AnySpiDmaChannel`. (#2334) - Added `AnyI2s` and `AnyI2sDmaChannel`. (#2367) - `Pins::steal()` to unsafely obtain GPIO. (#2335) +- `I2c::with_timeout` (#2361) ### Changed - Peripheral type erasure for SPI (#2334) - Peripheral type erasure for I2S (#2367) +- Peripheral type erasure for I2C (#2361) ### Fixed @@ -31,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The `i2s::{I2sWrite, I2sWriteDma, I2sRead, I2sReadDma, I2sWriteDmaAsync, I2sReadDmaAsync}` traits have been removed. (#2316) - The `ledc::ChannelHW` trait is no longer generic. (#2387) +- The `I2c::new_with_timeout` constructors have been removed (#2361) ## [0.21.1] diff --git a/esp-hal/MIGRATING-0.21.md b/esp-hal/MIGRATING-0.21.md index 0b8e1938bb1..b26ed414e06 100644 --- a/esp-hal/MIGRATING-0.21.md +++ b/esp-hal/MIGRATING-0.21.md @@ -30,6 +30,7 @@ peripherals: - SPI (both master and slave) - I2S +- I2C ```diff -Spi<'static, SPI2, FullDuplexMode> @@ -49,3 +50,12 @@ the peripheral instance has been moved to the last generic parameter position. ```rust let spi: Spi<'static, FullDuplexMode, SPI2> = Spi::new_typed(peripherals.SPI2, 1.MHz(), SpiMode::Mode0); ``` + +## I2C constructor changes + +The `with_timeout` constructors have been removed in favour of `set_timeout` or `with_timeout`. + +```diff +-let i2c = I2c::new_with_timeout(peripherals.I2C0, io.pins.gpio4, io.pins.gpio5, 100.kHz(), timeout); ++let i2c = I2c::new(peripherals.I2C0, io.pins.gpio4, io.pins.gpio5, 100.kHz()).with_timeout(timeout); +``` diff --git a/esp-hal/src/i2c.rs b/esp-hal/src/i2c.rs index 25a141c08a4..58f87ac5871 100644 --- a/esp-hal/src/i2c.rs +++ b/esp-hal/src/i2c.rs @@ -58,6 +58,7 @@ use fugit::HertzU32; use crate::{ clock::Clocks, + dma::PeripheralMarker, gpio::{InputSignal, OutputSignal, PeripheralInput, PeripheralOutput, Pull}, interrupt::InterruptHandler, peripheral::{Peripheral, PeripheralRef}, @@ -232,14 +233,14 @@ impl From for u32 { } /// I2C driver -pub struct I2c<'d, T, DM: Mode> { - peripheral: PeripheralRef<'d, T>, +pub struct I2c<'d, DM: Mode, T = AnyI2c> { + i2c: PeripheralRef<'d, T>, phantom: PhantomData, frequency: HertzU32, timeout: Option, } -impl I2c<'_, T, DM> +impl I2c<'_, DM, T> where T: Instance, DM: Mode, @@ -259,15 +260,15 @@ where let next_op = op_iter.peek().map(|v| v.kind()); let kind = op.kind(); // Clear all I2C interrupts - self.peripheral.clear_all_interrupts(); + self.i2c.clear_all_interrupts(); - let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); + let cmd_iterator = &mut self.i2c.register_block().comd_iter(); match op { Operation::Write(buffer) => { // execute a write operation: // - issue START/RSTART if op is different from previous // - issue STOP if op is the last one - self.peripheral + self.i2c .write_operation( address, buffer, @@ -282,7 +283,7 @@ where // - issue START/RSTART if op is different from previous // - issue STOP if op is the last one // - will_continue is true if there is another read operation next - self.peripheral + self.i2c .read_operation( address, buffer, @@ -302,7 +303,7 @@ where } } -impl I2c<'_, T, Blocking> +impl I2c<'_, Blocking, T> where T: Instance, { @@ -311,11 +312,11 @@ where let chunk_count = buffer.len().div_ceil(I2C_CHUNK_SIZE); for (idx, chunk) in buffer.chunks_mut(I2C_CHUNK_SIZE).enumerate() { // Clear all I2C interrupts - self.peripheral.clear_all_interrupts(); + self.i2c.clear_all_interrupts(); - let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); + let cmd_iterator = &mut self.i2c.register_block().comd_iter(); - self.peripheral + self.i2c .read_operation( address, chunk, @@ -335,11 +336,11 @@ where let chunk_count = buffer.len().div_ceil(I2C_CHUNK_SIZE); for (idx, chunk) in buffer.chunks(I2C_CHUNK_SIZE).enumerate() { // Clear all I2C interrupts - self.peripheral.clear_all_interrupts(); + self.i2c.clear_all_interrupts(); - let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); + let cmd_iterator = &mut self.i2c.register_block().comd_iter(); - self.peripheral + self.i2c .write_operation( address, chunk, @@ -366,11 +367,11 @@ where for (idx, chunk) in write_buffer.chunks(I2C_CHUNK_SIZE).enumerate() { // Clear all I2C interrupts - self.peripheral.clear_all_interrupts(); + self.i2c.clear_all_interrupts(); - let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); + let cmd_iterator = &mut self.i2c.register_block().comd_iter(); - self.peripheral + self.i2c .write_operation( address, chunk, @@ -383,11 +384,11 @@ where for (idx, chunk) in read_buffer.chunks_mut(I2C_CHUNK_SIZE).enumerate() { // Clear all I2C interrupts - self.peripheral.clear_all_interrupts(); + self.i2c.clear_all_interrupts(); - let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); + let cmd_iterator = &mut self.i2c.register_block().comd_iter(); - self.peripheral + self.i2c .read_operation( address, chunk, @@ -429,7 +430,7 @@ where } } -impl embedded_hal_02::blocking::i2c::Read for I2c<'_, T, Blocking> +impl embedded_hal_02::blocking::i2c::Read for I2c<'_, Blocking, T> where T: Instance, { @@ -440,7 +441,7 @@ where } } -impl embedded_hal_02::blocking::i2c::Write for I2c<'_, T, Blocking> +impl embedded_hal_02::blocking::i2c::Write for I2c<'_, Blocking, T> where T: Instance, { @@ -451,7 +452,7 @@ where } } -impl embedded_hal_02::blocking::i2c::WriteRead for I2c<'_, T, Blocking> +impl embedded_hal_02::blocking::i2c::WriteRead for I2c<'_, Blocking, T> where T: Instance, { @@ -467,11 +468,11 @@ where } } -impl embedded_hal::i2c::ErrorType for I2c<'_, T, DM> { +impl embedded_hal::i2c::ErrorType for I2c<'_, DM, T> { type Error = Error; } -impl embedded_hal::i2c::I2c for I2c<'_, T, DM> +impl embedded_hal::i2c::I2c for I2c<'_, DM, T> where T: Instance, { @@ -484,7 +485,7 @@ where } } -impl<'d, T, DM: Mode> I2c<'d, T, DM> +impl<'d, T, DM: Mode> I2c<'d, DM, T> where T: Instance, { @@ -496,20 +497,20 @@ where sda: impl Peripheral

+ 'd, scl: impl Peripheral

+ 'd, frequency: HertzU32, - timeout: Option, ) -> Self { crate::into_ref!(i2c, sda, scl); - PeripheralClockControl::reset(T::peripheral()); - PeripheralClockControl::enable(T::peripheral()); - let i2c = I2c { - peripheral: i2c, + i2c, phantom: PhantomData, frequency, - timeout, + timeout: None, }; + PeripheralClockControl::reset(i2c.i2c.peripheral()); + PeripheralClockControl::enable(i2c.i2c.peripheral()); + + // TODO: implement with_pins et. al. // avoid SCL/SDA going low during configuration scl.set_output_high(true, crate::private::Internal); sda.set_output_high(true, crate::private::Internal); @@ -518,65 +519,67 @@ where scl.enable_input(true, crate::private::Internal); scl.pull_direction(Pull::Up, crate::private::Internal); - scl.connect_input_to_peripheral( - i2c.peripheral.scl_input_signal(), - crate::private::Internal, - ); - scl.connect_peripheral_to_output( - i2c.peripheral.scl_output_signal(), - crate::private::Internal, - ); + scl.connect_input_to_peripheral(i2c.i2c.scl_input_signal(), crate::private::Internal); + scl.connect_peripheral_to_output(i2c.i2c.scl_output_signal(), crate::private::Internal); sda.set_to_open_drain_output(crate::private::Internal); sda.enable_input(true, crate::private::Internal); sda.pull_direction(Pull::Up, crate::private::Internal); - sda.connect_input_to_peripheral( - i2c.peripheral.sda_input_signal(), - crate::private::Internal, - ); - sda.connect_peripheral_to_output( - i2c.peripheral.sda_output_signal(), - crate::private::Internal, - ); + sda.connect_input_to_peripheral(i2c.i2c.sda_input_signal(), crate::private::Internal); + sda.connect_peripheral_to_output(i2c.i2c.sda_output_signal(), crate::private::Internal); - i2c.peripheral.setup(frequency, timeout); + i2c.i2c.setup(frequency, None); i2c } fn internal_set_interrupt_handler(&mut self, handler: InterruptHandler) { - unsafe { crate::interrupt::bind_interrupt(T::interrupt(), handler.handler()) }; - unwrap!(crate::interrupt::enable(T::interrupt(), handler.priority())); + unsafe { crate::interrupt::bind_interrupt(self.i2c.interrupt(), handler.handler()) }; + unwrap!(crate::interrupt::enable( + self.i2c.interrupt(), + handler.priority() + )); } fn internal_recover(&self) { - PeripheralClockControl::reset(T::peripheral()); - PeripheralClockControl::enable(T::peripheral()); + PeripheralClockControl::reset(self.i2c.peripheral()); + PeripheralClockControl::enable(self.i2c.peripheral()); + + self.i2c.setup(self.frequency, self.timeout); + } - self.peripheral.setup(self.frequency, self.timeout); + /// Set the I2C timeout. + // TODO: explain this function better - what's the unit, what happens on + // timeout, and just what exactly is a timeout in this context? + pub fn with_timeout(mut self, timeout: Option) -> Self { + self.timeout = timeout; + self.i2c.setup(self.frequency, self.timeout); + self } } -impl<'d, T> I2c<'d, T, Blocking> -where - T: Instance, -{ +impl<'d> I2c<'d, Blocking> { /// Create a new I2C instance /// This will enable the peripheral but the peripheral won't get /// automatically disabled when this gets dropped. pub fn new( - i2c: impl Peripheral

+ 'd, + i2c: impl Peripheral

+ 'd, sda: impl Peripheral

+ 'd, scl: impl Peripheral

+ 'd, frequency: HertzU32, ) -> Self { - Self::new_with_timeout(i2c, sda, scl, frequency, None) + Self::new_typed(i2c.map_into(), sda, scl, frequency) } +} - /// Create a new I2C instance with a custom timeout value. +impl<'d, T> I2c<'d, Blocking, T> +where + T: Instance, +{ + /// Create a new I2C instance /// This will enable the peripheral but the peripheral won't get /// automatically disabled when this gets dropped. - pub fn new_with_timeout< + pub fn new_typed< SDA: PeripheralOutput + PeripheralInput, SCL: PeripheralOutput + PeripheralInput, >( @@ -584,15 +587,14 @@ where sda: impl Peripheral

+ 'd, scl: impl Peripheral

+ 'd, frequency: HertzU32, - timeout: Option, ) -> Self { - Self::new_internal(i2c, sda, scl, frequency, timeout) + Self::new_internal(i2c, sda, scl, frequency) } } -impl<'d, T> crate::private::Sealed for I2c<'d, T, Blocking> where T: Instance {} +impl<'d, T> crate::private::Sealed for I2c<'d, Blocking, T> where T: Instance {} -impl<'d, T> InterruptConfigurable for I2c<'d, T, Blocking> +impl<'d, T> InterruptConfigurable for I2c<'d, Blocking, T> where T: Instance, { @@ -601,10 +603,7 @@ where } } -impl<'d, T> I2c<'d, T, Async> -where - T: Instance, -{ +impl<'d> I2c<'d, Async> { /// Create a new I2C instance /// This will enable the peripheral but the peripheral won't get /// automatically disabled when this gets dropped. @@ -612,18 +611,23 @@ where SDA: PeripheralOutput + PeripheralInput, SCL: PeripheralOutput + PeripheralInput, >( - i2c: impl Peripheral

+ 'd, + i2c: impl Peripheral

+ 'd, sda: impl Peripheral

+ 'd, scl: impl Peripheral

+ 'd, frequency: HertzU32, ) -> Self { - Self::new_with_timeout_async(i2c, sda, scl, frequency, None) + Self::new_async_typed(i2c.map_into(), sda, scl, frequency) } +} - /// Create a new I2C instance with a custom timeout value. +impl<'d, T> I2c<'d, Async, T> +where + T: Instance, +{ + /// Create a new I2C instance /// This will enable the peripheral but the peripheral won't get /// automatically disabled when this gets dropped. - pub fn new_with_timeout_async< + pub fn new_async_typed< SDA: PeripheralOutput + PeripheralInput, SCL: PeripheralOutput + PeripheralInput, >( @@ -631,18 +635,13 @@ where sda: impl Peripheral

+ 'd, scl: impl Peripheral

+ 'd, frequency: HertzU32, - timeout: Option, ) -> Self { - let mut this = Self::new_internal(i2c, sda, scl, frequency, timeout); + let mut this = Self::new_internal(i2c, sda, scl, frequency); - this.internal_set_interrupt_handler(T::async_handler()); + this.internal_set_interrupt_handler(this.i2c.async_handler()); this } - - pub(crate) fn inner(&self) -> &T { - &self.peripheral - } } mod asynch { @@ -671,7 +670,7 @@ mod asynch { #[cfg(not(esp32))] #[must_use = "futures do nothing unless you `.await` or poll them"] - pub(crate) struct I2cFuture<'a, T> + struct I2cFuture<'a, T> where T: Instance, { @@ -693,21 +692,13 @@ mod asynch { Event::TxFifoWatermark => w.txfifo_wm().set_bit(), }; - #[cfg(not(esp32))] - w.arbitration_lost() - .set_bit() - .time_out() - .set_bit() - .nack() - .set_bit(); + w.arbitration_lost().set_bit(); + w.time_out().set_bit(); #[cfg(esp32)] - w.arbitration_lost() - .set_bit() - .time_out() - .set_bit() - .ack_err() - .set_bit(); + w.ack_err().set_bit(); + #[cfg(not(esp32))] + w.nack().set_bit(); w }); @@ -788,7 +779,7 @@ mod asynch { } } - impl I2c<'_, T, Async> + impl I2c<'_, Async, T> where T: Instance, { @@ -801,7 +792,7 @@ mod asynch { self.wait_for_completion(false).await?; for byte in buffer.iter_mut() { - *byte = read_fifo(self.peripheral.register_block()); + *byte = read_fifo(self.i2c.register_block()); } Ok(()) @@ -809,7 +800,7 @@ mod asynch { #[cfg(not(any(esp32, esp32s2)))] async fn read_all_from_fifo(&self, buffer: &mut [u8]) -> Result<(), Error> { - self.peripheral.read_all_from_fifo(buffer) + self.i2c.read_all_from_fifo(buffer) } #[cfg(any(esp32, esp32s2))] @@ -823,8 +814,8 @@ mod asynch { } for b in bytes { - write_fifo(self.peripheral.register_block(), *b); - self.peripheral.check_errors()?; + write_fifo(self.i2c.register_block(), *b); + self.i2c.check_errors()?; } Ok(()) @@ -838,36 +829,36 @@ mod asynch { ) -> Result<(), Error> { let mut index = start_index; loop { - self.peripheral.check_errors()?; + self.i2c.check_errors()?; - I2cFuture::new(Event::TxFifoWatermark, self.inner()).await?; + I2cFuture::new(Event::TxFifoWatermark, &*self.i2c).await?; - self.peripheral + self.i2c .register_block() .int_clr() .write(|w| w.txfifo_wm().clear_bit_by_one()); - I2cFuture::new(Event::TxFifoWatermark, self.inner()).await?; + I2cFuture::new(Event::TxFifoWatermark, &*self.i2c).await?; if index >= bytes.len() { break Ok(()); } - write_fifo(self.peripheral.register_block(), bytes[index]); + write_fifo(self.i2c.register_block(), bytes[index]); index += 1; } } #[cfg(not(esp32))] async fn wait_for_completion(&self, end_only: bool) -> Result<(), Error> { - self.peripheral.check_errors()?; + self.i2c.check_errors()?; if end_only { - I2cFuture::new(Event::EndDetect, self.inner()).await?; + I2cFuture::new(Event::EndDetect, &*self.i2c).await?; } else { let res = embassy_futures::select::select( - I2cFuture::new(Event::TxComplete, self.inner()), - I2cFuture::new(Event::EndDetect, self.inner()), + I2cFuture::new(Event::TxComplete, &*self.i2c), + I2cFuture::new(Event::EndDetect, &*self.i2c), ) .await; @@ -876,7 +867,7 @@ mod asynch { embassy_futures::select::Either::Second(res) => res?, } } - self.peripheral.check_all_commands_done()?; + self.i2c.check_all_commands_done()?; Ok(()) } @@ -888,9 +879,9 @@ mod asynch { let mut tout = MAX_ITERATIONS / 10; // adjust the timeout because we are yielding in the loop loop { - let interrupts = self.inner().register_block().int_raw().read(); + let interrupts = self.i2c.register_block().int_raw().read(); - self.inner().check_errors()?; + self.i2c.check_errors()?; // Handle completion cases // A full transmission was completed (either a STOP condition or END was @@ -908,7 +899,7 @@ mod asynch { embassy_futures::yield_now().await; } - self.peripheral.check_all_commands_done()?; + self.i2c.check_all_commands_done()?; Ok(()) } @@ -938,19 +929,18 @@ mod asynch { } // Reset FIFO and command list - self.peripheral.reset_fifo(); - self.peripheral.reset_command_list(); + self.i2c.reset_fifo(); + self.i2c.reset_command_list(); if start { add_cmd(cmd_iterator, Command::Start)?; } - self.peripheral - .setup_write(address, bytes, start, cmd_iterator)?; + self.i2c.setup_write(address, bytes, start, 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(); + let index = self.i2c.fill_tx_fifo(bytes); + self.i2c.start_transmission(); // Fill the FIFO with the remaining bytes: self.write_remaining_tx_fifo(index, bytes).await?; @@ -987,18 +977,18 @@ mod asynch { } // Reset FIFO and command list - self.peripheral.reset_fifo(); - self.peripheral.reset_command_list(); + self.i2c.reset_fifo(); + self.i2c.reset_command_list(); if start { add_cmd(cmd_iterator, Command::Start)?; } - self.peripheral + self.i2c .setup_read(address, buffer, start, will_continue, cmd_iterator)?; add_cmd( cmd_iterator, if stop { Command::Stop } else { Command::End }, )?; - self.peripheral.start_transmission(); + self.i2c.start_transmission(); self.read_all_from_fifo(buffer).await?; self.wait_for_completion(!stop).await?; Ok(()) @@ -1009,9 +999,9 @@ mod asynch { let chunk_count = buffer.len().div_ceil(I2C_CHUNK_SIZE); for (idx, chunk) in buffer.chunks(I2C_CHUNK_SIZE).enumerate() { // Clear all I2C interrupts - self.peripheral.clear_all_interrupts(); + self.i2c.clear_all_interrupts(); - let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); + let cmd_iterator = &mut self.i2c.register_block().comd_iter(); self.write_operation( address, @@ -1031,9 +1021,9 @@ mod asynch { let chunk_count = buffer.len().div_ceil(I2C_CHUNK_SIZE); for (idx, chunk) in buffer.chunks_mut(I2C_CHUNK_SIZE).enumerate() { // Clear all I2C interrupts - self.peripheral.clear_all_interrupts(); + self.i2c.clear_all_interrupts(); - let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); + let cmd_iterator = &mut self.i2c.register_block().comd_iter(); self.read_operation( address, @@ -1061,9 +1051,9 @@ mod asynch { let read_count = read_buffer.len().div_ceil(I2C_CHUNK_SIZE); for (idx, chunk) in write_buffer.chunks(I2C_CHUNK_SIZE).enumerate() { // Clear all I2C interrupts - self.peripheral.clear_all_interrupts(); + self.i2c.clear_all_interrupts(); - let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); + let cmd_iterator = &mut self.i2c.register_block().comd_iter(); self.write_operation( address, @@ -1077,9 +1067,9 @@ mod asynch { for (idx, chunk) in read_buffer.chunks_mut(I2C_CHUNK_SIZE).enumerate() { // Clear all I2C interrupts - self.peripheral.clear_all_interrupts(); + self.i2c.clear_all_interrupts(); - let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); + let cmd_iterator = &mut self.i2c.register_block().comd_iter(); self.read_operation( address, @@ -1138,9 +1128,9 @@ mod asynch { let next_op = op_iter.peek().map(|v| v.kind()); let kind = op.kind(); // Clear all I2C interrupts - self.peripheral.clear_all_interrupts(); + self.i2c.clear_all_interrupts(); - let cmd_iterator = &mut self.peripheral.register_block().comd_iter(); + let cmd_iterator = &mut self.i2c.register_block().comd_iter(); match op { Operation::Write(buffer) => { // execute a write operation: @@ -1179,7 +1169,7 @@ mod asynch { } } - impl<'d, T> embedded_hal_async::i2c::I2c for I2c<'d, T, Async> + impl<'d, T> embedded_hal_async::i2c::I2c for I2c<'d, Async, T> where T: Instance, { @@ -1193,9 +1183,7 @@ mod asynch { } } - #[handler] - pub(super) fn i2c0_handler() { - let regs = unsafe { &*crate::peripherals::I2C0::PTR }; + fn handler(regs: &RegisterBlock) { regs.int_ena().modify(|_, w| { w.end_detect().clear_bit(); w.trans_complete().clear_bit(); @@ -1211,6 +1199,12 @@ mod asynch { #[cfg(esp32)] regs.int_ena().modify(|_, w| w.ack_err().clear_bit()); + } + + #[handler] + pub(super) fn i2c0_handler() { + let regs = unsafe { &*crate::peripherals::I2C0::PTR }; + handler(regs); WAKERS[0].wake(); } @@ -1219,25 +1213,7 @@ mod asynch { #[handler] pub(super) fn i2c1_handler() { let regs = unsafe { &*crate::peripherals::I2C1::PTR }; - regs.int_ena().modify(|_, w| { - w.end_detect() - .clear_bit() - .trans_complete() - .clear_bit() - .arbitration_lost() - .clear_bit() - .time_out() - .clear_bit() - }); - - #[cfg(not(any(esp32, esp32s2)))] - regs.int_ena().modify(|_, w| w.txfifo_wm().clear_bit()); - - #[cfg(not(esp32))] - regs.int_ena().modify(|_, w| w.nack().clear_bit()); - - #[cfg(esp32)] - regs.int_ena().modify(|_, w| w.ack_err().clear_bit()); + handler(regs); WAKERS[1].wake(); } @@ -1245,13 +1221,11 @@ mod asynch { /// I2C Peripheral Instance #[doc(hidden)] -pub trait Instance: crate::private::Sealed { +pub trait Instance: Peripheral

+ PeripheralMarker + Into + 'static { /// Returns the interrupt associated with this I2C peripheral. - fn interrupt() -> crate::peripherals::Interrupt; + fn interrupt(&self) -> crate::peripherals::Interrupt; - fn async_handler() -> InterruptHandler; - - fn peripheral() -> crate::system::Peripheral; + fn async_handler(&self) -> InterruptHandler; /// Returns the SCL output signal for this I2C peripheral. fn scl_output_signal(&self) -> OutputSignal; @@ -1271,25 +1245,17 @@ pub trait Instance: crate::private::Sealed { /// Configures the I2C peripheral with the specified frequency, clocks, and /// optional timeout. fn setup(&self, frequency: HertzU32, timeout: Option) { - self.register_block().ctr().modify(|_, w| unsafe { - // Clear register - w.bits(0) - // Set I2C controller to master mode - .ms_mode() - .set_bit() - // Use open drain output for SDA and SCL - .sda_force_out() - .set_bit() - .scl_force_out() - .set_bit() - // Use Most Significant Bit first for sending and receiving data - .tx_lsb_first() - .clear_bit() - .rx_lsb_first() - .clear_bit() - // Ensure that clock is enabled - .clk_en() - .set_bit() + self.register_block().ctr().write(|w| { + // Set I2C controller to master mode + w.ms_mode().set_bit(); + // Use open drain output for SDA and SCL + w.sda_force_out().set_bit(); + w.scl_force_out().set_bit(); + // Use Most Significant Bit first for sending and receiving data + w.tx_lsb_first().clear_bit(); + w.rx_lsb_first().clear_bit(); + // Ensure that clock is enabled + w.clk_en().set_bit() }); #[cfg(esp32s2)] @@ -1305,13 +1271,14 @@ pub trait Instance: crate::private::Sealed { let clocks = Clocks::get(); cfg_if::cfg_if! { if #[cfg(esp32)] { - self.set_frequency(clocks.i2c_clock.convert(), frequency, timeout); + let clock = clocks.i2c_clock.convert(); } else if #[cfg(esp32s2)] { - self.set_frequency(clocks.apb_clock.convert(), frequency, timeout); + let clock = clocks.apb_clock.convert(); } else { - self.set_frequency(clocks.xtal_clock.convert(), frequency, timeout); + let clock = clocks.xtal_clock.convert(); } } + self.set_frequency(clock, frequency, timeout); self.update_config(); @@ -1394,12 +1361,8 @@ pub trait Instance: crate::private::Sealed { let sda_sample = scl_high / 2; let setup = half_cycle; let hold = half_cycle; - let tout = if let Some(timeout) = timeout { - timeout - } else { - // default we set the timeout value to 10 bus cycles - half_cycle * 20 - }; + // default we set the timeout value to 10 bus cycles + let time_out_value = timeout.unwrap_or(half_cycle * 20); // SCL period. According to the TRM, we should always subtract 1 to SCL low // period @@ -1439,7 +1402,6 @@ pub trait Instance: crate::private::Sealed { // hold let scl_start_hold_time = hold; let scl_stop_hold_time = hold; - let time_out_value = tout; self.configure_clock( 0, @@ -1476,12 +1438,8 @@ pub trait Instance: crate::private::Sealed { let sda_sample = half_cycle / 2 - 1; let setup = half_cycle; let hold = half_cycle; - let tout = if let Some(timeout) = timeout { - timeout - } else { - // default we set the timeout value to 10 bus cycles - half_cycle * 20 - }; + // default we set the timeout value to 10 bus cycles + let time_out_value = timeout.unwrap_or(half_cycle * 20); // scl period let scl_low_period = scl_low - 1; @@ -1496,7 +1454,6 @@ pub trait Instance: crate::private::Sealed { // hold let scl_start_hold_time = hold - 1; let scl_stop_hold_time = hold; - let time_out_value = tout; let time_out_en = true; self.configure_clock( @@ -1543,7 +1500,7 @@ pub trait Instance: crate::private::Sealed { let setup = half_cycle; let hold = half_cycle; - let tout = if let Some(timeout) = timeout { + let time_out_value = if let Some(timeout) = timeout { timeout } else { // default we set the timeout value to about 10 bus cycles @@ -1570,7 +1527,6 @@ pub trait Instance: crate::private::Sealed { // hold let scl_start_hold_time = hold - 1; let scl_stop_hold_time = hold - 1; - let time_out_value = tout; let time_out_en = true; self.configure_clock( @@ -1606,20 +1562,21 @@ pub trait Instance: crate::private::Sealed { time_out_value: u32, time_out_en: bool, ) { + let register_block = self.register_block(); unsafe { // divider #[cfg(any(esp32c2, esp32c3, esp32c6, esp32h2, esp32s3))] - self.register_block().clk_conf().modify(|_, w| { + register_block.clk_conf().modify(|_, w| { w.sclk_sel().clear_bit(); w.sclk_div_num().bits((sclk_div - 1) as u8) }); // scl period - self.register_block() + register_block .scl_low_period() .write(|w| w.scl_low_period().bits(scl_low_period as u16)); - self.register_block().scl_high_period().write(|w| { + register_block.scl_high_period().write(|w| { #[cfg(not(esp32))] // ESP32 does not have a wait_high field w.scl_wait_high_period() .bits(scl_wait_high_period.try_into().unwrap()); @@ -1627,26 +1584,26 @@ pub trait Instance: crate::private::Sealed { }); // sda sample - self.register_block() + register_block .sda_hold() .write(|w| w.time().bits(sda_hold_time as u16)); - self.register_block() + register_block .sda_sample() .write(|w| w.time().bits(sda_sample_time as u16)); // setup - self.register_block() + register_block .scl_rstart_setup() .write(|w| w.time().bits(scl_rstart_setup_time as u16)); - self.register_block() + register_block .scl_stop_setup() .write(|w| w.time().bits(scl_stop_setup_time as u16)); // hold - self.register_block() + register_block .scl_start_hold() .write(|w| w.time().bits(scl_start_hold_time as u16)); - self.register_block() + register_block .scl_stop_hold() .write(|w| w.time().bits(scl_stop_hold_time as u16)); @@ -1655,14 +1612,14 @@ pub trait Instance: crate::private::Sealed { cfg_if::cfg_if! { if #[cfg(esp32)] { // timeout - self.register_block() + register_block .to() .write(|w| w.time_out().bits(time_out_value)); } else { // timeout // FIXME: Enable timout for other chips! #[allow(clippy::useless_conversion)] - self.register_block() + register_block .to() .write(|w| w.time_out_en().bit(time_out_en) .time_out_value() @@ -1911,36 +1868,36 @@ pub trait Instance: crate::private::Sealed { cfg_if::cfg_if! { if #[cfg(esp32)] { // Handle error cases - if interrupts.time_out().bit_is_set() { - self.reset(); - return Err(Error::TimeOut); + let retval = if interrupts.time_out().bit_is_set() { + Err(Error::TimeOut) } else if interrupts.ack_err().bit_is_set() { - self.reset(); - return Err(Error::AckCheckFailed); + Err(Error::AckCheckFailed) } else if interrupts.arbitration_lost().bit_is_set() { - self.reset(); - return Err(Error::ArbitrationLost); - } - } - else { + Err(Error::ArbitrationLost) + } else { + Ok(()) + }; + } else { // Handle error cases - if interrupts.time_out().bit_is_set() { - self.reset(); - return Err(Error::TimeOut); + let retval = if interrupts.time_out().bit_is_set() { + Err(Error::TimeOut) } else if interrupts.nack().bit_is_set() { - self.reset(); - return Err(Error::AckCheckFailed); + Err(Error::AckCheckFailed) } else if interrupts.arbitration_lost().bit_is_set() { - self.reset(); - return Err(Error::ArbitrationLost); - } else if interrupts.trans_complete().bit_is_set() && self.register_block().sr().read().resp_rec().bit_is_clear() { - self.reset(); - return Err(Error::AckCheckFailed); - } + Err(Error::ArbitrationLost) + } else if interrupts.trans_complete().bit_is_set() && self.register_block().sr().read().resp_rec().bit_is_clear() { + Err(Error::AckCheckFailed) + } else { + Ok(()) + }; } } - Ok(()) + if retval.is_err() { + self.reset(); + } + + retval } /// Updates the configuration of the I2C peripheral. @@ -2085,29 +2042,22 @@ pub trait Instance: crate::private::Sealed { fn reset_fifo(&self) { // First, reset the fifo buffers self.register_block().fifo_conf().modify(|_, w| unsafe { - w.tx_fifo_rst() - .set_bit() - .rx_fifo_rst() - .set_bit() - .nonfifo_en() - .clear_bit() - .fifo_prt_en() - .set_bit() - .rxfifo_wm_thrhd() - .bits(1) - .txfifo_wm_thrhd() - .bits(8) + w.tx_fifo_rst().set_bit(); + w.rx_fifo_rst().set_bit(); + w.nonfifo_en().clear_bit(); + w.fifo_prt_en().set_bit(); + w.rxfifo_wm_thrhd().bits(1); + w.txfifo_wm_thrhd().bits(8) }); - self.register_block() - .fifo_conf() - .modify(|_, w| w.tx_fifo_rst().clear_bit().rx_fifo_rst().clear_bit()); + self.register_block().fifo_conf().modify(|_, w| { + w.tx_fifo_rst().clear_bit(); + w.rx_fifo_rst().clear_bit() + }); self.register_block().int_clr().write(|w| { - w.rxfifo_wm() - .clear_bit_by_one() - .txfifo_wm() - .clear_bit_by_one() + w.rxfifo_wm().clear_bit_by_one(); + w.txfifo_wm().clear_bit_by_one() }); self.update_config(); @@ -2118,21 +2068,17 @@ pub trait Instance: crate::private::Sealed { fn reset_fifo(&self) { // First, reset the fifo buffers self.register_block().fifo_conf().modify(|_, w| unsafe { - w.tx_fifo_rst() - .set_bit() - .rx_fifo_rst() - .set_bit() - .nonfifo_en() - .clear_bit() - .nonfifo_rx_thres() - .bits(1) - .nonfifo_tx_thres() - .bits(32) + w.tx_fifo_rst().set_bit(); + w.rx_fifo_rst().set_bit(); + w.nonfifo_en().clear_bit(); + w.nonfifo_rx_thres().bits(1); + w.nonfifo_tx_thres().bits(32) }); - self.register_block() - .fifo_conf() - .modify(|_, w| w.tx_fifo_rst().clear_bit().rx_fifo_rst().clear_bit()); + self.register_block().fifo_conf().modify(|_, w| { + w.tx_fifo_rst().clear_bit(); + w.rx_fifo_rst().clear_bit() + }); self.register_block() .int_clr() @@ -2240,44 +2186,37 @@ where I: Iterator, { let cmd = cmd_iterator.next().ok_or(Error::CommandNrExceeded)?; - unsafe { - match command { - Command::Start => { - cmd.write(|w| w.opcode().rstart()); - } - Command::Stop => { - cmd.write(|w| w.opcode().stop()); - } - Command::End => { - cmd.write(|w| w.opcode().end()); - } - Command::Write { - ack_exp, - ack_check_en, - length, - } => { - cmd.write(|w| { - w.opcode().write(); - w.ack_exp().bit(ack_exp == Ack::Nack); - w.ack_check_en().bit(ack_check_en); - w.byte_num().bits(length); - w - }); - } - Command::Read { ack_value, length } => { - cmd.write(|w| { - w.opcode().read(); - w.ack_value().bit(ack_value == Ack::Nack); - w.byte_num().bits(length); - w - }); - } + + match command { + Command::Start => cmd.write(|w| w.opcode().rstart()), + Command::Stop => cmd.write(|w| w.opcode().stop()), + Command::End => cmd.write(|w| w.opcode().end()), + Command::Write { + ack_exp, + ack_check_en, + length, + } => { + cmd.write(|w| unsafe { + w.opcode().write(); + w.ack_exp().bit(ack_exp == Ack::Nack); + w.ack_check_en().bit(ack_check_en); + w.byte_num().bits(length); + w + }); + } + Command::Read { ack_value, length } => { + cmd.write(|w| unsafe { + w.opcode().read(); + w.ack_value().bit(ack_value == Ack::Nack); + w.byte_num().bits(length); + w + }); } } Ok(()) } -#[cfg(not(any(esp32, esp32s2)))] +#[cfg(not(esp32s2))] fn read_fifo(register_block: &RegisterBlock) -> u8 { register_block.data().read().fifo_rdata().bits() } @@ -2300,11 +2239,6 @@ fn read_fifo(register_block: &RegisterBlock) -> u8 { unsafe { (fifo_ptr.read_volatile() & 0xff) as u8 } } -#[cfg(esp32)] -fn read_fifo(register_block: &RegisterBlock) -> u8 { - register_block.data().read().fifo_rdata().bits() -} - #[cfg(esp32)] fn write_fifo(register_block: &RegisterBlock, data: u8) { let base_addr = register_block.scl_low_period().as_ptr(); @@ -2318,14 +2252,16 @@ fn write_fifo(register_block: &RegisterBlock, data: u8) { } } -impl Instance for crate::peripherals::I2C0 { +impl PeripheralMarker for crate::peripherals::I2C0 { #[inline(always)] - fn peripheral() -> crate::system::Peripheral { + fn peripheral(&self) -> crate::system::Peripheral { crate::system::Peripheral::I2cExt0 } +} +impl Instance for crate::peripherals::I2C0 { #[inline(always)] - fn async_handler() -> InterruptHandler { + fn async_handler(&self) -> InterruptHandler { asynch::i2c0_handler } @@ -2360,20 +2296,23 @@ impl Instance for crate::peripherals::I2C0 { } #[inline(always)] - fn interrupt() -> crate::peripherals::Interrupt { + fn interrupt(&self) -> crate::peripherals::Interrupt { crate::peripherals::Interrupt::I2C_EXT0 } } #[cfg(i2c1)] -impl Instance for crate::peripherals::I2C1 { +impl PeripheralMarker for crate::peripherals::I2C1 { #[inline(always)] - fn peripheral() -> crate::system::Peripheral { + fn peripheral(&self) -> crate::system::Peripheral { crate::system::Peripheral::I2cExt1 } +} +#[cfg(i2c1)] +impl Instance for crate::peripherals::I2C1 { #[inline(always)] - fn async_handler() -> InterruptHandler { + fn async_handler(&self) -> InterruptHandler { asynch::i2c1_handler } @@ -2408,11 +2347,40 @@ impl Instance for crate::peripherals::I2C1 { } #[inline(always)] - fn interrupt() -> crate::peripherals::Interrupt { + fn interrupt(&self) -> crate::peripherals::Interrupt { crate::peripherals::Interrupt::I2C_EXT1 } } +crate::any_peripheral! { + /// Represents any I2C peripheral. + pub peripheral AnyI2c { + #[cfg(i2c0)] + I2c0(crate::peripherals::I2C0), + #[cfg(i2c1)] + I2c1(crate::peripherals::I2C1), + } +} + +impl Instance for AnyI2c { + delegate::delegate! { + to match &self.0 { + AnyI2cInner::I2c0(i2c) => i2c, + #[cfg(i2c1)] + AnyI2cInner::I2c1(i2c) => i2c, + } { + fn interrupt(&self) -> crate::peripherals::Interrupt; + fn async_handler(&self) -> InterruptHandler; + fn scl_output_signal(&self) -> OutputSignal; + fn scl_input_signal(&self) -> InputSignal; + fn sda_output_signal(&self) -> OutputSignal; + fn sda_input_signal(&self) -> InputSignal; + fn register_block(&self) -> &RegisterBlock; + fn i2c_number(&self) -> usize; + } + } +} + #[cfg(lp_i2c0)] pub mod lp_i2c { //! Low-power I2C driver diff --git a/examples/src/bin/lcd_cam_ov2640.rs b/examples/src/bin/lcd_cam_ov2640.rs index 4416755334d..cdc6d64c1b2 100644 --- a/examples/src/bin/lcd_cam_ov2640.rs +++ b/examples/src/bin/lcd_cam_ov2640.rs @@ -165,14 +165,14 @@ fn main() -> ! { pub const OV2640_ADDRESS: u8 = 0x30; pub struct Sccb<'d, T> { - i2c: I2c<'d, T, Blocking>, + i2c: I2c<'d, Blocking, T>, } impl<'d, T> Sccb<'d, T> where T: i2c::Instance, { - pub fn new(i2c: I2c<'d, T, Blocking>) -> Self { + pub fn new(i2c: I2c<'d, Blocking, T>) -> Self { Self { i2c } } diff --git a/hil-test/tests/i2c.rs b/hil-test/tests/i2c.rs index c8784abca58..b0d37c72470 100644 --- a/hil-test/tests/i2c.rs +++ b/hil-test/tests/i2c.rs @@ -8,7 +8,6 @@ use esp_hal::{ gpio::Io, i2c::{I2c, Operation}, - peripherals::I2C0, prelude::*, Async, Blocking, @@ -16,11 +15,11 @@ use esp_hal::{ use hil_test as _; struct Context { - i2c: I2c<'static, I2C0, Blocking>, + i2c: I2c<'static, Blocking>, } fn _async_driver_is_compatible_with_blocking_ehal() { - fn _with_driver(driver: I2c<'static, I2C0, Async>) { + fn _with_driver(driver: I2c<'static, Async>) { _with_ehal(driver); }