diff mbox series

[1/2] RISC-V: fix ordering of Zbb extension

Message ID 20230208225328.1636017-2-heiko@sntech.de (mailing list archive)
State Accepted
Commit 1eac28201ac0725192f5ced34192d281a06692e5
Delegated to: Palmer Dabbelt
Headers show
Series Small fixups for the Zbb string functions | expand

Checks

Context Check Description
conchuod/cover_letter success Series has a cover letter
conchuod/tree_selection success Guessed tree name to be for-next
conchuod/fixes_present success Fixes tag not required for -next series
conchuod/maintainers_pattern success MAINTAINERS pattern errors before the patch: 13 and now 13
conchuod/verify_signedoff success Signed-off-by tag matches author and committer
conchuod/kdoc success Errors and warnings before: 0 this patch: 0
conchuod/build_rv64_clang_allmodconfig success Errors and warnings before: 0 this patch: 0
conchuod/module_param success Was 0 now: 0
conchuod/build_rv64_gcc_allmodconfig success Errors and warnings before: 1 this patch: 1
conchuod/alphanumeric_selects success Out of order selects before the patch: 59 and now 59
conchuod/build_rv32_defconfig success Build OK
conchuod/dtb_warn_rv64 success Errors and warnings before: 2 this patch: 2
conchuod/header_inline success No static functions without inline keyword in header files
conchuod/checkpatch success total: 0 errors, 0 warnings, 0 checks, 10 lines checked
conchuod/source_inline success Was 0 now: 0
conchuod/build_rv64_nommu_k210_defconfig success Build OK
conchuod/verify_fixes success No Fixes tag
conchuod/build_rv64_nommu_virt_defconfig success Build OK

Commit Message

Heiko Stübner Feb. 8, 2023, 10:53 p.m. UTC
From: Heiko Stuebner <heiko.stuebner@vrull.eu>

As Andrew reported,
    Zb* comes after Zi* according 27.11 "Subset Naming Convention"
so fix the ordering accordingly.

Reported-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
 arch/riscv/kernel/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Conor Dooley Feb. 8, 2023, 11:20 p.m. UTC | #1
Hey Heiko,

On 8 February 2023 22:53:27 GMT, Heiko Stuebner <heiko@sntech.de> wrote:
>From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
>As Andrew reported,
>    Zb* comes after Zi* according 27.11 "Subset Naming Convention"
>so fix the ordering accordingly.
>
>Reported-by: Andrew Jones <ajones@ventanamicro.com>
>Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>

The whole "getting it wrong immediately after fixing it up" ;)

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

>---
> arch/riscv/kernel/cpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
>index 420228e219f7..8400f0cc9704 100644
>--- a/arch/riscv/kernel/cpu.c
>+++ b/arch/riscv/kernel/cpu.c
>@@ -185,9 +185,9 @@ arch_initcall(riscv_cpuinfo_init);
>  * New entries to this struct should follow the ordering rules described above.
>  */
> static struct riscv_isa_ext_data isa_ext_arr[] = {
>-	__RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> 	__RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
> 	__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
>+	__RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> 	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> 	__RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> 	__RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
Heiko Stübner Feb. 8, 2023, 11:26 p.m. UTC | #2
Am Donnerstag, 9. Februar 2023, 00:20:10 CET schrieb Conor Dooley:
> Hey Heiko,
> 
> On 8 February 2023 22:53:27 GMT, Heiko Stuebner <heiko@sntech.de> wrote:
> >From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> >
> >As Andrew reported,
> >    Zb* comes after Zi* according 27.11 "Subset Naming Convention"
> >so fix the ordering accordingly.
> >
> >Reported-by: Andrew Jones <ajones@ventanamicro.com>
> >Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> 
> The whole "getting it wrong immediately after fixing it up" ;)
> 
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

I'm still hopefully that I'll learn at some point that "b" comes after "i",
at least with riscv extensions. Decades of sorting the other way around are
hard to break :-D .

> >---
> > arch/riscv/kernel/cpu.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> >index 420228e219f7..8400f0cc9704 100644
> >--- a/arch/riscv/kernel/cpu.c
> >+++ b/arch/riscv/kernel/cpu.c
> >@@ -185,9 +185,9 @@ arch_initcall(riscv_cpuinfo_init);
> >  * New entries to this struct should follow the ordering rules described above.
> >  */
> > static struct riscv_isa_ext_data isa_ext_arr[] = {
> >-	__RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> > 	__RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
> > 	__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
> >+	__RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> > 	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> > 	__RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> > 	__RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
>
Andrew Jones Feb. 9, 2023, 8:23 a.m. UTC | #3
On Wed, Feb 08, 2023 at 11:53:27PM +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> 
> As Andrew reported,
>     Zb* comes after Zi* according 27.11 "Subset Naming Convention"
> so fix the ordering accordingly.
> 
> Reported-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/kernel/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index 420228e219f7..8400f0cc9704 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -185,9 +185,9 @@ arch_initcall(riscv_cpuinfo_init);
>   * New entries to this struct should follow the ordering rules described above.
>   */
>  static struct riscv_isa_ext_data isa_ext_arr[] = {
> -	__RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
>  	__RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
>  	__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
> +	__RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
>  	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
>  	__RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
>  	__RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
> -- 
> 2.39.0
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Andrew Jones Feb. 9, 2023, 8:25 a.m. UTC | #4
On Wed, Feb 08, 2023 at 11:20:10PM +0000, Conor Dooley wrote:
> Hey Heiko,
> 
> On 8 February 2023 22:53:27 GMT, Heiko Stuebner <heiko@sntech.de> wrote:
> >From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> >
> >As Andrew reported,
> >    Zb* comes after Zi* according 27.11 "Subset Naming Convention"
> >so fix the ordering accordingly.
> >
> >Reported-by: Andrew Jones <ajones@ventanamicro.com>
> >Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> 
> The whole "getting it wrong immediately after fixing it up" ;)

Hi Conor,

Do you know any patchwork savvy people that could whip up a check
for this array? :-)

drew
Conor Dooley Feb. 9, 2023, 9:03 a.m. UTC | #5
On Thu, Feb 09, 2023 at 09:25:20AM +0100, Andrew Jones wrote:
> On Wed, Feb 08, 2023 at 11:20:10PM +0000, Conor Dooley wrote:
> > Hey Heiko,
> > 
> > On 8 February 2023 22:53:27 GMT, Heiko Stuebner <heiko@sntech.de> wrote:
> > >From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> > >
> > >As Andrew reported,
> > >    Zb* comes after Zi* according 27.11 "Subset Naming Convention"
> > >so fix the ordering accordingly.
> > >
> > >Reported-by: Andrew Jones <ajones@ventanamicro.com>
> > >Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> > 
> > The whole "getting it wrong immediately after fixing it up" ;)
> 
> Hi Conor,
> 
> Do you know any patchwork savvy people that could whip up a check
> for this array? :-)

Maybe that is more of a checkpatch type thing?

Either way, I'll put it on the todo list I suppose!
Andrew Jones Feb. 9, 2023, 9:28 a.m. UTC | #6
On Thu, Feb 09, 2023 at 09:03:50AM +0000, Conor Dooley wrote:
> On Thu, Feb 09, 2023 at 09:25:20AM +0100, Andrew Jones wrote:
> > On Wed, Feb 08, 2023 at 11:20:10PM +0000, Conor Dooley wrote:
> > > Hey Heiko,
> > > 
> > > On 8 February 2023 22:53:27 GMT, Heiko Stuebner <heiko@sntech.de> wrote:
> > > >From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> > > >
> > > >As Andrew reported,
> > > >    Zb* comes after Zi* according 27.11 "Subset Naming Convention"
> > > >so fix the ordering accordingly.
> > > >
> > > >Reported-by: Andrew Jones <ajones@ventanamicro.com>
> > > >Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> > > 
> > > The whole "getting it wrong immediately after fixing it up" ;)
> > 
> > Hi Conor,
> > 
> > Do you know any patchwork savvy people that could whip up a check
> > for this array? :-)
> 
> Maybe that is more of a checkpatch type thing?

I think this is too specific for general checkpatch. I once proposed on
the KVM mailing list that checkpatch should gain support for plugins,
allowing specific directories to provide a .checkpatch script, or
whatever, where it puts its own checks. I never followed-up by actually
proposing that to checkpatch maintainers though.

> 
> Either way, I'll put it on the todo list I suppose!

In the absence of checkpatch plugins, I think subsystem-specific
patch management tools, like patchwork, are the next best place
to put specific checks. But, I agree it's a bit late. It'd be better
if the developers could run the checks themselves before posting.

Thanks,
drew
diff mbox series

Patch

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 420228e219f7..8400f0cc9704 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -185,9 +185,9 @@  arch_initcall(riscv_cpuinfo_init);
  * New entries to this struct should follow the ordering rules described above.
  */
 static struct riscv_isa_ext_data isa_ext_arr[] = {
-	__RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
 	__RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
 	__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
+	__RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
 	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
 	__RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
 	__RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),