Skip to content

Introduce the for_each_set_clump macro

Syed Nayyar Waris linux-arch linux-arm-kernel linux-gpio linux-pm lkml oe-kbuild-all 1 version In review

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 Latest5 patches 14 repliesIn review

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/5clump_bits: Introduce the for_each_set_clump macrodrivers/gpio/clump_bits.h+101 -02 replies
2/5lib/test_bitmap.c: Add for_each_set_clump test caseslib/test_bitmap.c+144 -02 replies
3/5gpio: thunderx: Utilize for_each_set_clump macrodrivers/gpio/gpio-thunderx.c+8 -40 replies
4/5gpio: xilinx: Utilize generic bitmap_get_value and _set_valuedrivers/gpio/gpio-xilinx.c+33 -332 replies
5/5gpio: xilinx: Add extra check if sum of widths exceed 64drivers/gpio/gpio-xilinx.c+6 -02 replies

Reviews trailers per patch

1 of 5 patches has a Reviewed-by or Acked-by.

PatchMichal Simek
1/5
2/5
3/5
4/5
5/5A-b

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v1 thread

Cover letter [PATCH 0/5] Introduce the for_each_set_clump macro

Hello Linus,

Since this patchset primarily affects GPIO drivers, would you like to pick it up through your GPIO tree?

(Note: Patchset resent with the new macro and relevant functions shifted to a new header clump_bits.h [Linus Torvalds])

Michal,
What do you think of [PATCH 5/5]? Is the conditional check needed? And also does returning -EINVAL look good?

This patchset introduces a new generic version of for_each_set_clump. The previous version of for_each_set_clump8 used a fixed size 8-bit clump, but the new generic version can work with clump of any size but less than or equal to BITS_PER_LONG. The patchset utilizes the new macro in several GPIO drivers.

The earlier 8-bit for_each_set_clump8 facilitated a
for-loop syntax that iterates over a memory region entire groups of set bits at a time.

For example, suppose you would like to iterate over a 32-bit integer 8 bits at a time, skipping over 8-bit groups with no set bit, where XXXXXXXX represents the current 8-bit group:

    Example:        10111110 00000000 11111111 00110011
    First loop:     10111110 00000000 11111111 XXXXXXXX
    Second loop:    10111110 00000000 XXXXXXXX 00110011
    Third loop:     XXXXXXXX 00000000 11111111 00110011

Each iteration of the loop returns the next 8-bit group that has at least one set bit.

But with the new for_each_set_clump the clump size can be different from 8 bits. Moreover, the clump can be split at word boundary in situations where word size is not multiple of clump size. Following are examples showing the working of new macro for clump sizes of 24 bits and 6 bits.

Example 1:
clump size: 24 bits, Number of clumps (or ports): 10
bitmap stores the bit information from where successive clumps are retrieved.

     /* bitmap memory region */
        0x00aa0000ff000000;  /* Most significant bits */
        0xaaaaaa0000ff0000;
        0x000000aa000000aa;
        0xbbbbabcdeffedcba;  /* Least significant bits */

Different iterations of for_each_set_clump:-
'offset' is the bit position and 'clump' is the 24 bit clump from the above bitmap.

Iteration first:        offset: 0 clump: 0xfedcba
Iteration second:       offset: 24 clump: 0xabcdef
Iteration third:        offset: 48 clump: 0xaabbbb
Iteration fourth:       offset: 96 clump: 0xaa
Iteration fifth:        offset: 144 clump: 0xff
Iteration sixth:        offset: 168 clump: 0xaaaaaa
Iteration seventh:      offset: 216 clump: 0xff

Loop breaks because in the end the remaining bits (0x00aa) size was less than clump size of 24 bits.

In above example it can be seen that in iteration third, the 24 bit clump that was retrieved was split between bitmap[0] and bitmap[1]. This example also shows that 24 bit zeroes if present in between, were skipped (preserving the previous for_each_set_macro8 behaviour).

Example 2:
clump size = 6 bits, Number of clumps (or ports) = 3.

     /* bitmap memory region */
        0x00aa0000ff000000;  /* Most significant bits */
        0xaaaaaa0000ff0000;
        0x0f00000000000000;
        0x0000000000000ac0;  /* Least significant bits */

Different iterations of for_each_set_clump:
'offset' is the bit position and 'clump' is the 6 bit clump from the above bitmap.
Iteration first: offset: 6 clump: 0x2b
Loop breaks because 6 * 3 = 18 bits traversed in bitmap. Here 6 * 3 is clump size * no. of clumps.

GCC gives warning in bitmap_set_value(): https://godbolt.org/z/rjx34r
Add explicit check to see if the value being written into the bitmap does not fall outside the bitmap.
The situation that it is falling outside would never be possible in the code because the boundaries are required to be correct before the function is called. The responsibility is on the caller for ensuring the boundaries are correct.
The code change is simply to silence the GCC warning messages because GCC is not aware that the boundaries have already been checked. As such, we're better off using __builtin_unreachable() here because we can avoid the latency of the conditional check entirely.

Syed Nayyar Waris (5):
clump_bits: Introduce the for_each_set_clump macro lib/test_bitmap.c: Add for_each_set_clump test cases
gpio: thunderx: Utilize for_each_set_clump macro
gpio: xilinx: Utilize generic bitmap_get_value and _set_value
gpio: xilinx: Add extra check if sum of widths exceed 64

 drivers/gpio/clump_bits.h    | 101 ++++++++++++++++++++++++
 drivers/gpio/gpio-thunderx.c |  12 ++-
 drivers/gpio/gpio-xilinx.c   |  72 ++++++++++--------
 lib/test_bitmap.c            | 144 +++++++++++++++++++++++++++++++++++
 4 files changed, 292 insertions(+), 37 deletions(-)
 create mode 100644 drivers/gpio/clump_bits.h

base-commit: bbe2ba04c5a92a49db8a42c850a5a2f6481e47eb

Signature · 1 line
2.29.0