From 21f556b07b9cdf854483ea0478269a3c25499436 Mon Sep 17 00:00:00 2001 From: Anthony Rocha Date: Mon, 8 Jun 2026 16:19:33 -0700 Subject: [PATCH 1/3] Add PFR I2C mailbox bringup via IPC --- target/ast10x0/pfr/BUILD.bazel | 4 + target/ast10x0/pfr/lib.rs | 8 +- target/ast10x0/pfr/smbus_mailbox_client.rs | 193 ++++++++++++++++++ target/ast10x0/pfr/swmbx_ctrl.rs | 19 +- target/ast10x0/tests/pfr/BUILD.bazel | 118 +++++++++++ .../tests/pfr/pfr_i2c_notify_client_main.rs | 190 +++++++++++++++++ target/ast10x0/tests/pfr/server_main.rs | 56 +++++ target/ast10x0/tests/pfr/system.json5 | 78 +++++++ target/ast10x0/tests/pfr/target.rs | 52 +++++ 9 files changed, 708 insertions(+), 10 deletions(-) create mode 100644 target/ast10x0/pfr/smbus_mailbox_client.rs create mode 100644 target/ast10x0/tests/pfr/BUILD.bazel create mode 100644 target/ast10x0/tests/pfr/pfr_i2c_notify_client_main.rs create mode 100644 target/ast10x0/tests/pfr/server_main.rs create mode 100644 target/ast10x0/tests/pfr/system.json5 create mode 100644 target/ast10x0/tests/pfr/target.rs diff --git a/target/ast10x0/pfr/BUILD.bazel b/target/ast10x0/pfr/BUILD.bazel index da3f6c85..b725d1ec 100644 --- a/target/ast10x0/pfr/BUILD.bazel +++ b/target/ast10x0/pfr/BUILD.bazel @@ -11,12 +11,15 @@ rust_library( srcs = [ "lib.rs", "swmbx_ctrl.rs", + "smbus_mailbox_client.rs", ], crate_name = "ast10x0_pfr", crate_root = "lib.rs", edition = "2024", target_compatible_with = TARGET_COMPATIBLE_WITH, deps = [ + "//services/i2c/api:i2c_api", + "//services/i2c/client:i2c_client", "@rust_crates//:heapless", ], ) @@ -33,5 +36,6 @@ rust_test( ], deps = [ "@rust_crates//:heapless", + ], ) diff --git a/target/ast10x0/pfr/lib.rs b/target/ast10x0/pfr/lib.rs index 158f23c5..cac818af 100644 --- a/target/ast10x0/pfr/lib.rs +++ b/target/ast10x0/pfr/lib.rs @@ -3,6 +3,12 @@ #![no_std] +pub mod smbus_mailbox_client; pub mod swmbx_ctrl; -pub use swmbx_ctrl::{SwmbxCtrl, SwmbxError}; +pub use smbus_mailbox_client::{I2cPfrClientError, I2cPfrSmbusClient, Source, SourceAddressMap}; +pub use swmbx_ctrl::{ + SwmbxCtrl, SwmbxError, SWMBX_BUF_BASE, SWMBX_DEV_COUNT, SWMBX_FIFO, SWMBX_FIFO_COUNT, + SWMBX_FIFO_DEPTH, SWMBX_FIFO_NOTIFY_START, SWMBX_FIFO_NOTIFY_STOP, + SWMBX_NODE_COUNT, SWMBX_NOTIFY, SWMBX_PROTECT, +}; diff --git a/target/ast10x0/pfr/smbus_mailbox_client.rs b/target/ast10x0/pfr/smbus_mailbox_client.rs new file mode 100644 index 00000000..25ba25f5 --- /dev/null +++ b/target/ast10x0/pfr/smbus_mailbox_client.rs @@ -0,0 +1,193 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +use crate::swmbx_ctrl::{SwmbxCtrl, SwmbxError}; +use i2c_api::seam::SevenBitAddress; +use i2c_api::{SlaveEvent, Transport}; +use i2c_client::{ClientError as I2cClientError, I2cClient}; + +const RX_BUFFER_SIZE: usize = 256; + +/// Logical source port for mailbox transactions. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum Source { + /// Baseboard management controller source. + Bmc, + /// Host CPU/PCH source. + PchCpu, +} + +/// Source-address mapping for writes received over the i2c service slave path. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub struct SourceAddressMap { + /// Seven-bit source address used to identify BMC-originated writes. + pub bmc: SevenBitAddress, + /// Seven-bit source address used to identify PCH/CPU-originated writes. + pub pch_cpu: SevenBitAddress, +} + +/// Errors returned by the i2c-service-backed SMBus client. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum I2cPfrClientError { + /// Underlying I2C client/service failure. + I2c(I2cClientError), + /// SW mailbox controller operation failure. + Swmbx(SwmbxError), + /// Source address did not match any configured source mapping. + UnknownSource, +} + +impl From for I2cPfrClientError { + fn from(value: I2cClientError) -> Self { + Self::I2c(value) + } +} + +impl From for I2cPfrClientError { + fn from(value: SwmbxError) -> Self { + Self::Swmbx(value) + } +} + +/// Concrete client that consumes i2c service slave notifications and applies +/// mailbox transactions to the SWMBX controller. +pub struct I2cPfrSmbusClient { + i2c: I2cClient, + swmbx: SwmbxCtrl, + sources: SourceAddressMap, + read_cursor: u8, + active_port: usize, + first_write: bool, +} + +impl I2cPfrSmbusClient { + /// Creates a new i2c-backed PFR SMBus client. + pub fn new(i2c: I2cClient, swmbx: SwmbxCtrl, sources: SourceAddressMap) -> Self { + Self { + i2c, + swmbx, + sources, + read_cursor: 0, + active_port: 0, + first_write: true, + } + } + + /// Returns a mutable handle to the underlying SWMBX controller. + pub fn controller_mut(&mut self) -> &mut SwmbxCtrl { + &mut self.swmbx + } + + /// Configures and enables target mode for the given slave address. + pub fn start(&mut self, slave_address: SevenBitAddress) -> Result<(), I2cPfrClientError> { + self.i2c.configure_slave(slave_address)?; + self.i2c.enable_slave()?; + self.i2c.enable_notification()?; + Ok(()) + } + + /// Disables notifications and leaves target mode. + pub fn stop(&mut self) -> Result<(), I2cPfrClientError> { + self.i2c.disable_notification()?; + self.i2c.disable_slave()?; + Ok(()) + } + + /// Returns one byte from the software mailbox controller buffer path. + pub fn mailbox_byte(&mut self, addr: u8) -> Result { + Ok(self.swmbx.swmbx_read(false, addr)?) + } + + /// Processes at most one pending I2C slave event. + /// + /// This mirrors the Zephyr SWMBX target callback flow: + /// + /// - `DataReceived`: consumes received bytes through `handle_data_received`. + /// The first byte is treated as mailbox offset and opens a transaction + /// with `send_start`; following bytes are written via `send_msg` with + /// wrapping cursor semantics. + /// - `ReadRequest`: reads one byte from the active port/cursor via + /// `get_msg`, publishes it with `slave_set_response`, then advances the + /// cursor. + /// - `Stop`: finalizes the transaction via `send_stop` and resets + /// first-write state. + /// + /// If no data is pending, this returns `Ok(())`. + /// + /// # Errors + /// Returns `I2cPfrClientError::I2c` for transport failures and propagates + /// `Swmbx`/source-resolution errors encountered while handling an event. + pub fn process_one_event(&mut self) -> Result<(), I2cPfrClientError> { + let mut rx = [0u8; RX_BUFFER_SIZE]; + let event = match self.i2c.slave_receive(&mut rx) { + Ok(ev) => ev, + Err(I2cClientError::ServerError(i2c_api::I2cError::NoData)) => return Ok(()), + Err(e) => return Err(I2cPfrClientError::I2c(e)), + }; + + match event.kind { + SlaveEvent::DataReceived => { + self.handle_data_received(&rx[..event.data_len], event.source_address) + } + SlaveEvent::ReadRequest => self.handle_read_request(), + SlaveEvent::Stop => { + self.swmbx.send_stop(self.active_port)?; + // The next write transaction starts with an offset byte. + self.first_write = true; + Ok(()) + } + _ => Ok(()), + } + } + + fn resolve_source(&self, source_addr: Option) -> Result { + match source_addr { + Some(addr) if addr == self.sources.bmc => Ok(Source::Bmc), + Some(addr) if addr == self.sources.pch_cpu => Ok(Source::PchCpu), + Some(_) => Err(I2cPfrClientError::UnknownSource), + None => Err(I2cPfrClientError::UnknownSource), + } + } + + fn handle_read_request(&mut self) -> Result<(), I2cPfrClientError> { + let value = self.swmbx.get_msg(self.active_port, self.read_cursor)?; + self.i2c.slave_set_response(&[value])?; + self.read_cursor = self.read_cursor.wrapping_add(1); + Ok(()) + } + + fn handle_data_received( + &mut self, + data: &[u8], + source_addr: Option, + ) -> Result<(), I2cPfrClientError> { + if data.is_empty() { + return Ok(()); + } + + let source = self.resolve_source(source_addr)?; + self.active_port = source_to_port(source); + + // Mirrors Zephyr swmbx_target callback sequencing: + // first byte selects address and opens transaction; following bytes write. + for byte in data { + if self.first_write { + self.read_cursor = *byte; + self.swmbx.send_start(self.active_port, self.read_cursor)?; + self.first_write = false; + } else { + self.swmbx.send_msg(self.active_port, self.read_cursor, *byte)?; + self.read_cursor = self.read_cursor.wrapping_add(1); + } + } + + Ok(()) + } +} + +fn source_to_port(source: Source) -> usize { + match source { + Source::Bmc => 0, + Source::PchCpu => 1, + } +} diff --git a/target/ast10x0/pfr/swmbx_ctrl.rs b/target/ast10x0/pfr/swmbx_ctrl.rs index af633f1d..b5bb87dc 100644 --- a/target/ast10x0/pfr/swmbx_ctrl.rs +++ b/target/ast10x0/pfr/swmbx_ctrl.rs @@ -5,10 +5,11 @@ use core::ptr::NonNull; use core::ptr::{read_volatile, write_volatile}; use heapless::spsc::Queue; -const SWMBX_DEV_COUNT: usize = 2; -const SWMBX_NODE_COUNT: usize = 256; -const SWMBX_FIFO_COUNT: usize = 4; -const SWMBX_FIFO_DEPTH: usize = 256; +pub const SWMBX_DEV_COUNT: usize = 2; +pub const SWMBX_NODE_COUNT: usize = 256; +pub const SWMBX_FIFO_COUNT: usize = 4; +pub const SWMBX_FIFO_DEPTH: usize = 256; +pub const SWMBX_BUF_BASE: usize = 0x7e7b_0e00; const _: () = assert!(SWMBX_NODE_COUNT == 256); const _: () = assert!(SWMBX_NODE_COUNT == (u8::MAX as usize) + 1); @@ -20,13 +21,13 @@ const PROTECT_WORD_SHIFT: u32 = PROTECT_BITS_PER_WORD.ilog2(); /// Count of 32-bit words needed to cover every node's protection bit. const PROTECT_WORD_COUNT: usize = SWMBX_NODE_COUNT / PROTECT_BITS_PER_WORD; -const SWMBX_PROTECT: u8 = 1 << 0; -const SWMBX_NOTIFY: u8 = 1 << 1; -const SWMBX_FIFO: u8 = 1 << 2; +pub const SWMBX_PROTECT: u8 = 1 << 0; +pub const SWMBX_NOTIFY: u8 = 1 << 1; +pub const SWMBX_FIFO: u8 = 1 << 2; const FLAG_MASK: u8 = SWMBX_PROTECT | SWMBX_NOTIFY | SWMBX_FIFO; -const SWMBX_FIFO_NOTIFY_START: u8 = 1 << 0; -const SWMBX_FIFO_NOTIFY_STOP: u8 = 1 << 1; +pub const SWMBX_FIFO_NOTIFY_START: u8 = 1 << 0; +pub const SWMBX_FIFO_NOTIFY_STOP: u8 = 1 << 1; /// Error type for SW mailbox controller operations. #[derive(Copy, Clone, Debug, Eq, PartialEq)] diff --git a/target/ast10x0/tests/pfr/BUILD.bazel b/target/ast10x0/tests/pfr/BUILD.bazel new file mode 100644 index 00000000..5dc012fb --- /dev/null +++ b/target/ast10x0/tests/pfr/BUILD.bazel @@ -0,0 +1,118 @@ +# Licensed under the Apache-2.0 license +# SPDX-License-Identifier: Apache-2.0 + +load("@pigweed//pw_kernel/tooling:rust_app.bzl", "rust_app") +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") + +package(default_visibility = ["//visibility:public"]) + +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", + "//target/ast10x0:entry", + "//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_kernel/userspace", + "@pigweed//pw_log/rust:pw_log", + ], +) + +rust_app( + name = "i2c_server", + srcs = ["server_main.rs"], + codegen_crate_name = "app_i2c_server", + edition = "2024", + system_config = ":system_config", + tags = ["kernel"], + target_compatible_with = TARGET_COMPATIBLE_WITH, + deps = [ + "//services/i2c/server-runtime:i2c_server_runtime", + "//target/ast10x0/backend/i2c:i2c_backend_ast10x0", + "//target/ast10x0/peripherals", + "@pigweed//pw_kernel/userspace", + "@pigweed//pw_log/rust:pw_log", + ], +) + +rust_app( + name = "pfr_i2c_notify_client", + srcs = ["pfr_i2c_notify_client_main.rs"], + codegen_crate_name = "app_pfr_i2c_notify_client", + edition = "2024", + system_config = ":system_config", + tags = ["kernel"], + target_compatible_with = TARGET_COMPATIBLE_WITH, + deps = [ + "//services/i2c/api:i2c_api", + "//services/i2c/client:i2c_client", + "//services/i2c/client-ipc:i2c_client_ipc", + "//target/ast10x0/pfr:pfr", + "@pigweed//pw_kernel/userspace", + "@pigweed//pw_log/rust:pw_log", + "@pigweed//pw_status/rust:pw_status", + ], +) + +system_image( + name = "pfr_i2c_notify_image", + apps = [ + ":i2c_server", + ":pfr_i2c_notify_client", + ], + kernel = ":target", + platform = "//target/ast10x0", + system_config = ":system_config", + tags = ["kernel"], + target_compatible_with = TARGET_COMPATIBLE_WITH, +) + +system_image_test( + name = "pfr_i2c_notify_test", + image = ":pfr_i2c_notify_image", + tags = ["hardware"], + target_compatible_with = select({ + "//target/ast10x0:qemu_enabled": ["@platforms//:incompatible"], + "//conditions:default": [], + }), +) + +rust_binary_no_panics_test( + name = "no_panics_test", + binary = ":pfr_i2c_notify_image", + tags = ["kernel"], +) diff --git a/target/ast10x0/tests/pfr/pfr_i2c_notify_client_main.rs b/target/ast10x0/tests/pfr/pfr_i2c_notify_client_main.rs new file mode 100644 index 00000000..f0a401ca --- /dev/null +++ b/target/ast10x0/tests/pfr/pfr_i2c_notify_client_main.rs @@ -0,0 +1,190 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! Bringup client app for PFR mailbox-over-I2C validation. +//! +//! This app wires the userspace I2C IPC client to the in-process SWMBX +//! controller bridge (`I2cPfrSmbusClient`) and runs the mailbox target flow +//! used by PFR bringup. +//! +//! Startup sequence: +//! 1. Bind to the I2C server channel (`handle::I2C`). +//! 2. Construct SWMBX over the platform mailbox MMIO region +//! (`SWMBX_BUF_BASE`, `SWMBX_NODE_COUNT`). +//! 3. Configure SWMBX behavior bits (PROTECT/NOTIFY/FIFO), FIFO mappings, and +//! per-port protection bitmaps. +//! 4. Enter I2C target mode at `SLAVE_ADDR` and enable notifications. +//! 5. Emit `MAILBOX_READY` to provide an explicit external readiness marker. +//! +//! Runtime behavior: +//! - Receives slave notifications from the I2C server as `Signals::USER`. +//! - Blocks on `Signals::USER` from the server channel. +//! - Drains one pending slave event per wake via `process_one_event()`. +//! - Drops unknown-source events with rate-limited warnings. +//! - Fails fast on setup/runtime errors that indicate bringup misconfiguration. + +#![no_main] +#![no_std] + +use app_pfr_i2c_notify_client::handle; +use ast10x0_pfr::{ + I2cPfrClientError, I2cPfrSmbusClient, SourceAddressMap, SwmbxCtrl, + SWMBX_BUF_BASE, SWMBX_NODE_COUNT, + SWMBX_FIFO, SWMBX_FIFO_NOTIFY_STOP, SWMBX_NOTIFY, SWMBX_PROTECT, +}; +use i2c_client::I2cClient; +use i2c_client_ipc::IpcTransport; +use userspace::entry; +use userspace::syscall::{self, Signals}; +use userspace::time::Instant; + +const SLAVE_ADDR: u8 = 0x38; +const BMC_SOURCE_ADDR: u8 = 0x20; +const PCH_SOURCE_ADDR: u8 = 0x22; + +const UFM_WRITE_FIFO: u8 = 0x0d; +const UFM_READ_FIFO: u8 = 0x0e; +const SWMBX_WRITE_FIFO_SIZE: usize = 64; +const SWMBX_READ_FIFO_SIZE: usize = 128; +const BMC_UPDATE_INTENT: u8 = 0x13; + +macro_rules! fail { + ($msg:literal) => {{ + pw_log::error!($msg); + loop {} + }}; +} + +#[entry] +fn entry() { + let i2c = I2cClient::new(IpcTransport::new(handle::I2C)); + // SAFETY: SWMBX_BUF_BASE is the platform-defined SWMBX mailbox region. + let swmbx = unsafe { SwmbxCtrl::new_with_regions(SWMBX_NODE_COUNT, SWMBX_BUF_BASE) }; + let sources = SourceAddressMap { + bmc: BMC_SOURCE_ADDR, + pch_cpu: PCH_SOURCE_ADDR, + }; + let mut mailbox = I2cPfrSmbusClient::new(i2c, swmbx, sources); + + // Enable SWMBX policy engines used by the mailbox flow. + // PROTECT enforces access masks, NOTIFY latches mailbox events, FIFO enables + // FIFO-backed register semantics for selected addresses. + if mailbox + .controller_mut() + .enable_behavior(SWMBX_PROTECT | SWMBX_NOTIFY | SWMBX_FIFO, true) + .is_err() + { + fail!("enable_behavior failed"); + } + // Configure write FIFO (0x0d): host writes are queued and notify on STOP. + if mailbox + .controller_mut() + .update_fifo( + 0, + UFM_WRITE_FIFO, + SWMBX_WRITE_FIFO_SIZE, + SWMBX_FIFO_NOTIFY_STOP, + true, + ) + .is_err() + { + fail!("update_fifo write failed"); + } + // Configure read FIFO (0x0e): host reads drain queued data and notify on STOP. + if mailbox + .controller_mut() + .update_fifo( + 1, + UFM_READ_FIFO, + SWMBX_READ_FIFO_SIZE, + SWMBX_FIFO_NOTIFY_STOP, + true, + ) + .is_err() + { + fail!("update_fifo read failed"); + } + // Enable notification on update-intent mailbox register for BMC port. + if mailbox + .controller_mut() + .update_notify(0, BMC_UPDATE_INTENT, true) + .is_err() + { + fail!("update_notify failed"); + } + + let bmc_access_control: [u32; 8] = [ + 0xfff704ff, + 0xffffffff, + 0xffffffff, + 0xfffffff2, + 0xffffffff, + 0xffffffff, + 0x00000000, + 0x00000000, + ]; + let pch_access_control: [u32; 8] = [ + 0xfff884ff, + 0xffffffff, + 0xffffffff, + 0xfffffff5, + 0x00000000, + 0x00000000, + 0xffffffff, + 0xffffffff, + ]; + // Apply per-port protection bitmaps across the mailbox address space. + // A set bit marks an address protected for that port. + if mailbox + .controller_mut() + .apply_protect(0, &bmc_access_control, 0) + .is_err() + { + fail!("apply_protect BMC failed"); + } + if mailbox + .controller_mut() + .apply_protect(1, &pch_access_control, 0) + .is_err() + { + fail!("apply_protect PCH failed"); + } + + if mailbox.start(SLAVE_ADDR).is_err() { + fail!("mailbox start failed"); + } + + // Explicit readiness marker for external orchestration. + pw_log::info!("MAILBOX_READY addr=0x{:02x}", SLAVE_ADDR as u32); + + pw_log::info!( + "PFR bringup listening addr=0x{:02x}; waiting for target notifications", + SLAVE_ADDR as u32, + ); + + let mut unknown_source_events: u32 = 0; + loop { + if syscall::object_wait(handle::I2C, Signals::USER, Instant::MAX).is_err() { + fail!("object_wait USER failed"); + } + + match mailbox.process_one_event() { + Ok(()) => {} + Err(I2cPfrClientError::UnknownSource) => { + unknown_source_events = unknown_source_events.wrapping_add(1); + if unknown_source_events == 1 || (unknown_source_events % 32) == 0 { + pw_log::warn!( + "dropping event with unknown source (count={})", + unknown_source_events as u32 + ); + } + } + Err(_) => fail!("process_one_event failed"), + } + } +} + +#[panic_handler] +fn panic(_info: &core::panic::PanicInfo) -> ! { + loop {} +} diff --git a/target/ast10x0/tests/pfr/server_main.rs b/target/ast10x0/tests/pfr/server_main.rs new file mode 100644 index 00000000..62f09848 --- /dev/null +++ b/target/ast10x0/tests/pfr/server_main.rs @@ -0,0 +1,56 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +#![no_main] +#![no_std] + +use app_i2c_server::{handle, signals}; +use ast10x0_peripherals::i2c::{ClockConfig, I2cConfig, I2cSpeed, I2cXferMode}; +use i2c_server_runtime::{run, Bus}; +use userspace::entry; + +const SLAVE_CFG: I2cConfig = I2cConfig { + speed: I2cSpeed::Standard, + xfer_mode: I2cXferMode::DmaMode, + multi_master: false, + smbus_timeout: false, + smbus_alert: false, + clock_config: ClockConfig::ast1060_default(), +}; + +#[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]; + +#[entry] +fn entry() { + // SAFETY: server process exclusively owns Bus 2. + if unsafe { i2c_backend::init_bus(2, &SLAVE_CFG) }.is_err() { + pw_log::error!("init_bus(2) failed"); + loop {} + } + + // SAFETY: buffers are non-cached SRAM and uniquely owned for bus 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 driver = + match unsafe { i2c_backend::open_bus_dma(2, &SLAVE_CFG, master_dma_buf, slave_dma_buf) } { + Ok(d) => d, + Err(_) => { + pw_log::error!("open_bus_dma(2) failed"); + loop {} + } + }; + + pw_log::info!("I2C server ready on Bus 2"); + + let mut buses = [Bus::new(handle::I2C, handle::I2C2_IRQ, driver)]; + run(handle::WG, signals::I2C2, &mut buses); +} + +#[panic_handler] +fn panic(_info: &core::panic::PanicInfo) -> ! { + loop {} +} diff --git a/target/ast10x0/tests/pfr/system.json5 b/target/ast10x0/tests/pfr/system.json5 new file mode 100644 index 00000000..5daeb1b8 --- /dev/null +++ b/target/ast10x0/tests/pfr/system.json5 @@ -0,0 +1,78 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +// PFR I2C notification bringup image. +// +// Apps: +// i2c_server - owns I2C2 hardware and raises notifications. +// pfr_i2c_notify_client - IPC client that waits for target notifications. +{ + arch: { + type: "armv7m", + vector_table_start_address: 0x00000000, + vector_table_size_bytes: 2048, + }, + kernel: { + flash_start_address: 0x00000800, + flash_size_bytes: 129024, + ram_start_address: 0x00060000, + ram_size_bytes: 131072, + }, + apps: [ + { + name: "i2c_server", + flash_size_bytes: 131072, + processes: [ + { + name: "i2c_server_process", + ram_size_bytes: 65536, + objects: [ + { name: "wg", type: "wait_group" }, + { name: "i2c", type: "channel_handler" }, + { + name: "i2c2_irq", + type: "interrupt", + irqs: [{ name: "i2c2", number: 112 }], + }, + { + type: "thread", + name: "i2c_server_thread", + kernel_stack_size_bytes: 4096, + }, + ], + memory_mappings: [ + { + name: "i2c_regs", + type: "device", + start_address: 0x7e7b0000, + size_bytes: 0x4000, + }, + ], + }, + ], + }, + { + name: "pfr_i2c_notify_client", + flash_size_bytes: 131072, + processes: [ + { + name: "pfr_i2c_notify_client_process", + ram_size_bytes: 32768, + objects: [ + { + name: "i2c", + type: "channel_initiator", + handler_process: "i2c_server_process", + handler_object_name: "i2c", + }, + { + type: "thread", + name: "pfr_i2c_notify_client_thread", + kernel_stack_size_bytes: 2048, + }, + ], + }, + ], + }, + ], +} diff --git a/target/ast10x0/tests/pfr/target.rs b/target/ast10x0/tests/pfr/target.rs new file mode 100644 index 00000000..2356aa68 --- /dev/null +++ b/target/ast10x0/tests/pfr/target.rs @@ -0,0 +1,52 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +#![no_std] +#![no_main] + +use ast10x0_peripherals::i2c; +use ast10x0_peripherals::scu::pinctrl; +use console_backend::console_backend_write_all; +use entry as _; +use target_common::{declare_target, TargetInterface}; + +pub struct Target {} + +impl TargetInterface for Target { + const NAME: &'static str = "AST10x0 PFR I2C Notify Bringup"; + + fn main() -> ! { + // Kernel-owned early platform init for I2C2 userspace apps. + unsafe { + let scu = ast10x0_peripherals::scu::ScuRegisters::new_global_unlocked(); + scu.ungate_clock_mask(ast10x0_peripherals::scu::ClockRegisterHalf::Lower, 1 << 2); + scu.assert_reset_mask(ast10x0_peripherals::scu::ScuRegisterHalf::Upper, 1 << 2); + for _ in 0..10_000u32 { + core::hint::spin_loop(); + } + scu.deassert_reset_mask(ast10x0_peripherals::scu::ScuRegisterHalf::Upper, 1 << 2); + for _ in 0..10_000u32 { + core::hint::spin_loop(); + } + scu.apply_pinctrl_group(pinctrl::PINCTRL_I2C2); + i2c::init_i2c_global(); + } + + codegen::start(); + #[expect(clippy::empty_loop)] + loop {} + } + + fn shutdown(code: u32) -> ! { + let sentinel: &[u8] = if code == 0 { + b"TEST_RESULT:PASS\n" + } else { + b"TEST_RESULT:FAIL\n" + }; + let _ = console_backend_write_all(sentinel); + #[expect(clippy::empty_loop)] + loop {} + } +} + +declare_target!(Target); From d83c70a20c47aec9d1987fa7d9c5bd57744a07a4 Mon Sep 17 00:00:00 2001 From: Steven Lee Date: Wed, 10 Jun 2026 17:45:56 +0800 Subject: [PATCH 2/3] app/aspeed-pfr: i2c: Serve software mailbox synchronously in slave IRQ The previous PFR mailbox bringup split the read path across an IPC boundary: the i2c server-runtime drained the slave RX, woke the client, and the client (owning SwmbxCtrl) answered master reads over IPC. The round-trip never completed inside the AST controller's TX clock-stretch window, so the master sampled an idle bus (0xff) and FIFO / back-to-back reads failed. Move SwmbxCtrl into the server process and serve reads inline in the slave IRQ, mirroring aspeed-rust's in-ISR swmbx_target callback model: - server_main now owns SwmbxCtrl, configures PROTECT/NOTIFY/FIFO policy and per-port protection bitmaps, and answers get_msg + write_slave response before releasing the controller's TX stretch. No logging on the hot path between offset receipt and the TX-DMA arm. - pfr_i2c_notify_client is retired to an idle placeholder; the system image layout is unchanged. HAL / driver changes that make synchronous serving correct: - Add SlaveWrRecvdStop (HAL) / DataReceivedStop (peripheral) so a write burst that latches data and STOP in one status is delivered as one event and the transaction is finalized in place. - Stop setting AST_I2CC_SLAVE_PKT_SAVE_ADDR: the matched address is no longer prepended to RX, restoring plain offset-addressed SMBus [offset, data...] semantics. RX length and copy paths skip the addr byte only when the bit is set. - Normalize slave status (mask address-indicate / pending / NAK bits), surface the inactive-timeout as a Stop so the per-transaction cursor resets, and arm slave TX via TX_DMA_EN with a DMA actual-length reset to avoid clocking out stale 0xff. - server-runtime: replace the single-slot RX latch with a depth-4 event ring so a DataReceived/Stop burst is not split across wake-ups. SwmbxCtrl: zero the (uninitialized device-memory) backing region on init, add take_notify() to drain latched node notifications, and return 0 from get_msg on an empty FIFO instead of FifoEmpty. system.json5: move the server to Bus 0 (I2C0 @ 0x7e7b0080, IRQ 110), map the swmbx_buf SRAM region, and right-size the per-app flash. Signed-off-by: Steven Lee --- hal/blocking/src/i2c_hardware.rs | 2 + .../impls/baremetal/mock/src/i2c_hardware.rs | 3 +- services/i2c/api/src/seam.rs | 3 - services/i2c/server-runtime/src/lib.rs | 163 ++++++--- target/ast10x0/backend/i2c/src/lib.rs | 39 ++ target/ast10x0/peripherals/i2c/constants.rs | 12 + .../ast10x0/peripherals/i2c/hal_slave_impl.rs | 3 +- target/ast10x0/peripherals/i2c/slave.rs | 262 +++++++++---- .../ast10x0/peripherals/i2c/target_adapter.rs | 26 +- target/ast10x0/peripherals/scu/pinctrl.rs | 5 + target/ast10x0/pfr/README.md | 62 +++- target/ast10x0/pfr/smbus_mailbox_client.rs | 67 ++-- target/ast10x0/pfr/swmbx_ctrl.rs | 46 ++- .../peripherals/i2c/i2c_irq/slave_target.rs | 4 +- target/ast10x0/tests/pfr/BUILD.bazel | 3 +- .../tests/pfr/pfr_i2c_notify_client_main.rs | 174 +-------- target/ast10x0/tests/pfr/server_main.rs | 346 +++++++++++++++++- target/ast10x0/tests/pfr/system.json5 | 18 +- target/ast10x0/tests/pfr/target.rs | 2 +- 19 files changed, 863 insertions(+), 377 deletions(-) diff --git a/hal/blocking/src/i2c_hardware.rs b/hal/blocking/src/i2c_hardware.rs index 11eb6d28..3f6e7a34 100644 --- a/hal/blocking/src/i2c_hardware.rs +++ b/hal/blocking/src/i2c_hardware.rs @@ -147,6 +147,8 @@ pub mod slave { SlaveRdProc, /// Slave has received write data from master SlaveWrRecvd, + /// Slave received write data and the same hardware status included STOP + SlaveWrRecvdStop, /// Stop condition received SlaveStop, } diff --git a/platform/impls/baremetal/mock/src/i2c_hardware.rs b/platform/impls/baremetal/mock/src/i2c_hardware.rs index 2057aebb..0ca8c479 100644 --- a/platform/impls/baremetal/mock/src/i2c_hardware.rs +++ b/platform/impls/baremetal/mock/src/i2c_hardware.rs @@ -913,7 +913,8 @@ impl MockI2cHardware { // Simulate event handling based on event type match event { - openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent::SlaveWrRecvd => { + openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent::SlaveWrRecvd + | openprot_hal_blocking::i2c_hardware::slave::I2cIsrEvent::SlaveWrRecvdStop => { // Simulate receiving data using safe injection self.inject_slave_data(&[0xAA, 0xBB, 0xCC]); } diff --git a/services/i2c/api/src/seam.rs b/services/i2c/api/src/seam.rs index 0b30eb7f..df9ef9bd 100644 --- a/services/i2c/api/src/seam.rs +++ b/services/i2c/api/src/seam.rs @@ -40,9 +40,6 @@ pub trait I2cSlaveEvent: I2cSlaveBuffer { } } -/// Blanket impl so all I2cSlaveBuffer types get the default behavior. -impl I2cSlaveEvent for T {} - /// Map a wire status code onto the `embedded_hal::i2c::ErrorKind` taxonomy so /// the client can satisfy `embedded_hal::i2c::Error`. pub fn error_kind(err: I2cError) -> ErrorKind { diff --git a/services/i2c/server-runtime/src/lib.rs b/services/i2c/server-runtime/src/lib.rs index a8ced564..e462c03d 100644 --- a/services/i2c/server-runtime/src/lib.rs +++ b/services/i2c/server-runtime/src/lib.rs @@ -14,9 +14,9 @@ //! //! The WaitGroup multiplexes the per-bus client channels (`READABLE`) **and** //! the i2c hardware IRQ. On the IRQ: for every notification-armed bus, drain -//! the slave RX into that bus's latch, `interrupt_ack`, then raise +//! the slave RX into that bus's event queue, `interrupt_ack`, then raise //! `Signals::USER` on that bus's client channel. The client then issues -//! `SlaveReceive`, which returns the latched bytes (status `NoData` if empty). +//! `SlaveReceive`, which returns the oldest queued event (status `NoData` if empty). //! Arm/disarm is `EnableSlaveNotification` / `DisableSlaveNotification`. //! //! The **only** kernel-tagged server crate; it wraps the host-buildable @@ -33,8 +33,10 @@ use i2c_server::{dispatch, MAX_BUF_SIZE}; use userspace::syscall::{self, Signals}; use userspace::time::Instant; +const SLAVE_EVENT_QUEUE_DEPTH: usize = 4; + /// One bus the server owns: its dedicated IPC channel, IRQ handle, the driver instance -/// (master + slave), and the per-bus slave-RX notification latch. +/// (master + slave), and the per-bus slave event notification queue. pub struct Bus { /// IPC channel handle (`channel_handler`) dedicated to this bus. pub channel: u32, @@ -43,14 +45,13 @@ pub struct Bus { /// The bus driver — implements both the master and slave seams. pub driver: B, notif_enabled: bool, - rx: [u8; MAX_PAYLOAD_SIZE], - rx_len: usize, - /// Source address (7-bit) of the master that wrote to us; only valid - /// when rx_len > 0. Captured on IRQ drain; ignored if not available. - rx_source: u8, - /// Event kind that triggered the latch (DataReceived, ReadRequest, Stop). - /// Only meaningful when rx_len > 0 or notification was armed. - rx_event_kind: SlaveEvent, + event_data: [[u8; MAX_PAYLOAD_SIZE]; SLAVE_EVENT_QUEUE_DEPTH], + event_len: [usize; SLAVE_EVENT_QUEUE_DEPTH], + event_source: [u8; SLAVE_EVENT_QUEUE_DEPTH], + event_kind: [SlaveEvent; SLAVE_EVENT_QUEUE_DEPTH], + event_head: usize, + event_tail: usize, + event_count: usize, } impl Bus { @@ -60,11 +61,59 @@ impl Bus { irq, driver, notif_enabled: false, - rx: [0u8; MAX_PAYLOAD_SIZE], - rx_len: 0, - rx_source: 0, - rx_event_kind: SlaveEvent::DataReceived, + event_data: [[0u8; MAX_PAYLOAD_SIZE]; SLAVE_EVENT_QUEUE_DEPTH], + event_len: [0; SLAVE_EVENT_QUEUE_DEPTH], + event_source: [0; SLAVE_EVENT_QUEUE_DEPTH], + event_kind: [SlaveEvent::DataReceived; SLAVE_EVENT_QUEUE_DEPTH], + event_head: 0, + event_tail: 0, + event_count: 0, + } + } + + fn clear_slave_events(&mut self) { + self.event_head = 0; + self.event_tail = 0; + self.event_count = 0; + self.event_len = [0; SLAVE_EVENT_QUEUE_DEPTH]; + } + + fn has_slave_event(&self) -> bool { + self.event_count > 0 + } + + fn push_slave_event(&mut self, kind: SlaveEvent, source: u8, data: &[u8]) -> bool { + if self.event_count == SLAVE_EVENT_QUEUE_DEPTH { + return false; + } + + let slot = self.event_tail; + let n = data.len().min(MAX_PAYLOAD_SIZE); + if n > 0 { + self.event_data[slot][..n].copy_from_slice(&data[..n]); } + self.event_len[slot] = n; + self.event_source[slot] = source; + self.event_kind[slot] = kind; + self.event_tail = (self.event_tail + 1) % SLAVE_EVENT_QUEUE_DEPTH; + self.event_count += 1; + true + } + + /// Push an event, logging if the queue is full (the common call shape). + fn push_or_warn(&mut self, kind: SlaveEvent, source: u8, data: &[u8]) { + if !self.push_slave_event(kind, source, data) { + pw_log::error!("slave event queue full"); + } + } + + fn pop_slave_event(&mut self) { + if self.event_count == 0 { + return; + } + self.event_len[self.event_head] = 0; + self.event_head = (self.event_head + 1) % SLAVE_EVENT_QUEUE_DEPTH; + self.event_count -= 1; } } @@ -111,6 +160,7 @@ where let mut request_buf = [0u8; MAX_BUF_SIZE]; let mut response_buf = [0u8; MAX_BUF_SIZE]; + let mut slave_event_buf = [0u8; MAX_PAYLOAD_SIZE]; let wait_mask = Signals::READABLE | irq_signals; loop { @@ -128,35 +178,36 @@ where if bus.notif_enabled { match bus.driver.try_next_slave_event() { Ok(Some((kind, _))) => { - // Store the actual hardware event kind - bus.rx_event_kind = match kind { - I2cIsrEvent::SlaveWrRecvd => SlaveEvent::DataReceived, - I2cIsrEvent::SlaveRdReq => SlaveEvent::ReadRequest, - I2cIsrEvent::SlaveStop => SlaveEvent::Stop, - _ => SlaveEvent::DataReceived, - }; - // For DataReceived, read the buffer; other events have no data - if kind == I2cIsrEvent::SlaveWrRecvd { - match bus.driver.read_slave_buffer(&mut bus.rx) { - Ok(n) => { - if n > 0 { - bus.rx_len = n; - // Source address extraction from MCTP-I2C header. - // With AST_I2CC_SLAVE_PKT_SAVE_ADDR set, the hardware - // prepends dest address at byte[0]; the MCTP-I2C header - // carries source at byte[3]: src_addr << 1 | 1. - // Extract it (bits 7:1 = 7-bit address). - if n > 3 { - bus.rx_source = (bus.rx[3] >> 1) & 0x7F; - } else { - bus.rx_source = 0xFF; // Invalid: message too short - } - } + match kind { + I2cIsrEvent::SlaveWrRecvd + | I2cIsrEvent::SlaveWrRecvdStop => { + // Plain offset-addressed mailbox: RX is + // [offset, data...] with no MCTP source header + // (AST_I2CC_SLAVE_PKT_SAVE_ADDR is cleared). No + // source to extract; the client defaults to port 0. + let stop_after_data = + kind == I2cIsrEvent::SlaveWrRecvdStop; + slave_event_buf.fill(0); + match bus.driver.read_slave_buffer(&mut slave_event_buf) { + Ok(n) if n > 0 => bus.push_or_warn( + SlaveEvent::DataReceived, + 0xFF, + &slave_event_buf[..n], + ), + Ok(_) => {} + Err(_) => pw_log::error!("read_slave_buffer failed"), } - Err(_) => { - pw_log::error!("read_slave_buffer failed"); + if stop_after_data { + bus.push_or_warn(SlaveEvent::Stop, 0xFF, &[]); } } + I2cIsrEvent::SlaveRdReq | I2cIsrEvent::SlaveRdProc => { + bus.push_or_warn(SlaveEvent::ReadRequest, 0xFF, &[]); + } + I2cIsrEvent::SlaveStop => { + bus.push_or_warn(SlaveEvent::Stop, 0xFF, &[]); + } + I2cIsrEvent::SlaveWrReq => {} } } Ok(None) => { @@ -172,11 +223,7 @@ where if let Err(_) = syscall::interrupt_ack(irq, acked) { pw_log::error!("interrupt_ack failed"); } - // Wake client on data events (DataReceived with bytes) or transaction - // boundaries (Stop). ReadRequest without data is deferred (post-demo). - let should_wake = - bus.notif_enabled && (bus.rx_len > 0 || bus.rx_event_kind == SlaveEvent::Stop); - if should_wake { + if bus.notif_enabled && bus.has_slave_event() { // ORs USER onto the bus channel without disturbing READABLE. if let Err(_) = syscall::object_set_peer_user_signal(bus.channel, true) { pw_log::error!("object_set_peer_user_signal failed"); @@ -230,13 +277,11 @@ where } Some((I2cOp::DisableSlaveNotification, _)) => { bus.notif_enabled = false; - bus.rx_len = 0; - bus.rx_source = 0; - bus.rx_event_kind = SlaveEvent::DataReceived; + bus.clear_slave_events(); encode_ok(&mut response_buf, 0) } Some((I2cOp::SlaveReceive, max_len)) => { - if bus.rx_len == 0 { + if !bus.has_slave_event() { encode_error(&mut response_buf, I2cError::NoData) } else { // Response payload: [kind (1), source_addr (1), data (0..max_len)] @@ -245,14 +290,20 @@ where if cap < metadata_size { encode_error(&mut response_buf, I2cError::BufferTooSmall) } else { + let slot = bus.event_head; let data_cap = cap - metadata_size; - let n = bus.rx_len.min(max_len).min(data_cap); + let n = bus.event_len[slot].min(max_len).min(data_cap); let payload_offset = I2cResponseHeader::SIZE; - response_buf[payload_offset] = bus.rx_event_kind as u8; - response_buf[payload_offset + 1] = bus.rx_source; - response_buf[payload_offset + 2..payload_offset + 2 + n] - .copy_from_slice(&bus.rx[..n]); - bus.rx_len = 0; + response_buf[payload_offset] = bus.event_kind[slot] as u8; + response_buf[payload_offset + 1] = bus.event_source[slot]; + if n > 0 { + response_buf[payload_offset + 2..payload_offset + 2 + n] + .copy_from_slice(&bus.event_data[slot][..n]); + } + bus.pop_slave_event(); + if bus.has_slave_event() { + let _ = syscall::object_set_peer_user_signal(bus.channel, true); + } encode_ok(&mut response_buf, metadata_size + n) } } diff --git a/target/ast10x0/backend/i2c/src/lib.rs b/target/ast10x0/backend/i2c/src/lib.rs index 2e206afe..a3553f7a 100644 --- a/target/ast10x0/backend/i2c/src/lib.rs +++ b/target/ast10x0/backend/i2c/src/lib.rs @@ -41,11 +41,13 @@ use ast1060_pac::{i2c::RegisterBlock, i2cbuff::RegisterBlock as BuffRegisterBlock}; use embedded_hal::i2c::{ErrorType, I2c, Operation, SevenBitAddress}; +use i2c_api::seam::{I2cIsrEvent, I2cSlaveEvent}; use openprot_hal_blocking::i2c_hardware::slave::{I2cSlaveBuffer, I2cSlaveCore}; use openprot_hal_blocking::i2c_hardware::I2cBusRecovery; pub use ast10x0_peripherals::i2c::{ Ast1060I2c, Ast1060I2cRegisters, ClockConfig, I2cConfig, I2cError, I2cSpeed, I2cXferMode, + SlaveEvent, }; /// The yield closure type stored in every bus driver. @@ -192,6 +194,43 @@ impl I2cSlaveBuffer for Ast1060I2cBackend { } } +impl Ast1060I2cBackend { + /// True when the controller is clock-stretching a master read, waiting for + /// the firmware to load the TX byte. Lets the server serve a read inline. + pub fn slave_waiting_for_tx(&mut self) -> bool { + self.make_driver().slave_waiting_for_tx() + } + + /// Debug: (i2cs20 irq-enable mask, i2cc00 control, i2cs24 status). + pub fn dbg_slave_regs(&mut self) -> (u32, u32, u32) { + self.make_driver().dbg_slave_regs() + } +} + +impl I2cSlaveEvent for Ast1060I2cBackend { + fn try_next_slave_event(&mut self) -> Result, Self::Error> { + let event = { + let mut driver = self.make_driver(); + driver.handle_slave_interrupt() + }; + + Ok(match event { + Some(SlaveEvent::ReadRequest) => Some((I2cIsrEvent::SlaveRdReq, 0)), + Some(SlaveEvent::WriteRequest) => Some((I2cIsrEvent::SlaveWrReq, 0)), + Some(SlaveEvent::DataReceived { len }) => Some((I2cIsrEvent::SlaveWrRecvd, len)), + Some(SlaveEvent::DataReceivedStop { len }) => { + Some((I2cIsrEvent::SlaveWrRecvdStop, len)) + } + Some(SlaveEvent::DataReceivedAndSent { rx_len, .. }) => { + Some((I2cIsrEvent::SlaveWrRecvd, rx_len)) + } + Some(SlaveEvent::DataSent { len }) => Some((I2cIsrEvent::SlaveRdProc, len)), + Some(SlaveEvent::Stop) => Some((I2cIsrEvent::SlaveStop, 0)), + None => None, + }) + } +} + impl I2cBusRecovery for Ast1060I2cBackend { fn recover_bus(&mut self) -> Result<(), Self::Error> { self.make_driver().recover_bus() diff --git a/target/ast10x0/peripherals/i2c/constants.rs b/target/ast10x0/peripherals/i2c/constants.rs index b2bd6b90..dcb9cc03 100644 --- a/target/ast10x0/peripherals/i2c/constants.rs +++ b/target/ast10x0/peripherals/i2c/constants.rs @@ -170,6 +170,18 @@ pub const AST_I2CS_PKT_ERROR: u32 = 1 << 17; pub const AST_I2CS_WAIT_TX_DMA: u32 = 1 << 25; /// Waiting for RX DMA pub const AST_I2CS_WAIT_RX_DMA: u32 = 1 << 24; +/// Slave transaction is pending +pub const AST_I2CS_SLAVE_PENDING: u32 = 1 << 29; +/// Encoded matched-address indicator bits +pub const AST_I2CS_ADDR_INDICATE_MASK: u32 = 3 << 30; +/// Address 3 NAK status +pub const AST_I2CS_ADDR3_NAK: u32 = 1 << 22; +/// Address 2 NAK status +pub const AST_I2CS_ADDR2_NAK: u32 = 1 << 21; +/// Address 1 NAK status +pub const AST_I2CS_ADDR1_NAK: u32 = 1 << 20; +/// Address status field +pub const AST_I2CS_ADDR_MASK: u32 = 3 << 18; /// Helper to build packet mode address field #[inline] diff --git a/target/ast10x0/peripherals/i2c/hal_slave_impl.rs b/target/ast10x0/peripherals/i2c/hal_slave_impl.rs index e8009bdf..9d075bc4 100644 --- a/target/ast10x0/peripherals/i2c/hal_slave_impl.rs +++ b/target/ast10x0/peripherals/i2c/hal_slave_impl.rs @@ -32,6 +32,7 @@ fn to_hal_event(ev: SlaveEvent) -> I2cIsrEvent { SlaveEvent::DataReceived { .. } | SlaveEvent::DataReceivedAndSent { .. } => { I2cIsrEvent::SlaveWrRecvd } + SlaveEvent::DataReceivedStop { .. } => I2cIsrEvent::SlaveWrRecvdStop, SlaveEvent::DataSent { .. } => I2cIsrEvent::SlaveRdProc, SlaveEvent::Stop => I2cIsrEvent::SlaveStop, } @@ -40,7 +41,7 @@ fn to_hal_event(ev: SlaveEvent) -> I2cIsrEvent { /// Bytes received in the event, if any (the only thing the drain path needs). fn rx_len(ev: SlaveEvent) -> Option { match ev { - SlaveEvent::DataReceived { len } => Some(len), + SlaveEvent::DataReceived { len } | SlaveEvent::DataReceivedStop { len } => Some(len), SlaveEvent::DataReceivedAndSent { rx_len, .. } => Some(rx_len), _ => None, } diff --git a/target/ast10x0/peripherals/i2c/slave.rs b/target/ast10x0/peripherals/i2c/slave.rs index 9888549e..d9f2daf2 100644 --- a/target/ast10x0/peripherals/i2c/slave.rs +++ b/target/ast10x0/peripherals/i2c/slave.rs @@ -57,6 +57,8 @@ pub enum SlaveEvent { WriteRequest, /// Data received from master DataReceived { len: usize }, + /// Data received from master and stop condition received in the same status + DataReceivedStop { len: usize }, /// Data sent to master DataSent { len: usize }, /// Data received from master and send data to master (combined event) @@ -122,22 +124,54 @@ impl SlaveBuffer { } impl Ast1060I2c<'_, Y> { + #[inline] + fn slave_rx_includes_addr(&self) -> bool { + (self.regs().i2cc00().read().bits() & constants::AST_I2CC_SLAVE_PKT_SAVE_ADDR) != 0 + } + + #[inline] + pub fn slave_waiting_for_tx(&self) -> bool { + (self.regs().i2cs24().read().bits() & constants::AST_I2CS_WAIT_TX_DMA) != 0 + } + + /// Debug: (i2cs20 irq-enable mask, i2cc00 control, i2cs24 status). + #[inline] + pub fn dbg_slave_regs(&self) -> (u32, u32, u32) { + ( + self.regs().i2cs20().read().bits(), + self.regs().i2cc00().read().bits(), + self.regs().i2cs24().read().bits(), + ) + } + #[inline] fn slave_rx_len(&self) -> usize { if self.xfer_mode == I2cXferMode::DmaMode { - self.regs().i2cs4c().read().dmarx_actual_len_byte().bits() as usize + let raw = self.regs().i2cs4c().read().dmarx_actual_len_byte().bits() as usize; + raw.saturating_sub(usize::from(self.slave_rx_includes_addr())) } else { - // Hardware includes the I2C address byte in the buffer count (packet mode, - // I2CC00 bit 20). Subtract 1 to report only the payload byte count. - self.regs() + let raw = self + .regs() .i2cc0c() .read() .actual_rxd_pool_buffer_size() - .bits() - .saturating_sub(1) as usize + .bits() as usize; + raw.saturating_sub(usize::from(self.slave_rx_includes_addr())) } } + #[inline] + fn normalized_slave_status(status: u32) -> u32 { + status + & !(constants::AST_I2CS_ADDR_INDICATE_MASK + | constants::AST_I2CS_SLAVE_PENDING + | constants::AST_I2CS_ADDR_MASK + | constants::AST_I2CS_ADDR1_NAK + | constants::AST_I2CS_ADDR2_NAK + | constants::AST_I2CS_ADDR3_NAK + | constants::AST_I2CS_INACTIVE_TO) + } + /// Arm slave receive path based on transfer mode. fn arm_slave_receive(&mut self, cmd: &mut u32) { if self.xfer_mode == I2cXferMode::DmaMode { @@ -190,15 +224,6 @@ impl Ast1060I2c<'_, Y> { // Clear slave interrupts self.clear_slave_interrupts(); - // Enable slave mode and save address byte in packet mode (I2CC00 bit 20) - // This makes the hardware include the destination address byte in the receive buffer - // which is required for MCTP-over-SMBus (DSP0237) packet format. - self.regs().i2cc00().modify(|r, w| unsafe { - w.bits( - r.bits() | constants::AST_I2CC_SLAVE_EN | constants::AST_I2CC_SLAVE_PKT_SAVE_ADDR, - ) - }); - // Configure slave mode let mut cmd = 0u32; @@ -250,6 +275,13 @@ impl Ast1060I2c<'_, Y> { // Enable slave interrupts self.enable_slave_interrupts(); + // Enable slave mode only after RX DMA/buffer is armed. Do NOT set + // AST_I2CC_SLAVE_PKT_SAVE_ADDR: that prepends the matched destination + // address as RX byte[0], shifting SMBus [offset, data...] semantics. + self.regs().i2cc00().modify(|r, w| unsafe { + w.bits(r.bits() | constants::AST_I2CC_SLAVE_EN) + }); + Ok(()) } @@ -317,71 +349,79 @@ impl Ast1060I2c<'_, Y> { pub fn slave_read(&mut self, buffer: &mut [u8]) -> Result { // Get receive length from buffer length register if self.xfer_mode == I2cXferMode::BufferMode { - // AST_I2CC_SLAVE_PKT_SAVE_ADDR (I2CC00 bit 20) deposits the I2C - // address byte at buffer offset 0. Read the raw count once, subtract - // 1 for that byte, then copy only the payload (skipping offset 0). let raw = self .regs() .i2cc0c() .read() .actual_rxd_pool_buffer_size() .bits() as usize; - let data_len = raw.saturating_sub(1); - let to_read = data_len.min(buffer.len()).min(BUFFER_SIZE - 1); + let skip = usize::from(self.slave_rx_includes_addr()); + let data_len = raw.saturating_sub(skip); + let to_read = data_len.min(buffer.len()).min(BUFFER_SIZE.saturating_sub(skip)); let mut tmp = [0u8; BUFFER_SIZE]; - self.copy_from_buffer(&mut tmp[..1 + to_read])?; - buffer[..to_read].copy_from_slice(&tmp[1..1 + to_read]); + self.copy_from_buffer(&mut tmp[..skip + to_read])?; + buffer[..to_read].copy_from_slice(&tmp[skip..skip + to_read]); - // Re-enable RX buffer - let mut cmd = constants::AST_I2CS_ACTIVE_ALL | constants::AST_I2CS_PKT_MODE_EN; - cmd |= constants::AST_I2CS_RX_BUFF_EN; - unsafe { - self.regs().i2cs28().write(|w| w.bits(cmd)); + let rearm_rx = !self.slave_waiting_for_tx(); + if rearm_rx { + let mut cmd = constants::AST_I2CS_ACTIVE_ALL | constants::AST_I2CS_PKT_MODE_EN; + cmd |= constants::AST_I2CS_RX_BUFF_EN; + unsafe { + self.regs().i2cs28().write(|w| w.bits(cmd)); + } } + pw_log::info!( + "i2c slave_read buf raw={} skip={} n={} rx0=0x{:02x} rx1=0x{:02x} rearm={}", + raw as u32, + skip as u32, + to_read as u32, + buffer.get(0).copied().unwrap_or(0) as u32, + buffer.get(1).copied().unwrap_or(0) as u32, + rearm_rx as u32 + ); Ok(to_read) } else if self.xfer_mode == I2cXferMode::DmaMode { - // DMA mode: the hardware has already DMA'd into `self.dma_buf`. - // AST_I2CC_SLAVE_PKT_SAVE_ADDR deposits the address byte at dma_buf[0]; - // subtract 1 and skip it, matching the buffer-mode treatment above. let hw_len = self.regs().i2cs4c().read().dmarx_actual_len_byte().bits() as usize; - let data_len = hw_len.saturating_sub(1); + let skip = usize::from(self.slave_rx_includes_addr()); + let data_len = hw_len.saturating_sub(skip); let to_read = data_len.min(buffer.len()); if let Some(dma_buf) = self.slave_dma_buf.as_deref() { - let src_len = to_read.min(dma_buf.len().saturating_sub(1)); + let src_len = to_read.min(dma_buf.len().saturating_sub(skip)); if let (Some(src), Some(dst)) = - (dma_buf.get(1..1 + src_len), buffer.get_mut(..src_len)) + (dma_buf.get(skip..skip + src_len), buffer.get_mut(..src_len)) { dst.copy_from_slice(src); } } - // Re-arm slave DMA for next receive - let mut cmd = constants::AST_I2CS_ACTIVE_ALL | constants::AST_I2CS_PKT_MODE_EN; - if let Some(dma_buf) = self.slave_dma_buf.as_deref_mut() { - let dma_addr = dma_buf.as_mut_ptr() as u32; - let dma_len = u16::try_from(dma_buf.len().min(4096) - 1).unwrap_or(u16::MAX); + let rearm_rx = !self.slave_waiting_for_tx(); + if rearm_rx { + let mut cmd = constants::AST_I2CS_ACTIVE_ALL | constants::AST_I2CS_PKT_MODE_EN; + if let Some(dma_buf) = self.slave_dma_buf.as_deref_mut() { + let dma_addr = dma_buf.as_mut_ptr() as u32; + let dma_len = u16::try_from(dma_buf.len().min(4096) - 1).unwrap_or(u16::MAX); + unsafe { + self.regs().i2cs4c().write(|w| w.bits(0)); + self.regs().i2cs38().write(|w| w.bits(dma_addr)); + self.regs().i2cs3c().write(|w| w.bits(dma_addr)); + self.regs().i2cs2c().write(|w| { + w.dmarx_buf_len_byte() + .bits(dma_len) + .dmarx_buf_len_wr_enbl_for_cur_cmd() + .set_bit() + }); + } + cmd |= AST_I2CS_RX_DMA_EN; + } else { + cmd |= constants::AST_I2CS_RX_BUFF_EN; + } unsafe { - self.regs().i2cs4c().write(|w| w.bits(0)); - self.regs().i2cs38().write(|w| w.bits(dma_addr)); - self.regs().i2cs3c().write(|w| w.bits(dma_addr)); - self.regs().i2cs2c().write(|w| { - w.dmarx_buf_len_byte() - .bits(dma_len) - .dmarx_buf_len_wr_enbl_for_cur_cmd() - .set_bit() - }); + self.regs().i2cs28().write(|w| w.bits(cmd)); } - cmd |= AST_I2CS_RX_DMA_EN; - } else { - cmd |= constants::AST_I2CS_RX_BUFF_EN; - } - unsafe { - self.regs().i2cs28().write(|w| w.bits(cmd)); } - Ok(to_read) } else { // byte mode @@ -394,6 +434,7 @@ impl Ast1060I2c<'_, Y> { self.regs().i2cs28().write(|w| unsafe { w.bits(cmd) }); self.clear_slave_interrupts(); + pw_log::info!("i2c slave_read byte rx0=0x{:02x}", byte as u32); Ok(1) } } @@ -422,22 +463,42 @@ impl Ast1060I2c<'_, Y> { unsafe { self.regs().i2cs28().write(|w| w.bits(cmd)); } + pw_log::info!( + "i2c slave_write buf n={} tx0=0x{:02x} cmd=0x{:08x}", + to_write as u32, + data[0] as u32, + cmd as u32 + ); Ok(to_write) } else if self.xfer_mode == I2cXferMode::DmaMode { - // Slave TX always uses the 32-byte hardware FIFO, even in DMA mode. - // The DMA buffer is reserved exclusively for slave RX — writing TX data - // into dma_buf would alias the RX path. This matches base's - // slave_set_response() pattern (buffs.buff(0) + TX_BUFF_EN). - let to_write = data.len().min(BUFFER_SIZE); - self.copy_to_buffer(&data[..to_write])?; + let Some(dma_buf) = self.slave_dma_buf.as_deref_mut() else { + return Err(I2cError::Invalid); + }; + let to_write = data.len().min(dma_buf.len()).min(constants::DMA_MODE_MAX_SIZE); + dma_buf[..to_write].copy_from_slice(&data[..to_write]); + #[allow(clippy::cast_possible_truncation)] - self.regs() - .i2cc0c() - .write(|w| unsafe { w.tx_data_byte_count().bits((to_write - 1) as u8) }); + let dma_tx_len = u16::try_from(to_write - 1).unwrap_or(u16::MAX); + unsafe { + // Reset the DMA actual-length register before programming a new + // TX length. The stale dmatx_actual_len from a prior transfer + // otherwise leaves the engine thinking the TX is already + // complete, so it clocks out nothing and the master samples an + // idle bus as 0xff. Mirrors aspeed-rust's slave read path. + self.regs().i2cs4c().write(|w| w.bits(0)); + self.regs().i2cs2c().modify(|_, w| { + w.dmatx_buf_len_byte() + .bits(dma_tx_len) + .dmatx_buf_len_wr_enbl_for_cur_cmd() + .set_bit() + }); + } - // Arm TX via FIFO and keep RX DMA armed in one atomic i2cs28 write. - let mut cmd = constants::AST_I2CS_ACTIVE_ALL | constants::AST_I2CS_PKT_MODE_EN; - cmd |= constants::AST_I2CS_TX_BUFF_EN | AST_I2CS_RX_DMA_EN; + // DMA-mode slave reads must be released with TX_DMA_EN. This mirrors + // aspeed-rust's swmbx target path and avoids mixing FIFO TX with DMA RX. + let cmd = constants::AST_I2CS_ACTIVE_ALL + | constants::AST_I2CS_PKT_MODE_EN + | constants::AST_I2CS_TX_DMA_EN; unsafe { self.regs().i2cs28().write(|w| w.bits(cmd)); } @@ -454,6 +515,7 @@ impl Ast1060I2c<'_, Y> { } self.clear_slave_interrupts(); + pw_log::info!("i2c slave_write byte tx0=0x{:02x}", data[0] as u32); Ok(1) } } @@ -467,8 +529,30 @@ impl Ast1060I2c<'_, Y> { return None; } - // Check for errors first + if (status & constants::AST_I2CS_INACTIVE_TO) != 0 { + let mut cmd: u32 = constants::AST_I2CS_ACTIVE_ALL | constants::AST_I2CS_PKT_MODE_EN; + self.arm_slave_receive(&mut cmd); + unsafe { + self.regs().i2cs28().write(|w| w.bits(cmd)); + self.regs().i2cs24().write(|w| w.bits(status)); + let _ = self.regs().i2cs24().read().bits(); + } + pw_log::info!( + "i2c slave inactive timeout raw=0x{:08x} rearm=0x{:08x}", + status as u32, + cmd as u32 + ); + // Surface the timeout as a Stop so the application resets its + // per-transaction state (e.g. the SMBus offset cursor / first-byte + // flag). An aborted master read ends here with no real STOP on the + // bus; without this the next transaction mis-parses its offset byte + // as data and the cursor drifts across transactions. + return Some(SlaveEvent::Stop); + } + + // Check for real packet errors after the inactive-timeout cleanup path. if (status & constants::AST_I2CS_PKT_ERROR) != 0 { + pw_log::error!("i2c slave pkt_error raw=0x{:08x}", status as u32); self.clear_slave_interrupts(); return None; } @@ -480,7 +564,9 @@ impl Ast1060I2c<'_, Y> { .i2cs24() .write(|w| w.bits(constants::AST_I2CS_PKT_DONE)); } - let sts = status & (!(constants::AST_I2CS_PKT_DONE | constants::AST_I2CS_PKT_ERROR)); + let sts = Self::normalized_slave_status( + status & (!(constants::AST_I2CS_PKT_DONE | constants::AST_I2CS_PKT_ERROR)), + ); if sts == constants::AST_I2CS_SLAVE_MATCH || sts == constants::AST_I2CS_SLAVE_MATCH | constants::AST_I2CS_RX_DONE { @@ -507,8 +593,18 @@ impl Ast1060I2c<'_, Y> { self.regs().i2cs28().write(|w| w.bits(cmd)); } return Some(SlaveEvent::Stop); - } else if sts == constants::AST_I2CS_RX_DONE | constants::AST_I2CS_STOP - || sts == constants::AST_I2CS_RX_DONE | constants::AST_I2CS_WAIT_RX_DMA + } else if sts == constants::AST_I2CS_RX_DONE | constants::AST_I2CS_WAIT_RX_DMA { + // S: (Sw)|D + return Some(SlaveEvent::DataReceived { + len: self.slave_rx_len(), + }); + } else if sts + == constants::AST_I2CS_SLAVE_MATCH + | constants::AST_I2CS_RX_DONE + | constants::AST_I2CS_WAIT_RX_DMA + | constants::AST_I2CS_STOP + | constants::AST_I2CS_TX_NAK + || sts == constants::AST_I2CS_RX_DONE | constants::AST_I2CS_STOP || sts == constants::AST_I2CS_RX_DONE | constants::AST_I2CS_WAIT_RX_DMA @@ -533,7 +629,7 @@ impl Ast1060I2c<'_, Y> { | constants::AST_I2CS_STOP { // S: (Sw)|D|(P) - return Some(SlaveEvent::DataReceived { + return Some(SlaveEvent::DataReceivedStop { len: self.slave_rx_len(), }); } else if sts == constants::AST_I2CS_RX_DONE | constants::AST_I2CS_WAIT_TX_DMA @@ -573,11 +669,17 @@ impl Ast1060I2c<'_, Y> { } return Some(SlaveEvent::Stop); } else { - // TODO packet slave sts + pw_log::error!( + "i2c slave pkt unhandled raw=0x{:08x} sts=0x{:08x}", + status as u32, + sts as u32 + ); } } else { //byte irq let cmd: u32 = constants::AST_I2CS_ACTIVE_ALL; + let status = Self::normalized_slave_status(status); + pw_log::info!("i2c slave byte sts=0x{:08x}", status as u32); if status == constants::AST_I2CS_SLAVE_MATCH @@ -639,7 +741,21 @@ impl Ast1060I2c<'_, Y> { self.regs().i2cs24().write(|w| unsafe { w.bits(status) }); return Some(SlaveEvent::Stop); } - // TODO byte slave sts + // Benign transient sub-status edges that also raise an IRQ in + // packet/DMA mode (RX_DONE / WAIT_RX_DMA / WAIT_TX_DMA / TX_ACK + // latched mid-packet, with no MATCH/STOP). The real transaction is + // reported via PKT_DONE, so ignore these silently instead of + // logging them as unhandled. Anything carrying MATCH or STOP that + // still falls through here is a genuine gap and is logged. + let benign = (status + & !(constants::AST_I2CS_RX_DONE + | constants::AST_I2CS_WAIT_RX_DMA + | constants::AST_I2CS_WAIT_TX_DMA + | constants::AST_I2CS_TX_ACK)) + == 0; + if !benign { + pw_log::error!("i2c slave byte unhandled sts=0x{:08x}", status as u32); + } } None } diff --git a/target/ast10x0/peripherals/i2c/target_adapter.rs b/target/ast10x0/peripherals/i2c/target_adapter.rs index bc6d8407..04993ef0 100644 --- a/target/ast10x0/peripherals/i2c/target_adapter.rs +++ b/target/ast10x0/peripherals/i2c/target_adapter.rs @@ -152,7 +152,7 @@ where }; match event { - SlaveEvent::AddressMatch => { + SlaveEvent::WriteRequest => { if !self.in_transaction { self.target.on_transaction_start(false); self.in_transaction = true; @@ -174,12 +174,8 @@ where } } - SlaveEvent::WriteRequest => { - // Master wants to write to us - nothing to do here, - // data will arrive in DataReceived - } - - SlaveEvent::DataReceived { len } => { + SlaveEvent::DataReceived { len } | SlaveEvent::DataReceivedStop { len } => { + let stop_after_data = matches!(event, SlaveEvent::DataReceivedStop { .. }); // Read data from hardware let read_len = self.i2c.slave_read(&mut self.rx_buffer)?; let actual_len = read_len.min(len); @@ -190,6 +186,22 @@ where .on_write(&self.rx_buffer[..actual_len]) .map_err(|_| I2cError::SlaveError)?; } + if stop_after_data { + self.target.on_stop(); + self.in_transaction = false; + } + } + + SlaveEvent::DataReceivedAndSent { rx_len, .. } => { + let read_len = self.i2c.slave_read(&mut self.rx_buffer)?; + let actual_len = read_len.min(rx_len); + + if actual_len > 0 { + self.target + .on_write(&self.rx_buffer[..actual_len]) + .map_err(|_| I2cError::SlaveError)?; + } + self.target.on_data_sent(); } SlaveEvent::DataSent { len: _ } => { diff --git a/target/ast10x0/peripherals/scu/pinctrl.rs b/target/ast10x0/peripherals/scu/pinctrl.rs index 7655161a..cf22612f 100644 --- a/target/ast10x0/peripherals/scu/pinctrl.rs +++ b/target/ast10x0/peripherals/scu/pinctrl.rs @@ -537,6 +537,11 @@ paste! { gen_pin_pairs!(SCU6B0, 0x6B0, 31); } +/// I2C0 pin group: SCL1/SDA1 mux selection on SCU414[28:29]. +/// +/// Corresponds to PAC peripheral I2c (controller 0, base 0x7e7b_0080). +pub const PINCTRL_I2C0: &[PinctrlPin] = &[PIN_SCU414_28, PIN_SCU414_29]; + /// I2C1 pin group: SCL2/SDA2 mux selection on SCU414[30:31]. /// /// The SVD names these EnblSCL2FnPin/EnblSDA2FnPin, but they correspond to diff --git a/target/ast10x0/pfr/README.md b/target/ast10x0/pfr/README.md index 0ee308c6..82f58eef 100644 --- a/target/ast10x0/pfr/README.md +++ b/target/ast10x0/pfr/README.md @@ -183,7 +183,7 @@ FIFO routing is *per transaction*, bracketed by `send_start` / `send_stop`: │ • on FIFO full → latch node notify_flag, return FifoFull │ get_msg(port, addr) (repeatable) - │ FIFO path → fifo[i].dequeue() // removes (pops) the oldest byte + │ FIFO path → fifo[i].dequeue() // pops oldest byte; empty FIFO returns 0 │ ▼ send_stop(port) @@ -210,10 +210,9 @@ When not in a FIFO transaction, `send_msg` runs the node policy: `get_msg` on this path simply reads the byte from the buffer. -> **Current limitation:** `notify_flag` on a node is *written* but the crate -> exposes no API to *read or consume* it, so notifications are presently -> unobservable from outside. Wiring up a consumer (e.g. `take_notify(port, addr)`) -> is a known follow-up. +`take_notify()` exposes the latched node notify bits as `(port, addr)` pairs, +so firmware can poll and clear notifications after a transaction. The current +PFR server does exactly that once per service loop. ### Direct helpers @@ -258,6 +257,10 @@ signals the client. The mailbox itself is then driven **serially** by one consumer (one IPC channel per bus); the IRQ never reentrantly touches controller state. Because of this: +* In the current AST10x0 PFR test server, the controller lives in the same + process that handles the slave IRQ, so a master transaction can be served + inline and then polled for notifications immediately after the drain. + * All mutators take `&mut self`; `mbx_en` is a plain `u8` (no `Cell`, no atomics, no lock). The type is intentionally `!Sync`, so any accidental attempt to share one controller across contexts is a **compile error** rather than a silent data @@ -320,13 +323,54 @@ ctrl.send_msg(0, 0x0D, 0x11)?; ctrl.send_msg(0, 0x0D, 0x22)?; assert_eq!(ctrl.get_msg(0, 0x0D)?, 0x11); // pops in order assert_eq!(ctrl.get_msg(0, 0x0D)?, 0x22); -assert_eq!(ctrl.get_msg(0, 0x0D), Err(SwmbxError::FifoEmpty)); +assert_eq!(ctrl.get_msg(0, 0x0D)?, 0x00); ctrl.send_stop(0)?; ``` --- -## 10. Build & test +## 10. Quick test + +Use bus `0`, slave address `0x38`. + +### Protect check + +```sh +iic write_byte i2c@7e7b0080 38 13 88 +iic read_byte i2c@7e7b0080 38 13 +# expected: 0x88 + +iic write_byte i2c@7e7b0080 38 12 88 +iic read_byte i2c@7e7b0080 38 12 +# expected: 0x0 +``` + +### FIFO check + +```sh +iic write_byte i2c@7e7b0080 38 d aa +iic write_byte i2c@7e7b0080 38 d bb +iic write_byte i2c@7e7b0080 38 d cc + +iic read_byte i2c@7e7b0080 38 d +iic read_byte i2c@7e7b0080 38 d +iic read_byte i2c@7e7b0080 38 d +# expected: 0xaa, 0xbb, 0xcc +``` + +### Slave-side output + +```text +[INF] PFR mailbox server ready on Bus 0, slave addr=0x38 +[INF] notify triggered port=0 addr=0x13 +[INF] notify triggered port=0 addr=0x0d +[INF] notify triggered port=0 addr=0x0d +[INF] notify triggered port=0 addr=0x0d +``` + +--- + +## 11. Build & test ```sh # Build the (target-only) library @@ -338,7 +382,7 @@ bazel test //target/ast10x0/pfr:swmbx_ctrl_host_test --- -## 11. Quick reference: glossary +## 12. Quick reference: glossary | Term | In code | Meaning | |-----------------|--------------------------------------|-------------------------------------------| @@ -352,7 +396,7 @@ bazel test //target/ast10x0/pfr:swmbx_ctrl_host_test --- -## 12. How SWMBX sits in Zephyr PFR +## 13. How SWMBX sits in Zephyr PFR ``` ASPEED Zephyr PFR (apps/aspeed-pfr) diff --git a/target/ast10x0/pfr/smbus_mailbox_client.rs b/target/ast10x0/pfr/smbus_mailbox_client.rs index 25ba25f5..f80c5553 100644 --- a/target/ast10x0/pfr/smbus_mailbox_client.rs +++ b/target/ast10x0/pfr/smbus_mailbox_client.rs @@ -1,7 +1,7 @@ // Licensed under the Apache-2.0 license // SPDX-License-Identifier: Apache-2.0 -use crate::swmbx_ctrl::{SwmbxCtrl, SwmbxError}; +use crate::swmbx_ctrl::{SwmbxCtrl, SwmbxError}; use i2c_api::seam::SevenBitAddress; use i2c_api::{SlaveEvent, Transport}; use i2c_client::{ClientError as I2cClientError, I2cClient}; @@ -98,7 +98,7 @@ impl I2cPfrSmbusClient { Ok(self.swmbx.swmbx_read(false, addr)?) } - /// Processes at most one pending I2C slave event. + /// Processes all currently pending I2C slave events. /// /// This mirrors the Zephyr SWMBX target callback flow: /// @@ -112,40 +112,51 @@ impl I2cPfrSmbusClient { /// - `Stop`: finalizes the transaction via `send_stop` and resets /// first-write state. /// - /// If no data is pending, this returns `Ok(())`. + /// Returns `Ok(true)` if at least one event was handled, `Ok(false)` if + /// the queue is empty (`NoData`). The method drains the queue fully so a + /// `DataReceived`/`Stop` burst cannot be split across two caller wake-ups. + /// That keeps `first_write`, `active_port`, and `read_cursor` consistent + /// for the next FIFO-backed access. /// /// # Errors /// Returns `I2cPfrClientError::I2c` for transport failures and propagates - /// `Swmbx`/source-resolution errors encountered while handling an event. - pub fn process_one_event(&mut self) -> Result<(), I2cPfrClientError> { + /// `Swmbx` errors encountered while handling an event. + pub fn process_one_event(&mut self) -> Result { let mut rx = [0u8; RX_BUFFER_SIZE]; - let event = match self.i2c.slave_receive(&mut rx) { - Ok(ev) => ev, - Err(I2cClientError::ServerError(i2c_api::I2cError::NoData)) => return Ok(()), - Err(e) => return Err(I2cPfrClientError::I2c(e)), - }; - - match event.kind { - SlaveEvent::DataReceived => { - self.handle_data_received(&rx[..event.data_len], event.source_address) + let mut handled = false; + + loop { + let event = match self.i2c.slave_receive(&mut rx) { + Ok(ev) => ev, + Err(I2cClientError::ServerError(i2c_api::I2cError::NoData)) => { + return Ok(handled); + } + Err(e) => return Err(I2cPfrClientError::I2c(e)), + }; + + handled = true; + match event.kind { + SlaveEvent::DataReceived => { + self.handle_data_received(&rx[..event.data_len], event.source_address)? + } + SlaveEvent::ReadRequest => self.handle_read_request()?, + SlaveEvent::Stop => { + self.swmbx.send_stop(self.active_port)?; + // The next write transaction starts with an offset byte. + self.first_write = true; + } + _ => {} } - SlaveEvent::ReadRequest => self.handle_read_request(), - SlaveEvent::Stop => { - self.swmbx.send_stop(self.active_port)?; - // The next write transaction starts with an offset byte. - self.first_write = true; - Ok(()) - } - _ => Ok(()), } } - fn resolve_source(&self, source_addr: Option) -> Result { + /// Resolve the originating port. A recognized MCTP source address selects + /// its port; a plain offset-addressed SMBus access (no source header) + /// defaults to port 0 so it is served, not dropped. + fn resolve_source(&self, source_addr: Option) -> Source { match source_addr { - Some(addr) if addr == self.sources.bmc => Ok(Source::Bmc), - Some(addr) if addr == self.sources.pch_cpu => Ok(Source::PchCpu), - Some(_) => Err(I2cPfrClientError::UnknownSource), - None => Err(I2cPfrClientError::UnknownSource), + Some(addr) if addr == self.sources.pch_cpu => Source::PchCpu, + _ => Source::Bmc, } } @@ -165,7 +176,7 @@ impl I2cPfrSmbusClient { return Ok(()); } - let source = self.resolve_source(source_addr)?; + let source = self.resolve_source(source_addr); self.active_port = source_to_port(source); // Mirrors Zephyr swmbx_target callback sequencing: diff --git a/target/ast10x0/pfr/swmbx_ctrl.rs b/target/ast10x0/pfr/swmbx_ctrl.rs index b5bb87dc..5992c1d7 100644 --- a/target/ast10x0/pfr/swmbx_ctrl.rs +++ b/target/ast10x0/pfr/swmbx_ctrl.rs @@ -222,6 +222,16 @@ impl SwmbxCtrl { // contract. We validate null/alignment once and cache the pointer. let buffer_base = unsafe { SharedRegion::::from_addr(buffer_base).base }; + // Zero the backing region. It is a device-memory mapping that is not + // cleared at power-on, so a read of a never-written offset would + // otherwise return SRAM garbage instead of 0. Matches the zero-init + // backing buffer assumed by aspeed-rust's swmbx. + for i in 0..buffer_size { + // SAFETY: same validity contract as the cached `buffer_base`; + // `i < buffer_size` keeps the write inside the mapped region. + unsafe { write_volatile(buffer_base.as_ptr().add(i), 0) }; + } + SwmbxCtrl { mbx_en: 0, node: [[SwmbxNode::default(); SWMBX_NODE_COUNT]; SWMBX_DEV_COUNT], @@ -265,6 +275,26 @@ impl SwmbxCtrl { Ok(()) } + /// Consume the next pending notification, if any. + /// + /// Scans every node for a latched `notify_flag` (set by a write to a + /// notify-enabled address while global notify is on), clears the first one + /// found, and returns its `(port, addr)`. Returns `None` when no + /// notification is pending. Mirrors the polled-flag model of aspeed-rust's + /// `poll_notify`; the caller drains it once per service loop. + pub fn take_notify(&mut self) -> Option<(usize, u8)> { + for port in 0..SWMBX_DEV_COUNT { + for addr in 0..SWMBX_NODE_COUNT { + let node = &mut self.node[port][addr]; + if node.notify_flag { + node.notify_flag = false; + return Some((port, addr as u8)); + } + } + } + None + } + /// Configures one FIFO endpoint mapping. /// /// If `enable` is `true`, this binds FIFO `index` to `addr`, applies the @@ -431,7 +461,7 @@ impl SwmbxCtrl { /// /// # Errors /// Returns [`SwmbxError::InvalidPort`] if `port` is out of range. - /// Returns [`SwmbxError::FifoEmpty`] when reading from an empty active FIFO. + /// Returns `0` when reading from an empty active FIFO. /// Returns [`SwmbxError::InvalidAddress`] for out-of-bounds flat-buffer /// reads relative to `buffer_size`. pub fn get_msg(&mut self, port: usize, addr: u8) -> Result { @@ -441,7 +471,11 @@ impl SwmbxCtrl { if self.mbx_fifo_execute[port] && (self.mbx_en & SWMBX_FIFO) != 0 { let fifo_index = self.mbx_fifo_idx[port] as usize; - return self.fifo[fifo_index].dequeue(); + return match self.fifo[fifo_index].dequeue() { + Ok(value) => Ok(value), + Err(SwmbxError::FifoEmpty) => Ok(0), + Err(err) => Err(err), + }; } self.read_in_region(addr) @@ -489,7 +523,11 @@ impl SwmbxCtrl { } else { let node = &mut self.node[port][addr as usize]; - if (node.enabled_flags & SWMBX_PROTECT) == 0 || (self.mbx_en & SWMBX_PROTECT) == 0 { + let protected = + (node.enabled_flags & SWMBX_PROTECT) != 0 && (self.mbx_en & SWMBX_PROTECT) != 0; + + if protected { + } else { write_to_buffer = true; } @@ -668,7 +706,7 @@ mod tests { assert_eq!(ctrl.get_msg(0, 0x0d).expect("get_msg #1 failed"), 0x11); assert_eq!(ctrl.get_msg(0, 0x0d).expect("get_msg #2 failed"), 0x22); - assert_eq!(ctrl.get_msg(0, 0x0d), Err(SwmbxError::FifoEmpty)); + assert_eq!(ctrl.get_msg(0, 0x0d).expect("empty fifo read failed"), 0); ctrl.send_stop(0).expect("send_stop failed"); assert!(!ctrl.mbx_fifo_execute[0]); diff --git a/target/ast10x0/tests/peripherals/i2c/i2c_irq/slave_target.rs b/target/ast10x0/tests/peripherals/i2c/i2c_irq/slave_target.rs index ea0852c1..f02fa4eb 100644 --- a/target/ast10x0/tests/peripherals/i2c/i2c_irq/slave_target.rs +++ b/target/ast10x0/tests/peripherals/i2c/i2c_irq/slave_target.rs @@ -83,7 +83,8 @@ fn run_slave() -> Result<(), &'static str> { .map_err(|_| "test 1: configure_slave failed")?; match wait_event(&mut slave, 50_000_000) { - Some(SlaveEvent::DataReceived { len }) => { + Some(SlaveEvent::DataReceived { len }) + | Some(SlaveEvent::DataReceivedStop { len }) => { if len != EXPECTED_WRITE.len() { pw_log::error!( "test 1: DataReceived len={} expected={}", @@ -136,6 +137,7 @@ fn run_slave() -> Result<(), &'static str> { match wait_event(&mut slave, 50_000_000) { Some(SlaveEvent::DataReceived { len: _ }) + | Some(SlaveEvent::DataReceivedStop { len: _ }) | Some(SlaveEvent::Stop) | Some(SlaveEvent::WriteRequest) => { pw_log::info!("Test 3 passed: data/stop/write-req observed"); diff --git a/target/ast10x0/tests/pfr/BUILD.bazel b/target/ast10x0/tests/pfr/BUILD.bazel index 5dc012fb..a7b2a574 100644 --- a/target/ast10x0/tests/pfr/BUILD.bazel +++ b/target/ast10x0/tests/pfr/BUILD.bazel @@ -61,9 +61,10 @@ rust_app( tags = ["kernel"], target_compatible_with = TARGET_COMPATIBLE_WITH, deps = [ - "//services/i2c/server-runtime:i2c_server_runtime", + "//services/i2c/api:i2c_api", "//target/ast10x0/backend/i2c:i2c_backend_ast10x0", "//target/ast10x0/peripherals", + "//target/ast10x0/pfr:pfr", "@pigweed//pw_kernel/userspace", "@pigweed//pw_log/rust:pw_log", ], diff --git a/target/ast10x0/tests/pfr/pfr_i2c_notify_client_main.rs b/target/ast10x0/tests/pfr/pfr_i2c_notify_client_main.rs index f0a401ca..e4138f2f 100644 --- a/target/ast10x0/tests/pfr/pfr_i2c_notify_client_main.rs +++ b/target/ast10x0/tests/pfr/pfr_i2c_notify_client_main.rs @@ -1,186 +1,24 @@ // Licensed under the Apache-2.0 license // SPDX-License-Identifier: Apache-2.0 -//! Bringup client app for PFR mailbox-over-I2C validation. -//! -//! This app wires the userspace I2C IPC client to the in-process SWMBX -//! controller bridge (`I2cPfrSmbusClient`) and runs the mailbox target flow -//! used by PFR bringup. -//! -//! Startup sequence: -//! 1. Bind to the I2C server channel (`handle::I2C`). -//! 2. Construct SWMBX over the platform mailbox MMIO region -//! (`SWMBX_BUF_BASE`, `SWMBX_NODE_COUNT`). -//! 3. Configure SWMBX behavior bits (PROTECT/NOTIFY/FIFO), FIFO mappings, and -//! per-port protection bitmaps. -//! 4. Enter I2C target mode at `SLAVE_ADDR` and enable notifications. -//! 5. Emit `MAILBOX_READY` to provide an explicit external readiness marker. -//! -//! Runtime behavior: -//! - Receives slave notifications from the I2C server as `Signals::USER`. -//! - Blocks on `Signals::USER` from the server channel. -//! - Drains one pending slave event per wake via `process_one_event()`. -//! - Drops unknown-source events with rate-limited warnings. -//! - Fails fast on setup/runtime errors that indicate bringup misconfiguration. +//! Retired. The PFR mailbox is now driven entirely by `i2c_server` (server_main), +//! which owns `SwmbxCtrl` and serves master reads synchronously inside the slave +//! IRQ wake — no IPC client is needed in the read path. This process is kept as +//! an idle placeholder so the system image layout is unchanged. #![no_main] #![no_std] use app_pfr_i2c_notify_client::handle; -use ast10x0_pfr::{ - I2cPfrClientError, I2cPfrSmbusClient, SourceAddressMap, SwmbxCtrl, - SWMBX_BUF_BASE, SWMBX_NODE_COUNT, - SWMBX_FIFO, SWMBX_FIFO_NOTIFY_STOP, SWMBX_NOTIFY, SWMBX_PROTECT, -}; -use i2c_client::I2cClient; -use i2c_client_ipc::IpcTransport; use userspace::entry; use userspace::syscall::{self, Signals}; use userspace::time::Instant; -const SLAVE_ADDR: u8 = 0x38; -const BMC_SOURCE_ADDR: u8 = 0x20; -const PCH_SOURCE_ADDR: u8 = 0x22; - -const UFM_WRITE_FIFO: u8 = 0x0d; -const UFM_READ_FIFO: u8 = 0x0e; -const SWMBX_WRITE_FIFO_SIZE: usize = 64; -const SWMBX_READ_FIFO_SIZE: usize = 128; -const BMC_UPDATE_INTENT: u8 = 0x13; - -macro_rules! fail { - ($msg:literal) => {{ - pw_log::error!($msg); - loop {} - }}; -} - #[entry] fn entry() { - let i2c = I2cClient::new(IpcTransport::new(handle::I2C)); - // SAFETY: SWMBX_BUF_BASE is the platform-defined SWMBX mailbox region. - let swmbx = unsafe { SwmbxCtrl::new_with_regions(SWMBX_NODE_COUNT, SWMBX_BUF_BASE) }; - let sources = SourceAddressMap { - bmc: BMC_SOURCE_ADDR, - pch_cpu: PCH_SOURCE_ADDR, - }; - let mut mailbox = I2cPfrSmbusClient::new(i2c, swmbx, sources); - - // Enable SWMBX policy engines used by the mailbox flow. - // PROTECT enforces access masks, NOTIFY latches mailbox events, FIFO enables - // FIFO-backed register semantics for selected addresses. - if mailbox - .controller_mut() - .enable_behavior(SWMBX_PROTECT | SWMBX_NOTIFY | SWMBX_FIFO, true) - .is_err() - { - fail!("enable_behavior failed"); - } - // Configure write FIFO (0x0d): host writes are queued and notify on STOP. - if mailbox - .controller_mut() - .update_fifo( - 0, - UFM_WRITE_FIFO, - SWMBX_WRITE_FIFO_SIZE, - SWMBX_FIFO_NOTIFY_STOP, - true, - ) - .is_err() - { - fail!("update_fifo write failed"); - } - // Configure read FIFO (0x0e): host reads drain queued data and notify on STOP. - if mailbox - .controller_mut() - .update_fifo( - 1, - UFM_READ_FIFO, - SWMBX_READ_FIFO_SIZE, - SWMBX_FIFO_NOTIFY_STOP, - true, - ) - .is_err() - { - fail!("update_fifo read failed"); - } - // Enable notification on update-intent mailbox register for BMC port. - if mailbox - .controller_mut() - .update_notify(0, BMC_UPDATE_INTENT, true) - .is_err() - { - fail!("update_notify failed"); - } - - let bmc_access_control: [u32; 8] = [ - 0xfff704ff, - 0xffffffff, - 0xffffffff, - 0xfffffff2, - 0xffffffff, - 0xffffffff, - 0x00000000, - 0x00000000, - ]; - let pch_access_control: [u32; 8] = [ - 0xfff884ff, - 0xffffffff, - 0xffffffff, - 0xfffffff5, - 0x00000000, - 0x00000000, - 0xffffffff, - 0xffffffff, - ]; - // Apply per-port protection bitmaps across the mailbox address space. - // A set bit marks an address protected for that port. - if mailbox - .controller_mut() - .apply_protect(0, &bmc_access_control, 0) - .is_err() - { - fail!("apply_protect BMC failed"); - } - if mailbox - .controller_mut() - .apply_protect(1, &pch_access_control, 0) - .is_err() - { - fail!("apply_protect PCH failed"); - } - - if mailbox.start(SLAVE_ADDR).is_err() { - fail!("mailbox start failed"); - } - - // Explicit readiness marker for external orchestration. - pw_log::info!("MAILBOX_READY addr=0x{:02x}", SLAVE_ADDR as u32); - - pw_log::info!( - "PFR bringup listening addr=0x{:02x}; waiting for target notifications", - SLAVE_ADDR as u32, - ); - - let mut unknown_source_events: u32 = 0; loop { - if syscall::object_wait(handle::I2C, Signals::USER, Instant::MAX).is_err() { - fail!("object_wait USER failed"); - } - - match mailbox.process_one_event() { - Ok(()) => {} - Err(I2cPfrClientError::UnknownSource) => { - unknown_source_events = unknown_source_events.wrapping_add(1); - if unknown_source_events == 1 || (unknown_source_events % 32) == 0 { - pw_log::warn!( - "dropping event with unknown source (count={})", - unknown_source_events as u32 - ); - } - } - Err(_) => fail!("process_one_event failed"), - } + // Park forever; nothing to do. + let _ = syscall::object_wait(handle::I2C, Signals::USER, Instant::MAX); } } diff --git a/target/ast10x0/tests/pfr/server_main.rs b/target/ast10x0/tests/pfr/server_main.rs index 62f09848..0dff07c2 100644 --- a/target/ast10x0/tests/pfr/server_main.rs +++ b/target/ast10x0/tests/pfr/server_main.rs @@ -1,53 +1,361 @@ // Licensed under the Apache-2.0 license // SPDX-License-Identifier: Apache-2.0 +//! PFR software-mailbox server (single process, synchronous read serving). +//! +//! Unlike the generic i2c server-runtime + IPC client split, this app owns the +//! `SwmbxCtrl` mailbox state *in the same process that handles the slave IRQ*. +//! A master read is served inside the IRQ wake — `get_msg` + `write_slave_response` +//! run before the AST controller releases its TX clock-stretch — so there is no +//! cross-process IPC round-trip in the read path. This mirrors aspeed-rust's +//! in-ISR `swmbx_target` callback model and fixes the FIFO/back-to-back read +//! failures the IPC design suffered from. + #![no_main] #![no_std] use app_i2c_server::{handle, signals}; use ast10x0_peripherals::i2c::{ClockConfig, I2cConfig, I2cSpeed, I2cXferMode}; -use i2c_server_runtime::{run, Bus}; +use ast10x0_pfr::{ + SwmbxCtrl, SWMBX_BUF_BASE, SWMBX_FIFO, SWMBX_FIFO_NOTIFY_STOP, SWMBX_NODE_COUNT, SWMBX_NOTIFY, + SWMBX_PROTECT, +}; +use i2c_api::seam::{I2cIsrEvent, I2cSlaveBuffer, I2cSlaveCore, I2cSlaveEvent}; use userspace::entry; +use userspace::syscall; +use userspace::time::Duration; + +const SLAVE_ADDR: u8 = 0x38; +const RX_BUFFER_SIZE: usize = 256; + +const UFM_WRITE_FIFO: u8 = 0x0d; +const UFM_READ_FIFO: u8 = 0x0e; +const SWMBX_WRITE_FIFO_SIZE: usize = 64; +const SWMBX_READ_FIFO_SIZE: usize = 128; +const BMC_UPDATE_INTENT: u8 = 0x13; +const IRQ_POLL_MS: i64 = 1; + +// Per-port write-protection bitmaps: 8 packed 32-bit words = 256 mailbox nodes. +// Bit (word*32 + n) set => offset n is write-protected when global PROTECT is +// on. Values mirror aspeed-rust's swmbx access-control tables. +const BMC_ACCESS_CONTROL: [u32; 8] = [ + 0xfff7_04ff, 0xffff_ffff, 0xffff_ffff, 0xffff_fff2, 0xffff_ffff, 0xffff_ffff, 0x0000_0000, + 0x0000_0000, +]; +const PCH_ACCESS_CONTROL: [u32; 8] = [ + 0xfff8_84ff, 0xffff_ffff, 0xffff_ffff, 0xffff_fff5, 0x0000_0000, 0x0000_0000, 0xffff_ffff, + 0xffff_ffff, +]; const SLAVE_CFG: I2cConfig = I2cConfig { speed: I2cSpeed::Standard, xfer_mode: I2cXferMode::DmaMode, - multi_master: false, - smbus_timeout: false, + multi_master: true, + smbus_timeout: true, smbus_alert: false, clock_config: ClockConfig::ast1060_default(), }; +/// 16-byte-aligned DMA backing store. The AST1060 slave/master DMA +/// base-address registers ignore the low address bits (16-byte granularity); +/// an unaligned buffer makes the engine fetch/store at the aligned-down +/// address, so CPU and DMA see different memory. RX may survive by luck, but +/// slave TX then clocks out stale/idle bytes and the master reads 0xff. +#[repr(C, align(16))] +struct DmaBuf([u8; N]); + #[unsafe(link_section = ".ram_nc")] -static mut MASTER_DMA_BUF: [u8; 4096] = [0u8; 4096]; +static mut MASTER_DMA_BUF: DmaBuf<4096> = DmaBuf([0u8; 4096]); #[unsafe(link_section = ".ram_nc")] -static mut SLAVE_DMA_BUF: [u8; 256] = [0u8; 256]; +static mut SLAVE_DMA_BUF: DmaBuf<256> = DmaBuf([0u8; 256]); + +macro_rules! fail { + ($($arg:tt)*) => {{ + pw_log::error!($($arg)*); + loop {} + }}; +} + +#[allow(dead_code)] +fn isr_event_code(kind: I2cIsrEvent) -> u32 { + match kind { + I2cIsrEvent::SlaveRdReq => 1, + I2cIsrEvent::SlaveWrReq => 2, + I2cIsrEvent::SlaveRdProc => 3, + I2cIsrEvent::SlaveWrRecvd => 4, + I2cIsrEvent::SlaveWrRecvdStop => 5, + I2cIsrEvent::SlaveStop => 6, + } +} + +/// Per-transaction mailbox cursor (mirrors aspeed `swmbx_target` state). +struct MailboxState { + swmbx: SwmbxCtrl, + port: usize, + cursor: u8, + first_write: bool, +} + +impl MailboxState { + /// Apply a received RX burst: first byte is the offset (opens the FIFO + /// transaction), following bytes are written at the wrapping cursor. + fn on_write(&mut self, data: &[u8]) { + for &byte in data { + if self.first_write { + self.cursor = byte; + // Hot path: no logging between offset receipt and the TX-DMA + // arm in write_slave_response. Blocking UART here delays the + // arm past the master's read window (master samples 0xff). + let _ = self.swmbx.send_start(self.port, self.cursor); + self.first_write = false; + } else { + if let Err(e) = self.swmbx.send_msg(self.port, self.cursor, byte) { + pw_log::error!("MBX write err=0x{:04x}", e.code() as u32); + } + self.cursor = self.cursor.wrapping_add(1); + } + } + } + + /// Produce the next byte the master is reading. + fn on_read(&mut self) -> u8 { + let byte = match self.swmbx.get_msg(self.port, self.cursor) { + Ok(byte) => byte, + Err(e) => { + pw_log::error!("MBX read err=0x{:04x}", e.code() as u32); + 0 + } + }; + self.cursor = self.cursor.wrapping_add(1); + byte + } + + fn on_stop(&mut self) { + let _ = self.swmbx.send_stop(self.port); + self.first_write = true; + } +} + +fn drain_slave_events( + driver: &mut i2c_backend::BusDriver, + mbx: &mut MailboxState, + rx: &mut [u8; RX_BUFFER_SIZE], +) { + let mut pending_stop = false; + loop { + match driver.try_next_slave_event() { + Ok(Some((kind, len))) => { + let _ = len; + // HOT PATH — no logging from here until the TX-DMA arm in + // write_slave_response. Each blocking-UART line added ~ms of + // delay before the slave drove SDA, so the master sampled an + // idle bus (0xff) before the byte was clocked out. + match kind { + I2cIsrEvent::SlaveWrRecvd | I2cIsrEvent::SlaveWrRecvdStop => { + let stop_after_data = kind == I2cIsrEvent::SlaveWrRecvdStop; + let n = match driver.read_slave_buffer(rx) { + Ok(n) => n, + Err(_) => { + pw_log::error!("read_slave_buffer failed"); + 0 + } + }; + let wtx = driver.slave_waiting_for_tx(); + if n > 0 { + mbx.on_write(&rx[..n]); + } + if n == 1 || wtx { + let byte = mbx.on_read(); + let _ = driver.write_slave_response(&[byte]); + } + if stop_after_data { + pending_stop = true; + } + } + I2cIsrEvent::SlaveRdReq | I2cIsrEvent::SlaveRdProc => { + let byte = mbx.on_read(); + let _ = driver.write_slave_response(&[byte]); + } + I2cIsrEvent::SlaveStop => { + mbx.on_stop(); + pending_stop = false; + } + I2cIsrEvent::SlaveWrReq => { + mbx.first_write = true; + pw_log::info!("EV WrReq"); + } + } + } + Ok(None) => break, + Err(_) => { + let (_, _, s24) = driver.dbg_slave_regs(); + pw_log::error!("try_next_slave_event failed s24=0x{:08x}", s24 as u32); + break; + } + } + } + + // After the events are drained (TX already armed, so not on the hot path), + // report any latched mailbox notifications. Fires only when a write hit a + // notify-enabled address; mirrors aspeed-rust's `[NOTIFY] …` poll. + while let Some((port, addr)) = mbx.swmbx.take_notify() { + pw_log::info!("notify triggered port={} addr=0x{:02x}", port as u32, addr as u32); + } + + if pending_stop { + mbx.on_stop(); + } +} #[entry] fn entry() { - // SAFETY: server process exclusively owns Bus 2. - if unsafe { i2c_backend::init_bus(2, &SLAVE_CFG) }.is_err() { - pw_log::error!("init_bus(2) failed"); - loop {} + // SAFETY: server process exclusively owns Bus 0 (I2C0 @ 0x7e7b0080), the + // controller physically cross-wired to the master on this harness. + if unsafe { i2c_backend::init_bus(0, &SLAVE_CFG) }.is_err() { + fail!("init_bus(0) failed"); } // SAFETY: buffers are non-cached SRAM and uniquely owned for bus 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 driver = - match unsafe { i2c_backend::open_bus_dma(2, &SLAVE_CFG, master_dma_buf, slave_dma_buf) } { + unsafe { &mut (*core::ptr::addr_of_mut!(MASTER_DMA_BUF)).0 }; + let slave_dma_buf: &'static mut [u8] = + unsafe { &mut (*core::ptr::addr_of_mut!(SLAVE_DMA_BUF)).0 }; + let mut driver = + match unsafe { i2c_backend::open_bus_dma(0, &SLAVE_CFG, master_dma_buf, slave_dma_buf) } { Ok(d) => d, - Err(_) => { - pw_log::error!("open_bus_dma(2) failed"); - loop {} + Err(_) => fail!("open_bus_dma(0) failed"), + }; + + // SAFETY: SWMBX_BUF_BASE is the platform mailbox SRAM region, mapped to this + // process (see system.json5 `swmbx_buf`). + let swmbx = unsafe { SwmbxCtrl::new_with_regions(SWMBX_NODE_COUNT, SWMBX_BUF_BASE) }; + let mut mbx = MailboxState { + swmbx, + port: 0, + cursor: 0, + first_write: true, + }; + + // Mailbox policy: protect + notify + FIFO behaviors, two FIFO endpoints. + if mbx + .swmbx + .enable_behavior(SWMBX_PROTECT | SWMBX_NOTIFY | SWMBX_FIFO, true) + .is_err() + { + fail!("enable_behavior failed"); + } + if mbx + .swmbx + .update_notify(0, UFM_WRITE_FIFO, true) + .is_err() + { + fail!("update_notify fifo write failed"); + } + if mbx + .swmbx + .update_fifo(0, UFM_WRITE_FIFO, SWMBX_WRITE_FIFO_SIZE, SWMBX_FIFO_NOTIFY_STOP, true) + .is_err() + { + fail!("update_fifo write failed"); + } + if mbx + .swmbx + .update_fifo(1, UFM_READ_FIFO, SWMBX_READ_FIFO_SIZE, SWMBX_FIFO_NOTIFY_STOP, true) + .is_err() + { + fail!("update_fifo read failed"); + } + if mbx.swmbx.update_notify(0, BMC_UPDATE_INTENT, true).is_err() { + fail!("update_notify failed"); + } + + // Per-port write-protection bitmaps (packed 32-bit words, 8 words = 256 + // nodes/port). A 1 bit marks that mailbox offset write-protected once global + // PROTECT is enabled. Mirrors aspeed-rust's swmbx access-control tables + // (port 0 = BMC, port 1 = PCH). + pw_log::info!("apply_protect port=0 start_idx=0 words={}", BMC_ACCESS_CONTROL.len() as u32); + if mbx.swmbx.apply_protect(0, &BMC_ACCESS_CONTROL, 0).is_err() { + fail!("apply_protect BMC failed"); + } + pw_log::info!("apply_protect port=0 done"); + pw_log::info!("apply_protect port=1 start_idx=0 words={}", PCH_ACCESS_CONTROL.len() as u32); + if mbx.swmbx.apply_protect(1, &PCH_ACCESS_CONTROL, 0).is_err() { + fail!("apply_protect PCH failed"); + } + pw_log::info!("apply_protect port=1 done"); + + // Register the IRQ with the wait group BEFORE arming the controller's slave + // interrupts, so the kernel is ready to deliver the first edge. (The old + // server-runtime design registered the IRQ first and was armed later over + // IPC; arming the HW before registration loses the first interrupt.) + if syscall::wait_group_add( + handle::WG, + handle::I2C0_IRQ, + signals::I2C0, + handle::I2C0_IRQ as usize, + ) + .is_err() + { + fail!("wait_group_add irq failed"); + } + + // Enter slave/target mode at SLAVE_ADDR and arm the slave IRQ. + if driver.configure_slave_address(SLAVE_ADDR).is_err() { + fail!("configure_slave_address failed"); + } + if driver.enable_slave_mode().is_err() { + fail!("enable_slave_mode failed"); + } + + let (s20, c00, s24) = driver.dbg_slave_regs(); + pw_log::info!( + "slave regs: i2cs20(irq_en)=0x{:08x} i2cc00(ctrl)=0x{:08x} i2cs24(sts)=0x{:08x}", + s20 as u32, + c00 as u32, + s24 as u32 + ); + // Unmask the controller IRQ at the kernel: NVIC 110 starts masked after + // registration; the first interrupt_ack arms delivery of controller-driven + // interrupts (debug_trigger bypassed the mask, hence it woke but real slave + // activity did not). + let _ = syscall::interrupt_ack(handle::I2C0_IRQ, signals::I2C0); + let (s20, c00, s24) = driver.dbg_slave_regs(); + pw_log::info!( + "after initial irq_ack: i2cs20=0x{:08x} i2cc00=0x{:08x} i2cs24=0x{:08x}", + s20 as u32, + c00 as u32, + s24 as u32 + ); + + pw_log::info!("PFR mailbox server ready on Bus 0, slave addr=0x{:02x}", SLAVE_ADDR as u32); + + let mut rx = [0u8; RX_BUFFER_SIZE]; + loop { + let deadline = syscall::debug_clock_now() + Duration::from_millis(IRQ_POLL_MS); + let Ok(w) = syscall::object_wait(handle::WG, signals::I2C0, deadline) else { + // Poll fallback: drain if the controller latched anything while we + // were not woken. No logging on this path — blocking UART here slows + // servicing enough that the hardware batches multiple bus phases + // into one PKT_DONE status. + let (_, _, s24) = driver.dbg_slave_regs(); + if s24 != 0 { + drain_slave_events(&mut driver, &mut mbx, &mut rx); + let _ = syscall::interrupt_ack(handle::I2C0_IRQ, signals::I2C0); } + continue; }; + if !w.pending_signals.contains(signals::I2C0) { + continue; + } + let acked = w.pending_signals & signals::I2C0; - pw_log::info!("I2C server ready on Bus 2"); + // Drain every event the controller has latched this IRQ. + drain_slave_events(&mut driver, &mut mbx, &mut rx); - let mut buses = [Bus::new(handle::I2C, handle::I2C2_IRQ, driver)]; - run(handle::WG, signals::I2C2, &mut buses); + if syscall::interrupt_ack(handle::I2C0_IRQ, acked).is_err() { + fail!("interrupt_ack failed"); + } + } } #[panic_handler] diff --git a/target/ast10x0/tests/pfr/system.json5 b/target/ast10x0/tests/pfr/system.json5 index 5daeb1b8..74a27b5b 100644 --- a/target/ast10x0/tests/pfr/system.json5 +++ b/target/ast10x0/tests/pfr/system.json5 @@ -14,14 +14,14 @@ }, kernel: { flash_start_address: 0x00000800, - flash_size_bytes: 129024, + flash_size_bytes: 63488, ram_start_address: 0x00060000, ram_size_bytes: 131072, }, apps: [ { name: "i2c_server", - flash_size_bytes: 131072, + flash_size_bytes: 16384, processes: [ { name: "i2c_server_process", @@ -30,9 +30,9 @@ { name: "wg", type: "wait_group" }, { name: "i2c", type: "channel_handler" }, { - name: "i2c2_irq", + name: "i2c0_irq", type: "interrupt", - irqs: [{ name: "i2c2", number: 112 }], + irqs: [{ name: "i2c0", number: 110 }], }, { type: "thread", @@ -53,7 +53,7 @@ }, { name: "pfr_i2c_notify_client", - flash_size_bytes: 131072, + flash_size_bytes: 16384, processes: [ { name: "pfr_i2c_notify_client_process", @@ -71,6 +71,14 @@ kernel_stack_size_bytes: 2048, }, ], + memory_mappings: [ + { + name: "swmbx_buf", + type: "device", + start_address: 0x7e7b0e00, + size_bytes: 0x100, + }, + ], }, ], }, diff --git a/target/ast10x0/tests/pfr/target.rs b/target/ast10x0/tests/pfr/target.rs index 2356aa68..e4b1ea0b 100644 --- a/target/ast10x0/tests/pfr/target.rs +++ b/target/ast10x0/tests/pfr/target.rs @@ -28,7 +28,7 @@ impl TargetInterface for Target { for _ in 0..10_000u32 { core::hint::spin_loop(); } - scu.apply_pinctrl_group(pinctrl::PINCTRL_I2C2); + scu.apply_pinctrl_group(pinctrl::PINCTRL_I2C0); i2c::init_i2c_global(); } From 8401f3d86b5c87c2b850a173b2d70272042c1a0e Mon Sep 17 00:00:00 2001 From: Steven Lee Date: Fri, 12 Jun 2026 16:51:56 +0800 Subject: [PATCH 3/3] swmbx_ctrl: Fix FIFO queue depth and overflow notify - Add FIFO_QUEUE_SLOTS = SWMBX_FIFO_DEPTH + 1: heapless spsc::Queue stores at most N-1 elements, so a FIFO at max depth was one slot short. - Remove msg_index: dead manual cursor shadowing the queue's own head/tail. - Fix enable_behavior validation: reject flag == 0 and unknown bits; prior check accepted KNOWN | UNKNOWN combinations silently. - Gate FIFO overflow notify on SWMBX_NOTIFY (global + per-node) and latch on the FIFO's bound address, not the caller's running cursor. Signed-off-by: Steven Lee --- target/ast10x0/pfr/README.md | 20 ++++--- target/ast10x0/pfr/swmbx_ctrl.rs | 90 ++++++++++++++++++++++++++------ 2 files changed, 87 insertions(+), 23 deletions(-) diff --git a/target/ast10x0/pfr/README.md b/target/ast10x0/pfr/README.md index 82f58eef..3036bd59 100644 --- a/target/ast10x0/pfr/README.md +++ b/target/ast10x0/pfr/README.md @@ -53,7 +53,7 @@ shared memory is reached through a caller-supplied buffer address (see §7). SwmbxCtrl ├── mbx_en: u8 // GLOBAL feature switches ├── node: [[SwmbxNode; 256]; 2] // per-port, per-address policy -├── fifo: [SwmbxFifo<256>; 4] // the four FIFO endpoints +├── fifo: [SwmbxFifo<257>; 4] // the four FIFO endpoints (N = depth + 1, see below) ├── mbx_fifo_execute: [bool; 2] // per-port: in a FIFO transaction? ├── mbx_fifo_addr: [u8; 2] // per-port: address that opened it ├── mbx_fifo_idx: [u8; 2] // per-port: which FIFO is active @@ -97,11 +97,15 @@ struct SwmbxFifo { fifo_write: bool, // runtime: was anything written this txn? fifo_offset: u8, // the address this FIFO is bound to enabled: bool, // is this FIFO active? - msg_index: usize, // write cursor - max_msg_count: usize, // effective depth (<= N) + max_msg_count: usize, // effective depth (< N) } ``` +> The queue is sized `N = SWMBX_FIFO_DEPTH + 1` (`FIFO_QUEUE_SLOTS`) because +> heapless's `spsc::Queue` stores at most `N - 1` elements; the extra +> slot is what lets a FIFO configured at the maximum depth actually hold +> `SWMBX_FIFO_DEPTH` messages. + > Note the name collision: `SwmbxNode::notify_flag` is a **bool event latch**, > while `SwmbxFifo::notify_flag` is a **u8 configuration mask** of > `SWMBX_FIFO_NOTIFY_START (1<<0)` / `SWMBX_FIFO_NOTIFY_STOP (1<<1)`. They are @@ -180,7 +184,9 @@ FIFO routing is *per transaction*, bracketed by `send_start` / `send_stop`: │ • on first write, if NOTIFY armed + node NOTIFY set + │ fifo START-notify configured → latch node notify_flag, │ mark notify_start so it fires only once - │ • on FIFO full → latch node notify_flag, return FifoFull + │ • on FIFO full → return FifoFull; if NOTIFY armed + the + │ FIFO's bound node has NOTIFY set, latch that node's + │ notify_flag as an overflow event │ get_msg(port, addr) (repeatable) │ FIFO path → fifo[i].dequeue() // pops oldest byte; empty FIFO returns 0 @@ -283,7 +289,7 @@ state. Because of this: | `InvalidAddress` | `0x1002` | address outside the buffer / node range | | `InvalidFifoIndex` | `0x1003` | `index >= SWMBX_FIFO_COUNT` | | `InvalidFifoDepth` | `0x1004` | configured depth is 0 or `> SWMBX_FIFO_DEPTH` | -| `InvalidFlagMask` | `0x1005` | `enable_behavior` flag has no known bits | +| `InvalidFlagMask` | `0x1005` | `enable_behavior` flag is zero or has unknown bits | | `InvalidProtectRange` | `0x1006` | `apply_protect` slice out of range / overflows | | `FifoFull` | `0x1007` | append to a full FIFO | | `FifoEmpty` | `0x1008` | read from an empty FIFO | @@ -303,7 +309,7 @@ let mut backing = [0u8; BUF_LEN]; // SAFETY: `backing` is live, uniquely-owned memory for the controller's life. let mut ctrl = unsafe { - SwmbxCtrl::new_with_regions(BUF_LEN, backing.as_mut_ptr() as usize, 0) + SwmbxCtrl::new_with_regions(BUF_LEN, backing.as_mut_ptr() as usize) }; // --- Flat register with write-protect + notify ------------------------------- @@ -374,7 +380,7 @@ iic read_byte i2c@7e7b0080 38 d ```sh # Build the (target-only) library -bazel build //target/ast10x0/pfr:pfr +bazel build --config=k_ast1060_evb //target/ast10x0/pfr:pfr # Run the host unit tests bazel test //target/ast10x0/pfr:swmbx_ctrl_host_test diff --git a/target/ast10x0/pfr/swmbx_ctrl.rs b/target/ast10x0/pfr/swmbx_ctrl.rs index 5992c1d7..3a4547cc 100644 --- a/target/ast10x0/pfr/swmbx_ctrl.rs +++ b/target/ast10x0/pfr/swmbx_ctrl.rs @@ -11,6 +11,11 @@ pub const SWMBX_FIFO_COUNT: usize = 4; pub const SWMBX_FIFO_DEPTH: usize = 256; pub const SWMBX_BUF_BASE: usize = 0x7e7b_0e00; +/// heapless `spsc::Queue` stores at most `N - 1` elements, so the +/// backing queue needs one slot more than the deepest configurable FIFO for a +/// depth of [`SWMBX_FIFO_DEPTH`] to actually hold that many messages. +const FIFO_QUEUE_SLOTS: usize = SWMBX_FIFO_DEPTH + 1; + const _: () = assert!(SWMBX_NODE_COUNT == 256); const _: () = assert!(SWMBX_NODE_COUNT == (u8::MAX as usize) + 1); @@ -102,7 +107,6 @@ struct SwmbxFifo { fifo_write: bool, fifo_offset: u8, enabled: bool, - msg_index: usize, max_msg_count: usize, } @@ -115,7 +119,6 @@ impl SwmbxFifo { fifo_write: false, fifo_offset: 0, enabled: false, - msg_index: 0, max_msg_count: SWMBX_FIFO_DEPTH, } } @@ -129,11 +132,6 @@ impl SwmbxFifo { self.queue .enqueue(SwmbxFifoEntry { value }) .map_err(|_| SwmbxError::FifoFull)?; - if self.msg_index == (self.max_msg_count - 1) { - self.msg_index = 0; - } else { - self.msg_index = (self.msg_index + 1) % self.max_msg_count; - } Ok(()) } else { Err(SwmbxError::FifoFull) @@ -151,7 +149,6 @@ impl SwmbxFifo { pub fn flush(&mut self) { while self.queue.dequeue().is_some() {} - self.msg_index = 0; self.notify_start = false; self.fifo_write = false; } @@ -174,7 +171,7 @@ pub struct SwmbxCtrl { // type `!Sync`, so accidental cross-context sharing is a compile error. mbx_en: u8, node: [[SwmbxNode; SWMBX_NODE_COUNT]; SWMBX_DEV_COUNT], - fifo: [SwmbxFifo; SWMBX_FIFO_COUNT], + fifo: [SwmbxFifo; SWMBX_FIFO_COUNT], mbx_fifo_execute: [bool; SWMBX_DEV_COUNT], mbx_fifo_addr: [u8; SWMBX_DEV_COUNT], mbx_fifo_idx: [u8; SWMBX_DEV_COUNT], @@ -334,7 +331,6 @@ impl SwmbxCtrl { fifo.fifo_offset = addr; fifo.max_msg_count = depth; fifo.notify_flag = notify; - fifo.msg_index = 0; fifo.notify_start = false; fifo.fifo_write = false; fifo.queue = Queue::new(); @@ -366,10 +362,10 @@ impl SwmbxCtrl { /// Enables or disables global SWMBX behavior flags. /// /// # Errors - /// Returns [`SwmbxError::InvalidFlagMask`] if `flag` contains no known - /// behavior bits ([`FLAG_MASK`]). + /// Returns [`SwmbxError::InvalidFlagMask`] if `flag` is zero or contains + /// bits outside the known behavior set ([`FLAG_MASK`]). pub fn enable_behavior(&mut self, flag: u8, enable: bool) -> Result<(), SwmbxError> { - if (flag & FLAG_MASK) == 0 { + if flag == 0 || (flag & !FLAG_MASK) != 0 { return Err(SwmbxError::InvalidFlagMask); } @@ -504,7 +500,14 @@ impl SwmbxCtrl { let fifo_index = self.mbx_fifo_idx[port] as usize; if let Err(err) = self.fifo[fifo_index].append_write(val) { - self.node[port][addr as usize].notify_flag = true; + // Overflow notification belongs to the FIFO's bound address + // (`addr` is the caller's running cursor, which has usually + // walked past the FIFO offset by the time the queue fills) and + // obeys the same notify gating as every other latch. + let node = &mut self.node[port][fifo_addr as usize]; + if (self.mbx_en & SWMBX_NOTIFY) != 0 && (node.enabled_flags & SWMBX_NOTIFY) != 0 { + node.notify_flag = true; + } return Err(err); } @@ -860,7 +863,7 @@ mod tests { } #[test] - fn fifo_full_returns_error_and_sets_node_notify() { + fn fifo_full_notify_is_gated_and_latches_on_fifo_addr() { let mut host_buffer = [0u8; SWMBX_NODE_COUNT]; // SAFETY: `buffer_base` is live, uniquely-owned host storage for the test. let mut ctrl = unsafe { @@ -873,12 +876,57 @@ mod tests { ctrl.update_fifo(0, addr, 1, 0, true) .expect("fifo setup failed"); + // Notify disabled (globally and per node): overflow errors but must not + // latch a spurious notification. ctrl.send_start(0, addr).expect("send_start failed"); ctrl.send_msg(0, addr, 0x11) .expect("first fifo write failed"); - assert_eq!(ctrl.send_msg(0, addr, 0x22), Err(SwmbxError::FifoFull)); + assert!(ctrl.take_notify().is_none()); + ctrl.send_stop(0).expect("send_stop failed"); + + // With notify enabled, the overflow latch lands on the FIFO's bound + // address even when the caller's cursor has walked past it. + ctrl.enable_behavior(SWMBX_NOTIFY, true) + .expect("enable notify failed"); + ctrl.update_notify(0, addr, true) + .expect("update_notify failed"); + ctrl.flush_fifo(0).expect("flush_fifo failed"); + + ctrl.send_start(0, addr).expect("send_start failed"); + ctrl.send_msg(0, addr, 0x11) + .expect("first fifo write failed"); + let cursor = addr.wrapping_add(1); + assert_eq!(ctrl.send_msg(0, cursor, 0x22), Err(SwmbxError::FifoFull)); assert!(ctrl.node[0][addr as usize].notify_flag); + assert!(!ctrl.node[0][cursor as usize].notify_flag); + } + + #[test] + fn fifo_at_max_depth_holds_exactly_that_many_messages() { + let mut host_buffer = [0u8; SWMBX_NODE_COUNT]; + // SAFETY: `buffer_base` is live, uniquely-owned host storage for the test. + let mut ctrl = unsafe { + SwmbxCtrl::new_with_regions(host_buffer.len(), host_buffer.as_mut_ptr() as usize) + }; + + let addr = 0x0d; + ctrl.enable_behavior(SWMBX_FIFO, true) + .expect("enable fifo failed"); + ctrl.update_fifo(0, addr, SWMBX_FIFO_DEPTH, 0, true) + .expect("fifo setup failed"); + + ctrl.send_start(0, addr).expect("send_start failed"); + for i in 0..SWMBX_FIFO_DEPTH { + ctrl.send_msg(0, addr, i as u8) + .unwrap_or_else(|e| panic!("write {} failed: {:?}", i, e)); + } + assert_eq!(ctrl.send_msg(0, addr, 0xee), Err(SwmbxError::FifoFull)); + + for i in 0..SWMBX_FIFO_DEPTH { + assert_eq!(ctrl.get_msg(0, addr).expect("fifo read failed"), i as u8); + } + assert_eq!(ctrl.get_msg(0, addr).expect("empty fifo read failed"), 0); } #[test] @@ -1103,6 +1151,16 @@ mod tests { ctrl.enable_behavior(0, true), Err(SwmbxError::InvalidFlagMask) ); + // Unknown bits are rejected even when combined with valid ones, so + // stray bits can never be stored into the global enable mask. + assert_eq!( + ctrl.enable_behavior(0x80, true), + Err(SwmbxError::InvalidFlagMask) + ); + assert_eq!( + ctrl.enable_behavior(SWMBX_PROTECT | 0x80, true), + Err(SwmbxError::InvalidFlagMask) + ); assert_eq!( ctrl.flush_fifo(SWMBX_FIFO_COUNT), Err(SwmbxError::InvalidFifoIndex)