From 29e3fa7abda331ac9ad12b2b1ec2b59a7cdc249b Mon Sep 17 00:00:00 2001 From: JesseMelon Date: Thu, 30 Jul 2026 09:59:35 -0400 Subject: [PATCH 1/2] progress --- target/ast10x0/peripherals/BUILD.bazel | 1 + target/ast10x0/peripherals/i2c/constants.rs | 5 + target/ast10x0/peripherals/i2c/controller.rs | 8 ++ target/ast10x0/peripherals/i2c/dma.rs | 107 +++++++++++++++++++ target/ast10x0/peripherals/i2c/master.rs | 50 ++++----- target/ast10x0/peripherals/i2c/mod.rs | 1 + 6 files changed, 143 insertions(+), 29 deletions(-) create mode 100644 target/ast10x0/peripherals/i2c/dma.rs diff --git a/target/ast10x0/peripherals/BUILD.bazel b/target/ast10x0/peripherals/BUILD.bazel index 5e10fb2fa..394ccf29a 100644 --- a/target/ast10x0/peripherals/BUILD.bazel +++ b/target/ast10x0/peripherals/BUILD.bazel @@ -24,6 +24,7 @@ rust_library( "hace/registers.rs", "i2c/constants.rs", "i2c/controller.rs", + "i2c/dma.rs", "i2c/error.rs", "i2c/global.rs", "i2c/hal_impl.rs", diff --git a/target/ast10x0/peripherals/i2c/constants.rs b/target/ast10x0/peripherals/i2c/constants.rs index b2bd6b90f..146a5d56d 100644 --- a/target/ast10x0/peripherals/i2c/constants.rs +++ b/target/ast10x0/peripherals/i2c/constants.rs @@ -56,6 +56,11 @@ pub const I2C_BUF_SIZE: u8 = 0x20; /// Default timeout in microseconds pub const DEFAULT_TIMEOUT_US: u32 = 1_000_000; +/// Secondary timeout (loop iterations) for waiting on the master engine to +/// quiesce after a DMA transaction is aborted. Bounds the idle-poll in +/// `abort_master_dma` so a wedged controller cannot hang the caller. +pub const ABORT_TIMEOUT_US: u32 = 10_000; + /// Maximum retry attempts for operations pub const MAX_RETRY_ATTEMPTS: u32 = 3; diff --git a/target/ast10x0/peripherals/i2c/controller.rs b/target/ast10x0/peripherals/i2c/controller.rs index efdc58b30..9b107cd78 100644 --- a/target/ast10x0/peripherals/i2c/controller.rs +++ b/target/ast10x0/peripherals/i2c/controller.rs @@ -148,6 +148,14 @@ impl<'a, Y: FnMut(u32)> Ast1060I2c<'a, Y> { i2c } + /// Copy of the MMIO façade, for handing to short-lived helpers (e.g. the + /// [`super::dma::ArmedDma`] teardown guard) that must outlive an `&mut self` + /// borrow. Sound because the façade is `Copy` and access stays serialized. + #[inline] + pub(crate) fn mmio(&self) -> super::registers::Ast1060I2cRegisters { + self.mmio + } + /// I2C register block, via the MMIO façade (sole `unsafe` deref is inside /// [`Ast1060I2cRegisters`]). Driver-internal use. #[inline] diff --git a/target/ast10x0/peripherals/i2c/dma.rs b/target/ast10x0/peripherals/i2c/dma.rs new file mode 100644 index 000000000..e5652e2f7 --- /dev/null +++ b/target/ast10x0/peripherals/i2c/dma.rs @@ -0,0 +1,107 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! RAII teardown for in-flight master DMA transactions. +//! +//! A master DMA transfer arms the engine as an AHB bus master pointed at a +//! buffer in shared `.ram_nc`. If the transfer times out, returning an error +//! without stopping the engine leaves it free to keep writing into that buffer +//! (issue #359). The AST1060 has no master-only abort; the only teardown is a +//! controller soft-reset (datasheet §27.6.8). +//! +//! [`ArmedDma`] makes that teardown a property of the type system: constructing +//! it arms the engine, and dropping it without [`ArmedDma::commit`] soft-resets +//! the controller automatically — so no transfer error path can leave the +//! engine live. All DMA-lifecycle `unsafe` is confined to this module, holding +//! its own `Copy` of the [`Ast1060I2cRegisters`] façade. + +use super::constants; +use super::registers::Ast1060I2cRegisters; + +/// Guard for one armed master DMA transaction. +/// +/// Constructing an `ArmedDma` programs the DMA length + buffer-base registers +/// (the engine is now a potential AHB bus master). If the guard is dropped +/// without [`commit`](ArmedDma::commit), [`Drop`] soft-resets the controller +/// and waits for the engine to go idle. On the happy path the caller calls +/// [`commit`](ArmedDma::commit) and the drop is a no-op. +#[must_use = "drop tears down the DMA engine; bind it for the transfer's lifetime"] +pub(crate) struct ArmedDma { + mmio: Ast1060I2cRegisters, + committed: bool, +} + +impl ArmedDma { + /// Arm a TX DMA transaction: program i2cm1c (len-1) + i2cm30 (base addr). + pub(crate) fn arm_tx(mmio: Ast1060I2cRegisters, phy_addr: u32, len: usize) -> Self { + #[allow(clippy::cast_possible_truncation)] + mmio.i2c().i2cm1c().write(|w| unsafe { + w.dmatx_buf_len_byte() + .bits((len - 1) as u16) + .dmatx_buf_len_wr_enbl_for_cur_write_cmd() + .set_bit() + }); + mmio.i2c() + .i2cm30() + .write(|w| unsafe { w.sdramdmabuffer_base_addr().bits(phy_addr) }); + Self { + mmio, + committed: false, + } + } + + /// Arm an RX DMA transaction: program i2cm1c (len-1) + i2cm34 (base addr). + pub(crate) fn arm_rx(mmio: Ast1060I2cRegisters, phy_addr: u32, len: usize) -> Self { + #[allow(clippy::cast_possible_truncation)] + mmio.i2c().i2cm1c().modify(|_, w| unsafe { + w.dmarx_buf_len_byte() + .bits((len - 1) as u16) + .dmarx_buf_len_wr_enbl_for_cur_write_cmd() + .set_bit() + }); + mmio.i2c() + .i2cm34() + .modify(|_, w| unsafe { w.sdramdmabuffer_base_addr1().bits(phy_addr) }); + Self { + mmio, + committed: false, + } + } + + /// Transfer completed cleanly (STOP issued); no teardown needed. + pub(crate) fn commit(mut self) { + self.committed = true; + } +} + +impl Drop for ArmedDma { + fn drop(&mut self) { + if self.committed { + return; + } + // No master-only abort exists; soft-reset the controller (datasheet + // §27.6.8): clear I2CC00 function-control, then restore it. Timing in + // I2CC04 survives. Then spin until the engine reports idle, bounded so + // a wedged controller cannot hang the drop. + // + // This disables the slave function for the reset window, which is safe + // here: the i2c-server-runtime backend is master-only and never arms a + // concurrent slave on this controller. + let fun_ctrl = self.mmio.i2c().i2cc00().read().bits(); + unsafe { + self.mmio.i2c().i2cc00().write(|w| w.bits(0)); + self.mmio.i2c().i2cc00().write(|w| w.bits(fun_ctrl)); + } + + let mut timeout = constants::ABORT_TIMEOUT_US; + while timeout > 0 && self.mmio.i2c().i2cc08().read().bus_busy_status().bit() { + timeout = timeout.saturating_sub(1); + core::hint::spin_loop(); + } + + // Clear any latched interrupts from the aborted transaction. + unsafe { + self.mmio.i2c().i2cm14().write(|w| w.bits(0xffff_ffff)); + } + } +} diff --git a/target/ast10x0/peripherals/i2c/master.rs b/target/ast10x0/peripherals/i2c/master.rs index 1981631a7..8ffecc70d 100644 --- a/target/ast10x0/peripherals/i2c/master.rs +++ b/target/ast10x0/peripherals/i2c/master.rs @@ -28,7 +28,9 @@ //! - **i2cm14** (Interrupt Status Register): Read status, write-to-clear //! - Reference: `ast1060_i2c.rs:849-870` (`aspeed_i2c_master_irq`) -use super::{constants, controller::Ast1060I2c, error::I2cError, types::I2cXferMode}; +use super::{ + constants, controller::Ast1060I2c, dma::ArmedDma, error::I2cError, types::I2cXferMode, +}; impl Ast1060I2c<'_, Y> { /// Write bytes to an I2C device @@ -500,19 +502,9 @@ impl Ast1060I2c<'_, Y> { dma_buf.as_ptr() as u32 }; - // Set DMA TX length in i2cm1c (len - 1) - #[allow(clippy::cast_possible_truncation)] - self.regs().i2cm1c().write(|w| unsafe { - w.dmatx_buf_len_byte() - .bits((chunk_len - 1) as u16) - .dmatx_buf_len_wr_enbl_for_cur_write_cmd() - .set_bit() - }); - - // Set DMA TX buffer base address in i2cm30 - self.regs() - .i2cm30() - .write(|w| unsafe { w.sdramdmabuffer_base_addr().bits(phy_addr) }); + // Arm the DMA engine (i2cm1c length + i2cm30 base addr). The guard + // tears the engine down automatically if we return before commit. + let dma = ArmedDma::arm_tx(self.mmio(), phy_addr, chunk_len); self.clear_interrupts(0xffff_ffff); self.completion = false; @@ -532,7 +524,12 @@ impl Ast1060I2c<'_, Y> { self.regs().i2cm18().write(|w| unsafe { w.bits(cmd) }); - self.wait_completion(constants::DEFAULT_TIMEOUT_US)?; + match self.wait_completion(constants::DEFAULT_TIMEOUT_US) { + // STOP issued; the engine quiesced normally. + Ok(()) => dma.commit(), + // Timeout: `dma` drops here, soft-resetting the live engine. + Err(e) => return Err(e), + } let status = self.regs().i2cm14().read().bits(); if status & constants::AST_I2CM_PKT_ERROR != 0 { @@ -581,19 +578,9 @@ impl Ast1060I2c<'_, Y> { dma_buf.as_ptr() as u32 }; - // Set DMA RX length in i2cm1c (len - 1) - #[allow(clippy::cast_possible_truncation)] - self.regs().i2cm1c().modify(|_, w| unsafe { - w.dmarx_buf_len_byte() - .bits((chunk_len - 1) as u16) - .dmarx_buf_len_wr_enbl_for_cur_write_cmd() - .set_bit() - }); - - // Set DMA RX buffer base address in i2cm34 - self.regs() - .i2cm34() - .modify(|_, w| unsafe { w.sdramdmabuffer_base_addr1().bits(phy_addr) }); + // Arm the DMA engine (i2cm1c length + i2cm34 base addr). The guard + // tears the engine down automatically if we return before commit. + let dma = ArmedDma::arm_rx(self.mmio(), phy_addr, chunk_len); self.clear_interrupts(0xffff_ffff); self.completion = false; @@ -612,7 +599,12 @@ impl Ast1060I2c<'_, Y> { self.regs().i2cm18().write(|w| unsafe { w.bits(cmd) }); - self.wait_completion(constants::DEFAULT_TIMEOUT_US)?; + match self.wait_completion(constants::DEFAULT_TIMEOUT_US) { + // STOP issued; the engine quiesced normally. + Ok(()) => dma.commit(), + // Timeout: `dma` drops here, soft-resetting the live engine. + Err(e) => return Err(e), + } let status = self.regs().i2cm14().read().bits(); if status & constants::AST_I2CM_PKT_ERROR != 0 { diff --git a/target/ast10x0/peripherals/i2c/mod.rs b/target/ast10x0/peripherals/i2c/mod.rs index 8ffc2e591..a81bcd916 100644 --- a/target/ast10x0/peripherals/i2c/mod.rs +++ b/target/ast10x0/peripherals/i2c/mod.rs @@ -59,6 +59,7 @@ mod constants; mod controller; +mod dma; mod error; mod global; mod hal_impl; From 083a99754443cbba17adbaef83a61d862d5a912f Mon Sep 17 00:00:00 2001 From: JesseMelon Date: Fri, 14 Aug 2026 18:00:22 -0400 Subject: [PATCH 2/2] i2c: Type-state ArmedDma refactor + on-hw abort test Drop the runtime `committed` flag from ArmedDma; commit(self) now consumes the guard via mem::forget, so a committed transfer is no longer a droppable value and Drop tears down unconditionally. Add the two-image i2c_dma_abort hardware test (master DMA + clock-stretching slave) exercising the commit no-op path and the timeout -> guard-drop -> soft-reset teardown path. Co-Authored-By: Claude Opus 4 --- target/ast10x0/peripherals/i2c/dma.rs | 43 ++-- .../peripherals/i2c/i2c_dma_abort/BUILD.bazel | 148 +++++++++++++ .../i2c/i2c_dma_abort/master_system.json5 | 18 ++ .../i2c/i2c_dma_abort/master_target.rs | 203 ++++++++++++++++++ .../i2c/i2c_dma_abort/system.json5 | 18 ++ .../peripherals/i2c/i2c_dma_abort/target.rs | 145 +++++++++++++ 6 files changed, 553 insertions(+), 22 deletions(-) create mode 100644 target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/BUILD.bazel create mode 100644 target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/master_system.json5 create mode 100644 target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/master_target.rs create mode 100644 target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/system.json5 create mode 100644 target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/target.rs diff --git a/target/ast10x0/peripherals/i2c/dma.rs b/target/ast10x0/peripherals/i2c/dma.rs index e5652e2f7..bb26bbfac 100644 --- a/target/ast10x0/peripherals/i2c/dma.rs +++ b/target/ast10x0/peripherals/i2c/dma.rs @@ -10,10 +10,14 @@ //! controller soft-reset (datasheet §27.6.8). //! //! [`ArmedDma`] makes that teardown a property of the type system: constructing -//! it arms the engine, and dropping it without [`ArmedDma::commit`] soft-resets -//! the controller automatically — so no transfer error path can leave the -//! engine live. All DMA-lifecycle `unsafe` is confined to this module, holding -//! its own `Copy` of the [`Ast1060I2cRegisters`] façade. +//! it arms the engine, and its [`Drop`] soft-resets the controller +//! unconditionally. [`ArmedDma::commit`] *consumes* the guard (defusing the +//! teardown by forgetting it), so a committed transaction is no longer a +//! droppable `ArmedDma` — "committed" is the absence of the value, not a runtime +//! flag. Every error path that returns before commit therefore drops a live +//! guard and tears the engine down; there is no state in which the teardown can +//! be skipped by mistake. All DMA-lifecycle `unsafe` is confined to this module, +//! holding its own `Copy` of the [`Ast1060I2cRegisters`] façade. use super::constants; use super::registers::Ast1060I2cRegisters; @@ -21,14 +25,14 @@ use super::registers::Ast1060I2cRegisters; /// Guard for one armed master DMA transaction. /// /// Constructing an `ArmedDma` programs the DMA length + buffer-base registers -/// (the engine is now a potential AHB bus master). If the guard is dropped -/// without [`commit`](ArmedDma::commit), [`Drop`] soft-resets the controller -/// and waits for the engine to go idle. On the happy path the caller calls -/// [`commit`](ArmedDma::commit) and the drop is a no-op. +/// (the engine is now a potential AHB bus master). [`Drop`] soft-resets the +/// controller and waits for the engine to go idle. The happy path calls +/// [`commit`](ArmedDma::commit), which consumes the guard so its `Drop` never +/// runs — there is no "committed" flag, the committed state is simply the guard +/// no longer existing. #[must_use = "drop tears down the DMA engine; bind it for the transfer's lifetime"] pub(crate) struct ArmedDma { mmio: Ast1060I2cRegisters, - committed: bool, } impl ArmedDma { @@ -44,10 +48,7 @@ impl ArmedDma { mmio.i2c() .i2cm30() .write(|w| unsafe { w.sdramdmabuffer_base_addr().bits(phy_addr) }); - Self { - mmio, - committed: false, - } + Self { mmio } } /// Arm an RX DMA transaction: program i2cm1c (len-1) + i2cm34 (base addr). @@ -62,23 +63,21 @@ impl ArmedDma { mmio.i2c() .i2cm34() .modify(|_, w| unsafe { w.sdramdmabuffer_base_addr1().bits(phy_addr) }); - Self { - mmio, - committed: false, - } + Self { mmio } } /// Transfer completed cleanly (STOP issued); no teardown needed. - pub(crate) fn commit(mut self) { - self.committed = true; + pub(crate) fn commit(self) { + core::mem::forget(self); } } impl Drop for ArmedDma { fn drop(&mut self) { - if self.committed { - return; - } + // Reached only for an *uncommitted* guard: `commit` consumes and forgets + // the value, so a committed transaction never drops here. Teardown is + // therefore unconditional. + // // No master-only abort exists; soft-reset the controller (datasheet // §27.6.8): clear I2CC00 function-control, then restore it. Timing in // I2CC04 survives. Then spin until the engine reports idle, bounded so diff --git a/target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/BUILD.bazel b/target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/BUILD.bazel new file mode 100644 index 000000000..770fcb0ce --- /dev/null +++ b/target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/BUILD.bazel @@ -0,0 +1,148 @@ +# Licensed under the Apache-2.0 license +# SPDX-License-Identifier: Apache-2.0 + +load("@pigweed//pw_kernel/tooling:system_image.bzl", "system_image") +load("@pigweed//pw_kernel/tooling:target_codegen.bzl", "target_codegen") +load("@pigweed//pw_kernel/tooling:target_linker_script.bzl", "target_linker_script") +load("@pigweed//pw_kernel/tooling/panic_detector:rust_binary_no_panics_test.bzl", "rust_binary_no_panics_test") +load("@rules_rust//rust:defs.bzl", "rust_binary") +load("//target/ast10x0:defs.bzl", "TARGET_COMPATIBLE_WITH", "system_image_test") + +COMMON_DEPS = [ + "//target/ast10x0:config", + "//target/ast10x0:entry", + "//target/ast10x0/board:ast10x0_board", + "//target/ast10x0/peripherals", + "@ast1060_pac", + "@pigweed//pw_kernel/arch/arm_cortex_m:arch_arm_cortex_m", + "@pigweed//pw_kernel/kernel", + "@pigweed//pw_kernel/subsys/console:console_backend", + "@pigweed//pw_kernel/target:target_common", + "@pigweed//pw_log/rust:pw_log", +] + +# --------------------------------------------------------------------------- +# Master image (device A) — drives the DMA guard's commit and teardown paths. +# --------------------------------------------------------------------------- + +filegroup( + name = "master_system_config", + srcs = ["master_system.json5"], +) + +target_codegen( + name = "master_codegen", + arch = "@pigweed//pw_kernel/arch/arm_cortex_m:arch_arm_cortex_m", + system_config = ":master_system_config", + target_compatible_with = TARGET_COMPATIBLE_WITH, +) + +target_linker_script( + name = "master_linker_script", + system_config = ":master_system_config", + tags = ["kernel"], + target_compatible_with = TARGET_COMPATIBLE_WITH, + template = "//target/ast10x0:linker_script_template", +) + +rust_binary( + name = "master_target", + srcs = ["master_target.rs"], + aliases = {":master_codegen": "codegen"}, + edition = "2024", + tags = ["kernel"], + target_compatible_with = TARGET_COMPATIBLE_WITH, + deps = [ + ":master_codegen", + ":master_linker_script", + ] + COMMON_DEPS, +) + +system_image( + name = "master", + kernel = ":master_target", + platform = "//target/ast10x0", + system_config = ":master_system_config", + tags = ["kernel"], + userspace = False, +) + +system_image_test( + name = "i2c_dma_abort_test", + image = ":master", + slave_image = ":i2c_dma_abort", + tags = ["hardware"], + target_compatible_with = select({ + "//target/ast10x0:qemu_enabled": ["@platforms//:incompatible"], + "//conditions:default": [], + }), +) + +rust_binary_no_panics_test( + name = "no_panics_test", + binary = ":master", + tags = ["kernel"], +) + +# --------------------------------------------------------------------------- +# Stretcher slave image (device B) — serves one txn, then holds SCL low. +# --------------------------------------------------------------------------- + +filegroup( + name = "system_config", + srcs = ["system.json5"], +) + +target_codegen( + name = "codegen", + arch = "@pigweed//pw_kernel/arch/arm_cortex_m:arch_arm_cortex_m", + system_config = ":system_config", + target_compatible_with = TARGET_COMPATIBLE_WITH, +) + +target_linker_script( + name = "linker_script", + system_config = ":system_config", + tags = ["kernel"], + target_compatible_with = TARGET_COMPATIBLE_WITH, + template = "//target/ast10x0:linker_script_template", +) + +rust_binary( + name = "target", + srcs = ["target.rs"], + edition = "2024", + tags = ["kernel"], + target_compatible_with = TARGET_COMPATIBLE_WITH, + deps = [ + ":codegen", + ":linker_script", + "//hal/blocking", + "//target/ast10x0:config", + "//target/ast10x0:entry", + "//target/ast10x0/backend/i2c:i2c_backend_ast10x0", + "//target/ast10x0/board:ast10x0_board", + "//target/ast10x0/peripherals", + "@ast1060_pac", + "@pigweed//pw_kernel/arch/arm_cortex_m:arch_arm_cortex_m", + "@pigweed//pw_kernel/kernel", + "@pigweed//pw_kernel/subsys/console:console_backend", + "@pigweed//pw_kernel/target:target_common", + "@pigweed//pw_log/rust:pw_log", + ], +) + +system_image( + name = "i2c_dma_abort", + kernel = ":target", + platform = "//target/ast10x0", + system_config = ":system_config", + tags = ["kernel"], + userspace = False, +) + +rust_binary_no_panics_test( + name = "slave_no_panics_test", + binary = ":i2c_dma_abort", + tags = ["kernel"], +) diff --git a/target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/master_system.json5 b/target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/master_system.json5 new file mode 100644 index 000000000..e6eb06c83 --- /dev/null +++ b/target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/master_system.json5 @@ -0,0 +1,18 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +// I2C DMA-guard abort test — master side memory layout. +// Shared layout with the other two-image i2c tests. +{ + arch: { + type: "armv7m", + vector_table_start_address: 0x00000000, + vector_table_size_bytes: 1280, // 0x500 (320 vectors) + }, + kernel: { + flash_start_address: 0x00000500, + flash_size_bytes: 262144, // 256KB + ram_start_address: 0x00040500, + ram_size_bytes: 391936, // ends at RAM_NC boundary (0x000A0000) + }, +} diff --git a/target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/master_target.rs b/target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/master_target.rs new file mode 100644 index 000000000..7f0677c4d --- /dev/null +++ b/target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/master_target.rs @@ -0,0 +1,203 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! DMA-guard abort test — master side (device A). +//! +//! Exercises [`ArmedDma`](ast10x0_peripherals::i2c) black-box through the driver's +//! master DMA write path (I2C2, DMA mode). Two phases against device B on Bus 2: +//! +//! - **Phase 1 (commit / no-op):** a DMA `write()` to a responsive slave `0x42` +//! completes with `Ok`. The transfer's `wait_completion` returns cleanly, so the +//! guard is `commit()`ed and its teardown never runs — the happy path still works +//! and the engine is not spuriously reset. +//! - **Phase 2 (timeout → auto-teardown):** device B stops servicing and holds SCL +//! low. The DMA `write()` cannot complete, `wait_completion` times out, and the +//! uncommitted guard drops → controller soft-reset. We then read our own I2C2 +//! registers to prove the teardown ran: function-control was restored +//! (`i2cc00.enbl_master_fn` set) and latched interrupts cleared (`i2cm14 == 0`). +//! The call *returning at all* (Err, not a hang) demonstrates the bounded +//! busy-wait in the guard's Drop. +//! +//! Device B must be running its stretcher image before this image is loaded. + +#![no_std] +#![no_main] + +use ast10x0_board::{Ast10x0Board, Ast10x0BoardDescriptor}; +use ast10x0_peripherals::i2c::{ + Ast1060I2c, Ast1060I2cRegisters, ClockConfig, I2cConfig, I2cError, I2cSpeed, I2cXferMode, +}; +use ast10x0_peripherals::scu::pinctrl; +use codegen as _; +use console_backend::console_backend_write_all; +use entry as _; +use target_common::{declare_target, TargetInterface}; + +pub struct Target {} + +const SLAVE_ADDR: u8 = 0x42; +const PAYLOAD: &[u8] = &[0xDE, 0xAD, 0xBE, 0xEF]; + +/// Bounded retry budget for phase 1, absorbing device B's bring-up latency +/// (there is no explicit two-node handshake). Kept small so a genuine failure +/// surfaces quickly instead of burning the whole test timeout on retries. +const PHASE1_ATTEMPTS: u32 = 30; + +// Master TX staging buffer. A master DMA transfer points the engine (an AHB bus +// master) at this buffer, so it must live in non-cached SRAM the DMA engine and +// CPU observe coherently. The slave buffer is unused here but required by the +// DMA constructor. +#[unsafe(link_section = ".ram_nc")] +static mut MASTER_DMA_BUF: [u8; 4096] = [0u8; 4096]; +#[unsafe(link_section = ".ram_nc")] +static mut SLAVE_DMA_BUF: [u8; 256] = [0u8; 256]; + +fn i2c2_dma_config() -> I2cConfig { + I2cConfig { + xfer_mode: I2cXferMode::DmaMode, + speed: I2cSpeed::Standard, + multi_master: false, + smbus_timeout: false, + smbus_alert: false, + clock_config: ClockConfig::ast1060_default(), + } +} + +fn i2c_error_str(error: I2cError) -> &'static str { + match error { + I2cError::Overrun => "Overrun", + I2cError::NoAcknowledge => "NoAcknowledge", + I2cError::Timeout => "Timeout", + I2cError::BusRecoveryFailed => "BusRecoveryFailed", + I2cError::Bus => "Bus", + I2cError::Busy => "Busy", + I2cError::Invalid => "Invalid", + I2cError::Abnormal => "Abnormal", + I2cError::ArbitrationLoss => "ArbitrationLoss", + I2cError::SlaveError => "SlaveError", + I2cError::InvalidAddress => "InvalidAddress", + } +} + +/// Dump the master's I2C2 status registers for post-mortem diagnosis. +fn dump_master_regs(context: &str) { + // SAFETY: the test owns I2C2; read-only view of the same registers. + let regs = unsafe { &*ast1060_pac::I2c2::ptr() }; + pw_log::error!( + "{}: i2cc00=0x{:08x} i2cc08=0x{:08x} i2cm14=0x{:08x}", + context as &str, + regs.i2cc00().read().bits() as u32, + regs.i2cc08().read().bits() as u32, + regs.i2cm14().read().bits() as u32 + ); +} + +fn run_master() -> Result<(), &'static str> { + pw_log::info!("=== AST10x0 I2C DMA-guard abort test (master, Bus 2) ==="); + + let board = Ast10x0Board::new(Ast10x0BoardDescriptor { + pinctrl_groups: &[pinctrl::PINCTRL_I2C2], + i2c_buses: &[], + }); + // SAFETY: single call at boot with exclusive access to SCU/I2C global regs. + unsafe { board.init() }.map_err(|_| "board init failed")?; + + // SAFETY: I2C2 registers accessed only through `master` for this test. + let mmio = + unsafe { Ast1060I2cRegisters::new(ast1060_pac::I2c2::ptr(), ast1060_pac::I2cbuff2::ptr()) }; + // SAFETY: both buffers are non-cached SRAM statics uniquely owned by this + // driver for the test's lifetime. + let master_dma_buf: &'static mut [u8] = + unsafe { &mut *core::ptr::addr_of_mut!(MASTER_DMA_BUF) }; + let slave_dma_buf: &'static mut [u8] = unsafe { &mut *core::ptr::addr_of_mut!(SLAVE_DMA_BUF) }; + let mut master = Ast1060I2c::new_with_dma( + mmio, + &i2c2_dma_config(), + master_dma_buf, + slave_dma_buf, + |_| core::hint::spin_loop(), + ) + .map_err(|_| "I2C2 master DMA init failed")?; + + // -- Phase 1: commit / no-op path. Device B services exactly one write. -- + let mut attempts = PHASE1_ATTEMPTS; + loop { + match master.write(SLAVE_ADDR, PAYLOAD) { + Ok(()) => { + pw_log::info!("phase 1: committed DMA write OK (guard defused, no reset)"); + break; + } + Err(_) if attempts > 0 => { + attempts -= 1; + for _ in 0..10_000 { + core::hint::spin_loop(); + } + } + Err(e) => { + pw_log::error!("phase 1 DMA write failed: {}", i2c_error_str(e) as &str); + dump_master_regs("phase 1 failure"); + return Err("phase 1 commit path failed (device B not responding?)"); + } + } + } + + // -- Phase 2: timeout → auto-teardown. Device B now wedges, holding SCL. -- + // The DMA write cannot complete; wait_completion times out; the uncommitted + // ArmedDma drops and soft-resets the controller. Returning (not hanging) + // demonstrates the bounded busy-wait in the guard's Drop. + match master.write(SLAVE_ADDR, PAYLOAD) { + Err(I2cError::Timeout) => { + pw_log::info!("phase 2: DMA write timed out as expected (guard drop → teardown)"); + } + Err(other) => { + pw_log::error!( + "phase 2: expected Timeout, got {}", + i2c_error_str(other) as &str + ); + return Err("phase 2 did not time out (stretch recipe needs tuning on the rig)"); + } + Ok(()) => { + return Err("phase 2 unexpectedly succeeded (device B did not stall)"); + } + } + + // The guard's Drop soft-resets I2CC00 (clear → restore) and clears I2CM14. + // Read our own registers to confirm the teardown actually ran. + // SAFETY: the test owns I2C2; a read-only view of the same registers. + let regs = unsafe { &*ast1060_pac::I2c2::ptr() }; + if !regs.i2cc00().read().enbl_master_fn().bit() { + pw_log::error!( + "teardown check: i2cc00=0x{:08x} master-enable not restored", + regs.i2cc00().read().bits() as u32 + ); + return Err("teardown did not restore i2cc00 master-enable"); + } + let m14 = regs.i2cm14().read().bits(); + if m14 != 0 { + pw_log::error!("teardown check: i2cm14=0x{:08x} not cleared", m14 as u32); + return Err("teardown did not clear i2cm14 latched interrupts"); + } + pw_log::info!("phase 2: teardown verified (i2cc00 master-enable restored, i2cm14 clear)"); + + pw_log::info!("=== AST10x0 I2C DMA-guard abort test PASSED ==="); + Ok(()) +} + +impl TargetInterface for Target { + const NAME: &'static str = "AST10x0 I2C DMA Abort Master"; + + fn main() -> ! { + let sentinel: &[u8] = match run_master() { + Ok(()) => b"TEST_RESULT:PASS\n", + Err(e) => { + pw_log::error!("DMA abort master failed: {}", e as &str); + b"TEST_RESULT:FAIL\n" + } + }; + let _ = console_backend_write_all(sentinel); + #[expect(clippy::empty_loop)] + loop {} + } +} + +declare_target!(Target); diff --git a/target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/system.json5 b/target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/system.json5 new file mode 100644 index 000000000..1755e1b6c --- /dev/null +++ b/target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/system.json5 @@ -0,0 +1,18 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +// I2C DMA-guard abort test — stretcher slave memory layout. +// Shared layout with the other two-image i2c tests. +{ + arch: { + type: "armv7m", + vector_table_start_address: 0x00000000, + vector_table_size_bytes: 1280, // 0x500 (320 vectors) + }, + kernel: { + flash_start_address: 0x00000500, // After vector table + flash_size_bytes: 262144, // 256KB for kernel code (in RAM) + ram_start_address: 0x00040500, // RAM starts after code + ram_size_bytes: 391936, // ends at RAM_NC boundary (0x000A0000) + }, +} diff --git a/target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/target.rs b/target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/target.rs new file mode 100644 index 000000000..21ab25f0a --- /dev/null +++ b/target/ast10x0/tests/peripherals/i2c/i2c_dma_abort/target.rs @@ -0,0 +1,145 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! DMA-guard abort test — stretcher slave (device B). +//! +//! Provides the two bus states the master (device A) needs: +//! +//! 1. **Serviceable:** serves exactly one slave RX transaction so the master's +//! phase-1 DMA write completes with `Ok` (the guard-commit path). This uses +//! the same buffer-mode slave recipe as `i2c_slave_rx`, which is known to work +//! on the bench rig. +//! 2. **Wedged:** after that one transaction it stops polling / re-arming. The +//! next master write matches this address, and with no armed RX buffer command +//! the slave holds SCL low (clock stretch) until firmware re-arms — which it +//! never does. That sustained stretch makes the master's DMA `wait_completion` +//! time out, the trigger for the `ArmedDma` teardown under test. +//! +//! If a future rig shows the wedge NAKs instead of stretching (master would +//! report `NoAcknowledge`, not `Timeout`), the fallback is a fixture GPIO holding +//! SCL low — see this test's README. + +#![no_std] +#![no_main] + +use ast10x0_board::{Ast10x0Board, Ast10x0BoardDescriptor, I2cBusCfg}; +use ast10x0_peripherals::i2c::{ClockConfig, I2cConfig, I2cError, I2cSpeed, I2cXferMode}; +use ast10x0_peripherals::scu::pinctrl; +use codegen as _; +use console_backend::console_backend_write_all; +use entry as _; +use openprot_hal_blocking::i2c_hardware::slave::{I2cSlaveBuffer, I2cSlaveCore}; +use target_common::{declare_target, TargetInterface}; + +pub struct Target {} + +const SLAVE_ADDR: u8 = 0x42; + +/// Bus 2 config: standard-speed buffer mode (matches the working i2c_slave_rx). +const SLAVE_CFG: I2cConfig = I2cConfig { + speed: I2cSpeed::Standard, + xfer_mode: I2cXferMode::BufferMode, + multi_master: false, + smbus_timeout: false, + smbus_alert: false, + clock_config: ClockConfig::ast1060_default(), +}; + +fn i2c_error_str(error: I2cError) -> &'static str { + match error { + I2cError::Overrun => "Overrun", + I2cError::NoAcknowledge => "NoAcknowledge", + I2cError::Timeout => "Timeout", + I2cError::BusRecoveryFailed => "BusRecoveryFailed", + I2cError::Bus => "Bus", + I2cError::Busy => "Busy", + I2cError::Invalid => "Invalid", + I2cError::Abnormal => "Abnormal", + I2cError::ArbitrationLoss => "ArbitrationLoss", + I2cError::SlaveError => "SlaveError", + I2cError::InvalidAddress => "InvalidAddress", + } +} + +/// Bring up the Bus 2 slave and serve exactly one RX transaction. Returns the +/// live driver so the caller can hold it (keeping slave mode armed) while it +/// stops polling — that non-servicing state is what stretches SCL for phase 2. +fn setup_and_serve_one() -> Result { + pw_log::info!("=== AST10x0 I2C DMA-guard abort stretcher (Bus 2, addr 0x42) ==="); + + let board = Ast10x0Board::new(Ast10x0BoardDescriptor { + pinctrl_groups: &[pinctrl::PINCTRL_I2C2], + i2c_buses: &[I2cBusCfg { + bus: 2, + config: SLAVE_CFG, + }], + }); + // SAFETY: single call at boot with exclusive access to the board. + unsafe { board.init() }.map_err(|_| "board init failed")?; + + // SAFETY: board.init() ran init_bus(2); we are the sole owner of Bus 2. + let mut driver = unsafe { i2c_backend::open_bus(2, &SLAVE_CFG) }.map_err(|e| { + pw_log::error!("open_bus failed: {}", i2c_error_str(e) as &str); + "open_bus failed" + })?; + + driver + .configure_slave_address(SLAVE_ADDR) + .map_err(|_| "configure_slave_address failed")?; + driver + .enable_slave_mode() + .map_err(|_| "enable_slave_mode failed")?; + + pw_log::info!("stretcher ready — serving one transaction, then wedging"); + + // Serve exactly ONE transaction (drives the master's phase 1 to Ok). + // + // We consume the packet-done interrupt via `poll_slave_data` but deliberately + // do NOT call `read_slave_buffer`: draining re-arms the RX buffer command + // (slave.rs `slave_read`), which would let the master's phase-2 write complete + // cleanly. The `SLAVE_MATCH | RX_DONE | STOP` packet-done branch that this + // transaction hits does not re-arm on its own, so once we stop here the next + // master write finds no armed RX command and the slave stretches SCL — the + // phase-2 stall the guard-teardown test needs. + loop { + match driver.poll_slave_data() { + Ok(Some(_n)) => break, + Ok(None) => core::hint::spin_loop(), + Err(e) => { + pw_log::error!("poll_slave_data error: {}", i2c_error_str(e) as &str); + return Err("poll_slave_data failed"); + } + } + } + + pw_log::info!("stretcher: served one txn (RX left un-rearmed); now wedged"); + Ok(driver) +} + +impl TargetInterface for Target { + const NAME: &'static str = "AST10x0 I2C DMA Abort Stretcher"; + + fn main() -> ! { + match setup_and_serve_one() { + Ok(driver) => { + let _ = console_backend_write_all(b"TEST_RESULT:PASS\n"); + // Hold the driver so slave mode stays configured, and STOP + // polling: the next master write matches but finds no re-armed + // RX command, so the slave stretches SCL (phase 2 stall) until + // A gives up. + let _held = driver; + loop { + core::hint::spin_loop(); + } + } + Err(e) => { + pw_log::error!("stretcher setup failed: {}", e as &str); + let _ = console_backend_write_all(b"TEST_RESULT:FAIL\n"); + #[expect(clippy::empty_loop)] + loop {} + } + } + } +} + +declare_target!(Target);