Skip to content
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

include: zephyr: sys: Introduce IS_BIT_SET() macro #83683

Merged
merged 2 commits into from
Jan 10, 2025

Conversation

duda-patryk
Copy link
Collaborator

@duda-patryk duda-patryk commented Jan 8, 2025

This PR provides one definition of IS_BIT_SET() in util_macro.h and removes all other definitions.

This macro is defined in a few places which leads to macro redefinition error e.g. when compiling prometheus network sample for NPCX boards, see #81548.

This macro is defined in a few places which leads to macro redefinition
error e.g. when compiling prometheus network sample for NPCX boards.

Provide one definition of IS_BIT_SET() in util_macro.h to fix the
problem.

Signed-off-by: Patryk Duda <[email protected]>
Replace DAI_INTEL_SSP_IS_BIT_SET() macro with IS_BIT_SET() macro from
utils_macro.h.

Signed-off-by: Patryk Duda <[email protected]>
Copy link
Contributor

@andyross andyross left a comment

Choose a reason for hiding this comment

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

Seems like reasonable cleanup as far as it goes, but there are a LOT of places in the tree testing bits that never thought to use a standard API, these just happened to collide on a name. I guess my sense is that this doesn't really belong in util_macro.h, but if we're going to have it it makes sense to use it.

Also one tiny style nitpick.

* @param value Value that contain checked bit
* @param bit Bit number
*/
#define IS_BIT_SET(value, bit) ((((value) >> (bit)) & (0x1)) != 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Expressing this as a right shift seems needlessly confusing? ((BIT(bit) & (value)) != 0) is shorter and has fewer parens, FWIW.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

On systems with 32 bit unsigned long the ((BIT(bit) & (value)) != 0) will not work properly for higher 32 bits of 64 bit value, due to shifting 1UL for more bits than it has. Current approach doesn't have such issue.

@kartben kartben merged commit bcaa59d into zephyrproject-rtos:main Jan 10, 2025
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants