Re: [PATCH] MIPS: microMIPS: Refactor mips16 get_frame_info support

[David Daney <ddaney.cavm@gmail.com>]

On 05/24/2013 07:55 AM, Tony Wu wrote:
[...]
>
> Signed-off-by: Tony Wu <tung7970@gmail.com>
> Cc: Steven J. Hill <Steven.Hill@imgtec.com>
> ---
>   arch/mips/kernel/process.c |  214 +++++++++++++++++++++++++++++---------------
>   1 file changed, 141 insertions(+), 73 deletions(-)
>
> diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
> index c6a041d..c335a7f 100644
> --- a/arch/mips/kernel/process.c
> +++ b/arch/mips/kernel/process.c
> @@ -211,14 +211,17 @@ struct mips_frame_info {
>   	int		pc_offset;
>   };
>
> +#ifdef CONFIG_CPU_MICROMIPS
> +#define J_TARGET(pc,target)	\
> +		(((unsigned long)(pc) & 0xf8000000) | ((target) << 1))
> +#else
>   #define J_TARGET(pc,target)	\
>   		(((unsigned long)(pc) & 0xf0000000) | ((target) << 2))
> +#endif

I really dislike this #ifdefery.


>
>   static inline int is_ra_save_ins(union mips_instruction *ip)
>   {
>   #ifdef CONFIG_CPU_MICROMIPS

And here too.

Would it be better to leave the existing objects alone, and add 
microMIPS functions and macros with new names?



[...]
> + * 2. access the fetched word using halfword (defeat endian issue)
> + * 3. assemble 16/32 bit MIPS16 instruction from halfword(s)
> + */
> +static inline void MIPS16_fetch_halfword(union mips_instruction **ip,

No inline for any of these functions.

> +					 unsigned short *this_halfword,
> +					 unsigned short *prev_halfword)
> +{
> +	if (*prev_halfword) {
[...]
David Daney