Merge pull request #116 from Neotron-Compute/fix-rust-183

Removed all the static mut.

Author: thejpster

Cargo.lock

@@ -8,56 +8,31 @@ name = "neotron-pico-bios"
 version = "0.7.0"
 
 [dependencies]
-# Useful Cortex-M specific functions (e.g. SysTick)
-cortex-m = {version = "0.7", features = ["inline-asm"]}
-# The Raspberry Pi RP2040 HAL (so we can turn defmt on)
-rp2040-hal = { version = "0.10", features = [ "defmt", "rt", "critical-section-impl", "rom-func-cache" ] }
-# Cortex-M run-time (or start-up) code
-cortex-m-rt = "0.7"
-# The BIOS to OS API
-neotron-common-bios = "0.12.0"
-# For the RP2040 bootloader
-rp2040-boot2 = "0.3.0"
-# For hardware abstraction traits
-embedded-hal = "0.2"
-# Gives us formatted PC-side logging
-defmt = "=0.3.2"
-# Sends defmt logs to the SWD debugger
-defmt-rtt = "0.4"
-# Send panics to the debugger
-panic-probe = "0.2"
-# RP2040 PIO assembler
-pio = "0.2.1"
-# Macros for RP2040 PIO assembler
-pio-proc = "0.2"
-# Hardware locks for sharing data with interrupts
-critical-section = "1.0"
-# Commands for talking to a Neotron BMC. The tag is for the repo as a whole, of which the commands crate is a small part.
-neotron-bmc-commands = { version = "0.2.0" }
-# Protocol for talking to a Neotron BMC. The tag is for the repo as a whole, of which the protocol crate is a small part.
-neotron-bmc-protocol = { version = "0.1.0", features = ["defmt"] }
-# Time and frequency related functions
-fugit = "0.3"
-# PS/2 scancode decoding
-pc-keyboard = "0.7.0"
-# Useful queues and other structures
-heapless = "0.7"
-# The Dallas DS1307 RTC driver
-ds1307 = "0.5.0"
-# The Microchip MCP7940x RTC driver
-mcp794xx = "0.3.0"
-# I2C Bus Sharing
-shared-bus = "0.3"
-# Gets us compare-swap atomic operations
-atomic-polyfill = "1.0.2"
-# SD Card driver
-embedded-sdmmc = { version = "0.6", default-features = false, features = [
-    "defmt-log"
-] }
-# CODEC register control
-tlv320aic23 = "0.1.0"
-# Calendar/Date/Time functions and types 
-chrono = { version = "0.4.38", default-features = false }
+chrono = { version = "0.4.38", default-features = false } # Calendar/Date/Time functions and types 
+cortex-m = {version = "0.7", features = ["inline-asm"]} # Useful Cortex-M specific functions (e.g. SysTick)
+cortex-m-rt = "0.7" # Cortex-M run-time (or start-up) code
+critical-section = "1.0" # Hardware locks for sharing data with interrupts
+defmt = "=0.3.2" # Gives us formatted PC-side logging
+defmt-rtt = "0.4" # Sends defmt logs to the SWD debugger
+ds1307 = "0.5.0" # The Dallas DS1307 RTC driver
+embedded-hal = "0.2" # For hardware abstraction traits
+embedded-sdmmc = { version = "0.6", default-features = false, features = [ "defmt-log" ] } # SD Card driver
+fugit = "0.3" # Time and frequency related functions
+grounded = { version = "0.2.0", features = ["critical-section"] }
+heapless = "0.7" # Useful queues and other structures
+mcp794xx = "0.3.0" # The Microchip MCP7940x RTC driver
+neotron-bmc-commands = { version = "0.2.0" } # Commands for talking to a Neotron BMC. The tag is for the repo as a whole, of which the commands crate is a small part.
+neotron-bmc-protocol = { version = "0.1.0", features = ["defmt"] } # Protocol for talking to a Neotron BMC. The tag is for the repo as a whole, of which the protocol crate is a small part.
+neotron-common-bios = "0.12.0" # The BIOS to OS API
+panic-probe = "0.2" # Send panics to the debugger
+pc-keyboard = "0.7.0" # PS/2 scancode decoding
+pio = "0.2.1" # RP2040 PIO assembler
+pio-proc = "0.2" # Macros for RP2040 PIO assembler
+portable-atomic = { version = "1.10.0", features = ["critical-section"] } # Atomic CAS for non-CAS CPUs
+rp2040-boot2 = "0.3.0" # For the RP2040 bootloader
+rp2040-hal = { version = "0.10", features = [ "defmt", "rt", "critical-section-impl", "rom-func-cache" ] } # The Raspberry Pi RP2040 HAL (so we can turn defmt on)
+shared-bus = "0.3" # I2C Bus Sharing
+tlv320aic23 = "0.1.0" # CODEC register control
 
 [build-dependencies]
 neotron-common-bios = "0.12.0"

Cargo.toml

@@ -25,18 +25,18 @@ MEMORY {
     /*
      * This is the bottom of the four striped banks of SRAM in the RP2040.
      */
-    RAM_OS : ORIGIN = 0x20000000, LENGTH = 0x42000 - 0x9680
+    RAM_OS : ORIGIN = 0x20000000, LENGTH = 0x42000 - 0x9630
     /*
      * This is the top of the four striped banks of SRAM in the RP2040, plus
      * SRAM_BANK4 and SRAM_BANK5.
      *
      * This is carefully calculated to give us 8 KiB of stack space and ensure
      * the defmt buffer doesn't span across SRAM_BANK3 and SRAM_BANK4.
      *
-     * 0x9680 should be the (size of .data + size of .bss + size of .uninit +
+     * 0x9630 should be the (size of .data + size of .bss + size of .uninit +
      * 0x2000 for the stack).
      */
-    RAM : ORIGIN = 0x20042000 - 0x9680, LENGTH = 0x9680
+    RAM : ORIGIN = 0x20042000 - 0x9630, LENGTH = 0x9630
 }
 
 /*

memory.x

@@ -1,16 +1,22 @@
 //! Code to set up a PIO to generate I2S Audio
 
+use core::cell::UnsafeCell;
+
+use grounded::uninit::GroundedCell;
+
 use crate::hal::{pac::interrupt, prelude::*};
 
 /// Holds the objects we need to read from the RAM fifo and write to the PIO hardware FIFO.
-static mut PLAYBACK_TO_PIO: Option<PlaybackToPio> = None;
+static PLAYBACK_TO_PIO: GroundedCell<PlaybackToPio> = GroundedCell::uninit();
 
 /// The reader end of the RAM FIFO and the writer end of the PIO hardware FIFO.
 struct PlaybackToPio {
 	pio_fifo: crate::hal::pio::Tx<(crate::pac::PIO1, crate::hal::pio::SM0)>,
 	ram_fifo: heapless::spsc::Consumer<'static, u32, 1024>,
 }
 
+unsafe impl Sync for PlaybackToPio {}
+
 /// Used to 'play' samples by writing them to the RAM FIFO.
 ///
 /// Holds the writer end of the RAM FIFO.
@@ -181,17 +187,21 @@ pub fn init(pio: super::pac::PIO1, resets: &mut super::pac::RESETS) -> Player {
 
 	let _running_sam = samples_sm.start();
 
-	static mut SAMPLE_QUEUE: heapless::spsc::Queue<u32, 1024> = heapless::spsc::Queue::new();
+	static SAMPLE_QUEUE: QueueWrapper = QueueWrapper::new();
+	// # Safety
+	// Interrupts are disabled at this point, so we can split the queue.
 	let (q_producer, q_consumer) = unsafe { SAMPLE_QUEUE.split() };
 
 	pio_tx_fifo.enable_tx_not_full_interrupt(crate::hal::pio::PioIRQ::Irq0);
 
-	critical_section::with(|_| unsafe {
-		PLAYBACK_TO_PIO.replace(PlaybackToPio {
+	// # Safety
+	// the PIO interrupt is currently disabled, so this is OK
+	unsafe {
+		PLAYBACK_TO_PIO.get().write(PlaybackToPio {
 			pio_fifo: pio_tx_fifo,
 			ram_fifo: q_consumer,
 		});
-	});
+	}
 
 	unsafe {
 		cortex_m::peripheral::NVIC::unmask(crate::pac::Interrupt::PIO1_IRQ_0);
@@ -200,6 +210,28 @@ pub fn init(pio: super::pac::PIO1, resets: &mut super::pac::RESETS) -> Player {
 	Player { fifo: q_producer }
 }
 
+struct QueueWrapper(UnsafeCell<heapless::spsc::Queue<u32, 1024>>);
+
+impl QueueWrapper {
+	const fn new() -> QueueWrapper {
+		QueueWrapper(UnsafeCell::new(heapless::spsc::Queue::new()))
+	}
+
+	/// Only call this when interrupts are off and the queue isn't being used
+	unsafe fn split(
+		&self,
+	) -> (
+		heapless::spsc::Producer<'_, u32, 1024>,
+		heapless::spsc::Consumer<'_, u32, 1024>,
+	) {
+		let ptr = self.0.get();
+		let queue_ref = unsafe { &mut *ptr };
+		queue_ref.split()
+	}
+}
+
+unsafe impl Sync for QueueWrapper {}
+
 /// Called when the PIO1 IRQ fires.
 ///
 /// We mapped this to "TX FIFO is not empty", so this interrupt will be
@@ -209,9 +241,7 @@ fn PIO1_IRQ_0() {
 	// This is the only function (apart from `init`) which accesses this
 	// variable, and `init()` is sure to disable interrupts until the global is
 	// set up, so this is safe.
-	let Some(fifo) = (unsafe { PLAYBACK_TO_PIO.as_mut() }) else {
-		return;
-	};
+	let fifo = unsafe { &mut *PLAYBACK_TO_PIO.get() };
 	// Read from fifo.ram_fifo
 	if let Some(sample) = fifo.ram_fifo.dequeue() {
 		// .. and write to fifo.pio_fifo

src/i2s.rs

@@ -742,9 +742,13 @@ fn paint_stacks() {
 			*b = CORE0_STACK_PAINT_WORD;
 		}
 
-		info!("Painting Core 1 stack: {:?}", CORE1_STACK.as_ptr_range());
-		for b in CORE1_STACK.iter_mut() {
-			*b = CORE1_STACK_PAINT_WORD;
+		let stack_start = addr_of_mut!(CORE1_STACK) as *mut usize;
+		let stack_end = addr_of_mut!(CORE1_STACK).add(1) as *mut usize;
+		info!("Painting Core 1 stack @ {:?}", stack_start..stack_end);
+		let mut p = stack_start;
+		while p != stack_end {
+			p.write_volatile(CORE1_STACK_PAINT_WORD);
+			p = p.add(1);
 		}
 	}
 }
@@ -764,17 +768,12 @@ fn check_stacks() {
 		static mut __sheap: usize;
 		static mut _stack_start: usize;
 	}
-	let stack_len = unsafe { (addr_of!(_stack_start) as usize) - (addr_of!(__sheap) as usize) };
-	check_stack(
-		unsafe { addr_of!(__sheap) },
-		stack_len,
-		CORE0_STACK_PAINT_WORD,
-	);
-	check_stack(
-		unsafe { CORE1_STACK.as_ptr() },
-		unsafe { CORE1_STACK.len() * core::mem::size_of::<usize>() },
-		CORE1_STACK_PAINT_WORD,
-	);
+	let stack_len = (addr_of!(_stack_start) as usize) - (addr_of!(__sheap) as usize);
+	check_stack(addr_of!(__sheap), stack_len, CORE0_STACK_PAINT_WORD);
+	let stack_start = addr_of!(CORE1_STACK) as *const usize;
+	let stack_end = unsafe { addr_of!(CORE1_STACK).add(1) } as *const usize;
+	let stack_len = unsafe { stack_end.offset_from(stack_start) } as usize;
+	check_stack(stack_start, stack_len, CORE1_STACK_PAINT_WORD);
 }
 
 /// Dummy stack checker that does nothing
@@ -1915,8 +1914,8 @@ pub extern "C" fn memory_get_region(region: u8) -> FfiOption<common::MemoryRegio
 		0 => {
 			// Application Region
 			FfiOption::Some(MemoryRegion {
-				start: unsafe { addr_of!(_ram_os_start) } as *mut u8,
-				length: unsafe { addr_of!(_ram_os_len) } as usize,
+				start: addr_of!(_ram_os_start) as *mut u8,
+				length: addr_of!(_ram_os_len) as usize,
 				kind: common::MemoryKind::Ram.into(),
 			})
 		}
@@ -2663,17 +2662,7 @@ extern "C" fn compare_and_swap_bool(value: &AtomicBool, old_value: bool, new_val
 /// This should only be when the MCP23S17 has driven our IRQ pin low.
 #[interrupt]
 fn IO_IRQ_BANK0() {
-	// The `#[interrupt]` attribute covertly converts this to `&'static mut
-	// Option<IrqPin>`
-	static mut LOCAL_IRQ_PIN: Option<IrqPin> = None;
-
-	// This is one-time lazy initialisation. We steal the variables given to us
-	// via `IRQ_PIN`.
-	if LOCAL_IRQ_PIN.is_none() {
-		let mut lock = IRQ_PIN.lock();
-		*LOCAL_IRQ_PIN = lock.take();
-	}
-	if let Some(pin) = LOCAL_IRQ_PIN {
+	if let Some(pin) = IRQ_PIN.lock().as_mut() {
 		let is_low = pin.is_low().unwrap();
 		INTERRUPT_PENDING.store(is_low, Ordering::Relaxed);
 		pin.clear_interrupt(hal::gpio::Interrupt::EdgeLow);

src/main.rs

@@ -3,14 +3,14 @@
 //! Unlock the cortex-m mutex, it panics on collision, rather than disabling
 //! interrupts while the lock is held.
 
-use atomic_polyfill::{AtomicBool, Ordering};
+use portable_atomic::{AtomicBool, Ordering};
 
 /// A simple no-std mutex.
 ///
 /// Uses critical-section to hold an atomic bool, for when you don't have
 /// atomic-compare-swap.
 pub struct NeoMutex<T> {
-	locked: atomic_polyfill::AtomicBool,
+	locked: AtomicBool,
 	value: core::cell::UnsafeCell<T>,
 }
 
@@ -48,21 +48,21 @@ pub struct NeoMutexGuard<'a, T> {
 	parent: &'a NeoMutex<T>,
 }
 
-impl<'a, T> Drop for NeoMutexGuard<'a, T> {
+impl<T> Drop for NeoMutexGuard<'_, T> {
 	fn drop(&mut self) {
 		self.parent.locked.store(false, Ordering::Release);
 	}
 }
 
-impl<'a, T> core::ops::Deref for NeoMutexGuard<'a, T> {
+impl<T> core::ops::Deref for NeoMutexGuard<'_, T> {
 	type Target = T;
 
 	fn deref(&self) -> &Self::Target {
 		unsafe { &*self.parent.value.get() }
 	}
 }
 
-impl<'a, T> core::ops::DerefMut for NeoMutexGuard<'a, T> {
+impl<T> core::ops::DerefMut for NeoMutexGuard<'_, T> {
 	fn deref_mut(&mut self) -> &mut Self::Target {
 		unsafe { &mut *self.parent.value.get() }
 	}

src/mutex/mod.rs

@@ -27,7 +27,7 @@
 // Imports
 // -----------------------------------------------------------------------------
 
-use atomic_polyfill::{AtomicBool, Ordering};
+use core::sync::atomic::{AtomicBool, Ordering};
 
 use super::Hardware;
 
@@ -38,7 +38,7 @@ use super::Hardware;
 /// A type that `embedded-sdmmc` can use to talk over our SPI bus.
 pub(crate) struct FakeSpi<'a>(pub(crate) &'a mut Hardware, pub bool);
 
-impl<'a> embedded_hal::blocking::spi::Transfer<u8> for FakeSpi<'a> {
+impl embedded_hal::blocking::spi::Transfer<u8> for FakeSpi<'_> {
 	type Error = core::convert::Infallible;
 
 	fn transfer<'w>(&mut self, words: &'w mut [u8]) -> Result<&'w [u8], Self::Error> {
@@ -68,7 +68,7 @@ impl<'a> embedded_hal::blocking::spi::Transfer<u8> for FakeSpi<'a> {
 	}
 }
 
-impl<'a> embedded_hal::blocking::spi::Write<u8> for FakeSpi<'a> {
+impl embedded_hal::blocking::spi::Write<u8> for FakeSpi<'_> {
 	type Error = core::convert::Infallible;
 
 	fn write(&mut self, words: &[u8]) -> Result<(), Self::Error> {