From 9c0512c1be481d7fcda8821b0ed91faf7dc95627 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 7 May 2025 15:47:58 +0200 Subject: [PATCH] Hopefully innocent I2C-related cleanups (#3460) * Simplify test * Clean up --- esp-hal/src/i2c/master/mod.rs | 70 ++++++++++++----------------------- hil-test/tests/i2c.rs | 52 +++++++++----------------- 2 files changed, 41 insertions(+), 81 deletions(-) diff --git a/esp-hal/src/i2c/master/mod.rs b/esp-hal/src/i2c/master/mod.rs index 6764f62fe..938937c44 100644 --- a/esp-hal/src/i2c/master/mod.rs +++ b/esp-hal/src/i2c/master/mod.rs @@ -54,19 +54,8 @@ use crate::{ time::Rate, }; -cfg_if::cfg_if! { - if #[cfg(esp32s2)] { - const I2C_LL_INTR_MASK: u32 = 0x1ffff; - } else { - const I2C_LL_INTR_MASK: u32 = 0x3ffff; - } -} - -#[cfg(not(esp32c2))] -const I2C_FIFO_SIZE: usize = 32; - -#[cfg(esp32c2)] -const I2C_FIFO_SIZE: usize = 16; +const I2C_LL_INTR_MASK: u32 = if cfg!(esp32s2) { 0x1ffff } else { 0x3ffff }; +const I2C_FIFO_SIZE: usize = if cfg!(esp32c2) { 16 } else { 32 }; // Chunk writes/reads by this size const I2C_CHUNK_SIZE: usize = I2C_FIFO_SIZE - 1; @@ -145,14 +134,15 @@ pub enum BusTimeout { impl BusTimeout { fn cycles(&self) -> u32 { match self { - #[cfg(esp32)] - BusTimeout::Maximum => 0xF_FFFF, - - #[cfg(esp32s2)] - BusTimeout::Maximum => 0xFF_FFFF, - - #[cfg(not(any(esp32, esp32s2)))] - BusTimeout::Maximum => 0x1F, + BusTimeout::Maximum => { + if cfg!(esp32) { + 0xF_FFFF + } else if cfg!(esp32s2) { + 0xFF_FFFF + } else { + 0x1F + } + } #[cfg(not(any(esp32, esp32s2)))] BusTimeout::Disabled => 1, @@ -707,9 +697,10 @@ impl<'a> I2cFuture<'a> { w.time_out().set_bit(); w.nack().set_bit(); #[cfg(not(any(esp32, esp32s2)))] - w.scl_main_st_to().set_bit(); - #[cfg(not(any(esp32, esp32s2)))] - w.scl_st_to().set_bit(); + { + w.scl_main_st_to().set_bit(); + w.scl_st_to().set_bit(); + } w }); @@ -746,13 +737,13 @@ impl<'a> I2cFuture<'a> { } #[cfg(not(any(esp32, esp32s2)))] - if r.scl_st_to().bit_is_set() { - return Err(Error::Timeout); - } - - #[cfg(not(any(esp32, esp32s2)))] - if r.scl_main_st_to().bit_is_set() { - return Err(Error::Timeout); + { + if r.scl_st_to().bit_is_set() { + return Err(Error::Timeout); + } + if r.scl_main_st_to().bit_is_set() { + return Err(Error::Timeout); + } } #[cfg(not(esp32))] @@ -1115,22 +1106,7 @@ impl embedded_hal_async::i2c::I2c for I2c<'_, Async> { } fn async_handler(info: &Info, state: &State) { - let regs = info.regs(); - regs.int_ena().modify(|_, w| { - w.end_detect().clear_bit(); - w.trans_complete().clear_bit(); - w.arbitration_lost().clear_bit(); - w.time_out().clear_bit(); - #[cfg(not(esp32))] - w.scl_main_st_to().clear_bit(); - #[cfg(not(esp32))] - w.scl_st_to().clear_bit(); - - #[cfg(not(any(esp32, esp32s2)))] - w.txfifo_wm().clear_bit(); - - w.nack().clear_bit() - }); + info.regs().int_ena().write(|w| unsafe { w.bits(0) }); state.waker.wake(); } diff --git a/hil-test/tests/i2c.rs b/hil-test/tests/i2c.rs index 0f173224c..550857371 100644 --- a/hil-test/tests/i2c.rs +++ b/hil-test/tests/i2c.rs @@ -69,23 +69,16 @@ mod tests { fn empty_write_returns_ack_error_for_unknown_address(mut ctx: Context) { // on some chips we can determine the ack-check-failed reason but not on all // chips - cfg_if::cfg_if! { - if #[cfg(any(esp32,esp32s2,esp32c2,esp32c3))] { - assert_eq!( - ctx.i2c.write(NON_EXISTENT_ADDRESS, &[]), - Err(Error::AcknowledgeCheckFailed( - AcknowledgeCheckFailedReason::Unknown - )) - ); - } else { - assert_eq!( - ctx.i2c.write(NON_EXISTENT_ADDRESS, &[]), - Err(Error::AcknowledgeCheckFailed( - AcknowledgeCheckFailedReason::Address - )) - ); - } - } + let reason = if cfg!(any(esp32, esp32s2, esp32c2, esp32c3)) { + AcknowledgeCheckFailedReason::Unknown + } else { + AcknowledgeCheckFailedReason::Address + }; + + assert_eq!( + ctx.i2c.write(NON_EXISTENT_ADDRESS, &[]), + Err(Error::AcknowledgeCheckFailed(reason)) + ); assert_eq!(ctx.i2c.write(DUT_ADDRESS, &[]), Ok(())); } @@ -129,23 +122,14 @@ mod tests { // on some chips we can determine the ack-check-failed reason but not on all // chips - cfg_if::cfg_if! { - if #[cfg(any(esp32,esp32s2,esp32c2,esp32c3))] { - assert_eq!( - i2c.write_async(NON_EXISTENT_ADDRESS, &[]).await, - Err(Error::AcknowledgeCheckFailed( - AcknowledgeCheckFailedReason::Unknown - )) - ); - } else { - assert_eq!( - i2c.write_async(NON_EXISTENT_ADDRESS, &[]).await, - Err(Error::AcknowledgeCheckFailed( - AcknowledgeCheckFailedReason::Address - )) - ); - } - } + let reason = if cfg!(any(esp32, esp32s2, esp32c2, esp32c3)) { + AcknowledgeCheckFailedReason::Unknown + } else { + AcknowledgeCheckFailedReason::Address + }; + + let should_fail = i2c.write_async(NON_EXISTENT_ADDRESS, &[]).await; + assert_eq!(should_fail, Err(Error::AcknowledgeCheckFailed(reason))); assert_eq!(i2c.write_async(DUT_ADDRESS, &[]).await, Ok(())); }