Hi Andy,

On Thu, 29 Feb 2024 17:20:59 +0200
Andy Shevchenko [off-list ref] wrote:

On Thu, Feb 29, 2024 at 03:15:52PM +0100, Herve Codina wrote:

quoted

QMC channels support runtime timeslots changes but nothing is done at
the QMC HDLC driver to handle these changes.

Use existing IFACE ioctl in order to configure the timeslots to use.  

...

quoted

+	bitmap_scatter(ts_mask, map, ts_mask_avail, 64);  

Wondering if we may have returned value more useful and hence having something like

	n = bitmap_scatter(...);

I thought about it.

In bitmap_{scatter,gather}(dst, src, mask, nbits), only returning the
weight of the third parameter (i.e. mask) can be efficient regarding to the
for_each_set_bit() loop done in the functions.
For dst parameter, we need to add a counter in the loop to count the number
of bit set depending on the test_bit() result. Will this be more efficient
than a call to bitmap_weight() ?

Also, in my case, the third parameter is ts_mask_avail and I don't need
its weight.

I thing users that need to have the dst or src weight should call
bitmap_weight() themselves as this is users context dependent.

bitmap_{scatter,gather}(dst, src, mask, nbits) can be improved later with
no impact to current users (except performance).

That's why I concluded to return nothing from bitmap_{scatter,gather} when
I took the old existing patches.

quoted

+	if (bitmap_weight(ts_mask, 64) != bitmap_weight(map, 64)) {  

	if (n != ...) {

?

quoted

+		dev_err(qmc_hdlc->dev, "Cannot translate timeslots %64pb -> (%64pb, %64pb)\n",
+			map, ts_mask_avail, ts_mask);
+		return -EINVAL;
+	}  

...

quoted

+	bitmap_gather(map, ts_mask, ts_mask_avail, 64);
+
+	if (bitmap_weight(ts_mask, 64) != bitmap_weight(map, 64)) {
+		dev_err(qmc_hdlc->dev, "Cannot translate timeslots (%64pb, %64pb) -> %64pb\n",
+			ts_mask_avail, ts_mask, map);
+		return -EINVAL;
+	}  

Ditto.

Best regards,
Hervé