diff mbox series

[RFC] Replaces long number representation by BIT() macro

Message ID 20190613180227.29558-1-leonardo@linux.ibm.com (mailing list archive)
State Rejected, archived
Headers show
Series [RFC] Replaces long number representation by BIT() macro | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch next (a3bf9fbdad600b1e4335dd90979f8d6072e4f602)
snowpatch_ozlabs/build-ppc64le success Build succeeded
snowpatch_ozlabs/build-ppc64be success Build succeeded
snowpatch_ozlabs/build-ppc64e success Build succeeded
snowpatch_ozlabs/build-pmac32 success Build succeeded
snowpatch_ozlabs/checkpatch success total: 0 errors, 0 warnings, 0 checks, 83 lines checked

Commit Message

Leonardo Bras June 13, 2019, 6:02 p.m. UTC 
The main reason of this change is to make these bitmasks more readable.

The macro ASM_CONST() just appends an UL to it's parameter, so it can be
easily replaced by BIT_MASK, that already uses a UL representation.

ASM_CONST() in this file may behave different if __ASSEMBLY__ is defined,
as it is used on .S files, just leaving the parameter as is.

However, I have noticed no difference in the generated binary after this
change.

Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
---
 arch/powerpc/include/asm/firmware.h | 75 ++++++++++++++---------------
 1 file changed, 37 insertions(+), 38 deletions(-)

Comments

Leonardo Bras June 13, 2019, 8:24 p.m. UTC | #1 
Sorry, there is a typo on my commit message.
's/BIT_MASK/BIT/'

On Thu, 2019-06-13 at 15:02 -0300, Leonardo Bras wrote:
> The main reason of this change is to make these bitmasks more readable.
> 
> The macro ASM_CONST() just appends an UL to it's parameter, so it can be
> easily replaced by BIT_MASK, that already uses a UL representation.
> 
> ASM_CONST() in this file may behave different if __ASSEMBLY__ is defined,
> as it is used on .S files, just leaving the parameter as is.
> 
> However, I have noticed no difference in the generated binary after this
> change.
> 
> Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/firmware.h | 75 ++++++++++++++---------------
>  1 file changed, 37 insertions(+), 38 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
> index 00bc42d95679..7a5b0cc0bc85 100644
> --- a/arch/powerpc/include/asm/firmware.h
> +++ b/arch/powerpc/include/asm/firmware.h
> @@ -14,46 +14,45 @@
>  
>  #ifdef __KERNEL__
>  
> -#include <asm/asm-const.h>
> -
> +#include <linux/bits.h>
>  /* firmware feature bitmask values */
>  
> -#define FW_FEATURE_PFT		ASM_CONST(0x0000000000000001)
> -#define FW_FEATURE_TCE		ASM_CONST(0x0000000000000002)
> -#define FW_FEATURE_SPRG0	ASM_CONST(0x0000000000000004)
> -#define FW_FEATURE_DABR		ASM_CONST(0x0000000000000008)
> -#define FW_FEATURE_COPY		ASM_CONST(0x0000000000000010)
> -#define FW_FEATURE_ASR		ASM_CONST(0x0000000000000020)
> -#define FW_FEATURE_DEBUG	ASM_CONST(0x0000000000000040)
> -#define FW_FEATURE_TERM		ASM_CONST(0x0000000000000080)
> -#define FW_FEATURE_PERF		ASM_CONST(0x0000000000000100)
> -#define FW_FEATURE_DUMP		ASM_CONST(0x0000000000000200)
> -#define FW_FEATURE_INTERRUPT	ASM_CONST(0x0000000000000400)
> -#define FW_FEATURE_MIGRATE	ASM_CONST(0x0000000000000800)
> -#define FW_FEATURE_PERFMON	ASM_CONST(0x0000000000001000)
> -#define FW_FEATURE_CRQ		ASM_CONST(0x0000000000002000)
> -#define FW_FEATURE_VIO		ASM_CONST(0x0000000000004000)
> -#define FW_FEATURE_RDMA		ASM_CONST(0x0000000000008000)
> -#define FW_FEATURE_LLAN		ASM_CONST(0x0000000000010000)
> -#define FW_FEATURE_BULK_REMOVE	ASM_CONST(0x0000000000020000)
> -#define FW_FEATURE_XDABR	ASM_CONST(0x0000000000040000)
> -#define FW_FEATURE_MULTITCE	ASM_CONST(0x0000000000080000)
> -#define FW_FEATURE_SPLPAR	ASM_CONST(0x0000000000100000)
> -#define FW_FEATURE_LPAR		ASM_CONST(0x0000000000400000)
> -#define FW_FEATURE_PS3_LV1	ASM_CONST(0x0000000000800000)
> -#define FW_FEATURE_HPT_RESIZE	ASM_CONST(0x0000000001000000)
> -#define FW_FEATURE_CMO		ASM_CONST(0x0000000002000000)
> -#define FW_FEATURE_VPHN		ASM_CONST(0x0000000004000000)
> -#define FW_FEATURE_XCMO		ASM_CONST(0x0000000008000000)
> -#define FW_FEATURE_OPAL		ASM_CONST(0x0000000010000000)
> -#define FW_FEATURE_SET_MODE	ASM_CONST(0x0000000040000000)
> -#define FW_FEATURE_BEST_ENERGY	ASM_CONST(0x0000000080000000)
> -#define FW_FEATURE_TYPE1_AFFINITY ASM_CONST(0x0000000100000000)
> -#define FW_FEATURE_PRRN		ASM_CONST(0x0000000200000000)
> -#define FW_FEATURE_DRMEM_V2	ASM_CONST(0x0000000400000000)
> -#define FW_FEATURE_DRC_INFO	ASM_CONST(0x0000000800000000)
> -#define FW_FEATURE_BLOCK_REMOVE ASM_CONST(0x0000001000000000)
> -#define FW_FEATURE_PAPR_SCM 	ASM_CONST(0x0000002000000000)
> +#define FW_FEATURE_PFT		BIT(0)
> +#define FW_FEATURE_TCE		BIT(1)
> +#define FW_FEATURE_SPRG0		BIT(2)
> +#define FW_FEATURE_DABR		BIT(3)
> +#define FW_FEATURE_COPY		BIT(4)
> +#define FW_FEATURE_ASR		BIT(5)
> +#define FW_FEATURE_DEBUG		BIT(6)
> +#define FW_FEATURE_TERM		BIT(7)
> +#define FW_FEATURE_PERF		BIT(8)
> +#define FW_FEATURE_DUMP		BIT(9)
> +#define FW_FEATURE_INTERRUPT	BIT(10)
> +#define FW_FEATURE_MIGRATE	BIT(11)
> +#define FW_FEATURE_PERFMON	BIT(12)
> +#define FW_FEATURE_CRQ		BIT(13)
> +#define FW_FEATURE_VIO		BIT(14)
> +#define FW_FEATURE_RDMA		BIT(15)
> +#define FW_FEATURE_LLAN		BIT(16)
> +#define FW_FEATURE_BULK_REMOVE	BIT(17)
> +#define FW_FEATURE_XDABR		BIT(18)
> +#define FW_FEATURE_MULTITCE	BIT(19)
> +#define FW_FEATURE_SPLPAR	BIT(20)
> +#define FW_FEATURE_LPAR		BIT(22)
> +#define FW_FEATURE_PS3_LV1	BIT(23)
> +#define FW_FEATURE_HPT_RESIZE	BIT(24)
> +#define FW_FEATURE_CMO		BIT(25)
> +#define FW_FEATURE_VPHN		BIT(26)
> +#define FW_FEATURE_XCMO		BIT(27)
> +#define FW_FEATURE_OPAL		BIT(28)
> +#define FW_FEATURE_SET_MODE	BIT(30)
> +#define FW_FEATURE_BEST_ENERGY	BIT(31)
> +#define FW_FEATURE_TYPE1_AFFINITY BIT(32)
> +#define FW_FEATURE_PRRN		BIT(33)
> +#define FW_FEATURE_DRMEM_V2	BIT(34)
> +#define FW_FEATURE_DRC_INFO	BIT(35)
> +#define FW_FEATURE_BLOCK_REMOVE	BIT(36)
> +#define FW_FEATURE_PAPR_SCM	BIT(37)
>  
>  #ifndef __ASSEMBLY__
>
Michael Ellerman July 2, 2019, 3:19 p.m. UTC | #2 
Hi Leonardo,

Leonardo Bras <leonardo@linux.ibm.com> writes:
> The main reason of this change is to make these bitmasks more readable.
>
> The macro ASM_CONST() just appends an UL to it's parameter, so it can be
> easily replaced by BIT_MASK, that already uses a UL representation.
>
> ASM_CONST() in this file may behave different if __ASSEMBLY__ is defined,
> as it is used on .S files, just leaving the parameter as is.

Thanks for the patch, but I don't consider this an improvement in
readability.

At boot we print the firmware features, eg:

  firmware_features = 0x00000001c45ffc5f

And it's much easier to match that up to the full constants, than to the
bit numbers.

Similarly in memory or register dumps.

What we could do is switch to the `UL` macro from include/linux/const.h,
rather than using our own ASM_CONST.

cheers

> diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
> index 00bc42d95679..7a5b0cc0bc85 100644
> --- a/arch/powerpc/include/asm/firmware.h
> +++ b/arch/powerpc/include/asm/firmware.h
> @@ -14,46 +14,45 @@
>  
>  #ifdef __KERNEL__
>  
> -#include <asm/asm-const.h>
> -
> +#include <linux/bits.h>
>  /* firmware feature bitmask values */
>  
> -#define FW_FEATURE_PFT		ASM_CONST(0x0000000000000001)
> -#define FW_FEATURE_TCE		ASM_CONST(0x0000000000000002)
> -#define FW_FEATURE_SPRG0	ASM_CONST(0x0000000000000004)
> -#define FW_FEATURE_DABR		ASM_CONST(0x0000000000000008)
> -#define FW_FEATURE_COPY		ASM_CONST(0x0000000000000010)
> -#define FW_FEATURE_ASR		ASM_CONST(0x0000000000000020)
> -#define FW_FEATURE_DEBUG	ASM_CONST(0x0000000000000040)
> -#define FW_FEATURE_TERM		ASM_CONST(0x0000000000000080)
> -#define FW_FEATURE_PERF		ASM_CONST(0x0000000000000100)
> -#define FW_FEATURE_DUMP		ASM_CONST(0x0000000000000200)
> -#define FW_FEATURE_INTERRUPT	ASM_CONST(0x0000000000000400)
> -#define FW_FEATURE_MIGRATE	ASM_CONST(0x0000000000000800)
> -#define FW_FEATURE_PERFMON	ASM_CONST(0x0000000000001000)
> -#define FW_FEATURE_CRQ		ASM_CONST(0x0000000000002000)
> -#define FW_FEATURE_VIO		ASM_CONST(0x0000000000004000)
> -#define FW_FEATURE_RDMA		ASM_CONST(0x0000000000008000)
> -#define FW_FEATURE_LLAN		ASM_CONST(0x0000000000010000)
> -#define FW_FEATURE_BULK_REMOVE	ASM_CONST(0x0000000000020000)
> -#define FW_FEATURE_XDABR	ASM_CONST(0x0000000000040000)
> -#define FW_FEATURE_MULTITCE	ASM_CONST(0x0000000000080000)
> -#define FW_FEATURE_SPLPAR	ASM_CONST(0x0000000000100000)
> -#define FW_FEATURE_LPAR		ASM_CONST(0x0000000000400000)
> -#define FW_FEATURE_PS3_LV1	ASM_CONST(0x0000000000800000)
> -#define FW_FEATURE_HPT_RESIZE	ASM_CONST(0x0000000001000000)
> -#define FW_FEATURE_CMO		ASM_CONST(0x0000000002000000)
> -#define FW_FEATURE_VPHN		ASM_CONST(0x0000000004000000)
> -#define FW_FEATURE_XCMO		ASM_CONST(0x0000000008000000)
> -#define FW_FEATURE_OPAL		ASM_CONST(0x0000000010000000)
> -#define FW_FEATURE_SET_MODE	ASM_CONST(0x0000000040000000)
> -#define FW_FEATURE_BEST_ENERGY	ASM_CONST(0x0000000080000000)
> -#define FW_FEATURE_TYPE1_AFFINITY ASM_CONST(0x0000000100000000)
> -#define FW_FEATURE_PRRN		ASM_CONST(0x0000000200000000)
> -#define FW_FEATURE_DRMEM_V2	ASM_CONST(0x0000000400000000)
> -#define FW_FEATURE_DRC_INFO	ASM_CONST(0x0000000800000000)
> -#define FW_FEATURE_BLOCK_REMOVE ASM_CONST(0x0000001000000000)
> -#define FW_FEATURE_PAPR_SCM 	ASM_CONST(0x0000002000000000)
> +#define FW_FEATURE_PFT		BIT(0)
> +#define FW_FEATURE_TCE		BIT(1)
> +#define FW_FEATURE_SPRG0		BIT(2)
> +#define FW_FEATURE_DABR		BIT(3)
> +#define FW_FEATURE_COPY		BIT(4)
> +#define FW_FEATURE_ASR		BIT(5)
> +#define FW_FEATURE_DEBUG		BIT(6)
> +#define FW_FEATURE_TERM		BIT(7)
> +#define FW_FEATURE_PERF		BIT(8)
> +#define FW_FEATURE_DUMP		BIT(9)
> +#define FW_FEATURE_INTERRUPT	BIT(10)
> +#define FW_FEATURE_MIGRATE	BIT(11)
> +#define FW_FEATURE_PERFMON	BIT(12)
> +#define FW_FEATURE_CRQ		BIT(13)
> +#define FW_FEATURE_VIO		BIT(14)
> +#define FW_FEATURE_RDMA		BIT(15)
> +#define FW_FEATURE_LLAN		BIT(16)
> +#define FW_FEATURE_BULK_REMOVE	BIT(17)
> +#define FW_FEATURE_XDABR		BIT(18)
> +#define FW_FEATURE_MULTITCE	BIT(19)
> +#define FW_FEATURE_SPLPAR	BIT(20)
> +#define FW_FEATURE_LPAR		BIT(22)
> +#define FW_FEATURE_PS3_LV1	BIT(23)
> +#define FW_FEATURE_HPT_RESIZE	BIT(24)
> +#define FW_FEATURE_CMO		BIT(25)
> +#define FW_FEATURE_VPHN		BIT(26)
> +#define FW_FEATURE_XCMO		BIT(27)
> +#define FW_FEATURE_OPAL		BIT(28)
> +#define FW_FEATURE_SET_MODE	BIT(30)
> +#define FW_FEATURE_BEST_ENERGY	BIT(31)
> +#define FW_FEATURE_TYPE1_AFFINITY BIT(32)
> +#define FW_FEATURE_PRRN		BIT(33)
> +#define FW_FEATURE_DRMEM_V2	BIT(34)
> +#define FW_FEATURE_DRC_INFO	BIT(35)
> +#define FW_FEATURE_BLOCK_REMOVE	BIT(36)
> +#define FW_FEATURE_PAPR_SCM	BIT(37)
>  
>  #ifndef __ASSEMBLY__
>  
> -- 
> 2.17.1
Segher Boessenkool July 2, 2019, 4:16 p.m. UTC | #3 
On Wed, Jul 03, 2019 at 01:19:34AM +1000, Michael Ellerman wrote:
> What we could do is switch to the `UL` macro from include/linux/const.h,
> rather than using our own ASM_CONST.

You need gas 2.28 or later for that though.

https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=86b80085
https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=e140100a

What is the minimum required (for powerpc) now?


Segher
Segher Boessenkool July 2, 2019, 4:48 p.m. UTC | #4 
On Tue, Jul 02, 2019 at 11:16:35AM -0500, Segher Boessenkool wrote:
> On Wed, Jul 03, 2019 at 01:19:34AM +1000, Michael Ellerman wrote:
> > What we could do is switch to the `UL` macro from include/linux/const.h,
> > rather than using our own ASM_CONST.
> 
> You need gas 2.28 or later for that though.

Oh, but apparently I cannot read.  That macro should work fine.


Segher
Michael Ellerman July 7, 2019, 1:15 p.m. UTC | #5 
Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Tue, Jul 02, 2019 at 11:16:35AM -0500, Segher Boessenkool wrote:
>> On Wed, Jul 03, 2019 at 01:19:34AM +1000, Michael Ellerman wrote:
>> > What we could do is switch to the `UL` macro from include/linux/const.h,
>> > rather than using our own ASM_CONST.
>> 
>> You need gas 2.28 or later for that though.
>
> Oh, but apparently I cannot read.  That macro should work fine.

:)

Yeah one day we'll be able to drop them entirely, but not yet.

The official minimum is 2.20:
  https://www.kernel.org/doc/html/latest/process/changes.html


But my "old" toolchain is binutils 2.22, so that's effectively the
minimum for anything I test. I'm not sure many people are actually
testing with 2.20.

cheers
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
index 00bc42d95679..7a5b0cc0bc85 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -14,46 +14,45 @@ 
 
 #ifdef __KERNEL__
 
-#include <asm/asm-const.h>
-
+#include <linux/bits.h>
 /* firmware feature bitmask values */
 
-#define FW_FEATURE_PFT		ASM_CONST(0x0000000000000001)
-#define FW_FEATURE_TCE		ASM_CONST(0x0000000000000002)
-#define FW_FEATURE_SPRG0	ASM_CONST(0x0000000000000004)
-#define FW_FEATURE_DABR		ASM_CONST(0x0000000000000008)
-#define FW_FEATURE_COPY		ASM_CONST(0x0000000000000010)
-#define FW_FEATURE_ASR		ASM_CONST(0x0000000000000020)
-#define FW_FEATURE_DEBUG	ASM_CONST(0x0000000000000040)
-#define FW_FEATURE_TERM		ASM_CONST(0x0000000000000080)
-#define FW_FEATURE_PERF		ASM_CONST(0x0000000000000100)
-#define FW_FEATURE_DUMP		ASM_CONST(0x0000000000000200)
-#define FW_FEATURE_INTERRUPT	ASM_CONST(0x0000000000000400)
-#define FW_FEATURE_MIGRATE	ASM_CONST(0x0000000000000800)
-#define FW_FEATURE_PERFMON	ASM_CONST(0x0000000000001000)
-#define FW_FEATURE_CRQ		ASM_CONST(0x0000000000002000)
-#define FW_FEATURE_VIO		ASM_CONST(0x0000000000004000)
-#define FW_FEATURE_RDMA		ASM_CONST(0x0000000000008000)
-#define FW_FEATURE_LLAN		ASM_CONST(0x0000000000010000)
-#define FW_FEATURE_BULK_REMOVE	ASM_CONST(0x0000000000020000)
-#define FW_FEATURE_XDABR	ASM_CONST(0x0000000000040000)
-#define FW_FEATURE_MULTITCE	ASM_CONST(0x0000000000080000)
-#define FW_FEATURE_SPLPAR	ASM_CONST(0x0000000000100000)
-#define FW_FEATURE_LPAR		ASM_CONST(0x0000000000400000)
-#define FW_FEATURE_PS3_LV1	ASM_CONST(0x0000000000800000)
-#define FW_FEATURE_HPT_RESIZE	ASM_CONST(0x0000000001000000)
-#define FW_FEATURE_CMO		ASM_CONST(0x0000000002000000)
-#define FW_FEATURE_VPHN		ASM_CONST(0x0000000004000000)
-#define FW_FEATURE_XCMO		ASM_CONST(0x0000000008000000)
-#define FW_FEATURE_OPAL		ASM_CONST(0x0000000010000000)
-#define FW_FEATURE_SET_MODE	ASM_CONST(0x0000000040000000)
-#define FW_FEATURE_BEST_ENERGY	ASM_CONST(0x0000000080000000)
-#define FW_FEATURE_TYPE1_AFFINITY ASM_CONST(0x0000000100000000)
-#define FW_FEATURE_PRRN		ASM_CONST(0x0000000200000000)
-#define FW_FEATURE_DRMEM_V2	ASM_CONST(0x0000000400000000)
-#define FW_FEATURE_DRC_INFO	ASM_CONST(0x0000000800000000)
-#define FW_FEATURE_BLOCK_REMOVE ASM_CONST(0x0000001000000000)
-#define FW_FEATURE_PAPR_SCM 	ASM_CONST(0x0000002000000000)
+#define FW_FEATURE_PFT		BIT(0)
+#define FW_FEATURE_TCE		BIT(1)
+#define FW_FEATURE_SPRG0		BIT(2)
+#define FW_FEATURE_DABR		BIT(3)
+#define FW_FEATURE_COPY		BIT(4)
+#define FW_FEATURE_ASR		BIT(5)
+#define FW_FEATURE_DEBUG		BIT(6)
+#define FW_FEATURE_TERM		BIT(7)
+#define FW_FEATURE_PERF		BIT(8)
+#define FW_FEATURE_DUMP		BIT(9)
+#define FW_FEATURE_INTERRUPT	BIT(10)
+#define FW_FEATURE_MIGRATE	BIT(11)
+#define FW_FEATURE_PERFMON	BIT(12)
+#define FW_FEATURE_CRQ		BIT(13)
+#define FW_FEATURE_VIO		BIT(14)
+#define FW_FEATURE_RDMA		BIT(15)
+#define FW_FEATURE_LLAN		BIT(16)
+#define FW_FEATURE_BULK_REMOVE	BIT(17)
+#define FW_FEATURE_XDABR		BIT(18)
+#define FW_FEATURE_MULTITCE	BIT(19)
+#define FW_FEATURE_SPLPAR	BIT(20)
+#define FW_FEATURE_LPAR		BIT(22)
+#define FW_FEATURE_PS3_LV1	BIT(23)
+#define FW_FEATURE_HPT_RESIZE	BIT(24)
+#define FW_FEATURE_CMO		BIT(25)
+#define FW_FEATURE_VPHN		BIT(26)
+#define FW_FEATURE_XCMO		BIT(27)
+#define FW_FEATURE_OPAL		BIT(28)
+#define FW_FEATURE_SET_MODE	BIT(30)
+#define FW_FEATURE_BEST_ENERGY	BIT(31)
+#define FW_FEATURE_TYPE1_AFFINITY BIT(32)
+#define FW_FEATURE_PRRN		BIT(33)
+#define FW_FEATURE_DRMEM_V2	BIT(34)
+#define FW_FEATURE_DRC_INFO	BIT(35)
+#define FW_FEATURE_BLOCK_REMOVE	BIT(36)
+#define FW_FEATURE_PAPR_SCM	BIT(37)
 
 #ifndef __ASSEMBLY__