bitops: Fix GENMASK definition for Sandbox

RFC PATCH New Vignesh R 2 participants 4 messages Dec 17, 2018 – Jan 21, 2019 Bitops API

Share & tools

Vignesh R

author

2018-12-17 13:32 UTC #0 lore raw

RFC PATCH 1/1 bitops: Fix GENMASK definition for Sandbox

In arch/sandbox/include/asm/types.h we have
Therefore for 32 bit Sandbox build BITS_PER_LONG turns out to be 32 as CONFIG_PHYS64 is not set

This messes up the current logic of GENMASK macro due to mismatch b/w size of unsigned long (64 bit) and that of BITS_PER_LONG. Fix this by using CONFIG_SANDBOX_BITS_PER_LONG which is set to 64/32 based on the host machine on which its being compiled.

Without this patch:
GENMASK(14,0) => 0x7fffffffffff
After this patch:
GENMASK(14,0) => 0x7fff

Signed-off-by Vignesh R <[email protected]>

---

I marked it as RFC because, I am not really sure if I am running Sandbox tests properly.
My host machine is x86_64 (Ubuntu 18.04). Running below command[1] with some of my yet to be upstreamed patches that use GENMASK macro shows this issue.

[1]:

$ make check

 include/linux/bitops.h | 5 +++++
 1 file changed, 5 insertions(+)

include/linux/bitops.h +5 -0

index a47f6d17bb5f..259df43fb00f 100644
@@ -21,8 +21,13 @@
  * position @h. For example
  * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
  */
+#ifdef CONFIG_SANDBOX
+#define GENMASK(h, l) \
+	(((~0UL) << (l)) & (~0UL >> (CONFIG_SANDBOX_BITS_PER_LONG - 1 - (h))))
+#else
 #define GENMASK(h, l) \
 	(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
+#endif
 
 #define GENMASK_ULL(h, l) \
 	(((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))

Signature · 1 line

2.20.1

Vignesh R

author

2019-01-09 09:21 UTC #1 lore raw

RFC PATCH 1/1 bitops: Fix GENMASK definition for Sandbox

Quoted 9 lines · Vignesh R

In arch/sandbox/include/asm/types.h we have
Therefore for 32 bit Sandbox build BITS_PER_LONG turns out to be 32 as CONFIG_PHYS64 is not set
This messes up the current logic of GENMASK macro due to mismatch b/w size of unsigned long (64 bit) and that of BITS_PER_LONG. Fix this by using CONFIG_SANDBOX_BITS_PER_LONG which is set to 64/32 based on the host machine on which its being compiled.

Quoted 8 lines

Without this patch:
GENMASK(14,0) => 0x7fffffffffff
After this patch:
GENMASK(14,0) => 0x7fff
Signed-off-by Vignesh R <[email protected]>
---

Gentle ping.. Simon, this patch is required of upcoming SPI flash layer changes. Any feedback on this patch?

Regards
Vignesh

Quoted 31 lines

[1]:

$ make check

 include/linux/bitops.h | 5 +++++
 1 file changed, 5 insertions(+)

include/linux/bitops.h +5 -0

index a47f6d17bb5f..259df43fb00f 100644
@@ -21,8 +21,13 @@
  * position @h. For example
  * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
  */
+#ifdef CONFIG_SANDBOX
+#define GENMASK(h, l) \
+	(((~0UL) << (l)) & (~0UL >> (CONFIG_SANDBOX_BITS_PER_LONG - 1 - (h))))
+#else
 #define GENMASK(h, l) \
 	(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
+#endif
 
 #define GENMASK_ULL(h, l) \
 	(((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))

Simon Glass

2019-01-10 12:56 UTC #2 lore raw

RFC PATCH 1/1 bitops: Fix GENMASK definition for Sandbox

In reply to Vignesh R

Quoted 23 lines · Vignesh R

In arch/sandbox/include/asm/types.h we have
Therefore for 32 bit Sandbox build BITS_PER_LONG turns out to be 32 as CONFIG_PHYS64 is not set
This messes up the current logic of GENMASK macro due to mismatch b/w size of unsigned long (64 bit) and that of BITS_PER_LONG. Fix this by using CONFIG_SANDBOX_BITS_PER_LONG which is set to 64/32 based on the host machine on which its being compiled.
Without this patch:
GENMASK(14,0) => 0x7fffffffffff
After this patch:
GENMASK(14,0) => 0x7fff
Signed-off-by Vignesh R <[email protected]>
---
I marked it as RFC because, I am not really sure if I am running Sandbox tests properly.
My host machine is x86_64 (Ubuntu 18.04). Running below command[1] with some of my yet to be upstreamed patches that use GENMASK macro shows this issue.

Quoted 6 lines

[1]:

$ make check

 include/linux/bitops.h | 5 +++++
 1 file changed, 5 insertions(+)

I copied Bin who introduced this Kconfig option.

It is unfortunate that we need this #ifdef but I think it is correct. I think we need some Kconfig help for SANDBOX_BITS_PER_LONG.

Reviewed-by Simon Glass <[email protected]>

Simon Glass

2019-01-21 18:40 UTC #3 lore raw

RFC PATCH 1/1 bitops: Fix GENMASK definition for Sandbox

Quoted 32 lines · Simon Glass

Quoted 23 lines · Vignesh R
In arch/sandbox/include/asm/types.h we have
Therefore for 32 bit Sandbox build BITS_PER_LONG turns out to be 32 as CONFIG_PHYS64 is not set
This messes up the current logic of GENMASK macro due to mismatch b/w size of unsigned long (64 bit) and that of BITS_PER_LONG. Fix this by using CONFIG_SANDBOX_BITS_PER_LONG which is set to 64/32 based on the host machine on which its being compiled.
Without this patch:
GENMASK(14,0) => 0x7fffffffffff
After this patch:
GENMASK(14,0) => 0x7fff
Signed-off-by Vignesh R <[email protected]>
---
I marked it as RFC because, I am not really sure if I am running Sandbox tests properly.
My host machine is x86_64 (Ubuntu 18.04). Running below command[1] with some of my yet to be upstreamed patches that use GENMASK macro shows this issue.
Quoted 6 lines
[1]:
$ make check
 include/linux/bitops.h | 5 +++++
 1 file changed, 5 insertions(+)

Quoted 6 lines

I copied Bin who introduced this Kconfig option.
It is unfortunate that we need this #ifdef but I think it is correct. I think we need some Kconfig help for SANDBOX_BITS_PER_LONG.
Reviewed-by Simon Glass <[email protected]>

Applied to u-boot-dm, thanks!