Re: [dpdk-dev] [RFC PATCH 1/6] eal: introduce macros for getting value for bit

[Thomas Monjalon <thomas@monjalon.net>]

18/06/2020 14:16, Parav Pandit:
> From: Parav Pandit
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 15/06/2020 21:33, Gaëtan Rivet:
> > > > On 10/06/20 17:17 +0000, Parav Pandit wrote:
> > > > > There are several drivers which duplicate bit generation macro.
> > > > > Introduce a generic bit macros so that such drivers avoid
> > > > > redefining same in multiple drivers.
> > > > >
> > > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > > > ---
> > > [...]
> > > > > --- /dev/null
> > > > > +++ b/lib/librte_eal/include/rte_bits.h
> > > > > @@ -0,0 +1,10 @@
> > > > > +/* SPDX-License-Identifier: BSD-3-Clause
> > > > > + * Copyright 2020 Mellanox Technologies, Ltd.
> > > > > + */
> > > > > +
> > > > > +#ifndef _RTE_BITS_H_
> > > > > +#define _RTE_BITS_H_
> > > > > +
> > > > > +#define RTE_BIT(bit_num)	(1UL << (bit_num))
> > > >                            ^ The tab here should be replaced by a space.
> > > > > +
> > > > > +#endif
> > > >
> > > > I'm not sure this kind of macro is needed, but if multiple drivers
> > > > are using the patterns let's say ok.
> > > >
> > Yes. we certainly need it. Currently BIT() macro is used at 3000+ locations and
> > defined in 10+ drivers.
> > Once we have this macro, it can gradually be replaced.
> > 
> > > > However I don't think it needs its own header. Would it be ok in
> > > > lib/librte_eal/include/rte_common.h for example?
> > >
> > > If we want to reuse an existing file, it could be
> > > lib/librte_eal/include/rte_bitops.h
> > >
> > o.k.
> > I will rename file from rte_bits.h to rte_bitops.h More such macros will be
> > available here to avoid redefinitions in drivers.
> 
> I see that rte_bitops.h already exist and this new definition doesn't fit well in the rte_bitops.h.
> The intent of the rte_bitops.h is to have generic mostly macros to replace current duplication of:
> 
> Such as
> BIT()
> BIT_ULL()
> BITS_PER_BYTE()
> BITS_TO_LONGS()
> 
> They do not fit well in rte_bitops.h which works on the 'bitmap'.

Current bitops functions are getting and setting a bit.
I don't undertand why you say "works on the bitmap".
In my opinion, bit definition fits in this include file.