From e53501194448d9d09d958e91d1592469220212f9 Mon Sep 17 00:00:00 2001 From: Emil Gydesen Date: Wed, 15 Jan 2025 14:01:32 +0100 Subject: [PATCH 1/4] include: util: Add generic function to count bits set in a value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a generic function that will count the number of bits set in a value. It uses POPCOUNT (e.g. __builtin_popcount for GCC) if available, or else it will use Brian Kernighan’s Algorithm to count bits. POPCOUNT will likely always support unsigned ints, but the function was implemented to use it with uint8_t for the sake of simplicity and compatibility with Brian Kernighan’s Algorithm. A generic solution was chosen rather than a macro/function per type (e.g. uint8_t, uint16_t, etc.) as that is easier to maintain and also supports array types (e.g. counting the number of bits in 128 or 256 octet arrays). Signed-off-by: Emil Gydesen --- doc/releases/release-notes-4.2.rst | 1 + include/zephyr/sys/util.h | 111 ++++++++++++++++++----------- tests/unit/util/main.c | 28 +++++++- 3 files changed, 96 insertions(+), 44 deletions(-) diff --git a/doc/releases/release-notes-4.2.rst b/doc/releases/release-notes-4.2.rst index bdebeee4c5485..a49e7433294aa 100644 --- a/doc/releases/release-notes-4.2.rst +++ b/doc/releases/release-notes-4.2.rst @@ -269,6 +269,7 @@ New APIs and options * :c:func:`sys_clock_gettime` * :c:func:`sys_clock_settime` * :c:func:`sys_clock_nanosleep` + * :c:func:`zephyr_count_bits` * LoRaWAN * :c:func:`lorawan_request_link_check` diff --git a/include/zephyr/sys/util.h b/include/zephyr/sys/util.h index 0ea42ed315324..cad3821ea8e5d 100644 --- a/include/zephyr/sys/util.h +++ b/include/zephyr/sys/util.h @@ -46,16 +46,16 @@ extern "C" { */ /** @brief Cast @p x, a pointer, to an unsigned integer. */ -#define POINTER_TO_UINT(x) ((uintptr_t) (x)) +#define POINTER_TO_UINT(x) ((uintptr_t)(x)) /** @brief Cast @p x, an unsigned integer, to a void*. */ -#define UINT_TO_POINTER(x) ((void *) (uintptr_t) (x)) +#define UINT_TO_POINTER(x) ((void *)(uintptr_t)(x)) /** @brief Cast @p x, a pointer, to a signed integer. */ -#define POINTER_TO_INT(x) ((intptr_t) (x)) +#define POINTER_TO_INT(x) ((intptr_t)(x)) /** @brief Cast @p x, a signed integer, to a void*. */ -#define INT_TO_POINTER(x) ((void *) (intptr_t) (x)) +#define INT_TO_POINTER(x) ((void *)(intptr_t)(x)) #if !(defined(__CHAR_BIT__) && defined(__SIZEOF_LONG__) && defined(__SIZEOF_LONG_LONG__)) -# error Missing required predefined macros for BITS_PER_LONG calculation +#error Missing required predefined macros for BITS_PER_LONG calculation #endif /** Number of bits in a byte. */ @@ -68,27 +68,25 @@ extern "C" { #define NIBBLES_PER_BYTE (BITS_PER_BYTE / BITS_PER_NIBBLE) /** Number of bits in a long int. */ -#define BITS_PER_LONG (__CHAR_BIT__ * __SIZEOF_LONG__) +#define BITS_PER_LONG (__CHAR_BIT__ * __SIZEOF_LONG__) /** Number of bits in a long long int. */ -#define BITS_PER_LONG_LONG (__CHAR_BIT__ * __SIZEOF_LONG_LONG__) +#define BITS_PER_LONG_LONG (__CHAR_BIT__ * __SIZEOF_LONG_LONG__) /** * @brief Create a contiguous bitmask starting at bit position @p l * and ending at position @p h. */ -#define GENMASK(h, l) \ - (((~0UL) - (1UL << (l)) + 1) & (~0UL >> (BITS_PER_LONG - 1 - (h)))) +#define GENMASK(h, l) (((~0UL) - (1UL << (l)) + 1) & (~0UL >> (BITS_PER_LONG - 1 - (h)))) /** * @brief Create a contiguous 64-bit bitmask starting at bit position @p l * and ending at position @p h. */ -#define GENMASK64(h, l) \ - (((~0ULL) - (1ULL << (l)) + 1) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h)))) +#define GENMASK64(h, l) (((~0ULL) - (1ULL << (l)) + 1) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h)))) /** @brief 0 if @p cond is true-ish; causes a compile error otherwise. */ -#define ZERO_OR_COMPILE_ERROR(cond) ((int) sizeof(char[1 - 2 * !(cond)]) - 1) +#define ZERO_OR_COMPILE_ERROR(cond) ((int)sizeof(char[1 - 2 * !(cond)]) - 1) #if defined(__cplusplus) @@ -104,10 +102,9 @@ extern "C" { * * This macro is available only from C, not C++. */ -#define IS_ARRAY(array) \ - ZERO_OR_COMPILE_ERROR( \ - !__builtin_types_compatible_p(__typeof__(array), \ - __typeof__(&(array)[0]))) +#define IS_ARRAY(array) \ + ZERO_OR_COMPILE_ERROR( \ + !__builtin_types_compatible_p(__typeof__(array), __typeof__(&(array)[0]))) /** * @brief Number of elements in the given @p array @@ -118,8 +115,7 @@ extern "C" { * * In C, passing a pointer as @p array causes a compile error. */ -#define ARRAY_SIZE(array) \ - ((size_t) (IS_ARRAY(array) + (sizeof(array) / sizeof((array)[0])))) +#define ARRAY_SIZE(array) ((size_t)(IS_ARRAY(array) + (sizeof(array) / sizeof((array)[0])))) #endif /* __cplusplus */ @@ -140,10 +136,11 @@ extern "C" { * It is specially useful for cases where flexible arrays are * used in unions or are not the last element in the struct. */ -#define FLEXIBLE_ARRAY_DECLARE(type, name) \ - struct { \ - struct { } __unused_##name; \ - type name[]; \ +#define FLEXIBLE_ARRAY_DECLARE(type, name) \ + struct { \ + struct { \ + } __unused_##name; \ + type name[]; \ } /** @@ -161,7 +158,7 @@ extern "C" { * @return 1 if @p ptr is part of @p array, 0 otherwise */ #define IS_ARRAY_ELEMENT(array, ptr) \ - ((ptr) && POINTER_TO_UINT(array) <= POINTER_TO_UINT(ptr) && \ + ((ptr) && POINTER_TO_UINT(array) <= POINTER_TO_UINT(ptr) && \ POINTER_TO_UINT(ptr) < POINTER_TO_UINT(&(array)[ARRAY_SIZE(array)]) && \ (POINTER_TO_UINT(ptr) - POINTER_TO_UINT(array)) % sizeof((array)[0]) == 0) @@ -253,9 +250,8 @@ extern "C" { * @brief Validate CONTAINER_OF parameters, only applies to C mode. */ #ifndef __cplusplus -#define CONTAINER_OF_VALIDATE(ptr, type, field) \ - BUILD_ASSERT(SAME_TYPE(*(ptr), ((type *)0)->field) || \ - SAME_TYPE(*(ptr), void), \ +#define CONTAINER_OF_VALIDATE(ptr, type, field) \ + BUILD_ASSERT(SAME_TYPE(*(ptr), ((type *)0)->field) || SAME_TYPE(*(ptr), void), \ "pointer type mismatch in CONTAINER_OF"); #else #define CONTAINER_OF_VALIDATE(ptr, type, field) @@ -282,10 +278,10 @@ extern "C" { * @param field the name of the field within the struct @p ptr points to * @return a pointer to the structure that contains @p ptr */ -#define CONTAINER_OF(ptr, type, field) \ - ({ \ - CONTAINER_OF_VALIDATE(ptr, type, field) \ - ((type *)(((char *)(ptr)) - offsetof(type, field))); \ +#define CONTAINER_OF(ptr, type, field) \ + ({ \ + CONTAINER_OF_VALIDATE(ptr, type, field) \ + ((type *)(((char *)(ptr)) - offsetof(type, field))); \ }) /** @@ -309,8 +305,7 @@ extern "C" { * * @return Concatenated token. */ -#define CONCAT(...) \ - UTIL_CAT(_CONCAT_, NUM_VA_ARGS_LESS_1(__VA_ARGS__))(__VA_ARGS__) +#define CONCAT(...) UTIL_CAT(_CONCAT_, NUM_VA_ARGS_LESS_1(__VA_ARGS__))(__VA_ARGS__) /** * @brief Check if @p ptr is aligned to @p align alignment @@ -320,14 +315,14 @@ extern "C" { /** * @brief Value of @p x rounded up to the next multiple of @p align. */ -#define ROUND_UP(x, align) \ - ((((unsigned long)(x) + ((unsigned long)(align) - 1)) / \ - (unsigned long)(align)) * (unsigned long)(align)) +#define ROUND_UP(x, align) \ + ((((unsigned long)(x) + ((unsigned long)(align) - 1)) / (unsigned long)(align)) * \ + (unsigned long)(align)) /** * @brief Value of @p x rounded down to the previous multiple of @p align. */ -#define ROUND_DOWN(x, align) \ +#define ROUND_DOWN(x, align) \ (((unsigned long)(x) / (unsigned long)(align)) * (unsigned long)(align)) /** @brief Value of @p x rounded up to the next word boundary. */ @@ -690,7 +685,7 @@ char *utf8_lcpy(char *dst, const char *src, size_t n); #define __z_log2d(x) (32 - __builtin_clz(x) - 1) #define __z_log2q(x) (64 - __builtin_clzll(x) - 1) -#define __z_log2(x) (sizeof(__typeof__(x)) > 4 ? __z_log2q(x) : __z_log2d(x)) +#define __z_log2(x) (sizeof(__typeof__(x)) > 4 ? __z_log2q(x) : __z_log2d(x)) /** * @brief Compute log2(x) @@ -714,7 +709,7 @@ char *utf8_lcpy(char *dst, const char *src, size_t n); * * @return ceil(log2(x)) when 1 <= x <= max(type(x)), 0 when x < 1 */ -#define LOG2CEIL(x) ((x) <= 1 ? 0 : __z_log2((x)-1) + 1) +#define LOG2CEIL(x) ((x) <= 1 ? 0 : __z_log2((x) - 1) + 1) /** * @brief Compute next highest power of two @@ -728,7 +723,7 @@ char *utf8_lcpy(char *dst, const char *src, size_t n); * * @return 2^ceil(log2(x)) or 0 if 2^ceil(log2(x)) would saturate 64-bits */ -#define NHPOT(x) ((x) < 1 ? 1 : ((x) > (1ULL<<63) ? 0 : 1ULL << LOG2CEIL(x))) +#define NHPOT(x) ((x) < 1 ? 1 : ((x) > (1ULL << 63) ? 0 : 1ULL << LOG2CEIL(x))) /** * @brief Determine if a buffer exceeds highest address @@ -742,9 +737,8 @@ char *utf8_lcpy(char *dst, const char *src, size_t n); * * @return true if pointer overflow detected, false otherwise */ -#define Z_DETECT_POINTER_OVERFLOW(addr, buflen) \ - (((buflen) != 0) && \ - ((UINTPTR_MAX - (uintptr_t)(addr)) <= ((uintptr_t)((buflen) - 1)))) +#define Z_DETECT_POINTER_OVERFLOW(addr, buflen) \ + (((buflen) != 0) && ((UINTPTR_MAX - (uintptr_t)(addr)) <= ((uintptr_t)((buflen) - 1)))) /** * @brief XOR n bytes @@ -819,6 +813,39 @@ static inline bool util_eq(const void *m1, size_t len1, const void *m2, size_t l return len1 == len2 && (m1 == m2 || util_memeq(m1, m2, len1)); } +/** + * @brief Returns the number of bits set in a value + * + * @param value The value to count number of bits set of + * @param len The number of octets in @p value + */ +static inline size_t zephyr_count_bits(const void *value, size_t len) +{ + size_t cnt = 0U; + size_t i = 0U; + +#ifdef POPCOUNT + for (; i < len / sizeof(unsigned int); i++) { + unsigned int val = ((const unsigned int *)value)[i]; + + cnt += POPCOUNT(val); + } + i *= sizeof(unsigned int); /* convert to a uint8_t index for the remainder (if any) */ +#endif + + for (; i < len; i++) { + uint8_t value_u8 = ((const uint8_t *)value)[i]; + + /* Implements Brian Kernighan’s Algorithm to count bits */ + while (value_u8) { + value_u8 &= (value_u8 - 1); + cnt++; + } + } + + return cnt; +} + #ifdef __cplusplus } #endif diff --git a/tests/unit/util/main.c b/tests/unit/util/main.c index 2ea8724019908..c3edffe04cfba 100644 --- a/tests/unit/util/main.c +++ b/tests/unit/util/main.c @@ -1,14 +1,19 @@ /* * Copyright (c) 2019 Oticon A/S + * Copyright (c) 2025 Nordic Semiconductor ASA * * SPDX-License-Identifier: Apache-2.0 */ -#include -#include +#include #include #include +#include +#include +#include +#include + ZTEST(util, test_u8_to_dec) { char text[4]; uint8_t len; @@ -845,6 +850,25 @@ ZTEST(util, test_mem_xor_128) zassert_mem_equal(expected_result, dst, 16); } +ZTEST(util, test_zephyr_count_bits) +{ + uint8_t zero = 0U; + uint8_t u8 = 29U; + uint16_t u16 = 29999U; + uint32_t u32 = 2999999999U; + uint64_t u64 = 123456789012345ULL; + uint8_t u8_arr[] = {u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, + u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8}; + + zassert_equal(zephyr_count_bits(&zero, sizeof(zero)), 0); + zassert_equal(zephyr_count_bits(&u8, sizeof(u8)), 4); + zassert_equal(zephyr_count_bits(&u16, sizeof(u16)), 10); + zassert_equal(zephyr_count_bits(&u32, sizeof(u32)), 20); + zassert_equal(zephyr_count_bits(&u64, sizeof(u64)), 23); + + zassert_equal(zephyr_count_bits(u8_arr, sizeof(u8_arr)), 128); +} + ZTEST(util, test_CONCAT) { #define _CAT_PART1 1 From d3a287a0277312e4491efdc7fdba7d7ad53e8426 Mon Sep 17 00:00:00 2001 From: Emil Gydesen Date: Wed, 15 Jan 2025 14:04:38 +0100 Subject: [PATCH 2/4] Bluetooth: Audio: Use generic zephyr_count_bits to count bits Instead of re-implementing or assuming that POPCOUNT is available we now use the generic function from Zephyr. Signed-off-by: Emil Gydesen --- .../cap_acceptor/src/cap_acceptor_broadcast.c | 5 +++-- subsys/bluetooth/audio/audio.c | 15 +++------------ subsys/bluetooth/audio/bap_broadcast_sink.c | 18 +----------------- .../audio/src/bap_broadcast_sink_test.c | 2 +- 4 files changed, 8 insertions(+), 32 deletions(-) diff --git a/samples/bluetooth/cap_acceptor/src/cap_acceptor_broadcast.c b/samples/bluetooth/cap_acceptor/src/cap_acceptor_broadcast.c index 190bef26eabe1..10f7378ffb692 100644 --- a/samples/bluetooth/cap_acceptor/src/cap_acceptor_broadcast.c +++ b/samples/bluetooth/cap_acceptor/src/cap_acceptor_broadcast.c @@ -1,7 +1,7 @@ /** @file * @brief Bluetooth Common Audio Profile (CAP) Acceptor broadcast. * - * Copyright (c) 2024 Nordic Semiconductor ASA + * Copyright (c) 2024-2025 Nordic Semiconductor ASA * * SPDX-License-Identifier: Apache-2.0 */ @@ -459,7 +459,8 @@ static int bis_sync_req_cb(struct bt_conn *conn, LOG_INF("BIS sync request received for %p: 0x%08x", recv_state, bis_sync_req[0]); - if (new_bis_sync_req != BT_BAP_BIS_SYNC_NO_PREF && POPCOUNT(new_bis_sync_req) > 1U) { + if (new_bis_sync_req != BT_BAP_BIS_SYNC_NO_PREF && + zephyr_count_bits(&new_bis_sync_req, sizeof(new_bis_sync_req)) > 1U) { LOG_WRN("Rejecting BIS sync request for 0x%08X as we do not support that", new_bis_sync_req); diff --git a/subsys/bluetooth/audio/audio.c b/subsys/bluetooth/audio/audio.c index 75109d4fd9657..330e4a0b684c5 100644 --- a/subsys/bluetooth/audio/audio.c +++ b/subsys/bluetooth/audio/audio.c @@ -2,6 +2,7 @@ /* * Copyright (c) 2022 Codecoup + * Copyright (c) 2025 Nordic Semiconductor ASA * * SPDX-License-Identifier: Apache-2.0 */ @@ -23,6 +24,7 @@ #include #include #include +#include #include #include "audio_internal.h" @@ -146,18 +148,7 @@ uint8_t bt_audio_get_chan_count(enum bt_audio_location chan_allocation) return 1; } -#ifdef POPCOUNT - return POPCOUNT(chan_allocation); -#else - uint8_t cnt = 0U; - - while (chan_allocation != 0U) { - cnt += chan_allocation & 1U; - chan_allocation >>= 1U; - } - - return cnt; -#endif + return zephyr_count_bits(&chan_allocation, sizeof(chan_allocation)); } static bool valid_ltv_cb(struct bt_data *data, void *user_data) diff --git a/subsys/bluetooth/audio/bap_broadcast_sink.c b/subsys/bluetooth/audio/bap_broadcast_sink.c index 384ed6ca1e05f..3d233eceb8018 100644 --- a/subsys/bluetooth/audio/bap_broadcast_sink.c +++ b/subsys/bluetooth/audio/bap_broadcast_sink.c @@ -1093,22 +1093,6 @@ int bt_bap_broadcast_sink_create(struct bt_le_per_adv_sync *pa_sync, uint32_t br return 0; } -static uint8_t bit_count(uint32_t bitfield) -{ -#ifdef POPCOUNT - return POPCOUNT(bitfield); -#else - uint8_t cnt = 0U; - - while (bitfield != 0U) { - cnt += bitfield & 1U; - bitfield >>= 1U; - } - - return cnt; -#endif -} - struct sync_base_info_data { struct bt_audio_codec_cfg codec_cfgs[CONFIG_BT_BAP_BROADCAST_SNK_STREAM_COUNT]; struct bt_audio_codec_cfg *subgroup_codec_cfg; @@ -1293,7 +1277,7 @@ int bt_bap_broadcast_sink_sync(struct bt_bap_broadcast_sink *sink, uint32_t inde } /* Validate that number of bits set is within supported range */ - bis_count = bit_count(indexes_bitfield); + bis_count = zephyr_count_bits(&indexes_bitfield, sizeof(indexes_bitfield)); if (bis_count > CONFIG_BT_BAP_BROADCAST_SNK_STREAM_COUNT) { LOG_DBG("Cannot sync to more than %d streams (%u was requested)", CONFIG_BT_BAP_BROADCAST_SNK_STREAM_COUNT, bis_count); diff --git a/tests/bsim/bluetooth/audio/src/bap_broadcast_sink_test.c b/tests/bsim/bluetooth/audio/src/bap_broadcast_sink_test.c index be580e02ea61c..b364db2e416b1 100644 --- a/tests/bsim/bluetooth/audio/src/bap_broadcast_sink_test.c +++ b/tests/bsim/bluetooth/audio/src/bap_broadcast_sink_test.c @@ -801,7 +801,7 @@ static void test_broadcast_sync(const uint8_t broadcast_code[BT_ISO_BROADCAST_CO return; } - stream_sync_cnt = POPCOUNT(bis_index_bitfield); + stream_sync_cnt = zephyr_count_bits(&bis_index_bitfield, sizeof(bis_index_bitfield)); } static void test_broadcast_sync_inval(void) From b09f01bef246d534b3fddf162001614122ef7999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benjamin=20Cab=C3=A9?= Date: Tue, 18 Mar 2025 06:46:12 +0100 Subject: [PATCH 3/4] drivers: adc: adopt new zephyr_count_bits util function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopt new zephyr_count_bits helper from util.h and avoid having conflicting definition Signed-off-by: Benjamin Cabé Signed-off-by: Emil Gydesen --- drivers/adc/adc_sam.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/drivers/adc/adc_sam.c b/drivers/adc/adc_sam.c index 32b8450ef0fda..42b8c47638614 100644 --- a/drivers/adc/adc_sam.c +++ b/drivers/adc/adc_sam.c @@ -16,6 +16,7 @@ #include #include +#include LOG_MODULE_REGISTER(adc_sam, CONFIG_ADC_LOG_LEVEL); #define SAM_ADC_NUM_CHANNELS 16 @@ -51,18 +52,6 @@ struct adc_sam_data { uint8_t num_active_channels; }; -static uint8_t count_bits(uint32_t val) -{ - uint8_t res = 0; - - while (val) { - res += val & 1U; - val >>= 1; - } - - return res; -} - static int adc_sam_channel_setup(const struct device *dev, const struct adc_channel_cfg *channel_cfg) { @@ -156,7 +145,8 @@ static void adc_context_start_sampling(struct adc_context *ctx) const struct adc_sam_config *const cfg = data->dev->config; Adc *const adc = cfg->regs; - data->num_active_channels = count_bits(ctx->sequence.channels); + data->num_active_channels = + zephyr_count_bits(&ctx->sequence.channels, sizeof(ctx->sequence.channels)); /* Disable all */ adc->ADC_CHDR = 0xffff; @@ -222,7 +212,7 @@ static int start_read(const struct device *dev, return -EINVAL; } - data->num_active_channels = count_bits(channels); + data->num_active_channels = zephyr_count_bits(&channels, sizeof(channels)); error = check_buffer_size(sequence, data->num_active_channels); if (error) { From 9f23d9f6ca61a188c2bdf974f3159a8d88048a62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benjamin=20Cab=C3=A9?= Date: Tue, 18 Mar 2025 06:46:35 +0100 Subject: [PATCH 4/4] tests: kernel: adopt new zephyr_count_bits util function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopt new zephyr_count_bits helper from util.h and avoid having conflicting definition Signed-off-by: Benjamin Cabé Signed-off-by: Emil Gydesen --- tests/kernel/common/src/bitarray.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/tests/kernel/common/src/bitarray.c b/tests/kernel/common/src/bitarray.c index acac6efdf51af..b120d171207f7 100644 --- a/tests/kernel/common/src/bitarray.c +++ b/tests/kernel/common/src/bitarray.c @@ -360,29 +360,13 @@ void alloc_and_free_predefined(void) "sys_bitarray_alloc() failed bits comparison"); } -static inline size_t count_bits(uint32_t val) -{ - /* Implements Brian Kernighan’s Algorithm - * to count bits. - */ - - size_t cnt = 0; - - while (val != 0) { - val = val & (val - 1); - cnt++; - } - - return cnt; -} - size_t get_bitarray_popcnt(sys_bitarray_t *ba) { size_t popcnt = 0; unsigned int idx; for (idx = 0; idx < ba->num_bundles; idx++) { - popcnt += count_bits(ba->bundles[idx]); + popcnt += zephyr_count_bits(&ba->bundles[idx], sizeof(uint32_t)); } return popcnt;