Rework I3C driver to runtime-bus model with register facade One driver type now manages all four bus instances: the Instance type-parameter and its per-bus impl macro are removed, and the bus is selected at runtime in I3cRegisters::new(bus), mirroring the SmcRegisters shape. All I3C MMIO unsafe (pointer derefs) is confined to the new registers.rs; hardware.rs accesses the blocks through safe delegates. IRQ ownership moves to the integration layer: enable_irq/disable_irq are dropped from the hardware traits and the driver only exposes dispatch_i3c_irq plus an i3c_bus_interrupt(bus) mapping for the NVIC line the kernel owns. The per-bus handler registry becomes single-shot (typed Busy on a second claim) and panic-free (UnsafeCell + critical section, same rationale as the IBI rings, whose closure-based access is also replaced by leaf push/pop helpers). The controller is rebuilt around an ISR-shared I3cCore pinned at a 'static address, so the trampoline pointer's validity is type-guaranteed rather than a convention, with a light two-state lifecycle (Uninitialized -> start() -> Ready) matching the SMC precedent. Tests construct the core via cortex_m::singleton!, which also moves the large config off the 2 KiB bootstrap stack, and unmask the NVIC line themselves after start().
diff --git a/target/ast10x0/peripherals/BUILD.bazel b/target/ast10x0/peripherals/BUILD.bazel index 6e49c22..09db01b 100644 --- a/target/ast10x0/peripherals/BUILD.bazel +++ b/target/ast10x0/peripherals/BUILD.bazel
@@ -42,6 +42,7 @@ "i3c/hardware.rs", "i3c/ibi.rs", "i3c/mod.rs", + "i3c/registers.rs", "i3c/types.rs", "lib.rs", "scu/cache.rs",
diff --git a/target/ast10x0/peripherals/i3c/controller.rs b/target/ast10x0/peripherals/i3c/controller.rs index 98ed7a1..71cd71b 100644 --- a/target/ast10x0/peripherals/i3c/controller.rs +++ b/target/ast10x0/peripherals/i3c/controller.rs
@@ -5,14 +5,30 @@ //! //! Main hardware abstraction for I3C bus controller. //! -//! # Construction Patterns +//! # Lifecycle //! -//! The controller uses an explicit two-stage bring-up: +//! Two states, matching the SMC peripheral's `Uninitialized -> Ready` +//! precedent: //! -//! | 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 | +//! | State | Entered by | Available operations | +//! |-------|-----------|----------------------| +//! | [`Uninitialized`] | [`I3cController::new`] | [`start()`](I3cController::start) | +//! | [`Ready`] | `start()` (IRQ trampoline claimed + hardware programmed) | bus operations | +//! +//! After `start()` the integration layer unmasks the NVIC line it owns (see +//! [`i3c_bus_interrupt`](super::hardware::i3c_bus_interrupt)); the driver +//! never touches the NVIC. +//! +//! # ISR sharing +//! +//! The IRQ trampoline needs a pointer that outlives every interrupt, so the +//! hardware + config live in an [`I3cCore`] that the caller places in a +//! `static` and pins: [`I3cController::new`] takes `Pin<&'static mut +//! I3cCore<H>>`. Pointer *validity* in the ISR is therefore a type guarantee +//! (`'static` + pinned), not a convention; what remains documented contract is +//! aliasing: on this single-core target the ISR runs atomically with respect +//! to the thread, and the thread only polls atomic completion flags while a +//! transfer is in flight. //! //! # Example //! @@ -22,16 +38,19 @@ //! scu.enable_i3c_clock(bus); //! scu.deassert_i3c_reset(bus); //! -//! // 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(); +//! let hw = unsafe { Ast1060I3c::new(bus, yield_fn) }.ok_or(...)?; +//! let core = cortex_m::singleton!(: I3cCore<_> = I3cCore::new(hw, config)) +//! .ok_or("storage taken")?; +//! let mut ctrl = I3cController::new(Pin::static_mut(core)) +//! .start()?; // register IRQ (single-shot) + program hardware //! -//! // === HOT PATH (hardware already configured) === -//! let ctrl = I3cController::from_initialized(hw, config); -//! ctrl.do_transfer(...); +//! // Integration layer owns the NVIC line; unmask it now. +//! unsafe { NVIC::unmask(i3c_bus_interrupt(bus).unwrap()) }; +//! +//! ctrl.priv_write(pid, &mut data)?; //! ``` -use core::marker::PhantomPinned; +use core::marker::{PhantomData, PhantomPinned}; use core::pin::Pin; use super::ccc; @@ -42,51 +61,26 @@ use super::types::{DevKind, I3cIbi, I3cIbiType, I3cMsg}; use embedded_hal::i2c::SevenBitAddress; -/// I3C controller wrapping hardware interface -pub struct I3cController<H: HardwareInterface> { - /// Hardware interface implementation +// ============================================================================= +// Pinned core (the ISR-shared part) +// ============================================================================= + +/// Hardware + configuration for one I3C bus — the part the IRQ trampoline +/// dereferences, so it must live at a stable `'static` address. +/// +/// Construct with [`I3cCore::new`] (no I/O), park it in a `static` (e.g. +/// `cortex_m::singleton!`), and hand `Pin<&'static mut I3cCore<H>>` to +/// [`I3cController::new`]. +pub struct I3cCore<H: HardwareInterface> { hw: H, - /// Bus configuration config: I3cConfig, _pin: PhantomPinned, } -impl<H: HardwareInterface> I3cController<H> { - // ========================================================================= - // Construction - // ========================================================================= - - /// Construct an I3C controller value without touching hardware. - /// - /// 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) - } - - /// Wrap pre-initialized hardware (lightweight, no I/O) - /// - /// Creates instance without touching hardware registers. - /// - /// # When to Use - /// - /// - Hardware was initialized at boot before kernel/RTOS start - /// - Creating temporary instances for single operations - /// - Avoiding redundant re-initialization overhead - /// - Hot path where performance matters - /// - /// # Preconditions - /// - /// Caller must ensure hardware is already configured: - /// - [`new()`](Self::new) was called previously, OR - /// - Hardware initialized by bootloader/firmware - /// - /// # Performance - /// - /// No register writes - significantly faster than `new()`. +impl<H: HardwareInterface> I3cCore<H> { + /// Bundle hardware and configuration. No I/O is performed. #[must_use] - pub fn from_initialized(hw: H, config: I3cConfig) -> Self { + pub fn new(hw: H, config: I3cConfig) -> Self { Self { hw, config, @@ -94,47 +88,120 @@ } } - /// Initialize/reinitialize hardware registers - /// - /// Registers the IRQ handler and configures the hardware. - /// - /// 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); + /// IRQ trampoline registered (per bus) by [`I3cController::start`]. + fn irq_trampoline(ctx: usize) { + // SAFETY: `ctx` comes from a `Pin<&'static mut I3cCore<H>>` in + // `start`, so the pointer is valid and address-stable for the + // program's lifetime (type-guaranteed, not a convention). Aliasing: + // this single-core target runs the ISR atomically with respect to the + // thread, and the thread side only polls atomic completion flags while + // a transfer is in flight, so no `&mut` is concurrently *used*. + let core: &mut Self = unsafe { &mut *(ctx as *mut Self) }; + core.hw.i3c_aspeed_isr(&mut core.config); + } +} - // IMPORTANT: init() must complete before enable_irq() to prevent - // IRQ firing on partially-initialized hardware - this.hw.init(&mut this.config); +// ============================================================================= +// Lifecycle states +// ============================================================================= - // Memory barrier to ensure init writes are visible before IRQ enable +/// Initial state: nothing registered, no I/O done. +pub struct Uninitialized; +/// IRQ trampoline claimed and hardware programmed; bus operations available. +/// The integration layer unmasks the NVIC line it owns after entering this +/// state. +pub struct Ready; + +// ============================================================================= +// Controller shell +// ============================================================================= + +/// I3C controller: a movable shell over the pinned [`I3cCore`]. +/// +/// `H: 'static` because the core is `static`-pinned for the IRQ trampoline. +pub struct I3cController<H: HardwareInterface + 'static, S = Uninitialized> { + core: Pin<&'static mut I3cCore<H>>, + _state: PhantomData<S>, +} + +impl<H: HardwareInterface + 'static, S> I3cController<H, S> { + /// Project the pinned core to `(&mut hw, &mut config)`. + #[inline] + fn parts(&mut self) -> (&mut H, &mut I3cConfig) { + // SAFETY: structural pin projection — neither field is moved out and + // the core's address is unchanged. + let core = unsafe { self.core.as_mut().get_unchecked_mut() }; + (&mut core.hw, &mut core.config) + } + + #[inline] + fn core_ref(&self) -> &I3cCore<H> { + self.core.as_ref().get_ref() + } + + /// Return this controller's bus number. + #[inline] + #[must_use] + pub fn bus_num(&self) -> u8 { + self.core_ref().hw.bus_num() + } +} + +impl<H: HardwareInterface + 'static> I3cController<H, Uninitialized> { + /// Wrap a pinned core. No I/O, no registration. + #[must_use] + pub fn new(core: Pin<&'static mut I3cCore<H>>) -> Self { + Self { + core, + _state: PhantomData, + } + } + + /// Bring the controller up: claim this bus's IRQ slot (single-shot per + /// bus) and program the hardware. + /// + /// The target/kernel owns the top-level interrupt vector; its ISR calls + /// [`dispatch_i3c_irq`](super::hardware::dispatch_i3c_irq), which forwards + /// to the trampoline registered here. On return the device may assert its + /// IRQ line; nothing is delivered until the integration layer unmasks the + /// NVIC line it owns (see + /// [`i3c_bus_interrupt`](super::hardware::i3c_bus_interrupt)). + /// + /// Returns [`I3cError::Busy`] if the bus's IRQ slot was already claimed by + /// another controller. + pub fn start(mut self) -> Result<I3cController<H, Ready>, I3cError> { + { + // SAFETY: structural pin projection — the core is not moved; we + // only take its (stable, 'static) address for the IRQ registry. + let core = unsafe { self.core.as_mut().get_unchecked_mut() }; + let bus = core.hw.bus_num() as usize; + let ctx = core::ptr::from_mut::<I3cCore<H>>(core) as usize; + if !super::hardware::register_i3c_irq_handler(bus, I3cCore::<H>::irq_trampoline, ctx) + { + return Err(I3cError::Busy); + } + } + + { + let (hw, config) = self.parts(); + hw.init(config); + } + // Memory barrier so init writes are visible before the integration + // layer unmasks the IRQ line. cortex_m::asm::dmb(); - this.hw.enable_irq(); + Ok(I3cController { + core: self.core, + _state: PhantomData, + }) } +} - /// IRQ trampoline function - fn irq_trampoline(ctx: usize) { - // SAFETY: `ctx` was created from `&mut Self` in `init_hardware()`. - // Aliasing safety relies on caller not holding `&mut self` when IRQs enabled. - let ctrl: &mut Self = unsafe { &mut *(ctx as *mut Self) }; - ctrl.hw.i3c_aspeed_isr(&mut ctrl.config); - } +// ============================================================================= +// Bus operations (Ready) +// ============================================================================= - #[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) - } - +impl<H: HardwareInterface + 'static> I3cController<H, Ready> { // ========================================================================= // Device Management // ========================================================================= @@ -145,13 +212,8 @@ /// * `pid` - Provisional ID of the device /// * `desired_da` - Desired dynamic address /// * `slot` - DAT slot to use - pub fn attach_i3c_dev( - self: Pin<&mut Self>, - pid: u64, - desired_da: u8, - slot: u8, - ) -> Result<(), I3cError> { - let this = self.project_mut(); + pub fn attach_i3c_dev(&mut self, pid: u64, desired_da: u8, slot: u8) -> Result<(), I3cError> { + let (hw, config) = self.parts(); if desired_da == 0 || desired_da >= I3C_BROADCAST_ADDR { return Err(I3cError::InvalidArgs); } @@ -173,47 +235,50 @@ pos: Some(slot), }; - let idx = this - .config + let idx = config .attached .attach(dev) .map_err(|_| I3cError::AddrInUse)?; - this.config + config .attached .map_pos(slot, u8::try_from(idx).map_err(|_| I3cError::InvalidArgs)?); - this.config.addrbook.mark_use(desired_da, true); + config.addrbook.mark_use(desired_da, true); - this.hw - .attach_i3c_dev(slot.into(), desired_da) + hw.attach_i3c_dev(slot.into(), desired_da) .map_err(|_| I3cError::AddrInUse) } /// Detach an I3C device by DAT position - 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); + pub fn detach_i3c_dev(&mut self, pos: usize) { + let (hw, config) = self.parts(); + config.attached.detach_by_pos(pos); + hw.detach_i3c_dev(pos); } /// Detach an I3C device by device index - pub fn detach_i3c_dev_by_idx(self: Pin<&mut Self>, dev_idx: usize) { - let this = self.project_mut(); + pub fn detach_i3c_dev_by_idx(&mut self, dev_idx: usize) { + let (hw, config) = self.parts(); // `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) = this.config.attached.devices.get(dev_idx) else { + let Some(dev) = config.attached.devices.get(dev_idx) else { return; }; if dev.dyn_addr != 0 { - this.config.addrbook.mark_use(dev.dyn_addr, false); + let dyn_addr = dev.dyn_addr; + config.addrbook.mark_use(dyn_addr, false); } - let dev_pos = dev.pos; + let dev_pos = config + .attached + .devices + .get(dev_idx) + .and_then(|dev| dev.pos); if let Some(pos) = dev_pos { - this.hw.detach_i3c_dev(pos.into()); + hw.detach_i3c_dev(pos.into()); } - this.config.attached.detach(dev_idx); + config.attached.detach(dev_idx); } // ========================================================================= @@ -246,12 +311,12 @@ /// // More aggressive recovery /// ctrl.recover_bus(18); /// ``` - 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(); + pub fn recover_bus(&mut self, scl_toggles: u32) { + let (hw, _) = self.parts(); + hw.enter_sw_mode(); + hw.i3c_toggle_scl_in(scl_toggles); + hw.gen_internal_stop(); + hw.exit_sw_mode(); } /// Perform full bus recovery with controller reset @@ -273,30 +338,28 @@ /// 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: Pin<&mut Self>, reset_mask: u32) { - self.as_mut().recover_bus(8); - self.project_mut().hw.reset_ctrl(reset_mask); + pub fn recover_bus_full(&mut self, reset_mask: u32) { + self.recover_bus(8); + let (hw, _) = self.parts(); + hw.reset_ctrl(reset_mask); } + // ========================================================================= // Accessors // ========================================================================= - /// Return this controller's bus number. - #[inline] - pub fn bus_num(self: Pin<&Self>) -> u8 { - self.project_ref().hw.bus_num() - } - /// Allocate a dynamic address from `start_addr`. #[inline] - pub fn alloc_dynamic_address_from(self: Pin<&mut Self>, start_addr: u8) -> Option<u8> { - self.project_mut().config.addrbook.alloc_from(start_addr) + pub fn alloc_dynamic_address_from(&mut self, start_addr: u8) -> Option<u8> { + let (_, config) = self.parts(); + config.addrbook.alloc_from(start_addr) } /// Return the currently assigned target dynamic address, if any. #[inline] - pub fn target_dynamic_address(self: Pin<&Self>) -> Option<u8> { - self.project_ref() + #[must_use] + pub fn target_dynamic_address(&self) -> Option<u8> { + self.core_ref() .config .target_config .as_ref() @@ -304,15 +367,15 @@ } /// Set the device's IBI mandatory data byte and enable IBI delivery for `addr`. - pub fn enable_ibi(self: Pin<&mut Self>, addr: u8, mdb: u8) -> Result<(), I3cError> { - let this = self.project_mut(); - this.hw.set_ibi_mdb(mdb); - this.hw.ibi_enable(&mut this.config, addr) + pub fn enable_ibi(&mut self, addr: u8, mdb: u8) -> Result<(), I3cError> { + let (hw, config) = self.parts(); + hw.set_ibi_mdb(mdb); + hw.ibi_enable(config, addr) } /// Issue a private read to `pid`, returning the number of received bytes. - pub fn priv_read(self: Pin<&mut Self>, pid: u64, out: &mut [u8]) -> Result<u32, I3cError> { - let this = self.project_mut(); + pub fn priv_read(&mut self, pid: u64, out: &mut [u8]) -> Result<u32, I3cError> { + let (hw, config) = self.parts(); let actual_len = u32::try_from(out.len()).map_err(|_| I3cError::InvalidArgs)?; let mut msgs = [I3cMsg { buf: Some(out), @@ -322,13 +385,13 @@ hdr_mode: 0, hdr_cmd_mode: 0, }]; - this.hw.priv_xfer(&mut this.config, pid, &mut msgs)?; + hw.priv_xfer(config, pid, &mut msgs)?; Ok(msgs[0].actual_len) } /// Issue a private write to `pid`. - pub fn priv_write(self: Pin<&mut Self>, pid: u64, data: &mut [u8]) -> Result<(), I3cError> { - let this = self.project_mut(); + pub fn priv_write(&mut self, pid: u64, data: &mut [u8]) -> Result<(), I3cError> { + let (hw, config) = self.parts(); let actual_len = u32::try_from(data.len()).map_err(|_| I3cError::InvalidArgs)?; let mut msgs = [I3cMsg { buf: Some(data), @@ -338,70 +401,51 @@ hdr_mode: 0, hdr_cmd_mode: 0, }]; - this.hw.priv_xfer(&mut this.config, pid, &mut msgs) + hw.priv_xfer(config, pid, &mut msgs) } /// Raise a hot-join request from the target side. - pub fn target_raise_hot_join(self: Pin<&mut Self>) -> Result<(), I3cError> { - let this = self.project_mut(); - this.hw.target_ibi_raise_hj(&mut this.config) + pub fn target_raise_hot_join(&mut self) -> Result<(), I3cError> { + let (hw, config) = self.parts(); + hw.target_ibi_raise_hj(config) } -} -// ============================================================================= -// Conversions -// ============================================================================= + // ========================================================================= + // Master / Target operations (Delta D1) + // ========================================================================= + // + // The reference exposed these through `proposed_traits::i3c_master::I3c` + // and the `proposed_traits` target traits (`aspeed-rust/src/i3c/hal_impl.rs`). + // That crate is unavailable in openprot and embedded-hal 1.0 defines no I3C + // trait, so — as the I2C port did for `proposed_traits::i2c_target` — the + // logic is preserved verbatim here as **inherent methods**. The only change + // is that `ErrorKind`-mapped errors become direct `I3cError` variants + // (`DynamicAddressConflict` -> `AddrInUse`, `InvalidCcc` -> `Invalid`). -impl<H: HardwareInterface> From<(H, I3cConfig)> for I3cController<H> { - /// Lightweight conversion (no hardware I/O) - /// - /// Equivalent to [`from_initialized`](I3cController::from_initialized). - fn from((hw, config): (H, I3cConfig)) -> Self { - Self::from_initialized(hw, config) - } -} - -// ============================================================================= -// Master / Target operations (Delta D1) -// ============================================================================= -// -// The reference exposed these through `proposed_traits::i3c_master::I3c` and the -// `proposed_traits` target traits (`aspeed-rust/src/i3c/hal_impl.rs`). That crate -// is unavailable in openprot and embedded-hal 1.0 defines no I3C trait, so — as -// the I2C port did for `proposed_traits::i2c_target` — the logic is preserved -// verbatim here as **inherent methods**. The only change is that -// `ErrorKind`-mapped errors become direct `I3cError` variants -// (`DynamicAddressConflict` -> `AddrInUse`, `InvalidCcc` -> `Invalid`). - -impl<H: HardwareInterface> I3cController<H> { /// 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( - self: Pin<&mut Self>, + &mut self, static_address: SevenBitAddress, ) -> Result<SevenBitAddress, I3cError> { - let this = self.project_mut(); - let slot = this - .config + let (hw, config) = self.parts(); + let slot = config .attached .pos_of_addr(static_address) .ok_or(I3cError::AddrInUse)?; - this.hw - .do_entdaa(&mut this.config, slot.into()) + hw.do_entdaa(config, slot.into()) .map_err(|_| I3cError::AddrInUse)?; - let pid = ccc::ccc_getpid(&mut this.hw, &mut this.config, static_address) - .map_err(|_| I3cError::Invalid)?; + let pid = + ccc::ccc_getpid(hw, config, static_address).map_err(|_| I3cError::Invalid)?; - let dev_idx = this - .config + let dev_idx = config .attached .find_dev_idx_by_addr(static_address) .ok_or(I3cError::Other)?; - let old_pid = this - .config + let old_pid = config .attached .devices .get(dev_idx) @@ -414,12 +458,11 @@ return Err(I3cError::Other); } - let bcr = ccc::ccc_getbcr(&mut this.hw, &mut this.config, static_address) - .map_err(|_| I3cError::Invalid)?; + let bcr = + ccc::ccc_getbcr(hw, config, static_address).map_err(|_| I3cError::Invalid)?; { - let dev = this - .config + let dev = config .attached .devices .get_mut(dev_idx) @@ -429,34 +472,30 @@ dev.bcr = bcr; } - let dyn_addr: SevenBitAddress = this - .config + let dyn_addr: SevenBitAddress = config .attached .devices .get(dev_idx) .ok_or(I3cError::Other)? .dyn_addr; - this.hw - .ibi_enable(&mut this.config, dyn_addr) + hw.ibi_enable(config, dyn_addr) .map_err(|_| I3cError::Other)?; Ok(dyn_addr) } /// Acknowledge an IBI from `address` (validates the device is known). - pub fn acknowledge_ibi(self: Pin<&mut Self>, address: SevenBitAddress) -> Result<(), I3cError> { - let this = self.project_mut(); - let dev_idx = this - .config + pub fn acknowledge_ibi(&mut self, address: SevenBitAddress) -> Result<(), I3cError> { + let (_, config) = self.parts(); + let dev_idx = config .attached .find_dev_idx_by_addr(address) .ok_or(I3cError::Other)?; // `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 = this - .config + let dev = config .attached .devices .get(dev_idx) @@ -471,41 +510,41 @@ /// 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(self: Pin<&mut Self>) -> Result<(), I3cError> { + pub fn handle_hot_join(&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(self: Pin<&mut Self>) -> Result<(), I3cError> { + pub fn set_bus_speed(&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(self: Pin<&mut Self>) -> Result<(), I3cError> { + pub fn request_mastership(&mut self) -> Result<(), I3cError> { Ok(()) } // --- Target (secondary) mode callbacks --- /// Initialize target mode with `own_addr` (sets the static/target address). - 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() { + pub fn target_init(&mut self, own_addr: u8) { + let (_, config) = self.parts(); + if let Some(t) = config.target_config.as_mut() { if t.addr.is_none() { t.addr = Some(own_addr); } } else { - this.config.target_config = + 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: Pin<&Self>, addr: u8) -> bool { - self.project_ref() + pub fn target_on_address_match(&self, addr: u8) -> bool { + self.core_ref() .config .target_config .as_ref() @@ -515,25 +554,23 @@ /// Record that the controller assigned this target a dynamic address; SIRs /// are then permitted by software. - pub fn target_on_dynamic_address_assigned(self: Pin<&mut Self>) { - self.project_mut().config.sir_allowed_by_sw = true; + pub fn target_on_dynamic_address_assigned(&mut self) { + let (_, config) = self.parts(); + 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: Pin<&Self>) -> bool { + pub fn target_wants_ibi(&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( - 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() { + pub fn target_get_ibi_payload(&mut self, buffer: &mut [u8]) -> Result<usize, I3cError> { + let (hw, config) = self.parts(); + let (da, mdb) = match config.target_config.as_ref() { Some(t) => ( match t.addr { Some(da) => da, @@ -553,9 +590,7 @@ ibi_type: I3cIbiType::TargetIntr, payload: Some(&payload), }; - let rc = this - .hw - .target_pending_read_notify(&mut this.config, buffer, &mut ibi); + let rc = hw.target_pending_read_notify(config, buffer, &mut ibi); match rc { Ok(()) => Ok(buffer.len() + payload.len()),
diff --git a/target/ast10x0/peripherals/i3c/error.rs b/target/ast10x0/peripherals/i3c/error.rs index 5371ea0..ae34a3d 100644 --- a/target/ast10x0/peripherals/i3c/error.rs +++ b/target/ast10x0/peripherals/i3c/error.rs
@@ -47,6 +47,8 @@ InvalidParam, /// CCC (Common Command Code) error CccError(CccErrorKind), + /// Resource (bus IRQ slot) already claimed by another controller + Busy, /// Other unspecified error Other, } @@ -69,6 +71,7 @@ Self::DevAlreadyAttached => write!(f, "device already attached"), Self::InvalidParam => write!(f, "invalid parameter"), Self::CccError(kind) => write!(f, "CCC error: {kind:?}"), + Self::Busy => write!(f, "resource busy"), Self::Other => write!(f, "other error"), } }
diff --git a/target/ast10x0/peripherals/i3c/hardware.rs b/target/ast10x0/peripherals/i3c/hardware.rs index 4924c96..7463987 100644 --- a/target/ast10x0/peripherals/i3c/hardware.rs +++ b/target/ast10x0/peripherals/i3c/hardware.rs
@@ -25,7 +25,7 @@ //! They should be performed by the platform/board layer before creating the //! I3C controller. -use core::cell::RefCell; +use core::cell::UnsafeCell; use critical_section::Mutex; use super::ccc::{CccPayload, ccc_events_set}; @@ -61,10 +61,8 @@ use super::ibi as ibi_workq; use super::types::{I3cCmd, I3cIbi, I3cMsg, I3cXfer, SpeedI3c, Tid}; -use core::cell::UnsafeCell; -use core::marker::PhantomData; +use super::registers::I3cRegisters; use core::sync::atomic::Ordering; -use cortex_m::peripheral::NVIC; // ============================================================================= // IRQ Handler Infrastructure @@ -76,27 +74,62 @@ ctx: usize, } -static BUS_HANDLERS: [Mutex<RefCell<Option<Handler>>>; 4] = [ - Mutex::new(RefCell::new(None)), - Mutex::new(RefCell::new(None)), - Mutex::new(RefCell::new(None)), - Mutex::new(RefCell::new(None)), +// `UnsafeCell` (not `RefCell`) for the same reason as `IBI_RINGS` in `ibi.rs`: +// mutual exclusion comes from the critical section, and the access helpers +// below are leaf functions (no caller code runs while the reference is live), +// so the `RefCell` runtime borrow flag would only add a reachable panic path +// that the `no_panics` analysis must reject. +static BUS_HANDLERS: [Mutex<UnsafeCell<Option<Handler>>>; 4] = [ + Mutex::new(UnsafeCell::new(None)), + Mutex::new(UnsafeCell::new(None)), + Mutex::new(UnsafeCell::new(None)), + Mutex::new(UnsafeCell::new(None)), ]; -/// Register an IRQ handler for an I3C bus +/// Register an IRQ handler for an I3C bus. +/// +/// Single-shot per bus: the first registration claims the slot for the +/// program's lifetime, mirroring the one-controller-per-physical-bus contract +/// of [`Ast1060I3c::new`]. Returns `false` (and leaves the existing handler in +/// place) if `bus` is out of range or the slot is already claimed. /// /// # Arguments /// * `bus` - Bus index (0-3) /// * `func` - Handler function /// * `ctx` - Context value passed to handler -/// -/// # Panics -/// Panics if `bus >= 4`. -pub fn register_i3c_irq_handler(bus: usize, func: fn(usize), ctx: usize) { - assert!(bus < 4); +#[must_use] +pub fn register_i3c_irq_handler(bus: usize, func: fn(usize), ctx: usize) -> bool { + let Some(slot) = BUS_HANDLERS.get(bus) else { + return false; + }; critical_section::with(|cs| { - *BUS_HANDLERS[bus].borrow(cs).borrow_mut() = Some(Handler { func, ctx }); - }); + // SAFETY: the critical section excludes ISR/thread concurrency, and + // the `&mut` never escapes this leaf function, so this is the only + // live reference to the slot. + let handler: &mut Option<Handler> = unsafe { &mut *slot.borrow(cs).get() }; + if handler.is_some() { + return false; + } + *handler = Some(Handler { func, ctx }); + true + }) +} + +/// NVIC interrupt line for an I3C bus, if the bus exists. +/// +/// The driver does not touch the NVIC (Delta D6): the kernel/integration layer +/// owns the top-level vector *and* the line mask. After registering a handler +/// and initializing the hardware, the integration layer uses this mapping to +/// unmask (and, on teardown, mask) the line it owns. +#[must_use] +pub const fn i3c_bus_interrupt(bus: u8) -> Option<ast1060_pac::Interrupt> { + match bus { + 0 => Some(ast1060_pac::Interrupt::i3c), + 1 => Some(ast1060_pac::Interrupt::i3c1), + 2 => Some(ast1060_pac::Interrupt::i3c2), + 3 => Some(ast1060_pac::Interrupt::i3c3), + _ => None, + } } /// Dispatch IRQ for a specific bus @@ -105,8 +138,12 @@ #[inline] pub fn dispatch_i3c_irq(bus: usize) { // Copy handler out of critical section to avoid blocking IRQs during handler - let handler = - critical_section::with(|cs| BUS_HANDLERS.get(bus).and_then(|m| *m.borrow(cs).borrow())); + let handler = critical_section::with(|cs| { + // SAFETY: the critical section excludes the writer + // (`register_i3c_irq_handler`); `Handler` is `Copy`, so the value is + // copied out and no reference escapes. + BUS_HANDLERS.get(bus).and_then(|m| unsafe { *m.borrow(cs).get() }) + }); if let Some(h) = handler { (h.func)(h.ctx); } @@ -128,12 +165,6 @@ /// Get the bus number for this instance fn bus_num(&self) -> u8; - /// Enable interrupts - fn enable_irq(&mut self); - - /// Disable interrupts - fn disable_irq(&mut self); - /// Enable the I3C controller fn i3c_enable(&mut self, config: &I3cConfig); @@ -338,37 +369,6 @@ + HardwareTarget { } -pub trait Instance { - fn ptr() -> *const ast1060_pac::i3c::RegisterBlock; - fn ptr_global() -> *const ast1060_pac::i3cglobal::RegisterBlock; - fn scu() -> *const ast1060_pac::scu::RegisterBlock; - const BUS_NUM: u8; -} - -macro_rules! macro_i3c { - ($I3cx: ident, $x: literal) => { - impl Instance for ast1060_pac::$I3cx { - fn ptr() -> *const ast1060_pac::i3c::RegisterBlock { - ast1060_pac::$I3cx::ptr() - } - - fn ptr_global() -> *const ast1060_pac::i3cglobal::RegisterBlock { - ast1060_pac::I3cglobal::ptr() - } - - fn scu() -> *const ast1060_pac::scu::RegisterBlock { - ast1060_pac::Scu::ptr() - } - const BUS_NUM: u8 = $x; - } - }; -} - -macro_i3c!(I3c, 0); -macro_i3c!(I3c1, 1); -macro_i3c!(I3c2, 2); -macro_i3c!(I3c3, 3); - /// I3C bus 0 interrupt handler - call this from your ISR #[inline] pub fn i3c_irq_handler() { @@ -402,76 +402,67 @@ // kernel ISR and an `unexpected_cfgs` lint, with no observable difference in // the deployed (feature-off) build. -/// Concrete AST1060 I3C hardware implementation — a Confined-`unsafe` MMIO -/// façade (Delta D3) over the I3C / I3C-global / SCU register blocks for one -/// bus, plus a Cooperative-Yield wait policy (Delta D2). +/// Concrete AST1060 I3C hardware implementation: the per-bus +/// [`I3cRegisters`] façade (Delta D3 — all MMIO `unsafe` confined there) +/// plus a Cooperative-Yield wait policy (Delta D2). /// -/// The three register blocks are held as raw `*const` pointers; the entire -/// `unsafe` perimeter is the single [`new`](Self::new) constructor. `Y` is the -/// caller-injected yield closure invoked between completion polls (see -/// [`super::types::Completion::wait_for_us`]); pass -/// `|_| core::hint::spin_loop()` for a bare-metal busy-wait. -pub struct Ast1060I3c<I3C: Instance, Y: FnMut(u32)> { - i3c: *const ast1060_pac::i3c::RegisterBlock, - i3cg: *const ast1060_pac::i3cglobal::RegisterBlock, - scu: *const ast1060_pac::scu::RegisterBlock, +/// One driver type manages any of the bus instances — the bus is selected at +/// **runtime** in [`new`](Self::new), so several controllers (one per bus) +/// share this single type. `Y` is the caller-injected yield closure invoked +/// between completion polls (see [`super::types::Completion::wait_for_us`]); +/// pass `|_| core::hint::spin_loop()` for a bare-metal busy-wait. +/// +/// Not `Copy`/`Clone`: this value owns the (also non-`Copy`) registers +/// wrapper, so bus exclusivity follows from ownership. +pub struct Ast1060I3c<Y: FnMut(u32)> { + regs: I3cRegisters, /// Cooperative yield hook invoked between status polls. Argument is the - /// suggested wait window in nanoseconds (advisory). - pub(crate) yield_fn: Y, - _marker: PhantomData<I3C>, - /// Makes `Ast1060I3c` `!Sync` so the raw register pointers can't be shared - /// across threads without explicit synchronization. - _not_sync: PhantomData<UnsafeCell<()>>, + /// suggested wait window in nanoseconds (advisory). Private so external + /// code cannot swap the wait policy out from under an active driver. + yield_fn: Y, } -impl<I3C: Instance, Y: FnMut(u32)> Ast1060I3c<I3C, Y> { - /// Create a new I3C hardware façade for bus `I3C`. +impl<Y: FnMut(u32)> Ast1060I3c<Y> { + /// Create the I3C hardware driver for `bus` (0..=3). Returns `None` if + /// `bus` is out of range. /// /// # Safety /// - /// This is the entire `unsafe` perimeter for this type (Delta D3): - /// - `I3C::ptr()` / `I3C::ptr_global()` / `I3C::scu()` must return valid - /// pointers to the I3C, I3C-global, and SCU register blocks for the - /// program's lifetime (they do for the AST1060 PAC singletons). - /// - Access to the returned instance must be serialized by the caller - /// (the device is `!Sync`); only one `Ast1060I3c` per physical bus may - /// be active at a time. - pub unsafe fn new(yield_fn: Y) -> Self { - Self { - i3c: I3C::ptr(), - i3cg: I3C::ptr_global(), - scu: I3C::scu(), - yield_fn, - _marker: PhantomData, - _not_sync: PhantomData, - } + /// Delegates the [`I3cRegisters::new`] contract — the entire MMIO + /// `unsafe` perimeter: + /// - the AST1060 PAC singleton pointers are valid for the program's + /// lifetime (they are on AST1060 hardware); + /// - access to the returned instance is serialized by the caller (the + /// device is `!Sync`); only one `Ast1060I3c` per physical bus may be + /// active at a time. + pub unsafe fn new(bus: u8, yield_fn: Y) -> Option<Self> { + // SAFETY: forwarded — see this function's contract above. + let regs = unsafe { I3cRegisters::new(bus) }?; + Some(Self { regs, yield_fn }) } - /// The only repeated interior `unsafe` for the I3C block. - /// - /// Returns a `'static` reference: the constructor's contract guarantees the - /// pointer is valid for the program lifetime, so the borrow is not tied to - /// `&self`. This lets a register reference and `&mut self.yield_fn` be held - /// in disjoint statements at the bounded-poll sites without a borrow clash. + /// I3C register block (safe delegate to the façade). #[inline] fn i3c(&self) -> &'static ast1060_pac::i3c::RegisterBlock { - // SAFETY: `new` guarantees a valid pointer for the program lifetime; - // access is serialized by the caller (the type is `!Sync`). - unsafe { &*self.i3c } + self.regs.i3c() } - /// The only repeated interior `unsafe` for the I3C-global block. See [`i3c`](Self::i3c). + /// I3C-global register block (safe delegate to the façade). #[inline] fn i3cg(&self) -> &'static ast1060_pac::i3cglobal::RegisterBlock { - // SAFETY: see `i3c`. - unsafe { &*self.i3cg } + self.regs.i3cg() } - /// The only repeated interior `unsafe` for the SCU block. See [`i3c`](Self::i3c). + /// SCU register block (safe delegate to the façade). #[inline] fn scu(&self) -> &'static ast1060_pac::scu::RegisterBlock { - // SAFETY: see `i3c`. - unsafe { &*self.scu } + self.regs.scu() + } + + /// Bus index this driver was constructed for. + #[inline] + fn bus(&self) -> u8 { + self.regs.bus() } } @@ -499,7 +490,8 @@ 1 => $self.i3cg().i3c024().read().bits(), 2 => $self.i3cg().i3c034().read().bits(), 3 => $self.i3cg().i3c044().read().bits(), - _ => panic!("invalid I3C bus index: {}", $bus), + // Unreachable: `I3cRegisters::new` validates the bus index. + _ => 0, } }}; } @@ -511,7 +503,8 @@ 1 => $self.i3cg().i3c020().write(|$w| $body), 2 => $self.i3cg().i3c030().write(|$w| $body), 3 => $self.i3cg().i3c040().write(|$w| $body), - _ => panic!("invalid I3C bus index: {}", $bus), + // Unreachable: `I3cRegisters::new` validates the bus index. + _ => 0, } }}; } @@ -523,7 +516,8 @@ 1 => $self.i3cg().i3c020().read().bits(), 2 => $self.i3cg().i3c030().read().bits(), 3 => $self.i3cg().i3c040().read().bits(), - _ => panic!("invalid I3C bus index: {}", $bus), + // Unreachable: `I3cRegisters::new` validates the bus index. + _ => 0, } }}; } @@ -535,7 +529,8 @@ 1 => $self.i3cg().i3c024().write(|$w| $body), 2 => $self.i3cg().i3c034().write(|$w| $body), 3 => $self.i3cg().i3c044().write(|$w| $body), - _ => panic!("invalid I3C bus index: {}", $bus), + // Unreachable: `I3cRegisters::new` validates the bus index. + _ => 0, } }}; } @@ -547,7 +542,8 @@ 1 => $self.i3cg().i3c024().modify(|$r, $w| $body), 2 => $self.i3cg().i3c034().modify(|$r, $w| $body), 3 => $self.i3cg().i3c044().modify(|$r, $w| $body), - _ => panic!("invalid I3C bus index: {}", $bus), + // Unreachable: `I3cRegisters::new` validates the bus index. + _ => 0, } }}; } @@ -632,9 +628,9 @@ Err(PollError::Timeout) } -impl<I3C: Instance, Y: FnMut(u32)> Ast1060I3c<I3C, Y> { +impl<Y: FnMut(u32)> Ast1060I3c<Y> { fn toggle_scl_in(&mut self, count: u32) { - let bus = I3C::BUS_NUM; + let bus = self.bus(); for _ in 0..count { modify_i3cg_reg1!(self, bus, |r, w| unsafe { w.bits(r.bits() & !I3CG_REG1_SCL_IN_SW_MODE_VAL) @@ -646,7 +642,7 @@ } fn gen_internal_stop(&mut self) { - let bus = I3C::BUS_NUM; + let bus = self.bus(); modify_i3cg_reg1!(self, bus, |r, w| unsafe { w.bits(r.bits() & !I3CG_REG1_SCL_IN_SW_MODE_VAL) }); @@ -663,7 +659,7 @@ fn enter_sw_mode(&mut self) { i3c_debug!(self.logger, "enter sw mode"); - let bus = I3C::BUS_NUM; + let bus = self.bus(); let mut reg = read_i3cg_reg1!(self, bus); reg |= I3CG_REG1_SCL_IN_SW_MODE_VAL | I3CG_REG1_SDA_IN_SW_MODE_VAL; modify_i3cg_reg1!(self, bus, |_r, w| unsafe { w.bits(reg) }); @@ -672,7 +668,7 @@ } fn exit_sw_mode(&mut self) { - let bus = I3C::BUS_NUM; + let bus = self.bus(); let mut reg = read_i3cg_reg1!(self, bus); reg &= !(I3CG_REG1_SCL_IN_SW_MODE_EN | I3CG_REG1_SDA_IN_SW_MODE_EN); modify_i3cg_reg1!(self, bus, |_r, w| unsafe { w.bits(reg) }); @@ -696,7 +692,8 @@ .scu() .scu050() .modify(|_, w| w.rst_i3c3ctrl().set_bit()), - _ => panic!("invalid I3C bus index: {bus}"), + // Unreachable: `I3cRegisters::new` validates the bus index. + _ => 0, }; } @@ -729,37 +726,37 @@ } } -impl<I3C: Instance, Y: FnMut(u32)> HardwareCore for Ast1060I3c<I3C, Y> { +impl<Y: FnMut(u32)> HardwareCore for Ast1060I3c<Y> { #[allow(clippy::too_many_lines)] fn init(&mut self, config: &mut I3cConfig) { i3c_debug!(self.logger, "i3c init"); self.global_reset_deassert(); - write_i3cg_reg1!(self, I3C::BUS_NUM, |w| unsafe { + write_i3cg_reg1!(self, self.bus(), |w| unsafe { w.actmode() .bits(1) .instid() - .bits(I3C::BUS_NUM) + .bits(self.bus()) .staticaddr() .bits(I3C_DEFAULT_STATIC_ADDR) }); - let reg = read_i3cg_reg1!(self, I3C::BUS_NUM); + let reg = read_i3cg_reg1!(self, self.bus()); i3c_debug!(self.logger, "i3cg_reg1: {:#x}", reg); - write_i3cg_reg0!(self, I3C::BUS_NUM, |w| unsafe { w.bits(0x0) }); - let reg = read_i3cg_reg0!(self, I3C::BUS_NUM); + write_i3cg_reg0!(self, self.bus(), |w| unsafe { w.bits(0x0) }); + let reg = read_i3cg_reg0!(self, self.bus()); i3c_debug!(self.logger, "i3cg_reg0: {:#x}", reg); - self.core_reset_assert(I3C::BUS_NUM); - self.clock_on(I3C::BUS_NUM); - self.core_reset_deassert(I3C::BUS_NUM); + self.core_reset_assert(self.bus()); + self.clock_on(self.bus()); + self.core_reset_deassert(self.bus()); self.i3c_disable(config.is_secondary); i3c_debug!( self.logger, "bus num: {}, is_secondary: {}", - I3C::BUS_NUM, + self.bus(), config.is_secondary ); @@ -906,32 +903,7 @@ } fn bus_num(&self) -> u8 { - I3C::BUS_NUM - } - - 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), - 1 => NVIC::unmask(ast1060_pac::Interrupt::i3c1), - 2 => NVIC::unmask(ast1060_pac::Interrupt::i3c2), - 3 => NVIC::unmask(ast1060_pac::Interrupt::i3c3), - _ => {} - } - } - } - - fn disable_irq(&mut self) { - match I3C::BUS_NUM { - 0 => NVIC::mask(ast1060_pac::Interrupt::i3c), - 1 => NVIC::mask(ast1060_pac::Interrupt::i3c1), - 2 => NVIC::mask(ast1060_pac::Interrupt::i3c2), - 3 => NVIC::mask(ast1060_pac::Interrupt::i3c3), - _ => {} - } + self.bus() } fn i3c_disable(&mut self, is_secondary: bool) { @@ -1011,7 +983,7 @@ if let Some(tc) = &mut config.target_config { tc.addr = Some(da); } - let _ = ibi_workq::i3c_ibi_work_enqueue_target_da_assignment(I3C::BUS_NUM.into()); + let _ = ibi_workq::i3c_ibi_work_enqueue_target_da_assignment(self.bus().into()); } if (status & INTR_RESP_READY_STAT) != 0 { @@ -1037,7 +1009,7 @@ } } -impl<I3C: Instance, Y: FnMut(u32)> HardwareClock for Ast1060I3c<I3C, Y> { +impl<Y: FnMut(u32)> HardwareClock for Ast1060I3c<Y> { fn init_clock(&mut self, config: &mut I3cConfig) { // `unwrap_or` + `.max(1)` (not `.expect()` / raw divides) keep this // panic-free for the `no_panics` analysis: a missing/zero core clock @@ -1155,7 +1127,7 @@ } fn init_pid(&mut self, config: &mut I3cConfig) { - let bus = I3C::BUS_NUM; + let bus = self.bus(); self.i3c().i3cd070().write(|w| unsafe { w.slvmipimfgid() .bits(I3C_AST10X0_MIPI_MANUF_ID) @@ -1174,7 +1146,7 @@ } } -impl<I3C: Instance, Y: FnMut(u32)> HardwareFifo for Ast1060I3c<I3C, Y> { +impl<Y: FnMut(u32)> HardwareFifo for Ast1060I3c<Y> { fn wr_tx_fifo(&mut self, bytes: &[u8]) { let mut chunks = bytes.chunks_exact(4); for chunk in &mut chunks { @@ -1234,7 +1206,7 @@ } } -impl<I3C: Instance, Y: FnMut(u32)> HardwareRecovery for Ast1060I3c<I3C, Y> { +impl<Y: FnMut(u32)> HardwareRecovery for Ast1060I3c<Y> { fn enter_sw_mode(&mut self) { self.enter_sw_mode(); } @@ -1264,7 +1236,7 @@ } } -impl<I3C: Instance, Y: FnMut(u32)> HardwareTransfer for Ast1060I3c<I3C, Y> { +impl<Y: FnMut(u32)> HardwareTransfer for Ast1060I3c<Y> { fn set_ibi_mdb(&mut self, mdb: u8) { self.i3c() .i3cd000() @@ -1979,7 +1951,7 @@ let mut ibi_buf: [u8; 2] = [0u8; 2]; let take = core::cmp::min(len, ibi_buf.len()); self.rd_ibi_fifo(&mut ibi_buf[..take]); - let bus = I3C::BUS_NUM as usize; + let bus = self.bus() as usize; let _ = ibi_workq::i3c_ibi_work_enqueue_target_irq(bus, addr, &ibi_buf[..take]); } @@ -2014,7 +1986,7 @@ self.handle_ibi_sir(config, ibi_addr as u8, ibi_data_len); } else if ibi_addr == 2 && !rnw { // hot-join - let bus = I3C::BUS_NUM as usize; + let bus = self.bus() as usize; i3c_debug!(self.logger, "Hot-join IBI"); let _ = ibi_workq::i3c_ibi_work_enqueue_hotjoin(bus); } else { @@ -2027,7 +1999,7 @@ } } -impl<I3C: Instance, Y: FnMut(u32)> HardwareTarget for Ast1060I3c<I3C, Y> { +impl<Y: FnMut(u32)> HardwareTarget for Ast1060I3c<Y> { fn target_tx_write(&mut self, buf: &[u8]) { self.wr_tx_fifo(buf); let cmd = field_prep(COMMAND_PORT_ATTR, COMMAND_ATTR_SLAVE_DATA_CMD) @@ -2101,7 +2073,7 @@ self.rd_rx_fifo(dst); } let _ = ibi_workq::i3c_ibi_work_enqueue_target_master_write( - I3C::BUS_NUM.into(), + self.bus().into(), buf.get(..n).unwrap_or(&[]), ); i3c_debug!(
diff --git a/target/ast10x0/peripherals/i3c/ibi.rs b/target/ast10x0/peripherals/i3c/ibi.rs index 1c382d5..e44de6c 100644 --- a/target/ast10x0/peripherals/i3c/ibi.rs +++ b/target/ast10x0/peripherals/i3c/ibi.rs
@@ -124,28 +124,36 @@ Mutex::new(UnsafeCell::new(IbiRing::new())), ]; -/// Run `f` against the ring for `bus`, serialized by the critical section. +/// Push `work` onto the ring for `bus`. Returns `false` if `bus` is out of +/// range or the ring is full. /// -/// 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: 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`. +/// The `&mut IbiRing` is confined to this leaf function — no caller-provided +/// code runs while it is live — so the exclusive borrow cannot be re-entered. +fn ring_push(bus: usize, work: IbiWork) -> bool { + let Some(workq) = IBI_RINGS.get(bus) else { + return false; + }; + critical_section::with(|cs| { + // SAFETY: the critical section excludes ISR/thread concurrency, and + // the `&mut IbiRing` never escapes this function (the ring is only + // reachable via `ring_push`/`ring_pop`, neither of which calls back + // into caller code), so this is the only live reference. let ring: &mut IbiRing = unsafe { &mut *workq.borrow(cs).get() }; - f(ring) - })) + ring.push(work) + }) +} + +/// Pop the next work item from the ring for `bus`, if any. +/// +/// Same confinement argument as [`ring_push`]. +fn ring_pop(bus: usize) -> Option<IbiWork> { + let workq = IBI_RINGS.get(bus)?; + critical_section::with(|cs| { + // SAFETY: see `ring_push` — critical section + leaf confinement make + // this the only live reference to the ring. + let ring: &mut IbiRing = unsafe { &mut *workq.borrow(cs).get() }; + ring.pop() + }) } // ============================================================================= @@ -164,7 +172,7 @@ /// Dequeue the next IBI work item, if any. #[must_use] pub fn dequeue(&mut self) -> Option<IbiWork> { - with_ring(self.bus, IbiRing::pop).flatten() + ring_pop(self.bus) } } @@ -186,13 +194,13 @@ /// Enqueue a target dynamic address assignment notification #[must_use] pub fn i3c_ibi_work_enqueue_target_da_assignment(bus: usize) -> bool { - with_ring(bus, |r| r.push(IbiWork::TargetDaAssignment)).unwrap_or(false) + ring_push(bus, IbiWork::TargetDaAssignment) } /// Enqueue a Hot-Join notification #[must_use] pub fn i3c_ibi_work_enqueue_hotjoin(bus: usize) -> bool { - with_ring(bus, |r| r.push(IbiWork::HotJoin)).unwrap_or(false) + ring_push(bus, IbiWork::HotJoin) } /// Enqueue a target interrupt (SIR) notification @@ -206,7 +214,7 @@ len: u8::try_from(take).unwrap_or(IBI_DATA_MAX), data: ibi_buf, }; - with_ring(bus, |r| r.push(work)).unwrap_or(false) + ring_push(bus, work) } /// Enqueue a private write received by this target from the controller. @@ -219,5 +227,5 @@ len: u8::try_from(take).unwrap_or(IBI_DATA_MAX), data: buf, }; - with_ring(bus, |r| r.push(work)).unwrap_or(false) + ring_push(bus, work) }
diff --git a/target/ast10x0/peripherals/i3c/mod.rs b/target/ast10x0/peripherals/i3c/mod.rs index 2ebe6de..f96ce5e 100644 --- a/target/ast10x0/peripherals/i3c/mod.rs +++ b/target/ast10x0/peripherals/i3c/mod.rs
@@ -40,14 +40,16 @@ pub mod error; pub mod hardware; pub mod ibi; +pub mod registers; pub mod types; // ============================================================================= // Public Re-exports // ============================================================================= -// Controller -pub use controller::I3cController; +// Controller (two-state lifecycle: Uninitialized -> Ready, matching the SMC +// peripheral's precedent) +pub use controller::{I3cController, I3cCore, Ready, Uninitialized}; // Error types pub use error::{CccErrorKind, I3cError, Result}; @@ -67,9 +69,13 @@ // Hardware interface pub use hardware::{ Ast1060I3c, HardwareClock, HardwareCore, HardwareFifo, HardwareInterface, HardwareRecovery, - HardwareTarget, HardwareTransfer, Instance, dispatch_i3c_irq, register_i3c_irq_handler, + HardwareTarget, HardwareTransfer, dispatch_i3c_irq, i3c_bus_interrupt, + register_i3c_irq_handler, }; +// Confined-unsafe MMIO façade (runtime bus selection) +pub use registers::I3cRegisters; + // CCC operations pub use ccc::{ Ccc, CccPayload, CccRstActDefByte, CccTargetPayload, GetStatusDefByte, GetStatusFormat,
diff --git a/target/ast10x0/peripherals/i3c/registers.rs b/target/ast10x0/peripherals/i3c/registers.rs new file mode 100644 index 0000000..7292e02 --- /dev/null +++ b/target/ast10x0/peripherals/i3c/registers.rs
@@ -0,0 +1,112 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! Confined-`unsafe` MMIO façade over the per-bus I3C register blocks. +//! +//! One driver manages multiple bus instances: the bus is selected at +//! **runtime** by index (no per-instance type parameter), mirroring the +//! reference `aspeed-rust` driver. All `unsafe` needed to touch the I3C, +//! I3C-global, and SCU register blocks is confined to this type — one +//! `unsafe fn` constructor and three private deref helpers — so the rest of +//! the driver (`hardware.rs` upward) is `unsafe`-free for MMIO. + +use core::marker::PhantomData; + +use super::constants::MAX_BUSES; + +/// Safe wrapper around the I3C / I3C-global / SCU hardware registers of one bus. +/// +/// This struct consolidates all unsafe I3C MMIO access. All register +/// operations go through this single point, making it easy to audit safety +/// invariants — the same shape as `SmcRegisters` in `smc/registers.rs`. +/// +/// Not `Copy`/`Clone`: an `I3cRegisters` represents exclusive ownership of +/// one bus's register blocks. +pub struct I3cRegisters { + i3c: *const ast1060_pac::i3c::RegisterBlock, + i3cg: *const ast1060_pac::i3cglobal::RegisterBlock, + scu: *const ast1060_pac::scu::RegisterBlock, + bus: u8, + // `*const ()` marker keeps the handle `!Send` and `!Sync`. An + // `I3cRegisters` represents exclusive ownership of one bus's register + // blocks; it must not be shared between threads or moved into another + // execution context where it could alias the controller it owns. + _not_send_sync: PhantomData<*const ()>, +} + +impl I3cRegisters { + /// Create the register façade for `bus` (0..[`MAX_BUSES`]). + /// + /// Returns `None` if `bus` is out of range — every accessor below is + /// therefore panic-free: a constructed façade always holds valid pointers + /// and an in-range bus index. + /// + /// # Safety + /// + /// This is the entire `unsafe` perimeter for I3C MMIO (Delta D3): + /// - The AST1060 PAC singleton pointers (`I3c*::ptr()`, + /// `I3cglobal::ptr()`, `Scu::ptr()`) must point to valid register + /// blocks for the program's lifetime (they do on AST1060 hardware). + /// - Access through the returned façade must be serialized by the caller + /// (the type is `!Sync`); only one owner per physical bus may be + /// active at a time. + #[must_use] + pub const unsafe fn new(bus: u8) -> Option<Self> { + let i3c = match bus { + 0 => ast1060_pac::I3c::ptr(), + 1 => ast1060_pac::I3c1::ptr(), + 2 => ast1060_pac::I3c2::ptr(), + 3 => ast1060_pac::I3c3::ptr(), + _ => return None, + }; + // Redundant with the match above, but keeps the invariant explicit if + // MAX_BUSES and the match ever diverge. + if bus as usize >= MAX_BUSES { + return None; + } + Some(Self { + i3c, + i3cg: ast1060_pac::I3cglobal::ptr(), + scu: ast1060_pac::Scu::ptr(), + bus, + _not_send_sync: PhantomData, + }) + } + + /// Bus index this façade was constructed for (always `< MAX_BUSES`). + #[inline] + #[must_use] + pub fn bus(&self) -> u8 { + self.bus + } + + /// The only repeated interior `unsafe` for the I3C block. + /// + /// Returns a `'static` reference: the constructor's contract guarantees + /// the pointer is valid for the program lifetime, so the borrow is not + /// tied to `&self`. This lets a register reference and a `&mut yield_fn` + /// be held in disjoint statements at the bounded-poll sites without a + /// borrow clash. + #[inline] + pub(crate) fn i3c(&self) -> &'static ast1060_pac::i3c::RegisterBlock { + // SAFETY: `new` guarantees a valid pointer for the program lifetime; + // access is serialized by the caller (the type is `!Sync`). + unsafe { &*self.i3c } + } + + /// The only repeated interior `unsafe` for the I3C-global block. See + /// [`i3c`](Self::i3c). + #[inline] + pub(crate) fn i3cg(&self) -> &'static ast1060_pac::i3cglobal::RegisterBlock { + // SAFETY: see `i3c`. + unsafe { &*self.i3cg } + } + + /// The only repeated interior `unsafe` for the SCU block. See + /// [`i3c`](Self::i3c). + #[inline] + pub(crate) fn scu(&self) -> &'static ast1060_pac::scu::RegisterBlock { + // SAFETY: see `i3c`. + unsafe { &*self.scu } + } +}
diff --git a/target/ast10x0/tests/peripherals/i3c/i3c_init/BUILD.bazel b/target/ast10x0/tests/peripherals/i3c/i3c_init/BUILD.bazel index 1430026..8be8d5d 100644 --- a/target/ast10x0/tests/peripherals/i3c/i3c_init/BUILD.bazel +++ b/target/ast10x0/tests/peripherals/i3c/i3c_init/BUILD.bazel
@@ -73,6 +73,7 @@ "@pigweed//pw_kernel/target:target_common", "@pigweed//pw_log/rust:pw_log", "@pigweed//pw_status/rust:pw_status", + "@rust_crates//:cortex-m", "@rust_crates//:cortex-m-semihosting", ], )
diff --git a/target/ast10x0/tests/peripherals/i3c/i3c_init/target.rs b/target/ast10x0/tests/peripherals/i3c/i3c_init/target.rs index 4b2fe7a..a5807ce 100644 --- a/target/ast10x0/tests/peripherals/i3c/i3c_init/target.rs +++ b/target/ast10x0/tests/peripherals/i3c/i3c_init/target.rs
@@ -10,12 +10,15 @@ //! driver — the behavioral-parity port of `aspeed-rust/src/i3c/` //! (see `target/ast10x0/peripherals/i3c/plans/goal.md`). Validates the clock //! configuration, constructs the controller behind the confined-`unsafe` -//! façade with a busy-spin yield closure, runs `init_hardware`, and (on real -//! hardware) verifies the controller-enable bit. Reports PASS/FAIL via the -//! console sentinel, matching the I2C tests. +//! façade with a busy-spin yield hook, runs the `Uninitialized -> Ready` +//! bring-up (`start()`), and (on real hardware) verifies the +//! controller-enable bit. Reports PASS/FAIL via the console sentinel, +//! matching the I2C tests. + +use core::pin::Pin; use ast10x0_board::{Ast10x0Board, Ast10x0BoardDescriptor}; -use ast10x0_peripherals::i3c::{Ast1060I3c, I3cConfig, I3cController}; +use ast10x0_peripherals::i3c::{Ast1060I3c, I3cConfig, I3cController, I3cCore}; use ast10x0_peripherals::scu::pinctrl; use codegen as _; use console_backend::console_backend_write_all; @@ -24,6 +27,17 @@ pub struct Target {} +/// One driver type serves every bus; the instance is selected at runtime. +type I3cHw = Ast1060I3c<fn(u32)>; +/// Bus index under test (PAC `I3c`, the first instance). +const I3C_BUS: u8 = 0; + +/// Busy-spin yield hook (bare-metal wait policy). A named `fn` (not a closure) +/// keeps the `I3cCore` type nameable for the `singleton!` storage. +fn yield_spin(_ns: u32) { + core::hint::spin_loop(); +} + /// Example platform core clock (Hz) for timing computation. The AST1060 I3C /// core is fed from the HCLK domain; 200 MHz is a representative value and is /// only used to derive the timing-register fields during `init`. @@ -55,13 +69,20 @@ pw_log::info!("Clock configuration validated"); // 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 = core::pin::pin!(I3cController::new(hw, config)); + // PAC register blocks; the busy-spin hook is the bare-metal wait policy. + let hw = unsafe { I3cHw::new(I3C_BUS, yield_spin) }.ok_or("invalid I3C bus index")?; + // The ISR-shared core lives in a static so its address is stable and + // `'static` — the IRQ trampoline's pointer validity is type-guaranteed. + let i3c_core = cortex_m::singleton!(: I3cCore<I3cHw> = I3cCore::new(hw, config)) + .ok_or("I3C core storage already taken")?; + let ctrl = I3cController::new(Pin::static_mut(i3c_core)); pw_log::info!("Controller constructed"); - ctrl.as_mut().init_hardware(); - pw_log::info!("init_hardware complete"); + // `start()` claims the IRQ slot (single-shot) and programs the hardware. + // This smoke test leaves the NVIC line masked (its system.json5 has no + // I3C vector entry), so no interrupt can be delivered. + let _ctrl = ctrl.start().map_err(|_| "controller start failed")?; + pw_log::info!("controller start complete"); // On real hardware the controller-enable bit must be set after a primary // (non-secondary) init. QEMU `ast1030-evb` does not model the I3C block, so
diff --git a/target/ast10x0/tests/peripherals/i3c/i3c_irq/BUILD.bazel b/target/ast10x0/tests/peripherals/i3c/i3c_irq/BUILD.bazel index 8428ddc..f1193ca 100644 --- a/target/ast10x0/tests/peripherals/i3c/i3c_irq/BUILD.bazel +++ b/target/ast10x0/tests/peripherals/i3c/i3c_irq/BUILD.bazel
@@ -20,6 +20,7 @@ "@pigweed//pw_kernel/target:target_common", "@pigweed//pw_log/rust:pw_log", "@pigweed//pw_status/rust:pw_status", + "@rust_crates//:cortex-m", "@rust_crates//:cortex-m-semihosting", ]
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 301c820..af22bce 100644 --- a/target/ast10x0/tests/peripherals/i3c/i3c_irq/slave_target.rs +++ b/target/ast10x0/tests/peripherals/i3c/i3c_irq/slave_target.rs
@@ -22,20 +22,29 @@ #![no_std] #![no_main] +use core::pin::Pin; + use ast10x0_board::{Ast10x0Board, Ast10x0BoardDescriptor}; use ast10x0_peripherals::i3c::{ - Ast1060I3c, I3cConfig, I3cController, I3cTargetConfig, IbiConsumer, IbiWork, - i3c_ibi_workq_consumer, + Ast1060I3c, I3cConfig, I3cController, I3cCore, I3cTargetConfig, IbiConsumer, IbiWork, Ready, + i3c_bus_interrupt, i3c_ibi_workq_consumer, }; use ast10x0_peripherals::scu::pinctrl; use codegen as _; use console_backend::console_backend_write_all; +use cortex_m::peripheral::NVIC; use entry as _; use kernel::Kernel; use target_common::{TargetInterface, declare_target}; pub struct Target {} +/// One driver type serves every bus; the instance is selected at runtime. +type I3cHw = Ast1060I3c<fn(u32)>; + +/// Bus index under test (PAC `I3c2`, HV pads). +const I3C_BUS: u8 = 2; + /// Number of IBIs the target raises once it has a dynamic address. const MAX_IBIS: u32 = 10; /// Give the controller time to finish init and open the hot-join ACK window. @@ -136,10 +145,12 @@ } /// Build + validate the target controller in its own `#[inline(never)]` frame -/// so the temporary `I3cConfig` (256-byte `AddrBook` inside) is freed on return -/// rather than lingering alongside `ctrl` on the 2 KiB kernel bootstrap stack. +/// so the temporary `I3cConfig` (256-byte `AddrBook` inside) is freed on +/// return; the long-lived hardware + config live in the `singleton!` static +/// (`I3cCore`). Returns the controller in the `Ready` state: handler +/// registered, hardware programmed, NVIC line still masked. #[inline(never)] -fn build_target() -> Result<I3cController<Ast1060I3c<ast1060_pac::I3c2, fn(u32)>>, &'static str> { +fn build_target() -> Result<I3cController<I3cHw, Ready>, &'static str> { // Secondary (target) timing — identical to the reference target. let mut config = I3cConfig::new() .core_clk_hz(200_000_000) @@ -159,8 +170,12 @@ .map_err(|_| "invalid clock configuration")?; // SAFETY: the test owns I3C bus 2 and uses the matching PAC blocks. - let hw = unsafe { Ast1060I3c::<ast1060_pac::I3c2, fn(u32)>::new(yield_delay) }; - Ok(I3cController::new(hw, config)) + let hw = unsafe { I3cHw::new(I3C_BUS, yield_delay) }.ok_or("invalid I3C bus index")?; + let i3c_core = cortex_m::singleton!(: I3cCore<I3cHw> = I3cCore::new(hw, config)) + .ok_or("I3C core storage already taken")?; + I3cController::new(Pin::static_mut(i3c_core)) + .start() + .map_err(|_| "controller start failed") } fn run_target() -> Result<(), &'static str> { @@ -172,20 +187,24 @@ // SAFETY: single call at boot with exclusive access to the board. unsafe { board.init() }; - // Build the controller in a separate (never-inlined) frame so the temporary - // `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 = core::pin::pin!(build_target()?); - let bus = ctrl.as_ref().bus_num() as usize; + // Build the controller (register IRQ + program hardware) in a separate + // (never-inlined) frame so the temporary `I3cConfig` is freed on return + // (see `build_target`); the long-lived state lives in the `I3cCore` static. + let mut ctrl = build_target()?; + let bus = ctrl.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.as_mut().init_hardware(); + // Kernel vector is in place and the handler is registered; the integration + // layer owns the NVIC line, so unmask it now. + let irq = i3c_bus_interrupt(ctrl.bus_num()).ok_or("no IRQ line for bus")?; + // SAFETY: handler registered and hardware initialized (Ready state); + // unmasking cannot deliver an IRQ into partially-initialized state. + unsafe { NVIC::unmask(irq) }; let dyn_addr = 8u8; let dev_idx = 0usize; - let _ = ctrl.as_mut().attach_i3c_dev(0, dyn_addr, dev_idx as u8); + let _ = ctrl.attach_i3c_dev(0, dyn_addr, dev_idx as u8); pw_log::info!( "target dev at slot {}, dyn addr {}", dev_idx as u32, @@ -195,7 +214,7 @@ 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.as_mut().target_raise_hot_join().is_ok(); + let hj_ok = ctrl.target_raise_hot_join().is_ok(); pw_log::info!("[DBG] hot-join raise ok={}", hj_ok as u32); log_target_hj_state(0); @@ -207,7 +226,7 @@ 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.as_mut().target_raise_hot_join().is_ok(); + let hj_ok = ctrl.target_raise_hot_join().is_ok(); pw_log::info!("[DBG] hot-join retry ok={}", hj_ok as u32); log_target_hj_state(1); } @@ -215,11 +234,11 @@ }; match work { IbiWork::TargetDaAssignment => { - let da = ctrl.as_ref().target_dynamic_address(); + let da = ctrl.target_dynamic_address(); if let Some(da) = da { pw_log::info!("[IBI] dyn addr 0x{:02x} assigned by master", da as u32); } - ctrl.as_mut().target_on_dynamic_address_assigned(); + ctrl.target_on_dynamic_address_assigned(); break; } IbiWork::HotJoin => pw_log::info!("[IBI] hotjoin"), @@ -239,7 +258,7 @@ for (i, b) in data.iter_mut().enumerate() { *b = u8::try_from(i).unwrap_or(0); } - if ctrl.as_mut().target_get_ibi_payload(&mut data).is_err() { + if ctrl.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 f9de284..095f0a8 100644 --- a/target/ast10x0/tests/peripherals/i3c/i3c_irq/target.rs +++ b/target/ast10x0/tests/peripherals/i3c/i3c_irq/target.rs
@@ -30,19 +30,25 @@ use ast10x0_board::{Ast10x0Board, Ast10x0BoardDescriptor}; use ast10x0_peripherals::i3c::{ - Ast1060I3c, I3cConfig, I3cController, IbiWork, i3c_ibi_workq_consumer, + Ast1060I3c, I3cConfig, I3cController, I3cCore, IbiWork, Ready, i3c_bus_interrupt, + i3c_ibi_workq_consumer, }; use ast10x0_peripherals::scu::pinctrl; use codegen as _; use console_backend::console_backend_write_all; +use cortex_m::peripheral::NVIC; use entry as _; use kernel::Kernel; use target_common::{TargetInterface, declare_target}; pub struct Target {} -type I3c2Hw = Ast1060I3c<ast1060_pac::I3c2, fn(u32)>; -type I3c2Controller = I3cController<I3c2Hw>; +/// One driver type serves every bus; the instance is selected at runtime. +type I3cHw = Ast1060I3c<fn(u32)>; +type I3c2Controller = I3cController<I3cHw, Ready>; + +/// Bus index under test (PAC `I3c2`, HV pads). +const I3C_BUS: u8 = 2; /// PID of the peer target (matches the `:slave` image / the reference). const KNOWN_PID: u64 = 0x07ec_a003_2000; @@ -93,10 +99,11 @@ /// Build + validate the controller in its own `#[inline(never)]` frame. /// -/// The temporary `I3cConfig` embeds a 256-byte `AddrBook`; keeping it live -/// alongside `ctrl` (which owns a moved copy) would put two `I3cConfig`s on the -/// 2 KiB kernel bootstrap stack and overflow it. Building here frees the -/// temporary on return, leaving `run_controller` with only `ctrl`. +/// The temporary `I3cConfig` embeds a 256-byte `AddrBook` and the kernel +/// bootstrap stack is only 2 KiB, so the temporaries are freed on return; the +/// long-lived hardware + config live in the `singleton!` static (`I3cCore`), +/// not on the stack. Returns the controller in the `Ready` state: handler +/// registered, hardware programmed, NVIC line still masked. #[inline(never)] fn build_controller() -> Result<I3c2Controller, &'static str> { // Controller (primary) timing — identical to the reference master. @@ -116,13 +123,17 @@ .map_err(|_| "invalid clock configuration")?; // SAFETY: the test owns I3C bus 2 and uses the matching PAC blocks. - let hw = unsafe { Ast1060I3c::<ast1060_pac::I3c2, fn(u32)>::new(yield_delay) }; - Ok(I3cController::new(hw, config)) + let hw = unsafe { I3cHw::new(I3C_BUS, yield_delay) }.ok_or("invalid I3C bus index")?; + let i3c_core = cortex_m::singleton!(: I3cCore<I3cHw> = I3cCore::new(hw, config)) + .ok_or("I3C core storage already taken")?; + I3cController::new(Pin::static_mut(i3c_core)) + .start() + .map_err(|_| "controller start failed") } #[inline(never)] fn master_read_from_target( - ctrl: Pin<&mut I3c2Controller>, + ctrl: &mut I3c2Controller, ) -> Result<(u32, [u8; XFER_DATA_LEN]), &'static str> { let mut rx_buf = [0u8; 128]; let actual_len = ctrl @@ -135,10 +146,7 @@ } #[inline(never)] -fn master_write_to_target( - ctrl: Pin<&mut I3c2Controller>, - exchange: u32, -) -> Result<(), &'static str> { +fn master_write_to_target(ctrl: &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,28 +166,30 @@ // SAFETY: single call at boot with exclusive access to the board. unsafe { board.init() }; - // Build the controller in a separate (never-inlined) frame so the temporary - // `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 = core::pin::pin!(build_controller()?); - let bus = ctrl.as_ref().bus_num() as usize; + // Build the controller (register IRQ + program hardware) in a separate + // (never-inlined) frame so the temporary `I3cConfig` is freed on return + // (see `build_controller`): the kernel bootstrap thread stack is only + // 2 KiB; the long-lived state lives in the `I3cCore` static. + let mut ctrl = build_controller()?; + let bus = ctrl.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.as_mut().init_hardware(); + // The kernel vector (system.json5 IRQ 104 -> `i3c2_irq`) is in place and + // the handler is registered; the integration layer owns the NVIC line, so + // unmask it now. + let irq = i3c_bus_interrupt(ctrl.bus_num()).ok_or("no IRQ line for bus")?; + // SAFETY: handler registered and hardware initialized (Ready state); + // unmasking cannot deliver an IRQ into partially-initialized state. + unsafe { NVIC::unmask(irq) }; pw_log::info!("I3C2 controller ready"); let dyn_addr = ctrl - .as_mut() .alloc_dynamic_address_from(8) .ok_or("no dynamic address available")?; - ctrl.as_mut() - .attach_i3c_dev(KNOWN_PID, dyn_addr, 0) + ctrl.attach_i3c_dev(KNOWN_PID, dyn_addr, 0) .map_err(|_| "attach_i3c_dev failed")?; - ctrl.as_mut() - .enable_ibi(dyn_addr, 0) + ctrl.enable_ibi(dyn_addr, 0) .map_err(|_| "ibi_enable failed")?; pw_log::info!("pre-attached dev at slot 0, dyn addr {}", dyn_addr as u32); @@ -230,20 +240,20 @@ match work { IbiWork::HotJoin => { pw_log::info!("[IBI] hotjoin"); - let _ = ctrl.as_mut().handle_hot_join(); - let _ = ctrl.as_mut().assign_dynamic_address(dyn_addr); + let _ = ctrl.handle_hot_join(); + let _ = ctrl.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.as_mut().acknowledge_ibi(addr).is_err() { + if ctrl.acknowledge_ibi(addr).is_err() { pw_log::error!("acknowledge_ibi failed"); } let exchange = received; - let (read_len, read_data) = master_read_from_target(ctrl.as_mut())?; + let (read_len, read_data) = master_read_from_target(&mut ctrl)?; log_master_read_payload(exchange, read_len, &read_data); - master_write_to_target(ctrl.as_mut(), exchange)?; + master_write_to_target(&mut ctrl, exchange)?; received += 1; if received >= MAX_EXCHANGES {