-
Notifications
You must be signed in to change notification settings - Fork 2.1k
kinetis/bme: add bit_checkXX() functions #21705
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
base: master
Are you sure you want to change the base?
Conversation
A general `bit_checkXX` implementation was added with RIOT-OS#16522 to `sys/bit.h`, but still missing for kinetis MCUs.
Is this not already covered by the |
No, because the whole |
Ah the Kinetis Bit Manipulation Engine is indeed different from the ARM bit-banding extension! I would have thought they'd just use the standard functionality under a different name. |
*/ | ||
static inline bool bit_check32(volatile uint32_t *ptr, uint8_t bit) | ||
{ | ||
return *((volatile uint32_t *)(((uintptr_t)ptr) | BME_AND_MASK)) & (uint32_t)(1ul << bit); |
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.
Wouldn't this be
return *((volatile uint32_t *)(((uintptr_t)ptr) | BME_AND_MASK)) & (uint32_t)(1ul << bit); | |
return bme_bitfield32(ptr, bit, 32); |
I have openlabs-kw41z-mini
at home and can test
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.
Ah yeah, makes sense. I am not familiar with the kinetis bme at all, so I was just implementing it analogous to the other functions and didn't look at the bitfield functions above properly.
Should I already change it in this PR or do you want to do the testing first?
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.
Friendly ping @benpicco :)
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.
Sorry, I didn't get to it before my vaccation - I added a unit test so we can actually test those function and turns out bit_clearXX()
is already not working 😕
2025-09-30 00:44:56,723 # START
2025-09-30 00:44:56,723 # .clear bit 0
2025-09-30 00:44:56,723 # clear bit 2
2025-09-30 00:44:56,723 #
2025-09-30 00:44:56,724 # bit_tests.test_bit8 (tests/unittests/tests-bit/tests-bit.c 36) exp 250 was 251
2025-09-30 00:44:56,724 # .
2025-09-30 00:44:56,735 # bit_tests.test_bit16 (tests/unittests/tests-bit/tests-bit.c 60) exp 65530 was 65531
2025-09-30 00:44:56,735 # .
2025-09-30 00:44:56,740 # bit_tests.test_bit32 (tests/unittests/tests-bit/tests-bit.c 84) exp 4294967290 was 4294967291
If you want to move this forward, you can just set BITBAND_FUNCTIONS_PROVIDED 0
and I'll look into this later since I can actually test it.
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.
Okay, thanks!
If you want to move this forward, you can just set
BITBAND_FUNCTIONS_PROVIDED
0 and I'll look into this later since I can actually test it.
Opened #21750.
Contribution description
Add
bit_checkXX
functions for kinetis MCUs, analogous to the existingbit_clear
andbit_set
functions.A general
bit_checkXX
implementation was added with #16522 tosys/bit.h
, and probably just forgotten for the kinetis override?cc @benpicco.
Testing procedure
Unfortunately I don't have any kinetis-based boards with me. If someone has them and it is wanted, I can add some unitests for setting/clearing/ reading bits, analogous to the existing
tests-bitfield
test.Issues/PRs references
Came up in #21577, which adds a first usage of
bit_check
(Murdock error).