Repository navigation
Add ECDSA and SHA-384 image-verify timings to the TI C2000 example - #650
Merged
Merged
Conversation
aidangarske
reviewed
Oct 26, 2026
aidangarske
left a comment
aidangarske
left a comment
Member
There was a problem hiding this comment.
Skoll Code Review
Scan type: review
Overall recommendation: COMMENT
Findings: 5 total — 5 posted, 0 skipped
5 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [Medium] P-384 lacks an independent correctness test —
embedded/ti-c2000-f28p55x/Source/wolf_main.c:879-881 - [Medium] Failed iterations still produce successful-operation timing figures —
embedded/ti-c2000-f28p55x/Source/wolf_main.c:693-701,746-770,802-810,849-859 - [Medium] The flash-image benchmark repeatedly hashes only a 4 KiB allocation —
embedded/ti-c2000-f28p55x/Source/wolf_main.c:672-677 - [Medium] The documented ECCFAST option omits its required wolfSSL dependency —
embedded/ti-c2000-f28p55x/README.md:94 - [Low] ECC_USER_CURVES comment still says P-256 only —
embedded/ti-c2000-f28p55x/Header/user_settings.h:238
Review generated by Skoll
dgarske
force-pushed
the
c28x_ecdsa_bench
branch
from
October 26, 2026 23:33
917c3a6 to
0cd56ca
Compare
mattia-moffa
approved these changes
Oct 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
ECC384=1(SECP384R1),ECCFAST=1(dropsWOLFSSL_SP_SMALLfor the windowed SP path) andECCBENCH=1, which swaps the test and benchmark harness for ECDSA sign/verify, ECDH and SHA-256/SHA-384 image-hash timings over a flash-resident 128 KB array. The stockbenchmarkreports nothing for ECC here, because the example builds withNO_ASNandbench_ecc()compiles its ECDSA section out behind that macro. The README gains the resulting table.ECCFAST=1needs the companion SP recode fix in wolfSSL/wolfssl#11889. Validated on a LAUNCHXL-F28P55X.