linux-mips
[Top] [All Lists]

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

To: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>, "Steven J. Hill" <Steven.Hill@imgtec.com>
Subject: Re: [PATCH v3] MIPS: micromips: Fix improper definition of ISA exception bit.
From: David Daney <ddaney.cavm@gmail.com>
Date: Wed, 05 Jun 2013 09:36:27 -0700
Cc: linux-mips@linux-mips.org, ralf@linux-mips.org
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=hWfjmYrBggh/blImod7dLNR6sgHcEm2R8vF2ml4/0NM=; b=sxgKGXlmpL5M+C/QPz8/2jgHoT84UTdMlx0HOSLEOHQPwEKiqP8nHywwMF3sVsFLyq omVk3aua0qKR24r195SfJ0goDosaCb1OdUbNr0bWG/4TIUngJ4xh7QsJDQKHgrYTTYs0 NKc6A9tV9GjupapQcu70+/xB0trIKLSIRuaypae3kvA6U9vgGpL3vHt9eMh5/NjeYcIr FLIINECvmL5gCXT/hjl+h16TG5x0ky5cWhmK67zMTZdFifKta1yDFX6F1uijsR7Jhq8Z /IQmFWxWKFi4KEpCTmdxXaQojVBGFXlD9V4HnQFyu24BBEOsTKsCh2Fjq0lMNfAlwk3O sG1w==
In-reply-to: <51AF1C0B.6090904@cogentembedded.com>
List-archive: <http://www.linux-mips.org/archives/linux-mips/>
List-help: <mailto:ecartis@linux-mips.org?Subject=help>
List-id: linux-mips <linux-mips.eddie.linux-mips.org>
List-owner: <mailto:ralf@linux-mips.org>
List-post: <mailto:linux-mips@linux-mips.org>
List-software: Ecartis version 1.0.0
List-subscribe: <mailto:ecartis@linux-mips.org?subject=subscribe%20linux-mips>
List-unsubscribe: <mailto:ecartis@linux-mips.org?subject=unsubscribe%20linux-mips>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <1370372979-20634-1-git-send-email-Steven.Hill@imgtec.com> <51AF1C0B.6090904@cogentembedded.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4
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





<Prev in Thread] Current Thread [Next in Thread>