Address I3C review follow-ups

target/ast10x0/peripherals/i3c/constants.rs

diff --git a/target/ast10x0/peripherals/i3c/constants.rs b/target/ast10x0/peripherals/i3c/constants.rs
index 45aaeb0..27e59d5 100644
--- a/target/ast10x0/peripherals/i3c/constants.rs
+++ b/target/ast10x0/peripherals/i3c/constants.rs
@@ -262,12 +262,10 @@
 pub const I3C_DEFAULT_STATIC_ADDR: u8 = 0x74;
 /// One-second operation timeout expressed in microseconds.
 pub const I3C_OP_TIMEOUT_US: u32 = USEC_PER_SEC;
+/// Bring-up reset poll delay between iterations in nanoseconds.
+pub const I3C_INIT_POLL_DELAY_NS: u32 = 100_000;
 /// Generic bounded-poll iteration ceiling used by controller bring-up waits.
 pub const I3C_POLL_MAX_ITERS: u32 = 1_000_000;
-/// Generic bounded-poll delay between iterations in nanoseconds.
-pub const I3C_POLL_DELAY_NS: u32 = 10_000;
-/// IBI enable / halt / reset-control poll ceiling.
-pub const I3C_CTRL_POLL_MAX_ITERS: u32 = 1_000_000;
 /// Queue reset / halt / IBI enable poll delay in nanoseconds.
 pub const I3C_CTRL_POLL_DELAY_NS: u32 = 10_000;
 /// Program the maximum IBI data threshold supported by the controller.

target/ast10x0/peripherals/i3c/hardware.rs

diff --git a/target/ast10x0/peripherals/i3c/hardware.rs b/target/ast10x0/peripherals/i3c/hardware.rs
index f150e44..4924c96 100644
--- a/target/ast10x0/peripherals/i3c/hardware.rs
+++ b/target/ast10x0/peripherals/i3c/hardware.rs
@@ -43,18 +43,18 @@
     I3C_BUS_I2C_FMP_TLOW_MIN_NS, I3C_BUS_I2C_FMP_TR_MAX_NS, I3C_BUS_I2C_STD_TF_MAX_NS,
     I3C_BUS_I2C_STD_THIGH_MIN_NS, I3C_BUS_I2C_STD_TLOW_MIN_NS, I3C_BUS_I2C_STD_TR_MAX_NS,
     I3C_BUS_THIGH_MAX_NS, I3C_CCC_DEVCTRL, I3C_CCC_ENTDAA, I3C_CCC_EVT_INTR, I3C_CCC_SETHID,
-    I3C_CTRL_POLL_DELAY_NS, I3C_CTRL_POLL_MAX_ITERS, I3C_DEFAULT_STATIC_ADDR,
-    I3C_GLOBAL_RESET_DEASSERT_MASK, I3C_IBI_DATA_THRESHOLD_MAX, I3C_INTR_STATUS_ALL_BITS,
-    I3C_MSG_READ, I3C_OP_TIMEOUT_US, I3C_POLL_DELAY_NS, I3C_POLL_MAX_ITERS,
-    I3CG_REG1_SCL_IN_SW_MODE_EN, I3CG_REG1_SCL_IN_SW_MODE_VAL, I3CG_REG1_SDA_IN_SW_MODE_EN,
-    I3CG_REG1_SDA_IN_SW_MODE_VAL, IBIQ_STATUS_IBI_DATA_LEN, IBIQ_STATUS_IBI_DATA_LEN_SHIFT,
-    IBIQ_STATUS_IBI_ID, IBIQ_STATUS_IBI_ID_SHIFT, INTR_CCC_UPDATED_STAT, INTR_DYN_ADDR_ASSGN_STAT,
-    INTR_IBI_THLD_STAT, INTR_RESP_READY_STAT, INTR_TRANSFER_ABORT_STAT, INTR_TRANSFER_ERR_STAT,
-    MAX_CMDS, NSEC_PER_SEC, RESET_CTRL_ALL, RESET_CTRL_QUEUES, RESET_CTRL_XFER_QUEUES,
-    RESPONSE_ERROR_IBA_NACK, RESPONSE_PORT_DATA_LEN_MASK, RESPONSE_PORT_DATA_LEN_SHIFT,
-    RESPONSE_PORT_ERR_STATUS_MASK, RESPONSE_PORT_ERR_STATUS_SHIFT, RESPONSE_PORT_TID_MASK,
-    RESPONSE_PORT_TID_SHIFT, SDA_TX_HOLD_MASK, SDA_TX_HOLD_MAX, SDA_TX_HOLD_MIN, SLV_DCR_MASK,
-    SLV_EVENT_CTRL_SIR_EN, bit, field_get, field_prep,
+    I3C_CTRL_POLL_DELAY_NS, I3C_DEFAULT_STATIC_ADDR, I3C_GLOBAL_RESET_DEASSERT_MASK,
+    I3C_IBI_DATA_THRESHOLD_MAX, I3C_INIT_POLL_DELAY_NS, I3C_INTR_STATUS_ALL_BITS, I3C_MSG_READ,
+    I3C_OP_TIMEOUT_US, I3C_POLL_MAX_ITERS, I3CG_REG1_SCL_IN_SW_MODE_EN,
+    I3CG_REG1_SCL_IN_SW_MODE_VAL, I3CG_REG1_SDA_IN_SW_MODE_EN, I3CG_REG1_SDA_IN_SW_MODE_VAL,
+    IBIQ_STATUS_IBI_DATA_LEN, IBIQ_STATUS_IBI_DATA_LEN_SHIFT, IBIQ_STATUS_IBI_ID,
+    IBIQ_STATUS_IBI_ID_SHIFT, INTR_CCC_UPDATED_STAT, INTR_DYN_ADDR_ASSGN_STAT, INTR_IBI_THLD_STAT,
+    INTR_RESP_READY_STAT, INTR_TRANSFER_ABORT_STAT, INTR_TRANSFER_ERR_STAT, MAX_CMDS, NSEC_PER_SEC,
+    RESET_CTRL_ALL, RESET_CTRL_QUEUES, RESET_CTRL_XFER_QUEUES, RESPONSE_ERROR_IBA_NACK,
+    RESPONSE_PORT_DATA_LEN_MASK, RESPONSE_PORT_DATA_LEN_SHIFT, RESPONSE_PORT_ERR_STATUS_MASK,
+    RESPONSE_PORT_ERR_STATUS_SHIFT, RESPONSE_PORT_TID_MASK, RESPONSE_PORT_TID_SHIFT,
+    SDA_TX_HOLD_MASK, SDA_TX_HOLD_MAX, SDA_TX_HOLD_MIN, SLV_DCR_MASK, SLV_EVENT_CTRL_SIR_EN, bit,
+    field_get, field_prep,
 };
 use super::error::I3cError as I3cDrvError;
 use super::error::I3cError;
@@ -783,7 +783,7 @@
             || regs.i3cd034().read().bits(),
             |val| val == 0,
             &mut self.yield_fn,
-            I3C_POLL_DELAY_NS,
+            I3C_INIT_POLL_DELAY_NS,
             I3C_POLL_MAX_ITERS,
         );
 
@@ -910,6 +910,9 @@
     }
 
     fn enable_irq(&mut self) {
+        // The integration layer owns the top-level vector and should point it
+        // at `dispatch_i3c_irq(bus)`. This helper only unmasks the bus IRQ
+        // line after registration + hardware init have completed.
         unsafe {
             match I3C::BUS_NUM {
                 0 => NVIC::unmask(ast1060_pac::Interrupt::i3c),
@@ -1288,7 +1291,7 @@
             |val| val != u32::from(expected),
             &mut self.yield_fn,
             I3C_CTRL_POLL_DELAY_NS,
-            I3C_CTRL_POLL_MAX_ITERS,
+            I3C_POLL_MAX_ITERS,
         );
 
         if rc.is_err() {
@@ -1313,7 +1316,7 @@
             |val| val == u32::from(expected),
             &mut self.yield_fn,
             I3C_CTRL_POLL_DELAY_NS,
-            I3C_CTRL_POLL_MAX_ITERS,
+            I3C_POLL_MAX_ITERS,
         );
 
         if rc.is_err() {
@@ -1335,7 +1338,7 @@
             |val| val == 0,
             &mut self.yield_fn,
             I3C_CTRL_POLL_DELAY_NS,
-            I3C_CTRL_POLL_MAX_ITERS,
+            I3C_POLL_MAX_ITERS,
         );
 
         if rc.is_err() {
@@ -1473,11 +1476,14 @@
             return;
         }
 
-        // SAFETY: `curr_xfer` is written by `start_xfer` from a unique `&mut I3cXfer`
-        // for the duration of the active transfer, and is reset back to null exactly
-        // once here with `swap(..., AcqRel)` before reconstructing the reference.
-        // The caller waits for transfer completion before dropping the stack-owned
-        // `xfer`, so the pointed-to object remains valid for this ISR handoff.
+        // SAFETY: `curr_xfer` is published by `start_xfer` from a unique
+        // `&mut I3cXfer`. The ISR path here and the timeout cleanup paths both
+        // compete via `swap(null, AcqRel)`; only the side that observes a
+        // non-null pointer may reconstruct and use it, while the loser sees
+        // null and performs no dereference. This target runs the handoff on a
+        // single core, and the owning thread waits for completion or timeout
+        // before dropping the stack-owned `xfer`, so the pointee outlives this
+        // exclusive ownership transfer.
         let xfer: &mut I3cXfer = unsafe { &mut *(p.cast::<I3cXfer>()) };
 
         let nresp = self.i3c().i3cd04c().read().respbufblr().bits() as usize;

target/ast10x0/peripherals/i3c/ibi.rs

diff --git a/target/ast10x0/peripherals/i3c/ibi.rs b/target/ast10x0/peripherals/i3c/ibi.rs
index 3e590fd..1c382d5 100644
--- a/target/ast10x0/peripherals/i3c/ibi.rs
+++ b/target/ast10x0/peripherals/i3c/ibi.rs
@@ -10,12 +10,13 @@
 //!
 //! **Porting delta (queue mechanism).** The reference uses `heapless::spsc`
 //! `Producer`/`Consumer` handles, split once and parked in a global
-//! `Mutex<RefCell<..>>`. On this target (heapless 0.9 + this toolchain) those
-//! handles do not survive being stored in a `static` and re-accessed across
-//! separate critical sections: a split that read back `prod=Some, cons=Some`
-//! in-place would, after the consumer was taken in a later critical section,
-//! read back `prod=None, cons=Some` — i.e. the niche-`Option`/`'static`-erased
-//! handles got corrupted. The `RefCell` borrow flag was also observed stuck.
+//! `Mutex<UnsafeCell<..>>`. On this target (heapless 0.9 + this toolchain) we
+//! observed unstable behavior when those handles were stored in a `static` and
+//! later re-accessed across separate critical sections: a split that read back
+//! `prod=Some, cons=Some` in-place would, after the consumer was taken in a
+//! later critical section, read back `prod=None, cons=Some`. The root cause was
+//! not fully isolated, so this port uses a simpler fixed-size ring buffer whose
+//! aliasing and lifetime rules are easier to audit.
 //!
 //! So the SPSC split is replaced by a plain fixed-size ring buffer of
 //! `Option<IbiWork>` (`IbiWork` is `Copy`, no niche pointers), guarded by the
@@ -25,7 +26,7 @@
 //! API (`i3c_ibi_workq_consumer().dequeue()` + the three enqueue functions) is
 //! unchanged.
 
-use core::cell::RefCell;
+use core::cell::UnsafeCell;
 use critical_section::Mutex;
 
 /// IBI queue depth
@@ -116,25 +117,33 @@
     }
 }
 
-static IBI_RINGS: [Mutex<RefCell<IbiRing>>; 4] = [
-    Mutex::new(RefCell::new(IbiRing::new())),
-    Mutex::new(RefCell::new(IbiRing::new())),
-    Mutex::new(RefCell::new(IbiRing::new())),
-    Mutex::new(RefCell::new(IbiRing::new())),
+static IBI_RINGS: [Mutex<UnsafeCell<IbiRing>>; 4] = [
+    Mutex::new(UnsafeCell::new(IbiRing::new())),
+    Mutex::new(UnsafeCell::new(IbiRing::new())),
+    Mutex::new(UnsafeCell::new(IbiRing::new())),
+    Mutex::new(UnsafeCell::new(IbiRing::new())),
 ];
 
 /// Run `f` against the ring for `bus`, serialized by the critical section.
 ///
 /// Returns `None` if `bus` is out of range.
+///
+/// # Safety Contract
+///
+/// `f` must not re-enter this module's queue API (`with_ring`, `dequeue`, or
+/// any of the enqueue helpers). Nested `critical_section::with(...)` calls are
+/// legal on this target, so re-entering while `ring: &mut IbiRing` is live
+/// would violate the exclusive-borrow assumption documented below.
 fn with_ring<R>(bus: usize, f: impl FnOnce(&mut IbiRing) -> R) -> Option<R> {
     let workq = IBI_RINGS.get(bus)?;
     Some(critical_section::with(|cs| {
-        // SAFETY: the critical section serializes all access to this ring, so no
-        // other reference is live. We go through `as_ptr()` rather than
-        // `borrow_mut()`/`try_borrow_mut()` because the `RefCell` runtime borrow
-        // flag is unreliable on this target (it stuck "borrowed" after a clean
-        // borrow/release); mutual exclusion comes from the critical section.
-        let ring: &mut IbiRing = unsafe { &mut *workq.borrow(cs).as_ptr() };
+        // SAFETY: each ring is reachable only through this helper and wrapped
+        // in a `critical_section::Mutex`. While the critical section is held
+        // there is no ISR/thread concurrency. The caller-provided closure is
+        // also required not to re-enter this module's queue API while the
+        // mutable borrow is live, so it is sound to project the
+        // `UnsafeCell<IbiRing>` to `&mut IbiRing`.
+        let ring: &mut IbiRing = unsafe { &mut *workq.borrow(cs).get() };
         f(ring)
     }))
 }