| Message ID | 20230509051403.2748545-4-lucas.demarchi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixed-width mask/bit helpers | expand |
On Mon, 08 May 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote: > Convert the REG_* macros from i915_reg_defs.h to use the new macros > defined in linux/bits.h. This is just to help on the implementation > of the new macros and not intended to be applied. This drops a number of build time input checks as well as casts to the specified types. BR, Jani. > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/i915_reg_defs.h | 28 +++++----------------------- > 1 file changed, 5 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h > index 622d603080f9..61fbb8d62b25 100644 > --- a/drivers/gpu/drm/i915/i915_reg_defs.h > +++ b/drivers/gpu/drm/i915/i915_reg_defs.h > @@ -17,10 +17,7 @@ > * > * @return: Value with bit @__n set. > */ > -#define REG_BIT(__n) \ > - ((u32)(BIT(__n) + \ > - BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \ > - ((__n) < 0 || (__n) > 31)))) > +#define REG_BIT(__n) BIT_U32(__n) > > /** > * REG_BIT8() - Prepare a u8 bit value > @@ -30,10 +27,7 @@ > * > * @return: Value with bit @__n set. > */ > -#define REG_BIT8(__n) \ > - ((u8)(BIT(__n) + \ > - BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \ > - ((__n) < 0 || (__n) > 7)))) > +#define REG_BIT8(__n) BIT_U8(__n) > > /** > * REG_GENMASK() - Prepare a continuous u32 bitmask > @@ -44,11 +38,7 @@ > * > * @return: Continuous bitmask from @__high to @__low, inclusive. > */ > -#define REG_GENMASK(__high, __low) \ > - ((u32)(GENMASK(__high, __low) + \ > - BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \ > - __is_constexpr(__low) && \ > - ((__low) < 0 || (__high) > 31 || (__low) > (__high))))) > +#define REG_GENMASK(__high, __low) GENMASK_U32(__high, __low) > > /** > * REG_GENMASK64() - Prepare a continuous u64 bitmask > @@ -59,11 +49,7 @@ > * > * @return: Continuous bitmask from @__high to @__low, inclusive. > */ > -#define REG_GENMASK64(__high, __low) \ > - ((u64)(GENMASK_ULL(__high, __low) + \ > - BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \ > - __is_constexpr(__low) && \ > - ((__low) < 0 || (__high) > 63 || (__low) > (__high))))) > +#define REG_GENMASK64(__high, __low) GENMASK_ULL(__high, __low) > > /** > * REG_GENMASK8() - Prepare a continuous u8 bitmask > @@ -74,11 +60,7 @@ > * > * @return: Continuous bitmask from @__high to @__low, inclusive. > */ > -#define REG_GENMASK8(__high, __low) \ > - ((u8)(GENMASK(__high, __low) + \ > - BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \ > - __is_constexpr(__low) && \ > - ((__low) < 0 || (__high) > 7 || (__low) > (__high))))) > +#define REG_GENMASK8(__high, __low) GENMASK_U8(__high, __low) > > /* > * Local integer constant expression version of is_power_of_2().
On Tue, May 09, 2023 at 10:57:19AM +0300, Jani Nikula wrote: >On Mon, 08 May 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote: >> Convert the REG_* macros from i915_reg_defs.h to use the new macros >> defined in linux/bits.h. This is just to help on the implementation >> of the new macros and not intended to be applied. > >This drops a number of build time input checks as well as casts to the >specified types. the explicit checks... but the checks are still there and the compiler still gives me a warning or error for using invalid values. See test program in the second patch. Example: static const u32 a2 = GENMASK_U32(32, 0); In file included from /tmp/genmask.c:2: include/linux/bits.h:47:19: warning: right shift count is negative [-Wshift-count-negative] 47 | (~U32(0) >> (32 - 1 - (h)))) | ^~ It's a warning, not an error though. Same warning for the other fixed-widths with numbers above the supported width. For negative values: In file included from include/linux/bits.h:21, from /tmp/genmask.c:2: include/linux/build_bug.h:16:51: error: negative width in bit-field ‘<anonymous>’ 16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); }))) | ^ The cast to the specified type we lose indeed. Could you give an example where those are useful in the context they are used? I debated adding them, but couldn't find a justified use of them. Lucas De Marchi > >BR, >Jani. > >> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg_defs.h | 28 +++++----------------------- >> 1 file changed, 5 insertions(+), 23 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h >> index 622d603080f9..61fbb8d62b25 100644 >> --- a/drivers/gpu/drm/i915/i915_reg_defs.h >> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h >> @@ -17,10 +17,7 @@ >> * >> * @return: Value with bit @__n set. >> */ >> -#define REG_BIT(__n) \ >> - ((u32)(BIT(__n) + \ >> - BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \ >> - ((__n) < 0 || (__n) > 31)))) >> +#define REG_BIT(__n) BIT_U32(__n) >> >> /** >> * REG_BIT8() - Prepare a u8 bit value >> @@ -30,10 +27,7 @@ >> * >> * @return: Value with bit @__n set. >> */ >> -#define REG_BIT8(__n) \ >> - ((u8)(BIT(__n) + \ >> - BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \ >> - ((__n) < 0 || (__n) > 7)))) >> +#define REG_BIT8(__n) BIT_U8(__n) >> >> /** >> * REG_GENMASK() - Prepare a continuous u32 bitmask >> @@ -44,11 +38,7 @@ >> * >> * @return: Continuous bitmask from @__high to @__low, inclusive. >> */ >> -#define REG_GENMASK(__high, __low) \ >> - ((u32)(GENMASK(__high, __low) + \ >> - BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \ >> - __is_constexpr(__low) && \ >> - ((__low) < 0 || (__high) > 31 || (__low) > (__high))))) >> +#define REG_GENMASK(__high, __low) GENMASK_U32(__high, __low) >> >> /** >> * REG_GENMASK64() - Prepare a continuous u64 bitmask >> @@ -59,11 +49,7 @@ >> * >> * @return: Continuous bitmask from @__high to @__low, inclusive. >> */ >> -#define REG_GENMASK64(__high, __low) \ >> - ((u64)(GENMASK_ULL(__high, __low) + \ >> - BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \ >> - __is_constexpr(__low) && \ >> - ((__low) < 0 || (__high) > 63 || (__low) > (__high))))) >> +#define REG_GENMASK64(__high, __low) GENMASK_ULL(__high, __low) >> >> /** >> * REG_GENMASK8() - Prepare a continuous u8 bitmask >> @@ -74,11 +60,7 @@ >> * >> * @return: Continuous bitmask from @__high to @__low, inclusive. >> */ >> -#define REG_GENMASK8(__high, __low) \ >> - ((u8)(GENMASK(__high, __low) + \ >> - BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \ >> - __is_constexpr(__low) && \ >> - ((__low) < 0 || (__high) > 7 || (__low) > (__high))))) >> +#define REG_GENMASK8(__high, __low) GENMASK_U8(__high, __low) >> >> /* >> * Local integer constant expression version of is_power_of_2(). > >-- >Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h index 622d603080f9..61fbb8d62b25 100644 --- a/drivers/gpu/drm/i915/i915_reg_defs.h +++ b/drivers/gpu/drm/i915/i915_reg_defs.h @@ -17,10 +17,7 @@ * * @return: Value with bit @__n set. */ -#define REG_BIT(__n) \ - ((u32)(BIT(__n) + \ - BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \ - ((__n) < 0 || (__n) > 31)))) +#define REG_BIT(__n) BIT_U32(__n) /** * REG_BIT8() - Prepare a u8 bit value @@ -30,10 +27,7 @@ * * @return: Value with bit @__n set. */ -#define REG_BIT8(__n) \ - ((u8)(BIT(__n) + \ - BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \ - ((__n) < 0 || (__n) > 7)))) +#define REG_BIT8(__n) BIT_U8(__n) /** * REG_GENMASK() - Prepare a continuous u32 bitmask @@ -44,11 +38,7 @@ * * @return: Continuous bitmask from @__high to @__low, inclusive. */ -#define REG_GENMASK(__high, __low) \ - ((u32)(GENMASK(__high, __low) + \ - BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \ - __is_constexpr(__low) && \ - ((__low) < 0 || (__high) > 31 || (__low) > (__high))))) +#define REG_GENMASK(__high, __low) GENMASK_U32(__high, __low) /** * REG_GENMASK64() - Prepare a continuous u64 bitmask @@ -59,11 +49,7 @@ * * @return: Continuous bitmask from @__high to @__low, inclusive. */ -#define REG_GENMASK64(__high, __low) \ - ((u64)(GENMASK_ULL(__high, __low) + \ - BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \ - __is_constexpr(__low) && \ - ((__low) < 0 || (__high) > 63 || (__low) > (__high))))) +#define REG_GENMASK64(__high, __low) GENMASK_ULL(__high, __low) /** * REG_GENMASK8() - Prepare a continuous u8 bitmask @@ -74,11 +60,7 @@ * * @return: Continuous bitmask from @__high to @__low, inclusive. */ -#define REG_GENMASK8(__high, __low) \ - ((u8)(GENMASK(__high, __low) + \ - BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \ - __is_constexpr(__low) && \ - ((__low) < 0 || (__high) > 7 || (__low) > (__high))))) +#define REG_GENMASK8(__high, __low) GENMASK_U8(__high, __low) /* * Local integer constant expression version of is_power_of_2().
Convert the REG_* macros from i915_reg_defs.h to use the new macros defined in linux/bits.h. This is just to help on the implementation of the new macros and not intended to be applied. Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> --- drivers/gpu/drm/i915/i915_reg_defs.h | 28 +++++----------------------- 1 file changed, 5 insertions(+), 23 deletions(-)