Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Interrupts on RISC-V #847

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Interrupts on RISC-V #847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3241169
Adds a driver for the Xh3irq interrupt controller
thejpster Sep 1, 2024
3dfa983
Add a MachineTimer driver.
thejpster Sep 1, 2024
03e56ec
Fix powman_test so it runs on Arm and RISC-V.
thejpster Sep 1, 2024
9e18c29
Fix two more examples
thejpster Sep 1, 2024
8041088
Fix two more.
thejpster Sep 1, 2024
bc8aff8
Don't mention NVIC in the examples.
thejpster Sep 8, 2024
4fd6e7d
Clarify Xh3irq testing
thejpster Sep 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion rp235x-hal-examples/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,15 @@ futures = {version = "0.3.30", default-features = false, features = ["async-awai
hd44780-driver = "0.4.0"
heapless = "0.8.0"
nb = "1.0"
nostd_async = {version = "0.6.1", features = ["cortex_m"]}
panic-halt = "0.2.0"
pio = "0.2.0"
pio-proc = "0.2.0"
rp235x-hal = {path = "../rp235x-hal", version = "0.2.0", features = ["binary-info", "critical-section-impl", "rt", "defmt"]}
usb-device = "0.3.2"
usbd-serial = "0.2.2"

[target.'thumbv8m.main-none-eabihf'.dependencies]
nostd_async = {version = "0.6.1", features = ["cortex_m"]}

[target.riscv32imac-unknown-none-elf.dependencies]
nostd_async = {version = "0.6.1"}
jannic marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 5 additions & 0 deletions rp235x-hal-examples/riscv_examples.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,21 @@ block_loop
dht11
gpio_dyn_pin_array
gpio_in_out
gpio_irq_example
i2c
i2c_async
i2c_async_cancelled
lcd_display
mem_to_mem_dma
pio_blink
pio_dma
pio_proc_blink
pio_side_set
pio_synchronized
powman_test
pwm_blink
pwm_blink_embedded_hal_1
pwm_irq_input
rom_funcs
rosc_as_system_clock
spi
Expand Down
45 changes: 45 additions & 0 deletions rp235x-hal-examples/rp235x_riscv.x
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,51 @@ PROVIDE(__pre_init = default_pre_init);
/* A PAC/HAL defined routine that should initialize custom interrupt controller if needed. */
PROVIDE(_setup_interrupts = default_setup_interrupts);

PROVIDE(TIMER0_IRQ_0 = DefaultIrqHandler);
PROVIDE(TIMER0_IRQ_1 = DefaultIrqHandler);
PROVIDE(TIMER0_IRQ_2 = DefaultIrqHandler);
PROVIDE(TIMER0_IRQ_3 = DefaultIrqHandler);
PROVIDE(TIMER1_IRQ_0 = DefaultIrqHandler);
PROVIDE(TIMER1_IRQ_1 = DefaultIrqHandler);
PROVIDE(TIMER1_IRQ_2 = DefaultIrqHandler);
PROVIDE(TIMER1_IRQ_3 = DefaultIrqHandler);
PROVIDE(PWM_IRQ_WRAP_0 = DefaultIrqHandler);
PROVIDE(PWM_IRQ_WRAP_1 = DefaultIrqHandler);
PROVIDE(DMA_IRQ_0 = DefaultIrqHandler);
PROVIDE(DMA_IRQ_1 = DefaultIrqHandler);
PROVIDE(DMA_IRQ_2 = DefaultIrqHandler);
PROVIDE(DMA_IRQ_3 = DefaultIrqHandler);
PROVIDE(USBCTRL_IRQ = DefaultIrqHandler);
PROVIDE(PIO0_IRQ_0 = DefaultIrqHandler);
PROVIDE(PIO0_IRQ_1 = DefaultIrqHandler);
PROVIDE(PIO1_IRQ_0 = DefaultIrqHandler);
PROVIDE(PIO1_IRQ_1 = DefaultIrqHandler);
PROVIDE(PIO2_IRQ_0 = DefaultIrqHandler);
PROVIDE(PIO2_IRQ_1 = DefaultIrqHandler);
PROVIDE(IO_IRQ_BANK0 = DefaultIrqHandler);
PROVIDE(IO_IRQ_BANK0_NS = DefaultIrqHandler);
PROVIDE(IO_IRQ_QSPI = DefaultIrqHandler);
PROVIDE(IO_IRQ_QSPI_NS = DefaultIrqHandler);
PROVIDE(SIO_IRQ_FIFO = DefaultIrqHandler);
PROVIDE(SIO_IRQ_BELL = DefaultIrqHandler);
PROVIDE(SIO_IRQ_FIFO_NS = DefaultIrqHandler);
PROVIDE(SIO_IRQ_BELL_NS = DefaultIrqHandler);
PROVIDE(SIO_IRQ_MTIMECMP = DefaultIrqHandler);
PROVIDE(CLOCKS_IRQ = DefaultIrqHandler);
PROVIDE(SPI0_IRQ = DefaultIrqHandler);
PROVIDE(SPI1_IRQ = DefaultIrqHandler);
PROVIDE(UART0_IRQ = DefaultIrqHandler);
PROVIDE(UART1_IRQ = DefaultIrqHandler);
PROVIDE(ADC_IRQ_FIFO = DefaultIrqHandler);
PROVIDE(I2C0_IRQ = DefaultIrqHandler);
PROVIDE(I2C1_IRQ = DefaultIrqHandler);
PROVIDE(OTP_IRQ = DefaultIrqHandler);
PROVIDE(TRNG_IRQ = DefaultIrqHandler);
PROVIDE(PLL_SYS_IRQ = DefaultIrqHandler);
PROVIDE(PLL_USB_IRQ = DefaultIrqHandler);
PROVIDE(POWMAN_IRQ_POW = DefaultIrqHandler);
PROVIDE(POWMAN_IRQ_TIMER = DefaultIrqHandler);

/* # Multi-processing hook function
fn _mp_hook() -> bool;

Expand Down
28 changes: 16 additions & 12 deletions rp235x-hal-examples/src/bin/gpio_irq_example.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ use rp235x_hal as hal;
// Some things we need
use embedded_hal::digital::StatefulOutputPin;

// Our interrupt macro
use hal::pac::interrupt;

// Some short-cuts to useful types
use core::cell::RefCell;
use critical_section::Mutex;
Expand Down Expand Up @@ -128,12 +125,17 @@ fn main() -> ! {
GLOBAL_PINS.borrow(cs).replace(Some((led, in_pin)));
});

// Unmask the IO_BANK0 IRQ so that the NVIC interrupt controller
// will jump to the interrupt function when the interrupt occurs.
// We do this last so that the interrupt can't go off while
// it is in the middle of being configured
// Unmask the IRQ for I/O Bank 0 so that the RP2350's interrupt controller
// (NVIC in Arm mode, or Xh3irq in RISC-V mode) will jump to the interrupt
// function when the interrupt occurs. We do this last so that the interrupt
// can't go off while it is in the middle of being configured
unsafe {
hal::arch::interrupt_unmask(hal::pac::Interrupt::IO_IRQ_BANK0);
}

// Enable interrupts on this core
unsafe {
cortex_m::peripheral::NVIC::unmask(hal::pac::Interrupt::IO_IRQ_BANK0);
hal::arch::interrupt_enable();
}

loop {
Expand All @@ -142,21 +144,23 @@ fn main() -> ! {
}
}

#[interrupt]
#[no_mangle]
#[allow(non_snake_case)]
fn IO_IRQ_BANK0() {
// The `#[interrupt]` attribute covertly converts this to `&'static mut Option<LedAndButton>`
static mut LED_AND_BUTTON: Option<LedAndButton> = None;
let led_and_button = unsafe { &mut *core::ptr::addr_of_mut!(LED_AND_BUTTON) };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This &mut *core::ptr::addr_of_mut!( ... ) IMHO doesn't make sense.

The whole raison d'être of addr_of_mut! is avoiding the &mut, because having a &mut (even for a short time, and not derefencing it) can potentially cause UB if you are not careful.

But here, you convert it to a &mut anyway. So I don't see any advantage of this difficult to read incantation, you can as well just do unsafe { &mut LED_AND_BUTTON }; instead.

Now, that causes a warning. And this warning looks like it led you to writing the code above:

warning: creating a mutable reference to mutable static is discouraged
   --> src/bin/gpio_irq_example.rs:152:35
    |
152 |     let led_and_button = unsafe { &mut LED_AND_BUTTON };
    |                                   ^^^^^^^^^^^^^^^^^^^ mutable reference to mutable static
    |
    = note: for more information, see issue #114447 <https://github.com/rust-lang/rust/issues/114447>
    = note: this will be a hard error in the 2024 edition
    = note: this mutable reference has lifetime `'static`, but if the static gets accessed (read or written) by any other means, or any other reference is created, then any further use of this mutable reference is Undefined Behavior
    = note: `#[warn(static_mut_refs)]` on by default
help: use `addr_of_mut!` instead to create a raw pointer
    |
152 |     let led_and_button = unsafe { addr_of_mut!(LED_AND_BUTTON) };
    |                                   ~~~~~~~~~~~~~              +

I think the warning is valid, but the help is misleading. It even says "to create a raw pointer", and as written it would return a pointer, not a &mut, so the resulting code wouldn't build.

TL;DR:

Suggested change
let led_and_button = unsafe { &mut *core::ptr::addr_of_mut!(LED_AND_BUTTON) };
#[allow(static_mut_refs)]
// Safety: As long as this interrupt is only enabled on a single core,
// and the interrupt isn't somehow called reentrantly, there will not
// be a second reference to LED_AND_BUTTON at the same time.
let led_and_button = unsafe { &mut LED_AND_BUTTON };


// This is one-time lazy initialisation. We steal the variables given to us
// via `GLOBAL_PINS`.
if LED_AND_BUTTON.is_none() {
if led_and_button.is_none() {
critical_section::with(|cs| {
*LED_AND_BUTTON = GLOBAL_PINS.borrow(cs).take();
*led_and_button = GLOBAL_PINS.borrow(cs).take();
});
}

// Need to check if our Option<LedAndButtonPins> contains our pins
if let Some(gpios) = LED_AND_BUTTON {
if let Some(gpios) = led_and_button {
// borrow led and button by *destructuring* the tuple
// these will be of type `&mut LedPin` and `&mut ButtonPin`, so we don't have
// to move them back into the static after we use them
Expand Down
16 changes: 9 additions & 7 deletions rp235x-hal-examples/src/bin/i2c_async.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use hal::{
gpio::bank0::{Gpio20, Gpio21},
gpio::{FunctionI2C, Pin, PullUp},
i2c::Controller,
pac::interrupt,
Clock, I2C,
};

Expand All @@ -40,7 +39,8 @@ pub static IMAGE_DEF: hal::block::ImageDef = hal::block::ImageDef::secure_exe();
const XTAL_FREQ_HZ: u32 = 12_000_000u32;

/// Bind the interrupt handler with the peripheral
#[interrupt]
#[no_mangle]
#[allow(non_snake_case)]
unsafe fn I2C0_IRQ() {
use hal::async_utils::AsyncPeripheral;
I2C::<hal::pac::I2C0, (Gpio20, Gpio21), Controller>::on_interrupt();
Expand Down Expand Up @@ -94,14 +94,16 @@ async fn demo() {
clocks.system_clock.freq(),
);

// Unmask the interrupt in the NVIC to let the core wake up & enter the interrupt handler.
// Each core has its own NVIC so these needs to executed from the core where the IRQ are
// expected.
// Unmask the IRQ for I2C0. We do this after the driver init so that the
// interrupt can't go off while it is in the middle of being configured
unsafe {
cortex_m::peripheral::NVIC::unpend(hal::pac::Interrupt::I2C0_IRQ);
cortex_m::peripheral::NVIC::unmask(hal::pac::Interrupt::I2C0_IRQ);
hal::arch::interrupt_unmask(hal::pac::Interrupt::I2C0_IRQ);
}

// Enable interrupts on this core
unsafe {
hal::arch::interrupt_enable();
}
// Asynchronously write three bytes to the I²C device with 7-bit address 0x2C
i2c.write(0x76u8, &[1, 2, 3]).await.unwrap();

Expand Down
15 changes: 10 additions & 5 deletions rp235x-hal-examples/src/bin/i2c_async_cancelled.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ use hal::{
gpio::bank0::{Gpio20, Gpio21},
gpio::{FunctionI2C, Pin, PullUp},
i2c::Controller,
pac::interrupt,
Clock, I2C,
};

Expand All @@ -42,7 +41,8 @@ pub static IMAGE_DEF: hal::block::ImageDef = hal::block::ImageDef::secure_exe();
/// Adjust if your board has a different frequency
const XTAL_FREQ_HZ: u32 = 12_000_000u32;

#[interrupt]
#[no_mangle]
#[allow(non_snake_case)]
unsafe fn I2C0_IRQ() {
use hal::async_utils::AsyncPeripheral;
I2C::<hal::pac::I2C0, (Gpio20, Gpio21), Controller>::on_interrupt();
Expand Down Expand Up @@ -96,10 +96,15 @@ async fn demo() {
clocks.system_clock.freq(),
);

// Unmask the interrupt in the NVIC to let the core wake up & enter the interrupt handler.
// Unmask the IRQ for I2C0. We do this after the driver init so that the
// interrupt can't go off while it is in the middle of being configured
unsafe {
cortex_m::peripheral::NVIC::unpend(hal::pac::Interrupt::I2C0_IRQ);
cortex_m::peripheral::NVIC::unmask(hal::pac::Interrupt::I2C0_IRQ);
hal::arch::interrupt_unmask(hal::pac::Interrupt::I2C0_IRQ);
}

// Enable interrupts on this core
unsafe {
hal::arch::interrupt_enable();
}

let mut cnt = 0;
Expand Down
66 changes: 28 additions & 38 deletions rp235x-hal-examples/src/bin/powman_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ use rp235x_hal::{
uart::{DataBits, StopBits, UartConfig, UartPeripheral},
};

use cortex_m_rt::exception;
use pac::interrupt;

// Some traits we need
use core::fmt::Write;
use hal::fugit::RateExtU32;
Expand Down Expand Up @@ -67,7 +64,7 @@ impl GlobalUart {

/// Entry point to our bare-metal application.
///
/// The `#[hal::entry]` macro ensures the Cortex-M start-up code calls this function
/// The `#[hal::entry]` macro ensures the start-up code calls this function
/// as soon as all global variables and the spinlock are initialised.
///
/// The function configures the rp235x peripherals, then writes to the UART in
Expand All @@ -76,11 +73,6 @@ impl GlobalUart {
fn main() -> ! {
// Grab our singleton objects
let mut pac = pac::Peripherals::take().unwrap();
let mut cp = cortex_m::Peripherals::take().unwrap();

// Enable the cycle counter
cp.DCB.enable_trace();
cp.DWT.enable_cycle_counter();

// Set up the watchdog driver - needed by the clock setup code
let mut watchdog = hal::Watchdog::new(pac.WATCHDOG);
Expand All @@ -99,6 +91,8 @@ fn main() -> ! {

// The single-cycle I/O block controls our GPIO pins
let sio = hal::Sio::new(pac.SIO);
let mut mtimer = sio.machine_timer;
mtimer.set_enabled(true);

// Set the pins to their default state
let pins = gpio::Pins::new(
Expand Down Expand Up @@ -130,30 +124,38 @@ fn main() -> ! {
print_aot_status(&mut powman);
_ = writeln!(&GLOBAL_UART, "AOT time: 0x{:016x}", powman.aot_get_time());

// Unmask the IRQ for POWMAN's Timer. We do this after the driver init so
// that the interrupt can't go off while it is in the middle of being
// configured
unsafe {
hal::arch::interrupt_unmask(hal::pac::Interrupt::POWMAN_IRQ_TIMER);
}

// Enable interrupts on this core
unsafe {
cortex_m::peripheral::NVIC::unmask(pac::Interrupt::POWMAN_IRQ_TIMER);
hal::arch::interrupt_enable();
}

_ = writeln!(&GLOBAL_UART, "Starting AOT...");
powman.aot_start();
// we don't know what oscillator we're on, so give it time to start whatever it is
cortex_m::asm::delay(150_000);
hal::arch::delay(150_000);
print_aot_status(&mut powman);
rollover_test(&mut powman);
loop_test(&mut powman);
loop_test(&mut powman, &mtimer);
alarm_test(&mut powman);

let source = AotClockSource::Xosc(FractionalFrequency::from_hz(12_000_000));
_ = writeln!(&GLOBAL_UART, "Switching AOT to {}", source);
powman.aot_set_clock(source).expect("selecting XOSC");
print_aot_status(&mut powman);
loop_test(&mut powman);
loop_test(&mut powman, &mtimer);

let source = AotClockSource::Lposc(FractionalFrequency::from_hz(32768));
_ = writeln!(&GLOBAL_UART, "Switching AOT to {}", source);
powman.aot_set_clock(source).expect("selecting LPOSC");
print_aot_status(&mut powman);
loop_test(&mut powman);
loop_test(&mut powman, &mtimer);

_ = writeln!(&GLOBAL_UART, "Rebooting now");

Expand Down Expand Up @@ -202,7 +204,7 @@ fn rollover_test(powman: &mut Powman) {
}

/// In this function, we see how long it takes to pass a certain number of ticks.
fn loop_test(powman: &mut Powman) {
fn loop_test(powman: &mut Powman, mtimer: &hal::sio::MachineTimer) {
let start_loop = 0;
let end_loop = 2_000; // 2 seconds
_ = writeln!(
Expand All @@ -214,24 +216,24 @@ fn loop_test(powman: &mut Powman) {
powman.aot_set_time(start_loop);
powman.aot_start();

let start_clocks = cortex_m::peripheral::DWT::cycle_count();
let start_clocks = mtimer.read();
loop {
let now = powman.aot_get_time();
if now == end_loop {
break;
}
}
let end_clocks = cortex_m::peripheral::DWT::cycle_count();
// Compare our AOT against our CPU clock speed
let delta_clocks = end_clocks.wrapping_sub(start_clocks) as u64;
let end_clocks = mtimer.read();
// Compare our AOT against our Machine Timer
let delta_clocks = end_clocks.wrapping_sub(start_clocks);
let delta_ticks = end_loop - start_loop;
let cycles_per_tick = delta_clocks / delta_ticks;
// Assume we're running at 150 MHz
let ms_per_tick = (cycles_per_tick as f32 * 1000.0) / 150_000_000.0;
// Assume we're running at 1 MHz MTimer
let ms_per_tick = (cycles_per_tick as f32 * 1000.0) / 1_000_000.0;
let percent = ((ms_per_tick - 1.0) / 1.0) * 100.0;
_ = writeln!(
&GLOBAL_UART,
"Loop complete ... {delta_ticks} ticks in {delta_clocks} CPU clock cycles = {cycles_per_tick} cycles/tick ~= {ms_per_tick} ms/tick ({percent:.3}%)",
"Loop complete ... {delta_ticks} ticks in {delta_clocks} MTimer cycles = {cycles_per_tick} cycles/tick ~= {ms_per_tick} ms/tick ({percent:.3}%)",
)
;
}
Expand All @@ -258,8 +260,9 @@ fn alarm_test(powman: &mut Powman) {
&GLOBAL_UART,
"Sleeping until alarm (* = wakeup, ! = POWMAN interrupt)...",
);

while !powman.aot_alarm_ringing() {
cortex_m::asm::wfe();
hal::arch::wfe();
_ = write!(&GLOBAL_UART, "*",);
}

Expand All @@ -278,25 +281,12 @@ fn alarm_test(powman: &mut Powman) {
_ = writeln!(&GLOBAL_UART, "Alarm cleared OK");
}

#[interrupt]
#[no_mangle]
#[allow(non_snake_case)]
fn POWMAN_IRQ_TIMER() {
Powman::static_aot_alarm_interrupt_disable();
_ = write!(&GLOBAL_UART, "!");
cortex_m::asm::sev();
}

#[exception]
unsafe fn HardFault(ef: &cortex_m_rt::ExceptionFrame) -> ! {
let _ = writeln!(&GLOBAL_UART, "HARD FAULT:\n{:#?}", ef);

hal::reboot::reboot(
hal::reboot::RebootKind::BootSel {
msd_disabled: false,
picoboot_disabled: false,
},
hal::reboot::RebootArch::Normal,
);
hal::arch::sev();
}

#[panic_handler]
Expand Down
Loading