Skip to content

sys/psa_crypto: usa auto_init module for initialization#21322

Merged
crasbe merged 5 commits intoRIOT-OS:masterfrom
mguetschow:psa-crypto-auto-init
Apr 7, 2025
Merged

sys/psa_crypto: usa auto_init module for initialization#21322
crasbe merged 5 commits intoRIOT-OS:masterfrom
mguetschow:psa-crypto-auto-init

Conversation

@mguetschow
Copy link
Copy Markdown
Contributor

@mguetschow mguetschow commented Mar 25, 2025

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 MichelRottleuthner, aabadie and leandrolanzieri 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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Contributor Author

mguetschow commented Apr 2, 2025

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

mguetschow commented Apr 3, 2025

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
Copy Markdown
Contributor Author

mguetschow commented Apr 8, 2025

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