Re: [PATCH 6/7] ASoC: cs42l43: Refactor to use for_each_set_bit

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

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