![](https://secure.gravatar.com/avatar/2c82d4e2a3f6f360708276583a4ebca9.jpg?s=120&d=mm&r=g)
29 Jan
2024
29 Jan
'24
5:27 p.m.
On Thu, Jan 25, 2024 at 09:11:44PM +0200, Andy Shevchenko wrote:
On Thu, Jan 25, 2024 at 12:31 PM Charles Keepax ckeepax@opensource.cirrus.com wrote: 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.
I don't think this works, the limit here is on the number of channels not on the position of those channels. The last parameter of for_each_set_bits appears to measure against the bit position not the number of set bits. So for example 0xFC000000 would be a valid 6 channel mask, but would result in no slot positions being set in the above code.
Thanks, Charles