On Wed, Sep 27, 2023 at 09:32:11AM +0800, Kent Gibson wrote:

On Tue, Sep 26, 2023 at 08:20:07AM +0300, Andy Shevchenko wrote:

quoted

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.

Firstly, I didn't consider using the bitmap module here as, in my mind at
least, that is intended for bitmaps wider than 64 bits, or with variable
width. In this case the bitmap is fixed at 64 bits, so bitops seemed more
appropriate.

And I would argue that they aren't "open coded" - they are parallelized
to reduce the number of passes over the bitmap.
This change serialises them, e.g. the get used to require 2 passes over
the bitmap, it now requires 3 or 4.  The set used to require 1 and now
requires 2.
And there are additional copies that the original doesn't require.
So your change looks less efficient to me - unless there is direct
hardware support for bitmap ops??

Wrt the argument that the serialized form is clearer and more
maintainable, optimised code is frequently more cryptic - as noted in
bitmap.c itself, and this code has remained unchanged since it was merged
3 years ago, so the only maintenance it has required is to be more
maintainable??  Ok then.

Your patch is functionally equivalent and pass my uAPI tests, so 

Tested-by: Kent Gibson <warthog618@gmail.com>

Thanks for testing!

but my preference is to leave it as is.

As Yury mentioned we need to look at bitmap APIs and make them possible to have
a compile-time optimizations. With that in mind, I would prefer bitmap APIs
over open-coded stuff which is hardly to be understood (yes, I still point
out that it takes a few hours to me, maybe because I'm stupid enough, to
get what's the heck is going one there, esp. for the == 1 case).

Yet, it opens a way to scale this in case we might have v3 ABI that let's say
allows to work with 512 GPIOs at a time. With your code it will be much harder
to achieve and see what you wrote about maintenance (in that case).

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel