-
Notifications
You must be signed in to change notification settings - Fork 9
Adjust the driver to the new PSA API #7
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
Adjust the driver to the new PSA API #7
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found one change that may potentially cause problems in the -Werror
builds, and I have left a really small cosmetic remark.
Also, it would probably be easier to review the PR if the formatting changes were put in a separate commit.
Could we please rebase this to apply fixups and remove the merge commits? |
Mostly API changes, with an addition of a check-slot function. Changed the lifetime of the driver to fit the 0x00 - 0xFF range.
7cc48d3
to
b59115c
Compare
@Patater done. |
439758b
to
5b500ae
Compare
Cryptoauthlib will perform the checks instead and return ATCA_BAD_PARAM
The PSA core should never pass NULL to this function
@Patater, @k-stachowiak - I believe I addressed all of your comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing the review comments. LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What testing was done with the top commit in this branch (6febe1f)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's only one blocker, which is the TODO comment. I know it's only a comment, but this isn't just test code, it's example code, so this is important.
Also, again because this is example code, we should implement that TODO ASAP.
return PSA_ERROR_INVALID_ARGUMENT; | ||
} | ||
|
||
if (type != PSA_KEY_TYPE_ECC_PUBLIC_KEY(PSA_ECC_CURVE_SECP256R1)) | ||
{ | ||
if (type != PSA_KEY_TYPE_ECC_PUBLIC_KEY(PSA_ECC_CURVE_SECP256R1)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cosmetic: it would make more sense to check the type first, then check that the data length is correct for the sole supported type.
a885a15
6febe1f
to
a885a15
Compare
@gilles-peskine-arm Done. Please re-review. |
8778214
to
0856b76
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Make the example interactive
Mostly API changes, with an addition of a check-slot function.
Changed the lifetime of the driver to fit the 0x00 - 0xFF range.
This PR is based on:
This PR is a base for ARMmbed/mbed-os-example-atecc608a#10.