From e6d404327928756f2975542122a782343f46e75e Mon Sep 17 00:00:00 2001 From: pennae Date: Mon, 31 Jul 2023 18:28:31 +0200 Subject: [PATCH 1/4] rp: rename gpio::Pin::io to gpio::Pin::gpio we'll need access to the pin io bank registers for an upcoming fix, and having both `io` and `io_bank` or similar can get confusing quickly. rename `io` to `gpio` to avoid this, and also match the type while there. --- embassy-rp/src/clocks.rs | 8 ++++---- embassy-rp/src/gpio.rs | 6 +++--- embassy-rp/src/i2c.rs | 4 ++-- embassy-rp/src/pio.rs | 2 +- embassy-rp/src/pwm.rs | 8 ++++---- embassy-rp/src/spi.rs | 8 ++++---- embassy-rp/src/uart/mod.rs | 8 ++++---- 7 files changed, 22 insertions(+), 22 deletions(-) diff --git a/embassy-rp/src/clocks.rs b/embassy-rp/src/clocks.rs index 976d06de..a3398023 100644 --- a/embassy-rp/src/clocks.rs +++ b/embassy-rp/src/clocks.rs @@ -702,7 +702,7 @@ impl<'d, T: Pin> Gpin<'d, T> { pub fn new(gpin: impl Peripheral

+ 'd) -> Gpin<'d, P> { into_ref!(gpin); - gpin.io().ctrl().write(|w| w.set_funcsel(0x08)); + gpin.gpio().ctrl().write(|w| w.set_funcsel(0x08)); Gpin { gpin: gpin.map_into(), @@ -718,7 +718,7 @@ impl<'d, T: Pin> Gpin<'d, T> { impl<'d, T: Pin> Drop for Gpin<'d, T> { fn drop(&mut self) { self.gpin - .io() + .gpio() .ctrl() .write(|w| w.set_funcsel(pac::io::vals::Gpio0ctrlFuncsel::NULL as _)); } @@ -766,7 +766,7 @@ impl<'d, T: GpoutPin> Gpout<'d, T> { pub fn new(gpout: impl Peripheral

+ 'd) -> Self { into_ref!(gpout); - gpout.io().ctrl().write(|w| w.set_funcsel(0x08)); + gpout.gpio().ctrl().write(|w| w.set_funcsel(0x08)); Self { gpout } } @@ -831,7 +831,7 @@ impl<'d, T: GpoutPin> Drop for Gpout<'d, T> { fn drop(&mut self) { self.disable(); self.gpout - .io() + .gpio() .ctrl() .write(|w| w.set_funcsel(pac::io::vals::Gpio0ctrlFuncsel::NULL as _)); } diff --git a/embassy-rp/src/gpio.rs b/embassy-rp/src/gpio.rs index 2807eb67..7153325d 100644 --- a/embassy-rp/src/gpio.rs +++ b/embassy-rp/src/gpio.rs @@ -451,7 +451,7 @@ impl<'d, T: Pin> Flex<'d, T> { w.set_ie(true); }); - pin.io().ctrl().write(|w| { + pin.gpio().ctrl().write(|w| { w.set_funcsel(pac::io::vals::Gpio0ctrlFuncsel::SIO_0 as _); }); @@ -617,7 +617,7 @@ impl<'d, T: Pin> Drop for Flex<'d, T> { #[inline] fn drop(&mut self) { self.pin.pad_ctrl().write(|_| {}); - self.pin.io().ctrl().write(|w| { + self.pin.gpio().ctrl().write(|w| { w.set_funcsel(pac::io::vals::Gpio0ctrlFuncsel::NULL as _); }); } @@ -643,7 +643,7 @@ pub(crate) mod sealed { } } - fn io(&self) -> pac::io::Gpio { + fn gpio(&self) -> pac::io::Gpio { let block = match self._bank() { Bank::Bank0 => crate::pac::IO_BANK0, Bank::Qspi => crate::pac::IO_QSPI, diff --git a/embassy-rp/src/i2c.rs b/embassy-rp/src/i2c.rs index 536ad747..7a5ddd32 100644 --- a/embassy-rp/src/i2c.rs +++ b/embassy-rp/src/i2c.rs @@ -353,8 +353,8 @@ impl<'d, T: Instance + 'd, M: Mode> I2c<'d, T, M> { p.ic_rx_tl().write(|w| w.set_rx_tl(0)); // Configure SCL & SDA pins - scl.io().ctrl().write(|w| w.set_funcsel(3)); - sda.io().ctrl().write(|w| w.set_funcsel(3)); + scl.gpio().ctrl().write(|w| w.set_funcsel(3)); + sda.gpio().ctrl().write(|w| w.set_funcsel(3)); scl.pad_ctrl().write(|w| { w.set_schmitt(true); diff --git a/embassy-rp/src/pio.rs b/embassy-rp/src/pio.rs index 3de398af..c09d0914 100644 --- a/embassy-rp/src/pio.rs +++ b/embassy-rp/src/pio.rs @@ -852,7 +852,7 @@ impl<'d, PIO: Instance> Common<'d, PIO> { /// of [`Pio`] do not keep pin registrations alive.** pub fn make_pio_pin(&mut self, pin: impl Peripheral

+ 'd) -> Pin<'d, PIO> { into_ref!(pin); - pin.io().ctrl().write(|w| w.set_funcsel(PIO::FUNCSEL as _)); + pin.gpio().ctrl().write(|w| w.set_funcsel(PIO::FUNCSEL as _)); // we can be relaxed about this because we're &mut here and nothing is cached PIO::state().used_pins.fetch_or(1 << pin.pin_bank(), Ordering::Relaxed); Pin { diff --git a/embassy-rp/src/pwm.rs b/embassy-rp/src/pwm.rs index c0ddb2a9..c297d69a 100644 --- a/embassy-rp/src/pwm.rs +++ b/embassy-rp/src/pwm.rs @@ -79,10 +79,10 @@ impl<'d, T: Channel> Pwm<'d, T> { Self::configure(p, &config); if let Some(pin) = &a { - pin.io().ctrl().write(|w| w.set_funcsel(4)); + pin.gpio().ctrl().write(|w| w.set_funcsel(4)); } if let Some(pin) = &b { - pin.io().ctrl().write(|w| w.set_funcsel(4)); + pin.gpio().ctrl().write(|w| w.set_funcsel(4)); } Self { inner, @@ -243,10 +243,10 @@ impl<'d, T: Channel> Drop for Pwm<'d, T> { fn drop(&mut self) { self.inner.regs().csr().write_clear(|w| w.set_en(false)); if let Some(pin) = &self.pin_a { - pin.io().ctrl().write(|w| w.set_funcsel(31)); + pin.gpio().ctrl().write(|w| w.set_funcsel(31)); } if let Some(pin) = &self.pin_b { - pin.io().ctrl().write(|w| w.set_funcsel(31)); + pin.gpio().ctrl().write(|w| w.set_funcsel(31)); } } } diff --git a/embassy-rp/src/spi.rs b/embassy-rp/src/spi.rs index 544b542e..46c440b8 100644 --- a/embassy-rp/src/spi.rs +++ b/embassy-rp/src/spi.rs @@ -100,16 +100,16 @@ impl<'d, T: Instance, M: Mode> Spi<'d, T, M> { p.cr1().write(|w| w.set_sse(true)); if let Some(pin) = &clk { - pin.io().ctrl().write(|w| w.set_funcsel(1)); + pin.gpio().ctrl().write(|w| w.set_funcsel(1)); } if let Some(pin) = &mosi { - pin.io().ctrl().write(|w| w.set_funcsel(1)); + pin.gpio().ctrl().write(|w| w.set_funcsel(1)); } if let Some(pin) = &miso { - pin.io().ctrl().write(|w| w.set_funcsel(1)); + pin.gpio().ctrl().write(|w| w.set_funcsel(1)); } if let Some(pin) = &cs { - pin.io().ctrl().write(|w| w.set_funcsel(1)); + pin.gpio().ctrl().write(|w| w.set_funcsel(1)); } Self { inner, diff --git a/embassy-rp/src/uart/mod.rs b/embassy-rp/src/uart/mod.rs index 69c6ac2f..00070b80 100644 --- a/embassy-rp/src/uart/mod.rs +++ b/embassy-rp/src/uart/mod.rs @@ -565,7 +565,7 @@ impl<'d, T: Instance + 'd, M: Mode> Uart<'d, T, M> { ) { let r = T::regs(); if let Some(pin) = &tx { - pin.io().ctrl().write(|w| { + pin.gpio().ctrl().write(|w| { w.set_funcsel(2); w.set_outover(if config.invert_tx { Outover::INVERT @@ -576,7 +576,7 @@ impl<'d, T: Instance + 'd, M: Mode> Uart<'d, T, M> { pin.pad_ctrl().write(|w| w.set_ie(true)); } if let Some(pin) = &rx { - pin.io().ctrl().write(|w| { + pin.gpio().ctrl().write(|w| { w.set_funcsel(2); w.set_inover(if config.invert_rx { Inover::INVERT @@ -587,7 +587,7 @@ impl<'d, T: Instance + 'd, M: Mode> Uart<'d, T, M> { pin.pad_ctrl().write(|w| w.set_ie(true)); } if let Some(pin) = &cts { - pin.io().ctrl().write(|w| { + pin.gpio().ctrl().write(|w| { w.set_funcsel(2); w.set_inover(if config.invert_cts { Inover::INVERT @@ -598,7 +598,7 @@ impl<'d, T: Instance + 'd, M: Mode> Uart<'d, T, M> { pin.pad_ctrl().write(|w| w.set_ie(true)); } if let Some(pin) = &rts { - pin.io().ctrl().write(|w| { + pin.gpio().ctrl().write(|w| { w.set_funcsel(2); w.set_outover(if config.invert_rts { Outover::INVERT From 2c6fcdbd3f6ff40d77e200fdd98b727164379769 Mon Sep 17 00:00:00 2001 From: pennae Date: Mon, 31 Jul 2023 18:36:37 +0200 Subject: [PATCH 2/4] rp: add gpio::Pin::io() for access to io banks --- embassy-rp/src/gpio.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/embassy-rp/src/gpio.rs b/embassy-rp/src/gpio.rs index 7153325d..bac126d4 100644 --- a/embassy-rp/src/gpio.rs +++ b/embassy-rp/src/gpio.rs @@ -643,12 +643,15 @@ pub(crate) mod sealed { } } - fn gpio(&self) -> pac::io::Gpio { - let block = match self._bank() { + fn io(&self) -> pac::io::Io { + match self._bank() { Bank::Bank0 => crate::pac::IO_BANK0, Bank::Qspi => crate::pac::IO_QSPI, - }; - block.gpio(self._pin() as _) + } + } + + fn gpio(&self) -> pac::io::Gpio { + self.io().gpio(self._pin() as _) } fn pad_ctrl(&self) -> Reg { @@ -672,12 +675,8 @@ pub(crate) mod sealed { } fn int_proc(&self) -> pac::io::Int { - let io_block = match self._bank() { - Bank::Bank0 => crate::pac::IO_BANK0, - Bank::Qspi => crate::pac::IO_QSPI, - }; let proc = SIO.cpuid().read(); - io_block.int_proc(proc as _) + self.io().int_proc(proc as _) } } } From dca1777a2f3659b1acaa87aba31caf9afb09eae4 Mon Sep 17 00:00:00 2001 From: pennae Date: Mon, 31 Jul 2023 19:13:10 +0200 Subject: [PATCH 3/4] rp: make QSPI gpio support optional this will be mostly not useful to anyone since flash is attached to qspi, and using flash chips that don't use the *entire* qspi interface will severly slow down the chip. the code overhead is minimal right now, but if we also fix interrupt support on qspi pins this will change (adding more code to potentially hot paths, using more memory for wakers that are never used, and preventing the qspi gpio irq from being used in software interrupts as RTIC applications may want to do). --- ci.sh | 1 + ci_stable.sh | 1 + embassy-rp/Cargo.toml | 4 ++++ embassy-rp/src/gpio.rs | 17 +++++++++++++---- 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/ci.sh b/ci.sh index 2693c1be..954ad3e9 100755 --- a/ci.sh +++ b/ci.sh @@ -58,6 +58,7 @@ cargo batch \ --- build --release --manifest-path embassy-rp/Cargo.toml --target thumbv6m-none-eabi --features nightly,unstable-traits \ --- build --release --manifest-path embassy-rp/Cargo.toml --target thumbv6m-none-eabi --features nightly \ --- build --release --manifest-path embassy-rp/Cargo.toml --target thumbv6m-none-eabi --features nightly,intrinsics \ + --- build --release --manifest-path embassy-rp/Cargo.toml --target thumbv6m-none-eabi --features nightly,qspi-as-gpio \ --- build --release --manifest-path embassy-stm32/Cargo.toml --target thumbv8m.main-none-eabihf --features stm32l552ze,defmt,exti,time-driver-any,unstable-traits \ --- build --release --manifest-path embassy-stm32/Cargo.toml --target thumbv8m.main-none-eabihf --features stm32l552ze,defmt,exti,time-driver-any \ --- build --release --manifest-path embassy-stm32/Cargo.toml --target thumbv8m.main-none-eabihf --features stm32l552ze,defmt,time-driver-any \ diff --git a/ci_stable.sh b/ci_stable.sh index daae9896..56f72131 100755 --- a/ci_stable.sh +++ b/ci_stable.sh @@ -36,6 +36,7 @@ cargo batch \ --- build --release --manifest-path embassy-rp/Cargo.toml --target thumbv6m-none-eabi --features unstable-traits,defmt \ --- build --release --manifest-path embassy-rp/Cargo.toml --target thumbv6m-none-eabi --features unstable-traits,log \ --- build --release --manifest-path embassy-rp/Cargo.toml --target thumbv6m-none-eabi \ + --- build --release --manifest-path embassy-rp/Cargo.toml --target thumbv6m-none-eabi --features qspi-as-gpio \ --- build --release --manifest-path embassy-stm32/Cargo.toml --target thumbv7em-none-eabi --features stm32g473cc,defmt,exti,time-driver-any,unstable-traits \ --- build --release --manifest-path embassy-stm32/Cargo.toml --target thumbv7em-none-eabi --features stm32g491re,defmt,exti,time-driver-any,unstable-traits \ --- build --release --manifest-path embassy-stm32/Cargo.toml --target thumbv7em-none-eabi --features stm32u585zi,defmt,exti,time-driver-any,unstable-traits \ diff --git a/embassy-rp/Cargo.toml b/embassy-rp/Cargo.toml index 6310ffb6..564d44ec 100644 --- a/embassy-rp/Cargo.toml +++ b/embassy-rp/Cargo.toml @@ -42,6 +42,10 @@ boot2-ram-memcpy = [] boot2-w25q080 = [] boot2-w25x10cl = [] +# Allow using QSPI pins as GPIO pins. This is mostly not what you want (because your flash lives there) +# and would add both code and memory overhead when enabled needlessly. +qspi-as-gpio = [] + # Indicate code is running from RAM. # Set this if all code is in RAM, and the cores never access memory-mapped flash memory through XIP. # This allows the flash driver to not force pausing execution on both cores when doing flash operations. diff --git a/embassy-rp/src/gpio.rs b/embassy-rp/src/gpio.rs index bac126d4..9861429f 100644 --- a/embassy-rp/src/gpio.rs +++ b/embassy-rp/src/gpio.rs @@ -67,6 +67,7 @@ pub enum SlewRate { #[derive(Debug, Eq, PartialEq)] pub enum Bank { Bank0 = 0, + #[cfg(feature = "qspi-as-gpio")] Qspi = 1, } @@ -636,16 +637,17 @@ pub(crate) mod sealed { #[inline] fn _bank(&self) -> Bank { - if self.pin_bank() & 0x20 == 0 { - Bank::Bank0 - } else { - Bank::Qspi + match self.pin_bank() & 0x20 { + #[cfg(feature = "qspi-as-gpio")] + 1 => Bank::Qspi, + _ => Bank::Bank0, } } fn io(&self) -> pac::io::Io { match self._bank() { Bank::Bank0 => crate::pac::IO_BANK0, + #[cfg(feature = "qspi-as-gpio")] Bank::Qspi => crate::pac::IO_QSPI, } } @@ -657,6 +659,7 @@ pub(crate) mod sealed { fn pad_ctrl(&self) -> Reg { let block = match self._bank() { Bank::Bank0 => crate::pac::PADS_BANK0, + #[cfg(feature = "qspi-as-gpio")] Bank::Qspi => crate::pac::PADS_QSPI, }; block.gpio(self._pin() as _) @@ -766,11 +769,17 @@ impl_pin!(PIN_27, Bank::Bank0, 27); impl_pin!(PIN_28, Bank::Bank0, 28); impl_pin!(PIN_29, Bank::Bank0, 29); +#[cfg(feature = "qspi-as-gpio")] impl_pin!(PIN_QSPI_SCLK, Bank::Qspi, 0); +#[cfg(feature = "qspi-as-gpio")] impl_pin!(PIN_QSPI_SS, Bank::Qspi, 1); +#[cfg(feature = "qspi-as-gpio")] impl_pin!(PIN_QSPI_SD0, Bank::Qspi, 2); +#[cfg(feature = "qspi-as-gpio")] impl_pin!(PIN_QSPI_SD1, Bank::Qspi, 3); +#[cfg(feature = "qspi-as-gpio")] impl_pin!(PIN_QSPI_SD2, Bank::Qspi, 4); +#[cfg(feature = "qspi-as-gpio")] impl_pin!(PIN_QSPI_SD3, Bank::Qspi, 5); // ==================== From f3ad0c6adeb4212af52f4ff974a488765551e81e Mon Sep 17 00:00:00 2001 From: pennae Date: Mon, 31 Jul 2023 20:02:06 +0200 Subject: [PATCH 4/4] rp: fix qspi gpio interrupts so far only bank0 interrupts were processed and configured, even if a qspi pin was given. this is obviously not the right thing to do, so let's rectify that. the fact that no problems have shown up so far does suggest that most, if not all, applications don't use this functionality at all. --- embassy-rp/src/gpio.rs | 45 +++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/embassy-rp/src/gpio.rs b/embassy-rp/src/gpio.rs index 9861429f..73e89352 100644 --- a/embassy-rp/src/gpio.rs +++ b/embassy-rp/src/gpio.rs @@ -11,9 +11,13 @@ use crate::pac::common::{Reg, RW}; use crate::pac::SIO; use crate::{interrupt, pac, peripherals, Peripheral, RegExt}; -const PIN_COUNT: usize = 30; const NEW_AW: AtomicWaker = AtomicWaker::new(); -static INTERRUPT_WAKERS: [AtomicWaker; PIN_COUNT] = [NEW_AW; PIN_COUNT]; +const BANK0_PIN_COUNT: usize = 30; +static BANK0_WAKERS: [AtomicWaker; BANK0_PIN_COUNT] = [NEW_AW; BANK0_PIN_COUNT]; +#[cfg(feature = "qspi-as-gpio")] +const QSPI_PIN_COUNT: usize = 6; +#[cfg(feature = "qspi-as-gpio")] +static QSPI_WAKERS: [AtomicWaker; QSPI_PIN_COUNT] = [NEW_AW; QSPI_PIN_COUNT]; /// Represents a digital input or output level. #[derive(Debug, Eq, PartialEq, Clone, Copy)] @@ -141,17 +145,23 @@ pub(crate) unsafe fn init() { interrupt::IO_IRQ_BANK0.disable(); interrupt::IO_IRQ_BANK0.set_priority(interrupt::Priority::P3); interrupt::IO_IRQ_BANK0.enable(); + + #[cfg(feature = "qspi-as-gpio")] + { + interrupt::IO_IRQ_QSPI.disable(); + interrupt::IO_IRQ_QSPI.set_priority(interrupt::Priority::P3); + interrupt::IO_IRQ_QSPI.enable(); + } } #[cfg(feature = "rt")] -#[interrupt] -fn IO_IRQ_BANK0() { +fn irq_handler(bank: pac::io::Io, wakers: &[AtomicWaker; N]) { let cpu = SIO.cpuid().read() as usize; // There are two sets of interrupt registers, one for cpu0 and one for cpu1 // and here we are selecting the set that belongs to the currently executing // cpu. - let proc_intx: pac::io::Int = pac::IO_BANK0.int_proc(cpu); - for pin in 0..PIN_COUNT { + let proc_intx: pac::io::Int = bank.int_proc(cpu); + for pin in 0..N { // There are 4 raw interrupt status registers, PROCx_INTS0, PROCx_INTS1, // PROCx_INTS2, and PROCx_INTS3, and we are selecting the one that the // current pin belongs to. @@ -172,11 +182,23 @@ fn IO_IRQ_BANK0() { w.set_level_high(pin_group, true); w.set_level_low(pin_group, true); }); - INTERRUPT_WAKERS[pin as usize].wake(); + wakers[pin as usize].wake(); } } } +#[cfg(feature = "rt")] +#[interrupt] +fn IO_IRQ_BANK0() { + irq_handler(pac::IO_BANK0, &BANK0_WAKERS); +} + +#[cfg(all(feature = "rt", feature = "qspi-as-gpio"))] +#[interrupt] +fn IO_IRQ_QSPI() { + irq_handler(pac::IO_QSPI, &QSPI_WAKERS); +} + #[must_use = "futures do nothing unless you `.await` or poll them"] struct InputFuture<'a, T: Pin> { pin: PeripheralRef<'a, T>, @@ -195,7 +217,7 @@ impl<'d, T: Pin> InputFuture<'d, T> { // (the alternative being checking the current level and waiting for // its inverse, but that requires reading the current level and thus // missing anything that happened before the level was read.) - pac::IO_BANK0.intr(pin.pin() as usize / 8).write(|w| { + pin.io().intr(pin.pin() as usize / 8).write(|w| { w.set_edge_high(pin_group, true); w.set_edge_low(pin_group, true); }); @@ -235,7 +257,12 @@ impl<'d, T: Pin> Future for InputFuture<'d, T> { fn poll(self: FuturePin<&mut Self>, cx: &mut Context<'_>) -> Poll { // We need to register/re-register the waker for each poll because any // calls to wake will deregister the waker. - INTERRUPT_WAKERS[self.pin.pin() as usize].register(cx.waker()); + let waker = match self.pin.bank() { + Bank::Bank0 => &BANK0_WAKERS[self.pin.pin() as usize], + #[cfg(feature = "qspi-as-gpio")] + Bank::Qspi => &QSPI_WAKERS[self.pin.pin() as usize], + }; + waker.register(cx.waker()); // self.int_proc() will get the register offset for the current cpu, // then we want to access the interrupt enable register for our