diff --git a/Cargo.lock b/Cargo.lock index 99dd765a5..a68b25b1c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -116,7 +116,7 @@ dependencies = [ "embedded-batteries-async", "embedded-hal 1.0.0", "embedded-hal-async", - "embedded-services 0.1.0", + "embedded-services", "log", ] @@ -249,7 +249,7 @@ dependencies = [ "embassy-sync", "embassy-time", "embedded-cfu-protocol", - "embedded-services 0.1.0", + "embedded-services", "heapless 0.8.0", "log", ] @@ -480,7 +480,7 @@ checksum = "48c757948c5ede0e46177b7add2e67155f70e33c07fea8284df6576da70b3719" [[package]] name = "embassy-embedded-hal" version = "0.3.0" -source = "git+https://github.com/embassy-rs/embassy#4766cc6f9756b54bb2e25be5ba5276538ea4917b" +source = "git+https://github.com/embassy-rs/embassy#b528ed06e3025e0803e8fd6dc53ac968df9f49bc" dependencies = [ "embassy-futures", "embassy-hal-internal", @@ -497,7 +497,7 @@ dependencies = [ [[package]] name = "embassy-executor" version = "0.7.0" -source = "git+https://github.com/embassy-rs/embassy#141c170db426404444a454c063c2eec07c74a1c3" +source = "git+https://github.com/embassy-rs/embassy#b528ed06e3025e0803e8fd6dc53ac968df9f49bc" dependencies = [ "cortex-m", "critical-section", @@ -510,7 +510,7 @@ dependencies = [ [[package]] name = "embassy-executor-macros" version = "0.6.2" -source = "git+https://github.com/embassy-rs/embassy#141c170db426404444a454c063c2eec07c74a1c3" +source = "git+https://github.com/embassy-rs/embassy#b528ed06e3025e0803e8fd6dc53ac968df9f49bc" dependencies = [ "darling", "proc-macro2", @@ -521,7 +521,7 @@ dependencies = [ [[package]] name = "embassy-futures" version = "0.1.1" -source = "git+https://github.com/embassy-rs/embassy#4766cc6f9756b54bb2e25be5ba5276538ea4917b" +source = "git+https://github.com/embassy-rs/embassy#b528ed06e3025e0803e8fd6dc53ac968df9f49bc" dependencies = [ "defmt 1.0.1", "log", @@ -530,7 +530,7 @@ dependencies = [ [[package]] name = "embassy-hal-internal" version = "0.2.0" -source = "git+https://github.com/embassy-rs/embassy#4766cc6f9756b54bb2e25be5ba5276538ea4917b" +source = "git+https://github.com/embassy-rs/embassy#b528ed06e3025e0803e8fd6dc53ac968df9f49bc" dependencies = [ "cortex-m", "critical-section", @@ -541,7 +541,7 @@ dependencies = [ [[package]] name = "embassy-imxrt" version = "0.1.0" -source = "git+https://github.com/OpenDevicePartnership/embassy-imxrt#0cd2be9b9dc94b1dd50fb93edeee001c04313869" +source = "git+https://github.com/OpenDevicePartnership/embassy-imxrt#ce3db77e1364d9fdfee639ec94f4c7f3b3a870b1" dependencies = [ "cfg-if", "cortex-m", @@ -570,20 +570,20 @@ dependencies = [ "nb 1.1.0", "paste", "rand_core", - "storage_bus 0.1.1 (git+https://github.com/OpenDevicePartnership/embedded-services)", + "storage_bus 0.1.0", ] [[package]] name = "embassy-sync" version = "0.7.0" -source = "git+https://github.com/embassy-rs/embassy#141c170db426404444a454c063c2eec07c74a1c3" +source = "git+https://github.com/embassy-rs/embassy#b528ed06e3025e0803e8fd6dc53ac968df9f49bc" dependencies = [ "cfg-if", "critical-section", "defmt 1.0.1", "embedded-io-async", + "futures-core", "futures-sink", - "futures-util", "heapless 0.8.0", "log", ] @@ -591,7 +591,7 @@ dependencies = [ [[package]] name = "embassy-time" version = "0.4.0" -source = "git+https://github.com/embassy-rs/embassy#141c170db426404444a454c063c2eec07c74a1c3" +source = "git+https://github.com/embassy-rs/embassy#b528ed06e3025e0803e8fd6dc53ac968df9f49bc" dependencies = [ "cfg-if", "critical-section", @@ -602,14 +602,14 @@ dependencies = [ "embedded-hal 0.2.7", "embedded-hal 1.0.0", "embedded-hal-async", - "futures-util", + "futures-core", "log", ] [[package]] name = "embassy-time-driver" version = "0.2.0" -source = "git+https://github.com/embassy-rs/embassy#141c170db426404444a454c063c2eec07c74a1c3" +source = "git+https://github.com/embassy-rs/embassy#b528ed06e3025e0803e8fd6dc53ac968df9f49bc" dependencies = [ "document-features", ] @@ -617,7 +617,7 @@ dependencies = [ [[package]] name = "embassy-time-queue-utils" version = "0.1.0" -source = "git+https://github.com/embassy-rs/embassy#141c170db426404444a454c063c2eec07c74a1c3" +source = "git+https://github.com/embassy-rs/embassy#b528ed06e3025e0803e8fd6dc53ac968df9f49bc" dependencies = [ "embassy-executor", "heapless 0.8.0", @@ -647,7 +647,7 @@ dependencies = [ [[package]] name = "embedded-cfu-protocol" version = "0.1.0" -source = "git+https://github.com/OpenDevicePartnership/embedded-cfu#e2734a2bf953d584b11bbaa5931919345df9bf7c" +source = "git+https://github.com/OpenDevicePartnership/embedded-cfu#ffb9041a2ec9ca7a411f3439f45eec3bc01b975b" dependencies = [ "defmt 0.3.100", "embedded-io-async", @@ -744,39 +744,6 @@ dependencies = [ "tokio", ] -[[package]] -name = "embedded-services" -version = "0.1.0" -source = "git+https://github.com/OpenDevicePartnership/embedded-services#437a93233328685b57bd5a492a4263abd552f47d" -dependencies = [ - "bitfield 0.17.0", - "bitflags 2.9.0", - "bitvec", - "cfg-if", - "cortex-m", - "cortex-m-rt", - "critical-section", - "document-features", - "embassy-executor", - "embassy-futures", - "embassy-sync", - "embassy-time", - "embedded-batteries-async", - "embedded-cfu-protocol", - "embedded-hal-async", - "embedded-hal-nb", - "embedded-io", - "embedded-io-async", - "embedded-storage", - "embedded-storage-async", - "embedded-usb-pd", - "fixed", - "heapless 0.8.0", - "postcard", - "rand_core", - "serde", -] - [[package]] name = "embedded-storage" version = "0.3.1" @@ -795,7 +762,7 @@ dependencies = [ [[package]] name = "embedded-usb-pd" version = "0.1.0" -source = "git+https://github.com/OpenDevicePartnership/embedded-usb-pd#b8948c96b8c8e920c837307d30653cfd74cf80cd" +source = "git+https://github.com/OpenDevicePartnership/embedded-usb-pd#99232ecdfdd9f5aa93b1dedbfc22f11c8d12632c" dependencies = [ "bitfield 0.19.0", "defmt 0.3.100", @@ -828,7 +795,7 @@ dependencies = [ "embassy-imxrt", "embassy-sync", "embassy-time", - "embedded-services 0.1.0", + "embedded-services", "log", ] @@ -868,24 +835,6 @@ version = "0.3.31" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e575fab7d1e0dcb8d0c7bcf9a63ee213816ab51902e6d244a95819acacf1d4f7" -[[package]] -name = "futures-task" -version = "0.3.31" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f90f7dce0722e95104fcb095585910c0977252f286e354b5e3bd38902cd99988" - -[[package]] -name = "futures-util" -version = "0.3.31" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9fa08315bb612088cc391249efdc3bc77536f16c91f6cf495e6fbe85b20a4a81" -dependencies = [ - "futures-core", - "futures-task", - "pin-project-lite", - "pin-utils", -] - [[package]] name = "gimli" version = "0.31.1" @@ -977,7 +926,7 @@ dependencies = [ "embassy-time", "embedded-hal 1.0.0", "embedded-hal-async", - "embedded-services 0.1.0", + "embedded-services", "log", ] @@ -1177,12 +1126,6 @@ version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3b3cff922bd51709b605d9ead9aa71031d81447142d828eb4a6eba76fe619f9b" -[[package]] -name = "pin-utils" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" - [[package]] name = "platform-service" version = "0.1.0" @@ -1195,7 +1138,7 @@ dependencies = [ "embassy-sync", "embassy-time", "embedded-cfu-protocol", - "embedded-services 0.1.0", + "embedded-services", "heapless 0.8.0", "log", ] @@ -1231,7 +1174,7 @@ dependencies = [ "embassy-futures", "embassy-sync", "embassy-time", - "embedded-services 0.1.0", + "embedded-services", "log", ] @@ -1384,25 +1327,19 @@ checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" [[package]] name = "storage_bus" -version = "0.1.1" -dependencies = [ - "defmt 0.3.100", - "embassy-executor", - "embassy-sync", - "embassy-time", - "embedded-services 0.1.0", - "log", -] +version = "0.1.0" +source = "git+https://github.com/OpenDevicePartnership/embedded-mcu#004ccaa45d341a56073360b0764d2a538d46780d" [[package]] name = "storage_bus" version = "0.1.1" -source = "git+https://github.com/OpenDevicePartnership/embedded-services#437a93233328685b57bd5a492a4263abd552f47d" dependencies = [ + "defmt 0.3.100", "embassy-executor", "embassy-sync", "embassy-time", - "embedded-services 0.1.0 (git+https://github.com/OpenDevicePartnership/embedded-services)", + "embedded-services", + "log", ] [[package]] @@ -1539,7 +1476,7 @@ dependencies = [ "embedded-hal 1.0.0", "embedded-hal-async", "embedded-io-async", - "embedded-services 0.1.0", + "embedded-services", "embedded-usb-pd", "log", "tps6699x", diff --git a/battery-service/src/context.rs b/battery-service/src/context.rs index 36ca4c566..79a5c48f1 100644 --- a/battery-service/src/context.rs +++ b/battery-service/src/context.rs @@ -1,13 +1,14 @@ use crate::device::Device; use crate::device::{self, DeviceId}; use embassy_sync::channel::Channel; +use embassy_sync::channel::TrySendError; use embassy_sync::mutex::Mutex; -use embassy_sync::{blocking_mutex::raw::NoopRawMutex, channel::TrySendError}; use embassy_time::{with_timeout, Duration}; +use embedded_services::GlobalRawMutex; use embedded_services::{debug, error, info, intrusive_list, trace, warn, IntrusiveList}; -use core::cell::Cell; use core::ops::DerefMut; +use core::sync::atomic::AtomicUsize; /// Battery service states. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -104,10 +105,10 @@ pub struct BatteryEvent { /// Battery service context, hardware agnostic state. pub struct Context { fuel_gauges: IntrusiveList, - state: Mutex, - battery_event: Channel, - battery_response: Channel, - no_op_retry_count: Cell, + state: Mutex, + battery_event: Channel, + battery_response: Channel, + no_op_retry_count: AtomicUsize, config: Config, } @@ -133,7 +134,7 @@ impl Context { state: Mutex::new(State::NotPresent), battery_event: Channel::new(), battery_response: Channel::new(), - no_op_retry_count: Cell::new(0), + no_op_retry_count: AtomicUsize::new(0), config: Default::default(), } } @@ -144,7 +145,7 @@ impl Context { state: Mutex::new(State::NotPresent), battery_event: Channel::new(), battery_response: Channel::new(), - no_op_retry_count: Cell::new(0), + no_op_retry_count: AtomicUsize::new(0), config, } } @@ -161,12 +162,13 @@ impl Context { /// Get global state machine NotOperational retry count. fn get_state_machine_retry_count(&self) -> usize { - self.no_op_retry_count.get() + self.no_op_retry_count.load(core::sync::atomic::Ordering::Relaxed) } /// Set global state machine NotOperational retry count. fn set_state_machine_retry_count(&self, retry_count: usize) { - self.no_op_retry_count.set(retry_count) + self.no_op_retry_count + .store(retry_count, core::sync::atomic::Ordering::Relaxed) } /// Main processing function. @@ -185,7 +187,7 @@ impl Context { }, Err(_) => { error!("Battery state machine timeout!"); - // Should be infalliable + // Should be infallible self.do_state_machine(BatteryEvent { event: BatteryEventInner::Timeout, device_id: event.device_id, diff --git a/battery-service/src/device.rs b/battery-service/src/device.rs index 2d4ad3a6e..011f288dc 100644 --- a/battery-service/src/device.rs +++ b/battery-service/src/device.rs @@ -1,9 +1,8 @@ use core::cell::Cell; -use embassy_sync::blocking_mutex::raw::NoopRawMutex; use embassy_sync::channel::Channel; use embassy_time::Duration; -use embedded_services::{Node, NodeContainer}; +use embedded_services::{GlobalRawMutex, Node, NodeContainer}; #[derive(Debug, Clone, Copy)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] @@ -115,8 +114,8 @@ pub struct DeviceId(pub u8); pub struct Device { node: embedded_services::Node, id: DeviceId, - command: Channel, - response: Channel, + command: Channel, + response: Channel, dynamic_battery_cache: Cell, static_battery_cache: Cell, timeout: Cell, diff --git a/battery-service/src/wrapper.rs b/battery-service/src/wrapper.rs index 3d20b5325..5e8019d2c 100644 --- a/battery-service/src/wrapper.rs +++ b/battery-service/src/wrapper.rs @@ -1,7 +1,7 @@ -use core::cell::RefCell; - use embassy_futures::select::select; +use embassy_sync::mutex::Mutex; use embedded_services::trace; +use embedded_services::GlobalRawMutex; use crate::{ controller::{Controller, ControllerEvent}, @@ -11,7 +11,7 @@ use crate::{ /// Wrapper object to bind device to fuel gauge hardware driver. pub struct Wrapper<'a, C: Controller> { device: &'a Device, - controller: RefCell, + controller: Mutex, } impl<'a, C: Controller> Wrapper<'a, C> { @@ -22,14 +22,14 @@ impl<'a, C: Controller> Wrapper<'a, C> { Self { device, - controller: RefCell::new(controller), + controller: Mutex::new(controller), } } /// Process events from hardware controller or context device. - #[allow(clippy::await_holding_refcell_ref)] + /// Only call this fn ONCE, it will infinitely loop processing messages. Otherwise a deadlock could occur. pub async fn process(&self) { - let mut controller = self.controller.borrow_mut(); + let mut controller = self.controller.lock().await; loop { let res = select(controller.get_device_event(), self.device.receive_command()).await; match res { diff --git a/cfu-service/src/buffer.rs b/cfu-service/src/buffer.rs index 179a0ced8..49fe343ce 100644 --- a/cfu-service/src/buffer.rs +++ b/cfu-service/src/buffer.rs @@ -1,10 +1,13 @@ //! Module that can buffer CFU content //! This allows prompt responses to content requests even if the component is busy -use core::{cell::RefCell, future::pending}; +use core::future::pending; use embassy_futures::select::{select3, Either3}; -use embassy_sync::channel::{DynamicReceiver, DynamicSender}; +use embassy_sync::{ + channel::{DynamicReceiver, DynamicSender}, + mutex::Mutex, +}; use embassy_time::{with_timeout, Duration, TimeoutError}; use embedded_cfu_protocol::protocol_definitions::*; use embedded_services::{ @@ -12,7 +15,7 @@ use embedded_services::{ self, component::{CfuDevice, InternalResponseData, RequestData}, }, - error, intrusive_list, trace, + error, intrusive_list, trace, GlobalRawMutex, }; /// Internal state for [`Buffer`] @@ -50,7 +53,7 @@ pub struct Buffer<'a> { /// CFU device cfu_device: CfuDevice, /// Internal state - state: RefCell, + state: Mutex, /// Component ID to buffer requests for buffered_id: ComponentId, /// Sender for the buffer @@ -83,7 +86,7 @@ impl<'a> Buffer<'a> { ) -> Self { Self { cfu_device: CfuDevice::new(external_id), - state: RefCell::new(Default::default()), + state: Mutex::new(Default::default()), buffered_id, buffer_sender, buffer_receiver, @@ -228,7 +231,7 @@ impl<'a> Buffer<'a> { /// Wait for an event pub async fn wait_event(&self) -> Event { - let is_busy = self.state.borrow().component_busy; + let is_busy = self.state.lock().await.component_busy; match select3( // Wait for a buffered content request self.wait_buffered_content(is_busy), @@ -260,9 +263,8 @@ impl<'a> Buffer<'a> { } /// Top-level event processing function - #[allow(clippy::await_holding_refcell_ref)] pub async fn process(&self, event: Event) -> Option { - let mut state = self.state.borrow_mut(); + let mut state = self.state.lock().await; match event { Event::CfuRequest(request) => Some(self.process_request(&mut state, request).await), Event::BufferedContent(content) => { diff --git a/embedded-service/src/activity.rs b/embedded-service/src/activity.rs index aa32ea16d..3053003cc 100644 --- a/embedded-service/src/activity.rs +++ b/embedded-service/src/activity.rs @@ -2,7 +2,7 @@ use embassy_sync::once_lock::OnceLock; -use crate::{intrusive_list::*, sync_cell::SyncCell}; +use crate::{intrusive_list::*, SyncCell}; /// potential activity service states #[derive(Copy, Clone, Debug)] diff --git a/embedded-service/src/buffer.rs b/embedded-service/src/buffer.rs index 86cd974bb..3f1300b55 100644 --- a/embedded-service/src/buffer.rs +++ b/embedded-service/src/buffer.rs @@ -12,10 +12,12 @@ //! This allows for producer code to own the buffer through a `OwnedRef`, and then allow access to consumers //! through any number of `SharedRef`. use core::borrow::{Borrow, BorrowMut}; -use core::cell::Cell; use core::marker::PhantomData; use core::ops::Range; +use crate::SyncCell; +use core::sync::atomic::AtomicPtr; + #[derive(Copy, Clone, PartialEq, Eq)] enum Status { None, @@ -25,8 +27,9 @@ enum Status { /// Underlying buffer storage struct pub struct Buffer<'a, T> { - buffer: *mut [T], - status: Cell, + buffer: AtomicPtr, + len: usize, + status: SyncCell, _lifetime: PhantomData<&'a ()>, } @@ -36,8 +39,9 @@ impl<'a, T> Buffer<'a, T> { /// No other code should have access to the buffer pub unsafe fn new(raw_buffer: &'a mut [T]) -> Self { Buffer { - buffer: raw_buffer, - status: Cell::new(Status::None), + buffer: AtomicPtr::new(raw_buffer.as_mut_ptr()), + len: raw_buffer.len(), + status: SyncCell::new(Status::None), _lifetime: PhantomData, } } @@ -52,13 +56,13 @@ impl<'a, T> Buffer<'a, T> { /// Returns the length of the buffer // SAFETY: The buffer is always valid pub fn len(&self) -> usize { - unsafe { self.buffer.as_mut().unwrap().len() } + self.len } /// Returns `true`` if the buffer has a length of 0 // SAFETY: The buffer is always valid pub fn is_empty(&self) -> bool { - unsafe { self.buffer.as_mut().unwrap().is_empty() } + self.len == 0 } fn borrow(&self, mutable: bool) { @@ -129,14 +133,16 @@ impl<'a, T> AccessMut<'a, T> { // SAFETY: Access to the buffer is dynamically checked impl Borrow<[T]> for AccessMut<'_, T> { fn borrow(&self) -> &[T] { - unsafe { &*self.0.buffer } + unsafe { core::slice::from_raw_parts(self.0.buffer.load(core::sync::atomic::Ordering::Acquire), self.0.len) } } } // SAFETY: Access to the buffer is dynamically checked impl BorrowMut<[T]> for AccessMut<'_, T> { fn borrow_mut(&mut self) -> &mut [T] { - unsafe { &mut *self.0.buffer } + unsafe { + core::slice::from_raw_parts_mut(self.0.buffer.load(core::sync::atomic::Ordering::Acquire), self.0.len) + } } } @@ -203,7 +209,12 @@ impl<'a, T> Access<'a, T> { // SAFETY: Access to the buffer is dynamically checked impl Borrow<[T]> for Access<'_, T> { fn borrow(&self) -> &[T] { - let buffer = unsafe { &*self.buffer.buffer }; + let buffer = unsafe { + core::slice::from_raw_parts( + self.buffer.buffer.load(core::sync::atomic::Ordering::Acquire), + self.buffer.len, + ) + }; &buffer[self.slice.clone()] } } diff --git a/embedded-service/src/cfu/component.rs b/embedded-service/src/cfu/component.rs index eab81eaf6..91401b624 100644 --- a/embedded-service/src/cfu/component.rs +++ b/embedded-service/src/cfu/component.rs @@ -1,7 +1,6 @@ //! Device struct and methods for component communication use core::future::Future; -use embassy_sync::blocking_mutex::raw::NoopRawMutex; use embassy_sync::channel::Channel; use embassy_sync::mutex::Mutex; use embedded_cfu_protocol::components::{CfuComponentInfo, CfuComponentStorage, CfuComponentTraits}; @@ -12,6 +11,7 @@ use heapless::Vec; use super::CfuError; use crate::cfu::route_request; use crate::intrusive_list; +use crate::GlobalRawMutex; /// Component internal update state #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -101,9 +101,9 @@ pub const DEVICE_CHANNEL_SIZE: usize = 1; pub struct CfuDevice { node: intrusive_list::Node, component_id: ComponentId, - state: Mutex, - request: Channel, - response: Channel, + state: Mutex, + request: Channel, + response: Channel, } impl intrusive_list::NodeContainer for CfuDevice { @@ -179,7 +179,7 @@ pub struct CfuComponentDefault { is_primary: bool, storage_offset: usize, subcomponents: [Option; MAX_SUBCMPT_COUNT], - writer: Mutex, + writer: Mutex, } impl Default for CfuComponentDefault { diff --git a/embedded-service/src/cfu/mod.rs b/embedded-service/src/cfu/mod.rs index d0fc73159..36a46b989 100644 --- a/embedded-service/src/cfu/mod.rs +++ b/embedded-service/src/cfu/mod.rs @@ -4,7 +4,7 @@ pub mod component; use core::sync::atomic::{AtomicBool, Ordering}; -use embassy_sync::blocking_mutex::raw::NoopRawMutex; +use crate::GlobalRawMutex; use embassy_sync::channel::Channel; use embassy_sync::once_lock::OnceLock; use embedded_cfu_protocol::protocol_definitions::{CfuProtocolError, ComponentId}; @@ -41,9 +41,9 @@ struct ClientContext { /// Registered devices devices: intrusive_list::IntrusiveList, /// Request to components - request: Channel, + request: Channel, /// Response from components - response: Channel, + response: Channel, } impl ClientContext { diff --git a/embedded-service/src/comms.rs b/embedded-service/src/comms.rs index ce521808d..6015a585f 100644 --- a/embedded-service/src/comms.rs +++ b/embedded-service/src/comms.rs @@ -1,7 +1,6 @@ //! Comms Service Definitions use core::any::{Any, TypeId}; -use core::cell::Cell; use core::convert::Infallible; use embassy_sync::once_lock::OnceLock; @@ -9,6 +8,7 @@ use serde::{Deserialize, Serialize}; use crate::intrusive_list::{self, Node, NodeContainer}; use crate::IntrusiveList; +use crate::SyncCell; /// key type for OEM Endpoint declarations pub type OemKey = isize; @@ -185,7 +185,7 @@ pub enum MailboxDelegateError { pub struct Endpoint { node: Node, id: EndpointID, - delegator: Cell>, + delegator: SyncCell>, } impl NodeContainer for Endpoint { @@ -205,7 +205,7 @@ impl Endpoint { Self { node: Node::uninit(), id, - delegator: Cell::new(None), + delegator: SyncCell::new(None), } } diff --git a/embedded-service/src/sync_cell.rs b/embedded-service/src/critical_section_cell.rs similarity index 63% rename from embedded-service/src/sync_cell.rs rename to embedded-service/src/critical_section_cell.rs index ee366d8cf..196f9788f 100644 --- a/embedded-service/src/sync_cell.rs +++ b/embedded-service/src/critical_section_cell.rs @@ -1,13 +1,13 @@ -//! # SyncCell: a cell-like API for static interior mutability scenarios. Backed by a critical section, implying it's usage may delay or defer interrupts. Recommended to use sparingly. +//! # CriticalSectionCell: a cell-like API for static interior mutability scenarios. Backed by a critical section, implying it's usage may delay or defer interrupts. Recommended to use sparingly. use core::cell::Cell; /// A critical section backed Cell for sync scenarios where you want Cell behaviors, but need it to be thread safe (such as used in statics). Backed by a critical section, use sparingly. -pub struct SyncCell { +pub struct CriticalSectionCell { inner: Cell, } -impl SyncCell { - /// Constructs a SyncCell, initializing it with initial_value +impl CriticalSectionCell { + /// Constructs a CriticalSectionCell, initializing it with initial_value pub const fn new(initial_value: T) -> Self { Self { inner: Cell::new(initial_value), @@ -18,12 +18,12 @@ impl SyncCell { /// for read/write conditions but does not automatically handle logical data /// race conditions. It is still possible for a user to read a value but have /// it change after they've performed the read. This just ensures data integrity: - /// SyncCell will always contain a valid T, even if it's been read "late" + /// CriticalSectionCell will always contain a valid T, even if it's been read "late" pub fn set(&self, value: T) { critical_section::with(|_cs| self.inner.set(value)) } - /// Swap contents between two SyncCell's + /// Swap contents between two CriticalSectionCell's /// # Panics /// /// This function will panic if `self` and `other` are different `Cell`s that partially overlap. @@ -33,20 +33,20 @@ impl SyncCell { critical_section::with(|_cs| self.inner.swap(&other.inner)); } - /// consume the Synccell and return the inner value T + /// consume the CriticalSectionCell and return the inner value T pub fn into_inner(self) -> T { self.inner.into_inner() } } -impl SyncCell { +impl CriticalSectionCell { /// Reads the cell's content (in a critical section) and returns a copy pub fn get(&self) -> T { critical_section::with(|_cs| self.inner.get()) } } -impl SyncCell { +impl CriticalSectionCell { /// Return an address to the backing type /// Unsafe: allows reads and writes without critical section guard, violating Sync guarantees. /// # Safety @@ -57,81 +57,81 @@ impl SyncCell { } } -impl SyncCell { +impl CriticalSectionCell { /// consume the inner T, returning its value and replacing it with default() pub fn take(&self) -> T { critical_section::with(|_cs| self.inner.take()) } } -// SAFETY: Sync is implemented here for SyncCell as T is only accessed via nestable critical sections -unsafe impl Sync for SyncCell {} +// SAFETY: Sync is implemented here for CriticalSectionCell as T is only accessed via nestable critical sections +unsafe impl Sync for CriticalSectionCell {} // SAFETY: Can implement send here due to critical section without T being explicitly Send -unsafe impl Send for SyncCell where T: Send {} +unsafe impl Send for CriticalSectionCell where T: Send {} -impl Clone for SyncCell { +impl Clone for CriticalSectionCell { #[inline] - fn clone(&self) -> SyncCell { - SyncCell::new(self.get()) + fn clone(&self) -> CriticalSectionCell { + CriticalSectionCell::new(self.get()) } } -impl Default for SyncCell { +impl Default for CriticalSectionCell { /// Creates a `Cell`, with the `Default` value for T. #[inline] - fn default() -> SyncCell { - SyncCell::new(Default::default()) + fn default() -> CriticalSectionCell { + CriticalSectionCell::new(Default::default()) } } -impl PartialOrd for SyncCell { +impl PartialOrd for CriticalSectionCell { #[inline] - fn partial_cmp(&self, other: &SyncCell) -> Option { + fn partial_cmp(&self, other: &CriticalSectionCell) -> Option { self.get().partial_cmp(&other.get()) } #[inline] - fn lt(&self, other: &SyncCell) -> bool { + fn lt(&self, other: &CriticalSectionCell) -> bool { self.get() < other.get() } #[inline] - fn le(&self, other: &SyncCell) -> bool { + fn le(&self, other: &CriticalSectionCell) -> bool { self.get() <= other.get() } #[inline] - fn gt(&self, other: &SyncCell) -> bool { + fn gt(&self, other: &CriticalSectionCell) -> bool { self.get() > other.get() } #[inline] - fn ge(&self, other: &SyncCell) -> bool { + fn ge(&self, other: &CriticalSectionCell) -> bool { self.get() >= other.get() } } -impl PartialEq for SyncCell { +impl PartialEq for CriticalSectionCell { #[inline] - fn eq(&self, other: &SyncCell) -> bool { + fn eq(&self, other: &CriticalSectionCell) -> bool { self.get() == other.get() } } -impl Eq for SyncCell {} +impl Eq for CriticalSectionCell {} -impl Ord for SyncCell { +impl Ord for CriticalSectionCell { #[inline] - fn cmp(&self, other: &SyncCell) -> core::cmp::Ordering { + fn cmp(&self, other: &CriticalSectionCell) -> core::cmp::Ordering { self.get().cmp(&other.get()) } } -impl From for SyncCell { - /// Creates a new `SyncCell` containing the given value. - fn from(t: T) -> SyncCell { - SyncCell::new(t) +impl From for CriticalSectionCell { + /// Creates a new `CriticalSectionCell` containing the given value. + fn from(t: T) -> CriticalSectionCell { + CriticalSectionCell::new(t) } } @@ -141,9 +141,9 @@ mod tests { #[test] fn test_empty() { - let sc = SyncCell::<()>::new(()); + let sc = CriticalSectionCell::<()>::new(()); - // Ensure get() always returns the same type as the type the SyncCell was initialized with + // Ensure get() always returns the same type as the type the CriticalSectionCell was initialized with // This can be done statically at compile time let _: () = sc.get(); sc.set(()); @@ -152,7 +152,7 @@ mod tests { #[test] fn test_primitive() { - let sc = SyncCell::new(0usize); + let sc = CriticalSectionCell::new(0usize); assert_eq!(sc.get(), 0); sc.set(1); @@ -167,7 +167,7 @@ mod tests { b: u32, } - let sc = SyncCell::new(Example { a: 0, b: 0 }); + let sc = CriticalSectionCell::new(Example { a: 0, b: 0 }); assert_eq!(sc.get(), Example { a: 0, b: 0 }); sc.set(Example { a: 1, b: 2 }); @@ -176,7 +176,7 @@ mod tests { #[tokio::test] async fn test_across_threads() { - static SC: SyncCell = SyncCell::new(false); + static SC: CriticalSectionCell = CriticalSectionCell::new(false); let scr = &SC; let poller = tokio::spawn(async { diff --git a/embedded-service/src/hid/mod.rs b/embedded-service/src/hid/mod.rs index ef6657770..44017c1c8 100644 --- a/embedded-service/src/hid/mod.rs +++ b/embedded-service/src/hid/mod.rs @@ -2,13 +2,12 @@ //! See spec at http://msdn.microsoft.com/en-us/library/windows/hardware/hh852380.aspx use core::convert::Infallible; -use embassy_sync::blocking_mutex::raw::NoopRawMutex; use embassy_sync::once_lock::OnceLock; use embassy_sync::signal::Signal; use crate::buffer::SharedRef; use crate::comms::{self, Endpoint, EndpointID, External, Internal, MailboxDelegate}; -use crate::{error, intrusive_list, IntrusiveList, Node, NodeContainer}; +use crate::{error, intrusive_list, GlobalRawMutex, IntrusiveList, Node, NodeContainer}; mod command; pub use command::*; @@ -158,7 +157,7 @@ impl Default for RegisterFile { pub struct Device { node: Node, tp: Endpoint, - request: Signal>, + request: Signal>, /// Device ID pub id: DeviceId, /// Registers diff --git a/embedded-service/src/intrusive_list.rs b/embedded-service/src/intrusive_list.rs index 456bda521..b95069c6b 100644 --- a/embedded-service/src/intrusive_list.rs +++ b/embedded-service/src/intrusive_list.rs @@ -3,7 +3,7 @@ // Any type used for dynamic type coercion pub use core::any::Any; -use crate::sync_cell::SyncCell; +use crate::SyncCell; /// Interface error class information #[derive(Copy, Clone, Debug)] diff --git a/embedded-service/src/ipc/deferred.rs b/embedded-service/src/ipc/deferred.rs index 894097ad7..d0bc35ffd 100644 --- a/embedded-service/src/ipc/deferred.rs +++ b/embedded-service/src/ipc/deferred.rs @@ -101,13 +101,13 @@ impl Request<'_, M, C, R> { #[cfg(test)] mod tests { use super::*; - use embassy_sync::blocking_mutex::raw::CriticalSectionRawMutex; + use crate::GlobalRawMutex; use embassy_sync::once_lock::OnceLock; use tokio::time::Duration; #[test] fn test_autoincrement() { - let channel = Channel::::new(); + let channel = Channel::::new(); for i in 0..100 { let id = channel.get_next_request_id(); assert_eq!(id.0, i); @@ -132,7 +132,7 @@ mod tests { /// Mock command handler struct Handler { - channel: Channel, + channel: Channel, } impl Handler { diff --git a/embedded-service/src/keyboard.rs b/embedded-service/src/keyboard.rs index d28296a7b..8b74ae54a 100644 --- a/embedded-service/src/keyboard.rs +++ b/embedded-service/src/keyboard.rs @@ -1,10 +1,11 @@ //! Keyboard service data types and common functionality -use core::cell::Cell; +use embassy_sync::mutex::Mutex; use embassy_sync::once_lock::OnceLock; use crate::buffer::SharedRef; use crate::comms::{self, EndpointID, External, Internal}; +use crate::GlobalRawMutex; /// Keyboard device ID #[derive(Debug, Clone, Copy)] @@ -60,7 +61,7 @@ pub struct BroadcastConfig { /// Keyboard service context struct Context { - broadcast_config: Cell, + broadcast_config: Mutex, } static CONTEXT: OnceLock = OnceLock::new(); @@ -68,7 +69,7 @@ static CONTEXT: OnceLock = OnceLock::new(); /// Initialize common keyboard service functionality pub fn init() { CONTEXT.get_or_init(|| Context { - broadcast_config: Cell::new(BroadcastConfig::default()), + broadcast_config: Mutex::new(BroadcastConfig::default()), }); } @@ -76,18 +77,16 @@ pub fn init() { pub async fn enable_broadcast_host() { let context = CONTEXT.get().await; - let mut config = context.broadcast_config.get(); + let mut config = context.broadcast_config.lock().await; config.broadcast_host = true; - context.broadcast_config.set(config); } /// Enable broadcasting messages to the HID endpoint pub async fn enable_broadcast_hid() { let context = CONTEXT.get().await; - let mut config = context.broadcast_config.get(); + let mut config = context.broadcast_config.lock().await; config.broadcast_hid = true; - context.broadcast_config.set(config); } /// Broadcast a keyboard message to the specified endpoints @@ -115,6 +114,6 @@ pub async fn broadcast_message_with_config(from: DeviceId, config: BroadcastConf /// Broadcast a keyboard message using the global broadcast config pub async fn broadcast_message(from: DeviceId, data: MessageData<'static>) { - let config = CONTEXT.get().await.broadcast_config.get(); + let config = *CONTEXT.get().await.broadcast_config.lock().await; broadcast_message_with_config(from, config, data).await; } diff --git a/embedded-service/src/lib.rs b/embedded-service/src/lib.rs index 899015440..b6f6e1fab 100644 --- a/embedded-service/src/lib.rs +++ b/embedded-service/src/lib.rs @@ -6,7 +6,7 @@ pub mod intrusive_list; pub use intrusive_list::*; -pub mod sync_cell; +pub mod critical_section_cell; /// short-hand include all pre-baked services pub mod activity; @@ -22,6 +22,27 @@ pub mod keyboard; pub mod power; pub mod type_c; +/// Global Mutex type, ThreadModeRawMutex is used in a microcontroller context, whereas CriticalSectionRawMutex is used +/// in a standard context for unit testing. +/// +/// Used because ThreadModeRawMutex is not unit test friendly +/// but CriticalSectionRawMutex would incur a significant performance impact, since it disables interrupts. +#[cfg(any(test, not(target_os = "none")))] +pub type GlobalRawMutex = embassy_sync::blocking_mutex::raw::CriticalSectionRawMutex; +/// Global Mutex type, ThreadModeRawMutex is used in a microcontroller context, whereas CriticalSectionRawMutex is used +/// in a standard context for unit testing. +/// +/// Used because ThreadModeRawMutex is not unit test friendly +/// but CriticalSectionRawMutex would incur a significant performance impact, since it disables interrupts. +#[cfg(all(not(test), target_os = "none"))] +pub type GlobalRawMutex = embassy_sync::blocking_mutex::raw::ThreadModeRawMutex; + +/// A cell type that is Sync and Send. CriticalSectionCell is used in a standard context to support multiple cores and +/// executors, whereas ThreadModeCell is leaner and used in a microcontroller context for when there's a guarantee of a +/// single core and executor. +// #[cfg(any(test, not(target_os = "none")))] +pub type SyncCell = critical_section_cell::CriticalSectionCell; + /// initialize all service static interfaces as required. Ideally, this is done before subsystem initialization pub async fn init() { comms::init(); diff --git a/embedded-service/src/power/policy/charger.rs b/embedded-service/src/power/policy/charger.rs index c2ebfad91..9b52b5d2c 100644 --- a/embedded-service/src/power/policy/charger.rs +++ b/embedded-service/src/power/policy/charger.rs @@ -1,9 +1,9 @@ //! Charger device struct and controller use core::{future::Future, ops::DerefMut}; -use embassy_sync::{blocking_mutex::raw::NoopRawMutex, channel::Channel, mutex::Mutex}; +use embassy_sync::{channel::Channel, mutex::Mutex}; -use crate::{intrusive_list, power}; +use crate::{intrusive_list, power, GlobalRawMutex}; use super::PowerCapability; @@ -163,11 +163,11 @@ pub struct Device { /// Device ID id: ChargerId, /// Current state of the device - state: Mutex, + state: Mutex, /// Channel for requests to the device - commands: Channel, + commands: Channel, /// Channel for responses from the device - response: Channel, + response: Channel, } impl Device { diff --git a/embedded-service/src/power/policy/device.rs b/embedded-service/src/power/policy/device.rs index d161e178e..570b45120 100644 --- a/embedded-service/src/power/policy/device.rs +++ b/embedded-service/src/power/policy/device.rs @@ -1,12 +1,11 @@ //! Device struct and methods use core::ops::DerefMut; -use embassy_sync::blocking_mutex::raw::NoopRawMutex; use embassy_sync::mutex::Mutex; use super::{action, DeviceId, Error, PowerCapability}; -use crate::intrusive_list; use crate::ipc::deferred; +use crate::{intrusive_list, GlobalRawMutex}; /// Most basic device states #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -117,9 +116,9 @@ pub struct Device { /// Device ID id: DeviceId, /// Current state of the device - state: Mutex, + state: Mutex, /// Command channel - command: deferred::Channel, + command: deferred::Channel, } impl Device { @@ -181,7 +180,7 @@ impl Device { } /// Create a handler for the command channel - pub async fn receive(&self) -> deferred::Request<'_, NoopRawMutex, CommandData, InternalResponseData> { + pub async fn receive(&self) -> deferred::Request<'_, GlobalRawMutex, CommandData, InternalResponseData> { self.command.receive().await } diff --git a/embedded-service/src/power/policy/policy.rs b/embedded-service/src/power/policy/policy.rs index b501fd89c..8e64c3fb3 100644 --- a/embedded-service/src/power/policy/policy.rs +++ b/embedded-service/src/power/policy/policy.rs @@ -1,7 +1,7 @@ //! Context for any power policy implementations use core::sync::atomic::{AtomicBool, Ordering}; -use embassy_sync::blocking_mutex::raw::NoopRawMutex; +use crate::GlobalRawMutex; use embassy_sync::channel::Channel; use embassy_sync::once_lock::OnceLock; @@ -75,9 +75,9 @@ struct Context { /// Registered devices devices: intrusive_list::IntrusiveList, /// Policy request - policy_request: Channel, + policy_request: Channel, /// Policy response - policy_response: Channel, + policy_response: Channel, /// Registered chargers chargers: intrusive_list::IntrusiveList, } diff --git a/embedded-service/src/type_c/controller.rs b/embedded-service/src/type_c/controller.rs index bbea5ebf5..304ca3c29 100644 --- a/embedded-service/src/type_c/controller.rs +++ b/embedded-service/src/type_c/controller.rs @@ -2,7 +2,6 @@ use core::future::Future; use core::sync::atomic::{AtomicBool, Ordering}; -use embassy_sync::blocking_mutex::raw::NoopRawMutex; use embassy_sync::once_lock::OnceLock; use embassy_sync::signal::Signal; use embassy_time::{with_timeout, Duration}; @@ -17,7 +16,7 @@ use super::event::{PortEventFlags, PortEventKind}; use super::{external, ControllerId}; use crate::ipc::deferred; use crate::power::policy; -use crate::{error, intrusive_list, trace, IntrusiveNode}; +use crate::{error, intrusive_list, trace, GlobalRawMutex, IntrusiveNode}; /// Power contract #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -217,7 +216,7 @@ pub struct Device<'a> { id: ControllerId, ports: &'a [GlobalPortId], num_ports: usize, - command: deferred::Channel>, + command: deferred::Channel>, } impl intrusive_list::NodeContainer for Device<'static> { @@ -272,7 +271,7 @@ impl<'a> Device<'a> { } /// Create a command handler for this controller - pub async fn receive(&self) -> deferred::Request<'_, NoopRawMutex, Command, Response<'static>> { + pub async fn receive(&self) -> deferred::Request<'_, GlobalRawMutex, Command, Response<'static>> { self.command.receive().await } @@ -362,9 +361,9 @@ pub trait Controller { /// Internal context for managing PD controllers struct Context { controllers: intrusive_list::IntrusiveList, - port_events: Signal, + port_events: Signal, /// Channel for receiving commands to the type-C service - external_command: deferred::Channel>, + external_command: deferred::Channel>, } impl Context { @@ -709,7 +708,7 @@ impl ContextToken { /// Wait for an external command pub async fn wait_external_command( &self, - ) -> deferred::Request<'_, NoopRawMutex, external::Command, external::Response<'static>> { + ) -> deferred::Request<'_, GlobalRawMutex, external::Command, external::Response<'static>> { CONTEXT.get().await.external_command.receive().await } diff --git a/espi-service/src/espi_service.rs b/espi-service/src/espi_service.rs index 2e08e6ac3..80da90da7 100644 --- a/espi-service/src/espi_service.rs +++ b/espi-service/src/espi_service.rs @@ -1,44 +1,24 @@ -use core::cell::RefCell; use core::mem::offset_of; use core::slice; +use embassy_sync::mutex::Mutex; use embassy_sync::once_lock::OnceLock; use embedded_services::comms::{self, EndpointID, External, Internal}; -use embedded_services::{ec_type, error, info}; +use embedded_services::{ec_type, error, info, GlobalRawMutex}; pub struct Service<'a> { endpoint: comms::Endpoint, - ec_memory: RefCell<&'a mut ec_type::structure::ECMemory>, + ec_memory: Mutex, } impl Service<'_> { pub fn new(ec_memory: &'static mut ec_type::structure::ECMemory) -> Self { Service { endpoint: comms::Endpoint::uninit(EndpointID::External(External::Host)), - ec_memory: RefCell::new(ec_memory), + ec_memory: Mutex::new(ec_memory), } } - fn update_battery_section(&self, msg: &ec_type::message::BatteryMessage) { - let mut memory_map = self.ec_memory.borrow_mut(); - ec_type::update_battery_section(msg, &mut memory_map); - } - - fn update_capabilities_section(&self, msg: &ec_type::message::CapabilitiesMessage) { - let mut memory_map = self.ec_memory.borrow_mut(); - ec_type::update_capabilities_section(msg, &mut memory_map); - } - - fn update_thermal_section(&self, msg: &ec_type::message::ThermalMessage) { - let mut memory_map = self.ec_memory.borrow_mut(); - ec_type::update_thermal_section(msg, &mut memory_map); - } - - fn update_time_alarm_section(&self, msg: &ec_type::message::TimeAlarmMessage) { - let mut memory_map = self.ec_memory.borrow_mut(); - ec_type::update_time_alarm_section(msg, &mut memory_map); - } - async fn route_to_service(&self, offset: usize, length: usize) -> Result<(), ec_type::Error> { let mut offset = offset; let mut length = length; @@ -77,7 +57,10 @@ impl Service<'_> { async fn route_to_battery_service(&self, offset: &mut usize, length: &mut usize) -> Result<(), ec_type::Error> { let msg = { - let memory_map = self.ec_memory.borrow(); + let memory_map = self + .ec_memory + .try_lock() + .expect("Messages handled one after another, should be infallible."); ec_type::mem_map_to_battery_msg(&memory_map, offset, length)? }; @@ -94,7 +77,10 @@ impl Service<'_> { async fn route_to_thermal_service(&self, offset: &mut usize, length: &mut usize) -> Result<(), ec_type::Error> { let msg = { - let memory_map = self.ec_memory.borrow(); + let memory_map = self + .ec_memory + .try_lock() + .expect("Messages handled one after another, should be infallible."); ec_type::mem_map_to_thermal_msg(&memory_map, offset, length)? }; @@ -111,7 +97,10 @@ impl Service<'_> { async fn route_to_time_alarm_service(&self, offset: &mut usize, length: &mut usize) -> Result<(), ec_type::Error> { let msg = { - let memory_map = self.ec_memory.borrow(); + let memory_map = self + .ec_memory + .try_lock() + .expect("Messages handled one after another, should be infallible."); ec_type::mem_map_to_time_alarm_msg(&memory_map, offset, length)? }; @@ -129,14 +118,18 @@ impl Service<'_> { impl comms::MailboxDelegate for Service<'_> { fn receive(&self, message: &comms::Message) -> Result<(), comms::MailboxDelegateError> { + let mut memory_map = self + .ec_memory + .try_lock() + .expect("Messages handled one after another, should be infallible."); if let Some(msg) = message.data.get::() { - self.update_capabilities_section(msg); + ec_type::update_capabilities_section(msg, &mut memory_map); } else if let Some(msg) = message.data.get::() { - self.update_battery_section(msg); + ec_type::update_battery_section(msg, &mut memory_map); } else if let Some(msg) = message.data.get::() { - self.update_thermal_section(msg); + ec_type::update_thermal_section(msg, &mut memory_map); } else if let Some(msg) = message.data.get::() { - self.update_time_alarm_section(msg); + ec_type::update_time_alarm_section(msg, &mut memory_map); } else { return Err(comms::MailboxDelegateError::MessageNotFound); } diff --git a/hid-service/src/i2c/device.rs b/hid-service/src/i2c/device.rs index 8a857d4b3..45d27e0dc 100644 --- a/hid-service/src/i2c/device.rs +++ b/hid-service/src/i2c/device.rs @@ -1,9 +1,9 @@ use core::borrow::BorrowMut; -use core::cell::{Cell, RefCell}; +use embassy_sync::mutex::Mutex; use embedded_hal_async::i2c::{AddressMode, I2c}; -use embedded_services::buffer::*; use embedded_services::hid::{DeviceContainer, Opcode, Response}; +use embedded_services::{buffer::*, GlobalRawMutex}; use embedded_services::{error, hid, info, trace}; use crate::Error; @@ -12,8 +12,8 @@ pub struct Device> { device: hid::Device, buffer: OwnedRef<'static, u8>, address: A, - descriptor: Cell>, - bus: RefCell, + descriptor: Mutex>, + bus: Mutex, } impl> Device { @@ -22,18 +22,19 @@ impl> Device { device: hid::Device::new(id, regs), buffer, address, - descriptor: Cell::new(None), - bus: RefCell::new(bus), + descriptor: Mutex::new(None), + bus: Mutex::new(bus), } } - #[allow(clippy::await_holding_refcell_ref)] async fn get_hid_descriptor(&self) -> Result> { - if self.descriptor.get().is_some() { - return Ok(self.descriptor.get().unwrap()); + { + let descriptor = self.descriptor.lock().await; + if descriptor.is_some() { + return Ok(descriptor.unwrap()); + } } - - let mut bus = self.bus.borrow_mut(); + let mut bus = self.bus.lock().await; let mut borrow = self.buffer.borrow_mut(); let mut reg = [0u8; 2]; let buf: &mut [u8] = borrow.borrow_mut(); @@ -52,7 +53,10 @@ impl> Device { } let desc = res.unwrap(); info!("HID descriptor: {:#?}", desc); - self.descriptor.set(Some(desc)); + { + let mut descriptor = self.descriptor.lock().await; + *descriptor = Some(desc); + } Ok(desc) } @@ -68,7 +72,6 @@ impl> Device { Ok(self.buffer.reference().slice(0..len)) } - #[allow(clippy::await_holding_refcell_ref)] pub async fn read_report_descriptor(&self) -> Result, Error> { info!("Sending report descriptor"); @@ -78,7 +81,7 @@ impl> Device { let reg = desc.w_report_desc_register.to_le_bytes(); let len = desc.w_report_desc_length as usize; - let mut bus = self.bus.borrow_mut(); + let mut bus = self.bus.lock().await; if let Err(e) = bus.write_read(self.address, ®, &mut buf[0..len]).await { error!("Failed to read report descriptor"); return Err(Error::Bus(e)); @@ -87,7 +90,6 @@ impl> Device { Ok(self.buffer.reference().slice(0..len)) } - #[allow(clippy::await_holding_refcell_ref)] pub async fn handle_input_report(&self) -> Result, Error> { info!("Handling input report"); let desc = self.get_hid_descriptor().await?; @@ -96,7 +98,7 @@ impl> Device { let buf: &mut [u8] = borrow.borrow_mut(); let buf = &mut buf[0..desc.w_max_input_length as usize]; - let mut bus = self.bus.borrow_mut(); + let mut bus = self.bus.lock().await; if let Err(e) = bus.read(self.address, buf).await { error!("Failed to read input report"); return Err(Error::Bus(e)); @@ -105,7 +107,6 @@ impl> Device { Ok(self.buffer.reference().slice(0..desc.w_max_input_length as usize)) } - #[allow(clippy::await_holding_refcell_ref)] pub async fn handle_command( &self, cmd: &hid::Command<'static>, @@ -131,7 +132,7 @@ impl> Device { } let len = res.unwrap(); - let mut bus = self.bus.borrow_mut(); + let mut bus = self.bus.lock().await; if let Err(e) = bus.write(self.address, &buf[..len]).await { error!("Failed to write command"); return Err(Error::Bus(e)); diff --git a/hid-service/src/i2c/host.rs b/hid-service/src/i2c/host.rs index 482e6e5b9..805a77f31 100644 --- a/hid-service/src/i2c/host.rs +++ b/hid-service/src/i2c/host.rs @@ -1,13 +1,13 @@ //! I2C<->HID bridge use core::borrow::{Borrow, BorrowMut}; -use core::cell::RefCell; -use embassy_sync::blocking_mutex::raw::NoopRawMutex; +use embassy_sync::mutex::Mutex; use embassy_sync::signal::Signal; use embassy_time::{with_timeout, Duration}; use embedded_services::buffer::OwnedRef; use embedded_services::comms::{self, Endpoint, EndpointID, External, MailboxDelegate}; use embedded_services::hid::{self, DeviceId, Opcode}; +use embedded_services::GlobalRawMutex; use embedded_services::{error, trace}; use super::{Command as I2cCommand, I2cSlaveAsync}; @@ -25,9 +25,9 @@ pub enum Access { pub struct Host { id: DeviceId, pub tp: Endpoint, - response: Signal>>, + response: Signal>>, buffer: OwnedRef<'static, u8>, - bus: RefCell, + bus: Mutex, } impl Host { @@ -37,13 +37,12 @@ impl Host { tp: Endpoint::uninit(EndpointID::External(External::Host)), response: Signal::new(), buffer, - bus: RefCell::new(bus), + bus: Mutex::new(bus), } } - #[allow(clippy::await_holding_refcell_ref)] async fn read_bus(&self, timeout_ms: u64, buffer: &mut [u8]) -> Result<(), Error> { - let mut bus = self.bus.borrow_mut(); + let mut bus = self.bus.lock().await; let result = with_timeout(Duration::from_millis(timeout_ms), bus.respond_to_write(buffer)).await; if result.is_err() { error!("Response timeout"); @@ -58,9 +57,8 @@ impl Host { Ok(()) } - #[allow(clippy::await_holding_refcell_ref)] async fn write_bus(&self, timeout_ms: u64, buffer: &[u8]) -> Result<(), Error> { - let mut bus = self.bus.borrow_mut(); + let mut bus = self.bus.lock().await; // Send response, timeout if the host doesn't read so we don't get stuck here trace!("Sending {} bytes", buffer.len()); let result = with_timeout(Duration::from_millis(timeout_ms), bus.respond_to_read(buffer)).await; @@ -199,10 +197,9 @@ impl Host { } /// Process a request from the host - #[allow(clippy::await_holding_refcell_ref)] pub async fn wait_request(&self) -> Result> { // Wait for HID register address - let mut bus = self.bus.borrow_mut(); + let mut bus = self.bus.lock().await; loop { trace!("Waiting for host"); match bus.listen().await { @@ -226,7 +223,6 @@ impl Host { } } - #[allow(clippy::await_holding_refcell_ref)] pub async fn send_response(&self) -> Result<(), Error> { if let Some(response) = self.response.wait().await { match response { @@ -240,7 +236,7 @@ impl Host { // Wait for the read from the host // Input reports just a read so we don't need to wait for one if !matches!(response, hid::Response::InputReport(_)) { - let mut bus = self.bus.borrow_mut(); + let mut bus = self.bus.lock().await; match bus.listen().await { Err(e) => { error!("Bus error"); diff --git a/hid-service/src/i2c/passthrough/interrupt.rs b/hid-service/src/i2c/passthrough/interrupt.rs index 600fca789..09236d248 100644 --- a/hid-service/src/i2c/passthrough/interrupt.rs +++ b/hid-service/src/i2c/passthrough/interrupt.rs @@ -1,20 +1,17 @@ -use core::cell::{Cell, RefCell}; - -use embassy_sync::blocking_mutex::raw::NoopRawMutex; -use embassy_sync::signal::Signal; +use embassy_sync::{mutex::Mutex, signal::Signal}; use embedded_hal::digital::OutputPin; use embedded_hal_async::digital::Wait; -use embedded_services::trace; +use embedded_services::{trace, GlobalRawMutex}; /// This struct manages interrupt signal passthrough /// When an interrupt from the device occurs the interrupt to the host is assert /// The interrupt will be deasserted when we receive a request from the host /// We then ignore any further device interrupts until the response is sent to the host pub struct InterruptSignal { - state: Cell, - int_in: RefCell, - int_out: RefCell, - signal: Signal, + state: Mutex, + int_in: Mutex, + int_out: Mutex, + signal: Signal, } #[derive(Clone, Copy, PartialEq, Eq)] @@ -28,59 +25,71 @@ enum InterruptState { impl InterruptSignal { pub fn new(int_in: IN, int_out: OUT) -> Self { Self { - state: Cell::new(InterruptState::Idle), - int_in: RefCell::new(int_in), - int_out: RefCell::new(int_out), + state: Mutex::new(InterruptState::Idle), + int_in: Mutex::new(int_in), + int_out: Mutex::new(int_out), signal: Signal::new(), } } /// Deassert the interrupt signal - pub fn deassert(&self) { - if self.state.get() == InterruptState::Asserted { - self.state.set(InterruptState::Waiting); + pub async fn deassert(&self) { + let mut state = self.state.lock().await; + if *state == InterruptState::Asserted { + *state = InterruptState::Waiting; self.signal.signal(()); } } /// Release the interrupt signal, allowing device interrupts to passthrough again - pub fn release(&self) { - if self.state.get() == InterruptState::Waiting { - self.state.set(InterruptState::Idle); + pub async fn release(&self) { + let mut state = self.state.lock().await; + if *state == InterruptState::Waiting { + *state = InterruptState::Idle; self.signal.signal(()); } } /// Deassert and release the interrupt signal - pub fn reset(&self) { - self.state.set(InterruptState::Reset); + pub async fn reset(&self) { + let mut state = self.state.lock().await; + *state = InterruptState::Reset; self.signal.signal(()); } - #[allow(clippy::await_holding_refcell_ref)] pub async fn process(&self) { - let mut int_in = self.int_in.borrow_mut(); - let mut int_out = self.int_out.borrow_mut(); + let mut int_in = self.int_in.lock().await; + let mut int_out = self.int_out.lock().await; trace!("Waiting for interrupt"); int_in.wait_for_low().await.unwrap(); int_out.set_low().unwrap(); - self.state.set(InterruptState::Asserted); + { + let mut state = self.state.lock().await; + *state = InterruptState::Asserted; + } trace!("Interrupt received"); self.signal.wait().await; int_out.set_high().unwrap(); trace!("Interrupt deasserted"); - if self.state.get() == InterruptState::Reset { - self.state.set(InterruptState::Idle); - return; + { + let mut state = self.state.lock().await; + if *state == InterruptState::Reset { + *state = InterruptState::Idle; + return; + } } self.signal.wait().await; - self.state.set(InterruptState::Idle); + + { + let mut state = self.state.lock().await; + *state = InterruptState::Idle; + } trace!("Interrupt cleared"); } } diff --git a/platform-service/src/imxrt/embedded_crc.rs b/platform-service/src/imxrt/embedded_crc.rs index 82c874a92..8a14ed1e3 100644 --- a/platform-service/src/imxrt/embedded_crc.rs +++ b/platform-service/src/imxrt/embedded_crc.rs @@ -1,14 +1,14 @@ use embassy_imxrt::crc::{Config, Polynomial}; -use embassy_sync::blocking_mutex::raw::NoopRawMutex; use embassy_sync::mutex::Mutex; use embassy_sync::once_lock::OnceLock; use embassy_time::{Duration, WithTimeout}; +use embedded_services::GlobalRawMutex; use crate::embedded_crc::EmbeddedCrcError; // Initialize static CRC IMXRT object to access hardware registers // Locks register access when in use for each calculation -static CRC: OnceLock>> = OnceLock::new(); +static CRC: OnceLock>> = OnceLock::new(); pub const CRC_CCITT_POLY: u16 = 0x1021; pub const CRC_CRC16_POLY: u16 = 0x8005; diff --git a/platform-service/src/nvram.rs b/platform-service/src/nvram.rs index 11a4a577d..380357087 100644 --- a/platform-service/src/nvram.rs +++ b/platform-service/src/nvram.rs @@ -1,6 +1,6 @@ -use embassy_sync::blocking_mutex::raw::CriticalSectionRawMutex; use embassy_sync::blocking_mutex::Mutex; use embassy_sync::once_lock::OnceLock; +use embedded_services::GlobalRawMutex; use crate::nvram_valid_range; @@ -9,7 +9,7 @@ struct Info { // the starting address/offset and length in u32 words of the section offset: usize, - guard: Mutex, + guard: Mutex, } /// Section descriptor table for linking indices to offsets diff --git a/platform-service/src/reset.rs b/platform-service/src/reset.rs index 1f091be37..e14552965 100644 --- a/platform-service/src/reset.rs +++ b/platform-service/src/reset.rs @@ -2,16 +2,16 @@ use core::future::Future; -use embassy_sync::{blocking_mutex::raw::CriticalSectionRawMutex, lazy_lock::LazyLock, signal::Signal}; +use embassy_sync::{lazy_lock::LazyLock, signal::Signal}; -use embedded_services::{intrusive_list, IntrusiveList, Node, NodeContainer}; +use embedded_services::{intrusive_list, GlobalRawMutex, IntrusiveList, Node, NodeContainer}; static BLOCKERS: LazyLock = LazyLock::new(IntrusiveList::new); pub struct Blocker { node: Node, - reset_pending: Signal, - unblocked: Signal, + reset_pending: Signal, + unblocked: Signal, } impl NodeContainer for Blocker { diff --git a/power-policy-service/src/charger.rs b/power-policy-service/src/charger.rs index 91c15a4da..5d5fa8be6 100644 --- a/power-policy-service/src/charger.rs +++ b/power-policy-service/src/charger.rs @@ -1,4 +1,5 @@ -use core::cell::RefCell; +use embassy_sync::mutex::Mutex; +use embedded_services::GlobalRawMutex; use embassy_futures::select::select; use embedded_services::{ @@ -14,7 +15,7 @@ where charger::ChargerError: From<::ChargeControllerError>, { charger_policy_state: &'a charger::Device, - controller: RefCell, + controller: Mutex, } impl<'a, C: ChargeController> Wrapper<'a, C> @@ -24,7 +25,7 @@ where pub fn new(charger_policy_state: &'a charger::Device, controller: C) -> Self { Self { charger_policy_state, - controller: RefCell::new(controller), + controller: Mutex::new(controller), } } @@ -223,9 +224,8 @@ where self.charger_policy_state.send_response(res).await; } - #[allow(clippy::await_holding_refcell_ref)] pub async fn process(&self) { - let mut controller = self.controller.borrow_mut(); + let mut controller = self.controller.lock().await; loop { let res = select(controller.wait_event(), self.wait_policy_command()).await; match res { diff --git a/power-policy-service/src/lib.rs b/power-policy-service/src/lib.rs index a901a36f7..261408caf 100644 --- a/power-policy-service/src/lib.rs +++ b/power-policy-service/src/lib.rs @@ -1,10 +1,10 @@ #![no_std] use core::ops::DerefMut; -use embassy_sync::blocking_mutex::raw::NoopRawMutex; use embassy_sync::mutex::Mutex; use embassy_sync::once_lock::OnceLock; use embedded_services::power::policy::device::Device; use embedded_services::power::policy::{action, policy, *}; +use embedded_services::GlobalRawMutex; use embedded_services::{comms, error, info}; pub mod config; @@ -34,7 +34,7 @@ pub struct PowerPolicy { /// Power policy context context: policy::ContextToken, /// State - state: Mutex, + state: Mutex, /// Comms endpoint tp: comms::Endpoint, /// Config diff --git a/type-c-service/src/driver/tps6699x.rs b/type-c-service/src/driver/tps6699x.rs index 1da35b972..473ee534f 100644 --- a/type-c-service/src/driver/tps6699x.rs +++ b/type-c-service/src/driver/tps6699x.rs @@ -1,5 +1,4 @@ use core::array::from_fn; -use core::cell::{Cell, RefCell}; use core::iter::zip; use ::tps6699x::registers::field_sets::IntEventBus1; @@ -8,6 +7,7 @@ use ::tps6699x::{PORT0, PORT1, TPS66993_NUM_PORTS, TPS66994_NUM_PORTS}; use bitfield::bitfield; use embassy_futures::select::select; use embassy_sync::blocking_mutex::raw::RawMutex; +use embassy_sync::mutex::Mutex; use embassy_sync::signal::Signal; use embassy_time::Delay; use embedded_cfu_protocol::protocol_definitions::ComponentId; @@ -17,7 +17,7 @@ use embedded_services::power::policy::{self, PowerCapability}; use embedded_services::type_c::controller::{self, Controller, ControllerStatus, PortStatus}; use embedded_services::type_c::event::PortEventKind; use embedded_services::type_c::ControllerId; -use embedded_services::{debug, info, trace, type_c, warn}; +use embedded_services::{debug, info, trace, type_c, warn, GlobalRawMutex}; use embedded_usb_pd::pdinfo::PowerPathStatus; use embedded_usb_pd::pdo::{sink, source, Common, Rdo}; use embedded_usb_pd::type_c::Current as TypecCurrent; @@ -45,11 +45,11 @@ struct FwUpdateState<'a, M: RawMutex, B: I2c> { } pub struct Tps6699x<'a, const N: usize, M: RawMutex, B: I2c> { - port_events: [Cell; N], - port_status: [Cell; N], + port_events: [Mutex; N], + port_status: [Mutex; N], sw_event: Signal, - tps6699x: RefCell>, - update_state: RefCell>>, + tps6699x: Mutex>, + update_state: Mutex>>, /// Firmware update configuration fw_update_config: FwUpdateConfig, } @@ -57,11 +57,11 @@ pub struct Tps6699x<'a, const N: usize, M: RawMutex, B: I2c> { impl<'a, const N: usize, M: RawMutex, B: I2c> Tps6699x<'a, N, M, B> { pub fn new(tps6699x: tps6699x_drv::Tps6699x<'a, M, B>, fw_update_config: FwUpdateConfig) -> Self { Self { - port_events: [const { Cell::new(PortEventKind::none()) }; N], - port_status: [const { Cell::new(PortStatus::new()) }; N], + port_events: [const { Mutex::new(PortEventKind::none()) }; N], + port_status: [const { Mutex::new(PortStatus::new()) }; N], sw_event: Signal::new(), - tps6699x: RefCell::new(tps6699x), - update_state: RefCell::new(None), + tps6699x: Mutex::new(tps6699x), + update_state: Mutex::new(None), fw_update_config, } } @@ -157,7 +157,7 @@ impl<'a, const N: usize, M: RawMutex, B: I2c> Tps6699x<'a, N, M, B> { debug!("Port{} power path: {:#?}", port.0, port_status.power_path); } - self.port_status[port.0 as usize].set(port_status); + *self.port_status[port.0 as usize].lock().await = port_status; Ok(events) } @@ -168,39 +168,39 @@ impl<'a, const N: usize, M: RawMutex, B: I2c> Tps6699x<'a, N, M, B> { ) -> Result<(), Error> { let interrupts = tps6699x.wait_interrupt(false, |_, _| true).await; - for (interrupt, cell) in zip(interrupts.iter(), self.port_events.iter()) { + for (interrupt, mutex) in zip(interrupts.iter(), self.port_events.iter()) { if *interrupt == IntEventBus1::new_zero() { continue; } - let mut event = cell.get(); - if interrupt.plug_event() { - debug!("Event: Plug event"); - event.set_plug_inserted_or_removed(true); - } - if interrupt.source_caps_received() { - debug!("Event: Source Caps received"); - event.set_source_caps_received(true); - } + { + let mut event = mutex.lock().await; + if interrupt.plug_event() { + debug!("Event: Plug event"); + event.set_plug_inserted_or_removed(true); + } + if interrupt.source_caps_received() { + debug!("Event: Source Caps received"); + event.set_source_caps_received(true); + } - if interrupt.sink_ready() { - debug!("Event: Sink ready"); - event.set_sink_ready(true); - } + if interrupt.sink_ready() { + debug!("Event: Sink ready"); + event.set_sink_ready(true); + } - if interrupt.new_consumer_contract() { - debug!("Event: New contract as consumer, PD controller act as Sink"); - // Port is consumer and power negotiation is complete - event.set_new_power_contract_as_consumer(true); - } + if interrupt.new_consumer_contract() { + debug!("Event: New contract as consumer, PD controller act as Sink"); + // Port is consumer and power negotiation is complete + event.set_new_power_contract_as_consumer(true); + } - if interrupt.new_provider_contract() { - debug!("Event: New contract as provider, PD controller act as source"); - // Port is provider and power negotiation is complete - event.set_new_power_contract_as_provider(true); + if interrupt.new_provider_contract() { + debug!("Event: New contract as provider, PD controller act as source"); + // Port is provider and power negotiation is complete + event.set_new_power_contract_as_provider(true); + } } - - cell.set(event); } Ok(()) } @@ -211,14 +211,16 @@ impl<'a, const N: usize, M: RawMutex, B: I2c> Tps6699x<'a, N, M, B> { } /// Signal an event on the given port - fn signal_event(&self, port: LocalPortId, event: PortEventKind) { + async fn signal_event(&self, port: LocalPortId, event: PortEventKind) { if port.0 >= self.port_events.len() as u8 { return; } - let cell = &self.port_events[port.0 as usize]; - let current = cell.get(); - cell.set(current.union(event)); + { + let mut guard = self.port_events[port.0 as usize].lock().await; + let current = *guard; + *guard = current.union(event); + } self.sw_event.signal(()); } } @@ -227,28 +229,37 @@ impl Controller for Tps6699x<'_, N, M, B> { type BusError = B::Error; /// Controller specific initialization - #[allow(clippy::await_holding_refcell_ref)] async fn sync_state(&mut self) -> Result<(), Error> { for i in 0..N { let port = LocalPortId(i as u8); - let mut tps6699x = self.tps6699x.borrow_mut(); - let event = self.update_port_status(&mut tps6699x, port).await?; - self.signal_event(port, event); + let event: PortEventKind; + { + let mut tps6699x = self + .tps6699x + .try_lock() + .expect("Driver should not have been locked before this, thus infallible"); + event = self.update_port_status(&mut tps6699x, port).await?; + } + self.signal_event(port, event).await; } Ok(()) } /// Wait for an event on any port - #[allow(clippy::await_holding_refcell_ref)] async fn wait_port_event(&mut self) -> Result<(), Error> { - let mut tps6699x = self.tps6699x.borrow_mut(); + let mut tps6699x = self + .tps6699x + .try_lock() + .expect("Driver should not have been locked before this, thus infallible"); let _ = select(self.wait_interrupt_event(&mut tps6699x), self.wait_sw_event()).await; - for (i, cell) in self.port_events.iter().enumerate() { + for (i, mutex) in self.port_events.iter().enumerate() { let port = LocalPortId(i as u8); - let event = cell.get().union(self.update_port_status(&mut tps6699x, port).await?); + let mut guard = mutex.lock().await; + + let event = guard.union(self.update_port_status(&mut tps6699x, port).await?); // TODO: We get interrupts for certain status changes that don't currently map to a generic port event // Enable this when those get fleshed out @@ -258,7 +269,7 @@ impl Controller for Tps6699x<'_, N, M, B> { }*/ trace!("Port{} event: {:#?}", i, event); - cell.set(event); + *guard = event; } Ok(()) } @@ -268,8 +279,10 @@ impl Controller for Tps6699x<'_, N, M, B> { if port.0 >= self.port_events.len() as u8 { return PdError::InvalidPort.into(); } - - Ok(self.port_events[port.0 as usize].replace(PortEventKind::none())) + let mut guard = self.port_events[port.0 as usize].lock().await; + let port_events = *guard; + *guard = PortEventKind::none(); + Ok(port_events) } /// Returns the current status of the port @@ -281,15 +294,17 @@ impl Controller for Tps6699x<'_, N, M, B> { return PdError::InvalidPort.into(); } - Ok(self.port_status[port.0 as usize].get()) + Ok(*self.port_status[port.0 as usize].lock().await) } - #[allow(clippy::await_holding_refcell_ref)] async fn get_rt_fw_update_status( &mut self, port: LocalPortId, ) -> Result> { - let mut tps6699x = self.tps6699x.borrow_mut(); + let mut tps6699x = self + .tps6699x + .try_lock() + .expect("Driver should not have been locked before this, thus infallible"); match tps6699x.get_rt_fw_update_status(port).await { Ok(true) => Ok(type_c::controller::RetimerFwUpdateState::Active), Ok(false) => Ok(type_c::controller::RetimerFwUpdateState::Inactive), @@ -297,28 +312,36 @@ impl Controller for Tps6699x<'_, N, M, B> { } } - #[allow(clippy::await_holding_refcell_ref)] async fn set_rt_fw_update_state(&mut self, port: LocalPortId) -> Result<(), Error> { - let mut tps6699x = self.tps6699x.borrow_mut(); + let mut tps6699x = self + .tps6699x + .try_lock() + .expect("Driver should not have been locked before this, thus infallible"); tps6699x.set_rt_fw_update_state(port).await } - #[allow(clippy::await_holding_refcell_ref)] async fn clear_rt_fw_update_state(&mut self, port: LocalPortId) -> Result<(), Error> { - let mut tps6699x = self.tps6699x.borrow_mut(); + let mut tps6699x = self + .tps6699x + .try_lock() + .expect("Driver should not have been locked before this, thus infallible"); tps6699x.clear_rt_fw_update_state(port).await } - #[allow(clippy::await_holding_refcell_ref)] async fn set_rt_compliance(&mut self, port: LocalPortId) -> Result<(), Error> { - let mut tps6699x = self.tps6699x.borrow_mut(); + let mut tps6699x = self + .tps6699x + .try_lock() + .expect("Driver should not have been locked before this, thus infallible"); tps6699x.set_rt_compliance(port).await } - #[allow(clippy::await_holding_refcell_ref)] async fn enable_sink_path(&mut self, port: LocalPortId, enable: bool) -> Result<(), Error> { debug!("Port{} enable sink path: {}", port.0, enable); - let mut tps6699x = self.tps6699x.borrow_mut(); + let mut tps6699x = self + .tps6699x + .try_lock() + .expect("Driver should not have been locked before this, thus infallible"); match tps6699x.enable_sink_path(port, enable).await { // Temporary workaround for autofet rejection // Tracking bug: https://github.com/OpenDevicePartnership/embedded-services/issues/268 @@ -330,9 +353,11 @@ impl Controller for Tps6699x<'_, N, M, B> { } } - #[allow(clippy::await_holding_refcell_ref)] async fn get_controller_status(&mut self) -> Result, Error> { - let mut tps6699x = self.tps6699x.borrow_mut(); + let mut tps6699x = self + .tps6699x + .try_lock() + .expect("Driver should not have been locked before this, thus infallible"); let boot_flags = tps6699x.get_boot_flags().await?; let customer_use = CustomerUse(tps6699x.get_customer_use().await?); @@ -345,22 +370,31 @@ impl Controller for Tps6699x<'_, N, M, B> { }) } - #[allow(clippy::await_holding_refcell_ref)] async fn get_active_fw_version(&self) -> Result> { - let mut tps6699x = self.tps6699x.borrow_mut(); + let mut tps6699x = self + .tps6699x + .try_lock() + .expect("Driver should not have been locked before this, thus infallible"); let customer_use = CustomerUse(tps6699x.get_customer_use().await?); Ok(customer_use.custom_fw_version()) } - #[allow(clippy::await_holding_refcell_ref)] async fn start_fw_update(&mut self) -> Result<(), Error> { - let mut tps6699x = self.tps6699x.borrow_mut(); + let mut tps6699x = self + .tps6699x + .try_lock() + .expect("Driver should not have been locked before this, thus infallible"); let mut delay = Delay; let mut updater: BorrowedUpdater> = BorrowedUpdater::with_config(self.fw_update_config.clone()); // Abandon any previous in-progress update - if let Some(update) = self.update_state.replace(None) { + if let Some(update) = self + .update_state + .try_lock() + .expect("Update state should not have been locked before this, thus infallible") + .take() + { warn!("Abandoning in-progress update"); update.updater.abort_fw_update(&mut [&mut tps6699x], &mut delay).await; } @@ -371,23 +405,35 @@ impl Controller for Tps6699x<'_, N, M, B> { let in_progress = updater.start_fw_update(&mut [&mut tps6699x], &mut delay).await?; // Re-enable interrupts on port 0 only enable_port0_interrupts::>(&mut [&mut tps6699x], &mut guards[0..1]).await?; - self.update_state.replace(Some(FwUpdateState { + let mut state = self + .update_state + .try_lock() + .expect("Update state should not have been locked before this, thus infallible"); + *state = Some(FwUpdateState { updater: in_progress, guards, - })); + }); Ok(()) } /// Aborts the firmware update in progress /// /// This can reset the controller - #[allow(clippy::await_holding_refcell_ref)] async fn abort_fw_update(&mut self) -> Result<(), Error> { - let mut tps6699x = self.tps6699x.borrow_mut(); + let mut tps6699x = self + .tps6699x + .try_lock() + .expect("Driver should not have been locked before this, thus infallible"); // Check if we're still in firmware update mode if tps6699x.get_mode().await? == tps6699x::Mode::F211 { let mut delay = Delay; - if let Some(update) = self.update_state.replace(None) { + + if let Some(update) = self + .update_state + .try_lock() + .expect("Update state should not have been locked before this, thus infallible") + .take() + { // Attempt to abort the firmware update by consuming our update object update.updater.abort_fw_update(&mut [&mut tps6699x], &mut delay).await; Ok(()) @@ -404,10 +450,17 @@ impl Controller for Tps6699x<'_, N, M, B> { /// Finalize the firmware update /// /// This will reset the controller - #[allow(clippy::await_holding_refcell_ref)] async fn finalize_fw_update(&mut self) -> Result<(), Error> { - let mut tps6699x = self.tps6699x.borrow_mut(); - if let Some(update) = self.update_state.replace(None) { + let mut tps6699x = self + .tps6699x + .try_lock() + .expect("Driver should not have been locked before this, thus infallible"); + if let Some(update) = self + .update_state + .try_lock() + .expect("Update state should not have been locked before this, thus infallible") + .take() + { let mut delay = Delay; update .updater @@ -418,10 +471,15 @@ impl Controller for Tps6699x<'_, N, M, B> { } } - #[allow(clippy::await_holding_refcell_ref)] async fn write_fw_contents(&mut self, _offset: usize, data: &[u8]) -> Result<(), Error> { - let mut tps6699x = self.tps6699x.borrow_mut(); - let mut update_state = self.update_state.borrow_mut(); + let mut tps6699x = self + .tps6699x + .try_lock() + .expect("Driver should not have been locked before this, thus infallible"); + let mut update_state = self + .update_state + .try_lock() + .expect("Update state should not have been locked before this, thus infallible"); if let Some(update) = update_state.as_mut() { let mut delay = Delay; update diff --git a/type-c-service/src/task.rs b/type-c-service/src/task.rs index d89f581a4..a5f641af2 100644 --- a/type-c-service/src/task.rs +++ b/type-c-service/src/task.rs @@ -1,6 +1,6 @@ -use core::{cell::RefCell, future::Future}; +use core::future::Future; use embassy_futures::select::{select, Either}; -use embassy_sync::{blocking_mutex::raw::NoopRawMutex, once_lock::OnceLock}; +use embassy_sync::{mutex::Mutex, once_lock::OnceLock}; use embedded_services::{ comms::{self, EndpointID, Internal}, debug, error, info, intrusive_list, @@ -12,6 +12,7 @@ use embedded_services::{ external::{self, ControllerCommandData}, ControllerId, }, + GlobalRawMutex, }; use embedded_usb_pd::GlobalPortId; use embedded_usb_pd::PdError as Error; @@ -34,14 +35,14 @@ pub struct Service { /// Type-C context token context: type_c::controller::ContextToken, /// Current state - state: RefCell, + state: Mutex, } pub enum Event<'a> { /// Port event PortEvent(GlobalPortId, PortEventKind, PortStatus), /// External command - ExternalCommand(deferred::Request<'a, NoopRawMutex, external::Command, external::Response<'static>>), + ExternalCommand(deferred::Request<'a, GlobalRawMutex, external::Command, external::Response<'static>>), } impl Service { @@ -50,27 +51,27 @@ impl Service { Some(Self { tp: comms::Endpoint::uninit(EndpointID::Internal(Internal::Usbc)), context: type_c::controller::ContextToken::create()?, - state: RefCell::new(State::default()), + state: Mutex::new(State::default()), }) } /// Get the cached port status - pub fn get_cached_port_status(&self, port_id: GlobalPortId) -> Result { + pub async fn get_cached_port_status(&self, port_id: GlobalPortId) -> Result { if port_id.0 as usize >= MAX_SUPPORTED_PORTS { return Err(Error::InvalidPort); } - let state = self.state.borrow(); + let state = self.state.lock().await; Ok(state.port_status[port_id.0 as usize]) } /// Set the cached port status - fn set_cached_port_status(&self, port_id: GlobalPortId, status: PortStatus) -> Result<(), Error> { + async fn set_cached_port_status(&self, port_id: GlobalPortId, status: PortStatus) -> Result<(), Error> { if port_id.0 as usize >= MAX_SUPPORTED_PORTS { return Err(Error::InvalidPort); } - let mut state = self.state.borrow_mut(); + let mut state = self.state.lock().await; state.port_status[port_id.0 as usize] = status; Ok(()) } @@ -82,7 +83,7 @@ impl Service { event: PortEventKind, status: PortStatus, ) -> Result<(), Error> { - let old_status = self.get_cached_port_status(port_id)?; + let old_status = self.get_cached_port_status(port_id).await?; debug!("Port{}: Event: {:#?}", port_id.0, event); debug!("Port{} Previous status: {:#?}", port_id.0, old_status); @@ -107,7 +108,7 @@ impl Service { } } - self.set_cached_port_status(port_id, status)?; + self.set_cached_port_status(port_id, status).await?; Ok(()) } @@ -218,9 +219,8 @@ impl Service { } /// Wait for port flags - #[allow(clippy::await_holding_refcell_ref)] async fn wait_port_flags(&self) -> PortEventFlagsIter { - let mut state = self.state.borrow_mut(); + let mut state = self.state.lock().await; if state.event_iter.is_some() { // If we have an existing iterator, return it // Yield first to prevent starving other tasks @@ -232,12 +232,11 @@ impl Service { } /// Wait for the next event - #[allow(clippy::await_holding_refcell_ref)] pub async fn wait_next(&self) -> Result, Error> { loop { match select(self.wait_port_flags(), self.context.wait_external_command()).await { Either::First(mut pending) => { - let mut state = self.state.borrow_mut(); + let mut state = self.state.lock().await; if let Some(port_id) = pending.next() { debug!("Port{}: Event", port_id.0); state.event_iter = Some(pending); diff --git a/type-c-service/src/wrapper/mod.rs b/type-c-service/src/wrapper/mod.rs index 4525e974f..555a7bd2a 100644 --- a/type-c-service/src/wrapper/mod.rs +++ b/type-c-service/src/wrapper/mod.rs @@ -1,15 +1,17 @@ //! This module contains the `Controller` trait. Any types that implement this trait can be used with the `ControllerWrapper` struct //! which provides a bridge between various service messages and the actual controller functions. use core::array::from_fn; -use core::cell::{Cell, RefCell}; use embassy_futures::select::{select4, select_array, Either4}; +use embassy_sync::mutex::Mutex; use embedded_cfu_protocol::protocol_definitions::{FwUpdateOffer, FwUpdateOfferResponse, FwVersion}; use embedded_services::cfu::component::CfuDevice; use embedded_services::power::policy::device::StateKind; use embedded_services::power::policy::{self, action}; use embedded_services::type_c::controller::{self, Controller, PortStatus}; use embedded_services::type_c::event::{PortEventFlags, PortEventKind}; +use embedded_services::GlobalRawMutex; +use embedded_services::SyncCell; use embedded_services::{debug, error, info, trace, warn}; use embedded_usb_pd::{Error, PdError, PortId as LocalPortId}; @@ -60,9 +62,9 @@ pub struct ControllerWrapper<'a, const N: usize, C: Controller, V: FwOfferValida /// CFU device to interface with firmware update service cfu_device: CfuDevice, /// Internal state for the wrapper - state: RefCell, - controller: RefCell, - active_events: [Cell; N], + state: Mutex, + controller: Mutex, + active_events: [SyncCell; N], /// Trait object for validating firmware versions fw_version_validator: V, } @@ -80,9 +82,9 @@ impl<'a, const N: usize, C: Controller, V: FwOfferValidator> ControllerWrapper<' pd_controller, power, cfu_device, - state: RefCell::new(Default::default()), - controller: RefCell::new(controller), - active_events: [const { Cell::new(PortEventKind::none()) }; N], + state: Mutex::new(Default::default()), + controller: Mutex::new(controller), + active_events: [const { SyncCell::new(PortEventKind::none()) }; N], fw_version_validator, } } @@ -226,10 +228,10 @@ impl<'a, const N: usize, C: Controller, V: FwOfferValidator> ControllerWrapper<' } /// Top-level processing function - #[allow(clippy::await_holding_refcell_ref)] + /// Only call this fn from one place in a loop. Otherwise a deadlock could occur. pub async fn process(&self) { - let mut controller = self.controller.borrow_mut(); - let mut state = self.state.borrow_mut(); + let mut controller = self.controller.lock().await; + let mut state = self.state.lock().await; match select4( controller.wait_port_event(), self.wait_power_command(), diff --git a/type-c-service/src/wrapper/power.rs b/type-c-service/src/wrapper/power.rs index 0da3c80f2..9f6ad0a8e 100644 --- a/type-c-service/src/wrapper/power.rs +++ b/type-c-service/src/wrapper/power.rs @@ -1,5 +1,4 @@ //! Module contain power-policy related message handling -use embassy_sync::blocking_mutex::raw::NoopRawMutex; use embedded_services::{ debug, ipc::deferred, @@ -178,7 +177,7 @@ impl ControllerWrapper<'_, N pub(super) async fn wait_power_command( &self, ) -> ( - deferred::Request<'_, NoopRawMutex, CommandData, InternalResponseData>, + deferred::Request<'_, GlobalRawMutex, CommandData, InternalResponseData>, LocalPortId, ) { let futures: [_; N] = from_fn(|i| self.power[i].receive());