Re: [PATCH v2 2/5] lib/bitrev: Introduce GENERIC_BITREVERSE

[Jinjie Ruan <ruanjinjie@huawei.com>]

On 5/7/2026 1:52 AM, Yury Norov wrote:
> The generic bit reversal implementation is controlled by
> !HAVE_ARCH_BITREVERSE. This makes it difficult for architectures to
> provide a hardware-accelerated implementation while being able to
> fall back to the generic version if needed.
> 
> This patch adds GENERIC_BITREVERSE, so bitreverse API is controlled by
> BITREVERSE, GENERIC_BITREVERSE and HAVE_ARCH_BITREVERSE options. The
> relationship between them is described as follows:
> 
>  - BITREVERSE is selected by user code; it's required to generate the API;
>  - Architectures may select HAVE_ARCH_BITREVERSE and provide an arch
>    implementation in arch/$(ARCH)/include/asm/bitrev.h.
>  - if HAVE_ARCH_BITREVERSE isn't set, BITREVERSE selects GENERIC_BITREVERSE;
>  - if GENERIC_BITREVERSE is set and HAVE_ARCH_BITREVERSE is not, the kernel
>    provides generic implementation only, and wires bitrevXX() to it.
>  - if HAVE_ARCH_BITREVERSE is set and GENERIC_BITREVERSE is not, the arch
>    code provides __arch_bitrevXX(), and it is wired to bitrevXX();
>  - if both GENERIC_BITREVERSE and HAVE_ARCH_BITREVERSE are selected, the kernel
>    generates generic___bitrev(), but wires bitrev() to the __arch_bitrev().
> 
> The last option allows architectures to use generic___bitrev() as a
> fallback option.
> 
> Drivers and core code should never select GENERIC_BITREVERSE or
> HAVE_ARCH_BITREVERSE explicitly.
> 
> Architectures that require generic bitreverse API as a fallback should
> explicitly enable GENERIC_BITREVERSE together with HAVE_ARCH_BITREVERSE.
> 
> Signed-off-by: Yury Norov <ynorov@nvidia.com>
> ---
>  lib/Kconfig  | 12 ++++++++++++
>  lib/Makefile |  2 +-
>  lib/bitrev.c |  3 ---
>  3 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/Kconfig b/lib/Kconfig
> index d8e7e89ae320..a33988adfaa3 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -54,6 +54,7 @@ config PACKING_KUNIT_TEST
>  
>  config BITREVERSE
>  	tristate
> +	select GENERIC_BITREVERSE if !HAVE_ARCH_BITREVERSE
>  
>  config HAVE_ARCH_BITREVERSE
>  	bool
> @@ -63,6 +64,17 @@ config HAVE_ARCH_BITREVERSE
>  	  This option enables the use of hardware bit-reversal instructions on
>  	  architectures which support such operations.
>  
> +config GENERIC_BITREVERSE
> +	tristate
> +	depends on BITREVERSE
> +	help
> +	  Generic bit reversal implementation. Drivers should never enable
> +	  it explicitly. Instead, enable BITREVERSE.


The later riscv implementation force GENERIC_BITREVERSE even when
HAVE_ARCH_BITREVERSE=y but triggers a Kconfig unmet direct dependency
warning as below:

warning: (RISCV) selects GENERIC_BITREVERSE which has unmet direct
dependencies (BITREVERSE)

This happens because select ignores depends on clauses and can force a
tristate symbol to y even when its dependency BITREVERSE is only =m. The
warning is a symptom of an invalid dependency chain.

Link:
https://lore.kernel.org/all/20260506214943.1AAE8C2BCB0@smtp.kernel.org/

> +
> +	  Architectures may want to select it as a fall-back option for
> +	  HAVE_ARCH_BITREVERSE, when the hardware-accelerated bit reverse
> +	  instruction set is optional, like RISC-V ZBKB extension.
> +
>  config ARCH_HAS_STRNCPY_FROM_USER
>  	bool
>  
> diff --git a/lib/Makefile b/lib/Makefile
> index f33a24bf1c19..23e07d19d01c 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -145,7 +145,7 @@ obj-$(CONFIG_DEBUG_PREEMPT) += smp_processor_id.o
>  obj-$(CONFIG_LIST_HARDENED) += list_debug.o
>  obj-$(CONFIG_DEBUG_OBJECTS) += debugobjects.o
>  
> -obj-$(CONFIG_BITREVERSE) += bitrev.o
> +obj-$(CONFIG_GENERIC_BITREVERSE) += bitrev.o
>  obj-$(CONFIG_LINEAR_RANGES) += linear_ranges.o
>  obj-$(CONFIG_PACKING)	+= packing.o
>  obj-$(CONFIG_PACKING_KUNIT_TEST) += packing_test.o
> diff --git a/lib/bitrev.c b/lib/bitrev.c
> index 81b56e0a7f32..05088231f31f 100644
> --- a/lib/bitrev.c
> +++ b/lib/bitrev.c
> @@ -1,5 +1,4 @@
>  // SPDX-License-Identifier: GPL-2.0-only
> -#ifndef CONFIG_HAVE_ARCH_BITREVERSE
>  #include <linux/types.h>
>  #include <linux/module.h>
>  #include <linux/bitrev.h>
> @@ -43,5 +42,3 @@ const u8 byte_rev_table[256] = {
>  	0x1f, 0x9f, 0x5f, 0xdf, 0x3f, 0xbf, 0x7f, 0xff,
>  };
>  EXPORT_SYMBOL_GPL(byte_rev_table);
> -
> -#endif /* CONFIG_HAVE_ARCH_BITREVERSE */