| Message ID | 20230926052007.3917389-1-andriy.shevchenko@linux.intel.com |
|---|---|
| Headers | show |
| Series | bitmap: get rid of bitmap_remap() and bitmap_biremap() uses | expand |
On Tue, Sep 26, 2023 at 08:20:03AM +0300, Andy Shevchenko wrote: > test_bitmap_printlist() prints the input string which contains > a new line character. Instead of stripping it, escape that kind > of characters, so developer will see the actual input string Grammar nit: "that kind of characters" -> "those kinds of characters" or "that kind of character" or "such characters" or ... > that has been used. Without this change the new line splits > the string to two, and the first one is not guaranteed to be > followed by the first part immediatelly. immediately And the second "first" should be "second"?? "the second part is not guaranteed to immediately follow the first" is clearer (and hopefully what you mean), IMHO. Cheers, Kent. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > lib/test_bitmap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c > index f2ea9f30c7c5..1f2dc7fef17f 100644 > --- a/lib/test_bitmap.c > +++ b/lib/test_bitmap.c > @@ -523,7 +523,7 @@ static void __init test_bitmap_printlist(void) > goto out; > } > > - pr_err("bitmap_print_to_pagebuf: input is '%s', Time: %llu\n", buf, time); > + pr_err("bitmap_print_to_pagebuf: input is '%*pEs', Time: %llu\n", ret, buf, time); > out: > kfree(buf); > kfree(bmap); > -- > 2.40.0.1.gaa8946217a0b >
On Tue, Sep 26, 2023 at 10:52:05AM +0200, Linus Walleij wrote: > On Tue, Sep 26, 2023 at 7:20 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > As Rasmus suggested [1], the bit remapping APIs should be local to NUMA. > > However, they are being in use outside of that for a while. To make > > above happen, introduces simplified APIs that can be used otherwise. > > > > It seems we might have yet another user of the above mentioned APIs. > > > > The last patch has not been tested anyhow (except compilation, so > > all testing and comments are welcome). > > > > The idea is to get an immutable tag (via my tree) that can be pulled > > by bitmap and GPIO trees on the need (while usually I send PR to > > the GPIO subsystem). > > > > Link: https://lore.kernel.org/all/20230815235934.47782-1-yury.norov@gmail.com/ [1] > > I don't understand the bitmap changes very well, Me neither... Oops (it was a joke :-) > but the resulting > changes to cdev look very nice clearly making that code more readable > and maintainable, While the above is a joke it took quite a time to get into the logic. Hence this patch. TBH I'm in doubt, that's why asking for careful testing and reviews. > so FWIW: > Acked-by: Linus Walleij <linus.walleij@linaro.org> Thank you!
On Tue, Sep 26, 2023 at 08:20:07AM +0300, Andy Shevchenko wrote: > Currently we have a few bitmap calls that are open coded in the library > module. Let's convert them to use generic bitmap APIs instead. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/gpio/gpiolib-cdev.c | 79 +++++++++++++++++-------------------- > 1 file changed, 36 insertions(+), 43 deletions(-) > > diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c > index e39d344feb28..a5bbbd44531f 100644 > --- a/drivers/gpio/gpiolib-cdev.c > +++ b/drivers/gpio/gpiolib-cdev.c > @@ -1263,35 +1263,32 @@ static long linereq_get_values(struct linereq *lr, void __user *ip) > { > struct gpio_v2_line_values lv; > DECLARE_BITMAP(vals, GPIO_V2_LINES_MAX); > + DECLARE_BITMAP(mask, GPIO_V2_LINES_MAX); > + DECLARE_BITMAP(bits, GPIO_V2_LINES_MAX); > struct gpio_desc **descs; > unsigned int i, didx, num_get; > - bool val; > int ret; > > /* NOTE: It's ok to read values of output lines. */ > if (copy_from_user(&lv, ip, sizeof(lv))) > return -EFAULT; > > - for (num_get = 0, i = 0; i < lr->num_lines; i++) { > - if (lv.mask & BIT_ULL(i)) { > - num_get++; > - descs = &lr->lines[i].desc; > - } > - } > + bitmap_from_arr64(mask, &lv.mask, GPIO_V2_LINES_MAX); > > + num_get = bitmap_weight(mask, lr->num_lines); > if (num_get == 0) > return -EINVAL; > > - if (num_get != 1) { > + if (num_get == 1) { > + descs = &lr->lines[find_first_bit(mask, lr->num_lines)].desc; > + } else { > descs = kmalloc_array(num_get, sizeof(*descs), GFP_KERNEL); > if (!descs) > return -ENOMEM; > - for (didx = 0, i = 0; i < lr->num_lines; i++) { > - if (lv.mask & BIT_ULL(i)) { > - descs[didx] = lr->lines[i].desc; > - didx++; > - } > - } > + > + didx = 0; > + for_each_set_bit(i, mask, lr->num_lines) > + descs[didx++] = lr->lines[i].desc; > } > ret = gpiod_get_array_value_complex(false, true, num_get, > descs, NULL, vals); > @@ -1301,19 +1298,15 @@ static long linereq_get_values(struct linereq *lr, void __user *ip) > if (ret) > return ret; > > - lv.bits = 0; > - for (didx = 0, i = 0; i < lr->num_lines; i++) { > - if (lv.mask & BIT_ULL(i)) { > - if (lr->lines[i].sw_debounced) > - val = debounced_value(&lr->lines[i]); > - else > - val = test_bit(didx, vals); > - if (val) > - lv.bits |= BIT_ULL(i); > - didx++; > - } > + bitmap_scatter(bits, vals, mask, lr->num_lines); > + > + for_each_set_bit(i, mask, lr->num_lines) { > + if (lr->lines[i].sw_debounced) > + __assign_bit(i, bits, debounced_value(&lr->lines[i])); > } > > + bitmap_to_arr64(&lv.bits, bits, GPIO_V2_LINES_MAX); > + > if (copy_to_user(ip, &lv, sizeof(lv))) > return -EFAULT; > > @@ -1324,35 +1317,35 @@ static long linereq_set_values_unlocked(struct linereq *lr, > struct gpio_v2_line_values *lv) > { > DECLARE_BITMAP(vals, GPIO_V2_LINES_MAX); > + DECLARE_BITMAP(mask, GPIO_V2_LINES_MAX); > + DECLARE_BITMAP(bits, GPIO_V2_LINES_MAX); > struct gpio_desc **descs; > unsigned int i, didx, num_set; > int ret; > > - bitmap_zero(vals, GPIO_V2_LINES_MAX); > - for (num_set = 0, i = 0; i < lr->num_lines; i++) { > - if (lv->mask & BIT_ULL(i)) { > - if (!test_bit(FLAG_IS_OUT, &lr->lines[i].desc->flags)) > - return -EPERM; > - if (lv->bits & BIT_ULL(i)) > - __set_bit(num_set, vals); > - num_set++; > - descs = &lr->lines[i].desc; > - } > - } > + bitmap_from_arr64(mask, &lv->mask, GPIO_V2_LINES_MAX); > + bitmap_from_arr64(bits, &lv->bits, GPIO_V2_LINES_MAX); > + > + num_set = bitmap_gather(vals, bits, mask, lr->num_lines); It looks like GPIO_V2_LINES_MAX is always 64, and so I wonder: is my understanding correct that all bits in ->mask and ->bits beyond lr->num_lines are clear? If so, you can seemingly pass the GPIO_V2_LINES_MAX instead of lr->num_lines, and that way it will be small_cons_nbits()-optimized. > if (num_set == 0) > return -EINVAL; > > - if (num_set != 1) { > + for_each_set_bit(i, mask, lr->num_lines) { > + if (!test_bit(FLAG_IS_OUT, &lr->lines[i].desc->flags)) > + return -EPERM; > + } > + > + if (num_set == 1) { > + descs = &lr->lines[find_first_bit(mask, lr->num_lines)].desc; > + } else { > /* build compacted desc array and values */ > descs = kmalloc_array(num_set, sizeof(*descs), GFP_KERNEL); > if (!descs) > return -ENOMEM; > - for (didx = 0, i = 0; i < lr->num_lines; i++) { > - if (lv->mask & BIT_ULL(i)) { > - descs[didx] = lr->lines[i].desc; > - didx++; > - } > - } > + > + didx = 0; > + for_each_set_bit(i, mask, lr->num_lines) > + descs[didx++] = lr->lines[i].desc; > } > ret = gpiod_set_array_value_complex(false, true, num_set, > descs, NULL, vals); > -- > 2.40.0.1.gaa8946217a0b