Re: [PATCH v5 2/3] bitops: Define generic __bitrev8/16/32 for reuse

[Nathan Chancellor <nathan@kernel.org>]

On Wed, Apr 29, 2026 at 09:47:12PM -0400, Yury Norov wrote:
> + networking maintainers
> 
> On Wed, Apr 29, 2026 at 01:29:22PM -0700, Nathan Chancellor wrote:
> > Hi Jinjie,
> > 
> > On Tue, Apr 21, 2026 at 09:07:51PM +0800, Jinjie Ruan wrote:
> > > Define generic __bitrev8/16/32 using the implementation
> > > in <linux/bitrev.h>, so they can be reused in <asm/bitrev.h>,
> > > such as RISCV.
> > > 
> > > Reviewed-by: Yury Norov <ynorov@nvidia.com>
> > > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> > > ---
> > >  include/asm-generic/bitops/__bitrev.h | 25 +++++++++++++++++++++++++
> > >  include/linux/bitrev.h                | 20 ++++----------------
> > >  2 files changed, 29 insertions(+), 16 deletions(-)
> > >  create mode 100644 include/asm-generic/bitops/__bitrev.h
> > > 
> > > diff --git a/include/asm-generic/bitops/__bitrev.h b/include/asm-generic/bitops/__bitrev.h
> > > new file mode 100644
> > > index 000000000000..f06af929678d
> > > --- /dev/null
> > > +++ b/include/asm-generic/bitops/__bitrev.h
> > > @@ -0,0 +1,25 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +#ifndef _ASM_GENERIC_BITOPS___BITREV_H_
> > > +#define _ASM_GENERIC_BITOPS___BITREV_H_
> > > +
> > > +#ifdef CONFIG_GENERIC_BITREVERSE
> > 
> > The dependencies of this symbol seem insufficient, as I can trigger a
> > build failure on next-20260429 like so:
> > 
> >   $ make -skj"$(nproc)" ARCH=s390 CROSS_COMPILE=s390-linux- mrproper tinyconfig fs/select.o
> >   In file included from include/linux/crc32.h:6,
> >                    from include/linux/etherdevice.h:23,
> >                    from include/linux/if_vlan.h:11,
> >                    from include/linux/filter.h:21,
> >                    from include/net/xdp.h:10,
> >                    from include/net/busy_poll.h:19,
> >                    from fs/select.c:33:
> >   include/linux/etherdevice.h: In function 'eth_hw_addr_crc':
> >   include/linux/bitrev.h:16:20: error: implicit declaration of function 'generic___bitrev32' [-Wimplicit-function-declaration]
> >      16 | #define __bitrev32 generic___bitrev32
> >         |                    ^~~~~~~~~~~~~~~~~~
> >   include/linux/bitrev.h:67:9: note: in expansion of macro '__bitrev32'
> >      67 |         __bitrev32(__x);                                \
> >         |         ^~~~~~~~~~
> >   include/linux/crc32.h:107:36: note: in expansion of macro 'bitrev32'
> >     107 | #define ether_crc(length, data)    bitrev32(crc32_le(~0, data, length))
> >         |                                    ^~~~~~~~
> >   include/linux/etherdevice.h:292:16: note: in expansion of macro 'ether_crc'
> >     292 |         return ether_crc(ETH_ALEN, ha->addr);
> >         |                ^~~~~~~~~
> >   make[5]: *** [scripts/Makefile.build:289: fs/select.o] Error 1
> >   ...
> > 
> >   $ scripts/config -s BITREVERSE
> >   undef
> > 
> >   $ rg BITREVERSE .config
> 
> Confirm the same for x86 tinyconfig.
> 
> The problem is that the patch makes generic bitrevXX() conditional
> on CONFIG_GENERIC_BITREVERSE, while before they were conditional on
> !CONFIG_HAVE_ARCH_BITREVERSE. So if you don't have arch bitreverse(),
> and dont' enable BITREVERSE, the generic implementation is not defined
> now.
> 
> Luckily, the only user of bitrev() in unconditionally compiled objects
> is CRC32, and it's not needed if CRC32 is disabled.
> 
> This is the minimal working fix for me. Please let me know what do you
> think. I can prepend Jinjie's pathes in my tree with it if it's OK for
> you.

I will run it through my full build matrix overnight and report back
with any problems noticed.

> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> index df8f88f63a70..245b206dd38b 100644
> --- a/include/linux/etherdevice.h
> +++ b/include/linux/etherdevice.h
> @@ -20,7 +20,9 @@
>  #include <linux/if_ether.h>
>  #include <linux/netdevice.h>
>  #include <linux/random.h>
> +#ifdef CONFIG_CRCC32

Initial testing shows that this should be

  #ifdef CONFIG_CRC32

here and below though.

>  #include <linux/crc32.h>
> +#endif
>  #include <linux/unaligned.h>
>  #include <asm/bitsperlong.h>
>  
> @@ -281,6 +283,7 @@ static inline void eth_hw_addr_random(struct net_device *dev)
>  	dev->addr_assign_type = NET_ADDR_RANDOM;
>  }
>  
> +#ifdef CONFIG_CRCC32
>  /**
>   * eth_hw_addr_crc - Calculate CRC from netdev_hw_addr
>   * @ha: pointer to hardware address
> @@ -291,6 +294,7 @@ static inline u32 eth_hw_addr_crc(struct netdev_hw_addr *ha)
>  {
>  	return ether_crc(ETH_ALEN, ha->addr);
>  }
> +#endif
>  
>  /**
>   * ether_addr_copy - Copy an Ethernet address

-- 
Cheers,
Nathan