Re: [PATCH 2/4] RISC-V: add alternative-field for bits to not match against

[Conor Dooley <conor@kernel.org>]

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

Hey Heiko,

Just one really minor thing that bugged me a little as I was reading
through the patch.

On Fri, Jan 13, 2023 at 10:23:49PM +0100, Heiko Stuebner wrote:
> diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> index 1bd4027d34ca..d08c563ab7d8 100644
> --- a/arch/riscv/include/asm/alternative.h
> +++ b/arch/riscv/include/asm/alternative.h
> @@ -36,6 +36,7 @@ struct alt_entry {
>  	unsigned long vendor_id; /* cpu vendor id */
>  	unsigned long alt_len;   /* The replacement size */
>  	unsigned int errata_id;  /* The errata id */

Should we ditch the pretence that these are errata at this point, and
note in the comment that this may be a cpufeature bitmap too?

> +	unsigned int errata_not; /* Errata id not to match against */

...because "errata_not" & the description here make very little sense I
think. To me, "incompatible_errata" might be more verbose, but gets
across the point better.
"errata not to match against" is a bit confusing I think, taking things
on face value is it a bit difficult to understand. Ignoring
"cpufeatures" and extensions, which the comments do, the point of these
alternatives is to to match against one specific errata_id & the concept
of having to not match against something else seems a bit odd.

Luckily though, that's not a big deal at all, since all we'd need to do
is update the comment to explain that that argument is intended to be
used for cpufeatures that preclude the use of a variant.

Thanks,
Conor.


[-- 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