18/06/2020 14:16, Parav Pandit: > From: Parav Pandit > > From: Thomas Monjalon <tho...@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 <pa...@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.