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.

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

cpu/stm32: Restructure stm32 clk kconfig model #18771

Closed
wants to merge 2 commits into from

Conversation

MrKevinWeiss
Copy link
Contributor

Contribution description

Currently nightlies are failing due to kconfig hash mismatches, ultimately due to clock tree configurations.
It is pretty hard to understand what is happening so this restructuring should be a first step to fixing the current issues (essentially not enough information is given to kconfig such as the HSE crystal speed, to make as good a choice for the clock settings are make).

This PR splits the Kconfig clock configurations for the stm32s from one file for all series, into many each series and any common combinations of series. This makes it more readable and matches what is done in make a bit closer.

As only the series that is applicable gets sourced, it also removes a lot of noise created by other series configurations being included.

Testing procedure

Murdock with nightlies enabled should have the same errors and passes as the latest nightlies.

Locally, all stms can be tested for an app with the following few lines:

Get a variable to hold all stm32 based boards

export TEST_BOARDS=$(FEATURES_REQUIRED=cpu_stm32 make -C tests/shell info-boards-supported --no-print-directory)

Overwrite the nightly flag to run all kconfig tests and binary comparisons with make

export NIGHTLY=1

Run through each stm based board for a single test, doing the kconfig comparison

for board in $TEST_BOARDS; do echo $board && /bin/bash -c "source .murdock; JOBS=4 compile tests/shell ${board}:gnu" | grep mismatch; done;

it should take maybe 10 to 20 mins, still faster than waiting for the CI.

Issues/PRs references

@github-actions github-actions bot added Area: CI Area: Continuous Integration of RIOT components Area: cpu Area: CPU/MCU ports Area: Kconfig Area: Kconfig integration Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Oct 19, 2022
@MrKevinWeiss
Copy link
Contributor Author

This is not meant to provide any fixes, we should have pretty much the same result as before. Only a restructuring, a follow-up PR will be provided that actually fixes the broken nightlies, but it is much easier to accomplish this (and test) with this one first.

@MrKevinWeiss
Copy link
Contributor Author

Maybe ping @aabadie if he has an opinion on this (at least the idea as he was the one who did most of the work with the STM clock tree).

@MrKevinWeiss MrKevinWeiss added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 20, 2022
@riot-ci
Copy link

riot-ci commented Oct 20, 2022

Murdock results

✔️ PASSED

b08c71d REMOVEME: Pretend it is nightly to do full tests

Success Failures Total Runtime
1994 0 1994 08m:31s

Artifacts

This only reflects a subset of all builds from https://ci-prod.riot-os.org. Please refer to https://ci.riot-os.org for a complete build for now.

@MrKevinWeiss
Copy link
Contributor Author

Hmm it seems that some CLOCK_PLL_M and CLOCK_PLL_N are hidden in some board files... I should do a search for that...

@MrKevinWeiss
Copy link
Contributor Author

Also it looks like STM32_U5 is not modelled?

@MrKevinWeiss
Copy link
Contributor Author

menuconfig looks a bit ugly, maybe this needs to all be hidden in a menu...

@MrKevinWeiss
Copy link
Contributor Author

I wonder if breaking this up may create issues with potential speed-up methods in the future? If we just need one file to load rather then several depending on the CPU_FAM?

Any opinions @leandrolanzieri

This PR splits the Kconfig clock configurations for the stm32s
from one file for all series,
into many each series and any common combinations of series.
This makes it more readable and matches what is done in make a bit
closer.

As only the series that is applicable gets sourced, it also removes
a lot of noise created by other series configurations being included.
@MrKevinWeiss MrKevinWeiss removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 21, 2022
@MrKevinWeiss
Copy link
Contributor Author

probably will just be superseded by #18797 instead of dealing with rebase pain...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: CI Area: Continuous Integration of RIOT components Area: cpu Area: CPU/MCU ports Area: Kconfig Area: Kconfig integration Platform: ARM Platform: This PR/issue effects ARM-based platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants