diff mbox series

[v1] RISC-V: hwprobe: sort EXT_KEY()s in hwprobe_isa_ext0() alphabetically

Message ID 20240717-dedicate-squeamish-7e4ab54df58f@spud (mailing list archive)
State Accepted
Commit 3d8d459c8ba282a67df3b518c5f80bcb217a5b38
Headers show
Series [v1] RISC-V: hwprobe: sort EXT_KEY()s in hwprobe_isa_ext0() alphabetically | expand

Checks

Context Check Description
conchuod/vmtest-for-next-PR success PR summary
conchuod/patch-1-test-1 success .github/scripts/patches/tests/build_rv32_defconfig.sh
conchuod/patch-1-test-2 success .github/scripts/patches/tests/build_rv64_clang_allmodconfig.sh
conchuod/patch-1-test-3 success .github/scripts/patches/tests/build_rv64_gcc_allmodconfig.sh
conchuod/patch-1-test-4 success .github/scripts/patches/tests/build_rv64_nommu_k210_defconfig.sh
conchuod/patch-1-test-5 success .github/scripts/patches/tests/build_rv64_nommu_virt_defconfig.sh
conchuod/patch-1-test-6 success .github/scripts/patches/tests/checkpatch.sh
conchuod/patch-1-test-7 success .github/scripts/patches/tests/dtb_warn_rv64.sh
conchuod/patch-1-test-8 success .github/scripts/patches/tests/header_inline.sh
conchuod/patch-1-test-9 success .github/scripts/patches/tests/kdoc.sh
conchuod/patch-1-test-10 success .github/scripts/patches/tests/module_param.sh
conchuod/patch-1-test-11 success .github/scripts/patches/tests/verify_fixes.sh
conchuod/patch-1-test-12 success .github/scripts/patches/tests/verify_signedoff.sh

Commit Message

Conor Dooley July 17, 2024, 8:54 a.m. UTC
From: Conor Dooley <conor.dooley@microchip.com>

Currently the entries appear to be in a random order (although according
to Palmer he has tried to sort them by key value) which makes it harder
to find entries in a growing list, and more likely to have conflicts as
all patches are adding to the end of the list. Sort them alphabetically
instead.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
CC: Paul Walmsley <paul.walmsley@sifive.com>
CC: Palmer Dabbelt <palmer@dabbelt.com>
CC: linux-riscv@lists.infradead.org
CC: linux-kernel@vger.kernel.org
---
 arch/riscv/kernel/sys_hwprobe.c | 43 ++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 22 deletions(-)

Comments

Clément Léger July 17, 2024, 1:34 p.m. UTC | #1
On 17/07/2024 10:54, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Currently the entries appear to be in a random order (although according
> to Palmer he has tried to sort them by key value) which makes it harder
> to find entries in a growing list, and more likely to have conflicts as
> all patches are adding to the end of the list. Sort them alphabetically
> instead.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> CC: Paul Walmsley <paul.walmsley@sifive.com>
> CC: Palmer Dabbelt <palmer@dabbelt.com>
> CC: linux-riscv@lists.infradead.org
> CC: linux-kernel@vger.kernel.org
> ---
>  arch/riscv/kernel/sys_hwprobe.c | 43 ++++++++++++++++-----------------
>  1 file changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> index 685594769535..8a1c9ce170e8 100644
> --- a/arch/riscv/kernel/sys_hwprobe.c
> +++ b/arch/riscv/kernel/sys_hwprobe.c
> @@ -93,44 +93,45 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
>  		 * regardless of the kernel's configuration, as no other checks, besides
>  		 * presence in the hart_isa bitmap, are made.
>  		 */
> +		EXT_KEY(ZACAS);
> +		EXT_KEY(ZAWRS);
>  		EXT_KEY(ZBA);
>  		EXT_KEY(ZBB);
> -		EXT_KEY(ZBS);
> -		EXT_KEY(ZICBOZ);
>  		EXT_KEY(ZBC);
> -
>  		EXT_KEY(ZBKB);
>  		EXT_KEY(ZBKC);
>  		EXT_KEY(ZBKX);
> +		EXT_KEY(ZBS);
> +		EXT_KEY(ZCA);
> +		EXT_KEY(ZCB);
> +		EXT_KEY(ZCMOP);
> +		EXT_KEY(ZICBOZ);
> +		EXT_KEY(ZICOND);
> +		EXT_KEY(ZIHINTNTL);
> +		EXT_KEY(ZIHINTPAUSE);
> +		EXT_KEY(ZIMOP);
>  		EXT_KEY(ZKND);
>  		EXT_KEY(ZKNE);
>  		EXT_KEY(ZKNH);
>  		EXT_KEY(ZKSED);
>  		EXT_KEY(ZKSH);
>  		EXT_KEY(ZKT);
> -		EXT_KEY(ZIHINTNTL);
>  		EXT_KEY(ZTSO);
> -		EXT_KEY(ZACAS);
> -		EXT_KEY(ZICOND);
> -		EXT_KEY(ZIHINTPAUSE);
> -		EXT_KEY(ZIMOP);
> -		EXT_KEY(ZCA);
> -		EXT_KEY(ZCB);
> -		EXT_KEY(ZCMOP);
> -		EXT_KEY(ZAWRS);
>  
>  		/*
>  		 * All the following extensions must depend on the kernel
>  		 * support of V.
>  		 */
>  		if (has_vector()) {
> -			EXT_KEY(ZVE32X);
> -			EXT_KEY(ZVE32F);
> -			EXT_KEY(ZVE64X);
> -			EXT_KEY(ZVE64F);
> -			EXT_KEY(ZVE64D);
>  			EXT_KEY(ZVBB);
>  			EXT_KEY(ZVBC);
> +			EXT_KEY(ZVE32F);
> +			EXT_KEY(ZVE32X);
> +			EXT_KEY(ZVE64D);
> +			EXT_KEY(ZVE64F);
> +			EXT_KEY(ZVE64X);
> +			EXT_KEY(ZVFH);
> +			EXT_KEY(ZVFHMIN);
>  			EXT_KEY(ZVKB);
>  			EXT_KEY(ZVKG);
>  			EXT_KEY(ZVKNED);
> @@ -139,16 +140,14 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
>  			EXT_KEY(ZVKSED);
>  			EXT_KEY(ZVKSH);
>  			EXT_KEY(ZVKT);
> -			EXT_KEY(ZVFH);
> -			EXT_KEY(ZVFHMIN);
>  		}
>  
>  		if (has_fpu()) {
> -			EXT_KEY(ZFH);
> -			EXT_KEY(ZFHMIN);
> -			EXT_KEY(ZFA);
>  			EXT_KEY(ZCD);
>  			EXT_KEY(ZCF);
> +			EXT_KEY(ZFA);
> +			EXT_KEY(ZFH);
> +			EXT_KEY(ZFHMIN);
>  		}
>  #undef EXT_KEY
>  	}

I'd prefer that to be done after removing the "if
(has_vector()/has_fpu()) by using the .validate callback for ISA
extension. This way, you'll have only a single commit reordering everything.

Thanks,

Clément
Conor Dooley July 17, 2024, 1:42 p.m. UTC | #2
On Wed, Jul 17, 2024 at 03:34:06PM +0200, Clément Léger wrote:
> On 17/07/2024 10:54, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@microchip.com>
> > 
> > Currently the entries appear to be in a random order (although according
> > to Palmer he has tried to sort them by key value) which makes it harder
> > to find entries in a growing list, and more likely to have conflicts as
> > all patches are adding to the end of the list. Sort them alphabetically
> > instead.
> > 
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> >  		if (has_fpu()) {
> > -			EXT_KEY(ZFH);
> > -			EXT_KEY(ZFHMIN);
> > -			EXT_KEY(ZFA);
> >  			EXT_KEY(ZCD);
> >  			EXT_KEY(ZCF);
> > +			EXT_KEY(ZFA);
> > +			EXT_KEY(ZFH);
> > +			EXT_KEY(ZFHMIN);
> >  		}
> >  #undef EXT_KEY
> >  	}
> 
> I'd prefer that to be done after removing the "if
> (has_vector()/has_fpu()) by using the .validate callback for ISA
> extension. This way, you'll have only a single commit reordering everything.

Right, and I do have some WIP for that here
https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/log/?h=validate_fpu_and_vector
but won't be sending that until it's cleaned up after the merge window.
I was intentionally sending this during it so that there would be no
moving pieces for this to conflict with - because it will conflict with
any other patch adding things to the list tails.
Clément Léger July 17, 2024, 1:47 p.m. UTC | #3
On 17/07/2024 15:42, Conor Dooley wrote:
> On Wed, Jul 17, 2024 at 03:34:06PM +0200, Clément Léger wrote:
>> On 17/07/2024 10:54, Conor Dooley wrote:
>>> From: Conor Dooley <conor.dooley@microchip.com>
>>>
>>> Currently the entries appear to be in a random order (although according
>>> to Palmer he has tried to sort them by key value) which makes it harder
>>> to find entries in a growing list, and more likely to have conflicts as
>>> all patches are adding to the end of the list. Sort them alphabetically
>>> instead.
>>>
>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>>>  		if (has_fpu()) {
>>> -			EXT_KEY(ZFH);
>>> -			EXT_KEY(ZFHMIN);
>>> -			EXT_KEY(ZFA);
>>>  			EXT_KEY(ZCD);
>>>  			EXT_KEY(ZCF);
>>> +			EXT_KEY(ZFA);
>>> +			EXT_KEY(ZFH);
>>> +			EXT_KEY(ZFHMIN);
>>>  		}
>>>  #undef EXT_KEY
>>>  	}
>>
>> I'd prefer that to be done after removing the "if
>> (has_vector()/has_fpu()) by using the .validate callback for ISA
>> extension. This way, you'll have only a single commit reordering everything.
> 
> Right, and I do have some WIP for that here
> https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/log/?h=validate_fpu_and_vector
> but won't be sending that until it's cleaned up after the merge window.
> I was intentionally sending this during it so that there would be no
> moving pieces for this to conflict with - because it will conflict with
> any other patch adding things to the list tails.

Makes sense. If you think it's worth it:

Reviewed-by: Clément Léger <cleger@rivosinc.com>

Thanks,

Clément
patchwork-bot+linux-riscv@kernel.org July 23, 2024, 12:58 p.m. UTC | #4
Hello:

This patch was applied to riscv/linux.git (for-next)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Wed, 17 Jul 2024 09:54:38 +0100 you wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Currently the entries appear to be in a random order (although according
> to Palmer he has tried to sort them by key value) which makes it harder
> to find entries in a growing list, and more likely to have conflicts as
> all patches are adding to the end of the list. Sort them alphabetically
> instead.
> 
> [...]

Here is the summary with links:
  - [v1] RISC-V: hwprobe: sort EXT_KEY()s in hwprobe_isa_ext0() alphabetically
    https://git.kernel.org/riscv/c/3d8d459c8ba2

You are awesome, thank you!
diff mbox series

Patch

diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
index 685594769535..8a1c9ce170e8 100644
--- a/arch/riscv/kernel/sys_hwprobe.c
+++ b/arch/riscv/kernel/sys_hwprobe.c
@@ -93,44 +93,45 @@  static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
 		 * regardless of the kernel's configuration, as no other checks, besides
 		 * presence in the hart_isa bitmap, are made.
 		 */
+		EXT_KEY(ZACAS);
+		EXT_KEY(ZAWRS);
 		EXT_KEY(ZBA);
 		EXT_KEY(ZBB);
-		EXT_KEY(ZBS);
-		EXT_KEY(ZICBOZ);
 		EXT_KEY(ZBC);
-
 		EXT_KEY(ZBKB);
 		EXT_KEY(ZBKC);
 		EXT_KEY(ZBKX);
+		EXT_KEY(ZBS);
+		EXT_KEY(ZCA);
+		EXT_KEY(ZCB);
+		EXT_KEY(ZCMOP);
+		EXT_KEY(ZICBOZ);
+		EXT_KEY(ZICOND);
+		EXT_KEY(ZIHINTNTL);
+		EXT_KEY(ZIHINTPAUSE);
+		EXT_KEY(ZIMOP);
 		EXT_KEY(ZKND);
 		EXT_KEY(ZKNE);
 		EXT_KEY(ZKNH);
 		EXT_KEY(ZKSED);
 		EXT_KEY(ZKSH);
 		EXT_KEY(ZKT);
-		EXT_KEY(ZIHINTNTL);
 		EXT_KEY(ZTSO);
-		EXT_KEY(ZACAS);
-		EXT_KEY(ZICOND);
-		EXT_KEY(ZIHINTPAUSE);
-		EXT_KEY(ZIMOP);
-		EXT_KEY(ZCA);
-		EXT_KEY(ZCB);
-		EXT_KEY(ZCMOP);
-		EXT_KEY(ZAWRS);
 
 		/*
 		 * All the following extensions must depend on the kernel
 		 * support of V.
 		 */
 		if (has_vector()) {
-			EXT_KEY(ZVE32X);
-			EXT_KEY(ZVE32F);
-			EXT_KEY(ZVE64X);
-			EXT_KEY(ZVE64F);
-			EXT_KEY(ZVE64D);
 			EXT_KEY(ZVBB);
 			EXT_KEY(ZVBC);
+			EXT_KEY(ZVE32F);
+			EXT_KEY(ZVE32X);
+			EXT_KEY(ZVE64D);
+			EXT_KEY(ZVE64F);
+			EXT_KEY(ZVE64X);
+			EXT_KEY(ZVFH);
+			EXT_KEY(ZVFHMIN);
 			EXT_KEY(ZVKB);
 			EXT_KEY(ZVKG);
 			EXT_KEY(ZVKNED);
@@ -139,16 +140,14 @@  static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
 			EXT_KEY(ZVKSED);
 			EXT_KEY(ZVKSH);
 			EXT_KEY(ZVKT);
-			EXT_KEY(ZVFH);
-			EXT_KEY(ZVFHMIN);
 		}
 
 		if (has_fpu()) {
-			EXT_KEY(ZFH);
-			EXT_KEY(ZFHMIN);
-			EXT_KEY(ZFA);
 			EXT_KEY(ZCD);
 			EXT_KEY(ZCF);
+			EXT_KEY(ZFA);
+			EXT_KEY(ZFH);
+			EXT_KEY(ZFHMIN);
 		}
 #undef EXT_KEY
 	}