Skip to content

[reg, test] Use 32-bit register accesses on all targets - #62

Open
night1rider wants to merge 3 commits into
wolfSSL:mainfrom
night1rider:reg-32bit
Open

night1rider wants to merge 3 commits into
wolfSSL:mainfrom
night1rider:reg-32bit

Conversation

@night1rider

Copy link
Copy Markdown

The whal_Reg_* helpers read and wrote through size_t, which is 64 bits on AArch64, so a write also changed the next register. They now always access one 32-bit register, as the header already said.

Adds core reg tests that check a write leaves the next register alone, and builds src/reg.c in the core tests.

@night1rider night1rider self-assigned this Nov 5, 2026
Copilot AI balanced review requested due to automatic review settings November 5, 2026 04:36

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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

The whal_Reg_* helpers read and wrote through size_t, which is 64 bits on AArch64, so a write also changed the next register. They now always access one 32-bit register, as the header already said. Value and mask bits above bit 31 are dropped; reg.h and the driver guide say so.

Adds core reg tests that check each helper touches only its own register and drops bits above bit 31, that an update clears field bits, and that a poll ignores bits outside the mask, retries until the field matches, waits for bits to clear, and times out. Builds src/reg.c in the core tests.
…opt-in

whal_Reg32_* and whal_Reg64_* take a uintptr_t base, a uint32_t offset and values of the register's width, so the access width is explicit and no value is cast. whal_Reg64_* exists only where a 64-bit access is one access (WHAL_HAVE_REG64).

The size_t whal_Reg_* API is now only built with WHAL_CFG_REG_LEGACY. It does 32-bit accesses through whal_Reg32_*. Where size_t is 64 bits, a value or mask above bit 31 returns WHAL_EINVAL and nothing is written; Write, Update and Get now return whal_Error. Where size_t is 32 bits there is no check, and the code is the same as before. The boards in boards/ define WHAL_CFG_REG_LEGACY, so the existing drivers build unchanged.

Core tests cover Reg32, Reg64 and the legacy API, including the range checks, and check that whal_Reg_* is not defined without WHAL_CFG_REG_LEGACY. The driver and test guides use whal_Reg32_*.
Comment thread boards/pic32cz_curiosity_ultra/board.mk Outdated
OBJCOPY = $(GCC_PATH)arm-none-eabi-objcopy

# Drivers here use the legacy whal_Reg_* API
CFLAGS += -DWHAL_CFG_REG_LEGACY

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.

Since wolfHAL is relatively new I'm not super concerned with retaining the legacy size_t interface. I think we should just rip the band-aid off now while we can and only provide the whal_Reg32/64 interface

Per review, keep only whal_Reg32_* and whal_Reg64_*. All drivers, board
files and tests now call whal_Reg32_*. Device base fields and base
locals are uintptr_t. Variables read with whal_Reg32_Get are uint32_t.
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