Re: [PATCH 1/6] powerpc/ppc-opcode: introduce PPC_RAW_* macros for base instruction encoding

[Michael Ellerman <mpe@ellerman.id.au>]

Balamuruhan S <bala24@linux.ibm.com> writes:
> Introduce PPC_RAW_* macros to have all the bare encoding of ppc
> instructions. Move `VSX_XX*()` and `TMRN()` macros up to reuse it.
>
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Tested-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/ppc-opcode.h | 183 ++++++++++++++++++++++++--
>  1 file changed, 175 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
> index 2a39c716c343..e3540be1fc17 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -431,6 +431,181 @@
>  #define __PPC_EH(eh)	0
>  #endif
>  
> +/* Base instruction encoding */
> +#define PPC_RAW_CP_ABORT		(PPC_INST_CP_ABORT)
> +#define PPC_RAW_COPY(a, b)		(PPC_INST_COPY | ___PPC_RA(a) | \
> +						___PPC_RB(b))
> +#define PPC_RAW_DARN(t, l)		(PPC_INST_DARN | ___PPC_RT(t) | \
> +						(((l) & 0x3) << 16))

I know you're copying the existing formatting by wrapping these lines,
but please don't.

It hurts rather than improves readability.

For a line like the one above, just let it be long, eg:

#define PPC_RAW_DARN(t, l)		(PPC_INST_DARN | ___PPC_RT(t) | (((l) & 0x3) << 16))

Yeah it's 91 columns but who cares.

For the really long ones like:

> +#define PPC_RAW_TLBIE_5(rb, rs, ric, prs, r) \
> +					(PPC_INST_TLBIE | \
> +						___PPC_RB(rb) | \
> +						___PPC_RS(rs) | \
> +						___PPC_RIC(ric) | \
> +						___PPC_PRS(prs) | \
> +						___PPC_R(r))

I think this is the best option:

#define PPC_RAW_TLBIE_5(rb, rs, ric, prs, r) \
	(PPC_INST_TLBIE | ___PPC_RB(rb) | ___PPC_RS(rs) | ___PPC_RIC(ric) | ___PPC_PRS(prs) | ___PPC_R(r))


Which is long, but I don't think wrapping it helps.

If we didn't have those ridiculous ___PPC_RX macros it would be a bit
shorter, but I guess that's a rework for another day.


cheers