From 508292c227807d8b8a080f3e8fe7ae12309d0a81 Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Tue, 4 Jan 2022 13:48:43 +0100 Subject: [PATCH 01/18] itm: improve unlock documentation --- src/peripheral/itm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/peripheral/itm.rs b/src/peripheral/itm.rs index f8e9e25d..44e8f66a 100644 --- a/src/peripheral/itm.rs +++ b/src/peripheral/itm.rs @@ -175,7 +175,7 @@ pub struct ITMSettings { } impl ITM { - /// Removes the software lock on the ITM. + /// Removes the software lock on the ITM. Must be called before any other [ITM] functions. #[inline] pub fn unlock(&mut self) { // NOTE(unsafe) atomic write to a stateless, write-only register From 8a2ab274458454fc230510e21508817172007c5f Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Tue, 4 Jan 2022 13:49:16 +0100 Subject: [PATCH 02/18] itm: configure: check feature support during configuration Related to #382. --- src/peripheral/itm.rs | 94 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 82 insertions(+), 12 deletions(-) diff --git a/src/peripheral/itm.rs b/src/peripheral/itm.rs index 44e8f66a..4842432a 100644 --- a/src/peripheral/itm.rs +++ b/src/peripheral/itm.rs @@ -174,6 +174,28 @@ pub struct ITMSettings { pub timestamp_clk_src: TimestampClkSrc, } +/// Possible errors on [ITM::configure]. +#[derive(Debug, Eq, PartialEq, Copy, Clone)] +pub enum ITMConfigurationError { + /// Global timestamp generation is not supported on this target. + /// Request [GlobalTimestampOptions::Disabled] instead. + /// + /// `ITM_TCR` register remains unchanged on this error. + GTS, + /// The requested timestamp clock source is not supported on this target. + /// + /// *NOTE*: `ITM_TCR.GTSFREQ` field has potentially been changed on + /// this error. + TimestampClkSrc, + /// The target does not implement the local timestamp prescaler. + /// Request [LocalTimestampOptions::Disabled] or + /// [LocalTimestampOptions::Disabled] instead. + /// + /// *NOTE*: `ITM_TCR.{GTSFREQ,SWOENA}` fields have potentially + /// changed on this error. + TSPrescale, +} + impl ITM { /// Removes the software lock on the ITM. Must be called before any other [ITM] functions. #[inline] @@ -182,34 +204,82 @@ impl ITM { unsafe { self.lar.write(0xC5AC_CE55) } } - /// Configures the ITM with the passed [ITMSettings]. - #[inline] - pub fn configure(&mut self, settings: ITMSettings) { + /// Configures the ITM with the passed [ITMSettings]. Returns `true` + /// if the configuration was successfully applied. + #[allow(clippy::missing_inline_in_public_items)] + pub fn configure(&mut self, settings: ITMSettings) -> Result<(), ITMConfigurationError> { + use ITMConfigurationError as Error; + unsafe { self.tcr.modify(|mut r| { - r.set_itmena(settings.enable); - r.set_tsena(settings.local_timestamps != LocalTimestampOptions::Disabled); - r.set_txena(settings.forward_dwt); - r.set_tsprescale(match settings.local_timestamps { - LocalTimestampOptions::Disabled | LocalTimestampOptions::Enabled => 0b00, - LocalTimestampOptions::EnabledDiv4 => 0b10, - LocalTimestampOptions::EnabledDiv16 => 0b10, - LocalTimestampOptions::EnabledDiv64 => 0b11, - }); r.set_gtsfreq(match settings.global_timestamps { GlobalTimestampOptions::Disabled => 0b00, GlobalTimestampOptions::Every128Cycles => 0b01, GlobalTimestampOptions::Every8192Cycles => 0b10, GlobalTimestampOptions::EveryPacket => 0b11, }); + + r + }); + } + // GTSFREQ is potentially RAZ/WI + if settings.global_timestamps != GlobalTimestampOptions::Disabled + && self.tcr.read().gtsfreq() == 0 + { + return Err(Error::GTS); + } + + unsafe { + self.tcr.modify(|mut r| { r.set_swoena(match settings.timestamp_clk_src { TimestampClkSrc::SystemClock => false, TimestampClkSrc::AsyncTPIU => true, }); + + r + }); + } + // SWOENA is potentially either RAZ or RAO + if !{ + match settings.timestamp_clk_src { + TimestampClkSrc::SystemClock => !self.tcr.read().swoena(), + TimestampClkSrc::AsyncTPIU => self.tcr.read().swoena(), + } + } { + return Err(Error::TimestampClkSrc); + } + + unsafe { + self.tcr.modify(|mut r| { + r.set_tsprescale(match settings.local_timestamps { + LocalTimestampOptions::Disabled | LocalTimestampOptions::Enabled => 0b00, + LocalTimestampOptions::EnabledDiv4 => 0b10, + LocalTimestampOptions::EnabledDiv16 => 0b10, + LocalTimestampOptions::EnabledDiv64 => 0b11, + }); + + r + }) + } + // TSPrescale is potentially RAZ/WI + if settings.local_timestamps != LocalTimestampOptions::Disabled + && settings.local_timestamps != LocalTimestampOptions::Enabled + && self.tcr.read().tsprescale() == 0 + { + return Err(Error::TSPrescale); + } + + unsafe { + self.tcr.modify(|mut r| { + r.set_itmena(settings.enable); + r.set_tsena(settings.local_timestamps != LocalTimestampOptions::Disabled); + r.set_txena(settings.forward_dwt); // forward hardware event packets from the DWT to the ITM r.set_tracebusid(settings.bus_id.unwrap_or(0)); r }); } + + Ok(()) } } From e1a84c751f189776ea79925b20636c8918e5d267 Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Tue, 4 Jan 2022 13:50:52 +0100 Subject: [PATCH 03/18] itm: add busy check --- src/peripheral/itm.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/peripheral/itm.rs b/src/peripheral/itm.rs index 4842432a..67a25ffa 100644 --- a/src/peripheral/itm.rs +++ b/src/peripheral/itm.rs @@ -204,6 +204,13 @@ impl ITM { unsafe { self.lar.write(0xC5AC_CE55) } } + /// Indicates whether the ITM is currently processing events. + /// Returns `true` if ITM events are present and are being drained. + #[inline] + pub fn busy(&self) -> bool { + self.tcr.read().busy() + } + /// Configures the ITM with the passed [ITMSettings]. Returns `true` /// if the configuration was successfully applied. #[allow(clippy::missing_inline_in_public_items)] From bb7319bb2474855ad63076047fe25878d77789fd Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Wed, 5 Jan 2022 20:33:59 +0100 Subject: [PATCH 04/18] itm: improve documentation --- src/peripheral/itm.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/peripheral/itm.rs b/src/peripheral/itm.rs index 67a25ffa..8533af33 100644 --- a/src/peripheral/itm.rs +++ b/src/peripheral/itm.rs @@ -178,40 +178,40 @@ pub struct ITMSettings { #[derive(Debug, Eq, PartialEq, Copy, Clone)] pub enum ITMConfigurationError { /// Global timestamp generation is not supported on this target. - /// Request [GlobalTimestampOptions::Disabled] instead. + /// Request [`GlobalTimestampOptions::Disabled`] instead. /// - /// `ITM_TCR` register remains unchanged on this error. + /// [`ITM_TCR`](struct@Tcr) register remains unchanged on this error. GTS, /// The requested timestamp clock source is not supported on this target. /// - /// *NOTE*: `ITM_TCR.GTSFREQ` field has potentially been changed on - /// this error. + /// *NOTE*: `GTSFREQ` in [`ITM_TCR`](struct@Tcr) field has + /// potentially been changed on this error. TimestampClkSrc, /// The target does not implement the local timestamp prescaler. - /// Request [LocalTimestampOptions::Disabled] or - /// [LocalTimestampOptions::Disabled] instead. + /// Request [`LocalTimestampOptions::Disabled`] or + /// [`LocalTimestampOptions::Disabled`] instead. /// - /// *NOTE*: `ITM_TCR.{GTSFREQ,SWOENA}` fields have potentially - /// changed on this error. + /// *NOTE*: `GTSFREQ` and `SWOENA` in [`ITM_TCR`](struct@Tcr) fields + /// have potentially changed on this error. TSPrescale, } impl ITM { - /// Removes the software lock on the ITM. Must be called before any other [ITM] functions. + /// Removes the software lock on the [`ITM`]. Must be called before any other [`ITM`] functions. #[inline] pub fn unlock(&mut self) { // NOTE(unsafe) atomic write to a stateless, write-only register unsafe { self.lar.write(0xC5AC_CE55) } } - /// Indicates whether the ITM is currently processing events. - /// Returns `true` if ITM events are present and are being drained. + /// Indicates whether the [`ITM`] is currently processing events. + /// Returns `true` if [`ITM`] events are present and are being drained. #[inline] pub fn busy(&self) -> bool { self.tcr.read().busy() } - /// Configures the ITM with the passed [ITMSettings]. Returns `true` + /// Configures the [`ITM`] with the passed [`ITMSettings`]. Returns `true` /// if the configuration was successfully applied. #[allow(clippy::missing_inline_in_public_items)] pub fn configure(&mut self, settings: ITMSettings) -> Result<(), ITMConfigurationError> { From c856ff712c723083578f4ae93403d0ec4d5fc8ce Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Thu, 13 Jan 2022 17:15:22 +0100 Subject: [PATCH 05/18] itm: add lock function, improve unlock documentation --- src/peripheral/itm.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/peripheral/itm.rs b/src/peripheral/itm.rs index 8533af33..3a9d9d53 100644 --- a/src/peripheral/itm.rs +++ b/src/peripheral/itm.rs @@ -1,5 +1,8 @@ //! Instrumentation Trace Macrocell //! +//! The documentation in this module contains references to ARM specifications, namely: +//! - coresight: [*ARM CoreSight Architecture Specification*, Version 3.0](https://developer.arm.com/documentation/ihi0029/latest). +//! //! *NOTE* Not available on Armv6-M and Armv8-M Baseline. use core::cell::UnsafeCell; @@ -198,12 +201,23 @@ pub enum ITMConfigurationError { impl ITM { /// Removes the software lock on the [`ITM`]. Must be called before any other [`ITM`] functions. + /// + /// See (coresight, B2.3.10). #[inline] pub fn unlock(&mut self) { // NOTE(unsafe) atomic write to a stateless, write-only register unsafe { self.lar.write(0xC5AC_CE55) } } + /// Adds the software lock on the [`ITM`]. Should be called after any other [`ITM`] functions. + /// + /// See (coresight, B2.3.10). + #[inline] + pub fn lock(&mut self) { + // NOTE(unsafe) atomic write to a stateless, write-only register + unsafe { self.lar.write(0) } + } + /// Indicates whether the [`ITM`] is currently processing events. /// Returns `true` if [`ITM`] events are present and are being drained. #[inline] From 570ce0aefb4fdf6cc256807ab5bd50c576c2cdf4 Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Thu, 13 Jan 2022 17:21:25 +0100 Subject: [PATCH 06/18] CHANGELOG: update for added ITM API --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1e61b6b..b6d405c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). Also fixes `VectActive::from` to take a `u16` and subtract `16` for `VectActive::Interrupt`s to match `SBC::vect_active()` (#373). - DWT: add `configure` API for address, cycle count comparison (#342, #367). -- ITM: add `configure` API (#342). +- ITM: add `configure` API; `lock`, `unlock`, and `busy` functions (#342, #383). - TPIU: add API for *Formatter and Flush Control* (FFCR) and *Selected Pin Control* (SPPR) registers (#342). - Add `std` and `serde` crate features for improved host-side ITM decode functionality when working with the downstream `itm`, `cargo-rtic-scope` crates (#363, #366). From 61ee6edd48d5619b11c21647b8e17b93539b1f89 Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Thu, 13 Jan 2022 17:34:03 +0100 Subject: [PATCH 07/18] itm: bitfield LSR; add has_software_lock, locked; amend docs Related to #392. --- CHANGELOG.md | 2 +- src/peripheral/itm.rs | 35 ++++++++++++++++++++++++++++++++--- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6d405c6..0a91cb8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). Also fixes `VectActive::from` to take a `u16` and subtract `16` for `VectActive::Interrupt`s to match `SBC::vect_active()` (#373). - DWT: add `configure` API for address, cycle count comparison (#342, #367). -- ITM: add `configure` API; `lock`, `unlock`, and `busy` functions (#342, #383). +- ITM: add `configure` API; `lock`, `unlock`, `busy`, `locked` functions (#342, #383). - TPIU: add API for *Formatter and Flush Control* (FFCR) and *Selected Pin Control* (SPPR) registers (#342). - Add `std` and `serde` crate features for improved host-side ITM decode functionality when working with the downstream `itm`, `cargo-rtic-scope` crates (#363, #366). diff --git a/src/peripheral/itm.rs b/src/peripheral/itm.rs index 3a9d9d53..25f8256c 100644 --- a/src/peripheral/itm.rs +++ b/src/peripheral/itm.rs @@ -34,7 +34,7 @@ pub struct RegisterBlock { /// Lock Access pub lar: WO, /// Lock Status - pub lsr: RO, + pub lsr: RO, } bitfield! { @@ -53,6 +53,15 @@ bitfield! { busy, _: 23; } +bitfield! { + /// Software Lock Status Register + #[repr(C)] + #[derive(Copy, Clone)] + pub struct Lsr(u32); + sli, _: 0; + slk, _: 1; +} + /// Stimulus Port pub struct Stim { register: UnsafeCell, @@ -200,7 +209,9 @@ pub enum ITMConfigurationError { } impl ITM { - /// Removes the software lock on the [`ITM`]. Must be called before any other [`ITM`] functions. + /// Removes the software lock on the [`ITM`]. Must be called before + /// any mutating [`ITM`] functions if a software lock mechanism is + /// implemented. See [`has_software_lock`]. /// /// See (coresight, B2.3.10). #[inline] @@ -209,7 +220,7 @@ impl ITM { unsafe { self.lar.write(0xC5AC_CE55) } } - /// Adds the software lock on the [`ITM`]. Should be called after any other [`ITM`] functions. + /// Adds the software lock on the [`ITM`]. Should be called after any other mutating [`ITM`] functions. /// /// See (coresight, B2.3.10). #[inline] @@ -218,6 +229,24 @@ impl ITM { unsafe { self.lar.write(0) } } + /// Checks whether the target implements the software lock + /// mechanism. If `true`, [`unlock`] must be called before any other + /// mutating [`ITM`] functions. + /// + /// See (coresight, B2.3.10). + #[inline] + pub fn has_software_lock(&self) -> bool { + self.lsr.read().sli() + } + + /// Checks whether the peripheral is locked. + /// + /// See (coresight, B2.3.10). + #[inline] + pub fn locked(&self) -> bool { + self.lsr.read().slk() + } + /// Indicates whether the [`ITM`] is currently processing events. /// Returns `true` if [`ITM`] events are present and are being drained. #[inline] From e7b4a454e09ddbab4ee2469e899812b4c24caa50 Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Thu, 13 Jan 2022 17:42:13 +0100 Subject: [PATCH 08/18] itm: configure: unlock and disable ITM before modifying, then lock --- src/peripheral/itm.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/peripheral/itm.rs b/src/peripheral/itm.rs index 25f8256c..b3e8bd9f 100644 --- a/src/peripheral/itm.rs +++ b/src/peripheral/itm.rs @@ -260,6 +260,25 @@ impl ITM { pub fn configure(&mut self, settings: ITMSettings) -> Result<(), ITMConfigurationError> { use ITMConfigurationError as Error; + // The ITM must be unlocked before we apply any changes. + if self.has_software_lock() && self.locked() { + self.unlock(); + while self.locked() {} + } + + // The ITM must then be disabled before altering certain fields + // in order to avoid trace stream corruption. + // + // NOTE: this is only required before modifying the TraceBusID + // field, but better be on the safe side for now. + unsafe { + self.tcr.modify(|mut r| { + r.set_itmena(false); + r + }); + while self.busy() {} + } + unsafe { self.tcr.modify(|mut r| { r.set_gtsfreq(match settings.global_timestamps { @@ -330,6 +349,10 @@ impl ITM { }); } + if self.has_software_lock() { + self.lock(); + } + Ok(()) } } From 4e1230ec3c4a39294cc526bb8655131b97b06d08 Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Thu, 13 Jan 2022 17:44:42 +0100 Subject: [PATCH 09/18] itm: configure: set ITMENA after TraceBusID Related to #392. --- src/peripheral/itm.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/peripheral/itm.rs b/src/peripheral/itm.rs index b3e8bd9f..db08b84d 100644 --- a/src/peripheral/itm.rs +++ b/src/peripheral/itm.rs @@ -340,11 +340,14 @@ impl ITM { unsafe { self.tcr.modify(|mut r| { - r.set_itmena(settings.enable); r.set_tsena(settings.local_timestamps != LocalTimestampOptions::Disabled); r.set_txena(settings.forward_dwt); // forward hardware event packets from the DWT to the ITM r.set_tracebusid(settings.bus_id.unwrap_or(0)); + // must be modified after TraceBusID, see last section in + // + r.set_itmena(settings.enable); + r }); } From 2c99e80e2ca18fdade8c8d0a7455a4c56db6d45f Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Fri, 14 Jan 2022 14:12:35 +0100 Subject: [PATCH 10/18] itm: update documentation --- src/peripheral/itm.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/peripheral/itm.rs b/src/peripheral/itm.rs index db08b84d..83558414 100644 --- a/src/peripheral/itm.rs +++ b/src/peripheral/itm.rs @@ -209,9 +209,9 @@ pub enum ITMConfigurationError { } impl ITM { - /// Removes the software lock on the [`ITM`]. Must be called before - /// any mutating [`ITM`] functions if a software lock mechanism is - /// implemented. See [`has_software_lock`]. + /// Disengage the software lock on the [`ITM`]. Must be called + /// before any mutating [`ITM`] functions if a software lock + /// mechanism is implemented. See [`has_software_lock`]. /// /// See (coresight, B2.3.10). #[inline] @@ -220,7 +220,8 @@ impl ITM { unsafe { self.lar.write(0xC5AC_CE55) } } - /// Adds the software lock on the [`ITM`]. Should be called after any other mutating [`ITM`] functions. + /// Engages the software lock on the [`ITM`]. Should be called after + /// any other mutating [`ITM`] functions. /// /// See (coresight, B2.3.10). #[inline] @@ -254,8 +255,7 @@ impl ITM { self.tcr.read().busy() } - /// Configures the [`ITM`] with the passed [`ITMSettings`]. Returns `true` - /// if the configuration was successfully applied. + /// Tries to configure the [`ITM`] with the passed [`ITMSettings`]. #[allow(clippy::missing_inline_in_public_items)] pub fn configure(&mut self, settings: ITMSettings) -> Result<(), ITMConfigurationError> { use ITMConfigurationError as Error; From 189c3e4dec4092dac6435af4955fd71c931bc0f3 Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Fri, 14 Jan 2022 15:01:50 +0100 Subject: [PATCH 11/18] dcb: improve enable_trace documentation --- src/peripheral/dcb.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/peripheral/dcb.rs b/src/peripheral/dcb.rs index ef879ac6..854d3559 100644 --- a/src/peripheral/dcb.rs +++ b/src/peripheral/dcb.rs @@ -3,6 +3,8 @@ use volatile_register::{RW, WO}; use crate::peripheral::DCB; +#[allow(unused_imports)] +use crate::peripheral::{DWT, ITM}; use core::ptr; const DCB_DEMCR_TRCENA: u32 = 1 << 24; @@ -22,10 +24,7 @@ pub struct RegisterBlock { } impl DCB { - /// Enables TRACE. This is for example required by the - /// `peripheral::DWT` cycle counter to work properly. - /// As by STM documentation, this flag is not reset on - /// soft-reset, only on power reset. + /// Global enable for all [`DWT`] and [`ITM`] features. /// /// Note: vendor-specific registers may have to be set to completely /// enable tracing. For example, on the STM32F401RE, `TRACE_MODE` From 1e85dd0a7bbc1b2eadbfccbf67943a804f0b859b Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Fri, 14 Jan 2022 15:09:28 +0100 Subject: [PATCH 12/18] itm: fix intra-doc links --- src/peripheral/itm.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/peripheral/itm.rs b/src/peripheral/itm.rs index 83558414..421e8cad 100644 --- a/src/peripheral/itm.rs +++ b/src/peripheral/itm.rs @@ -211,7 +211,8 @@ pub enum ITMConfigurationError { impl ITM { /// Disengage the software lock on the [`ITM`]. Must be called /// before any mutating [`ITM`] functions if a software lock - /// mechanism is implemented. See [`has_software_lock`]. + /// mechanism is implemented. See + /// [`has_software_lock`](ITM::has_software_lock). /// /// See (coresight, B2.3.10). #[inline] @@ -231,8 +232,8 @@ impl ITM { } /// Checks whether the target implements the software lock - /// mechanism. If `true`, [`unlock`] must be called before any other - /// mutating [`ITM`] functions. + /// mechanism. If `true`, [`unlock`](ITM::unlock) must be called + /// before any other mutating [`ITM`] functions. /// /// See (coresight, B2.3.10). #[inline] From a9a6f558e85d1defc5e970275ca878006bd9ef1a Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Fri, 14 Jan 2022 15:10:13 +0100 Subject: [PATCH 13/18] itm: ITMSettings -> ITMConfiguration --- src/peripheral/itm.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/peripheral/itm.rs b/src/peripheral/itm.rs index 421e8cad..e8fa3f25 100644 --- a/src/peripheral/itm.rs +++ b/src/peripheral/itm.rs @@ -169,7 +169,7 @@ pub enum TimestampClkSrc { /// Available settings for the ITM peripheral. #[derive(Debug, Eq, PartialEq, Copy, Clone)] -pub struct ITMSettings { +pub struct ITMConfiguration { /// Whether to enable ITM. pub enable: bool, /// Whether DWT packets should be forwarded to ITM. @@ -256,9 +256,9 @@ impl ITM { self.tcr.read().busy() } - /// Tries to configure the [`ITM`] with the passed [`ITMSettings`]. + /// Tries to configure the [`ITM`] with the passed [`ITMConfiguration`]. #[allow(clippy::missing_inline_in_public_items)] - pub fn configure(&mut self, settings: ITMSettings) -> Result<(), ITMConfigurationError> { + pub fn configure(&mut self, settings: ITMConfiguration) -> Result<(), ITMConfigurationError> { use ITMConfigurationError as Error; // The ITM must be unlocked before we apply any changes. From f2a1d92e45c4d0d2111d772e4938ba20b5ebf450 Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Sat, 15 Jan 2022 19:24:01 +0100 Subject: [PATCH 14/18] dcb: dont import for intra-doc links --- src/peripheral/dcb.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/peripheral/dcb.rs b/src/peripheral/dcb.rs index 854d3559..4e614423 100644 --- a/src/peripheral/dcb.rs +++ b/src/peripheral/dcb.rs @@ -3,8 +3,6 @@ use volatile_register::{RW, WO}; use crate::peripheral::DCB; -#[allow(unused_imports)] -use crate::peripheral::{DWT, ITM}; use core::ptr; const DCB_DEMCR_TRCENA: u32 = 1 << 24; @@ -24,7 +22,8 @@ pub struct RegisterBlock { } impl DCB { - /// Global enable for all [`DWT`] and [`ITM`] features. + /// Global enable for all [`DWT`](crate::peripheral::DWT) and + /// [`ITM`](crate::peripheral::ITM) features. /// /// Note: vendor-specific registers may have to be set to completely /// enable tracing. For example, on the STM32F401RE, `TRACE_MODE` From 7b2dbb0120d80605d2de24868de7b5fcd9eb4991 Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Sat, 15 Jan 2022 19:25:12 +0100 Subject: [PATCH 15/18] itm: configure: note that peripheral is not relocked on Err --- src/peripheral/itm.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/peripheral/itm.rs b/src/peripheral/itm.rs index e8fa3f25..d3e5a401 100644 --- a/src/peripheral/itm.rs +++ b/src/peripheral/itm.rs @@ -256,7 +256,9 @@ impl ITM { self.tcr.read().busy() } - /// Tries to configure the [`ITM`] with the passed [`ITMConfiguration`]. + /// Tries to configure the [`ITM`] with the passed + /// [`ITMConfiguration`]. Handles register unlocks. On `Err`, the + /// [`ITM`] will not be relocked. #[allow(clippy::missing_inline_in_public_items)] pub fn configure(&mut self, settings: ITMConfiguration) -> Result<(), ITMConfigurationError> { use ITMConfigurationError as Error; From c8b8ca439f8a0f6da3764b554b45c33606636487 Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Sat, 15 Jan 2022 21:56:03 +0100 Subject: [PATCH 16/18] itm: streamline lock, unlock --- src/peripheral/itm.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/peripheral/itm.rs b/src/peripheral/itm.rs index d3e5a401..34d59fcc 100644 --- a/src/peripheral/itm.rs +++ b/src/peripheral/itm.rs @@ -217,8 +217,16 @@ impl ITM { /// See (coresight, B2.3.10). #[inline] pub fn unlock(&mut self) { + if !self.locked() { + return; + } + // NOTE(unsafe) atomic write to a stateless, write-only register - unsafe { self.lar.write(0xC5AC_CE55) } + unsafe { + self.lar.write(0xC5AC_CE55); + } + + while self.locked() {} } /// Engages the software lock on the [`ITM`]. Should be called after @@ -227,8 +235,14 @@ impl ITM { /// See (coresight, B2.3.10). #[inline] pub fn lock(&mut self) { + if self.locked() { + return; + } + // NOTE(unsafe) atomic write to a stateless, write-only register unsafe { self.lar.write(0) } + + while !self.locked() {} } /// Checks whether the target implements the software lock @@ -246,7 +260,7 @@ impl ITM { /// See (coresight, B2.3.10). #[inline] pub fn locked(&self) -> bool { - self.lsr.read().slk() + self.has_software_lock() && self.lsr.read().slk() } /// Indicates whether the [`ITM`] is currently processing events. @@ -264,10 +278,7 @@ impl ITM { use ITMConfigurationError as Error; // The ITM must be unlocked before we apply any changes. - if self.has_software_lock() && self.locked() { - self.unlock(); - while self.locked() {} - } + self.unlock(); // The ITM must then be disabled before altering certain fields // in order to avoid trace stream corruption. @@ -355,9 +366,7 @@ impl ITM { }); } - if self.has_software_lock() { - self.lock(); - } + self.lock(); Ok(()) } From c80da84927cfcc4abb8664d8fdc7a21a945863a3 Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Sat, 15 Jan 2022 21:57:05 +0100 Subject: [PATCH 17/18] itm: hide has_software_lock, locked There functions are internally queried inside lock, unlock. --- CHANGELOG.md | 2 +- src/peripheral/itm.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a91cb8b..78fb8bd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). Also fixes `VectActive::from` to take a `u16` and subtract `16` for `VectActive::Interrupt`s to match `SBC::vect_active()` (#373). - DWT: add `configure` API for address, cycle count comparison (#342, #367). -- ITM: add `configure` API; `lock`, `unlock`, `busy`, `locked` functions (#342, #383). +- ITM: add `configure` API; `lock`, `unlock`, `busy` functions (#342, #383). - TPIU: add API for *Formatter and Flush Control* (FFCR) and *Selected Pin Control* (SPPR) registers (#342). - Add `std` and `serde` crate features for improved host-side ITM decode functionality when working with the downstream `itm`, `cargo-rtic-scope` crates (#363, #366). diff --git a/src/peripheral/itm.rs b/src/peripheral/itm.rs index 34d59fcc..2f49456c 100644 --- a/src/peripheral/itm.rs +++ b/src/peripheral/itm.rs @@ -251,7 +251,7 @@ impl ITM { /// /// See (coresight, B2.3.10). #[inline] - pub fn has_software_lock(&self) -> bool { + fn has_software_lock(&self) -> bool { self.lsr.read().sli() } @@ -259,7 +259,7 @@ impl ITM { /// /// See (coresight, B2.3.10). #[inline] - pub fn locked(&self) -> bool { + fn locked(&self) -> bool { self.has_software_lock() && self.lsr.read().slk() } From e18b52db64d15db9d6b5fcb021424621e06966c8 Mon Sep 17 00:00:00 2001 From: Viktor Sonesten Date: Sat, 15 Jan 2022 23:09:00 +0100 Subject: [PATCH 18/18] itm: make ITMConfiguration non-exhaustive At least one more error enum will be added in the future. See #393. --- src/peripheral/itm.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/peripheral/itm.rs b/src/peripheral/itm.rs index 2f49456c..224bfbfc 100644 --- a/src/peripheral/itm.rs +++ b/src/peripheral/itm.rs @@ -188,6 +188,7 @@ pub struct ITMConfiguration { /// Possible errors on [ITM::configure]. #[derive(Debug, Eq, PartialEq, Copy, Clone)] +#[non_exhaustive] pub enum ITMConfigurationError { /// Global timestamp generation is not supported on this target. /// Request [`GlobalTimestampOptions::Disabled`] instead.