Re: [PATCH 2/2] RISC-V: fix the comment for ISA string workaround

From: Conor Dooley <conor@kernel.org>
To: Tsukasa OI <research_trasio@irq.a4lg.com>
Cc: linux-riscv@lists.infradead.org
Subject: Re: [PATCH 2/2] RISC-V: fix the comment for ISA string workaround
Date: Sat, 22 Jul 2023 12:28:40 +0100	[thread overview]
Message-ID: <20230722-playlist-arrange-5b58ee25ee68@spud> (raw)
In-Reply-To: <d7dfcb4e-d8a8-7b90-aab5-9be0fc3c9027@irq.a4lg.com>

[-- Attachment #1.1: Type: text/plain, Size: 2401 bytes --]

On Sat, Jul 22, 2023 at 08:22:25PM +0900, Tsukasa OI wrote:
> 
> 
> On 2023/07/22 19:52, Conor Dooley wrote:
> > On Sat, Jul 22, 2023 at 06:22:38AM +0000, Tsukasa OI wrote:
> >> From: Tsukasa OI <research_trasio@irq.a4lg.com>
> >>
> >> Extensions prefixed with "Su" won't corrupt the workaround in many
> >> cases.  The only exception is when the first multi-letter extension in the
> >> ISA string begins with "Su" and is not prefixed with an underscore.
> >>
> >> For instance, following ISA string can confuse this QEMU workaround.
> >>
> >> *   "rv64imacsuclic" (RV64I + M + A + C + "Suclic")
> >>
> >> However, this case is very unlikely because extensions prefixed by either
> >> "Z", "Sm" or "Ss" will most likely precede first.
> >>
> >> For instance, the "Suclic" extension (draft as of now) will be placed after
> >> related "Smclic" and "Ssclic" extensions.  It's also highly likely that
> >> other unprivileged extensions like "Zba" will precede.
> >>
> >> It's also possible to suppress the issue in the QEMU workaround with an
> >> underscore.  Following ISA string won't confuse the QEMU workaround.
> >>
> >> *   "rv64imac_suclic" (RV64I + M + A + C + delimited "Suclic")
> >>
> >> This fix is to tell kernel developers the nature of this workaround
> >> precisely.  There are some "Su*" extensions to be ratified but don't worry
> >> about this workaround too much.
> >>
> > 
> >> This commit comes with another minor editorial fix.
> > 
> > Which is what?
> > 
> > The new wording is fine by me though..
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> 
> 1. Use of plurals ("letters" and "bits")
> 2. Second "'s' & 'u'" to "'s' and 'u'"
> 3. Spacing after the first "'s' & 'u'" (before "(QEMU).")
> 
> It feels they are too minor to separate to another commit.
> At least I should replace the commit message to "other minor editorial
> fixes" and... should I clarify editorial fixes?

I dunno, I just wasn't sure what you meant.

> >> +				 * Workaround for invalid single-letters 's' & 'u' (QEMU).
> >> +				 * No need to set the bits in riscv_isa as 's' and 'u' are
> >> +				 * not valid ISA extensions. It works unless the first multi-letter
> >> +				 * extension in the ISA string begins with "Su" and not prefixed

Re-reading I noticed this should be "is not prefixed".

> >> +				 * with an underscore.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv