Skip to content

boards/nucleo-f722ze: add ADC support#21233

Merged
mguetschow merged 2 commits intoRIOT-OS:masterfrom
krzysztof-cabaj:nucleo-f722ze-ADC
Feb 21, 2025
Merged

boards/nucleo-f722ze: add ADC support#21233
mguetschow merged 2 commits intoRIOT-OS:masterfrom
krzysztof-cabaj:nucleo-f722ze-ADC

Conversation

@krzysztof-cabaj
Copy link
Copy Markdown
Contributor

@krzysztof-cabaj krzysztof-cabaj commented Feb 20, 2025

Contribution description

This PR adds ADC support for nucleo-f722ze.

Testing procedure

Flash the board using tests/periph/adc program. Check if measured values changes when A0-A5 pins are
connected to the 3,3V, GND or to the potentiometer.

Unfortunately I don't have access to this board and cannot check if everything is working.
@crasbe - you mentioned in the PR #20971 that you have this board but it lacks configuration. Could you test it?

Issues/PRs references

PR #20971

@github-actions github-actions bot added the Area: boards Area: Board ports label Feb 20, 2025
crasbe
crasbe reviewed Feb 20, 2025
View reviewed changes
@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Feb 20, 2025

Wow, thank you for implementing this so quickly!
It does compile, but the assertion in the initialization fails:

main(): This is RIOT! (Version: 2025.04-devel-134-gd7ca65-nucleo-f722ze-ADC)

RIOT ADC peripheral driver test

This test will sample all available ADC lines once every 100ms with
6 to 16-bit resolution and print the sampled results to STDOUT.
Not all MCUs support all resolutions, unsupported resolutions
are printed as -1.

0x80010b5 => FAILED ASSERTION.

I assume()ed it was this line here, because it's the only assume().

RIOT/cpu/stm32/periph/adc_f4_f7.c

Line 105 in f67055c

assume((periph_apb_clk(APB2) / clk_div) <= ADC_CLK_MAX);

It's necessary to add a higher ADC_CLK_MAX to the boards/nucleo-f722ze/include/periph_conf.h file, similar to what the F767 does:

RIOT/boards/nucleo-f767zi/include/periph_conf.h

Lines 210 to 212 in f67055c

#define VBAT_ADC ADC_LINE(6) /**< VBAT ADC line */
#define ADC_CLK_MAX MHZ(36) /**< Use a faster than default ADC clock */
#define ADC_NUMOF ARRAY_SIZE(adc_config)

The reason for that is that periph_apb_clk(APB2) is 108MHz, which divided by 8 is still more than 12MHz which the assertion checked.

With that change it works as it should:

main(): This is RIOT! (Version: 2025.04-devel-134-gd7ca65-nucleo-f722ze-ADC)

RIOT ADC peripheral driver test

This test will sample all available ADC lines once every 100ms with
6 to 16-bit resolution and print the sampled results to STDOUT.
Not all MCUs support all resolutions, unsupported resolutions
are printed as -1.

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)
Successfully initialized ADC_LINE(6)
ADC_LINE(0): 63 255 1023 4095    -1    -1
ADC_LINE(1): 26  96  358 1356    -1    -1
ADC_LINE(2): 11  54  240 1018    -1    -1
ADC_LINE(3): 12  52  225  943    -1    -1
ADC_LINE(4):  8  40  183  825    -1    -1
ADC_LINE(5):  3  27  164  762    -1    -1
ADC_LINE(6):  0   1    0    0    -1    -1

ADC_LINE(0): 63 255 1023 4094    -1    -1
ADC_LINE(1): 29 110  415 1585    -1    -1
ADC_LINE(2): 15  64  263 1077    -1    -1
ADC_LINE(3): 13  58  243 1013    -1    -1
ADC_LINE(4): 12  52  225  942    -1    -1
ADC_LINE(5):  3  28  167  764    -1    -1
ADC_LINE(6):  0   0    0    0    -1    -1

ADC_LINE(0): 63 255 1023 4094    -1    -1
ADC_LINE(1): 31 114  424 1574    -1    -1
ADC_LINE(2): 15  64  263 1079    -1    -1
ADC_LINE(3): 14  60  250 1038    -1    -1
ADC_LINE(4): 13  57  236  982    -1    -1
ADC_LINE(5):  5  29  170  784    -1    -1
ADC_LINE(6):  0   0    0    0    -1    -1

The ADC line for VBAT is pegged to zero, which I'm not sure why that is. But the NUCLEO-F722ZE does not have a battery holder. I'll check the schematic to see what's going on.

@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Feb 20, 2025

Oh, nevermind, the GPIO for the VBAT pin is GPIO_UNDEF, just like for the nucleo-f767. That would explain why it is at 0 🤣

So from my side, this is good to merge with the one change. But I'm not a maintainer.

@krzysztof-cabaj
Copy link
Copy Markdown
Contributor Author

krzysztof-cabaj commented Feb 20, 2025

Thanks for testing this PR, finding bug ... and giving solution :D
I hope it helps with PR #20971

@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Feb 20, 2025

Wait... Vbat should work and show something high, because it is connected to +3.3V.

I'll investigate :)

@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Feb 20, 2025

This is funny again. So everything works as it should and the other Nucleos behave odd.

To enable the V_BAT measurement, one is supposed to add USEMODULE += periph_vbat to the Makefile. This is not the case for tests/periph/adc, therefore the ADC->CCR register does not have the VBATEN bit set (which the initialization would usually do). For some reason, on other STM32s, you can still measure something on the VBAT Channel (idk if it's actually VBAT) without having the VBATEN bit set (I confirmed that with the STM32F302).

With USEMODULE+=periph_vbat BOARD=nucleo-f722ze make it works as it should:

main(): This is RIOT! (Version: 2025.04-devel-134-gd7ca65-nucleo-f722ze-ADC)

RIOT ADC peripheral driver test

This test will sample all available ADC lines once every 100ms with
6 to 16-bit resolution and print the sampled results to STDOUT.
Not all MCUs support all resolutions, unsupported resolutions
are printed as -1.

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)
Successfully initialized ADC_LINE(6)
ADC_LINE(0): 63 255 1023 4095    -1    -1
ADC_LINE(1): 28 106  393 1486    -1    -1
ADC_LINE(2): 13  60  255 1084    -1    -1
ADC_LINE(3): 14  59  246 1024    -1    -1
ADC_LINE(4): 12  53  230  961    -1    -1
ADC_LINE(5):  3  28  151  785    -1    -1
ADC_LINE(6): 15  63  254 1019    -1    -1

ADC_LINE(0): 63 255 1023 4095    -1    -1
ADC_LINE(1): 30 111  420 1556    -1    -1
ADC_LINE(2): 15  64  263 1080    -1    -1
ADC_LINE(3): 14  59  247 1032    -1    -1
ADC_LINE(4): 13  57  238  990    -1    -1
ADC_LINE(5):  4  28  150  728    -1    -1
ADC_LINE(6): 15  63  254 1019    -1    -1

So yes, you did nothing wrong, it works as intended.

mguetschow
mguetschow approved these changes Feb 21, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look straightforward enough and I've double-checked the pins with the pinout on https://doc.riot-os.org/group__boards__nucleo-f722ze.html.

Haven't tested this locally, but trusting @crasbe's reporting.

Thanks everyone!

@mguetschow mguetschow added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 21, 2025
@mguetschow mguetschow enabled auto-merge February 21, 2025 08:53
@riot-ci
Copy link
Copy Markdown

riot-ci commented Feb 21, 2025

Murdock results

✔️ PASSED

83a0c14 boards/nucleo-f722ze: add full ADC config

Success Failures Total Runtime
528 0 528 05m:07s

Artifacts

@mguetschow mguetschow added this pull request to the merge queue Feb 21, 2025
Merged via the queue into RIOT-OS:master with commit 06eb1cd Feb 21, 2025
28 checks passed
@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Feb 21, 2025

Thank you @krzysztof-cabaj for doing this PR and thank you @mguetschow for checking and approving this so quickly!

@krzysztof-cabaj if you want to, you can do the same for boards/nucleo-wl55jc 😇

There are more Nucleo boards that are missing the ADC definitions, but I already bought 150€ worth of Nucleo boards this month for RIOT development, so that has to wait a little bit longer 😅

@krzysztof-cabaj
Copy link
Copy Markdown
Contributor Author

krzysztof-cabaj commented Feb 21, 2025 •
edited
Loading

Thank you @crasbe for testing and @mguetschow for servicing this PR ...

... I started a while ago work on nucleo-wl55jc ADC - I hope I should make PR in hour or two :D

@mguetschow mguetschow added this to the Release 2025.04 milestone Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crasbe crasbe crasbe left review comments

@mguetschow mguetschow mguetschow approved these changes

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

Assignees

No one assigned

Labels

Area: boards Area: Board ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR

Projects

None yet

Milestone

Release 2025.04

Development

Successfully merging this pull request may close these issues.

4 participants

@krzysztof-cabaj @crasbe @riot-ci @mguetschow