pkg/cmsis: use unique package for CMSIS headers, DSP and NN modules #18056

aabadie · 2022-05-04T19:03:10Z

Contribution description

The cpu/cortexm_common directory contains the Core CMSIS headers in a vendor subdirectory. The CMSIS-DSP and CMSIS-NN packages are both fetching code from the same ARM-Software/CMSIS_5 repository.
This situation leads to:

not in sync code: I don't know what version of the Core headers is used in cortexm_common, in master CMSIS-DSP and CMSIS-NN don't use the same version of the code (5.4 for the former, 5.6 for the latter)
hard to maintain: a code update on one side (CMSIS-NN updated to 5.9.0 for example), might result in a failed build unless the vendor code is also updated.

This PR proposes to move all that in a single package and make the cortexm_common module to depend on it. cmsis-dsp and cmsis-nn becomes 2 extra modules of the cmsis package.

The last version, 5.9.0, is used by the cmsis package. Some adaptation were required in some i2c implementations that are defining a _start function. The CMSIS headers also contains a similar definition. Some changes were also required in the DSP code (mostly build system changes).

Also note that the cmsis package is only downloaded once (the very first time an application is built for a cortexm target) in build/pkg/cmsis, after that the same code is reused so there won't be any build overhead in the long run for a normal user.

Using a package for a such central component that is touching a very large part of the hardware supported by RIOT might have some impact on CI performance. Even if I find this problem secondary, it must be taken into account and that's why I marked this PR as RFC.

Testing procedure

Green Murdock
Test on various ARM boards should be performed

Issues/PRs references

None

krzysztof-cabaj · 2022-05-05T07:41:26Z

Hi! What kind of test or tests you think about? I have just clone your PR, compile blinky program and flash it to l031k6 (M0, nucleo-32), l073rz (M0, nucleo-64) and f428zi (M4, nucleo-144). Everything works perfect. I could tomorrow test it in such manner on l552ze-q, f334r8 and discovery stm32f469i-disco.

aabadie · 2022-05-05T07:48:52Z

What kind of test or tests you think about?

I think that running ./dist/tools/compile_and_test_for_board/compile_and_test_for_board.py . <board> --with-test-only --jobs=4 would be useful but will take some time (a couple of hours per board).

krzysztof-cabaj · 2022-05-05T08:01:53Z

Thanks for clarification. I try run mentioned tests tomorrow.

aabadie · 2022-05-06T19:29:36Z

This seems to almost pass the CI now but I don't understand the Rust failure: riot_wrappers.0bcd1c26-cgu.11:(.text.rust_begin_unwind+0xc): undefined reference to llvm_asm_is_not_supported_any_more'`. Any idea @chrysn ?

krzysztof-cabaj · 2022-05-09T12:24:54Z

Hi, during the weekend I run test on nucleo-l552ze-q. 218 test ended with success, 20 with failure - but in most cases due to lack of some python modules or additional programs. However, there are some strange timeouts. Below output from script summary - with my comments concerning failure reason.

ERROR:nucleo-l552ze-q:Tests failed: 20
Failures during compilation:
- [tests/pkg_emlearn](tests/pkg_emlearn/compilation.failed)
     "No module named 'emlearn'
- [tests/pkg_nanopb](tests/pkg_nanopb/compilation.failed)
    "protoc: Command not found"
- [tests/suit_manifest](tests/suit_manifest/compilation.failed)
    "No module named 'cbor2'

Failures during test:
- [examples/micropython](examples/micropython/test.failed)
    ests/congure_reno](tests/congure_reno/test.failed)
    "No module named 'riotctrl'
- [tests/congure_test](tests/congure_test/test.failed)
    "No module named 'riotctrl'
- [tests/cpp11_condition_variable](tests/cpp11_condition_variable/test.failed)
     pexpect.exceptions.TIMEOUT: Timeout exceeded
- [tests/gnrc_rpl](tests/gnrc_rpl/test.failed)
    "No module named 'riotctrl'
- [tests/malloc_thread_safety](tests/malloc_thread_safety/test.failed)
  "Timeout in expect script at "child.expect("TEST PASSED")"
- [tests/pbkdf2](tests/pbkdf2/test.failed)
   lack of socat program
- [tests/periph_timer_short_relative_set](tests/periph_timer_short_relative_set/test.failed)
    "too long delay, aborted after 1001 ... TEST FAILED"
- [tests/periph_wdt](tests/periph_wdt/test.failed)
    "Timeout in expect script" -> line 54
- [tests/pthread_rwlock](tests/pthread_rwlock/test.failed)
    "Timeout in expect script" -> line 13  + stack pointer corrupted
- [tests/riotboot](tests/riotboot/test.failed)
    "pexcpect.exceptions.TIMEOUT: Timeout exceeed."
- [tests/riotboot_hdr](tests/riotboot_hdr/test.failed)
    "pexcpect.exceptions.TIMEOUT: Timeout exceeed."
- [tests/sched_change_priority](tests/sched_change_priority/test.failed)
    lack of socat program
- [tests/shell](tests/shell/test.failed)
    lack of socat program
- [tests/sys_sema_inv](tests/sys_sema_inv/test.failed)
    "Timeout in expect script" -> line 12  + stack pointer corrupted
- [tests/turo](tests/turo/test.failed)
    "No module named 'riotctrl'
- [tests/unittests](tests/unittests/test.failed)
    "pexcpect.exceptions.TIMEOUT: Timeout exceeed."

If any output files are needed I could place them in forum.

I could run these tests at other boards - are you interested? Should I first install all remaining software?

aabadie · 2023-05-14T19:34:07Z

I rebased this PR and fixed Murdock but I still don't really know the reason of the ~30% build time increase and if that's acceptable. I do believe that this PR is very useful (less vendor code in the code base, consistent CMSIS versions used, etc) but it might have a significant CI build cost.

kaspar030 · 2023-05-15T06:51:35Z

Hm, so now every Cortex-M build depends on that package instead of the in-tree version. That at least trashes all of ccache.
Let's re-build this a couple of times.

aabadie · 2023-05-15T08:47:02Z

so now every Cortex-M build depends on that package instead of the in-tree version. That at least trashes all of ccache.
Let's re-build this a couple of times.

After a few runs the CI build times is reduced, so cache warmup indeed helps.

kaspar030 · 2023-05-15T09:12:53Z

After a few runs the CI build times is reduced, so cache warmup indeed helps.

Looks like. Phew, I would've shed a tear otherwise.

ACK, nice cleanup!

kaspar030 · 2023-05-15T09:13:08Z

bors merge

bors · 2023-05-15T09:13:11Z

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

aabadie · 2023-05-15T09:32:13Z

bors merge

18056: pkg/cmsis: use unique package for CMSIS headers, DSP and NN modules r=aabadie a=aabadie Co-authored-by: Alexandre Abadie <alexandre.abadie@inria.fr>

bors · 2023-05-15T09:35:13Z

Build failed:

Murdock

benpicco · 2023-05-15T11:43:53Z

bors merge

18056: pkg/cmsis: use unique package for CMSIS headers, DSP and NN modules r=benpicco a=aabadie 19571: cpu/stm32/periph_adc: fixes and improvements for L4 support r=benpicco a=gschorcht ### Contribution description This PR provides the following fixes and improvements for the `periph_adc` implementation for STM32L4. - Support STM32L496AG added. - Instead of defining the number of ADC devices for each MCU model, the number of ADC devices is determined from ADCx definitions in CMSIS header. - MCU specific register/value defines are valid for all L4 MCUs, model based conditional compilation is removed. - The ADC clock disable function is fixed using a counter. The counter is incremented in `prep` and decremented in `done`. The ADC clock is disabled if the counter becomes 0. - For boards that have not connected the V_REF+ pin to an external reference voltage, the VREFBUF peripheral can be used as V_REF+ (if supported) by setting `VREFBUF_ENABLE=1`. - The ASCR register is available and has to be set for all STM32L471xx, STM32L475xx, STM32L476xx, STM32L485xx and STM32L486xx MCUs. Instead of using the CPU model for conditional compilation, the CPU line is used to support all MCU of that lines. - Setting of SQR1 is fixed. Setting the SQR1 did only work before because the `ADC_SRQ_L` is set to 0 for a sequence length of 1. - Setting the `ADC_CCR_CKMODE` did only work for the reset state. It is now cleared before it is set. Instead of using the `ADC_CCR_CKMODE_x` bits to set the mode, the mode defines are used. - Support for V_REFINT as ADC channel added. ### Testing procedure 19589: gnrc/gnrc_netif_hdr_print: printout timestamp if enabled r=benpicco a=chudov Co-authored-by: Alexandre Abadie <alexandre.abadie@inria.fr> Co-authored-by: Gunar Schorcht <gunar@schorcht.net> Co-authored-by: chudov <chudov@gmail.com>

bors · 2023-05-15T12:09:16Z

Build failed (retrying...):

Murdock

aabadie · 2023-05-15T12:34:13Z

bors merge

bors · 2023-05-15T12:34:16Z

Already running a review

bors · 2023-05-15T15:35:02Z

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

aabadie requested review from leandrolanzieri, jia200x, MrKevinWeiss, smlng, MichelRottleuthner, fjmolinas, DipSwitch, vincent-d, benpicco, dylad, keestux, kaspar030 and bergzand as code owners May 4, 2022 19:03

github-actions bot added Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Area: Kconfig Area: Kconfig integration Area: pkg Area: External package ports Area: tests Area: tests and testing framework Platform: ARM Platform: This PR/issue effects ARM-based platforms labels May 4, 2022

aabadie added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels May 4, 2022

aabadie force-pushed the pr/pkg/cmsis branch from 5074548 to 80d4927 Compare May 4, 2022 19:46

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 5, 2022

aabadie requested a review from basilfx as a code owner May 6, 2022 10:23

aabadie force-pushed the pr/pkg/cmsis branch from 90b4d0f to c35546d Compare May 6, 2022 16:50

aabadie added 8 commits May 14, 2023 21:08

cpu/cortexm_common: remove CMSIS vendor code

9b32854

pkg/cmsis-dsp: drop package

8f030ca

pkg/cmsis-nn: drop package

15220a5

tests/pkg_cmsis-*: extend whitelisted boards

c482090

cpu/efm32: add CMSIS DSP include for arm_math.h

2467ff0

tests/driver_lpd8808: tests/driver_apa102: rename STEP constant

c034f6e

tests/float: rename STEP constant

1bd1963

cpu/efm32: fix dependency to cmsis-dsp module

625f1f5

aabadie force-pushed the pr/pkg/cmsis branch from 1b0ff3b to 625f1f5 Compare May 14, 2023 19:19

aabadie changed the title ~~[RFC] pkg/cmsis: use unique package for CMSIS headers, DSP and NN modules~~ pkg/cmsis: use unique package for CMSIS headers, DSP and NN modules May 14, 2023

bors bot added a commit that referenced this pull request May 15, 2023

Merge #18056

f6e2236

18056: pkg/cmsis: use unique package for CMSIS headers, DSP and NN modules r=aabadie a=aabadie Co-authored-by: Alexandre Abadie <alexandre.abadie@inria.fr>

pkg/gecko_sdk: disable cast-align globally

eac9cdf

aabadie force-pushed the pr/pkg/cmsis branch from 705c07d to eac9cdf Compare May 15, 2023 10:20

bors bot merged commit 2b97b76 into RIOT-OS:master May 15, 2023
25 checks passed

aabadie deleted the pr/pkg/cmsis branch May 15, 2023 17:19

benpicco added this to the Release 2023.07 milestone Aug 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pkg/cmsis: use unique package for CMSIS headers, DSP and NN modules #18056

pkg/cmsis: use unique package for CMSIS headers, DSP and NN modules #18056

aabadie commented May 4, 2022 •

edited

krzysztof-cabaj commented May 5, 2022

aabadie commented May 5, 2022

krzysztof-cabaj commented May 5, 2022

aabadie commented May 6, 2022

krzysztof-cabaj commented May 9, 2022

aabadie commented May 14, 2023

kaspar030 commented May 15, 2023

aabadie commented May 15, 2023

kaspar030 commented May 15, 2023

kaspar030 commented May 15, 2023

bors bot commented May 15, 2023

aabadie commented May 15, 2023

bors bot commented May 15, 2023

benpicco commented May 15, 2023

bors bot commented May 15, 2023

aabadie commented May 15, 2023

bors bot commented May 15, 2023

bors bot commented May 15, 2023

pkg/cmsis: use unique package for CMSIS headers, DSP and NN modules #18056

pkg/cmsis: use unique package for CMSIS headers, DSP and NN modules #18056

Conversation

aabadie commented May 4, 2022 • edited

Contribution description

Testing procedure

Issues/PRs references

krzysztof-cabaj commented May 5, 2022

aabadie commented May 5, 2022

krzysztof-cabaj commented May 5, 2022

aabadie commented May 6, 2022

krzysztof-cabaj commented May 9, 2022

aabadie commented May 14, 2023

kaspar030 commented May 15, 2023

aabadie commented May 15, 2023

kaspar030 commented May 15, 2023

kaspar030 commented May 15, 2023

bors bot commented May 15, 2023

aabadie commented May 15, 2023

bors bot commented May 15, 2023

benpicco commented May 15, 2023

bors bot commented May 15, 2023

aabadie commented May 15, 2023

bors bot commented May 15, 2023

bors bot commented May 15, 2023

aabadie commented May 4, 2022 •

edited