Skip to content

[tick, timeout] Add support for a 64 bit tick source - #53

Merged
aidangarske merged 1 commit into
wolfSSL:mainfrom
AlexLanzano:64bit-tick
Aug 3, 2026
Merged

[tick, timeout] Add support for a 64 bit tick source#53
aidangarske merged 1 commit into
wolfSSL:mainfrom
AlexLanzano:64bit-tick

Conversation

@AlexLanzano

@AlexLanzano AlexLanzano commented Aug 2, 2026

Copy link
Copy Markdown
Member

This PR adds optional support for a 64 bit tick source by setting the config macro WHAL_CFG_64BIT_TICK.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds an opt-in 64-bit tick mode for the whal_Timeout abstraction so boards can use wider tick counters while preserving the existing 32-bit default behavior.

Changes:

  • Introduces WHAL_CFG_64BIT_TICK to switch whal_Timeout’s tick fields and GetTick callback between uint32_t and uint64_t.
  • Adds WHAL_TICK_MAX and updates WHAL_TIMEOUT_EXPIRED() to use the configured tick width.
  • Updates the timeout unit test to work in both 32-bit and 64-bit tick configurations (including wraparound behavior).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
wolfHAL/timeout.h Adds a build-time option for 64-bit tick sources and adjusts timeout calculations accordingly.
tests/core/test_timeout.c Adapts fake tick source + wrap test to use the configured tick width (WHAL_TICK_MAX).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread wolfHAL/timeout.h

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (2)

wolfHAL/timeout.h:89

  • The casts in WHAL_TIMEOUT_EXPIRED are currently applied to the result of the comparison (0/1) rather than to the tick subtraction. This makes the cast ineffective/misleading and defeats the intent of explicitly evaluating the elapsed tick delta at the configured width.
#ifdef WHAL_CFG_64BIT_TICK
#define WHAL_TIMEOUT_EXPIRED(t) \
    ((t) && ((uint64_t)((t)->GetTick() - (t)->startTick) >= (t)->timeoutTicks))
#else
#define WHAL_TIMEOUT_EXPIRED(t) \
    ((t) && ((uint32_t)((t)->GetTick() - (t)->startTick) >= (t)->timeoutTicks))
#endif

docs/writing_a_driver.md:342

  • The 64-bit tick guidance should also mention atomicity: on 32-bit MCUs, reading/updating a 64-bit tick counter is often not atomic and can yield torn reads (non-monotonic GetTick results), which can break timeout behavior.
When the macro is defined, the board's tick source must match the width: its
`g_tick` counter and `GetTick` callback (typically `Board_GetTick`) must be
`uint64_t`, since `whal_Timeout.GetTick` becomes `uint64_t (*)(void)`. The

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #53

Scan targets checked: wolfhal-bugs, wolfhal-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread tests/core/test_timeout.c

@aidangarske aidangarske left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Skoll Multi-Scan Review

Modes: review + review-security
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] [review+review-security] WHAL_CFG_64BIT_TICK duplicates whal_Timeout and WHAL_TIMEOUT_EXPIRED per width and no in-tree board can enable it; a whal_Tick typedef fixes bothwolfHAL/timeout.h:41-89
  • [Medium] [review] 64-bit tick config is never exercised with a timeout that actually requires 64-bit arithmetictests/core/test_timeout.c:43-46
  • [Medium] [review] 64-bit tick callbacks require an atomic snapshot contractwolfHAL/timeout.h:72-85
  • [Low] [review-security] Mixed-width builds silently corrupt whal_Timeout across library boundarieswolfHAL/timeout.h:47-57
  • [Info] [review-security] CI matrix does not cover WHAL_CFG_64BIT_TICK combined with WHAL_CFG_NO_TIMEOUT.github/workflows/core-tests.yml:14

Review generated by Skoll

Comment thread wolfHAL/timeout.h
Comment thread tests/core/test_timeout.c
Comment thread wolfHAL/timeout.h
Comment thread wolfHAL/timeout.h
Comment thread .github/workflows/core-tests.yml
@aidangarske
aidangarske merged commit 5302069 into wolfSSL:main Aug 3, 2026
63 of 65 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants