Skip to content

boards/nucleo-wl55jc: add ADC configuration#21235

Merged
maribu merged 3 commits intoRIOT-OS:masterfrom
krzysztof-cabaj:nucleo-wl55jc-ADC
Mar 10, 2025
Merged

boards/nucleo-wl55jc: add ADC configuration#21235
maribu merged 3 commits intoRIOT-OS:masterfrom
krzysztof-cabaj:nucleo-wl55jc-ADC

Conversation

@krzysztof-cabaj
Copy link
Copy Markdown
Contributor

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

Contribution description

This PR adds ADC support for nucleo-wl55jc.

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.

@crasbe - could you be so kind and test ADC config once again ;)

Issues/PRs references

PR #20971

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

crasbe commented Feb 21, 2025

I'll test the code once I'm home in the evening. Thank you for providing the PR :)

@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
@riot-ci
Copy link
Copy Markdown

riot-ci commented Feb 21, 2025 •
edited
Loading

Murdock results

✔️ PASSED

0434087 boards/nucleo-wl55jc: add Doxygen doc

Success Failures Total Runtime
555 0 555 02m:51s

Artifacts

@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Feb 21, 2025

There is good news and there is bad news :D

Good news: Generally your PR works.

main(): This is RIOT! (Version: 2025.04-devel-142-g04340-nucleo-wl55jc-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): 4095 4095 4095 4094    -1    -1
ADC_LINE(1): 1675 1572 1554 1550    -1    -1
ADC_LINE(2): 1335 1503 1542 1561    -1    -1
ADC_LINE(3): 2086 1902 1750 1703    -1    -1
ADC_LINE(4): 1483 1533 1546 1558    -1    -1
ADC_LINE(5): 1473 1526 1538 1550    -1    -1
ADC_LINE(6): 440 156   39    0    -1    -1

ADC_LINE(0): 4095 4095 4095 4094    -1    -1
ADC_LINE(1): 1834 1609 1560 1551    -1    -1
ADC_LINE(2): 1457 1535 1551 1563    -1    -1
ADC_LINE(3): 1663 1674 1679 1680    -1    -1
ADC_LINE(4): 1543 1557 1560 1563    -1    -1
ADC_LINE(5): 1523 1539 1551 1553    -1    -1
ADC_LINE(6): 423 143   34    0    -1    -1

Bad news 1: Apparently the adc_wl.c driver is not able to switch between different resolutions either.

Bad news 2: Enabling periph_vbat leads to a kernel panic:

0x8001741 => *** RIOT kernel panic:
FAILED ASSERTION.

*** halted.

Maybe I have some time to investigate this later today. However I am 99% sure that this is not caused by your PR.

@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Feb 21, 2025

Bad News 1 is related to this Errata:
image

https://www.st.com/resource/en/errata_sheet/es0500-stm32wl55xx-stm32wl54xx-device-errata-stmicroelectronics.pdf p.13

I'll adapt the initialization sequence as with the L0, L1 and F0, G0 and C0. However I still don't know where the assertion comes from.

@krzysztof-cabaj
Copy link
Copy Markdown
Contributor Author

krzysztof-cabaj commented Feb 21, 2025

@crasbe thanks for your tests.

I add module periph_vbat and I try to find wher assert is using arm-none-eabi-addr2line. But it seams that my environment produce other executable, because this command shows that assert is in /sys/ztimet/core.c.

$ arm-none-eabi-addr2line -a 0x8001741 -e ./bin/nucleo-wl55jc/tests_adc.elf 
0x08001741
/data/RIOT/RIOT/sys/ztimer/core.c:268

Maybe in your environment this gives reasonable results and can help you.

@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Feb 21, 2025

It points to this in my case:

~/riot-dev/RIOT/tests/periph/adc$ arm-none-eabi-addr2line -a 0x8001741 -e ./bin/nucleo-wl55jc/tests_adc.elf
0x08001741
/home/cbuec/riot-dev/RIOT/build/pkg/cmsis/CMSIS/Core/Include/cmsis_gcc.h:1208

The _ASM line is line 1208.

/**
  \brief   Get Priority Mask
  \details Returns the current state of the priority mask bit from the Priority Mask Register.
  \return               Priority Mask value
 */
__STATIC_FORCEINLINE uint32_t __get_PRIMASK(void)
{
  uint32_t result;

  __ASM volatile ("MRS %0, primask" : "=r" (result) );
  return(result);
}

This is so odd, because it doesn't even get to main() before it crashes. At least nothing from the initial lines in main is printed.

@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Feb 21, 2025

Okay I found the issue. With periph_vbat, this function calls adc_init():

RIOT/cpu/stm32/cpu_init.c

Lines 418 to 425 in 0cb550a

__attribute__((unused))
static inline bool _backup_battery_connected(void) {
#if IS_USED(MODULE_PERIPH_VBAT)
vbat_init(); /* early use of VBAT requires init() */
return !vbat_is_empty();
#endif
return false;
}

This function is called from here:

RIOT/cpu/stm32/cpu_init.c

Lines 427 to 435 in 0cb550a

bool cpu_woke_from_backup(void) {
#if IS_ACTIVE(CPU_HAS_BACKUP_RAM)
static const char _signature[] BACKUP_RAM_DATA = BACKUP_RAM_MAGIC;
if (_backup_battery_connected()) {
/* in case the board has a backup battery the regulator must be on
to mitigate (unexpected) outage of VDD, so RTC register and
backup domain register contents are not lost */
pm_backup_regulator_on();
}

And this function is called from here, which is the reset_handler_default:

RIOT/cpu/cortexm_common/vectors_cortexm.c

Lines 144 to 156 in 0cb550a

#ifdef CPU_HAS_BACKUP_RAM
#if BACKUP_RAM_HAS_INIT
backup_ram_init();
#endif
if (!cpu_woke_from_backup() ||
CPU_BACKUP_RAM_NOT_RETAINED) {
/* load low-power data section. */
for (dst = _sbackup_data, src = _sbackup_data_load;
dst < _ebackup_data;
dst++, src++) {
*dst = *src;
}

Now... why does adc_init() crash? Because it uses ztimer and I assume that ztimer is not safe to use this early in the startup sequence.

RIOT/cpu/stm32/periph/adc_wl.c

Lines 86 to 92 in 0cb550a

#if IS_USED(MODULE_ZTIMER_USEC)
ztimer_sleep(ZTIMER_USEC, ADC_T_ADCVREG_STUP_US);
#else
/* to avoid using ZTIMER_USEC unless already included round up the
internal voltage regulator start up to 1ms */
ztimer_sleep(ZTIMER_MSEC, 1);
#endif

I will create a PR that replaces ztimer in the periph_wl.c file with busy_wait. The exact time that the program waits here is absolutely uncritical anyways, so this is absolutely fine.

@crasbe crasbe mentioned this pull request Feb 21, 2025
Merged
@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Mar 4, 2025

I tested this extensively during the development of #21238 and IMO this is ready to be merged.

@maribu maribu added this pull request to the merge queue Mar 10, 2025
Merged via the queue into RIOT-OS:master with commit d398a41 Mar 10, 2025
26 checks passed
@krzysztof-cabaj
Copy link
Copy Markdown
Contributor Author

krzysztof-cabaj commented Mar 10, 2025

@crasbe, @maribu thanks for help with this PR.

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

Reviewers

@crasbe crasbe crasbe 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.

5 participants

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