Skip to content

Conversation

@jakoblell
Copy link

The function read_many_inner will use write_set for writing the adc div register. The write_set method will by definition only set bits in the register but it will never clear bits. This can lead to an incorrect value (bitwise or of all values configured in the past) in the div register when switching between different values without fully resetting the ADC in between.

Reproducer:

#![no_main]
#![no_std]


use embassy_executor::Spawner;
use embassy_rp::{bind_interrupts, pac};
use embassy_rp::adc::{Adc, Channel, Config, InterruptHandler};
use {defmt_rtt as _, panic_probe as _};
use defmt::{info};

bind_interrupts!(
    /// Binds the ADC interrupts.
    struct Irqs {
        ADC_IRQ_FIFO => InterruptHandler;
    }
);

#[embassy_executor::main]
async fn main(_spawner: Spawner) {
    let p = embassy_rp::init(Default::default());
    let mut adc = Adc::new(p.ADC, Irqs, Config::default());let mut dma = p.DMA_CH0;
    let mut ts = Channel::new_temp_sensor(p.ADC_TEMP_SENSOR);
    let mut buf = [0u16;32];
    // Run ADC with div=95 => one conversion every 96 clock cycles
    adc.read_many(&mut ts, &mut buf, 95, dma.reborrow()).await.unwrap();
    // Run ADC with div=96 => one conversion every 97 clock cycles
    adc.read_many(&mut ts, &mut buf, 96, dma.reborrow()).await.unwrap();
    // Read back the value from the raw div register, should be 96 but is actually 127
    let div = pac::ADC.div().read().int();
    info!("div={}", div);
}

This will print div=127 instead of the expected value div=96.

Please note that the read_many API does not support fractional dividers (and this cannot be changed without a breaking API change). In order to allow working around this restriction by manually setting the fractional part via pac::ADC (something which was already possible before this PR), this PR is using r.div().modify(...) (and not r.div().write(...)) so that it won't touch a fractional part configured manually before.

@jakoblell
Copy link
Author

Any way to get this PR merged soon? It's just a pretty simple bugfix - and I already had to fix a merge conflict in the changelog since some other changes have been merged in the mean time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant