[PATCH 3/5] riscv: alternatives: Rename errata_id to patch_id
Andrew Jones
ajones at ventanamicro.com
Fri Feb 24 00:06:02 PST 2023
On Thu, Feb 23, 2023 at 11:24:41PM +0000, Conor Dooley wrote:
> On Tue, Feb 21, 2023 at 07:56:01PM +0100, Andrew Jones wrote:
> > Alternatives are used for both errata and cpufeatures. Use a more
> > generic name, 'patch_id', as in "ID of code patching site", to
> > avoid confusion when alternatives are used for cpufeatures.
> >
> > Signed-off-by: Andrew Jones <ajones at ventanamicro.com>
> > ---
> > arch/riscv/Kconfig | 6 +-
> > arch/riscv/errata/sifive/errata.c | 6 +-
> > arch/riscv/errata/thead/errata.c | 4 +-
> > arch/riscv/include/asm/alternative-macros.h | 72 ++++++++++-----------
> > arch/riscv/include/asm/alternative.h | 2 +-
> > arch/riscv/kernel/cpufeature.c | 6 +-
> > 6 files changed, 48 insertions(+), 48 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 736f42f572aa..0c494c36e911 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -377,9 +377,9 @@ config RISCV_ALTERNATIVE
> > depends on !XIP_KERNEL
> > help
> > This Kconfig allows the kernel to automatically patch the
> > - errata required by the execution platform at run time. The
> > - code patching is performed once in the boot stages. It means
> > - that the overhead from this mechanism is just taken once.
> > + erratum or cpufeature required by the execution platform at run
> > + time. The code patching is performed once in the boot stages. It
> > + means that the overhead from this mechanism is just taken once.
>
> A line in this comment doesn't really make much sense to an unfamiliar
> reader. "Once in the boot stages"? That's just not true, is it?
> Code patching for alternatives happens more than once and not just
> during boot? Not this patch's doing though!
Right, code patching also happens at module load time. Reworking this
paragraph would be another nice cleanup. I can respin this series with
another patch doing that, along with picking up other suggestions you
have.
>
> > diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> > index 3beef400a971..8f39d4e8598d 100644
> > --- a/arch/riscv/include/asm/alternative.h
> > +++ b/arch/riscv/include/asm/alternative.h
> > @@ -38,7 +38,7 @@ struct alt_entry {
> > s32 alt_offset; /* offset relative to replacement instruction or data */
> > u16 vendor_id; /* cpu vendor id */
> > u16 alt_len; /* The replacement size */
> > - u32 errata_id; /* The errata id */
> > + u32 patch_id; /* The patch ID (erratum ID or cpufeature ID) */
>
> The inconsistent use of ID versus id in these comments bothers my OCD!
I'll sneak a change in of vendor_id's comment to 'CPU vendor ID' for v2.
>
> Either way, "patch" is a more accurate description of the usage than
> errata, so I am on board.
> Reviewed-by: Conor Dooley <conor.dooley at microchip.com>
Thanks,
drew
More information about the linux-riscv
mailing list