Introduce the ast10x0 i3c peripheral#278
Open
stevenlee7189 wants to merge 18 commits into
Open
Conversation
Port the AST1060 I3C controller/target driver from aspeed-rust (src/i3c @ ce3b567) into the ast10x0_peripherals crate, mirroring the existing I2C port and following the pac-design-patterns conventions. Driver (peripherals/i3c/): ccc, config, constants, controller, error, hardware, ibi, mod, types. hal_impl is folded into controller as inherent methods (proposed_traits is unavailable in openprot and embedded-hal has no I3C trait). Design-pattern conformance: - Confined-`unsafe` MMIO facade: raw `*const` register pointers, a single `unsafe fn new`, private `&'static` derefs, `!Sync`; no unsafe or PAC types leak above the facade. - Cooperative-yield bounded poll: an injected `Y: FnMut(u32)` yield closure replaces embedded-hal DelayNs, type-erased at the poll loops. - Borrow-arbitrated exclusivity for per-call state; the global IBI/IRQ plane is kept (documented delta) since an ISR cannot borrow a stack-owned device. Notable deltas: drop the Logger generic and the `#[no_mangle]` ISR symbol exports (kernel owns the vector and calls dispatch_i3c_irq); heapless 0.9 SPSC with edition-2024 `addr_of_mut!`; reachable transfer/clock paths hardened to be panic-free (no_panics_test) without changing success-path behavior. Also: add scu pinctrl groups (PINCTRL_I3C0..3 / PINCTRL_HVI3C0..3, the HV groups clearing the conflicting LV pad bits); wire critical-section and cortex-m `critical-section-single-core` into the crate universe; add i3c_init and i3c_irq tests, the latter a faithful controller/target IBI pair mirroring aspeed-rust tests-hw on I3C2 HV. See target/ast10x0/peripherals/i3c/plans/goal.md for the behavioral parity plan, authority pin, and deltas ledger. Signed-off-by: Steven Lee <steven_lee@aspeedtech.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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().
Complete the SmcRegisters alignment: instead of only confining the pointer derefs, every register operation now goes through an intent-named method on I3cRegisters (enable_controller_primary, assert_all_queue_resets, dat_program_addr, ...), so hardware.rs is pure protocol logic with zero PAC register types and zero MMIO unsafe. Its remaining unsafe is non-MMIO only: the IRQ-registry critical-section leaves, the forwarded constructor gate, the curr_xfer handoff cast, and the transfer buffer re-slicing. The per-bus I3C-global reg0/reg1 dispatch and the eight-slot DAT dispatch move into private macros inside the facade (the bus index already lives there), deleting the macro layer from hardware.rs along with its unreachable fallback arms. The FIFO word loops sink into the facade as tx_fifo_write/rx_fifo_read/rx_fifo_drain/ibi_fifo_*, which also retires the closure-taking rd_fifo/drain_fifo trait methods.
Restore console_backend.rs to main: the lock -> try_lock change rode along in the I3C IRQ test stabilization commit but alters shared console behavior for every target, which does not belong in the I3C series. If ISR-context console writes need a deadlock-free path, that is its own change with its own review.
The ISR no longer fabricates a &mut over any thread-owned object. The per-bus registry now parks an IsrCtx (an ISR-side register handle plus the role flag) instead of a raw context pointer, and the service routine works exclusively through &self register methods, per-bus ISR_EVENTS atomics, and the global IBI rings. Master transfer completion follows the SMC flag-and-defer model: the ISR masks the completion sources and latches the status; the polling thread drains the response queue into its own transfer and re-enables the sources. Target-mode completions and the master-assigned dynamic address move into the per-bus event block; ISR-side halt/resume recovery (which needs the wait policy) is deferred to the thread as a fault flag. This dissolves the machinery the old design needed: the curr_xfer AtomicPtr handoff and its unsafe reconstruction, I3cXfer's completion flag, and the pinned 'static I3cCore (with its singleton! storage and Pin plumbing) are all gone. The controller is a plain owned value with zero unsafe; in-flight transfer overlap is structurally impossible because the transfer never leaves the thread. The controller borrows its I3cConfig (&'c mut) rather than owning it: the config embeds ~0.5 KiB of device tables, and the typestate transition moves the controller by value, so owning the config would transiently stack multiple copies and overflow the 2 KiB kernel bootstrap stack (observed as a silent hang during bring-up on the EVB). Borrowing keeps exactly one config alive in the caller's frame, the same footprint as the validated pre-rework layout, and matches the I2C driver's caller-owned-resource convention. Behavioral deltas from the reference ISR, all thread-visible only: SIR address validation moves from the ISR to the consumer (which already validates), and target error recovery runs at the next operation instead of inside the interrupt. Verified on a two-board AST1060 setup: the dual-device IBI exchange test passes 10/10 exchanges.
Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
Make init, halt, and reset_ctrl return Result instead of swallowing poll timeouts; start() releases the IRQ slot on init failure so a retry is not wedged on Busy. Harden priv_xfer_build_cmds: cap transfers at 16 (4-bit TID aliased larger batches) and reject over-long lengths (16-bit field truncated silently), both before consuming any buffer. Replace the unsafe raw-pointer reborrow with Option::take. Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
# Conflicts: # MODULE.bazel.lock
Set i2c_buses: &[] and handle init's new Result in the three i3c targets; the i2c commit added both but only updated i2c tests. Patch off pigweed's syscall_defs incompatibility gate. The roll makes kernel hard-depend on syscall_defs, which stays incompatible unless userspace_build is set, breaking every userspace=False system_image. syscall_defs is pure type defs the kernel always pulls in, so dropping the constraint changes no runtime behavior. Re-apply on next roll. Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
- Drain RX/IBI FIFO leftovers to prevent transfer misalignment - SETNEWDA now updates DAT slot + address book - Address bookkeeping: alloc marking, slot bounds, occupancy checks - Master error recovery preserves IBI queue (don't reset it) - validate_clock: fix overflow panic Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
attach_i2c_dev + i2c_write/read/write_read and an embedded-hal I2c impl route legacy devices through DAT slots by static address. Add SETMWL/SETMRL/GETMWL/GETMRL/GETMXDS, and latch target-side MWL/MRL updates from SLV_MAX_LEN. Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This PR aligns the AST10x0 I3C driver design with
https://github.com/rusty1968/pac-design-patterns.git.Pattern Alignment
Confined-unsafe MMIO facade
The design follows the confined-unsafe MMIO facade pattern by concentrating the
MMIO construction contract and PAC-facing register access inside the
I3cRegisterslayer.Cooperative-yield bounded-poll device
The design also follows the cooperative-yield bounded-poll pattern.
The hardware binding accepts a caller-supplied yield strategy, and polling paths
are explicitly bounded. As a result, wait behavior is portable across execution
models and timeout remains an observable, structured failure mode rather than an
unbounded hang.
Interrupt-model exceptions
There are two intentional structural exceptions worth calling out explicitly.
ISR-side aliasing
Interrupt handling requires an ISR-side register handle because the IRQ path
cannot directly borrow the thread-owned controller value. This alias is a narrow
exception made specifically for interrupt dispatch.
Global dispatch and event handoff
The IRQ path also relies on a bounded per-bus global dispatch registry and
atomic event handoff. This is an interrupt-integration mechanism, not a general
ownership model for the driver.
These exceptions are deliberate and localized. They do not change the primary
architecture, which remains a runtime-selected single-driver design.
Test Result