Subject: Re: [PATCH v4 03/16] powerpc: Use a datatype for instructions

From: Michael Ellerman <[email-masked]>
Date: Tue, 24 Mar 2020 13:58:51 +1100

Nicholas Piggin <[email-masked]> writes:
> Jordan Niethe's on March 23, 2020 7:28 pm:
>> On Mon, Mar 23, 2020 at 5:27 PM Nicholas Piggin <[email-masked]> wrote:
>>> Jordan Niethe's on March 20, 2020 3:17 pm:
>>> > Currently unsigned ints are used to represent instructions on powerpc.
>>> > This has worked well as instructions have always been 4 byte words.
>>> > However, a future ISA version will introduce some changes to
>>> > instructions that mean this scheme will no longer work as well. This
>>> > change is Prefixed Instructions. A prefixed instruction is made up of a
>>> > word prefix followed by a word suffix to make an 8 byte double word
>>> > instruction. No matter the endianess of the system the prefix always
>>> > comes first. Prefixed instructions are only planned for powerpc64.
>>> >
>>> > Introduce a ppc_inst type to represent both prefixed and word
>>> > instructions on powerpc64 while keeping it possible to exclusively have
>>> > word instructions on powerpc32, A latter patch will expand the type to
>>> > include prefixed instructions but for now just typedef it to a u32.
>>> >
>>> > Later patches will introduce helper functions and macros for
>>> > manipulating the instructions so that powerpc64 and powerpc32 might
>>> > maintain separate type definitions.
>>>
>>> ppc_inst_t I would slightly prefer for a typedef like this.
>> Are _t types meant to be reserved?
>
> No, just convention that structs are not normally typedefed unless
> they are a pervasive interface that gets passed around a lot but
> does not get accessed without accessor functions much. When you do
> typedef them, add a _t (or less frequently _s/_u/etc). pte_t,
> cpumask_t, atomic_t.

Ideally we wouldn't use a typedef, we'd just have:

struct ppc_inst {
	u32 val;
#ifdef CONFIG_PPC64
        u32 suffix;
#endif
};

That may make the conversion harder though, because you more or less
have to update all usages at once.

cheers