Re: [PATCH v7] MIPS: micromips: Fix improper definition of ISA exception bit.

[Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>]

On 06/06/2013 01:13 AM, Steven J. Hill wrote:
> The ISA exception bit selects whether exceptions are taken in classic
> or microMIPS mode. This bit is Config3.ISAOnExc and was improperly
> defined as bits 16 and 17 instead of just bit 16. A new function was
> added so that platforms could set this bit when running a kernel
> compiled with only microMIPS instructions.
>
> Signed-off-by: Steven J. Hill <Steven.Hill@imgtec.com>
> Acked-by: David Daney <david.daney@cavium.com>
> ---
> Changes from v6:
> * Add '#ifdef SYS_SUPPORTS_MICROMIPS' around body of
>    'set_micromips_exception_mode' function. Platforms that
>    do not support microMIPS will optimize it out.
>
>   arch/mips/include/asm/mipsregs.h |   20 +++++++++++++++++++-
>   arch/mips/kernel/cpu-probe.c     |    3 ---
>   arch/mips/kernel/traps.c         |    5 +++++
>   3 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
> index 87e6207..0a9544a 100644
> --- a/arch/mips/include/asm/mipsregs.h
> +++ b/arch/mips/include/asm/mipsregs.h
> @@ -596,7 +596,7 @@
>   #define MIPS_CONF3_RXI		(_ULCAST_(1) << 12)
>   #define MIPS_CONF3_ULRI		(_ULCAST_(1) << 13)
>   #define MIPS_CONF3_ISA		(_ULCAST_(3) << 14)
> -#define MIPS_CONF3_ISA_OE	(_ULCAST_(3) << 16)
> +#define MIPS_CONF3_ISA_OE	(_ULCAST_(1) << 16)
>   #define MIPS_CONF3_VZ		(_ULCAST_(1) << 23)
>   
>   #define MIPS_CONF4_MMUSIZEEXT	(_ULCAST_(255) << 0)
> @@ -1161,6 +1161,24 @@ do {									\
>   #define write_c0_brcm_sleepcount(val)	__write_32bit_c0_register($22, 7, val)
>   
>   /*
> + * Set exceptions to be taken in microMIPS mode only, otherwise
> + * set for classic exceptions.
> + */
> +static inline void set_micromips_exception_mode(void)
> +{
> +#ifdef SYS_SUPPORTS_MICROMIPS
> +	unsigned int config3 = read_c0_config3();
> +
> +#ifdef CONFIG_CPU_MICROMIPS
> +	if (config3 & MIPS_CONF3_ISA)
> +		write_c0_config3(config3 | MIPS_CONF3_ISA_OE);
> +	else
> +#endif
> +		write_c0_config3(config3 & ~MIPS_CONF3_ISA_OE);
> +#endif

    Let me remind that #ifdef's inside the function body are considered 
ugly and
should be avoided if at all possible (by defining an empty 
implementation in the
#else branch).

WBR, Sergei