Pin I3C controller before IRQ init
diff --git a/target/ast10x0/peripherals/i3c/controller.rs b/target/ast10x0/peripherals/i3c/controller.rs index 1871019..704a995 100644 --- a/target/ast10x0/peripherals/i3c/controller.rs +++ b/target/ast10x0/peripherals/i3c/controller.rs
@@ -7,12 +7,12 @@ //! //! # Construction Patterns //! -//! Two construction paths are provided: +//! The controller uses an explicit two-stage bring-up: //! -//! | Constructor | Purpose | Performance | Use Case | -//! |-------------|---------|-------------|----------| -//! | [`new()`](I3cController::new) | Full hardware init | Slower (register writes) | First-time setup, reset | -//! | [`from_initialized()`](I3cController::from_initialized) | Wrap pre-configured HW | Fast (no I/O) | Per-operation, hot path | +//! | Step | Purpose | Performance | Use Case | +//! |------|---------|-------------|----------| +//! | [`new()`](I3cController::new) / [`from_initialized()`](I3cController::from_initialized) | Construct controller value only | Fast (no I/O) | Build the owner that will be pinned | +//! | [`init_hardware()`](I3cController::init_hardware) | Register IRQ handler + program hardware | Slower (register writes) | First-time setup after the controller is pinned | //! //! # Example //! @@ -22,14 +22,18 @@ //! scu.enable_i3c_clock(bus); //! scu.deassert_i3c_reset(bus); //! -//! // Full hardware init -//! let mut ctrl = I3cController::new(hw, config)?; +//! // Construct, pin, then initialize so the IRQ handler sees a stable address. +//! let mut ctrl = core::pin::pin!(I3cController::new(hw, config)); +//! ctrl.as_mut().init_hardware(); //! //! // === HOT PATH (hardware already configured) === //! let ctrl = I3cController::from_initialized(hw, config); //! ctrl.do_transfer(...); //! ``` +use core::marker::PhantomPinned; +use core::pin::Pin; + use super::ccc; use super::config::{DeviceEntry, I3cConfig, I3cTargetConfig}; use super::constants::I3C_BROADCAST_ADDR; @@ -44,6 +48,7 @@ pub hw: H, /// Bus configuration pub config: I3cConfig, + _pin: PhantomPinned, } impl<H: HardwareInterface> I3cController<H> { @@ -51,26 +56,11 @@ // Construction // ========================================================================= - /// Create and initialize I3C controller (full init) + /// Construct an I3C controller value without touching hardware. /// - /// Performs complete hardware initialization: - /// - Registers IRQ handler - /// - Enables interrupts - /// - Initializes hardware registers - /// - /// Use [`from_initialized`](Self::from_initialized) if hardware is already - /// configured. - /// - /// # Preconditions - /// - /// Platform initialization must be done before calling this: - /// - Clocks enabled (via SCU) - /// - Reset deasserted (via SCU) - /// - Pin mux configured - /// - /// # Returns - /// - /// Initialized controller ready for use. + /// This does **not** register an IRQ handler or program registers. Call + /// [`init_hardware`](Self::init_hardware) after pinning the controller to a + /// stable address. pub fn new(hw: H, config: I3cConfig) -> Self { Self::from_initialized(hw, config) } @@ -97,33 +87,34 @@ /// No register writes - significantly faster than `new()`. #[must_use] pub fn from_initialized(hw: H, config: I3cConfig) -> Self { - Self { hw, config } + Self { + hw, + config, + _pin: PhantomPinned, + } } /// Initialize/reinitialize hardware registers /// /// Registers the IRQ handler and configures the hardware. - /// Called automatically by [`new()`](Self::new), but can be called - /// explicitly to reinitialize after error recovery. /// - /// # Safety Invariant - /// - /// After calling this method, the caller must ensure that no `&mut self` - /// methods are called while interrupts are enabled, as the IRQ handler - /// also takes `&mut self`. Violation causes undefined behavior. - pub fn init_hardware(&mut self) { - let ctx = core::ptr::from_mut::<Self>(self) as usize; - let bus = self.hw.bus_num() as usize; + /// This method requires a pinned controller so the IRQ registry can keep a + /// stable pointer to it. The target/kernel owns the top-level interrupt + /// vector; its ISR should call [`dispatch_i3c_irq`](super::hardware::dispatch_i3c_irq). + pub fn init_hardware(self: Pin<&mut Self>) { + let this = unsafe { self.get_unchecked_mut() }; + let ctx = core::ptr::from_mut::<Self>(this) as usize; + let bus = this.hw.bus_num() as usize; super::hardware::register_i3c_irq_handler(bus, Self::irq_trampoline, ctx); // IMPORTANT: init() must complete before enable_irq() to prevent // IRQ firing on partially-initialized hardware - self.hw.init(&mut self.config); + this.hw.init(&mut this.config); // Memory barrier to ensure init writes are visible before IRQ enable cortex_m::asm::dmb(); - self.hw.enable_irq(); + this.hw.enable_irq(); } /// IRQ trampoline function @@ -134,6 +125,16 @@ ctrl.hw.i3c_aspeed_isr(&mut ctrl.config); } + #[inline] + fn project_mut(self: Pin<&mut Self>) -> &mut Self { + unsafe { self.get_unchecked_mut() } + } + + #[inline] + fn project_ref(self: Pin<&Self>) -> &Self { + Pin::get_ref(self) + } + // ========================================================================= // Device Management // ========================================================================= @@ -144,7 +145,13 @@ /// * `pid` - Provisional ID of the device /// * `desired_da` - Desired dynamic address /// * `slot` - DAT slot to use - pub fn attach_i3c_dev(&mut self, pid: u64, desired_da: u8, slot: u8) -> Result<(), I3cError> { + pub fn attach_i3c_dev( + self: Pin<&mut Self>, + pid: u64, + desired_da: u8, + slot: u8, + ) -> Result<(), I3cError> { + let this = self.project_mut(); if desired_da == 0 || desired_da >= I3C_BROADCAST_ADDR { return Err(I3cError::InvalidArgs); } @@ -166,45 +173,47 @@ pos: Some(slot), }; - let idx = self + let idx = this .config .attached .attach(dev) .map_err(|_| I3cError::AddrInUse)?; - self.config + this.config .attached .map_pos(slot, u8::try_from(idx).map_err(|_| I3cError::InvalidArgs)?); - self.config.addrbook.mark_use(desired_da, true); + this.config.addrbook.mark_use(desired_da, true); - self.hw + this.hw .attach_i3c_dev(slot.into(), desired_da) .map_err(|_| I3cError::AddrInUse) } /// Detach an I3C device by DAT position - pub fn detach_i3c_dev(&mut self, pos: usize) { - self.config.attached.detach_by_pos(pos); - self.hw.detach_i3c_dev(pos); + pub fn detach_i3c_dev(self: Pin<&mut Self>, pos: usize) { + let this = self.project_mut(); + this.config.attached.detach_by_pos(pos); + this.hw.detach_i3c_dev(pos); } /// Detach an I3C device by device index - pub fn detach_i3c_dev_by_idx(&mut self, dev_idx: usize) { + pub fn detach_i3c_dev_by_idx(self: Pin<&mut Self>, dev_idx: usize) { + let this = self.project_mut(); // `get` (not `[dev_idx]`) keeps this panic-free for the `no_panics` // analysis; an out-of-range index is simply a no-op. - let Some(dev) = self.config.attached.devices.get(dev_idx) else { + let Some(dev) = this.config.attached.devices.get(dev_idx) else { return; }; if dev.dyn_addr != 0 { - self.config.addrbook.mark_use(dev.dyn_addr, false); + this.config.addrbook.mark_use(dev.dyn_addr, false); } let dev_pos = dev.pos; if let Some(pos) = dev_pos { - self.hw.detach_i3c_dev(pos.into()); + this.hw.detach_i3c_dev(pos.into()); } - self.config.attached.detach(dev_idx); + this.config.attached.detach(dev_idx); } // ========================================================================= @@ -237,11 +246,12 @@ /// // More aggressive recovery /// ctrl.recover_bus(18); /// ``` - pub fn recover_bus(&mut self, scl_toggles: u32) { - self.hw.enter_sw_mode(); - self.hw.i3c_toggle_scl_in(scl_toggles); - self.hw.gen_internal_stop(); - self.hw.exit_sw_mode(); + pub fn recover_bus(self: Pin<&mut Self>, scl_toggles: u32) { + let this = self.project_mut(); + this.hw.enter_sw_mode(); + this.hw.i3c_toggle_scl_in(scl_toggles); + this.hw.gen_internal_stop(); + this.hw.exit_sw_mode(); } /// Perform full bus recovery with controller reset @@ -263,9 +273,9 @@ /// let reset = RESET_CTRL_RX_FIFO | RESET_CTRL_TX_FIFO | RESET_CTRL_CMD_QUEUE; /// ctrl.recover_bus_full(reset); /// ``` - pub fn recover_bus_full(&mut self, reset_mask: u32) { - self.recover_bus(8); - self.hw.reset_ctrl(reset_mask); + pub fn recover_bus_full(mut self: Pin<&mut Self>, reset_mask: u32) { + self.as_mut().recover_bus(8); + self.project_mut().hw.reset_ctrl(reset_mask); } // Accessors @@ -273,26 +283,37 @@ /// Get a reference to the hardware interface #[inline] - pub fn hw(&self) -> &H { - &self.hw + pub fn hw(self: Pin<&Self>) -> &H { + &self.project_ref().hw } /// Get a mutable reference to the hardware interface #[inline] - pub fn hw_mut(&mut self) -> &mut H { - &mut self.hw + pub fn hw_mut(self: Pin<&mut Self>) -> &mut H { + &mut self.project_mut().hw } /// Get a reference to the configuration #[inline] - pub fn config(&self) -> &I3cConfig { - &self.config + pub fn config(self: Pin<&Self>) -> &I3cConfig { + &self.project_ref().config } /// Get a mutable reference to the configuration #[inline] - pub fn config_mut(&mut self) -> &mut I3cConfig { - &mut self.config + pub fn config_mut(self: Pin<&mut Self>) -> &mut I3cConfig { + &mut self.project_mut().config + } + + /// Borrow the hardware interface and config together for a single + /// controller-local operation. + #[inline] + pub fn with_hw_and_config<R>( + self: Pin<&mut Self>, + f: impl FnOnce(&mut H, &mut I3cConfig) -> R, + ) -> R { + let this = self.project_mut(); + f(&mut this.hw, &mut this.config) } } @@ -325,29 +346,30 @@ /// Assign a dynamic address to the device at `static_address` via ENTDAA, /// then read back PID/BCR and enable IBI. Returns the assigned address. pub fn assign_dynamic_address( - &mut self, + self: Pin<&mut Self>, static_address: SevenBitAddress, ) -> Result<SevenBitAddress, I3cError> { - let slot = self + let this = self.project_mut(); + let slot = this .config .attached .pos_of_addr(static_address) .ok_or(I3cError::AddrInUse)?; - self.hw - .do_entdaa(&mut self.config, slot.into()) + this.hw + .do_entdaa(&mut this.config, slot.into()) .map_err(|_| I3cError::AddrInUse)?; - let pid = ccc::ccc_getpid(&mut self.hw, &mut self.config, static_address) + let pid = ccc::ccc_getpid(&mut this.hw, &mut this.config, static_address) .map_err(|_| I3cError::Invalid)?; - let dev_idx = self + let dev_idx = this .config .attached .find_dev_idx_by_addr(static_address) .ok_or(I3cError::Other)?; - let old_pid = self + let old_pid = this .config .attached .devices @@ -361,11 +383,11 @@ return Err(I3cError::Other); } - let bcr = ccc::ccc_getbcr(&mut self.hw, &mut self.config, static_address) + let bcr = ccc::ccc_getbcr(&mut this.hw, &mut this.config, static_address) .map_err(|_| I3cError::Invalid)?; { - let dev = self + let dev = this .config .attached .devices @@ -376,7 +398,7 @@ dev.bcr = bcr; } - let dyn_addr: SevenBitAddress = self + let dyn_addr: SevenBitAddress = this .config .attached .devices @@ -384,16 +406,17 @@ .ok_or(I3cError::Other)? .dyn_addr; - self.hw - .ibi_enable(&mut self.config, dyn_addr) + this.hw + .ibi_enable(&mut this.config, dyn_addr) .map_err(|_| I3cError::Other)?; Ok(dyn_addr) } /// Acknowledge an IBI from `address` (validates the device is known). - pub fn acknowledge_ibi(&mut self, address: SevenBitAddress) -> Result<(), I3cError> { - let dev_idx = self + pub fn acknowledge_ibi(self: Pin<&mut Self>, address: SevenBitAddress) -> Result<(), I3cError> { + let this = self.project_mut(); + let dev_idx = this .config .attached .find_dev_idx_by_addr(address) @@ -401,7 +424,7 @@ // `get` (not `[dev_idx]`) keeps this panic-free for the `no_panics` // analysis; `find_dev_idx_by_addr` already returns a valid index. - let dev = self + let dev = this .config .attached .devices @@ -417,59 +440,69 @@ /// Hot-join handler hook. Call [`assign_dynamic_address`](Self::assign_dynamic_address) /// after receiving a hot-join IBI; nothing else is required here. #[allow(clippy::unused_self)] - pub fn handle_hot_join(&mut self) -> Result<(), I3cError> { + pub fn handle_hot_join(self: Pin<&mut Self>) -> Result<(), I3cError> { Ok(()) } /// Bus speed is fixed on the AST1060 controller; this is a no-op. #[allow(clippy::unused_self)] - pub fn set_bus_speed(&mut self) -> Result<(), I3cError> { + pub fn set_bus_speed(self: Pin<&mut Self>) -> Result<(), I3cError> { Ok(()) } /// The AST1060 controller does not support multi-master; this is a no-op. #[allow(clippy::unused_self)] - pub fn request_mastership(&mut self) -> Result<(), I3cError> { + pub fn request_mastership(self: Pin<&mut Self>) -> Result<(), I3cError> { Ok(()) } // --- Target (secondary) mode callbacks --- /// Initialize target mode with `own_addr` (sets the static/target address). - pub fn target_init(&mut self, own_addr: u8) { - if let Some(t) = self.config.target_config.as_mut() { + pub fn target_init(self: Pin<&mut Self>, own_addr: u8) { + let this = self.project_mut(); + if let Some(t) = this.config.target_config.as_mut() { if t.addr.is_none() { t.addr = Some(own_addr); } } else { - self.config.target_config = + this.config.target_config = Some(I3cTargetConfig::new(0, Some(own_addr), /* mdb */ 0xae)); } } /// Returns `true` if `addr` matches this target's assigned address. #[must_use] - pub fn target_on_address_match(&self, addr: u8) -> bool { - self.config.target_config.as_ref().and_then(|t| t.addr) == Some(addr) + pub fn target_on_address_match(self: Pin<&Self>, addr: u8) -> bool { + self.project_ref() + .config + .target_config + .as_ref() + .and_then(|t| t.addr) + == Some(addr) } /// Record that the controller assigned this target a dynamic address; SIRs /// are then permitted by software. - pub fn target_on_dynamic_address_assigned(&mut self) { - self.config.sir_allowed_by_sw = true; + pub fn target_on_dynamic_address_assigned(self: Pin<&mut Self>) { + self.project_mut().config.sir_allowed_by_sw = true; } /// This target always wants to raise IBIs when it has data. #[must_use] #[allow(clippy::unused_self)] - pub fn target_wants_ibi(&self) -> bool { + pub fn target_wants_ibi(self: Pin<&Self>) -> bool { true } /// Build and submit the IBI payload `[mdb, crc8_ccitt(addr_rnw, mdb)]` for a /// pending target read, returning the number of bytes made available. - pub fn target_get_ibi_payload(&mut self, buffer: &mut [u8]) -> Result<usize, I3cError> { - let (da, mdb) = match self.config.target_config.as_ref() { + pub fn target_get_ibi_payload( + self: Pin<&mut Self>, + buffer: &mut [u8], + ) -> Result<usize, I3cError> { + let this = self.project_mut(); + let (da, mdb) = match this.config.target_config.as_ref() { Some(t) => ( match t.addr { Some(da) => da, @@ -489,9 +522,9 @@ ibi_type: I3cIbiType::TargetIntr, payload: Some(&payload), }; - let rc = self + let rc = this .hw - .target_pending_read_notify(&mut self.config, buffer, &mut ibi); + .target_pending_read_notify(&mut this.config, buffer, &mut ibi); match rc { Ok(()) => Ok(buffer.len() + payload.len()),
diff --git a/target/ast10x0/peripherals/i3c/plans/goal.md b/target/ast10x0/peripherals/i3c/plans/goal.md deleted file mode 100644 index 4b230b8..0000000 --- a/target/ast10x0/peripherals/i3c/plans/goal.md +++ /dev/null
@@ -1,427 +0,0 @@ -# I3C Behavioral Parity Goal (AST10x0 / openprot) - -## Objective - -- **Authority** = aspeed-rust `src/i3c/` @ `ce3b567` (frozen 2026-06-02; - pinned at `plans/i3c-reference/PINNED_COMMIT.txt`). -- **Informative-only**: DesignWare/Zephyr/Linux i3c controller drivers (register - semantics only); `proposed_traits` @ `85641310` (operation *shape* only, not - available in openprot). Authority wins on any divergence; informative refs are - treated as not-our-target. -- **Parity standard (decided)**: *Observable parity, keep fixes.* The port is - behaviorally equivalent to aspeed-rust on the success path; panicking slice - indexing / unchecked arithmetic may be hardened to typed errors, and each such - fix is recorded in the deltas ledger (§2). This mirrors the decision made for - the I2C port (`peripherals/i2c/master.rs` swapped `&bytes[a..b]` → - `.get(..).ok_or(I2cError::Invalid)?`). -- **Scope (decided)**: full 1:1 functional port — master + target(secondary) + - IBI/hot-join + CCC + legacy-I2C device support. AST10x0 only. -- **Design-pattern depth (decided)**: mirror the I2C port. Apply the three - `pac-design-patterns` structural patterns at the same depth the I2C port did; - keep aspeed-rust's `HardwareInterface` 6-trait split and its global IBI/IRQ - statics, recording those globals as an intentional delta vs. - *Borrow-Arbitrated Engine Exclusivity* (§5 ADR-3). - ---- - -## 1. Reference behavior to replicate (Phase 1 — every claim cites authority `file:line`) - -Paths below are relative to `aspeed-rust/src/i3c/`. - -### 1.1 Module / type model -- `I3cController<H: HardwareInterface>` wraps `hw: H` + `config: I3cConfig` - (`controller.rs:39-44`). `new` == `from_initialized` (no I/O); - `init_hardware` does the register init (`controller.rs:71-124`). -- Concrete hardware impl `Ast1060I3c<I3C: Instance, L: Logger>` holds - `&'static` register blocks for `i3c`, `i3cg`, `scu` + a `Logger` - (`hardware.rs:419-440`). Built from the `Instance` trait - (`hardware.rs:338-367`, buses 0..3 via `macro_i3c!`). -- `HardwareInterface` = supertrait of `HardwareCore + HardwareClock + - HardwareFifo + HardwareTransfer + HardwareRecovery + HardwareTarget` - (`hardware.rs:6-19`, blanket impl `hardware.rs:328-336`). - -### 1.2 Init / reset / clock sequence (`hardware.rs:677-854`) -1. `global_reset_deassert()`, then program `i3cg` reg1 (actmode=1, instid=bus, - staticaddr=0x74) and reg0=0 (`hardware.rs:680-695`). -2. `core_reset_assert` → `clock_on` → `core_reset_deassert` → `i3c_disable` - (`hardware.rs:699-702`). **Clock + reset live inside the driver**, via the - `scu`/`i3cg` registers — unlike the I2C port, which delegated SCU to the - board. (See §5 ADR-2.) -3. Soft-reset all queues via `i3cd034` (IBI/RX/TX/response/cmd/core), poll until - `i3cd034 == 0` (`hardware.rs:721-743`). -4. `set_role` / `init_clock` (`hardware.rs:744-745`); DAT init with SIR/MR - reject (`hardware.rs:806-818`); interrupt-enable + device static/dynamic - address + controller enable (`hardware.rs:820-846`). - -### 1.3 Clock timing (`hardware.rs:990-1111`) -- `ns_to_cnt_u8 = |ns| ns.div_ceil(core_period)` clamped to `u8::MAX` - (`hardware.rs:995-998`). Computes I2C-FM hi/lo, I3C OD hi/lo, I3C PP hi - (clamped ≤ 41 ns/spec), SDA-TX-hold clamped to [1,7]. Clock validity bounds - in `config.rs:601-666` (`I3C_MIN_CORE_CLK_SDR=12.5M`, `_HDR=25M`, - `MAX_CORE_CLK=400M`; core ≥ 4× SCL). - -### 1.4 Transfer / completion model -- `start_xfer` writes all cmd TX-FIFO entries + sets response threshold - (`hardware.rs:1347-1382`); `end_xfer` drains the response queue (≤32), - parses TID/len/error, scatters RX (`hardware.rs:1384-1466`). Error codes - `constants.rs:194-205`. -- Completion wait uses `Completion` (`types.rs:323-378`): `complete()` does - `store(Release)` + `cortex_m::asm::sev()`; `wait_for_us<D: DelayNs>` spins - `delay.delay_us(1)` up to `timeout_us`. Transfer/CCC/ENTDAA wait - `1_000_000_000 us` with a `DummyDelay` (`hardware.rs:1635-1636,1693-1695, - 1816-1817,2024-2039`). -- Generic poll: `poll_with_timeout<F,C,D: DelayNs>` (`hardware.rs:569-589`), - called for queue-reset and FIFO waits (`hardware.rs:736,1223,1247,1268`). - -### 1.5 ENTDAA / device management -- `do_entdaa` builds `ADDR_ASSGN_CMD | ENTDAA | DEV_COUNT=1 | DEV_INDEX=pos | - ROC | TOC` and waits ≤1 s (`hardware.rs:1664-1710`). `attach_i3c_dev` updates - `AddrBook`/`Attached` then `hw.attach_i3c_dev` (`controller.rs:144-179`). - Even-parity MSB on dynamic addr in DAT (`hardware.rs:1189-1199,1484-1496`). - -### 1.6 CCC (`ccc.rs`) -- GETPID(0x8D) `ccc.rs:339-366`, GETBCR(0x8E) `:251-284`, GETSTATUS(0x90) - `:370-420`, SETNEWDA(0x88) `:287-329`, RSTDAA(0x06) `:434-450`, RSTACT - `:228-249`, ENEC/DISEC `:154-225`. Broadcast (id ≤ 0x7F) vs direct cmd build - `hardware.rs:1502-1662` (`CP|ROC|TOC`, READ_TRANSFER=rnw). - -### 1.7 IBI / hot-join / IRQ (`ibi.rs`, `hardware.rs:67-417,1857-1897`) -- Per-bus 16-deep SPSC `heapless::spsc::Queue` (`ibi.rs`), `critical_section` - guarded; work items `HotJoin | Sirq{addr,len,data[16]} | TargetDaAssignment` - (`ibi.rs:22-37`). Enqueue `ibi.rs:118-176`, consume `i3c_ibi_workq_consumer`. -- IRQ registry: `static BUS_HANDLERS: [Mutex<RefCell<Option<Handler>>>;4]` - (`hardware.rs:76`); `register_i3c_irq_handler`/`dispatch_i3c_irq` - (`hardware.rs:92-110`); per-bus entry points + optional `#[no_mangle]` ISRs - (`hardware.rs:369-417`). `enable_irq`/`disable_irq` via `cortex_m NVIC` - (`hardware.rs:863-877`). `init_hardware` registers a `&mut Self`-derived - context + `dmb()` barrier before `enable_irq` (`controller.rs:111-132`). -- IBI parse: count from `i3cd04c`, id from `i3cd018`; addr 0x02 = hot-join, - rnw addr = SIR (`hardware.rs:1857-1897`). Device IBI-enable clears SIR-reject, - sets MDB/PEC, sends ENEC (`hardware.rs:1281-1345`). - -### 1.8 Target (secondary) mode (`hardware.rs:748-834,956-971,1984-2049`) -- Secondary interrupt-enable set, static addr program (`hardware.rs:748-834`); - ISR handles dyn-addr-assign / resp-ready / ccc-update (`:956-971`); SIR raise - writes TX-FIFO + IBI cmd + waits (`:1984-2049`). - -### 1.9 HAL surface (`hal_impl.rs`) -- `proposed_traits::i3c_master::I3c` for `I3cController`: `assign_dynamic_address` - (ENTDAA → GETPID → verify pid → GETBCR → ibi_enable, `hal_impl.rs:33-99`), - plus no-op `handle_hot_join`/`set_bus_speed`/`request_mastership`. -- `proposed_traits` target traits (`I2CCoreTarget`, `I3CCoreTarget`, - `DynamicAddressable`, `IBICapable`) `hal_impl.rs:135-218`; `get_ibi_payload` - builds `[mdb, crc8_ccitt]` (`hal_impl.rs:186-239`). - ---- - -## 2. Deltas vs. the authority (Phase 3 ledger) - -Classification ∈ { conformance · intentional delta · out-of-scope }. Every -*intentional delta* carries a reachability trace (consumer cited) or a stated -acceptance. - -| ID | Authority behavior (`file:line`) | Port behavior | Classification | -|----|----------------------------------|---------------|----------------| -| D1 | hal_impl implements `proposed_traits` i3c master + target traits (`hal_impl.rs:10-12,33,143-218`) | `proposed_traits` is absent from openprot. Convert the master ops (`assign_dynamic_address`, `handle_hot_join`, `set_bus_speed`, `request_mastership`) to **inherent methods** on `I3cController`; convert target traits to an internal `TargetCallbacks`-style trait. Same logic, no external trait dep. | **intentional delta** — mirrors the I2C port, which dropped `proposed_traits` for an internal `TargetCallbacks` (`peripherals/i2c/target_adapter.rs`). Reachability: no openprot consumer references `proposed_traits::i3c_*` (grep: zero hits under `openprot/`). embedded-hal 1.0 has **no** i3c trait, so there is no standard seam to retarget to — inherent methods are the I2C-consistent choice. | -| D2 | `Completion::wait_for_us<D: DelayNs>` + `poll_with_timeout<…,D: DelayNs>` + local `DummyDelay` busy-spin (`types.rs:367`, `hardware.rs:569-589,697`) | Inject a **`Y: FnMut(u32)` yield closure** at the `Ast1060I3c` construction gate; `wait_for_us`/`poll_with_timeout` take `&mut dyn FnMut(u32)` (type-erased), invoked once per non-completing poll. Bare-metal callers pass `\|_\| core::hint::spin_loop()`. | **intentional delta** — *Cooperative-Yield Bounded-Poll Device* pattern; identical to the I2C port's `yield_ns` closure (`peripherals/i2c/controller.rs`). Observable behavior on a spin closure == authority's `DummyDelay`. | -| D3 | `Ast1060I3c` holds `&'static RegisterBlock` obtained by `unsafe{&*ptr()}` in a **safe** `new` (`hardware.rs:419-439`) | Hold raw `*const RegisterBlock`; **single `unsafe fn new`** documenting the pointer-validity + serialization contract; one private `regs()`/`i3cg()`/`scu()` deref; `!Sync` via `PhantomData<UnsafeCell<()>>`. No `unsafe`/PAC types above the façade. | **intentional delta** — *Confined-`unsafe` MMIO Façade*; identical to the I2C port (`peripherals/i2c/controller.rs` raw-pointer + `_not_sync`). Pure structural; no behavior change. | -| D4 | `Ast1060I3c<I3C, L: Logger>` + `i3c_debug!` writing to a `heapless::String<128>` Logger (`hardware.rs:419-449`) | Drop the `L: Logger` generic and the `i3c_debug!` string-formatting path; debug logging removed (or routed to `pw_log` where genuinely useful). Drops the `heapless::String` formatting surface. | **intentional delta** — mirrors the I2C port (no `Logger`; tests use `pw_log` directly). Logging is non-functional; no observable bus behavior. | -| D5 | Panicking slice indexing / unchecked arithmetic in FIFO/response scatter paths (e.g. `hardware.rs` `end_xfer` RX distribution) | Harden to `.get(..).ok_or(I3cError::…)?` where a malformed length could panic, matching the I2C hardening. | **intentional delta** (allowed by parity standard) — to be enumerated row-by-row during implementation as each site is touched; each gets a one-line note here. Success path unchanged. | -| D6 | Global IBI SPSC queues (`static mut IBIQ_BUFS`) + IRQ registry (`static BUS_HANDLERS`) + `#[no_mangle]` ISR exports (`ibi.rs`, `hardware.rs:76-417`) | **Kept as-is** (behavior-preserving). `isr-handlers`-style `#[no_mangle]` exports gated off by default for kernel integration (the AST10x0 target defines ISRs and calls `dispatch_i3c_irq`). | **intentional delta vs. *Borrow-Arbitrated Engine Exclusivity*** — the engine state is process-global, not threaded through a `&mut` device, so that pattern's no-global-op-state box is **knowingly not met**. Justification: the ISR architecture requires a static handler/queue reachable from the interrupt vector; this is the authority's design and the parity target. Recorded as ADR-3. | -| D7 | `heapless = 0.8` (`aspeed-rust/Cargo.toml:40`), `spsc::Queue::split()` API | openprot ships `heapless = 0.9`. Port to the 0.9 `spsc` API (verify `Queue`/`split`/`Producer`/`Consumer` signatures during impl). | **intentional delta** (dep version) — API-compat shim only; no behavior change. Flagged for verification (Phase 7 if it misbehaves). | -| D8 | `critical-section = 1.2` + `cortex-m` feature `critical-section-single-core` (`aspeed-rust/Cargo.toml:46-47`) | openprot `@rust_crates` lists `cortex-m 0.7.7` **without** that feature and **no** `critical-section`. Add `critical-section` to `third_party/crates_io/Cargo.toml` and enable `cortex-m/critical-section-single-core` (or provide the CS impl the target already uses). | **intentional delta** (build wiring) — must be resolved before compile; see Plan item 1. | -| D9 | Authority style triggers openprot's `-D warnings` clippy (collapsible-if, unnecessary-cast, RefCell `borrow_mut` panic path) | Source-level, behavior-identical adjustments so the **new i3c code is clippy-clean** (the surrounding i2c/smc/uart already carry pre-existing clippy errors, left untouched): collapsed `if`/`if let` into let-chains; dropped a `u32 as u32`; and in `ibi.rs` replaced `RefCell::borrow_mut` with `try_borrow_mut` (panic-free — a conflicting borrow is impossible inside the `critical_section`) and array `[bus]` with `get_mut(bus)`, so the IBI-consumer path passes `no_panics_test`. | **intentional delta** (lint/panic hygiene) — observably identical; the `try_borrow_mut`/`get_mut` changes also discharge the relevant part of D5 for the IBI plane. | - -### 2.x Independent authority split (Phase 4) -1. **Parity authority** — aspeed-rust `src/i3c/` @ `ce3b567` (the behavior to - match). The done-criteria parity tests gate on this. -2. **Correctness authority** — MIPI I3C Basic v1.1.1 for CCC codes / address - reservations / parity (used to sanity-check, NOT to override the authority; - where aspeed-rust diverges from the spec, that divergence is a D-row, not a - silent "fix"). -3. **Interface authority** — the openprot consumer seam. **Verify-the-mandate:** - embedded-hal 1.0 defines **no** i3c master/target trait (confirmed: no i3c in - `embedded-hal`), and openprot has no i3c HAL trait of its own (grep: zero i3c - trait defs under `openprot/hal`, `openprot/drivers`). Therefore the interface - obligation is *only* "compile as a `pub mod i3c` in `ast10x0_peripherals` and - expose inherent methods + an internal target-callback trait" — there is no - external trait contract to satisfy. Do **not** invent one. - -### 2.x OPEN ISSUE — RESOLVED -- **OPEN-1 (pinctrl) — RESOLVED** via `../ast1060-pac/ast1060.svd`. The I3C pad - function-enable bits live in two SCU registers (set bit = enable function, - same `clear:false` semantics as the I2C groups): - - **SCU418** (Low-Voltage pads): I3C1 SCL=bit16/SDA=bit17, I3C2 SCL=18/SDA=19, - I3C3 SCL=20/SDA=21, I3C4 SCL=22/SDA=23 (SVD `EnblI3CSCLn/SDAnLVFnPin`). - - **SCU4B8** (High-Voltage pads): I3C1 SCL=bit8/SDA=bit9, I3C2 SCL=10/SDA=11, - I3C3 SCL=12/SDA=13, I3C4 SCL=14/SDA=15 (SVD `EnblI3CSCLn/SDAnHVFnPin`). - - Bus mapping: aspeed-rust `BUS_NUM` 0/1/2/3 (`I3c/I3c1/I3c2/I3c3`) → hardware - I3C1/I3C2/I3C3/I3C4. `scu/pinctrl.rs` already generates `PIN_SCU418_16..23` - and `PIN_SCU4B8_8..15` and `apply_pinctrl_group` already matches `0x418` / - `0x4B8` — so the fix is **zero PAC changes**: add LV groups (default) - ``` - pub const PINCTRL_I3C1: &[PinctrlPin] = &[PIN_SCU418_16, PIN_SCU418_17]; - pub const PINCTRL_I3C2: &[PinctrlPin] = &[PIN_SCU418_18, PIN_SCU418_19]; - pub const PINCTRL_I3C3: &[PinctrlPin] = &[PIN_SCU418_20, PIN_SCU418_21]; - pub const PINCTRL_I3C4: &[PinctrlPin] = &[PIN_SCU418_22, PIN_SCU418_23]; - ``` - plus optional `PINCTRL_I3Cn_HV` (SCU4B8) variants. LV vs HV is a board - decision; default to LV (the common I3C low-voltage rail). New Plan item 0 - below. QEMU `ast1030-evb` does not model pads, so the init smoke test still - passes without pad mux; the group matters only for on-hardware bring-up. - ---- - -## 3. Implementation plan (numbered; each ends with Acceptance) - -> Layout mirrors `peripherals/i2c/`: a flat `i3c/` module set inside the single -> `ast10x0_peripherals` bazel `rust_library`. Files ported 1:1 by name where -> possible: `mod.rs, controller.rs, config.rs, types.rs, error.rs, constants.rs, -> ccc.rs, ibi.rs, hardware.rs`, plus `hal_impl`/target callbacks folded per D1. - -0. **Pinctrl groups (OPEN-1, resolved).** Add `PINCTRL_I3C1..4` (LV / SCU418) - const groups to `scu/pinctrl.rs`, composing existing `PIN_SCU418_16..23`; - optional `_HV` variants over `PIN_SCU4B8_8..15`. No PAC change. - *Acceptance*: `peripherals` crate builds with the new consts; an `i3c_init` - test can pass `&[pinctrl::PINCTRL_I3C1]` to `Ast10x0Board`. -1. **Dependency wiring (D7, D8).** Add `critical-section` to - `third_party/crates_io/Cargo.toml`; enable `cortex-m/critical-section-single-core`; - confirm `heapless 0.9` + `cortex-m` resolve for the `thumbv7em` target. Add - `i3c/*.rs` to `peripherals/BUILD.bazel srcs` and the new deps to its `deps`. - *Acceptance*: `bazel build //target/ast10x0/peripherals:peripherals` resolves - all i3c crates (even before i3c code is added — deps compile). -2. **Port leaf modules verbatim-of-behavior**: `error.rs`, `constants.rs`, - `types.rs` (incl. `Completion`, but `wait_for_us` re-signatured to - `&mut dyn FnMut(u32)` per D2), `config.rs`. No `proposed_traits`, no `Logger`. - *Acceptance*: these four compile standalone in the crate; `Completion` unit - test (signaled/timeout) passes with a spin closure. -3. **Confined-`unsafe` façade (D3)** in `hardware.rs`: `Ast1060I3c<I3C: Instance, - Y: FnMut(u32)>` holding `*const` for i3c/i3cg/scu; one `unsafe fn new(.., yield_fn)` - with the 2-obligation `# Safety` doc; private `regs()/i3cg()/scu()`; `!Sync` - marker. Drop `L: Logger`/`i3c_debug!` (D4). - *Acceptance*: no `unsafe` or PAC type appears outside the façade methods - (grep check); `cargo`/`bazel` clippy clean on the struct + ctor. -4. **Port `HardwareCore/Clock/Fifo/Transfer/Recovery/Target` impls** (the bulk of - `hardware.rs`) onto the façade, threading the type-erased `&mut dyn FnMut(u32)` - into every `wait_for_us`/`poll_with_timeout` call site (D2). Harden panicking - index/arith sites to typed errors as touched, logging each in §2 D5. - *Acceptance*: `start_xfer`/`end_xfer`/`do_ccc`/`do_entdaa` compile; a host or - QEMU unit asserting a queue-reset poll completes (or times out typed) passes. -5. **Port `controller.rs`** (`I3cController<H>`, attach/detach, recover_bus, - init_hardware with `dmb()` + IRQ registration). Keep generic over `H`. - *Acceptance*: `I3cController::new` + `init_hardware` build; `attach_i3c_dev` - address-book bookkeeping unit test passes. -6. **Port `ibi.rs`** (heapless 0.9 SPSC, `critical_section` guards) and the IRQ - registry/`dispatch_i3c_irq` in `hardware.rs` (D6). `#[no_mangle]` ISR exports - behind a default-off cfg; document the kernel-calls-`dispatch_i3c_irq` path. - *Acceptance*: enqueue/consume round-trip unit test (HotJoin / Sirq / - TargetDaAssignment) passes under a single-core CS impl. -7. **CCC + master ops (D1)**: port `ccc.rs`; reimplement - `assign_dynamic_address` & friends as inherent `I3cController` methods. - *Acceptance*: CCC command-word composition unit tests (broadcast vs direct, - ROC/TOC/CP bits) match authority byte-for-byte. -8. **Target mode + IBI payload (D1)**: internal `TargetCallbacks`-style trait; - port `get_ibi_payload` (`crc8_ccitt`, `[mdb, crc]`) and the secondary ISR - paths. *Acceptance*: `crc8_ccitt` KAT + payload `[mdb,crc]` shape test pass. -9. **Wire into crate**: `pub mod i3c;` in `peripherals/lib.rs`; re-export the - public surface in `i3c/mod.rs` (mirroring the authority's `mod.rs:59-97`, - minus `proposed_traits`). - *Acceptance*: `bazel build //target/ast10x0/peripherals:peripherals` is green. -10. **Parity / smoke tests** under `target/ast10x0/tests/peripherals/i3c/` - (mirror `i2c/i2c_init` + `i2c/i2c_irq`): an `i3c_init` register-verify smoke - test (clock-timing registers vs computed expected, like the I2C test's - `verify_init_registers`) and an `i3c_irq` IBI/transfer test where feasible - under QEMU `ast1030-evb`. - *Acceptance*: see Done criteria. - -> Plan honesty: items 7–8 collapse `hal_impl.rs` into `controller.rs` + -> target-callback module (D1); the standalone `hal_impl.rs` file is **struck** — -> there is no external trait to host. - ---- - -## 4. Done criteria (testable, production-dominant workload) - -- `bazel build //target/ast10x0/peripherals:peripherals` green with i3c included. -- `bazel test --config=virt_ast10x0 //target/ast10x0/tests/peripherals/i3c/...` - passes under QEMU (`TEST_RESULT:PASS`), covering at minimum: - - **i3c_init**: full init sequence runs; the computed I3C/I2C clock-timing - register fields (`init_clock`, §1.3) read back equal to values derived from - the authority's formulas for a known `core_clk_hz` — the register-verify - gate, analogous to the I2C `verify_init_registers`. - - **ENTDAA / CCC word composition**: the command words built for ENTDAA and a - representative direct + broadcast CCC equal the authority's bit layout - (host unit test; production-dominant control path). - - **IBI work queue**: enqueue→consume round-trip for HotJoin and SIR. -- No `unsafe` and no `ast1060_pac` type outside the `Ast1060I3c` façade methods - (grep gate). -- Every §2 delta row is "discharged": authority lines read AND consumer/accept - trace recorded; D5 rows each enumerated. -- `./pw presubmit` (clippy + license/SPDX headers + format) clean on the new - files. - ---- - -## 5. Architecture decisions (Phase 8 ADRs — grounded) - -**ADR-1 — Keep the `HardwareInterface` 6-trait split (do not collapse).** -The authority separates `HardwareCore/Clock/Fifo/Transfer/Recovery/Target` -(`hardware.rs:6-19`) and makes `I3cController` generic over the supertrait. The -I2C port collapsed its layers because I2C had a single concrete type; I3C's -generic-over-`H` design is load-bearing for testability (mock `H`) and is the -parity target. Decision (user-confirmed): preserve it. The design-pattern -façade/yield/`!Sync` work is applied to the **concrete `Ast1060I3c`**, which is -exactly the layer those patterns target. - -**ADR-2 — I3C retains in-driver clock/reset; board does pinctrl only.** -Unlike I2C (where `Ast10x0Board::init()` owns SCU clock/reset and -`init_i2c_global` only sets I2CG), the authority interleaves -`global_reset_deassert`/`core_reset_assert`/`clock_on`/`core_reset_deassert` -with `i3cg` register writes inside `init()` (`hardware.rs:680-702`). Splitting -that out risks reordering a sequenced reset. Decision: keep the clock/reset -sequence inside the i3c driver (reached through the confined `scu()`/`i3cg()` -façade derefs); the board layer contributes only the I3C pinctrl group -(OPEN-1). This is a *deliberate* divergence from the I2C board-split, justified -by sequencing coupling — recorded so it is not mistaken for an oversight. - -**ADR-3 — Global IBI/IRQ state is an accepted delta vs Borrow-Arbitrated -Exclusivity.** `static BUS_HANDLERS` + `static mut IBIQ_BUFS` (`hardware.rs:76`, -`ibi.rs`) are process-global, reached from the interrupt vector via -`dispatch_i3c_irq`. The *Borrow-Arbitrated Engine Exclusivity* checklist -forbids global op-state aliased outside a `&mut` device — this port **knowingly -fails that box**, because an ISR cannot borrow a stack-owned device. Mutual -exclusion of the queues rests on `critical_section` + SPSC discipline (the -authority's design), not on `&mut` arbitration. The *Confined-`unsafe` Façade* -and *Cooperative-Yield* patterns (ADR applies to `Ast1060I3c`) ARE conformed -to; only the exclusivity pattern is consciously out-of-scope for the global -IBI/IRQ plane. Stated per the pattern's own "state the language dependency / -gate-delegated" discipline. - -**ADR-4 — Pattern conformance summary.** -- *Confined-`unsafe` MMIO Façade*: **conformed** (D3) — one `unsafe fn new`, one - private deref per block, `!Sync`, no PAC leakage. -- *Cooperative-Yield Bounded-Poll Device*: **conformed** (D2) — `Y: FnMut(u32)` - injected at gate, type-erased `&mut dyn FnMut(u32)` at the poll loops, bounded - iteration → typed timeout, advisory ns arg. -- *Borrow-Arbitrated Engine Exclusivity*: **partially out-of-scope** (ADR-3) for - the IBI/IRQ globals; the per-call transfer state is still threaded through - `&mut I3cController`. - ---- - -## 6. Outcome (implementation pass — 2026-06-02) - -**Status: ported and building green.** All Plan items 0–10 landed; the driver is -9 files (`ccc, config, constants, controller, error, hardware, ibi, mod, types`) -under `peripherals/i3c/`, wired into `ast10x0_peripherals` via `lib.rs` + -`BUILD.bazel`. `hal_impl.rs` was struck (D1) — its logic lives as inherent -methods on `I3cController` in `controller.rs`. - -Verified gates: -- `bazel build --platforms=//target/ast10x0 //target/ast10x0/peripherals:peripherals` - → **green** (full driver compiles for thumbv7em). -- `bazel build --platforms=//target/ast10x0 .../i3c/i3c_init:target` → **green** - (the init smoke-test kernel image builds). -- `bazel test .../i3c/i3c_init:no_panics_test` → **PASSED** (the driver + test - binary are panic-free; the bus-index `panic!` arms fold out under the const - `BUS_NUM`). -- `bazel test --config=virt_ast10x0 //target/ast10x0/tests/peripherals/i3c/...` - → builds the QEMU image (588 actions, green); the `hardware`-tagged - `i3c_init_test` is QEMU-incompatible by design (same as the I2C tests) and - runs on real silicon only. - -Delta resolutions vs §2: -- **D1, D2, D3, D4, D6, D7, OPEN-1** — all implemented as specified above. -- **D8** — `critical-section` added to `third_party/crates_io/Cargo.toml` and - `cortex-m/critical-section-single-core` enabled; `@rust_crates//:heapless` - resolved to 0.9.2; repinned cleanly (no Cargo.lock churn needed — all three - crates were already present transitively). -- **D7 (edition 2024)** — beyond the `Producer/Consumer` generic change, the - `static mut IBIQ_BUFS` split was rewritten through `addr_of_mut!` to satisfy - the edition-2024 `static_mut_refs` rule. -- **D5 (panic hardening) — DEFERRED.** No FIFO/response-scatter index was - hardened in this pass; the authority's indexing is retained verbatim (the - parity standard permits keeping reference behavior). `no_panics_test` passing - shows no reachable panic in the init path; revisit per-site if a malformed - hardware length is shown to reach a slice index. (The deferral is the only - open §2 item; everything else is discharged.) - -Façade-cleanliness note: the borrow-split required by the free-function -`poll_with_timeout` / `rd_fifo` / `drain_fifo` led the three deref helpers -(`i3c()/i3cg()/scu()`) to return `&'static` references (sound under the `new` -contract: pointers valid for the program lifetime), so a register reference and -`&mut self.yield_fn` can be held in disjoint statements. No `unsafe` or PAC type -appears above those three helpers + `new`. - -Second pass (continued — tests + lint): -- **`i3c_irq` test added** (`tests/peripherals/i3c/i3c_irq/`): dual-image - controller + secondary-target, mirroring `i2c_irq`. The controller drains the - IBI work queue; the target raises a SIR. `no_panics_test` (controller) + - `slave_no_panics_test` both **PASSED**; the two-device exchange runs under the - `hardware`-tagged `irq_test` on real silicon only. -- **clippy** (`rust_clippy_aspect`, `-D warnings`): the **7 i3c findings are all - fixed** (D9) — re-running the aspect leaves only the 8 *pre-existing* - i2c/smc/uart findings, which are out of scope and untouched. -- **`no_panics` (all three i3c images)**: PASSED. - -Third pass (parity with the I2C test/CI bar): -- **`./pw format`**: all 26 changed files (14 `.rs` incl. every new i3c file) - reformatted to rustfmt canonical → "No formatting changes needed"; the crate - still builds after. -- **`./pw presubmit`**: **all three recipes OK** — `build clippy //...` - (whole-repo clippy, i3c included), `check format`, and `check - presubmit_checks` (license/SPDX/include-guard/json). The only fix needed was - adding the license header to `PINNED_COMMIT.txt`. (Note: the repo's CI clippy - config — exercised by `build clippy //...` — passes cleanly; the 8 i2c/smc/uart - findings seen earlier come only from a stricter `--platforms=ast10x0` - `-D warnings` aspect invocation and are pre-existing, not introduced here.) - -This brings i3c to the same structure + test layout + CI bar as the I2C port: -`i3c_init` (mirrors `i2c_init`) and `i3c_irq` (mirrors `i2c_irq`), each with its -`no_panics_test` (kernel) + hardware-only execution test, all green. - -Fourth pass (EVB-faithful tests + reachable-path panic hardening): -- **`i3c_irq` rewritten to mirror the reference EVB tests** `tests-hw/src/i3c_test.rs::test_i3c_master`/`test_i3c_target`: I3C **bus 2** (PAC `I3c2`) on the **HV** pads (`PINCTRL_HVI3C2`) — the bus/pad set the AST1060 Test Harness wires and the reference uses. Controller pre-attaches a device by PID, enables IBI, and on each target SIR does a private read + private write (10 exchanges); target raises Hot-Join, waits for its dynamic address, then sends 10 IBIs. Differences from the reference are panic-hygiene only (`unwrap`→`?`/`pw_log`, `DummyDelay` dropped). -- **pinctrl reworked to bus-number naming + HV LV-clear**: `PINCTRL_I3C0..3` (LV) and `PINCTRL_HVI3C0..3` (HV), matching the reference's `HVI3Cn`. The HV groups now also **clear** the conflicting LV function bits on the same pads (`CLR_PIN_SCU418_*`) — the earlier HV groups only set the HV bit, which would have left both functions muxed. -- **D5 reachable-path hardening (now required for I2C parity)**: `i2c_irq:no_panics_test` passes, so panic-free transfer paths are the bar. Stop-and-instrument (objdump of `controller.elf`, ARM has no backtrace) localized the residual panics to `init_clock` (a `.expect()` on `core_clk_hz` and `div_ceil` by a not-provably-non-zero `core_period`/`fscl_hz` → `panic_const_div_by_zero`), surfaced once the test stopped const-folding the clock config. Hardened: `expect`→`unwrap_or(I3C_MIN_CORE_CLK_SDR)`, divisors bound to local `.max(1)` values; plus `end_xfer`/`priv_xfer_build_cmds`/`priv_xfer`/`ibi_enable`/`acknowledge_ibi`/`detach_i3c_dev_by_idx` slice/index sites moved to `get`/`get_mut`/`zip`/`?`. Success-path behavior unchanged. **All three i3c `no_panics_test`s now pass.** - -Building/running on the EVB (AST1060 Test Harness, two daughter cards A/B on the -I3C2 HV link): -``` -# Build the two images (controller = device A, target = device B): -bazel build --config=k_ast1060_evb \ - //target/ast10x0/tests/peripherals/i3c/i3c_irq:controller \ - //target/ast10x0/tests/peripherals/i3c/i3c_irq:slave -# Run the two-board IBI test via the Raspberry-Pi harness: -AST1060_EVB_PI_HOST=<pi-host> bazel test --config=k_ast1060_evb \ - //target/ast10x0/tests/peripherals/i3c/i3c_irq:irq_test -# Single-board init check: -AST1060_EVB_PI_HOST=<pi-host> bazel test --config=k_ast1060_evb \ - //target/ast10x0/tests/peripherals/i3c/i3c_init:i3c_init_test -``` - -Firmware images after a build (the `system_image` rule emits both `.bin` for -flashing and `.elf` for `pw_tokenizer` log decode): -`bazel-bin/.../i3c_irq/{controller,slave}.{bin,elf}`. A renamed copy is staged at -`out/i3c_evb_fw/{i3c_master,i3c_target}.{bin,elf}` for convenience. - -**Boot order (matches the reference `test_i3c_master`/`test_i3c_target`): power -the MASTER (`controller`/device A) first** so it is already draining the IBI -work queue, **then the TARGET** (`slave`/device B). The target raises a Hot-Join -which the master answers with `assign_dynamic_address`; the target then sends -its IBIs. (The reference's pre-Hot-Join `DummyDelay` is a no-op, so ordering is -operator-controlled — master up first.) Manual two-board flash without the bazel -test runner (UART boot via `harness/uart_test_exec.py`, device B on GPIO -`--srst-pin 25 --fwspick-pin 24`): -``` -./uart_test_exec.py /dev/ttyUSB_A out/i3c_evb_fw/i3c_master.bin --elf out/i3c_evb_fw/i3c_master.elf -./uart_test_exec.py --srst-pin 25 --fwspick-pin 24 /dev/ttyUSB_B \ - out/i3c_evb_fw/i3c_target.bin --elf out/i3c_evb_fw/i3c_target.elf -``` - -Still not done (honest scope note): -- **CCC word-composition host unit tests** named in §4 remain pending — but the - I2C port has no analogous standalone unit tests either, so this is *beyond* - I2C parity. The `no_panics` + build + on-HW `irq_test`/`i3c_init_test` (the - full master/target IBI exchange) cover the paths.
diff --git a/target/ast10x0/peripherals/i3c/plans/i3c-reference/PINNED_COMMIT.txt b/target/ast10x0/peripherals/i3c/plans/i3c-reference/PINNED_COMMIT.txt deleted file mode 100644 index 5201cad..0000000 --- a/target/ast10x0/peripherals/i3c/plans/i3c-reference/PINNED_COMMIT.txt +++ /dev/null
@@ -1,41 +0,0 @@ -# Licensed under the Apache-2.0 license -# SPDX-License-Identifier: Apache-2.0 - -Authority (parity-normative) source for the AST10x0 I3C port -============================================================ - -Repository : aspeed-rust (OpenPRoT/aspeed-ddk working tree) -Upstream : github.com/OpenPRoT (aspeed-rust), branch `main` -Revision : ce3b5677b95bc98a61ebf8783d00d93a910c4495 - "Merge pull request #74 from wmaroneAMD/dma-mode-linlin-fix" -Frozen : 2026-06-02 -Path : src/i3c/ (10 files, 4835 LoC) - ccc.rs config.rs constants.rs controller.rs error.rs - hal_impl.rs hardware.rs ibi.rs mod.rs types.rs - -Why authoritative ------------------ -The aspeed-rust I3C driver is the pinned behavioral-parity authority for this -openprot AST10x0 port. It is the same driver family already ported for I2C -(target/ast10x0/peripherals/i2c/, itself a port of aspeed-rust/src/i2c_core/), -so it is both the normative and the convenient reference here; there is no -separate deployed implementation to prefer over it. - -Informative-only references (authority wins on any divergence) --------------------------------------------------------------- -- Linux/Zephyr dw-i3c / aspeed-i3c controller drivers (DesignWare-style - command/response-queue model). Useful for register semantics ONLY; where - they differ from aspeed-rust, aspeed-rust wins and the informative ref is - treated as not-our-target. -- proposed_traits (github.com/rusty1968/proposed_traits.git @85641310) — the - trait surface aspeed-rust's hal_impl.rs targets. NOT available in openprot - (same as for the I2C port); treated as informative for the shape of the - master/target operations only. See goal.md Delta D1. - -Verbatim vendoring ------------------- -The 10 authority files are NOT re-copied into this directory to avoid a stale -second copy drifting from the live tree; they are pinned by the revision above -and read in place at ../../../../../../../aspeed-rust/src/i3c/ . If this working -tree is ever detached from aspeed-rust, copy the 10 files here verbatim and -delete this paragraph.
diff --git a/target/ast10x0/tests/peripherals/i3c/i3c_init/target.rs b/target/ast10x0/tests/peripherals/i3c/i3c_init/target.rs index 1fc95a5..4b2fe7a 100644 --- a/target/ast10x0/tests/peripherals/i3c/i3c_init/target.rs +++ b/target/ast10x0/tests/peripherals/i3c/i3c_init/target.rs
@@ -20,7 +20,7 @@ use codegen as _; use console_backend::console_backend_write_all; use entry as _; -use target_common::{declare_target, TargetInterface}; +use target_common::{TargetInterface, declare_target}; pub struct Target {} @@ -57,10 +57,10 @@ // SAFETY: the test owns I3C bus 0 for its lifetime and uses the matching // PAC register blocks; the busy-spin closure is the bare-metal wait policy. let hw = unsafe { Ast1060I3c::<ast1060_pac::I3c, _>::new(|_| core::hint::spin_loop()) }; - let mut ctrl = I3cController::new(hw, config); + let mut ctrl = core::pin::pin!(I3cController::new(hw, config)); pw_log::info!("Controller constructed"); - ctrl.init_hardware(); + ctrl.as_mut().init_hardware(); pw_log::info!("init_hardware complete"); // On real hardware the controller-enable bit must be set after a primary
diff --git a/target/ast10x0/tests/peripherals/i3c/i3c_irq/slave_target.rs b/target/ast10x0/tests/peripherals/i3c/i3c_irq/slave_target.rs index cc9395a..afe102a 100644 --- a/target/ast10x0/tests/peripherals/i3c/i3c_irq/slave_target.rs +++ b/target/ast10x0/tests/peripherals/i3c/i3c_irq/slave_target.rs
@@ -176,16 +176,16 @@ // `I3cConfig` (256-byte `AddrBook` inside) is freed before `ctrl` is used — // the kernel bootstrap thread stack is only 2 KiB and two live `I3cConfig`s // overflow it. See `build_target`. - let mut ctrl = build_target()?; - let bus = ctrl.hw.bus_num() as usize; + let mut ctrl = core::pin::pin!(build_target()?); + let bus = ctrl.as_ref().hw().bus_num() as usize; let mut ibi_cons = i3c_ibi_workq_consumer(bus).ok_or("IBI consumer unavailable")?; pw_log::info!("IBI work queue ready on bus {}", bus as u32); - ctrl.init_hardware(); + ctrl.as_mut().init_hardware(); let dyn_addr = 8u8; let dev_idx = 0usize; - let _ = ctrl.hw.attach_i3c_dev(dev_idx, dyn_addr); + let _ = ctrl.as_mut().hw_mut().attach_i3c_dev(dev_idx, dyn_addr); pw_log::info!( "target dev at slot {}, dyn addr {}", dev_idx as u32, @@ -195,7 +195,10 @@ pw_log::info!("waiting before hot-join..."); spin_wait(HOT_JOIN_STARTUP_DELAY_SPINS); pw_log::info!("raising hot-join; waiting for dynamic address assignment..."); - let hj_ok = ctrl.hw.target_ibi_raise_hj(&mut ctrl.config).is_ok(); + let hj_ok = ctrl + .as_mut() + .with_hw_and_config(|hw, config| hw.target_ibi_raise_hj(config)) + .is_ok(); pw_log::info!("[DBG] hot-join raise ok={}", hj_ok as u32); log_target_hj_state(0); @@ -207,7 +210,10 @@ spin_count = spin_count.wrapping_add(1); if spin_count & (HOT_JOIN_RETRY_SPINS - 1) == 0 { pw_log::info!("[DBG] retry hot-join"); - let hj_ok = ctrl.hw.target_ibi_raise_hj(&mut ctrl.config).is_ok(); + let hj_ok = ctrl + .as_mut() + .with_hw_and_config(|hw, config| hw.target_ibi_raise_hj(config)) + .is_ok(); pw_log::info!("[DBG] hot-join retry ok={}", hj_ok as u32); log_target_hj_state(1); } @@ -215,11 +221,16 @@ }; match work { IbiWork::TargetDaAssignment => { - let da = ctrl.config.target_config.as_ref().and_then(|t| t.addr); + let da = ctrl + .as_ref() + .config() + .target_config + .as_ref() + .and_then(|t| t.addr); if let Some(da) = da { pw_log::info!("[IBI] dyn addr 0x{:02x} assigned by master", da as u32); } - ctrl.target_on_dynamic_address_assigned(); + ctrl.as_mut().target_on_dynamic_address_assigned(); break; } IbiWork::HotJoin => pw_log::info!("[IBI] hotjoin"), @@ -239,7 +250,7 @@ for (i, b) in data.iter_mut().enumerate() { *b = u8::try_from(i).unwrap_or(0); } - if ctrl.target_get_ibi_payload(&mut data).is_err() { + if ctrl.as_mut().target_get_ibi_payload(&mut data).is_err() { return Err("target_get_ibi_payload failed"); } log_target_read_payload(ibi_count, &data);
diff --git a/target/ast10x0/tests/peripherals/i3c/i3c_irq/target.rs b/target/ast10x0/tests/peripherals/i3c/i3c_irq/target.rs index 149d032..c3918aa 100644 --- a/target/ast10x0/tests/peripherals/i3c/i3c_irq/target.rs +++ b/target/ast10x0/tests/peripherals/i3c/i3c_irq/target.rs
@@ -26,6 +26,8 @@ #![no_std] #![no_main] +use core::pin::Pin; + use ast10x0_board::{Ast10x0Board, Ast10x0BoardDescriptor}; use ast10x0_peripherals::i3c::{ Ast1060I3c, HardwareCore, HardwareTransfer, I3C_MSG_READ, I3C_MSG_STOP, I3C_MSG_WRITE, @@ -121,7 +123,7 @@ #[inline(never)] fn master_read_from_target( - ctrl: &mut I3c2Controller, + ctrl: Pin<&mut I3c2Controller>, ) -> Result<(u32, [u8; XFER_DATA_LEN]), &'static str> { let mut rx_buf = [0u8; 128]; let actual_len = { @@ -133,8 +135,7 @@ hdr_mode: 0, hdr_cmd_mode: 0, }]; - ctrl.hw - .priv_xfer(&mut ctrl.config, KNOWN_PID, &mut rd_msgs) + ctrl.with_hw_and_config(|hw, config| hw.priv_xfer(config, KNOWN_PID, &mut rd_msgs)) .map_err(|_| "private read failed")?; rd_msgs[0].actual_len }; @@ -145,7 +146,10 @@ } #[inline(never)] -fn master_write_to_target(ctrl: &mut I3c2Controller, exchange: u32) -> Result<(), &'static str> { +fn master_write_to_target( + ctrl: Pin<&mut I3c2Controller>, + exchange: u32, +) -> Result<(), &'static str> { let mut tx_buf: [u8; XFER_DATA_LEN] = [ 0xde, 0xad, 0xbe, 0xef, 0xca, 0xfe, 0xba, 0xbe, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, @@ -158,8 +162,7 @@ hdr_mode: 0, hdr_cmd_mode: 0, }]; - ctrl.hw - .priv_xfer(&mut ctrl.config, KNOWN_PID, &mut wr_msgs) + ctrl.with_hw_and_config(|hw, config| hw.priv_xfer(config, KNOWN_PID, &mut wr_msgs)) .map_err(|_| "private write failed")?; log_master_write_payload(exchange, &tx_buf); Ok(()) @@ -178,25 +181,27 @@ // `I3cConfig` is freed before the long-lived `ctrl` is used (see // `build_controller`): the kernel bootstrap thread stack is only 2 KiB and // two live `I3cConfig`s (each embeds a 256-byte `AddrBook`) overflow it. - let mut ctrl = build_controller()?; - let bus = ctrl.hw.bus_num() as usize; + let mut ctrl = core::pin::pin!(build_controller()?); + let bus = ctrl.as_ref().hw().bus_num() as usize; let mut ibi_cons = i3c_ibi_workq_consumer(bus).ok_or("IBI consumer unavailable")?; pw_log::info!("IBI work queue ready on bus {}", bus as u32); pw_log::info!("initializing I3C2 controller"); - ctrl.init_hardware(); + ctrl.as_mut().init_hardware(); pw_log::info!("I3C2 controller ready"); let dyn_addr = ctrl - .config + .as_mut() + .config_mut() .addrbook .alloc_from(8) .ok_or("no dynamic address available")?; - ctrl.attach_i3c_dev(KNOWN_PID, dyn_addr, 0) + ctrl.as_mut() + .attach_i3c_dev(KNOWN_PID, dyn_addr, 0) .map_err(|_| "attach_i3c_dev failed")?; - ctrl.hw.set_ibi_mdb(0); - ctrl.hw - .ibi_enable(&mut ctrl.config, dyn_addr) + ctrl.as_mut().hw_mut().set_ibi_mdb(0); + ctrl.as_mut() + .with_hw_and_config(|hw, config| hw.ibi_enable(config, dyn_addr)) .map_err(|_| "ibi_enable failed")?; pw_log::info!("pre-attached dev at slot 0, dyn addr {}", dyn_addr as u32); @@ -247,20 +252,20 @@ match work { IbiWork::HotJoin => { pw_log::info!("[IBI] hotjoin"); - let _ = ctrl.handle_hot_join(); - let _ = ctrl.assign_dynamic_address(dyn_addr); + let _ = ctrl.as_mut().handle_hot_join(); + let _ = ctrl.as_mut().assign_dynamic_address(dyn_addr); } IbiWork::Sirq { addr, len, .. } => { pw_log::info!("[IBI] SIRQ from 0x{:02x} len {}", addr as u32, len as u32); - if ctrl.acknowledge_ibi(addr).is_err() { + if ctrl.as_mut().acknowledge_ibi(addr).is_err() { pw_log::error!("acknowledge_ibi failed"); } let exchange = received; - let (read_len, read_data) = master_read_from_target(&mut ctrl)?; + let (read_len, read_data) = master_read_from_target(ctrl.as_mut())?; log_master_read_payload(exchange, read_len, &read_data); - master_write_to_target(&mut ctrl, exchange)?; + master_write_to_target(ctrl.as_mut(), exchange)?; received += 1; if received >= MAX_EXCHANGES {