diff mbox series

[v5,1/7] bits: split the definition of the asm and non-asm GENMASK()

Message ID 20250306-fixed-type-genmasks-v5-1-b443e9dcba63@wanadoo.fr (mailing list archive)
State New
Headers show
Series bits: Fixed-type GENMASK()/BIT() | expand

Commit Message

Vincent Mailhol via B4 Relay March 6, 2025, 11:29 a.m. UTC 
From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

In an upcoming change, GENMASK() and its friends will indirectly
depend on sizeof() which is not available in asm.

Instead of adding further complexity to __GENMASK() to make it work
for both asm and non asm, just split the definition of the two
variants.

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
Changelog:

  v4 -> v5:

    - Use tab indentations instead of single space to separate the
      macro name from its body.

  v3 -> v4:

    - New patch in the series
---
 include/linux/bits.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Andy Shevchenko March 6, 2025, 1:05 p.m. UTC | #1 
On Thu, Mar 06, 2025 at 08:29:52PM +0900, Vincent Mailhol via B4 Relay wrote:
> From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> 
> In an upcoming change, GENMASK() and its friends will indirectly
> depend on sizeof() which is not available in asm.
> 
> Instead of adding further complexity to __GENMASK() to make it work
> for both asm and non asm, just split the definition of the two
> variants.

...

> -/*
> - * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
> - * disable the input check if that is the case.
> - */

I believe this comment is still valid...

> +#else /* defined(__ASSEMBLY__) */


...here.

Otherwise justify its removal in the commit message.

> +#define GENMASK(h, l)		__GENMASK(h, l)
> +#define GENMASK_ULL(h, l)	__GENMASK_ULL(h, l)
> +
> +#endif /* !defined(__ASSEMBLY__) */
Lucas De Marchi March 6, 2025, 2:34 p.m. UTC | #2 
On Thu, Mar 06, 2025 at 08:29:52PM +0900, Vincent Mailhol via B4 Relay wrote:
>From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
>
>In an upcoming change, GENMASK() and its friends will indirectly
>depend on sizeof() which is not available in asm.
>
>Instead of adding further complexity to __GENMASK() to make it work
>for both asm and non asm, just split the definition of the two
>variants.
>
>Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
>---
>Changelog:
>
>  v4 -> v5:
>
>    - Use tab indentations instead of single space to separate the
>      macro name from its body.
>
>  v3 -> v4:
>
>    - New patch in the series
>---
> include/linux/bits.h | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
>diff --git a/include/linux/bits.h b/include/linux/bits.h
>index 14fd0ca9a6cd17339dd2f69e449558312a8a001b..4819cbe7bd48fbae796fc6005c9f92b1c93a034c 100644
>--- a/include/linux/bits.h
>+++ b/include/linux/bits.h
>@@ -19,23 +19,17 @@
>  * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
>  */
> #if !defined(__ASSEMBLY__)
>+
> #include <linux/build_bug.h>
> #include <linux/compiler.h>
>+
> #define GENMASK_INPUT_CHECK(h, l) BUILD_BUG_ON_ZERO(const_true((l) > (h)))
>-#else
>-/*
>- * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
>- * disable the input check if that is the case.
>- */

it seems we now have 1 inconsistency that we comment why
GENMASK_U128() is not available in asm, but we don't comment why
GENMASK_INPUT_CHECK() is not available there. Maybe move this comment on
top of GENMASK_INPUT_CHECK().

Anyway,

	Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

thanks for picking up this series.

Lucas De Marchi

>-#define GENMASK_INPUT_CHECK(h, l) 0
>-#endif
>
> #define GENMASK(h, l) \
> 	(GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
> #define GENMASK_ULL(h, l) \
> 	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
>
>-#if !defined(__ASSEMBLY__)
> /*
>  * Missing asm support
>  *
>@@ -48,6 +42,12 @@
>  */
> #define GENMASK_U128(h, l) \
> 	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_U128(h, l))
>-#endif
>+
>+#else /* defined(__ASSEMBLY__) */
>+
>+#define GENMASK(h, l)		__GENMASK(h, l)
>+#define GENMASK_ULL(h, l)	__GENMASK_ULL(h, l)
>+
>+#endif /* !defined(__ASSEMBLY__) */
>
> #endif	/* __LINUX_BITS_H */
>
>-- 
>2.45.3
>
>
diff mbox series

Patch

diff --git a/include/linux/bits.h b/include/linux/bits.h
index 14fd0ca9a6cd17339dd2f69e449558312a8a001b..4819cbe7bd48fbae796fc6005c9f92b1c93a034c 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -19,23 +19,17 @@ 
  * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
  */
 #if !defined(__ASSEMBLY__)
+
 #include <linux/build_bug.h>
 #include <linux/compiler.h>
+
 #define GENMASK_INPUT_CHECK(h, l) BUILD_BUG_ON_ZERO(const_true((l) > (h)))
-#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
-#endif
 
 #define GENMASK(h, l) \
 	(GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
 #define GENMASK_ULL(h, l) \
 	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
 
-#if !defined(__ASSEMBLY__)
 /*
  * Missing asm support
  *
@@ -48,6 +42,12 @@ 
  */
 #define GENMASK_U128(h, l) \
 	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_U128(h, l))
-#endif
+
+#else /* defined(__ASSEMBLY__) */
+
+#define GENMASK(h, l)		__GENMASK(h, l)
+#define GENMASK_ULL(h, l)	__GENMASK_ULL(h, l)
+
+#endif /* !defined(__ASSEMBLY__) */
 
 #endif	/* __LINUX_BITS_H */