Why does adc_sample on sam0 disable all (ADC) interrupts?

oleg · 22 October 2020 19:09

Dear reading IOTlers,

while trying to use some non-basic ADC features on the SAMD21 I realized that the adc_sample() function of the periph/adc driver of the sam0_common implementation disable all interrupts:

github.com

RIOT-OS/RIOT/blob/master/cpu/sam0_common/periph/adc.c#L219


    ADC_DEV->CTRLC.reg |= res;
#else
    /* Reset resolution bits in CTRLB */
    ADC_DEV->CTRLB.reg &= ~ADC_CTRLB_RESSEL_Msk;
    ADC_DEV->CTRLB.reg |= res;
#endif

    /* Set Voltage Reference */
    ADC_DEV->REFCTRL.reg = ADC_REF_DEFAULT;
    /* Disable all interrupts */
    ADC_DEV->INTENCLR.reg = 0xFF;

#ifdef SYSCTRL_VREF_BGOUTEN
    /* Enable bandgap if VREF is internal 1V */
    if (ADC_REF_DEFAULT == ADC_REFCTRL_REFSEL_INT1V) {
        SYSCTRL->VREF.reg |= SYSCTRL_VREF_BGOUTEN;
    }
#else
    /* Enable bandgap if necessary */
    if (ADC_REF_DEFAULT == ADC_REFCTRL_REFSEL_INTREF) {
        SUPC->VREF.reg |= SUPC_VREF_VREFOE;

However, I failed to understand why this is done (a comment might help here). Does anyone remember the reason? (The original code is quite old (more than four years).)

Cheers Oleg

Kaspar · 26 October 2020 09:50

Well, if nothing is handling the interrupt, it needs to be disabled. Or what would you expect to happen if the ADC fires an ISR e.g., for the sampling-done event?

benpicco · 26 October 2020 19:52

@maribu proposed a new ADC API that will also allow for conversions in the background.

Unfortunately it seems like nobody has time to look into that right now.

The current ADC API is indeed quite limited.

oleg · 26 October 2020 20:02

I think I found the reason in the mean time: interrupts must be disabled when polling for the result.

However, it is suboptimal to have this kind of side effect.

@kaspar, of course, interrupts should be disabled without a ISR defined, but this should be asserted during CPU initialization not in an API function.

benpicco · 26 October 2020 20:04

Actually this line should not be necessary at all. As long as the ADC interrupt is no enabled in NVIC, setting which bits on the ADC may trigger such an interrupt should not be necessary.