Re: [PATCH v1 03/14] xen/riscv: add <asm/riscv_encoding.h header

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Mon, 2023-01-23 at 14:52 +0100, Jan Beulich wrote:
> On 20.01.2023 15:59, Oleksii Kurochko wrote:
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> 
> I was about to commit this, but ...
> 
> > --- /dev/null
> > +++ b/xen/arch/riscv/include/asm/riscv_encoding.h
> > @@ -0,0 +1,945 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) */
> > +/*
> > + * Copyright (c) 2019 Western Digital Corporation or its
> > affiliates.
> > + *
> > + * Authors:
> > + *   Anup Patel <anup.patel@xxxxxxx>
> 
> ... this raises a patch authorship question: Are you missing her/his
> S-o-b: and/or From:? 
> 
It is not clear who should be in S-o-b and/or From. So let me explain
situation:

Anup Patel <anup.patel@xxxxxxx> is a person who introduced
riscv_encoding.h in OpenSBI.

A person who introduced the header to Xen isn't clear as I see 3 people
who did it:
- Bobby Eshleman <bobbyeshleman@xxxxxxxxx>
- Alistair Francis <alistair.francis@xxxxxxx>
- One more person whoose last name, unfortunately, I can't find
And in all cases I saw that an author is different.

> > + * The source has been largely adapted from OpenSBI:
> > + * include/sbi/riscv_encodnig.h
> 
> Nit: Typo.
> 
> > + * 
> 
> Nit: trailing blank.
> 
> There also look to be hard tabs in the file. This is fine if the file
> is being imported (almost) verbatim from elsewhere, but then the
> origin
> wants stating in an Origin: tag (see docs/process/sending-
> patches.pandoc).
> 
> > [...]
> > +#define IMM_I(insn)                    ((s32)(insn) >> 20)
> > +#define IMM_S(insn)                    (((s32)(insn) >> 25 << 5) |
> > \
> > +                                        (s32)(((insn) >> 7) &
> > 0x1f))
> 
> Please can you avoid introducing new instances of s<N> or u<N>? See
> ./CODING_STYLE.
> 
Thanks. I will update the header.
> Jan