From 972cdd4265b24efb101c6b9df373466fcbccac5d Mon Sep 17 00:00:00 2001 From: pennae Date: Wed, 5 Jul 2023 20:05:34 +0200 Subject: [PATCH] rp/adc: rewrite the module - don't require an irq binding for blocking-only adc - abstract adc pins into an AnyPin like interface, erasing the actual peripheral type at runtime. - add pull-up/pull-down functions for adc pins - add a test (mostly a copy of the example, to be honest) - configure adc pads according to datasheet - report conversion errors (although they seem exceedingly rare?) - drop embedded-hal interfaces. embedded-hal channels can do neither AnyPin nor pullup/pulldown without encoding both into the type --- embassy-rp/src/adc.rs | 227 +++++++++++++++++++++++-------------- embassy-rp/src/gpio.rs | 2 +- examples/rp/src/bin/adc.rs | 17 +-- tests/rp/src/bin/adc.rs | 86 ++++++++++++++ 4 files changed, 238 insertions(+), 94 deletions(-) create mode 100644 tests/rp/src/bin/adc.rs diff --git a/embassy-rp/src/adc.rs b/embassy-rp/src/adc.rs index 699a0d61d..dfa1b877a 100644 --- a/embassy-rp/src/adc.rs +++ b/embassy-rp/src/adc.rs @@ -3,22 +3,17 @@ use core::marker::PhantomData; use core::sync::atomic::{compiler_fence, Ordering}; use core::task::Poll; +use embassy_hal_common::{into_ref, PeripheralRef}; use embassy_sync::waitqueue::AtomicWaker; -use embedded_hal_02::adc::{Channel, OneShot}; -use crate::gpio::Pin; +use crate::gpio::sealed::Pin as GpioPin; +use crate::gpio::{self, AnyPin, Pull}; use crate::interrupt::typelevel::Binding; use crate::interrupt::InterruptExt; use crate::peripherals::ADC; use crate::{interrupt, pac, peripherals, Peripheral}; -static WAKER: AtomicWaker = AtomicWaker::new(); -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] -#[non_exhaustive] -pub enum Error { - // No errors for now -} +static WAKER: AtomicWaker = AtomicWaker::new(); #[non_exhaustive] pub struct Config {} @@ -28,11 +23,65 @@ impl Default for Config { Self {} } } -pub struct Adc<'d> { - phantom: PhantomData<&'d ADC>, + +pub struct Pin<'p> { + pin: PeripheralRef<'p, AnyPin>, } -impl<'d> Adc<'d> { +impl<'p> Pin<'p> { + pub fn new(pin: impl Peripheral

+ 'p, pull: Pull) -> Self { + into_ref!(pin); + pin.pad_ctrl().modify(|w| { + // manual says: + // + // > When using an ADC input shared with a GPIO pin, the pin’s + // > digital functions must be disabled by setting IE low and OD + // > high in the pin’s pad control register + w.set_ie(false); + w.set_od(true); + w.set_pue(pull == Pull::Up); + w.set_pde(pull == Pull::Down); + }); + Self { pin: pin.map_into() } + } + + fn channel(&self) -> u8 { + // this requires adc pins to be sequential and matching the adc channels, + // which is the case for rp2040 + self.pin._pin() - 26 + } +} + +impl<'d> Drop for Pin<'d> { + fn drop(&mut self) { + self.pin.pad_ctrl().modify(|w| { + w.set_ie(true); + w.set_od(false); + w.set_pue(false); + w.set_pde(true); + }); + } +} + +#[derive(Debug, Eq, PartialEq, Copy, Clone)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum Error { + ConversionFailed, +} + +pub trait Mode {} + +pub struct Async; +impl Mode for Async {} + +pub struct Blocking; +impl Mode for Blocking {} + +pub struct Adc<'d, M: Mode> { + phantom: PhantomData<(&'d ADC, M)>, +} + +impl<'d, M: Mode> Adc<'d, M> { #[inline] fn regs() -> pac::adc::Adc { pac::ADC @@ -45,11 +94,7 @@ impl<'d> Adc<'d> { ret } - pub fn new( - _inner: impl Peripheral

+ 'd, - _irq: impl Binding, - _config: Config, - ) -> Self { + fn setup() { let reset = Self::reset(); crate::reset::reset(reset); crate::reset::unreset_wait(reset); @@ -58,6 +103,43 @@ impl<'d> Adc<'d> { r.cs().write(|w| w.set_en(true)); // Wait for ADC ready while !r.cs().read().ready() {} + } + + fn sample_blocking(channel: u8) -> Result { + let r = Self::regs(); + r.cs().modify(|w| { + w.set_ainsel(channel); + w.set_start_once(true); + w.set_err(true); + }); + while !r.cs().read().ready() {} + match r.cs().read().err() { + true => Err(Error::ConversionFailed), + false => Ok(r.result().read().result().into()), + } + } + + pub fn blocking_read(&mut self, pin: &mut Pin) -> Result { + Self::sample_blocking(pin.channel()) + } + + pub fn blocking_read_temperature(&mut self) -> Result { + let r = Self::regs(); + r.cs().modify(|w| w.set_ts_en(true)); + while !r.cs().read().ready() {} + let result = Self::sample_blocking(4); + r.cs().modify(|w| w.set_ts_en(false)); + result + } +} + +impl<'d> Adc<'d, Async> { + pub fn new( + _inner: impl Peripheral

+ 'd, + _irq: impl Binding, + _config: Config, + ) -> Self { + Self::setup(); // Setup IRQ interrupt::ADC_IRQ_FIFO.unpend(); @@ -80,76 +162,42 @@ impl<'d> Adc<'d> { .await; } - pub async fn read, ID = u8> + Pin>(&mut self, pin: &mut PIN) -> u16 { + async fn sample_async(channel: u8) -> Result { let r = Self::regs(); - // disable pull-down and pull-up resistors - // pull-down resistors are enabled by default - pin.pad_ctrl().modify(|w| { - w.set_ie(true); - let (pu, pd) = (false, false); - w.set_pue(pu); - w.set_pde(pd); - }); r.cs().modify(|w| { - w.set_ainsel(PIN::channel()); - w.set_start_once(true) + w.set_ainsel(channel); + w.set_start_once(true); + w.set_err(true); }); Self::wait_for_ready().await; - r.result().read().result().into() + match r.cs().read().err() { + true => Err(Error::ConversionFailed), + false => Ok(r.result().read().result().into()), + } } - pub async fn read_temperature(&mut self) -> u16 { + pub async fn read(&mut self, pin: &mut Pin<'_>) -> Result { + Self::sample_async(pin.channel()).await + } + + pub async fn read_temperature(&mut self) -> Result { let r = Self::regs(); r.cs().modify(|w| w.set_ts_en(true)); if !r.cs().read().ready() { Self::wait_for_ready().await; } - r.cs().modify(|w| { - w.set_ainsel(4); - w.set_start_once(true) - }); - Self::wait_for_ready().await; - r.result().read().result().into() - } - - pub fn blocking_read, ID = u8> + Pin>(&mut self, pin: &mut PIN) -> u16 { - let r = Self::regs(); - pin.pad_ctrl().modify(|w| { - w.set_ie(true); - let (pu, pd) = (false, false); - w.set_pue(pu); - w.set_pde(pd); - }); - r.cs().modify(|w| { - w.set_ainsel(PIN::channel()); - w.set_start_once(true) - }); - while !r.cs().read().ready() {} - r.result().read().result().into() - } - - pub fn blocking_read_temperature(&mut self) -> u16 { - let r = Self::regs(); - r.cs().modify(|w| w.set_ts_en(true)); - while !r.cs().read().ready() {} - r.cs().modify(|w| { - w.set_ainsel(4); - w.set_start_once(true) - }); - while !r.cs().read().ready() {} - r.result().read().result().into() + let result = Self::sample_async(4).await; + r.cs().modify(|w| w.set_ts_en(false)); + result } } -macro_rules! impl_pin { - ($pin:ident, $channel:expr) => { - impl Channel> for peripherals::$pin { - type ID = u8; - fn channel() -> u8 { - $channel - } - } - }; +impl<'d> Adc<'d, Blocking> { + pub fn new_blocking(_inner: impl Peripheral

+ 'd, _config: Config) -> Self { + Self::setup(); + + Self { phantom: PhantomData } + } } pub struct InterruptHandler { @@ -158,24 +206,33 @@ pub struct InterruptHandler { impl interrupt::typelevel::Handler for InterruptHandler { unsafe fn on_interrupt() { - let r = Adc::regs(); + let r = Adc::::regs(); r.inte().write(|w| w.set_fifo(false)); WAKER.wake(); } } +mod sealed { + pub trait AdcPin: crate::gpio::sealed::Pin { + fn channel(&mut self) -> u8; + } +} + +pub trait AdcPin: sealed::AdcPin + gpio::Pin {} + +macro_rules! impl_pin { + ($pin:ident, $channel:expr) => { + impl sealed::AdcPin for peripherals::$pin { + fn channel(&mut self) -> u8 { + $channel + } + } + + impl AdcPin for peripherals::$pin {} + }; +} + impl_pin!(PIN_26, 0); impl_pin!(PIN_27, 1); impl_pin!(PIN_28, 2); impl_pin!(PIN_29, 3); - -impl OneShot, WORD, PIN> for Adc<'static> -where - WORD: From, - PIN: Channel, ID = u8> + Pin, -{ - type Error = (); - fn read(&mut self, pin: &mut PIN) -> nb::Result { - Ok(self.blocking_read(pin).into()) - } -} diff --git a/embassy-rp/src/gpio.rs b/embassy-rp/src/gpio.rs index f8048a4dd..d18fb909c 100644 --- a/embassy-rp/src/gpio.rs +++ b/embassy-rp/src/gpio.rs @@ -41,7 +41,7 @@ impl From for bool { } /// Represents a pull setting for an input. -#[derive(Debug, Eq, PartialEq)] +#[derive(Debug, Clone, Copy, Eq, PartialEq)] pub enum Pull { None, Up, diff --git a/examples/rp/src/bin/adc.rs b/examples/rp/src/bin/adc.rs index 7c2ca19f7..65069cde1 100644 --- a/examples/rp/src/bin/adc.rs +++ b/examples/rp/src/bin/adc.rs @@ -4,8 +4,9 @@ use defmt::*; use embassy_executor::Spawner; -use embassy_rp::adc::{Adc, Config, InterruptHandler}; +use embassy_rp::adc::{Adc, Config, InterruptHandler, Pin}; use embassy_rp::bind_interrupts; +use embassy_rp::gpio::Pull; use embassy_time::{Duration, Timer}; use {defmt_rtt as _, panic_probe as _}; @@ -18,18 +19,18 @@ async fn main(_spawner: Spawner) { let p = embassy_rp::init(Default::default()); let mut adc = Adc::new(p.ADC, Irqs, Config::default()); - let mut p26 = p.PIN_26; - let mut p27 = p.PIN_27; - let mut p28 = p.PIN_28; + let mut p26 = Pin::new(p.PIN_26, Pull::None); + let mut p27 = Pin::new(p.PIN_27, Pull::None); + let mut p28 = Pin::new(p.PIN_28, Pull::None); loop { - let level = adc.read(&mut p26).await; + let level = adc.read(&mut p26).await.unwrap(); info!("Pin 26 ADC: {}", level); - let level = adc.read(&mut p27).await; + let level = adc.read(&mut p27).await.unwrap(); info!("Pin 27 ADC: {}", level); - let level = adc.read(&mut p28).await; + let level = adc.read(&mut p28).await.unwrap(); info!("Pin 28 ADC: {}", level); - let temp = adc.read_temperature().await; + let temp = adc.read_temperature().await.unwrap(); info!("Temp: {} degrees", convert_to_celsius(temp)); Timer::after(Duration::from_secs(1)).await; } diff --git a/tests/rp/src/bin/adc.rs b/tests/rp/src/bin/adc.rs new file mode 100644 index 000000000..e659844ae --- /dev/null +++ b/tests/rp/src/bin/adc.rs @@ -0,0 +1,86 @@ +#![no_std] +#![no_main] +#![feature(type_alias_impl_trait)] +#[path = "../common.rs"] +mod common; + +use defmt::*; +use embassy_executor::Spawner; +use embassy_rp::adc::{Adc, Config, InterruptHandler, Pin}; +use embassy_rp::bind_interrupts; +use embassy_rp::gpio::Pull; +use {defmt_rtt as _, panic_probe as _}; + +bind_interrupts!(struct Irqs { + ADC_IRQ_FIFO => InterruptHandler; +}); + +#[embassy_executor::main] +async fn main(_spawner: Spawner) { + let mut p = embassy_rp::init(Default::default()); + let mut adc = Adc::new(p.ADC, Irqs, Config::default()); + + { + { + let mut p = Pin::new(&mut p.PIN_26, Pull::Down); + defmt::assert!(adc.blocking_read(&mut p).unwrap() < 0b01_0000_0000); + defmt::assert!(adc.read(&mut p).await.unwrap() < 0b01_0000_0000); + } + { + let mut p = Pin::new(&mut p.PIN_26, Pull::Up); + defmt::assert!(adc.blocking_read(&mut p).unwrap() > 0b11_0000_0000); + defmt::assert!(adc.read(&mut p).await.unwrap() > 0b11_0000_0000); + } + } + // not bothering with async reads from now on + { + { + let mut p = Pin::new(&mut p.PIN_27, Pull::Down); + defmt::assert!(adc.blocking_read(&mut p).unwrap() < 0b01_0000_0000); + } + { + let mut p = Pin::new(&mut p.PIN_27, Pull::Up); + defmt::assert!(adc.blocking_read(&mut p).unwrap() > 0b11_0000_0000); + } + } + { + { + let mut p = Pin::new(&mut p.PIN_28, Pull::Down); + defmt::assert!(adc.blocking_read(&mut p).unwrap() < 0b01_0000_0000); + } + { + let mut p = Pin::new(&mut p.PIN_28, Pull::Up); + defmt::assert!(adc.blocking_read(&mut p).unwrap() > 0b11_0000_0000); + } + } + { + // gp29 is connected to vsys through a 200k/100k divider, + // adding pulls should change the value + let low = { + let mut p = Pin::new(&mut p.PIN_29, Pull::Down); + adc.blocking_read(&mut p).unwrap() + }; + let none = { + let mut p = Pin::new(&mut p.PIN_29, Pull::None); + adc.blocking_read(&mut p).unwrap() + }; + let up = { + let mut p = Pin::new(&mut p.PIN_29, Pull::Up); + adc.blocking_read(&mut p).unwrap() + }; + defmt::assert!(low < none); + defmt::assert!(none < up); + } + + let temp = convert_to_celsius(adc.read_temperature().await.unwrap()); + defmt::assert!(temp > 0.0); + defmt::assert!(temp < 60.0); + + info!("Test OK"); + cortex_m::asm::bkpt(); +} + +fn convert_to_celsius(raw_temp: u16) -> f32 { + // According to chapter 4.9.5. Temperature Sensor in RP2040 datasheet + 27.0 - (raw_temp as f32 * 3.3 / 4096.0 - 0.706) / 0.001721 as f32 +}