From 4ed287cec8ebf85a02f7b8b6da78dd1b7f82f2b0 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Thu, 8 Feb 2024 14:31:46 +0100 Subject: [PATCH] cpu/stm32/periph_adc: fix register access The register access to SMPR1/SMPR2 was incorrect in three aspects: 1. For channels < 10, SMPR1 was cleared but SMPR2 should have been cleared 2. The code was not thread-safe 3. An unneeded write was issued. (The compiler won't combine the in-place bitwise operations into a single read-modify-write sequence on `volatile` memory.) Fixes https://github.com/RIOT-OS/RIOT/issues/20261 --- cpu/stm32/periph/adc_f4_f7.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/cpu/stm32/periph/adc_f4_f7.c b/cpu/stm32/periph/adc_f4_f7.c index 4c1dfa4b97e5e..b52fc401c691b 100644 --- a/cpu/stm32/periph/adc_f4_f7.c +++ b/cpu/stm32/periph/adc_f4_f7.c @@ -20,10 +20,11 @@ */ #include "cpu.h" +#include "irq.h" #include "mutex.h" #include "periph/adc.h" -#include "periph_conf.h" #include "periph/vbat.h" +#include "periph_conf.h" /** * @brief Maximum allowed ADC clock speed @@ -100,14 +101,20 @@ int adc_init(adc_t line) } ADC->CCR = ((clk_div / 2) - 1) << 16; /* set sampling time to the maximum */ + unsigned irq_state = irq_disable(); if (adc_config[line].chan >= 10) { - dev(line)->SMPR1 &= ~(MAX_ADC_SMP << (3 * (adc_config[line].chan - 10))); - dev(line)->SMPR1 |= MAX_ADC_SMP << (3 * (adc_config[line].chan - 10)); + uint32_t smpr1 = dev(line)->SMPR1; + smpr1 &= ~(MAX_ADC_SMP << (3 * (adc_config[line].chan - 10))); + smpr1 |= MAX_ADC_SMP << (3 * (adc_config[line].chan - 10)); + dev(line)->SMPR1 = smpr1; } else { - dev(line)->SMPR1 &= ~(MAX_ADC_SMP << (3 * adc_config[line].chan)); - dev(line)->SMPR2 |= MAX_ADC_SMP << (3 * adc_config[line].chan); + uint32_t smpr2 = dev(line)->SMPR2; + smpr2 &= ~(MAX_ADC_SMP << (3 * adc_config[line].chan)); + smpr2 |= MAX_ADC_SMP << (3 * adc_config[line].chan); + dev(line)->SMPR2 = smpr2; } + irq_restore(irq_state); /* free the device again */ done(line); return 0;