diff mbox series

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

Message ID	20230208225328.1636017-2-heiko@sntech.de
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; From: Heiko Stuebner <heiko@sntech.de> To: palmer@rivosinc.com Cc: conor@kernel.org, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org, christoph.muellner@vrull.eu, ajones@ventanamicro.com, Heiko Stuebner <heiko.stuebner@vrull.eu> Subject: [PATCH 1/2] RISC-V: fix ordering of Zbb extension Date: Wed, 8 Feb 2023 23:53:27 +0100 Message-Id: <20230208225328.1636017-2-heiko@sntech.de> In-Reply-To: <20230208225328.1636017-1-heiko@sntech.de> References: <20230208225328.1636017-1-heiko@sntech.de> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	Small fixups for the Zbb string functions \| [0/2] Small fixups for the Zbb string functions [1/2] RISC-V: fix ordering of Zbb extension [2/2] RISC-V: improve string-function assembly

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),