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

@@ -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 = impl AdcPin + 'p> + '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<P = ADC> + 'd,
-        _irq: impl Binding<interrupt::typelevel::ADC_IRQ_FIFO, InterruptHandler>,
-        _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<u16, Error> {
+        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<u16, Error> {
+        Self::sample_blocking(pin.channel())
+    }
+
+    pub fn blocking_read_temperature(&mut self) -> Result<u16, Error> {
+        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<P = ADC> + 'd,
+        _irq: impl Binding<interrupt::typelevel::ADC_IRQ_FIFO, InterruptHandler>,
+        _config: Config,
+    ) -> Self {
+        Self::setup();
 
         // Setup IRQ
         interrupt::ADC_IRQ_FIFO.unpend();
@@ -80,77 +162,43 @@ impl<'d> Adc<'d> {
         .await;
     }
 
-    pub async fn read<PIN: Channel<Adc<'d>, ID = u8> + Pin>(&mut self, pin: &mut PIN) -> u16 {
+    async fn sample_async(channel: u8) -> Result<u16, Error> {
         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<u16, Error> {
+        Self::sample_async(pin.channel()).await
+    }
+
+    pub async fn read_temperature(&mut self) -> Result<u16, Error> {
         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<PIN: Channel<Adc<'d>, 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<Adc<'static>> for peripherals::$pin {
-            type ID = u8;
-            fn channel() -> u8 {
-                $channel
+impl<'d> Adc<'d, Blocking> {
+    pub fn new_blocking(_inner: impl Peripheral<P = ADC> + 'd, _config: Config) -> Self {
+        Self::setup();
+
+        Self { phantom: PhantomData }
     }
 }
-    };
-}
 
 pub struct InterruptHandler {
     _empty: (),
@@ -158,24 +206,33 @@ pub struct InterruptHandler {
 
 impl interrupt::typelevel::Handler<interrupt::typelevel::ADC_IRQ_FIFO> for InterruptHandler {
     unsafe fn on_interrupt() {
-        let r = Adc::regs();
+        let r = Adc::<Async>::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<WORD, PIN> OneShot<Adc<'static>, WORD, PIN> for Adc<'static>
-where
-    WORD: From<u16>,
-    PIN: Channel<Adc<'static>, ID = u8> + Pin,
-{
-    type Error = ();
-    fn read(&mut self, pin: &mut PIN) -> nb::Result<WORD, Self::Error> {
-        Ok(self.blocking_read(pin).into())
-    }
-}

embassy-rp/src/gpio.rs

@@ -41,7 +41,7 @@ impl From<Level> for bool {
 }
 
 /// Represents a pull setting for an input.
-#[derive(Debug, Eq, PartialEq)]
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
 pub enum Pull {
     None,
     Up,

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;
     }

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
+}