cpu/stm32: Make ADC Resolution Definition uniform
Contribution description
This PR is based on #20780.
The different implementations for the STM32 ADCs had wildly different styles of defining (in cpu/stm32/include/periph/xx/periph_cpu.h) and checking the resolution flags (in cpu/stm32/periph/adc_xxx.c ).
The c0 and g0 did not define the adc_res_t structure at all, so the generic fallback from drivers/include/periph/adc.h was used. However it appears that this did not set the right flags, because for example the value for 6-bit resolution from the generic structure is 00 whereas it should be (0x3 << 3). Even worse, for higher resolutions, the DMA bits would be set instead of the resolution bits.
I did not check it yet, but it seems like the c0 and g0 STM32s were always stuck in 12-bit mode with the current code.
This PR makes all adc_res_t structure definitions uniformly, using the official register macros instead of magic numbers. Furthermore, the resolution check uses the official register mask for evaluating the resolution.
(Note: The F1 series only supports 12-bit mode, so nothing was changed here. There are no registers to be set, so the generic structure works for it.)
Testing procedure
I did not test this PR yet, because I don't have any Nucleos on hand until next week, but the tests/periph/adc test should be working on all affected Nucleos now.
@krzysztof-cabaj if you have time, you could look into testing this PR with some Nucleos that I don't have?
- [ ] C0 Series tested
- [ ] F0 Series tested
- [x] F2 Series tested
- [x] F3 Series tested
- [x] F4 Series tested
- [ ] F7 Series tested
- [ ] G0 Series tested
- [ ] L0 Series tested
- [ ] L1 Series tested
- [x] L4 Series tested
- [ ] WB Series tested
- [ ] WL Series tested
Issues/PRs references
This closes #20780.
STM32C0x1 Reference Manual: https://www.st.com/resource/en/reference_manual/rm0490-stm32c0x1-advanced-armbased-32bit-mcus-stmicroelectronics.pdf p.215
STM32F205, 207, 215, 217 Reference Manual: https://www.st.com/resource/en/reference_manual/rm0033-stm32f205xx-stm32f207xx-stm32f215xx-and-stm32f217xx-advanced-armbased-32bit-mcus-stmicroelectronics.pdf p.240
STM32G0x0 Reference Manual: https://www.st.com/resource/en/reference_manual/rm0454-stm32g0x0-advanced-armbased-32bit-mcus-stmicroelectronics.pdf p.320
STM32G0x1 Reference Manual: https://www.st.com/resource/en/reference_manual/dm00371828-stm32g0x1-advanced-arm-based-32-bit-mcus-stmicroelectronics.pdf The resolution bits are defined on p.389
As described in https://github.com/RIOT-OS/RIOT/issues/20780#issuecomment-2482672894, this PR requires some additional work, at least for the L0 series. The resolution setting is ignored due to a incorrect configuration sequence of the ADC registers.
Perhaps I'll create another PR to fix that before this one can be properly tested.
Are you planning to merge the sprawling STM32 ADC driver implementations into one? :smiley:
Are you planning to merge the sprawling STM32 ADC driver implementations into one? 😃
I don't think that would be a good idea, the ADCs are actually pretty different between the microcontrollers and the current implementations reflect all the different flavors. Even worse, the STM32WB0x line introduces yet another ADC flavor, so another implementation will be added.
While you could merge all the files into one, but it would be a big #ifdef party that nobody can handle anymore 😅
Similarly to the discussion in #21011, I changed the definitions used in the resolution check from the ones with _Msk suffix to the "normal" definition that is used everywhere else in the code.
Now that PR #21011 is merged, I can confirm that this PR does work as it should with the L0 and L1 series now. The boards used for testing were the NUCLEO-L073RZ and NUCLEO-L152RE.
Edit: I hope that I can get more Nucleos for testing soon.
I can confirm that it works with:
- nucleo-c071rb (with the STM32C0 pull request)
- nucleo-f030r8 (with master and #21222)
- nucleo-f207zg
- nucleo-f302r8
Furthermore I have the following Nucleos which should be supported, but don't have the feature defined nor the pins mapped:
- nucleo-f722zg
- nucleo-wl55jc (it's a Nucleo-WL55JC2, but JC1 and JC2 only differ in the radio frequency)
@crasbe
I add support for nucleo-f722ze - but only looking at documentation. Could you test it as you have access to this boards?
Now that the last PR is in, this PR got rebased and I retested everything with everything I have here at home:
-
nucleo-c031c6(has no vbat) (withPROGRAMMER=cpy2remed, because OpenOCD v0.11.0 does not support the C0 series yet) -
nucleo-f030r8(has no vbat) -
nucleo-f207zgwithperiph_vbat(with #21346) -
nucleo-f302r8withperiph_vbat -
nucleo-f722zewithperiph_vbat -
nucleo-g071rbwithperiph_vbat -
nucleo-wb55jcwithperiph_vbat -
p-nucleo-wb55withperiph_vbat
Murdock results
:heavy_check_mark: PASSED
1d3939d882ca1966187bbbb5965f51a73de4a478 cpu/stm32: make ADC resolution uniform
| Success | Failures | Total | Runtime |
|---|---|---|---|
| 10279 | 0 | 10280 | 08m:38s |
Artifacts
Thanks everyone for helping me on this journey :)