On Wed, Jan 24, 2024 at 6:56 PM Charles Keepax
<ckeepax@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> Refactor the code in cs42l43_mask_to_slots to use for_each_set_bit.
..._bit()
...
> #include <linux/bitops.h>
> +#include <linux/bits.h>
> +#include <linux/find.h>
No need, it's included by bitops.h (and there is kinda guarantee for these).
...
> + for_each_set_bit(slot, &mask, sizeof(mask) * BITS_PER_BYTE) {
BITS_PER_TYPE() ?
But actually it's unsigned long, so BITS_PER_LONG should suffice.
BUT. Why not use ..._MAX_CHANNELS here directly as it was in the
original loop? You might need a static_assert() that tells you it's
not longer than bits in the mask, but it probably is an overkill
check.
...
> + if (i == CS42L43_ASP_MAX_CHANNELS) {
> + dev_warn(priv->dev, "Too many channels in TDM mask: %lx\n",
> + mask);
This is invariant to the loop, you may check even before it (I'm
writing by memory, might have made mistake(s) in the snippet):
nslots = hweight_long(mask);
if (nslots >= ..._MAX_CHANNELS)
dev_warn(...);
> return;
> + }
--
With Best Regards,
Andy Shevchenko
