Skip to content

sys/psa_crypto: usa auto_init module for initialization #21322

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
Merged
merged 5 commits into from
Apr 7, 2025
Merged

sys/psa_crypto: usa auto_init module for initialization #21322

merged 5 commits into from
Apr 7, 2025

Conversation

mguetschow
Copy link
Contributor

Contribution description

Most modules in RIOT are initialized automatically on-start (unless DISABLE_MODULE += auto_init is present). Do the same for psa_crypto.

RFC: Would it be better practice to have a pseudomodule auto_init_psa_crypto? I'd argue that the initialization function is pretty small and that I can't think of a reason why one wouldn't want to initialize PSA Crypto from the start up. Maybe @Einhornhool has some opinion on this?

Testing procedure

CI should show that the tests still run through.

@mguetschow mguetschow requested review from Einhornhool and removed request for aabadie, leandrolanzieri and MichelRottleuthner March 25, 2025 18:08
@github-actions github-actions bot added Area: tests Area: tests and testing framework Area: sys Area: System Area: examples Area: Example Applications labels Mar 25, 2025
@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 Mar 25, 2025
@mguetschow mguetschow mentioned this pull request Mar 25, 2025
Merged
@riot-ci
Copy link

riot-ci commented Mar 25, 2025 •
edited
Loading

Murdock results

✔️ PASSED

ad80fe9 sys/psa_crypto: document default auto-initialization

Success Failures Total Runtime
10279 0 10280 09m:14s

Artifacts

Einhornhool
Einhornhool reviewed Apr 2, 2025
View reviewed changes
Copy link
Contributor

@Einhornhool Einhornhool left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This makes total sense, I did not do this on purpose back then.
You should remove it from the docs, though:

RIOT/sys/psa_crypto/doc.txt

Line 37 in b1932dd

* Call `psa_crypto_init()` before calling any other operation.

@github-actions github-actions bot added the Area: doc Area: Documentation label Apr 2, 2025
@mguetschow
Copy link
Contributor Author

You should remove it from the docs, though

Very good point, thanks. Done. And while at it, I've also moved the documentation to doc.md. Let's see whether everything still renders as expected in the CI.

Einhornhool
Einhornhool approved these changes Apr 2, 2025
View reviewed changes
Copy link
Contributor

@Einhornhool Einhornhool left a comment

Choose a reason for hiding this comment

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

LGTM :)

@mguetschow mguetschow force-pushed the psa-crypto-auto-init branch from 26373bb to ad80fe9 Compare April 2, 2025 16:25
@mguetschow mguetschow requested a review from jia200x as a code owner April 2, 2025 16:25
mcr
mcr reviewed Apr 2, 2025
View reviewed changes
Copy link
Contributor

@mcr mcr left a comment

Choose a reason for hiding this comment

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

The doc.txt -> doc.md rename would have been better as a different PR.
Unclear if there are any changes, or just a rename.

@mguetschow
Copy link
Contributor Author

The doc.txt -> doc.md rename would have been better as a different PR.

At least it's a separate commit: 5e6a539

Unclear if there are any changes, or just a rename.

I've changed @code/@endcode to the Github markdown compatible ```, but didn't do any content-related changes in that commit.

The content change can be checked in ad80fe9

@Teufelchen1 Teufelchen1 added this pull request to the merge queue Apr 7, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 7, 2025
@mguetschow mguetschow added this pull request to the merge queue Apr 7, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 7, 2025
@crasbe crasbe added this pull request to the merge queue Apr 7, 2025
Merged via the queue into RIOT-OS:master with commit ff6bab9 Apr 7, 2025
27 checks passed
@mguetschow mguetschow deleted the psa-crypto-auto-init branch April 8, 2025 08:10
@mguetschow
Copy link
Contributor Author

Thanks everyone!

@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

@mcr mcr mcr left review comments

@Teufelchen1 Teufelchen1 Teufelchen1 approved these changes

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

+1 more reviewer

@Einhornhool Einhornhool Einhornhool approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Area: doc Area: Documentation Area: examples Area: Example Applications Area: sys Area: System Area: tests Area: tests and testing framework 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.
6 participants
@mguetschow @riot-ci @mcr @Teufelchen1 @Einhornhool @crasbe