From 48d82c096d09e2fccf754391714cdb1c1500e95c Mon Sep 17 00:00:00 2001
From: Brad Campbell <bradjc5@gmail.com>
Date: Thu, 10 Oct 2024 12:31:21 -0400
Subject: [PATCH 1/4] tock regs: document macros

---
 .../tock-register-interface/src/fields.rs     | 29 +++++++++++++++++++
 .../tock-register-interface/src/macros.rs     | 29 +++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/libraries/tock-register-interface/src/fields.rs b/libraries/tock-register-interface/src/fields.rs
index 87575c5290..06147faf3e 100644
--- a/libraries/tock-register-interface/src/fields.rs
+++ b/libraries/tock-register-interface/src/fields.rs
@@ -555,6 +555,35 @@ macro_rules! register_bitmasks {
 }
 
 /// Define register types and fields.
+///
+/// Implementations of memory-mapped registers can use this macro to define the
+/// structure and bitwise meaning of individual registers in the peripheral. An
+/// example use for a hypothetical UART driver might look like:
+///
+/// ```rust,ignore
+/// register_bitfields![u32,
+///     CONTROL [
+///         ENABLE OFFSET(0) NUMBITS(1),
+///         STOP_BITS OFFSET(1) NUMBITS(2) [
+///             StopBits1 = 0,
+///             StopBits2 = 1,
+///             StopBits0 = 2
+///         ]
+///     ],
+///     BYTE [
+///         CHARACTER OFFSET(0) NUMBITS(8)
+///     ],
+///     INTERRUPT [
+///         TRANSMITTED OFFSET(0) NUMBITS(1),
+///         RECEIVED OFFSET(1) NUMBITS(1),
+///         FIFO_FULL OFFSET(2) NUMBITS(1)
+///     ]
+/// ];
+/// ```
+///
+/// Each field in the register can be identified by its offset within the
+/// register and its bitwidth. Fields that have discrete options with semantic
+/// meaning can be enumerated.
 #[macro_export]
 macro_rules! register_bitfields {
     {
diff --git a/libraries/tock-register-interface/src/macros.rs b/libraries/tock-register-interface/src/macros.rs
index f65d8b78cc..fda318fbcf 100644
--- a/libraries/tock-register-interface/src/macros.rs
+++ b/libraries/tock-register-interface/src/macros.rs
@@ -358,6 +358,35 @@ macro_rules! test_fields {
     };
 }
 
+/// Define a peripheral memory map containing registers.
+///
+/// Implementations of memory-mapped registers can use this macro to define the
+/// individual registers in the peripheral and their relative address offset
+/// from the start of the peripheral's mapped address. An example use for a
+/// hypothetical UART driver might look like:
+///
+/// ```rust,ignore
+/// register_structs! {
+///     pub UartRegisters {
+///         (0x00 => control: ReadWrite<u32, CONTROL::Register>),
+///         (0x04 => write_byte: ReadWrite<u32, BYTE::Register>),
+///         (0x08 => _reserved1),
+///         (0x20 => interrupt_enable: ReadWrite<u32, INTERRUPT::Register>),
+///         (0x24 => interrupt_status: ReadWrite<u32, INTERRUPT::Register>),
+///         (0x28 => @END),
+///     }
+/// }
+/// ```
+///
+/// By convention, gaps in the register memory map are named `_reserved`. The
+/// macro will automatically compute the size of the reserved field so that the
+/// next register is at the correct address.
+///
+/// The size of the register is denoted by the first parameter in the
+/// [`ReadWrite`](crate::registers::ReadWrite) type. The second parameter in the
+/// [`ReadWrite`](crate::registers::ReadWrite) type is a register definition
+/// which is specified with the
+/// [`register_bitfields!()`](crate::register_bitfields) macro.
 #[macro_export]
 macro_rules! register_structs {
     {

From 8f2f38f8e0561f77782793f96f7f1b8e6574acae Mon Sep 17 00:00:00 2001
From: Brad Campbell <bradjc5@gmail.com>
Date: Thu, 10 Oct 2024 12:31:47 -0400
Subject: [PATCH 2/4] kernel: utilites: update documentation

Add documentation for missing types, add rustdoc links, wrap lines,
format header comments.
---
 kernel/src/utilities/copy_slice.rs            | 51 ++++++++++++------
 kernel/src/utilities/helpers.rs               |  7 ++-
 kernel/src/utilities/leasable_buffer.rs       |  7 ++-
 kernel/src/utilities/math.rs                  | 46 ++++++++--------
 kernel/src/utilities/mod.rs                   | 12 ++++-
 kernel/src/utilities/mut_imut_buffer.rs       | 52 +++++++++----------
 kernel/src/utilities/peripheral_management.rs | 40 ++++++++------
 kernel/src/utilities/static_init.rs           | 51 +++++++++++++-----
 kernel/src/utilities/static_ref.rs            |  8 +--
 kernel/src/utilities/storage_volume.rs        | 23 ++++----
 10 files changed, 177 insertions(+), 120 deletions(-)

diff --git a/kernel/src/utilities/copy_slice.rs b/kernel/src/utilities/copy_slice.rs
index 03471990f9..ff982e7081 100644
--- a/kernel/src/utilities/copy_slice.rs
+++ b/kernel/src/utilities/copy_slice.rs
@@ -2,21 +2,38 @@
 // SPDX-License-Identifier: Apache-2.0 OR MIT
 // Copyright Tock Contributors 2022.
 
+//! Helper functions for copying buffers.
+//!
+//! This utility provides an implementation of the standard Rust
+//! [`slice::copy_from_slice()`] method that cannot panic. This method is
+//! provided through the [`CopyOrErr`] trait.
+//!
+//! This functionality is currently provided for the following types:
+//! - `u8`
+//! - `u16`
+//! - `u32`
+//! - `u64`
+//! - `usize`
+
 use crate::ErrorCode;
 use core::ptr;
 
+/// Interface for copying buffers that cannot panic.
 pub trait CopyOrErr {
-    /// Copies a nonoverlapping slice from src to self. Returns Err(ErrorCode) if source and self
-    /// are not the same length. This is a non-panicing version of slice::copy_from_slice.
+    /// Copy a non-overlapping slice from `src` to `self`.
+    ///
+    /// This is a non-panicing version of [`slice::copy_from_slice`].
+    ///
+    /// Returns `Err(ErrorCode)` if `src` and `self` are not the same length.
     fn copy_from_slice_or_err(&mut self, src: &Self) -> Result<(), ErrorCode>;
 }
 
 impl CopyOrErr for [u8] {
     fn copy_from_slice_or_err(&mut self, src: &Self) -> Result<(), ErrorCode> {
         if self.len() == src.len() {
-            // SAFETY: `self` is valid for `self.len()` elements by definition, and `src` was
-            // checked to have the same length. The slices cannot overlap because
-            // mutable references are exclusive.
+            // SAFETY: `self` is valid for `self.len()` elements by definition,
+            // and `src` was checked to have the same length. The slices cannot
+            // overlap because mutable references are exclusive.
             unsafe {
                 ptr::copy_nonoverlapping(src.as_ptr(), self.as_mut_ptr(), self.len());
             }
@@ -30,9 +47,9 @@ impl CopyOrErr for [u8] {
 impl CopyOrErr for [u16] {
     fn copy_from_slice_or_err(&mut self, src: &Self) -> Result<(), ErrorCode> {
         if self.len() == src.len() {
-            // SAFETY: `self` is valid for `self.len()` elements by definition, and `src` was
-            // checked to have the same length. The slices cannot overlap because
-            // mutable references are exclusive.
+            // SAFETY: `self` is valid for `self.len()` elements by definition,
+            // and `src` was checked to have the same length. The slices cannot
+            // overlap because mutable references are exclusive.
             unsafe {
                 ptr::copy_nonoverlapping(src.as_ptr(), self.as_mut_ptr(), self.len());
             }
@@ -46,9 +63,9 @@ impl CopyOrErr for [u16] {
 impl CopyOrErr for [u32] {
     fn copy_from_slice_or_err(&mut self, src: &Self) -> Result<(), ErrorCode> {
         if self.len() == src.len() {
-            // SAFETY: `self` is valid for `self.len()` elements by definition, and `src` was
-            // checked to have the same length. The slices cannot overlap because
-            // mutable references are exclusive.
+            // SAFETY: `self` is valid for `self.len()` elements by definition,
+            // and `src` was checked to have the same length. The slices cannot
+            // overlap because mutable references are exclusive.
             unsafe {
                 ptr::copy_nonoverlapping(src.as_ptr(), self.as_mut_ptr(), self.len());
             }
@@ -62,9 +79,9 @@ impl CopyOrErr for [u32] {
 impl CopyOrErr for [u64] {
     fn copy_from_slice_or_err(&mut self, src: &Self) -> Result<(), ErrorCode> {
         if self.len() == src.len() {
-            // SAFETY: `self` is valid for `self.len()` elements by definition, and `src` was
-            // checked to have the same length. The slices cannot overlap because
-            // mutable references are exclusive.
+            // SAFETY: `self` is valid for `self.len()` elements by definition,
+            // and `src` was checked to have the same length. The slices cannot
+            // overlap because mutable references are exclusive.
             unsafe {
                 ptr::copy_nonoverlapping(src.as_ptr(), self.as_mut_ptr(), self.len());
             }
@@ -78,9 +95,9 @@ impl CopyOrErr for [u64] {
 impl CopyOrErr for [usize] {
     fn copy_from_slice_or_err(&mut self, src: &Self) -> Result<(), ErrorCode> {
         if self.len() == src.len() {
-            // SAFETY: `self` is valid for `self.len()` elements by definition, and `src` was
-            // checked to have the same length. The slices cannot overlap because
-            // mutable references are exclusive.
+            // SAFETY: `self` is valid for `self.len()` elements by definition,
+            // and `src` was checked to have the same length. The slices cannot
+            // overlap because mutable references are exclusive.
             unsafe {
                 ptr::copy_nonoverlapping(src.as_ptr(), self.as_mut_ptr(), self.len());
             }
diff --git a/kernel/src/utilities/helpers.rs b/kernel/src/utilities/helpers.rs
index 6fe2575367..ce8b7dbb22 100644
--- a/kernel/src/utilities/helpers.rs
+++ b/kernel/src/utilities/helpers.rs
@@ -2,7 +2,12 @@
 // SPDX-License-Identifier: Apache-2.0 OR MIT
 // Copyright Tock Contributors 2022.
 
-//! Helper macros.
+//! Helper functions and macros.
+//!
+//! These are various utility functions and macros that are useful throughout
+//! the Tock kernel and are provided here for convenience.
+//!
+//! The macros are exported through the top level of the `kernel` crate.
 
 /// Create an object with the given capability.
 ///
diff --git a/kernel/src/utilities/leasable_buffer.rs b/kernel/src/utilities/leasable_buffer.rs
index 68ff35de3a..c3d8bdaf61 100644
--- a/kernel/src/utilities/leasable_buffer.rs
+++ b/kernel/src/utilities/leasable_buffer.rs
@@ -143,9 +143,8 @@
 //! will be called prior to passing the buffer down to lower layers, and
 //! `reset()` will be called once the `SubSlice` is returned via a callback.
 //!
-//!  ```rust
+//! ```rust
 //! # use kernel::utilities::leasable_buffer::SubSlice;
-//!
 //! let mut internal = ['a', 'b', 'c', 'd'];
 //! let original_base_addr = internal.as_ptr();
 //!
@@ -164,8 +163,8 @@
 //! assert_eq!((buffer[0], buffer[1]), ('a', 'b'));
 //!
 //!  ```
-//!
-//! Author: Amit Levy
+
+// Author: Amit Levy
 
 use core::ops::{Bound, Range, RangeBounds};
 use core::ops::{Index, IndexMut};
diff --git a/kernel/src/utilities/math.rs b/kernel/src/utilities/math.rs
index 041aef07cb..baba99ecae 100644
--- a/kernel/src/utilities/math.rs
+++ b/kernel/src/utilities/math.rs
@@ -18,10 +18,10 @@ pub fn closest_power_of_two(mut num: u32) -> u32 {
     num
 }
 
+/// Represents an integral power-of-two as an exponent.
 #[derive(Copy, Clone, Debug, PartialEq, PartialOrd, Eq, Ord)]
 pub struct PowerOfTwo(u32);
 
-/// Represents an integral power-of-two as an exponent
 impl PowerOfTwo {
     /// Returns the base-2 exponent as a numeric type
     pub fn exp<R>(self) -> R
@@ -31,31 +31,32 @@ impl PowerOfTwo {
         From::from(self.0)
     }
 
-    /// Converts a number two the nearest `PowerOfTwo` less-than-or-equal to it.
+    /// Converts a number two the nearest [`PowerOfTwo`] less-than-or-equal to it.
     pub fn floor<F: Into<u32>>(f: F) -> PowerOfTwo {
         PowerOfTwo(log_base_two(f.into()))
     }
 
-    /// Converts a number two the nearest `PowerOfTwo` greater-than-or-equal to
+    /// Converts a number two the nearest [`PowerOfTwo`] greater-than-or-equal to
     /// it.
     pub fn ceiling<F: Into<u32>>(f: F) -> PowerOfTwo {
         PowerOfTwo(log_base_two(closest_power_of_two(f.into())))
     }
 
-    /// Creates a new `PowerOfTwo` representing the number zero.
+    /// Creates a new [`PowerOfTwo`] representing the number zero.
     pub fn zero() -> PowerOfTwo {
         PowerOfTwo(0)
     }
 
-    /// Converts a `PowerOfTwo` to a number.
+    /// Converts a [`PowerOfTwo`] to a number.
     pub fn as_num<F: From<u32>>(self) -> F {
         (1 << self.0).into()
     }
 }
 
 /// Get log base 2 of a number.
+///
 /// Note: this is the floor of the result. Also, an input of 0 results in an
-/// output of 0
+/// output of 0.
 pub fn log_base_two(num: u32) -> u32 {
     if num == 0 {
         0
@@ -77,6 +78,7 @@ pub fn log_base_two_u64(num: u64) -> u32 {
 const EXPONENT_MASK: u32 = 0b01111111_10000000_00000000_00000000;
 const EXPONENT_BIAS: u32 = 127;
 
+/// Return the absolute value of the floating point number.
 pub fn abs(n: f32) -> f32 {
     f32::from_bits(n.to_bits() & 0x7FFF_FFFF)
 }
@@ -90,28 +92,29 @@ fn extract_exponent_value(x: f32) -> i32 {
 }
 
 fn ln_1to2_series_approximation(x: f32) -> f32 {
-    // idea from https://stackoverflow.com/a/44232045/
-    // modified to not be restricted to int range and only values of x above 1.0.
-    // and got rid of most of the slow conversions,
-    // should work for all positive values of x.
+    // Idea from https://stackoverflow.com/a/44232045/. Modified to not be
+    // restricted to int range and only values of x above 1.0 and got rid of
+    // most of the slow conversions, should work for all positive values of x.
 
-    //x may essentially be 1.0 but, as clippy notes, these kinds of
-    //floating point comparisons can fail when the bit pattern is not the sames
+    // x may essentially be 1.0 but, as clippy notes, these kinds of floating
+    // point comparisons can fail when the bit pattern is not the same.
     if abs(x - 1.0_f32) < f32::EPSILON {
         return 0.0_f32;
     }
     let x_less_than_1: bool = x < 1.0;
-    // Note: we could use the fast inverse approximation here found in super::inv::inv_approx, but
-    // the precision of such an approximation is assumed not good enough.
+    // Note: we could use the fast inverse approximation here found in
+    // super::inv::inv_approx, but the precision of such an approximation is
+    // assumed not good enough.
     let x_working: f32 = if x_less_than_1 { 1.0 / x } else { x };
-    //according to the SO post ln(x) = ln((2^n)*y)= ln(2^n) + ln(y) = ln(2) * n + ln(y)
-    //get exponent value
+    // According to the SO post:
+    // ln(x) = ln((2^n)*y)= ln(2^n) + ln(y) = ln(2) * n + ln(y)
+    // Get exponent value.
     let base2_exponent: u32 = extract_exponent_value(x_working) as u32;
     let divisor: f32 = f32::from_bits(x_working.to_bits() & EXPONENT_MASK);
-    //supposedly normalizing between 1.0 and 2.0
+    // Supposedly normalizing between 1.0 and 2.0.
     let x_working: f32 = x_working / divisor;
-    //approximate polynomial generated from maple in the post using Remez Algorithm:
-    //https://en.wikipedia.org/wiki/Remez_algorithm
+    // Approximate polynomial generated from maple in the post using Remez
+    // Algorithm: https://en.wikipedia.org/wiki/Remez_algorithm.
     let ln_1to2_polynomial: f32 = -1.741_793_9_f32
         + (2.821_202_6_f32
             + (-1.469_956_8_f32 + (0.447_179_55_f32 - 0.056_570_85_f32 * x_working) * x_working)
@@ -126,12 +129,11 @@ fn ln_1to2_series_approximation(x: f32) -> f32 {
     }
 }
 
+/// Compute the base 10 logarithm of `f`.
 pub fn log10(x: f32) -> f32 {
-    //using change of base log10(x) = ln(x)/ln(10)
+    // Using change of base log10(x) = ln(x)/ln(10)
     let ln10_recip = f32::consts::LOG10_E;
     let fract_base_ln = ln10_recip;
     let value_ln = ln_1to2_series_approximation(x);
     value_ln * fract_base_ln
 }
-
-//-----------------------------------------------------------
diff --git a/kernel/src/utilities/mod.rs b/kernel/src/utilities/mod.rs
index 5fc700c0fe..5f8f9dcd67 100644
--- a/kernel/src/utilities/mod.rs
+++ b/kernel/src/utilities/mod.rs
@@ -18,7 +18,13 @@ mod static_ref;
 
 pub use self::static_ref::StaticRef;
 
-/// Re-export the tock-register-interface library.
+/// The Tock Register Interface.
+///
+/// This is a re-export of the `tock-register-interface` crate provided for
+/// convenience.
+///
+/// The Tock Register Interface provides a mechanism for accessing hardware
+/// registers and MMIO interfaces.
 pub mod registers {
     pub use tock_registers::fields::{Field, FieldValue};
     pub use tock_registers::interfaces;
@@ -28,7 +34,9 @@ pub mod registers {
     pub use tock_registers::{LocalRegisterCopy, RegisterLongName};
 }
 
-/// Create a "fake" module inside of `common` for all of the Tock `Cell` types.
+/// The Tock `Cell` types.
+///
+/// This is a re-export of the `tock-cells` crate provided for convenience.
 ///
 /// To use `TakeCell`, for example, users should use:
 ///
diff --git a/kernel/src/utilities/mut_imut_buffer.rs b/kernel/src/utilities/mut_imut_buffer.rs
index 1c1f47a556..eb26150abc 100644
--- a/kernel/src/utilities/mut_imut_buffer.rs
+++ b/kernel/src/utilities/mut_imut_buffer.rs
@@ -2,36 +2,30 @@
 // SPDX-License-Identifier: Apache-2.0 OR MIT
 // Copyright Tock Contributors 2022.
 
-//! An enum that can contain a reference to either a mutable or
-//! an immutable buffer.
-//!
-//! This type is intended for internal use in implementations of HILs
-//! or abstractions which need to handle both mutable and immutable
-//! buffers.
-//!
-//! One motivating use case is keys for public key
-//! cryptography. Public keys are often distributed as constant values
-//! in the flash of a kernel image (e.g., to verify signatures), which
-//! requires they be immutable.  Copying them to RAM is expensive
-//! because these keys can be very large (e.g., 512 bytes for a
-//! 4096-bit RSA key). At the same time, some clients may use
-//! dynamically generated or received keys, which are stored in
-//! mutable RAM. Requiring that keys be immutable would
-//! discard mut on this memory. An
-//! implementation can use this type to store either mutable and
-//! immutable buffers. The OTBN (OpenTitan Big Number accelerator) is
-//! one example use of MutImutBuffer.
-//!
-//! Because this type requires dynamic runtime checks that types
-//! match, it should not be used in any HILs or standard, external
-//! APIs. It is intended only for internal use in implementations.
-//!
-//! Author: Alistair Francis
+//! An enum that can contain a reference to either a mutable or an immutable
+//! buffer.
+//!
+//! This type is intended for internal use in implementations of HILs or
+//! abstractions which need to handle both mutable and immutable buffers.
+//!
+//! One motivating use case is keys for public key cryptography. Public keys are
+//! often distributed as constant values in the flash of a kernel image (e.g.,
+//! to verify signatures), which requires they be immutable. Copying them to
+//! RAM is expensive because these keys can be very large (e.g., 512 bytes for a
+//! 4096-bit RSA key). At the same time, some clients may use dynamically
+//! generated or received keys, which are stored in mutable RAM. Requiring that
+//! keys be immutable would discard `mut` on this memory. An implementation can
+//! use this type to store either mutable and immutable buffers. The OTBN
+//! (OpenTitan Big Number accelerator) is one example use of [`MutImutBuffer`].
+//!
+//! Because this type requires dynamic runtime checks that types match, it
+//! should not be used in any HILs or standard, external APIs. It is intended
+//! only for internal use within implementations.
 //!
 //! Usage
 //! -----
 //!
-//!  ```rust
+//! ```rust
 //! use kernel::utilities::mut_imut_buffer::MutImutBuffer;
 //!
 //! let mut mutable = ['a', 'b', 'c', 'd'];
@@ -41,14 +35,16 @@
 //! let shared_buf2 = MutImutBuffer::Immutable(&immutable);
 //!  ```
 
-/// An enum which can hold either a mutable or an immutable buffer
+// Author: Alistair Francis
+
+/// An enum which can hold either a mutable or an immutable buffer.
 pub enum MutImutBuffer<'a, T> {
     Mutable(&'a mut [T]),
     Immutable(&'a [T]),
 }
 
 impl<'a, T> MutImutBuffer<'a, T> {
-    /// Returns the length of the underlying buffer
+    /// Returns the length of the underlying buffer.
     pub fn len(&self) -> usize {
         match self {
             MutImutBuffer::Mutable(buf) => buf.len(),
diff --git a/kernel/src/utilities/peripheral_management.rs b/kernel/src/utilities/peripheral_management.rs
index ac581456ce..4a9805def5 100644
--- a/kernel/src/utilities/peripheral_management.rs
+++ b/kernel/src/utilities/peripheral_management.rs
@@ -4,9 +4,9 @@
 
 //! Peripheral Management
 //!
-//! Most peripherals are implemented as memory mapped I/O (MMIO).
-//! Intrinsically, this means that accessing a peripheral requires
-//! dereferencing a raw pointer that points to the peripheral's memory.
+//! Most peripherals are implemented as memory mapped I/O (MMIO). Intrinsically,
+//! this means that accessing a peripheral requires dereferencing a raw pointer
+//! that points to the peripheral's memory.
 //!
 //! Generally, Tock peripherals are modeled by two structures, such as:
 //!
@@ -88,19 +88,20 @@
 //!
 //! Note, this example kept the `mmio_address` in the `PeripheralHardware`
 //! structure, which is useful when there are multiple copies of the same
-//! peripheral (e.g. multiple UARTs). For single-instance peripherals, it's
-//! fine to simply return the address directly from `get_registers`.
+//! peripheral (e.g. multiple UARTs). For single-instance peripherals, it's fine
+//! to simply return the address directly from `get_registers`.
 //!
 //! Peripheral Clocks
 //! -----------------
 //!
-//! To facilitate low-power operation, PeripheralManager captures the peripheral's
-//! clock upon instantiation. The intention is to exploit
-//! [Ownership Based Resource Management](https://doc.rust-lang.org/beta/nomicon/obrm.html)
-//! to capture peripheral power state.
+//! To facilitate low-power operation, PeripheralManager captures the
+//! peripheral's clock upon instantiation. The intention is to exploit
+//! [Ownership Based Resource
+//! Management](https://doc.rust-lang.org/beta/nomicon/obrm.html) to capture
+//! peripheral power state.
 //!
-//! To enable this, peripherals must inform the kernel which clock they use,
-//! and when the clock should be enabled and disabled. Implementations of the
+//! To enable this, peripherals must inform the kernel which clock they use, and
+//! when the clock should be enabled and disabled. Implementations of the
 //! `before/after_mmio_access` methods must take care to not access hardware
 //! without enabling clocks if needed if they use hardware for bookkeeping.
 //!
@@ -154,30 +155,35 @@ where
 {
     type RegisterType;
 
-    /// How to get a reference to the physical hardware registers (the MMIO struct).
+    /// How to get a reference to the physical hardware registers (the MMIO
+    /// struct).
     fn get_registers(&self) -> &Self::RegisterType;
 
     /// Which clock feeds this peripheral.
     ///
-    /// For peripherals with no clock, use `&::kernel::platform::chip::NO_CLOCK_CONTROL`.
+    /// For peripherals with no clock, use
+    /// `&::kernel::platform::chip::NO_CLOCK_CONTROL`.
     fn get_clock(&self) -> &C;
 
     /// Called before peripheral access.
     ///
-    /// Responsible for ensure the periphal can be safely accessed, e.g. that
+    /// Responsible for ensure the peripheral can be safely accessed, e.g. that
     /// its clock is powered on.
     fn before_peripheral_access(&self, _: &C, _: &Self::RegisterType);
 
-    /// Called after periphal access.
+    /// Called after peripheral access.
     ///
     /// Currently used primarily for power management to check whether the
     /// peripheral can be powered off.
     fn after_peripheral_access(&self, _: &C, _: &Self::RegisterType);
 }
 
+/// Object used to access memory mapped registers with the
+/// [`PeripheralManagement`] interface.
+///
 /// Structures encapsulating peripheral hardware (those implementing the
-/// PeripheralManagement trait) should instantiate an instance of this
-/// method to access memory mapped registers.
+/// [`PeripheralManagement`] trait) should instantiate an instance of this
+/// object.
 ///
 /// ```
 /// # use kernel::utilities::cells::VolatileCell;
diff --git a/kernel/src/utilities/static_init.rs b/kernel/src/utilities/static_init.rs
index 8c761ae843..5025fe03c7 100644
--- a/kernel/src/utilities/static_init.rs
+++ b/kernel/src/utilities/static_init.rs
@@ -28,15 +28,36 @@ macro_rules! static_init {
     }};
 }
 
-/// An `#[inline(never)]` function that panics internally if the passed reference
-/// is `true`. This function is intended for use within
-/// the `static_buf!()` macro, which removes the size bloat of track_caller
-/// saving the location of every single call to `static_init!()`.
-/// If you hit this panic, you are either calling `static_buf!()` in
-/// a loop or calling a function multiple times which internally
-/// contains a call to `static_buf!()`. Typically, calls to
-/// `static_buf!()` are hidden within calls to `static_init!()` or
-/// component helper macros, so start your search there.
+/// Internal helper function for [`static_buf!()`](crate::static_buf).
+///
+/// This must be public to work within the macro but should never be used
+/// directly.
+///
+/// This is a `#[inline(never)]` function that panics internally if the passed
+/// reference is `true`. This function is intended for use within the
+/// [`static_buf!()`](crate::static_buf) macro to detect multiple uses of the
+/// macro on the same buffer.
+///
+/// This function is implemented separately without inlining to removes the size
+/// bloat of track_caller saving the location of every single call to
+/// [`static_buf!()`](crate::static_buf) or
+/// [`static_init!()`](crate::static_init).
+///
+/// If Tock panics with this message:
+///
+/// ```text
+/// Error! Single static_buf!() called twice.
+/// ```
+///
+/// then you have tried to initialize the same static memory twice. This is
+/// prohibited because you would have multiple mutable references to the same
+/// memory. This is likely due to either calling
+/// [`static_buf!()`](crate::static_buf) in a loop or calling a function
+/// multiple times which internally contains a call to
+/// [`static_buf!()`](crate::static_buf). Typically, calls to
+/// [`static_buf!()`](crate::static_buf) are hidden within calls to
+/// [`static_init!()`](crate::static_init) or component helper macros, so start
+/// your search there.
 #[inline(never)]
 pub fn static_buf_check_used(used: &mut bool) {
     // Check if this `BUF` has already been declared and initialized. If it
@@ -53,12 +74,14 @@ pub fn static_buf_check_used(used: &mut bool) {
     }
 }
 
-/// Allocates a statically-sized global region of memory for data structures but
-/// does not initialize the memory. Checks that the buffer is not aliased and is
-/// only used once.
+/// Allocates an uninitialized statically-sized global region of memory for a
+/// data structure.
+///
+/// This macro allocates the static memory but does not initialize the memory.
+/// This also checks that the buffer is not aliased and is only used once.
 ///
 /// This macro creates the static buffer, and returns a
-/// `StaticUninitializedBuffer` wrapper containing the buffer. The memory is
+/// [`core::mem::MaybeUninit`] wrapper containing the buffer. The memory is
 /// allocated, but it is guaranteed to be uninitialized inside of the wrapper.
 ///
 /// Before the static buffer can be used it must be initialized. For example:
@@ -109,7 +132,7 @@ macro_rules! static_buf {
 /// only used once.
 ///
 /// This macro creates the static buffer, and returns a
-/// `StaticUninitializedBuffer` wrapper containing the buffer. The memory is
+/// [`core::mem::MaybeUninit`] wrapper containing the buffer. The memory is
 /// allocated, but it is guaranteed to be uninitialized inside of the wrapper.
 ///
 /// Before the static buffer can be used it must be initialized. For example:
diff --git a/kernel/src/utilities/static_ref.rs b/kernel/src/utilities/static_ref.rs
index d72196454c..cfa1f71575 100644
--- a/kernel/src/utilities/static_ref.rs
+++ b/kernel/src/utilities/static_ref.rs
@@ -13,7 +13,7 @@ use core::ptr::NonNull;
 /// This is a simple wrapper around a raw pointer that encapsulates an unsafe
 /// dereference in a safe manner. It serve the role of creating a `&'static T`
 /// given a raw address and acts similarly to `extern` definitions, except
-/// `StaticRef` is subject to module and crate boundaries, while `extern`
+/// [`StaticRef`] is subject to module and crate boundaries, while `extern`
 /// definitions can be imported anywhere.
 ///
 /// Because this defers the actual dereference, this can be put in a `const`,
@@ -26,7 +26,7 @@ pub struct StaticRef<T> {
 }
 
 impl<T> StaticRef<T> {
-    /// Create a new `StaticRef` from a raw pointer
+    /// Create a new [`StaticRef`] from a raw pointer
     ///
     /// ## Safety
     ///
@@ -51,8 +51,8 @@ impl<T> Copy for StaticRef<T> {}
 impl<T> Deref for StaticRef<T> {
     type Target = T;
     fn deref(&self) -> &T {
-        // SAFETY: `ptr` is aligned and dereferencable for the program
-        // duration as promised by the caller of `StaticRef::new`.
+        // SAFETY: `ptr` is aligned and dereferencable for the program duration
+        // as promised by the caller of `StaticRef::new`.
         unsafe { self.ptr.as_ref() }
     }
 }
diff --git a/kernel/src/utilities/storage_volume.rs b/kernel/src/utilities/storage_volume.rs
index 9954101afc..1be5764e34 100644
--- a/kernel/src/utilities/storage_volume.rs
+++ b/kernel/src/utilities/storage_volume.rs
@@ -7,20 +7,21 @@
 /// Allocates space in the kernel image for on-chip non-volatile storage.
 ///
 /// Storage volumes are placed after the kernel code and before relocated
-/// variables (those copied into RAM on boot). They are placed in
-/// a section called `.storage`.
+/// variables (those copied into RAM on boot). They are placed in a section
+/// called `.storage`.
 ///
-/// Non-volatile storage abstractions can then refer to the block of
-/// allocate flash in terms of the name of the volume. For example,
+/// Non-volatile storage abstractions can then refer to the block of allocate
+/// flash in terms of the name of the volume. For example,
 ///
-/// `storage_volume!(LOG, 32);`
+/// ```ignore
+/// storage_volume!(LOG, 32);
+/// ```
 ///
-/// will allocate 32kB of space in the flash and define a symbol LOG
-/// at the start address of that flash region. The intention is that
-/// storage abstractions can then be passed this address and size to
-/// initialize their state. The linker script kernel_layout.ld makes
-/// sure that the .storage section is aligned on a 512-byte boundary
-/// and the next section is aligned as well.
+/// will allocate 32kB of space in the flash and define a symbol `LOG` at the
+/// start address of that flash region. The intention is that storage
+/// abstractions can then be passed this address and size to initialize their
+/// state. The default Tock linker script makes sure that the `.storage` section
+/// is aligned on a 512-byte boundary and the next section is aligned as well.
 #[macro_export]
 macro_rules! storage_volume {
     ($N:ident, $kB:expr $(,)?) => {

From 4d4371e2808a1f6386b434eb95eb3fb301980898 Mon Sep 17 00:00:00 2001
From: Brad Campbell <bradjc5@gmail.com>
Date: Thu, 10 Oct 2024 12:32:26 -0400
Subject: [PATCH 3/4] kernel: debug: add missing documentation

Also reformat comments.
---
 kernel/src/debug.rs | 104 +++++++++++++++++++++++++++-----------------
 1 file changed, 63 insertions(+), 41 deletions(-)

diff --git a/kernel/src/debug.rs b/kernel/src/debug.rs
index 5dd6169958..8d559e16e8 100644
--- a/kernel/src/debug.rs
+++ b/kernel/src/debug.rs
@@ -8,7 +8,8 @@
 //! If you are writing and the buffer fills up, you can make the size of
 //! `output_buffer` larger.
 //!
-//! Before debug interfaces can be used, the board file must assign them hardware:
+//! Before debug interfaces can be used, the board file must assign them
+//! hardware:
 //!
 //! ```ignore
 //! kernel::debug::assign_gpios(
@@ -17,11 +18,12 @@
 //!     None,
 //! );
 //!
-//! components::debug_writer::DebugWriterComponent::new(uart_mux).finalize(components::debug_writer_component_static!());
+//! components::debug_writer::DebugWriterComponent::new(uart_mux)
+//!     .finalize(components::debug_writer_component_static!());
 //! ```
 //!
-//! The debug queue is optional, if not set in the board it is just ignored.
-//! You can add one in the board file as follows:
+//! The debug queue is optional, if not set in the board it is just ignored. You
+//! can add one in the board file as follows:
 //!
 //! ```ignore
 //! components::debug_queue::DebugQueueComponent::new()
@@ -70,14 +72,18 @@ use crate::utilities::cells::NumericCellExt;
 use crate::utilities::cells::{MapCell, TakeCell};
 use crate::ErrorCode;
 
-/// This trait is similar to std::io::Write in that it takes bytes instead of a string (contrary to
-/// core::fmt::Write), but io::Write isn't available in no_std (due to std::io::Error not being
-/// available).
+/// Implementation of `std::io::Write` for `no_std`.
 ///
-/// Also, in our use cases, writes are infaillible, so the write function cannot return an Error,
-/// however it might not be able to write everything, so it returns  the number of bytes written.
+/// This takes bytes instead of a string (contrary to [`core::fmt::Write`]), but
+/// we cannot use `std::io::Write' as it isn't available in `no_std` (due to
+/// `std::io::Error` not being available).
 ///
-/// See also the tracking issue: <https://github.com/rust-lang/rfcs/issues/2262>
+/// Also, in our use cases, writes are infallible, so the write function cannot
+/// return an `Err`, however it might not be able to write everything, so it
+/// returns the number of bytes written.
+///
+/// See also the tracking issue:
+/// <https://github.com/rust-lang/rfcs/issues/2262>.
 pub trait IoWrite {
     fn write(&mut self, buf: &[u8]) -> usize;
 
@@ -99,13 +105,13 @@ pub trait IoWrite {
 
 /// Tock panic routine, without the infinite LED-blinking loop.
 ///
-/// This is useful for boards which do not feature LEDs to blink or
-/// want to implement their own behaviour. This method returns after
-/// performing the panic dump.
+/// This is useful for boards which do not feature LEDs to blink or want to
+/// implement their own behavior. This method returns after performing the panic
+/// dump.
 ///
-/// After this method returns, the system is no longer in a
-/// well-defined state. Care must be taken on how one interacts with
-/// the system once this function returns.
+/// After this method returns, the system is no longer in a well-defined state.
+/// Care must be taken on how one interacts with the system once this function
+/// returns.
 ///
 /// **NOTE:** The supplied `writer` must be synchronous.
 pub unsafe fn panic_print<W: Write + IoWrite, C: Chip, PP: ProcessPrinter>(
@@ -122,11 +128,10 @@ pub unsafe fn panic_print<W: Write + IoWrite, C: Chip, PP: ProcessPrinter>(
     panic_banner(writer, panic_info);
     panic_cpu_state(chip, writer);
 
-    // Some systems may enforce memory protection regions for the
-    // kernel, making application memory inaccessible. However,
-    // printing process information will attempt to access memory. If
-    // we are provided a chip reference, attempt to disable userspace
-    // memory protection first:
+    // Some systems may enforce memory protection regions for the kernel, making
+    // application memory inaccessible. However, printing process information
+    // will attempt to access memory. If we are provided a chip reference,
+    // attempt to disable userspace memory protection first:
     chip.map(|c| {
         use crate::platform::mpu::MPU;
         c.mpu().disable_app_mpu()
@@ -149,8 +154,8 @@ pub unsafe fn panic<L: hil::led::Led, W: Write + IoWrite, C: Chip, PP: ProcessPr
     chip: &'static Option<&'static C>,
     process_printer: &'static Option<&'static PP>,
 ) -> ! {
-    // Call `panic_print` first which will print out the panic
-    // information and return
+    // Call `panic_print` first which will print out the panic information and
+    // return
     panic_print(writer, panic_info, nop, processes, chip, process_printer);
 
     // The system is no longer in a well-defined state, we cannot
@@ -260,12 +265,14 @@ pub fn panic_blink_forever<L: hil::led::Led>(leds: &mut [&L]) -> ! {
 ///////////////////////////////////////////////////////////////////
 // debug_gpio! support
 
+/// Object to hold the assigned debugging GPIOs.
 pub static mut DEBUG_GPIOS: (
     Option<&'static dyn hil::gpio::Pin>,
     Option<&'static dyn hil::gpio::Pin>,
     Option<&'static dyn hil::gpio::Pin>,
 ) = (None, None, None);
 
+/// Map up to three GPIO pins to use for debugging.
 pub unsafe fn assign_gpios(
     gpio0: Option<&'static dyn hil::gpio::Pin>,
     gpio1: Option<&'static dyn hil::gpio::Pin>,
@@ -276,7 +283,7 @@ pub unsafe fn assign_gpios(
     DEBUG_GPIOS.2 = gpio2;
 }
 
-/// In-kernel gpio debugging, accepts any GPIO HIL method
+/// In-kernel gpio debugging that accepts any GPIO HIL method.
 #[macro_export]
 macro_rules! debug_gpio {
     ($i:tt, $method:ident $(,)?) => {{
@@ -290,8 +297,8 @@ macro_rules! debug_gpio {
 ///////////////////////////////////////////////////////////////////
 // debug_enqueue! support
 
-/// Wrapper type that we need a mutable reference to for the core::fmt::Write
-/// interface.
+/// Wrapper type that we need a mutable reference to for the
+/// [`core::fmt::Write`] interface.
 pub struct DebugQueueWrapper {
     dw: MapCell<&'static DebugQueue>,
 }
@@ -304,6 +311,7 @@ impl DebugQueueWrapper {
     }
 }
 
+/// Queue to hold debug strings.
 pub struct DebugQueue {
     ring_buffer: TakeCell<'static, RingBuffer<'static, u8>>,
 }
@@ -316,6 +324,7 @@ impl DebugQueue {
     }
 }
 
+/// Global reference used by debug macros.
 static mut DEBUG_QUEUE: Option<&'static mut DebugQueueWrapper> = None;
 
 /// Function used by board main.rs to set a reference to the debug queue.
@@ -338,6 +347,7 @@ impl Write for DebugQueueWrapper {
     }
 }
 
+/// Add a format string to the debug queue.
 pub fn debug_enqueue_fmt(args: Arguments) {
     unsafe { DEBUG_QUEUE.as_deref_mut() }.map(|buffer| {
         let _ = write(buffer, args);
@@ -345,6 +355,7 @@ pub fn debug_enqueue_fmt(args: Arguments) {
     });
 }
 
+/// Flush the debug queue by writing to the underlying writer implementation.
 pub fn debug_flush_queue_() {
     let writer = unsafe { get_debug_writer() };
 
@@ -358,8 +369,11 @@ pub fn debug_flush_queue_() {
     }
 }
 
-/// This macro prints a new line to an internal ring buffer, the contents of
-/// which are only flushed with `debug_flush_queue!` and in the panic handler.
+/// Add a new line to an internal ring buffer.
+///
+/// The internal queue is only flushed with
+/// [`debug_flush_queue!()`](crate::debug_flush_queue) or within the panic
+/// handler.
 #[macro_export]
 macro_rules! debug_enqueue {
     () => ({
@@ -373,8 +387,7 @@ macro_rules! debug_enqueue {
     });
 }
 
-/// This macro flushes the contents of the debug queue into the regular
-/// debug output.
+/// Flushes the contents of the debug queue into the regular debug output.
 #[macro_export]
 macro_rules! debug_flush_queue {
     () => {{
@@ -385,14 +398,13 @@ macro_rules! debug_flush_queue {
 ///////////////////////////////////////////////////////////////////
 // debug! and debug_verbose! support
 
-/// Wrapper type that we need a mutable reference to for the core::fmt::Write
-/// interface.
+/// Wrapper type that we need a mutable reference to for the
+/// [`core::fmt::Write`] interface.
 pub struct DebugWriterWrapper {
     dw: MapCell<&'static DebugWriter>,
 }
 
-/// Main type that we need an immutable reference to so we can share it with
-/// the UART provider and this debug module.
+/// Main type that we share with the UART provider and this debug module.
 pub struct DebugWriter {
     // What provides the actual writing mechanism.
     uart: &'static dyn hil::uart::Transmit<'static>,
@@ -404,8 +416,9 @@ pub struct DebugWriter {
     count: Cell<usize>,
 }
 
-/// Static variable that holds the kernel's reference to the debug tool. This is
-/// needed so the debug!() macros have a reference to the object to use.
+/// Static variable that holds the kernel's reference to the debug tool.
+///
+/// This is needed so the debug!() macros have a reference to the object to use.
 static mut DEBUG_WRITER: Option<&'static mut DebugWriterWrapper> = None;
 
 unsafe fn try_get_debug_writer() -> Option<&'static mut DebugWriterWrapper> {
@@ -577,6 +590,7 @@ impl Write for DebugWriterWrapper {
     }
 }
 
+/// Write a debug message without a trailing newline.
 pub fn debug_print(args: Arguments) {
     let writer = unsafe { get_debug_writer() };
 
@@ -584,6 +598,7 @@ pub fn debug_print(args: Arguments) {
     writer.publish_bytes();
 }
 
+/// Write a debug message with a trailing newline.
 pub fn debug_println(args: Arguments) {
     let writer = unsafe { get_debug_writer() };
 
@@ -592,6 +607,7 @@ pub fn debug_println(args: Arguments) {
     writer.publish_bytes();
 }
 
+/// Write a [`ReadableProcessSlice`] to the debug output.
 pub fn debug_slice(slice: &ReadableProcessSlice) -> usize {
     let writer = unsafe { get_debug_writer() };
     let mut total = 0;
@@ -608,6 +624,7 @@ pub fn debug_slice(slice: &ReadableProcessSlice) -> usize {
     total
 }
 
+/// Return how many bytes are remaining in the internal debug buffer.
 pub fn debug_available_len() -> usize {
     let writer = unsafe { get_debug_writer() };
     writer.available_len()
@@ -619,6 +636,8 @@ fn write_header(writer: &mut DebugWriterWrapper, (file, line): &(&'static str, u
     writer.write_fmt(format_args!("TOCK_DEBUG({}): {}:{}: ", count, file, line))
 }
 
+/// Write a debug message with file and line information without a trailing
+/// newline.
 pub fn debug_verbose_print(args: Arguments, file_line: &(&'static str, u32)) {
     let writer = unsafe { get_debug_writer() };
 
@@ -627,6 +646,8 @@ pub fn debug_verbose_print(args: Arguments, file_line: &(&'static str, u32)) {
     writer.publish_bytes();
 }
 
+/// Write a debug message with file and line information with a trailing
+/// newline.
 pub fn debug_verbose_println(args: Arguments, file_line: &(&'static str, u32)) {
     let writer = unsafe { get_debug_writer() };
 
@@ -682,18 +703,18 @@ macro_rules! debug_verbose {
     });
 }
 
-#[macro_export]
 /// Prints out the expression and its location, then returns it.
 ///
 /// ```rust,ignore
 /// let foo: u8 = debug_expr!(0xff);
 /// // Prints [main.rs:2] 0xff = 255
 /// ```
-/// Taken straight from Rust std::dbg.
+/// Taken straight from Rust `std::dbg`.
+#[macro_export]
 macro_rules! debug_expr {
-    // NOTE: We cannot use `concat!` to make a static string as a format argument
-    // of `eprintln!` because `file!` could contain a `{` or
-    // `$val` expression could be a block (`{ .. }`), in which case the `eprintln!`
+    // NOTE: We cannot use `concat!` to make a static string as a format
+    // argument of `eprintln!` because `file!` could contain a `{` or `$val`
+    // expression could be a block (`{ .. }`), in which case the `eprintln!`
     // will be malformed.
     () => {
         $crate::debug!("[{}:{}]", file!(), line!())
@@ -714,6 +735,7 @@ macro_rules! debug_expr {
     };
 }
 
+/// Flush any stored messages to the output writer.
 pub unsafe fn flush<W: Write + IoWrite>(writer: &mut W) {
     if let Some(debug_writer) = try_get_debug_writer() {
         if let Some(ring_buffer) = debug_writer.extract() {

From 826248493b3ed2b7bf2f9e3f0595e2d3e04caaff Mon Sep 17 00:00:00 2001
From: Brad Campbell <bradjc5@gmail.com>
Date: Fri, 18 Oct 2024 14:38:24 -0400
Subject: [PATCH 4/4] kernel: doc: fixes and improvements

---
 kernel/src/debug.rs                | 3 ++-
 kernel/src/utilities/copy_slice.rs | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/src/debug.rs b/kernel/src/debug.rs
index 8d559e16e8..6af34a9dd4 100644
--- a/kernel/src/debug.rs
+++ b/kernel/src/debug.rs
@@ -418,7 +418,8 @@ pub struct DebugWriter {
 
 /// Static variable that holds the kernel's reference to the debug tool.
 ///
-/// This is needed so the debug!() macros have a reference to the object to use.
+/// This is needed so the `debug!()` macros have a reference to the object to
+/// use.
 static mut DEBUG_WRITER: Option<&'static mut DebugWriterWrapper> = None;
 
 unsafe fn try_get_debug_writer() -> Option<&'static mut DebugWriterWrapper> {
diff --git a/kernel/src/utilities/copy_slice.rs b/kernel/src/utilities/copy_slice.rs
index ff982e7081..1bac48d9f1 100644
--- a/kernel/src/utilities/copy_slice.rs
+++ b/kernel/src/utilities/copy_slice.rs
@@ -22,7 +22,7 @@ use core::ptr;
 pub trait CopyOrErr {
     /// Copy a non-overlapping slice from `src` to `self`.
     ///
-    /// This is a non-panicing version of [`slice::copy_from_slice`].
+    /// This is a non-panicking version of [`slice::copy_from_slice`].
     ///
     /// Returns `Err(ErrorCode)` if `src` and `self` are not the same length.
     fn copy_from_slice_or_err(&mut self, src: &Self) -> Result<(), ErrorCode>;