On Thu, Jan 25, 2024 at 12:31 PM Charles Keepax
<ckeepax@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> Refactor the code in cs42l43_mask_to_slots() to use for_each_set_bit().
...
> +#include <linux/bits.h>
> +#include <linux/find.h>
Of course you can leave them, but I think we more or less _guarantee_
their inclusion by bitops.h, otherwise bitops.h will require those two
in _each_ instance of use which sounds not such a clever decision.
That said, I would avoid adding them here as the compiler would need
to mmap() the first page of each header, check the guard and unmap,
and repeat for each header. This will slow down the build for no
particular reason.
...
Adding nslots parameter is a good idea, but I still think the code can
be refactored better (have you checked the code generation, btw? I
believe my version would be better or not worse).
> + for_each_set_bit(slot, &mask, BITS_PER_TYPE(mask)) {
> + if (i == nslots) {
> + dev_warn(priv->dev, "Too many channels in TDM mask: %lx\n",
> + mask);
> return;
> + }
>
> + slots[i++] = slot;
> }
i = 0;
for_each_set_bit(slot, &mask, CS42L43_ASP_MAX_CHANNELS)
slots[i++] = slot;
if (hweight_long(mask) >= CS42L43_ASP_MAX_CHANNELS)
dev_warn(priv->dev, "Too many channels in TDM mask\n");
The code is simpler and behaviour is not changed.
--
With Best Regards,
Andy Shevchenko
