linux-mips
[Top] [All Lists]

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

To: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Subject: Re: [PATCH v6] MIPS: micromips: Fix improper definition of ISA exception bit.
From: David Daney <ddaney.cavm@gmail.com>
Date: Wed, 05 Jun 2013 14:07:05 -0700
Cc: "Steven J. Hill" <Steven.Hill@imgtec.com>, 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=Zl44ksQMkrkUdaO2X07BHILvOh9vsCCi4KP5Yv8eQow=; b=yplT22f/SVHib1mqsF7mwaDgLxHq0YTb6RucOXgQuRTn9eiNJ4E9rHinUt1Y3Qm7Or R54A33cNF2YwFvaqDzwpPRXL4D60xor+ecICEJXkCRLs9U7rPzm6n1bxkrdRyMV7Xmxp qTKjFCxFRwHEjVhOnvaGj6jYbNI+NI4Yj3MgGI9ZlQp7g9WhZ0D+TsM2pqIVbv4DpZ7w MBNXdC9SHkovfmFWtzwgUCAXoSOhd+MNnwYNHUSzQY1LhrAJJLSq0c0kyFIvttd1Lupj a5wwHO1kA+dNcMLsI3kFSheRvMGbJAWa//FmqDRfLw6p3EyePpuxKLS+WOCpDpXYfhIW NfTg==
In-reply-to: <51AF9923.7000606@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: <1370461798-20296-1-git-send-email-Steven.Hill@imgtec.com> <51AF9923.7000606@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 01:01 PM, Sergei Shtylyov wrote:
Hello.

On 06/05/2013 11:49 PM, 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.

     Ahem, isn't that function a material for another patch?


I think you might be going overboard. The entire patch relates to exactly one bit a config register, can't you let it be a single patch?

David Daney


Signed-off-by: Steven J. Hill <Steven.Hill@imgtec.com>
---
Changes from v5:
* Make 'set_micromips_exception_mode' function to always be called.

  arch/mips/include/asm/mipsregs.h |   18 +++++++++++++++++-
  arch/mips/kernel/cpu-probe.c     |    3 ---
  arch/mips/kernel/traps.c         |    5 +++++
  3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/arch/mips/include/asm/mipsregs.h
b/arch/mips/include/asm/mipsregs.h
index 87e6207..cc0f5d7 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,22 @@ 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)
+{
+    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);
+}
+
+/*
   * Macros to access the floating point coprocessor control registers
   */
  #define read_32bit_cp1_register(source)                    \
[...]
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index a75ae40..151ed59 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -1837,6 +1837,11 @@ void __init trap_init(void)
              ebase += (read_c0_ebase() & 0x3ffff000);
      }
+    /*
+     * Set microMIPS exceptions for platforms that support it.
+     */
+    set_micromips_exception_mode();

     If we have reduced the call sites to 1, do we still need the
function? :-)

WBR, Sergei






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