diff mbox series

[v3,2/3] bits: Introduce fixed-type BIT

Message ID 20240208074521.577076-3-lucas.demarchi@intel.com
State New
Headers
Series Fixed-type GENMASK/BIT |

Commit Message

Lucas De Marchi Feb. 8, 2024, 7:45 a.m. UTC 
  Implement fixed-type BIT() to help drivers add stricter checks, like was
done for GENMASK.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
---
 include/linux/bits.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Andi Shyti Feb. 8, 2024, 8:04 p.m. UTC | #1 
Hi Lucas,

looks good, just one idea...

..

> +#define BIT_U8(b)		((u8)(BIT_INPUT_CHECK(u8, b) + BIT(b)))
> +#define BIT_U16(b)		((u16)(BIT_INPUT_CHECK(u16, b) + BIT(b)))
> +#define BIT_U32(b)		((u32)(BIT_INPUT_CHECK(u32, b) + BIT(b)))
> +#define BIT_U64(b)		((u64)(BIT_INPUT_CHECK(u64, b) + BIT(b)))

considering that BIT defines are always referred to unsigned
types, I would just call them

#define BIT8
#define BIT16
#define BIT32
#define BIT64

what do you think?

Andi
Lucas De Marchi Feb. 8, 2024, 9:17 p.m. UTC | #2 
On Thu, Feb 08, 2024 at 09:04:45PM +0100, Andi Shyti wrote:
>Hi Lucas,
>
>looks good, just one idea...
>
>...
>
>> +#define BIT_U8(b)		((u8)(BIT_INPUT_CHECK(u8, b) + BIT(b)))
>> +#define BIT_U16(b)		((u16)(BIT_INPUT_CHECK(u16, b) + BIT(b)))
>> +#define BIT_U32(b)		((u32)(BIT_INPUT_CHECK(u32, b) + BIT(b)))
>> +#define BIT_U64(b)		((u64)(BIT_INPUT_CHECK(u64, b) + BIT(b)))
>
>considering that BIT defines are always referred to unsigned
>types, I would just call them
>
>#define BIT8
>#define BIT16
>#define BIT32
>#define BIT64
>
>what do you think?

it will clash with defines from other headers and not match the ones for
GENMASK  so I prefer it the other way.

thanks
Lucas De Marchi

>
>Andi
Jani Nikula Feb. 9, 2024, 8:01 a.m. UTC | #3 
On Thu, 08 Feb 2024, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Thu, Feb 08, 2024 at 09:04:45PM +0100, Andi Shyti wrote:
>>Hi Lucas,
>>
>>looks good, just one idea...
>>
>>...
>>
>>> +#define BIT_U8(b)		((u8)(BIT_INPUT_CHECK(u8, b) + BIT(b)))
>>> +#define BIT_U16(b)		((u16)(BIT_INPUT_CHECK(u16, b) + BIT(b)))
>>> +#define BIT_U32(b)		((u32)(BIT_INPUT_CHECK(u32, b) + BIT(b)))
>>> +#define BIT_U64(b)		((u64)(BIT_INPUT_CHECK(u64, b) + BIT(b)))
>>
>>considering that BIT defines are always referred to unsigned
>>types, I would just call them
>>
>>#define BIT8
>>#define BIT16
>>#define BIT32
>>#define BIT64
>>
>>what do you think?
>
> it will clash with defines from other headers and not match the ones for
> GENMASK  so I prefer it the other way.

Agreed.
Yury Norov Feb. 9, 2024, 4:53 p.m. UTC | #4 
On Wed, Feb 07, 2024 at 11:45:20PM -0800, Lucas De Marchi wrote:
> Implement fixed-type BIT() to help drivers add stricter checks, like was
> done for GENMASK.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Acked-by: Jani Nikula <jani.nikula@intel.com>

So I get v1 from Jan.23 in my mailbox, and this one is v3. Did I miss
a v2? Anyways, please bear my reviewed-by from v1 for this patch.

Thanks,
Yury

> ---
>  include/linux/bits.h | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/include/linux/bits.h b/include/linux/bits.h
> index bd56f32de44e..811846ce110e 100644
> --- a/include/linux/bits.h
> +++ b/include/linux/bits.h
> @@ -24,12 +24,16 @@
>  #define GENMASK_INPUT_CHECK(h, l) \
>  	(BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
>  		__is_constexpr((l) > (h)), (l) > (h), 0)))
> +#define BIT_INPUT_CHECK(type, b) \
> +	((BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
> +		__is_constexpr(b), (b) >= BITS_PER_TYPE(type), 0))))
>  #else
>  /*
>   * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
>   * disable the input check if that is the case.
>   */
>  #define GENMASK_INPUT_CHECK(h, l) 0
> +#define BIT_INPUT_CHECK(type, b) 0
>  #endif
>  
>  /*
> @@ -54,4 +58,17 @@
>  #define GENMASK_U32(h, l)	__GENMASK(u32, h, l)
>  #define GENMASK_U64(h, l)	__GENMASK(u64, h, l)
>  
> +/*
> + * Fixed-type variants of BIT(), with additional checks like __GENMASK().  The
> + * following examples generate compiler warnings due to shift-count-overflow:
> + *
> + * - BIT_U8(8)
> + * - BIT_U32(-1)
> + * - BIT_U32(40)
> + */
> +#define BIT_U8(b)		((u8)(BIT_INPUT_CHECK(u8, b) + BIT(b)))
> +#define BIT_U16(b)		((u16)(BIT_INPUT_CHECK(u16, b) + BIT(b)))
> +#define BIT_U32(b)		((u32)(BIT_INPUT_CHECK(u32, b) + BIT(b)))
> +#define BIT_U64(b)		((u64)(BIT_INPUT_CHECK(u64, b) + BIT(b)))
> +
>  #endif	/* __LINUX_BITS_H */
> -- 
> 2.43.0
David Laight Feb. 10, 2024, 2:22 p.m. UTC | #5 
..
> >> +#define BIT_U8(b)		((u8)(BIT_INPUT_CHECK(u8, b) + BIT(b)))
> >> +#define BIT_U16(b)		((u16)(BIT_INPUT_CHECK(u16, b) + BIT(b)))
> >> +#define BIT_U32(b)		((u32)(BIT_INPUT_CHECK(u32, b) + BIT(b)))
> >> +#define BIT_U64(b)		((u64)(BIT_INPUT_CHECK(u64, b) + BIT(b)))
> >
> >considering that BIT defines are always referred to unsigned
> >types, I would just call them

Except that pretty much as soon as you breath on them
the u8 and u16 types get converted to int.
If you want them to be an unsigned type then you need
to cast them to (unsigned int).

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Lucas De Marchi Feb. 20, 2024, 5:13 a.m. UTC | #6 
On Fri, Feb 09, 2024 at 08:53:25AM -0800, Yury Norov wrote:
>On Wed, Feb 07, 2024 at 11:45:20PM -0800, Lucas De Marchi wrote:
>> Implement fixed-type BIT() to help drivers add stricter checks, like was
>> done for GENMASK.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> Acked-by: Jani Nikula <jani.nikula@intel.com>
>
>So I get v1 from Jan.23 in my mailbox, and this one is v3. Did I miss
>a v2? Anyways, please bear my reviewed-by from v1 for this patch.

Jan 23 was actually the v2 and I missed the subject prefix.

My understanding was that you were going to apply this through some
bitmap tree, but checking MAINTAINERS now it seems there's no git tree
associated.  So I will just add your r-b and merge this through
drm-xe.

thanks
Lucas De Marchi

>
>Thanks,
>Yury
>
>> ---
>>  include/linux/bits.h | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/include/linux/bits.h b/include/linux/bits.h
>> index bd56f32de44e..811846ce110e 100644
>> --- a/include/linux/bits.h
>> +++ b/include/linux/bits.h
>> @@ -24,12 +24,16 @@
>>  #define GENMASK_INPUT_CHECK(h, l) \
>>  	(BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
>>  		__is_constexpr((l) > (h)), (l) > (h), 0)))
>> +#define BIT_INPUT_CHECK(type, b) \
>> +	((BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
>> +		__is_constexpr(b), (b) >= BITS_PER_TYPE(type), 0))))
>>  #else
>>  /*
>>   * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
>>   * disable the input check if that is the case.
>>   */
>>  #define GENMASK_INPUT_CHECK(h, l) 0
>> +#define BIT_INPUT_CHECK(type, b) 0
>>  #endif
>>
>>  /*
>> @@ -54,4 +58,17 @@
>>  #define GENMASK_U32(h, l)	__GENMASK(u32, h, l)
>>  #define GENMASK_U64(h, l)	__GENMASK(u64, h, l)
>>
>> +/*
>> + * Fixed-type variants of BIT(), with additional checks like __GENMASK().  The
>> + * following examples generate compiler warnings due to shift-count-overflow:
>> + *
>> + * - BIT_U8(8)
>> + * - BIT_U32(-1)
>> + * - BIT_U32(40)
>> + */
>> +#define BIT_U8(b)		((u8)(BIT_INPUT_CHECK(u8, b) + BIT(b)))
>> +#define BIT_U16(b)		((u16)(BIT_INPUT_CHECK(u16, b) + BIT(b)))
>> +#define BIT_U32(b)		((u32)(BIT_INPUT_CHECK(u32, b) + BIT(b)))
>> +#define BIT_U64(b)		((u64)(BIT_INPUT_CHECK(u64, b) + BIT(b)))
>> +
>>  #endif	/* __LINUX_BITS_H */
>> --
>> 2.43.0
Yury Norov Feb. 22, 2024, 2:51 p.m. UTC | #7 
On Mon, Feb 19, 2024 at 11:13:57PM -0600, Lucas De Marchi wrote:
> On Fri, Feb 09, 2024 at 08:53:25AM -0800, Yury Norov wrote:
> > On Wed, Feb 07, 2024 at 11:45:20PM -0800, Lucas De Marchi wrote:
> > > Implement fixed-type BIT() to help drivers add stricter checks, like was
> > > done for GENMASK.
> > > 
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > Acked-by: Jani Nikula <jani.nikula@intel.com>
> > 
> > So I get v1 from Jan.23 in my mailbox, and this one is v3. Did I miss
> > a v2? Anyways, please bear my reviewed-by from v1 for this patch.
> 
> Jan 23 was actually the v2 and I missed the subject prefix.
> 
> My understanding was that you were going to apply this through some
> bitmap tree, but checking MAINTAINERS now it seems there's no git tree
> associated.  So I will just add your r-b and merge this through
> drm-xe.

I've got a bitmap-related branch. I can move this series in there if
you prefer. At your discretion.

https://github.com/norov/linux/tree/bitmap_for_next

Thanks,
Yury
Lucas De Marchi Feb. 22, 2024, 4:35 p.m. UTC | #8 
On Thu, Feb 22, 2024 at 06:51:52AM -0800, Yury Norov wrote:
>On Mon, Feb 19, 2024 at 11:13:57PM -0600, Lucas De Marchi wrote:
>> On Fri, Feb 09, 2024 at 08:53:25AM -0800, Yury Norov wrote:
>> > On Wed, Feb 07, 2024 at 11:45:20PM -0800, Lucas De Marchi wrote:
>> > > Implement fixed-type BIT() to help drivers add stricter checks, like was
>> > > done for GENMASK.
>> > >
>> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> > > Acked-by: Jani Nikula <jani.nikula@intel.com>
>> >
>> > So I get v1 from Jan.23 in my mailbox, and this one is v3. Did I miss
>> > a v2? Anyways, please bear my reviewed-by from v1 for this patch.
>>
>> Jan 23 was actually the v2 and I missed the subject prefix.
>>
>> My understanding was that you were going to apply this through some
>> bitmap tree, but checking MAINTAINERS now it seems there's no git tree
>> associated.  So I will just add your r-b and merge this through
>> drm-xe.
>
>I've got a bitmap-related branch. I can move this series in there if
>you prefer. At your discretion.
>
>https://github.com/norov/linux/tree/bitmap_for_next

yeah, sounds good.

thanks
Lucas De Marchi

>
>Thanks,
>Yury
diff mbox series

Patch

diff --git a/include/linux/bits.h b/include/linux/bits.h
index bd56f32de44e..811846ce110e 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -24,12 +24,16 @@ 
 #define GENMASK_INPUT_CHECK(h, l) \
 	(BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
 		__is_constexpr((l) > (h)), (l) > (h), 0)))
+#define BIT_INPUT_CHECK(type, b) \
+	((BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
+		__is_constexpr(b), (b) >= BITS_PER_TYPE(type), 0))))
 #else
 /*
  * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
  * disable the input check if that is the case.
  */
 #define GENMASK_INPUT_CHECK(h, l) 0
+#define BIT_INPUT_CHECK(type, b) 0
 #endif
 
 /*
@@ -54,4 +58,17 @@ 
 #define GENMASK_U32(h, l)	__GENMASK(u32, h, l)
 #define GENMASK_U64(h, l)	__GENMASK(u64, h, l)
 
+/*
+ * Fixed-type variants of BIT(), with additional checks like __GENMASK().  The
+ * following examples generate compiler warnings due to shift-count-overflow:
+ *
+ * - BIT_U8(8)
+ * - BIT_U32(-1)
+ * - BIT_U32(40)
+ */
+#define BIT_U8(b)		((u8)(BIT_INPUT_CHECK(u8, b) + BIT(b)))
+#define BIT_U16(b)		((u16)(BIT_INPUT_CHECK(u16, b) + BIT(b)))
+#define BIT_U32(b)		((u32)(BIT_INPUT_CHECK(u32, b) + BIT(b)))
+#define BIT_U64(b)		((u64)(BIT_INPUT_CHECK(u64, b) + BIT(b)))
+
 #endif	/* __LINUX_BITS_H */