RIOT-OS
stm32/adc: make resolution flag checks consistent for all STM32 families
Description
As discovered in https://github.com/RIOT-OS/RIOT/pull/20773#discussion_r1665435304, some variants of the STM32 ADC peripheral use magic numbers in an undocumented way to check if the ADC resolution has a valid value or or not: https://github.com/RIOT-OS/RIOT/blob/master/cpu/stm32/periph/adc_f0_g0_c0.c#L103-L106 https://github.com/RIOT-OS/RIOT/blob/master/cpu/stm32/periph/adc_f2.c#L112-L114 https://github.com/RIOT-OS/RIOT/blob/master/cpu/stm32/periph/adc_f3.c#L196-L199 https://github.com/RIOT-OS/RIOT/blob/master/cpu/stm32/periph/adc_f4_f7.c#L131-L134 https://github.com/RIOT-OS/RIOT/blob/master/cpu/stm32/periph/adc_l4_wb.c#L220-L223 https://github.com/RIOT-OS/RIOT/blob/master/cpu/stm32/periph/adc_wl.c#L121-L124
Some other implementations explicitly check if the resolution value is one of the valid values, so IMO this is the better approach: https://github.com/RIOT-OS/RIOT/blob/master/cpu/stm32/periph/adc_f1.c#L146-L149 https://github.com/RIOT-OS/RIOT/blob/master/cpu/stm32/periph/adc_l0.c#L120-L126 https://github.com/RIOT-OS/RIOT/blob/master/cpu/stm32/periph/adc_l1.c#L149-L155
I have the following STM32 boards available: NUCLEO-F413ZH NUCLEO-G474RE (not supported yet, seems to have the same ADC as the H7 series and quite similar to L4, just a little more options) NUCLEO-L073RZ (irrelevant, won't be changed) NUCLEO-L452RE P-NUCLEO-WB55 (duplicate for L4)
So we're still missing F0/F0/C0, F2, F3 and WL boards for testing. Maybe I can get some of them from Mouser in the future, but maybe someone else has some of these boards for testing.
Useful links
STM32G474 Reference Manual: https://www.st.com/resource/en/datasheet/stm32g474cb.pdf p.591
I have access to F207 and F334. I will try to arrange access to C031.
It looks like this is turning into a bit of a rabbit hole again :melting_face:
To test the different resolutions, I modified the tests/periph/adc test to sample once with every resolution:
...
while (1) {
for (unsigned i = 0; i < ADC_NUMOF; i++) {
int sample6 = adc_sample(ADC_LINE(i), ADC_RES_6BIT);
int sample8 = adc_sample(ADC_LINE(i), ADC_RES_8BIT);
int sample10 = adc_sample(ADC_LINE(i), ADC_RES_10BIT);
int sample12 = adc_sample(ADC_LINE(i), ADC_RES_12BIT);
int sample14 = adc_sample(ADC_LINE(i), ADC_RES_14BIT);
int sample16 = adc_sample(ADC_LINE(i), ADC_RES_16BIT);
printf("ADC_LINE(%u): %i %i %i %i %i %i\n", i, sample6, sample8, sample10, sample12, sample14, sample16);
}
ztimer_sleep(ZTIMER_MSEC, DELAY_MS);
}
...
The variable for the sample has to be commented out, so it does not throw a compiler error.
- int sample = 0;
+//int sample = 0;
When I connect one of the ADC inputs to 3.3V, this is what the output looks like:
2024-11-18 11:26:53,002 # ADC_LINE(0): 4095 4095 4095 4095 -1 -1
2024-11-18 11:26:53,006 # ADC_LINE(1): 923 672 589 544 -1 -1
2024-11-18 11:26:53,010 # ADC_LINE(2): 301 150 86 48 -1 -1
2024-11-18 11:26:53,013 # ADC_LINE(3): 480 510 530 528 -1 -1
2024-11-18 11:26:53,017 # ADC_LINE(4): 496 520 542 549 -1 -1
2024-11-18 11:26:53,021 # ADC_LINE(5): 489 515 518 521 -1 -1
The two -1 at the end are expected, because the L073 does not support 14-bit and 16-bit conversions, but the other values shouldn't be all 4096 = 0b00001111.11111111, that is 12-bit resolution.
I'm not 100% sure yet what the reason is for this behavior, but it looks like the ADC configuration in cpu/stm32/periph/adc_l0.c (and potentially others as well) is incorrect. The reference manual[1] states the following on page 311, section 14.3.7:
The software must write the ADCAL and ADEN bits in the ADC_CR register and configure
the ADC_CFGR1 and ADC_CFGR2 registers only when the ADC is disabled (ADEN must
be cleared).
This is not the case in the current code, the resolution (among other things) is configured with the ADC enabled, which might lead to the ADC ignoring the configuration.
[1] https://www.st.com/resource/en/reference_manual/rm0367-ultralowpower-stm32l0x3-advanced-armbased-32bit-mcus-stmicroelectronics.pdf
Interestingly, for the STM32L4, the behavior is different. I have a NUCLEO-L452RE here, and this is what the output looks like:
2024-11-18 11:52:59,127 # ADC_LINE(0): 63 255 1023 4095 -1 -1
2024-11-18 11:52:59,130 # ADC_LINE(1): 24 90 364 1455 -1 -1
2024-11-18 11:52:59,133 # ADC_LINE(2): 22 89 369 1482 -1 -1
2024-11-18 11:52:59,136 # ADC_LINE(3): 22 89 368 1474 -1 -1
2024-11-18 11:52:59,139 # ADC_LINE(4): 21 89 366 1474 -1 -1
2024-11-18 11:52:59,142 # ADC_LINE(5): 21 88 363 1459 -1 -1
2024-11-18 11:52:59,145 # ADC_LINE(6): 13 46 199 792 -1 -1
As expected, the resolution was applied correctly.
The reference manual of the STM32L4 [2] page 631, section 21.4.10 explains that you're only allowed to write to the configuration when the ADC is enabled, so exactly the opposite of the L0.
For all the other control bits of the ADC_CFGR, ADC_SMPRx, ADC_SQRy, ADC_JDRy,
ADC_OFRy, ADC_OFCHRy and ADC_IER registers:
• For control bits related to configuration of regular conversions, the software is allowed
to write them only if the ADC is enabled (ADEN=1) and if there is no regular conversion
ongoing (ADSTART must be equal to 0).
• For control bits related to configuration of injected conversions, the software is allowed
to write them only if the ADC is enabled (ADEN=1) and if there is no injected
conversion ongoing (JADSTART must be equal to 0).
Now I only have to find out how the ADCs of the other devices behave (or should behave).
[2] https://www.st.com/resource/en/reference_manual/rm0432-stm32l4-series-advanced-armbased-32bit-mcus-stmicroelectronics.pdf
The test on the NUCLEO-L152RE does not run successfully at all, it runs into a hard fault after the first loop iteration:
2024-11-18 15:04:39,478 # main(): This is RIOT! (Version: 2025.01-devel-30-ge6bf3-pr/stm32_adc)
2024-11-18 15:04:39,478 #
2024-11-18 15:04:39,481 # RIOT ADC peripheral driver test
2024-11-18 15:04:39,481 #
2024-11-18 15:04:39,487 # This test will sample all available ADC lines once every 100ms with
2024-11-18 15:04:39,492 # a 10-bit resolution and print the sampled results to STDIO
2024-11-18 15:04:39,492 #
2024-11-18 15:04:39,493 #
2024-11-18 15:04:39,496 # Successfully initialized ADC_LINE(0)
2024-11-18 15:04:39,499 # Successfully initialized ADC_LINE(1)
2024-11-18 15:04:39,502 # Successfully initialized ADC_LINE(2)
2024-11-18 15:04:39,506 # Successfully initialized ADC_LINE(3)
2024-11-18 15:04:39,509 # Successfully initialized ADC_LINE(4)
2024-11-18 15:04:39,512 # Successfully initialized ADC_LINE(5)
2024-11-18 15:04:39,515 # ADC_LINE(0): 19 25 28 30 -1 -1
2024-11-18 15:04:39,518 # ADC_LINE(1): 63 63 63 63 -1 -1
2024-11-18 15:04:39,521 # ADC_LINE(2): 27 28 29 30 -1 -1
2024-11-18 15:04:39,524 # ADC_LINE(3): 24 27 28 29 -1 -1
2024-11-18 15:04:39,527 # ADC_LINE(4): 22 27 30 31 -1 -1
2024-11-18 15:04:39,530 # ADC_LINE(5): 25 29 30 31 -1 -1
2024-11-18 15:04:39,630 #
2024-11-18 15:04:39,633 # Context before hardfault:
(Nothing is printed after the "Context before hardfault:" message.)
Furthermore the ADC seems to be stuck in 6 bit mode, I connected Pin A1 to 3.3V.
The behavior is the same on the master branch (both hard fault and 6 bit).
I'm not sure yet what the cause of the issue is, but it might have to do with the watchdog not being correctly configured. But first I'd have to check where exactly the test program crashes.
[3] https://www.st.com/resource/en/reference_manual/cd00240193-stm32l100xx-stm32l151xx-stm32l152xx-and-stm32l162xx-advanced-arm-based-32-bit-mcus-stmicroelectronics.pdf
Update: The L0 fix was rather easy, just move the _enable_adc() function from line 129 to 144 (after the last access to one of the config registers) in cpu/stm32/periph/adc_l0.c.
The L1 issue is not caused by the ADC, but by ztimer. Commenting out the ztimer delay gets rid of the Hard Fault.
Furthermore, the reference manual [3] has a small note about the resolution flags on page 291:
This bit must be written only when ADON=0.
However interestingly, this was not the reason why the ADC was stuck in 6-bit mode. The reason was that the resolution bits of the ADC->CR1 register were never cleared. With every operation, the resolution was only OR-ed onto the register and 6-bit has the bits 11. Therefore, after setting the resolution to 6-bit once, it was stuck there.
Likewise, a very simple fix in cpu/stm32/periph/adc_l1.c
/* set resolution, conversion channel and single read */
+ ADC1->CR1 &= ~ADC_CR1_RES_Msk;
ADC1->CR1 |= res & ADC_CR1_RES;
ADC1->SQR1 &= ~ADC_SQR1_L;
ADC1->SQR5 = adc_config[line].chan;
None of the other ADC implementations suffer from the same bug as the L1, so I'll create a PR to fix the L0 and L1 ADCs, so we can actually test #20971 🙃 The ztimer stuff is a different beast again, I'll have to look at it with a debugger tomorrow.
The L1 issue is not caused by the ADC, but by
ztimer. Commenting out the ztimer delay gets rid of the Hard Fault.
This error is the weirdest thing. The error depends on the way the program is flashed. If you flash it with OpenOCD make flash, the microcontroller will run into a Hard Fault and will Hard Fault when you press the reset button as well.
HOWEVER if you power cycle the device, it'll run as it should.
On the other hand, when you program it using the Mass Storage programmer of the built-in ST-Link, the program runs as well.
Programming the chip with the STM32CubeProgrammer will yield the same results as OpenOCD, so the internal MSD flasher does something different than the other programming mechanisms.
So my hypothesis is, that the ztimer code for the STM32L152RE does not fully configure something which makes the program crash (and crash again) and the reset of the ST-Link is somehow "harder" than the OpenOCD reset and the reset caused by the reset button.
I'll open a separate issue about it, because that doesn't really has anything to do and I already put in way too much time...