Merge #2189

2189: Board based instantiation of chip drivers and interrupt mappings: Msp432 r=hudson-ayers a=hudson-ayers

### Pull Request Overview

This pull request ports the MSP432 chip/board to the new peripheral instantiation approach that does not rely on global variables (first proposed in #2069).

This chip was relatively easy to change compared to some of the others, but @lebakassemmerl I would still appreciate a quick look over the changes or a test of a couple apps to ensure I haven't done anything stupid.

This is the last remaining chip before all upstream chips/boards have been ported!


### Testing Strategy

This pull request was tested by compiling, hardware testing would be nice.


### TODO or Help Wanted

N/A

### Documentation Updated

- [x] Updated the relevant files in `/docs`, or no updates are required.

### Formatting

- [x] Ran `make prepush`.


Co-authored-by: Hudson Ayers <hayers@stanford.edu>

boards/msp_exp432p401r/src/io.rs

@@ -33,9 +33,8 @@ impl Write for Uart {
 
 impl IoWrite for Uart {
     fn write(&mut self, buf: &[u8]) {
-        unsafe {
-            msp432::uart::UART0.transmit_sync(buf);
-        }
+        let uart0 = msp432::uart::Uart::new(msp432::usci::USCI_A0_BASE, 0, 1, 1, 1);
+        uart0.transmit_sync(buf);
     }
 }
 
@@ -44,7 +43,8 @@ impl IoWrite for Uart {
 #[panic_handler]
 pub unsafe extern "C" fn panic_fmt(info: &PanicInfo) -> ! {
     const LED1_PIN: IntPinNr = IntPinNr::P01_0;
-    let led = &mut led::LedHigh::new(&msp432::gpio::INT_PINS[LED1_PIN as usize]);
+    let gpio_pin = msp432::gpio::IntPin::new(LED1_PIN);
+    let led = &mut led::LedHigh::new(&gpio_pin);
     let writer = &mut UART;
     let wdt = Wdt::new();
 

chips/msp432/src/adc.rs

@@ -11,21 +11,6 @@ use kernel::common::StaticRef;
 use kernel::hil;
 use kernel::ReturnCode;
 
-pub static mut ADC: Adc = Adc {
-    registers: ADC_BASE,
-    resolution: DEFAULT_ADC_RESOLUTION,
-    mode: Cell::new(AdcMode::Disabled),
-    active_channel: Cell::new(Channel::Channel0),
-    ref_module: OptionalCell::empty(),
-    timer: OptionalCell::empty(), // must be TIMER_A3!
-    dma: OptionalCell::empty(),
-    dma_chan: 7,
-    dma_src: 7,
-    buffer1: TakeCell::empty(),
-    buffer2: TakeCell::empty(),
-    client: OptionalCell::empty(),
-};
-
 const ADC_BASE: StaticRef<AdcRegisters> =
     unsafe { StaticRef::new(0x4001_2000 as *const AdcRegisters) };
 
@@ -505,21 +490,40 @@ register_bitfields![u32,
 pub trait EverythingClient: hil::adc::Client + hil::adc::HighSpeedClient {}
 impl<C: hil::adc::Client + hil::adc::HighSpeedClient> EverythingClient for C {}
 
-pub struct Adc {
+pub struct Adc<'a> {
     registers: StaticRef<AdcRegisters>,
     resolution: AdcResolution,
     mode: Cell<AdcMode>,
     active_channel: Cell<Channel>,
-    ref_module: OptionalCell<&'static dyn ref_module::AnalogReference>,
-    timer: OptionalCell<&'static dyn timer::InternalTimer>,
-    dma: OptionalCell<&'static dma::DmaChannel<'static>>,
+    ref_module: OptionalCell<&'a dyn ref_module::AnalogReference>,
+    timer: OptionalCell<&'a dyn timer::InternalTimer>,
+    dma: OptionalCell<&'a dma::DmaChannel<'a>>,
     pub(crate) dma_chan: usize,
     dma_src: u8,
     buffer1: TakeCell<'static, [u16]>,
     buffer2: TakeCell<'static, [u16]>,
     client: OptionalCell<&'static dyn EverythingClient>,
 }
 
+impl Adc<'_> {
+    pub const fn new() -> Self {
+        Self {
+            registers: ADC_BASE,
+            resolution: DEFAULT_ADC_RESOLUTION,
+            mode: Cell::new(AdcMode::Disabled),
+            active_channel: Cell::new(Channel::Channel0),
+            ref_module: OptionalCell::empty(),
+            timer: OptionalCell::empty(), // must be TIMER_A3!
+            dma: OptionalCell::empty(),
+            dma_chan: 7,
+            dma_src: 7,
+            buffer1: TakeCell::empty(),
+            buffer2: TakeCell::empty(),
+            client: OptionalCell::empty(),
+        }
+    }
+}
+
 #[repr(u32)]
 #[derive(Copy, Clone, PartialEq)]
 pub enum Channel {
@@ -591,7 +595,7 @@ unsafe fn buf_u16_to_buf_u8(buf: &'static mut [u16]) -> &'static mut [u8] {
     slice::from_raw_parts_mut(buf_ptr, buf.len() * 2)
 }
 
-impl Adc {
+impl<'a> Adc<'a> {
     fn is_enabled(&self) -> bool {
         self.registers.ctl0.is_set(CTL0::ON)
     }
@@ -686,9 +690,9 @@ impl Adc {
 
     pub fn set_modules(
         &self,
-        ref_module: &'static dyn ref_module::AnalogReference,
-        timer: &'static dyn timer::InternalTimer,
-        dma: &'static dma::DmaChannel,
+        ref_module: &'a dyn ref_module::AnalogReference,
+        timer: &'a dyn timer::InternalTimer,
+        dma: &'a dma::DmaChannel<'a>,
     ) {
         self.ref_module.set(ref_module);
         self.timer.set(timer);
@@ -731,7 +735,7 @@ impl Adc {
     }
 }
 
-impl dma::DmaClient for Adc {
+impl dma::DmaClient for Adc<'_> {
     fn transfer_done(
         &self,
         _tx_buf: Option<&'static mut [u8]>,
@@ -754,7 +758,7 @@ impl dma::DmaClient for Adc {
     }
 }
 
-impl hil::adc::Adc for Adc {
+impl hil::adc::Adc for Adc<'_> {
     type Channel = Channel;
 
     fn sample(&self, channel: &Self::Channel) -> ReturnCode {
@@ -884,7 +888,7 @@ impl hil::adc::Adc for Adc {
     }
 }
 
-impl hil::adc::AdcHighSpeed for Adc {
+impl hil::adc::AdcHighSpeed for Adc<'_> {
     fn sample_highspeed(
         &self,
         channel: &Self::Channel,

chips/msp432/src/chip.rs

@@ -2,50 +2,108 @@ use core::fmt::Write;
 use cortexm4;
 use kernel::Chip;
 
-use crate::adc;
-use crate::dma;
-use crate::gpio;
 use crate::nvic;
-use crate::ref_module;
-use crate::timer;
-use crate::uart;
 use crate::wdt;
+use kernel::InterruptService;
 
-pub struct Msp432 {
+pub struct Msp432<'a, I: InterruptService<()> + 'a> {
     mpu: cortexm4::mpu::MPU,
     userspace_kernel_boundary: cortexm4::syscall::SysCall,
     scheduler_timer: cortexm4::systick::SysTick,
     watchdog: wdt::Wdt,
+    interrupt_service: &'a I,
 }
 
-impl Msp432 {
-    pub unsafe fn new() -> Msp432 {
+pub struct Msp432DefaultPeripherals<'a> {
+    pub adc: crate::adc::Adc<'a>,
+    pub uart0: crate::uart::Uart<'a>,
+    pub cs: crate::cs::ClockSystem,
+    pub dma_channels: crate::dma::DmaChannels<'a>,
+    pub adc_ref: crate::ref_module::Ref,
+    pub timer_a0: crate::timer::TimerA<'a>,
+    pub timer_a1: crate::timer::TimerA<'a>,
+    pub timer_a2: crate::timer::TimerA<'a>,
+    pub timer_a3: crate::timer::TimerA<'a>,
+    pub gpio: crate::gpio::GpioManager<'a>,
+}
+
+impl<'a> Msp432DefaultPeripherals<'a> {
+    pub fn new() -> Self {
+        Self {
+            adc: crate::adc::Adc::new(),
+            uart0: crate::uart::Uart::new(crate::usci::USCI_A0_BASE, 0, 1, 1, 1),
+            cs: crate::cs::ClockSystem::new(),
+            dma_channels: crate::dma::DmaChannels::new(),
+            adc_ref: crate::ref_module::Ref::new(),
+            timer_a0: crate::timer::TimerA::new(crate::timer::TIMER_A0_BASE),
+            timer_a1: crate::timer::TimerA::new(crate::timer::TIMER_A1_BASE),
+            timer_a2: crate::timer::TimerA::new(crate::timer::TIMER_A2_BASE),
+            timer_a3: crate::timer::TimerA::new(crate::timer::TIMER_A3_BASE),
+            gpio: crate::gpio::GpioManager::new(),
+        }
+    }
+
+    pub unsafe fn init(&'a self) {
         // Setup DMA channels
-        uart::UART0.set_dma(
-            &dma::DMA_CHANNELS[uart::UART0.tx_dma_chan],
-            &dma::DMA_CHANNELS[uart::UART0.rx_dma_chan],
+        self.uart0.set_dma(
+            &self.dma_channels[self.uart0.tx_dma_chan],
+            &self.dma_channels[self.uart0.rx_dma_chan],
         );
-        dma::DMA_CHANNELS[uart::UART0.tx_dma_chan].set_client(&uart::UART0);
-        dma::DMA_CHANNELS[uart::UART0.rx_dma_chan].set_client(&uart::UART0);
+        self.dma_channels[self.uart0.tx_dma_chan].set_client(&self.uart0);
+        self.dma_channels[self.uart0.rx_dma_chan].set_client(&self.uart0);
 
         // Setup Reference Module, Timer and DMA for ADC
-        adc::ADC.set_modules(
-            &ref_module::REF,
-            &timer::TIMER_A3,
-            &dma::DMA_CHANNELS[adc::ADC.dma_chan],
+        self.adc.set_modules(
+            &self.adc_ref,
+            &self.timer_a3,
+            &self.dma_channels[self.adc.dma_chan],
         );
-        dma::DMA_CHANNELS[adc::ADC.dma_chan].set_client(&adc::ADC);
+        self.dma_channels[self.adc.dma_chan].set_client(&self.adc);
+    }
+}
+
+impl<'a> kernel::InterruptService<()> for Msp432DefaultPeripherals<'a> {
+    unsafe fn service_interrupt(&self, interrupt: u32) -> bool {
+        match interrupt {
+            nvic::ADC => self.adc.handle_interrupt(),
+            nvic::DMA_INT0 => self.dma_channels.handle_interrupt(0),
+            nvic::DMA_INT1 => self.dma_channels.handle_interrupt(1),
+            nvic::DMA_INT2 => self.dma_channels.handle_interrupt(2),
+            nvic::DMA_INT3 => self.dma_channels.handle_interrupt(3),
+            nvic::DMA_ERR => self.dma_channels.handle_interrupt(-1),
+            nvic::IO_PORT1 => self.gpio.handle_interrupt(0),
+            nvic::IO_PORT2 => self.gpio.handle_interrupt(1),
+            nvic::IO_PORT3 => self.gpio.handle_interrupt(2),
+            nvic::IO_PORT4 => self.gpio.handle_interrupt(3),
+            nvic::IO_PORT5 => self.gpio.handle_interrupt(4),
+            nvic::IO_PORT6 => self.gpio.handle_interrupt(5),
+            nvic::TIMER_A0_0 | nvic::TIMER_A0_1 => self.timer_a0.handle_interrupt(),
+            nvic::TIMER_A1_0 | nvic::TIMER_A1_1 => self.timer_a1.handle_interrupt(),
+            nvic::TIMER_A2_0 | nvic::TIMER_A2_1 => self.timer_a2.handle_interrupt(),
+            nvic::TIMER_A3_0 | nvic::TIMER_A3_1 => self.timer_a3.handle_interrupt(),
+
+            _ => return false,
+        }
+        true
+    }
+    unsafe fn service_deferred_call(&self, _: ()) -> bool {
+        false
+    }
+}
 
-        Msp432 {
+impl<'a, I: InterruptService<()> + 'a> Msp432<'a, I> {
+    pub unsafe fn new(interrupt_service: &'a I) -> Self {
+        Self {
             mpu: cortexm4::mpu::MPU::new(),
             userspace_kernel_boundary: cortexm4::syscall::SysCall::new(),
             scheduler_timer: cortexm4::systick::SysTick::new_with_calibration(48_000_000),
             watchdog: wdt::Wdt::new(),
+            interrupt_service,
         }
     }
 }
 
-impl Chip for Msp432 {
+impl<'a, I: InterruptService<()> + 'a> Chip for Msp432<'a, I> {
     type MPU = cortexm4::mpu::MPU;
     type UserspaceKernelBoundary = cortexm4::syscall::SysCall;
     type SchedulerTimer = cortexm4::systick::SysTick;
@@ -55,26 +113,8 @@ impl Chip for Msp432 {
         unsafe {
             loop {
                 if let Some(interrupt) = cortexm4::nvic::next_pending() {
-                    match interrupt {
-                        nvic::ADC => adc::ADC.handle_interrupt(),
-                        nvic::DMA_INT0 => dma::handle_interrupt(0),
-                        nvic::DMA_INT1 => dma::handle_interrupt(1),
-                        nvic::DMA_INT2 => dma::handle_interrupt(2),
-                        nvic::DMA_INT3 => dma::handle_interrupt(3),
-                        nvic::DMA_ERR => dma::handle_interrupt(-1),
-                        nvic::IO_PORT1 => gpio::handle_interrupt(0),
-                        nvic::IO_PORT2 => gpio::handle_interrupt(1),
-                        nvic::IO_PORT3 => gpio::handle_interrupt(2),
-                        nvic::IO_PORT4 => gpio::handle_interrupt(3),
-                        nvic::IO_PORT5 => gpio::handle_interrupt(4),
-                        nvic::IO_PORT6 => gpio::handle_interrupt(5),
-                        nvic::TIMER_A0_0 | nvic::TIMER_A0_1 => timer::TIMER_A0.handle_interrupt(),
-                        nvic::TIMER_A1_0 | nvic::TIMER_A1_1 => timer::TIMER_A1.handle_interrupt(),
-                        nvic::TIMER_A2_0 | nvic::TIMER_A2_1 => timer::TIMER_A2.handle_interrupt(),
-                        nvic::TIMER_A3_0 | nvic::TIMER_A3_1 => timer::TIMER_A3.handle_interrupt(),
-                        _ => {
-                            panic!("unhandled interrupt {}", interrupt);
-                        }
+                    if !self.interrupt_service.service_interrupt(interrupt) {
+                        panic!("unhandled interrupt {}", interrupt);
                     }
 
                     let n = cortexm4::nvic::Nvic::new(interrupt);

chips/msp432/src/cs.rs

@@ -7,8 +7,6 @@ use kernel::common::registers::{
 use kernel::common::StaticRef;
 use kernel::NoClockControl;
 
-pub static mut CS: ClockSystem = ClockSystem::new();
-
 pub const MCLK_HZ: u32 = 48_000_000;
 pub const HSMCLK_HZ: u32 = 12_000_000;
 pub const SMCLK_HZ: u32 = 1_500_000;
@@ -250,11 +248,13 @@ register_bitfields! [u32,
 
 type CsRegisterManager<'a> = PeripheralManager<'a, ClockSystem, NoClockControl>;
 
-pub struct ClockSystem {}
+pub struct ClockSystem {
+    registers: StaticRef<CsRegisters>,
+}
 
 impl ClockSystem {
-    const fn new() -> ClockSystem {
-        ClockSystem {}
+    pub const fn new() -> ClockSystem {
+        ClockSystem { registers: CS_BASE }
     }
 
     fn set_mclk_48mhz(&self) {
@@ -320,7 +320,7 @@ impl<'a> PeripheralManagement<NoClockControl> for ClockSystem {
     type RegisterType = CsRegisters;
 
     fn get_registers(&self) -> &CsRegisters {
-        &*CS_BASE
+        &self.registers
     }
 
     fn get_clock(&self) -> &NoClockControl {