Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cpu/stm32l0,l1: Fix ADC initialization order #21011

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

cpu/stm32l0,l1: Fix ADC initialization order #21011

wants to merge 2 commits into from
+15 −7

Conversation

crasbe
Copy link
Contributor

@crasbe crasbe commented Nov 19, 2024 •
edited
Loading

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 to print all resolution values at once, as described in #20780 (comment).

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.

@github-actions github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports labels Nov 19, 2024
@benpicco benpicco added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@vincent-d vincent-d Awaiting requested review from vincent-d vincent-d is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Area: cpu Area: CPU/MCU ports Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@crasbe @benpicco