RIOT-OS
cpu/stm32l0,l1: Fix ADC initialization order
Contribution description
Most of this PR is already described in #20780.
The STM32L0 was enabled before the resolution bits were set, but any access to configuration registers is prohibited (and seemingly ignored by the microcontroller). Therefore, the resolution set in adc_sample was not actually applied and the resolution was always 12-bit.
The same issue was present in the STM32L1, however this ADC hardware implementation does not seem to ignore configuration register calls (even though they are prohibited when the ADC is on). However, the resolution bits were not masked in the configuration register before setting the new resolution, leading to the resolution being stuck at 6-bit (0b11). The resolution was only OR-ed to the register.
Testing procedure
For convenience you can change the test application (tests/periph/adc) to print all resolution values at once, as described in https://github.com/RIOT-OS/RIOT/issues/20780#issuecomment-2482672894.
Due to the issue described in #21010, I recommend flashing the NUCLEO-L152RE board with the Mass Storage Driver of the ST-Link or doing a powercycle (unplug and plug the USB cable back in) after flashing. This issue is not related to these changes.
You can connect one of the A0 to A5 pins of the Nucleo to +3.3V and observe the output.
tl;dr: The output should look something like this, when the channel is maxed out on all implemented bit sizes. The two -1 at the end show that the 14-bit and 16-bit resolution is not implemented.
expected:
ADC_LINE(5): 63 255 1023 4095 -1 -1
actual:
ADC_LINE(5): 4095 4095 4095 4095 -1 -1 (NUCLEO-L073RZ)
ADC_LINE(5): 63 63 63 63 -1 -1 (NUCLEO-L152RE)
Nucleo-L073RZ Master:
main(): This is RIOT! (Version: 2025.01-devel-29-g76b9b)
RIOT ADC peripheral driver test
This test will sample all available ADC lines once every 100ms with
a 10-bit resolution and print the sampled results to STDIO
Successfully initialized ADC_LINE(0)
Successfully initialized ADC_LINE(1)
Successfully initialized ADC_LINE(2)
Successfully initialized ADC_LINE(3)
Successfully initialized ADC_LINE(4)
Successfully initialized ADC_LINE(5)
ADC_LINE(0): 73 32 14 6 -1 -1
ADC_LINE(1): 515 526 530 530 -1 -1
ADC_LINE(2): 325 152 74 39 -1 -1
ADC_LINE(3): 476 518 524 531 -1 -1
ADC_LINE(4): 482 514 537 536 -1 -1
ADC_LINE(5): 4095 4095 4095 4095 -1 -1
Nucleo-L073RZ this PR:
main(): This is RIOT! (Version: 2025.01-devel-31-g5de25c-pr/stm32l0_adc_fix)
RIOT ADC peripheral driver test
This test will sample all available ADC lines once every 100ms with
a 10-bit resolution and print the sampled results to STDIO
Successfully initialized ADC_LINE(0)
Successfully initialized ADC_LINE(1)
Successfully initialized ADC_LINE(2)
Successfully initialized ADC_LINE(3)
Successfully initialized ADC_LINE(4)
Successfully initialized ADC_LINE(5)
ADC_LINE(0): 1 2 4 8 -1 -1
ADC_LINE(1): 7 32 133 522 -1 -1
ADC_LINE(2): 5 10 19 52 -1 -1
ADC_LINE(3): 7 32 129 528 -1 -1
ADC_LINE(4): 7 32 132 537 -1 -1
ADC_LINE(5): 63 255 1023 4095 -1 -1
Nucleo-L152RE Master:
main(): This is RIOT! (Version: 2025.01-devel-29-g76b9b)
RIOT ADC peripheral driver test
This test will sample all available ADC lines once every 100ms with
a 10-bit resolution and print the sampled results to STDIO
Successfully initialized ADC_LINE(0)
Successfully initialized ADC_LINE(1)
Successfully initialized ADC_LINE(2)
Successfully initialized ADC_LINE(3)
Successfully initialized ADC_LINE(4)
Successfully initialized ADC_LINE(5)
ADC_LINE(0): 24 27 29 30 -1 -1
ADC_LINE(1): 25 29 30 31 -1 -1
ADC_LINE(2): 21 24 26 28 -1 -1
ADC_LINE(3): 23 26 28 29 -1 -1
ADC_LINE(4): 24 28 30 31 -1 -1
ADC_LINE(5): 63 63 63 63 -1 -1
Nucleo-L152RE with the fixes applied:
main(): This is RIOT! (Version: 2025.01-devel-31-g5de25c-pr/stm32l0_adc_fix)
RIOT ADC peripheral driver test
This test will sample all available ADC lines once every 100ms with
a 10-bit resolution and print the sampled results to STDIO
Successfully initialized ADC_LINE(0)
Successfully initialized ADC_LINE(1)
Successfully initialized ADC_LINE(2)
Successfully initialized ADC_LINE(3)
Successfully initialized ADC_LINE(4)
Successfully initialized ADC_LINE(5)
ADC_LINE(0): 23 110 473 1967 -1 -1
ADC_LINE(1): 28 123 503 2039 -1 -1
ADC_LINE(2): 21 98 423 1813 -1 -1
ADC_LINE(3): 22 103 447 1879 -1 -1
ADC_LINE(4): 22 111 481 1999 -1 -1
ADC_LINE(5): 63 255 1023 4095 -1 -1
Issues/PRs references
This PR is a requirement for #20971, otherwise the changes in that PR can't be tested.
Murdock results
:heavy_check_mark: PASSED
17ee40dafa93e28ea1e05fd739a5e793980c3cb0 cpu/stm32l1: fix ADC initialization & resolution setting
| Success | Failures | Total | Runtime |
|---|---|---|---|
| 10249 | 0 | 10249 | 17m:35s |
Artifacts
@crasbe: if you provide test instruction it is nice if you tell what test you are taking about ( added that info in your message)
I usually just include the command-line e.g.:
<RIOT>/tests/periph/adc/ > BOARD=nucleo-l073rz make flash term
other people use the make -D <dir>
I just looked at the other code in the file and it does not use _Msk at all, only the code I introduced.
For consistency it would probably be best to not use the _Msk definitions at all then.
Likewise for the additions in my other PR #20971. Only the code introduced by me uses the _Msk definition.
I just looked at the other code in the file and it does not use
_Mskat all, only the code I introduced. For consistency it would probably be best to not use the_Mskdefinitions at all then.Likewise for the additions in my other PR #20971. Only the code introduced by me uses the
_Mskdefinition.
I think either is ok (I would just prefer that there is one), diverting from cmsis isn't a problem as long a the manufacturer keeps doing that (microchip just removed their extra (as in: I could not find them in the cmsis manual) bitfield structs)
to me the _Msk variant is a little more telling what it actually is, but the current file make little to no use of _Msk.
i might have missed something in cmsis that defines the non _Msk values but i did not see it
Is there anything left to do for me? I think @benpicco and @kfessel were generally approving of the PR?
Thanks for this! I'll give Ben and Karl a bit of time to react before hitting merge.
Thanks everyone for working with me on this. Even though it was a small change, I still learned a lot :)