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

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

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
>
>
>