From d026b26d8492287e2272e64b68979e27cda8ee1c Mon Sep 17 00:00:00 2001 From: Anthony Rocha Date: Thu, 25 Sep 2025 17:06:21 -0700 Subject: [PATCH 1/2] [feat] Start incremental i2c refactoring. Modify HardwareInterface trait. - Introduce init_with_system_control function. - Make init function fallible. --- src/i2c/ast1060_i2c.rs | 25 ++++++++++++++++++++++++- src/i2c/i2c_controller.rs | 13 ++++++++++++- src/tests/functional/i2c_test.rs | 2 +- 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/i2c/ast1060_i2c.rs b/src/i2c/ast1060_i2c.rs index e007a8e..3acf609 100644 --- a/src/i2c/ast1060_i2c.rs +++ b/src/i2c/ast1060_i2c.rs @@ -9,6 +9,7 @@ use ast1060_pac::{I2cglobal, Scu}; use core::cmp::min; use core::fmt::Write; use core::marker::PhantomData; +use core::ops::Drop; use core::sync::atomic::{AtomicBool, Ordering}; use embedded_hal::delay::DelayNs; @@ -305,7 +306,27 @@ macro_rules! i2c_error { impl HardwareInterface for Ast1060I2c<'_, I2C, I2CT, L> { type Error = Error; - fn init(&mut self, config: &mut I2cConfig) { + fn init_with_system_control< + S: proposed_traits::system_control::ResetControl + + proposed_traits::system_control::ClockControl, + >( + &mut self, + system_control: &mut S, + config: &mut I2cConfig, + ) -> Result<(), Self::Error> { + use crate::syscon::ResetId; + + // Enable I2C clock and deassert reset + let reset_id = ResetId::RstI2C; + system_control + .reset_deassert(&reset_id) + .map_err(|_| Error::Bus)?; + + // Initialize the I2C controller + self.init(config) + } + + fn init(&mut self, config: &mut I2cConfig) -> Result<(), Self::Error> { i2c_debug!(self.logger, "i2c init"); i2c_debug!( self.logger, @@ -408,6 +429,8 @@ impl HardwareInterface for Ast1060I2c }); } } + + Ok(()) } #[allow(clippy::too_many_lines)] fn configure_timing(&mut self, config: &mut I2cConfig) { diff --git a/src/i2c/i2c_controller.rs b/src/i2c/i2c_controller.rs index 3332f8f..a363f0f 100644 --- a/src/i2c/i2c_controller.rs +++ b/src/i2c/i2c_controller.rs @@ -2,13 +2,24 @@ use crate::common::{Logger, NoOpLogger}; use crate::i2c::common::I2cConfig; +use core::result::Result; use embedded_hal::i2c::{Operation, SevenBitAddress}; +use proposed_traits::system_control::{ClockControl, ResetControl}; pub trait HardwareInterface { type Error: embedded_hal::i2c::Error + core::fmt::Debug; + fn init_with_system_control< + S: ResetControl + + ClockControl, + >( + &mut self, + system_control: &mut S, + config: &mut I2cConfig, + ) -> Result<(), Self::Error>; + // Methods return hardware-specific errors - fn init(&mut self, config: &mut I2cConfig); + fn init(&mut self, config: &mut I2cConfig) -> Result<(), Self::Error>; fn configure_timing(&mut self, config: &mut I2cConfig); fn enable_interrupts(&mut self, mask: u32); fn clear_interrupts(&mut self, mask: u32); diff --git a/src/tests/functional/i2c_test.rs b/src/tests/functional/i2c_test.rs index 2f33a50..233d5c8 100644 --- a/src/tests/functional/i2c_test.rs +++ b/src/tests/functional/i2c_test.rs @@ -122,7 +122,7 @@ pub fn test_i2c_master(uart: &mut UartController<'_>) { }; pinctrl::Pinctrl::apply_pinctrl_group(pinctrl::PINCTRL_I2C1); - i2c1.hardware.init(&mut i2c1.config); + let _ = i2c1.hardware.init(&mut i2c1.config); let addr = 0x2e; //device ADT7490 let mut buf = [0x4e]; From 1d0628d15197e41968f27ecf6b5b411a9d41dfcc Mon Sep 17 00:00:00 2001 From: Anthony Rocha Date: Thu, 25 Sep 2025 18:03:41 -0700 Subject: [PATCH 2/2] [feat] I2c refactor to enable system level resource management. 1 - Introduce `init_peripheral_only()` that handles only: - I2C controller register setup (`i2c.i2cc00()`) - Interrupt configuration (`i2c.i2cm10()`, `i2c.i2cm14()`) - Slave mode setup (if enabled) - Timing configuration (peripheral-level only) 2. Introduce init_i2c_global_system(system_control: &mut S) --- src/i2c/ast1060_i2c.rs | 143 ++++++++++++++++++++++++++++--- src/tests/functional/i2c_test.rs | 98 +++++++++++++++++++++ 2 files changed, 231 insertions(+), 10 deletions(-) diff --git a/src/i2c/ast1060_i2c.rs b/src/i2c/ast1060_i2c.rs index 3acf609..0f11674 100644 --- a/src/i2c/ast1060_i2c.rs +++ b/src/i2c/ast1060_i2c.rs @@ -1,5 +1,4 @@ // Licensed under the Apache-2.0 license - use crate::common::{DmaBuffer, DummyDelay, Logger}; #[cfg(feature = "i2c_target")] use crate::i2c::common::I2cSEvent; @@ -10,6 +9,7 @@ use core::cmp::min; use core::fmt::Write; use core::marker::PhantomData; use core::ops::Drop; +use core::result::Result; use core::sync::atomic::{AtomicBool, Ordering}; use embedded_hal::delay::DelayNs; @@ -314,16 +314,11 @@ impl HardwareInterface for Ast1060I2c system_control: &mut S, config: &mut I2cConfig, ) -> Result<(), Self::Error> { - use crate::syscon::ResetId; + // Handle system-level initialization + Self::init_i2c_global_system(system_control)?; - // Enable I2C clock and deassert reset - let reset_id = ResetId::RstI2C; - system_control - .reset_deassert(&reset_id) - .map_err(|_| Error::Bus)?; - - // Initialize the I2C controller - self.init(config) + // Handle peripheral-level initialization (without SCU coupling) + self.init_peripheral_only(config) } fn init(&mut self, config: &mut I2cConfig) -> Result<(), Self::Error> { @@ -1820,6 +1815,134 @@ impl<'a, I2C: Instance, I2CT: I2CTarget, L: Logger> Ast1060I2c<'a, I2C, I2CT, L> // Fallthrough is success Ok(()) } + + /// Initialize the global I2C system using system control interface + fn init_i2c_global_system< + S: proposed_traits::system_control::ResetControl, + >( + system_control: &mut S, + ) -> Result<(), Error> { + use crate::syscon::ResetId; + + if I2CGLOBAL_INIT + .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) + .is_ok() + { + // Use system_control for reset operations instead of direct SCU access + let reset_id = ResetId::RstI2C; + system_control + .reset_assert(&reset_id) + .map_err(|_| Error::Bus)?; + + let mut delay = DummyDelay {}; + delay.delay_ns(1_000_000); // 1ms delay + + system_control + .reset_deassert(&reset_id) + .map_err(|_| Error::Bus)?; + delay.delay_ns(1_000_000); // 1ms delay + + // I2C global configuration (still needs I2cglobal access) + let i2cg = unsafe { &*I2cglobal::ptr() }; + i2cg.i2cg0c().write(|w| { + w.clk_divider_mode_sel() + .set_bit() + .reg_definition_sel() + .set_bit() + .select_the_action_when_slave_pkt_mode_rxbuf_empty() + .set_bit() + }); + /* + * APB clk : 50Mhz + * div : scl : baseclk [APB/((div/2) + 1)] : tBuf [1/bclk * 16] + * I2CG10[31:24] base clk4 for i2c auto recovery timeout counter (0x62) + * I2CG10[23:16] base clk3 for Standard-mode (100Khz) min tBuf 4.7us + * 0x1d : 100.8Khz : 3.225Mhz : 4.96us + * 0x1e : 97.66Khz : 3.125Mhz : 5.12us + * 0x1f : 97.85Khz : 3.03Mhz : 5.28us + * 0x20 : 98.04Khz : 2.94Mhz : 5.44us + * 0x21 : 98.61Khz : 2.857Mhz : 5.6us + * 0x22 : 99.21Khz : 2.77Mhz : 5.76us (default) + * I2CG10[15:8] base clk2 for Fast-mode (400Khz) min tBuf 1.3us + * 0x08 : 400Khz : 10Mhz : 1.6us + * I2CG10[7:0] base clk1 for Fast-mode Plus (1Mhz) min tBuf 0.5us + * 0x03 : 1Mhz : 20Mhz : 0.8us + */ + i2cg.i2cg10().write(|w| unsafe { w.bits(0x6222_0803) }); + } + Ok(()) + } + + /// Initialize only the peripheral-level I2C controller (no SCU coupling) + #[allow(clippy::unnecessary_wraps)] + fn init_peripheral_only(&mut self, config: &mut I2cConfig) -> Result<(), Error> { + i2c_debug!(self.logger, "i2c peripheral init"); + i2c_debug!( + self.logger, + "mdma_buf {:p}, sdma_buf {:p}", + self.mdma_buf.as_ptr(), + self.sdma_buf.as_ptr() + ); + + // Store configuration + self.xfer_mode = config.xfer_mode; + self.multi_master = config.multi_master; + self.smbus_alert = config.smbus_alert; + + // I2C peripheral reset and configuration (no SCU dependency) + self.i2c.i2cc00().write(|w| unsafe { w.bits(0) }); + if !self.multi_master { + self.i2c + .i2cc00() + .write(|w| w.dis_multimaster_capability_for_master_fn_only().set_bit()); + } + self.i2c.i2cc00().write(|w| { + w.enbl_bus_autorelease_when_scllow_sdalow_or_slave_mode_inactive_timeout() + .set_bit() + .enbl_master_fn() + .set_bit() + }); + + // set AC timing + self.configure_timing(config); + // clear interrupts + self.i2c.i2cm14().write(|w| unsafe { w.bits(0xffff_ffff) }); + // set interrupt + self.i2c.i2cm10().write(|w| { + w.enbl_pkt_cmd_done_int() + .set_bit() + .enbl_bus_recover_done_int() + .set_bit() + }); + i2c_debug!( + self.logger, + "i2c init after set interrupt: {:#x}", + self.i2c.i2cm14().read().bits() + ); + if self.smbus_alert { + self.i2c + .i2cm10() + .write(|w| w.enbl_smbus_dev_alert_int().set_bit()); + } + + if cfg!(feature = "i2c_target") { + i2c_debug!(self.logger, "i2c target enabled"); + // clear slave interrupts + self.i2c.i2cs24().write(|w| unsafe { w.bits(0xffff_ffff) }); + if self.xfer_mode == I2cXferMode::ByteMode { + self.i2c.i2cs20().write(|w| unsafe { w.bits(0xffff) }); + } else { + self.i2c.i2cs20().write(|w| { + w.enbl_slave_mode_inactive_timeout_int() + .set_bit() + .enbl_pkt_cmd_done_int() + .set_bit() + }); + } + } + + Ok(()) + } } macro_rules! transaction_impl { diff --git a/src/tests/functional/i2c_test.rs b/src/tests/functional/i2c_test.rs index 233d5c8..8444f65 100644 --- a/src/tests/functional/i2c_test.rs +++ b/src/tests/functional/i2c_test.rs @@ -9,6 +9,7 @@ use crate::uart::{self, Config, UartController}; use ast1060_pac::Peripherals; #[cfg(feature = "i2c_target")] use cortex_m::peripheral::NVIC; +use embedded_hal::delay::DelayNs; use embedded_hal::i2c::ErrorKind; use embedded_io::Write; use proposed_traits::i2c_target::{ @@ -301,3 +302,100 @@ pub fn test_i2c_slave(uart: &mut UartController<'_>) { NVIC::unmask(ast1060_pac::Interrupt::i2c); } } + +/// Test the new `init_with_system_control` functionality +pub fn test_i2c_init_with_system_control( + uart: &mut UartController<'_>, + syscon: &mut crate::syscon::SysCon, +) { + use crate::common::DummyDelay; + use ast1060_pac::Peripherals; + + writeln!( + uart, + "\r\n####### I2C init_with_system_control test #######\r\n" + ) + .unwrap(); + + let peripherals = unsafe { Peripherals::steal() }; + let mut delay = DummyDelay {}; + let mut dbg_uart = UartController::new(peripherals.uart, &mut delay); + + unsafe { + dbg_uart.init(&Config { + baud_rate: 115_200, + word_length: uart::WordLength::Eight as u8, + parity: uart::Parity::None, + stop_bits: uart::StopBits::One, + clock: 24_000_000, + }); + } + + let i2c_config = I2cConfigBuilder::new() + .xfer_mode(I2cXferMode::DmaMode) + .multi_master(true) + .smbus_timeout(true) + .smbus_alert(false) + .speed(I2cSpeed::Standard) + .build(); + + let mut i2c1: I2cController< + Ast1060I2c, + NoOpLogger, + > = I2cController { + hardware: Ast1060I2c::new(UartLogger::new(&mut dbg_uart)), + config: i2c_config, + logger: NoOpLogger {}, + }; + + // Apply pin control for I2C1 + pinctrl::Pinctrl::apply_pinctrl_group(pinctrl::PINCTRL_I2C1); + + // Test the new init_with_system_control function + match i2c1 + .hardware + .init_with_system_control(syscon, &mut i2c1.config) + { + Ok(()) => { + writeln!(uart, "✅ init_with_system_control succeeded\r").unwrap(); + + // Test basic I2C functionality after system control init + let addr = 0x2e; // ADT7490 device + let mut buf = [0x4e]; + + // Test write operation + match i2c1.hardware.write(addr, &buf) { + Ok(()) => { + writeln!(uart, "✅ I2C write after syscon init: OK\r").unwrap(); + } + Err(e) => { + writeln!(uart, "⚠️ I2C write after syscon init failed: {e:?}\r").unwrap(); + } + } + + // Test read operation + match i2c1.hardware.read(addr, &mut buf) { + Ok(()) => { + writeln!( + uart, + "✅ I2C read after syscon init: OK, data={:#x}\r", + buf[0] + ) + .unwrap(); + } + Err(e) => { + writeln!(uart, "⚠️ I2C read after syscon init failed: {e:?}\r").unwrap(); + } + } + } + Err(e) => { + writeln!(uart, "❌ init_with_system_control failed: {e:?}\r").unwrap(); + } + } + + writeln!( + uart, + "####### I2C init_with_system_control test complete #######\r\n" + ) + .unwrap(); +}