All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] MIPS: micromips: Fix improper definition of ISA exception bit.
@ 2013-06-04 19:09 Steven J. Hill
  2013-06-05 11:07 ` Sergei Shtylyov
  0 siblings, 1 reply; 4+ messages in thread
From: Steven J. Hill @ 2013-06-04 19:09 UTC (permalink / raw)
  To: linux-mips; +Cc: Steven J. Hill, ralf, ddaney.cavm

The ISA exception bit selects whether exceptions are taken in classic
MIPS or microMIPS mode. This bit is Config3.ISAOnExc and is bit 16. It
It was improperly defined as bits 16 and 17. Fortunately, bit 17 is
read-only and did not effect microMIPS operation. However, detecting
a classic or microMIPS kernel when examining the /proc/cpuinfo file,
the result always showed a microMIPS kernel.

Signed-off-by: Steven J. Hill <Steven.Hill@imgtec.com>
---
 arch/mips/include/asm/mipsregs.h |    2 +-
 arch/mips/kernel/cpu-probe.c     |    7 ++-----
 arch/mips/mti-malta/malta-init.c |    7 +++++++
 arch/mips/mti-sead3/sead3-init.c |    7 +++++++
 4 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index 87e6207..fed1c3e 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)
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index c6568bf..d6c5230 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -256,6 +256,8 @@ static inline unsigned int decode_config3(struct cpuinfo_mips *c)
 		c->ases |= MIPS_ASE_SMARTMIPS;
 		c->options |= MIPS_CPU_RIXI;
 	}
+	if (config3 & MIPS_CONF3_ISA)
+		c->options |= MIPS_CPU_MICROMIPS;
 	if (config3 & MIPS_CONF3_RXI)
 		c->options |= MIPS_CPU_RIXI;
 	if (config3 & MIPS_CONF3_DSP)
@@ -270,11 +272,6 @@ static inline unsigned int decode_config3(struct cpuinfo_mips *c)
 		c->ases |= MIPS_ASE_MIPSMT;
 	if (config3 & MIPS_CONF3_ULRI)
 		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..3598f1d 100644
--- a/arch/mips/mti-malta/malta-init.c
+++ b/arch/mips/mti-malta/malta-init.c
@@ -106,6 +106,13 @@ extern struct plat_smp_ops msmtc_smp_ops;
 
 void __init prom_init(void)
 {
+#ifdef CONFIG_CPU_MICROMIPS
+	unsigned int config3 = read_c0_config3();
+
+	if (config3 & MIPS_CONF3_ISA)
+		write_c0_config3(config3 | MIPS_CONF3_ISA_OE);
+#endif
+
 	mips_display_message("LINUX");
 
 	/*
diff --git a/arch/mips/mti-sead3/sead3-init.c b/arch/mips/mti-sead3/sead3-init.c
index bfbd17b..e68bfd3 100644
--- a/arch/mips/mti-sead3/sead3-init.c
+++ b/arch/mips/mti-sead3/sead3-init.c
@@ -130,6 +130,13 @@ static void __init mips_ejtag_setup(void)
 
 void __init prom_init(void)
 {
+#ifdef CONFIG_CPU_MICROMIPS
+	unsigned int config3 = read_c0_config3();
+
+	if (config3 & MIPS_CONF3_ISA)
+		write_c0_config3(config3 | MIPS_CONF3_ISA_OE);
+#endif
+
 	board_nmi_handler_setup = mips_nmi_setup;
 	board_ejtag_handler_setup = mips_ejtag_setup;
 
-- 
1.7.2.5

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] MIPS: micromips: Fix improper definition of ISA exception bit.
  2013-06-04 19:09 [PATCH v3] MIPS: micromips: Fix improper definition of ISA exception bit Steven J. Hill
@ 2013-06-05 11:07 ` Sergei Shtylyov
  2013-06-05 16:36   ` David Daney
  0 siblings, 1 reply; 4+ messages in thread
From: Sergei Shtylyov @ 2013-06-05 11:07 UTC (permalink / raw)
  To: Steven J. Hill; +Cc: linux-mips, ralf, ddaney.cavm

Hello.

On 04-06-2013 23:09, Steven J. Hill wrote:

> The ISA exception bit selects whether exceptions are taken in classic
> MIPS or microMIPS mode. This bit is Config3.ISAOnExc and is bit 16. It
> It was improperly defined as bits 16 and 17. Fortunately, bit 17 is
> read-only and did not effect microMIPS operation. However, detecting
> a classic or microMIPS kernel when examining the /proc/cpuinfo file,
> the result always showed a microMIPS kernel.

> Signed-off-by: Steven J. Hill <Steven.Hill@imgtec.com>
> ---
>   arch/mips/include/asm/mipsregs.h |    2 +-
>   arch/mips/kernel/cpu-probe.c     |    7 ++-----
>   arch/mips/mti-malta/malta-init.c |    7 +++++++
>   arch/mips/mti-sead3/sead3-init.c |    7 +++++++
>   4 files changed, 17 insertions(+), 6 deletions(-)

[...]
> diff --git a/arch/mips/mti-malta/malta-init.c b/arch/mips/mti-malta/malta-init.c
> index ff8caff..3598f1d 100644
> --- a/arch/mips/mti-malta/malta-init.c
> +++ b/arch/mips/mti-malta/malta-init.c
> @@ -106,6 +106,13 @@ extern struct plat_smp_ops msmtc_smp_ops;
>
>   void __init prom_init(void)
>   {
> +#ifdef CONFIG_CPU_MICROMIPS
> +	unsigned int config3 = read_c0_config3();
> +
> +	if (config3 & MIPS_CONF3_ISA)
> +		write_c0_config3(config3 | MIPS_CONF3_ISA_OE);
> +#endif
> +
>   	mips_display_message("LINUX");
>
>   	/*
> diff --git a/arch/mips/mti-sead3/sead3-init.c b/arch/mips/mti-sead3/sead3-init.c
> index bfbd17b..e68bfd3 100644
> --- a/arch/mips/mti-sead3/sead3-init.c
> +++ b/arch/mips/mti-sead3/sead3-init.c
> @@ -130,6 +130,13 @@ static void __init mips_ejtag_setup(void)
>
>   void __init prom_init(void)
>   {
> +#ifdef CONFIG_CPU_MICROMIPS
> +	unsigned int config3 = read_c0_config3();
> +
> +	if (config3 & MIPS_CONF3_ISA)
> +		write_c0_config3(config3 | MIPS_CONF3_ISA_OE);
> +#endif
> +

    I see it's repeated twice and enclosed in #ifdef... Couldn't you
factor it out in some sort of inline function and put into some header:

#ifdef CONFIG_CPU_MICROMIPS
static inline void mips_set_config3_isa_oe(void)
{
	unsigned int config3 = read_c0_config3();

	if (config3 & MIPS_CONF3_ISA)
		write_c0_config3(config3 | MIPS_CONF3_ISA_OE);
}
#else
static inline void mips_set_config3_isa_oe(void) {}
#endif

WBR, Sergei

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] MIPS: micromips: Fix improper definition of ISA exception bit.
  2013-06-05 11:07 ` Sergei Shtylyov
@ 2013-06-05 16:36   ` David Daney
  2013-06-05 17:01     ` Sergei Shtylyov
  0 siblings, 1 reply; 4+ messages in thread
From: David Daney @ 2013-06-05 16:36 UTC (permalink / raw)
  To: Sergei Shtylyov, Steven J. Hill; +Cc: linux-mips, ralf

On 06/05/2013 04:07 AM, Sergei Shtylyov wrote:
> Hello.
>
> On 04-06-2013 23:09, Steven J. Hill wrote:
>
>> The ISA exception bit selects whether exceptions are taken in classic
>> MIPS or microMIPS mode. This bit is Config3.ISAOnExc and is bit 16. It
>> It was improperly defined as bits 16 and 17. Fortunately, bit 17 is
>> read-only and did not effect microMIPS operation. However, detecting
>> a classic or microMIPS kernel when examining the /proc/cpuinfo file,
>> the result always showed a microMIPS kernel.

I don't see anything in the patch that would make Classic CPUs be 
misidentified.  Is the change log still accurate?

...

>> Signed-off-by: Steven J. Hill <Steven.Hill@imgtec.com>
>> ---
>>   arch/mips/include/asm/mipsregs.h |    2 +-
>>   arch/mips/kernel/cpu-probe.c     |    7 ++-----
>>   arch/mips/mti-malta/malta-init.c |    7 +++++++
>>   arch/mips/mti-sead3/sead3-init.c |    7 +++++++
>>   4 files changed, 17 insertions(+), 6 deletions(-)
>
> [...]
>> diff --git a/arch/mips/mti-malta/malta-init.c
>> b/arch/mips/mti-malta/malta-init.c
>> index ff8caff..3598f1d 100644
[...]
>> --- a/arch/mips/mti-sead3/sead3-init.c
>> +++ b/arch/mips/mti-sead3/sead3-init.c
>> @@ -130,6 +130,13 @@ static void __init mips_ejtag_setup(void)
>>
>>   void __init prom_init(void)
>>   {
>> +#ifdef CONFIG_CPU_MICROMIPS
>> +    unsigned int config3 = read_c0_config3();
>> +
>> +    if (config3 & MIPS_CONF3_ISA)
>> +        write_c0_config3(config3 | MIPS_CONF3_ISA_OE);
>> +#endif
>> +
>
>     I see it's repeated twice and enclosed in #ifdef... Couldn't you
> factor it out in some sort of inline function and put into some header:
>
> #ifdef CONFIG_CPU_MICROMIPS
> static inline void mips_set_config3_isa_oe(void)


I don't have a strong opinion about factoring it out like this, but we 
do, let's give it a better name.  Something like 
enable_micromips_exception_mode() or similar.  That way we know what it 
does.

> {
>      unsigned int config3 = read_c0_config3();
>
>      if (config3 & MIPS_CONF3_ISA)
>          write_c0_config3(config3 | MIPS_CONF3_ISA_OE);
> }
> #else
> static inline void mips_set_config3_isa_oe(void) {}
> #endif
>
> WBR, Sergei
>
>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] MIPS: micromips: Fix improper definition of ISA exception bit.
  2013-06-05 16:36   ` David Daney
@ 2013-06-05 17:01     ` Sergei Shtylyov
  0 siblings, 0 replies; 4+ messages in thread
From: Sergei Shtylyov @ 2013-06-05 17:01 UTC (permalink / raw)
  To: David Daney; +Cc: Steven J. Hill, linux-mips, ralf

Hello.

On 06/05/2013 08:36 PM, David Daney wrote:

>
>>
>>> The ISA exception bit selects whether exceptions are taken in classic
>>> MIPS or microMIPS mode. This bit is Config3.ISAOnExc and is bit 16. It
>>> It was improperly defined as bits 16 and 17. Fortunately, bit 17 is
>>> read-only and did not effect microMIPS operation. However, detecting
>>> a classic or microMIPS kernel when examining the /proc/cpuinfo file,
>>> the result always showed a microMIPS kernel.
>
> I don't see anything in the patch that would make Classic CPUs be 
> misidentified.  Is the change log still accurate?
>
> ...
>
>>> Signed-off-by: Steven J. Hill <Steven.Hill@imgtec.com>
>>> ---
>>>   arch/mips/include/asm/mipsregs.h |    2 +-
>>>   arch/mips/kernel/cpu-probe.c     |    7 ++-----
>>>   arch/mips/mti-malta/malta-init.c |    7 +++++++
>>>   arch/mips/mti-sead3/sead3-init.c |    7 +++++++
>>>   4 files changed, 17 insertions(+), 6 deletions(-)
>>
>> [...]
>>> diff --git a/arch/mips/mti-malta/malta-init.c
>>> b/arch/mips/mti-malta/malta-init.c
>>> index ff8caff..3598f1d 100644
> [...]
>>> --- a/arch/mips/mti-sead3/sead3-init.c
>>> +++ b/arch/mips/mti-sead3/sead3-init.c
>>> @@ -130,6 +130,13 @@ static void __init mips_ejtag_setup(void)
>>>
>>>   void __init prom_init(void)
>>>   {
>>> +#ifdef CONFIG_CPU_MICROMIPS
>>> +    unsigned int config3 = read_c0_config3();
>>> +
>>> +    if (config3 & MIPS_CONF3_ISA)
>>> +        write_c0_config3(config3 | MIPS_CONF3_ISA_OE);
>>> +#endif
>>> +
>>
>>     I see it's repeated twice and enclosed in #ifdef... Couldn't you
>> factor it out in some sort of inline function and put into some header:
>>
>> #ifdef CONFIG_CPU_MICROMIPS
>> static inline void mips_set_config3_isa_oe(void)
>
>
> I don't have a strong opinion about factoring it out like this, but we 
> do, let's give it a better name.  Something like 
> enable_micromips_exception_mode() or similar.  That way we know what 
> it does.

     Yes, the name was only a (bad) example, since I didn't really 
understand what the code was doing.

>
>> {
>>      unsigned int config3 = read_c0_config3();
>>
>>      if (config3 & MIPS_CONF3_ISA)
>>          write_c0_config3(config3 | MIPS_CONF3_ISA_OE);
>> }
>> #else
>> static inline void mips_set_config3_isa_oe(void) {}
>> #endif
>>

WBR, Sergei

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-06-05 17:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-04 19:09 [PATCH v3] MIPS: micromips: Fix improper definition of ISA exception bit Steven J. Hill
2013-06-05 11:07 ` Sergei Shtylyov
2013-06-05 16:36   ` David Daney
2013-06-05 17:01     ` Sergei Shtylyov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.