On Thu, Jan 25, 2024 at 12:31 PM Charles Keepax ckeepax@opensource.cirrus.com 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.