| Message ID | 20181217133236.15034-1-vigneshr@ti.com |
|---|---|
| State | Accepted |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | [U-Boot,RFC] bitops: Fix GENMASK definition for Sandbox | expand |
On 17/12/18 7:02 PM, Vignesh R wrote: > 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 <vigneshr@ti.com> > --- > Gentle ping.. Simon, this patch is required of upcoming SPI flash layer changes. Any feedback on this patch? Regards Vignesh > 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(+) > > diff --git a/include/linux/bitops.h b/include/linux/bitops.h > index a47f6d17bb5f..259df43fb00f 100644 > --- a/include/linux/bitops.h > +++ b/include/linux/bitops.h > @@ -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)))) >
On Mon, 17 Dec 2018 at 06:31, Vignesh R <vigneshr@ti.com> wrote: > > 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 <vigneshr@ti.com> > --- > > 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(+) 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 <sjg@chromium.org>
On Fri, 11 Jan 2019 at 01:56, Simon Glass <sjg@chromium.org> wrote: > > On Mon, 17 Dec 2018 at 06:31, Vignesh R <vigneshr@ti.com> wrote: > > > > 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 <vigneshr@ti.com> > > --- > > > > 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(+) > > 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 <sjg@chromium.org> Applied to u-boot-dm, thanks!
diff --git a/include/linux/bitops.h b/include/linux/bitops.h index a47f6d17bb5f..259df43fb00f 100644 --- a/include/linux/bitops.h +++ b/include/linux/bitops.h @@ -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))))
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 <vigneshr@ti.com> --- 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(+)