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

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

[Steven J. Hill]

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>
---
Changes in v4:
* Removed code from 'cpu-probe.c' and added new inline function to
  set exception mode.
* Reworded and simplified commit message.

 arch/mips/include/asm/mipsregs.h |   17 ++++++++++++++++-
 arch/mips/kernel/cpu-probe.c     |    3 ---
 arch/mips/mti-malta/malta-init.c |    2 ++
 arch/mips/mti-sead3/sead3-init.c |    2 ++
 4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index 87e6207..434fd26 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,21 @@ do {									\
 #define write_c0_brcm_sleepcount(val)	__write_32bit_c0_register($22, 7, val)
 
 /*
+ * Set exceptions to be taken in microMIPS mode only.
+ */
+#ifdef CONFIG_CPU_MICROMIPS
+static inline void set_micromips_exception_mode(void)
+{
+	unsigned int config3 = read_c0_config3();
+
+	if (config3 & MIPS_CONF3_ISA)
+		write_c0_config3(config3 | MIPS_CONF3_ISA_OE);
+}
+#else
+static inline void set_micromips_exception_mode(void) {}
+#endif 
+
+/*
  * Macros to access the floating point coprocessor control registers
  */
 #define read_32bit_cp1_register(source)					\
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index c6568bf..b0d04a2 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -272,9 +272,6 @@ static inline unsigned int decode_config3(struct cpuinfo_mips *c)
 		c->options |= MIPS_CPU_ULRI;
 	if (config3 & MIPS_CONF3_ISA)
 		c->options |= MIPS_CPU_MICROMIPS;
-#ifdef CONFIG_CPU_MICROMIPS
-	write_c0_config3(read_c0_config3() | MIPS_CONF3_ISA_OE);
-#endif
 	if (config3 & MIPS_CONF3_VZ)
 		c->ases |= MIPS_ASE_VZ;
 
diff --git a/arch/mips/mti-malta/malta-init.c b/arch/mips/mti-malta/malta-init.c
index ff8caff..76e0205 100644
--- a/arch/mips/mti-malta/malta-init.c
+++ b/arch/mips/mti-malta/malta-init.c
@@ -106,6 +106,8 @@ extern struct plat_smp_ops msmtc_smp_ops;
 
 void __init prom_init(void)
 {
+	set_micromips_exception_mode();
+
 	mips_display_message("LINUX");
 
 	/*
diff --git a/arch/mips/mti-sead3/sead3-init.c b/arch/mips/mti-sead3/sead3-init.c
index bfbd17b..9e314cb 100644
--- a/arch/mips/mti-sead3/sead3-init.c
+++ b/arch/mips/mti-sead3/sead3-init.c
@@ -130,6 +130,8 @@ static void __init mips_ejtag_setup(void)
 
 void __init prom_init(void)
 {
+	set_micromips_exception_mode();
+
 	board_nmi_handler_setup = mips_nmi_setup;
 	board_ejtag_handler_setup = mips_ejtag_setup;
 
-- 
1.7.2.5

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

[David Daney]

On 06/05/2013 11:05 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>

This looks good now,
Acked-by: David Daney <david.daney@cavium.com>

> ---
> Changes in v4:
> * Removed code from 'cpu-probe.c' and added new inline function to
>    set exception mode.
> * Reworded and simplified commit message.
>
>   arch/mips/include/asm/mipsregs.h |   17 ++++++++++++++++-
>   arch/mips/kernel/cpu-probe.c     |    3 ---
>   arch/mips/mti-malta/malta-init.c |    2 ++
>   arch/mips/mti-sead3/sead3-init.c |    2 ++
>   4 files changed, 20 insertions(+), 4 deletions(-)
>

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

[Maciej W. Rozycki]

On Wed, 5 Jun 2013, Steven J. Hill wrote:

> diff --git a/arch/mips/mti-malta/malta-init.c b/arch/mips/mti-malta/malta-init.c
> index ff8caff..76e0205 100644
> --- a/arch/mips/mti-malta/malta-init.c
> +++ b/arch/mips/mti-malta/malta-init.c
> @@ -106,6 +106,8 @@ extern struct plat_smp_ops msmtc_smp_ops;
>  
>  void __init prom_init(void)
>  {
> +	set_micromips_exception_mode();
> +
>  	mips_display_message("LINUX");
>  
>  	/*
> diff --git a/arch/mips/mti-sead3/sead3-init.c b/arch/mips/mti-sead3/sead3-init.c
> index bfbd17b..9e314cb 100644
> --- a/arch/mips/mti-sead3/sead3-init.c
> +++ b/arch/mips/mti-sead3/sead3-init.c
> @@ -130,6 +130,8 @@ static void __init mips_ejtag_setup(void)
>  
>  void __init prom_init(void)
>  {
> +	set_micromips_exception_mode();
> +
>  	board_nmi_handler_setup = mips_nmi_setup;
>  	board_ejtag_handler_setup = mips_ejtag_setup;

 Shouldn't this be in a generic place such as trap_init instead?

  Maciej

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

[David Daney]

On 06/05/2013 12:08 PM, Maciej W. Rozycki wrote:
> On Wed, 5 Jun 2013, Steven J. Hill wrote:
>
>> diff --git a/arch/mips/mti-malta/malta-init.c b/arch/mips/mti-malta/malta-init.c
>> index ff8caff..76e0205 100644
>> --- a/arch/mips/mti-malta/malta-init.c
>> +++ b/arch/mips/mti-malta/malta-init.c
>> @@ -106,6 +106,8 @@ extern struct plat_smp_ops msmtc_smp_ops;
>>
>>   void __init prom_init(void)
>>   {
>> +	set_micromips_exception_mode();
>> +
>>   	mips_display_message("LINUX");
>>
>>   	/*
>> diff --git a/arch/mips/mti-sead3/sead3-init.c b/arch/mips/mti-sead3/sead3-init.c
>> index bfbd17b..9e314cb 100644
>> --- a/arch/mips/mti-sead3/sead3-init.c
>> +++ b/arch/mips/mti-sead3/sead3-init.c
>> @@ -130,6 +130,8 @@ static void __init mips_ejtag_setup(void)
>>
>>   void __init prom_init(void)
>>   {
>> +	set_micromips_exception_mode();
>> +
>>   	board_nmi_handler_setup = mips_nmi_setup;
>>   	board_ejtag_handler_setup = mips_ejtag_setup;
>
>   Shouldn't this be in a generic place such as trap_init instead?
>

I think it is fine here.  If it spreads to more systems, then factoring 
them out into trap_init might make sense.  For now it doesn't seem like 
we should clutter up trap_init when there aren't many microMIPS systems 
in existence.

David Daney


>    Maciej
>
>

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

[Maciej W. Rozycki]

On Wed, 5 Jun 2013, David Daney wrote:

> >   Shouldn't this be in a generic place such as trap_init instead?
> > 
> 
> I think it is fine here.  If it spreads to more systems, then factoring them
> out into trap_init might make sense.  For now it doesn't seem like we should
> clutter up trap_init when there aren't many microMIPS systems in existence.

 I disagree.  I don't think a generic processor feature should be handled 
in board-specific files.  A kernel built as a microMIPS binary has 
microMIPS exception handlers so no matter what system it is for it'll need 
the mode bit set correctly (unless someone implements support for a mixed 
setup), so having to add this change to some board-specific file for every 
system that gets support for a microMIPS-ISA-enabled processor looks like 
no more than a maintenance burden to me.  Especially as the !CPU_MICROMIPS 
version of the function is empty and will be optimised away (although 
actually it shouldn't be -- it should clear the ISAOnExc bit to avoid 
surprises).

  Maciej