diff mbox series

[v2,1/3] RISC-V: Add separate asm/encoding.h for spec related defines

Message ID 20190413153807.116227-2-anup.patel@wdc.com (mailing list archive)
State New, archived
Headers show
Series Allow accessing CSR using CSR number | expand

Commit Message

Anup Patel April 13, 2019, 3:38 p.m. UTC 
It's better to have all RISC-V spec related defines in one place
so this patch adds separate asm/encoding.h for such defines which
can be included in assembly as well as C code.

Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 arch/riscv/include/asm/csr.h      | 52 +-------------------------
 arch/riscv/include/asm/encoding.h | 62 +++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 51 deletions(-)
 create mode 100644 arch/riscv/include/asm/encoding.h

Comments

Christoph Hellwig April 15, 2019, 5:49 a.m. UTC | #1 
On Sat, Apr 13, 2019 at 03:38:35PM +0000, Anup Patel wrote:
> It's better to have all RISC-V spec related defines in one place
> so this patch adds separate asm/encoding.h for such defines which
> can be included in assembly as well as C code.

As per the discussion of version 1 on Saturday I disagree with this
move.  There is both a philosophical and a practical reason for that:

 a) in RISC-V CSR access is really nicely split out into a separate
    number space, just accessed through a few special instructions.
    It has no overlap with the rest of instruction encoding
 b) <asm/csr.h> is pulled into just about every file build in the kernel
    though <asm/irqflags.h>, <asm/switch_to.h> and <asm/thread_info.h>

So even if you want to later add a new <asm/encoding.h> for instruction
encoding details later, I'd rather prefer to keep CSR access separate
from it.
Anup Patel April 15, 2019, 7:29 a.m. UTC | #2 
On Mon, Apr 15, 2019 at 11:19 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Sat, Apr 13, 2019 at 03:38:35PM +0000, Anup Patel wrote:
> > It's better to have all RISC-V spec related defines in one place
> > so this patch adds separate asm/encoding.h for such defines which
> > can be included in assembly as well as C code.
>
> As per the discussion of version 1 on Saturday I disagree with this
> move.  There is both a philosophical and a practical reason for that:
>
>  a) in RISC-V CSR access is really nicely split out into a separate
>     number space, just accessed through a few special instructions.
>     It has no overlap with the rest of instruction encoding
>  b) <asm/csr.h> is pulled into just about every file build in the kernel
>     though <asm/irqflags.h>, <asm/switch_to.h> and <asm/thread_info.h>
>
> So even if you want to later add a new <asm/encoding.h> for instruction
> encoding details later, I'd rather prefer to keep CSR access separate
> from it.

Sure, no problem. I will add asm/encoding.h for instructions later.

Regards,
Anup
diff mbox series

Patch

diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index 28a0d1cb374c..8cf698e39463 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -14,57 +14,7 @@ 
 #ifndef _ASM_RISCV_CSR_H
 #define _ASM_RISCV_CSR_H
 
-#include <linux/const.h>
-
-/* Status register flags */
-#define SR_SIE	_AC(0x00000002, UL) /* Supervisor Interrupt Enable */
-#define SR_SPIE	_AC(0x00000020, UL) /* Previous Supervisor IE */
-#define SR_SPP	_AC(0x00000100, UL) /* Previously Supervisor */
-#define SR_SUM	_AC(0x00040000, UL) /* Supervisor may access User Memory */
-
-#define SR_FS           _AC(0x00006000, UL) /* Floating-point Status */
-#define SR_FS_OFF       _AC(0x00000000, UL)
-#define SR_FS_INITIAL   _AC(0x00002000, UL)
-#define SR_FS_CLEAN     _AC(0x00004000, UL)
-#define SR_FS_DIRTY     _AC(0x00006000, UL)
-
-#define SR_XS           _AC(0x00018000, UL) /* Extension Status */
-#define SR_XS_OFF       _AC(0x00000000, UL)
-#define SR_XS_INITIAL   _AC(0x00008000, UL)
-#define SR_XS_CLEAN     _AC(0x00010000, UL)
-#define SR_XS_DIRTY     _AC(0x00018000, UL)
-
-#ifndef CONFIG_64BIT
-#define SR_SD   _AC(0x80000000, UL) /* FS/XS dirty */
-#else
-#define SR_SD   _AC(0x8000000000000000, UL) /* FS/XS dirty */
-#endif
-
-/* SATP flags */
-#if __riscv_xlen == 32
-#define SATP_PPN     _AC(0x003FFFFF, UL)
-#define SATP_MODE_32 _AC(0x80000000, UL)
-#define SATP_MODE    SATP_MODE_32
-#else
-#define SATP_PPN     _AC(0x00000FFFFFFFFFFF, UL)
-#define SATP_MODE_39 _AC(0x8000000000000000, UL)
-#define SATP_MODE    SATP_MODE_39
-#endif
-
-/* Interrupt Enable and Interrupt Pending flags */
-#define SIE_SSIE _AC(0x00000002, UL) /* Software Interrupt Enable */
-#define SIE_STIE _AC(0x00000020, UL) /* Timer Interrupt Enable */
-#define SIE_SEIE _AC(0x00000200, UL) /* External Interrupt Enable */
-
-#define EXC_INST_MISALIGNED     0
-#define EXC_INST_ACCESS         1
-#define EXC_BREAKPOINT          3
-#define EXC_LOAD_ACCESS         5
-#define EXC_STORE_ACCESS        7
-#define EXC_SYSCALL             8
-#define EXC_INST_PAGE_FAULT     12
-#define EXC_LOAD_PAGE_FAULT     13
-#define EXC_STORE_PAGE_FAULT    15
+#include <asm/encoding.h>
 
 #ifndef __ASSEMBLY__
 
diff --git a/arch/riscv/include/asm/encoding.h b/arch/riscv/include/asm/encoding.h
new file mode 100644
index 000000000000..29699705dc36
--- /dev/null
+++ b/arch/riscv/include/asm/encoding.h
@@ -0,0 +1,62 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2015 Regents of the University of California
+ * Copyright (C) 2019 Western Digital Corporation or its affiliates.
+ */
+
+#ifndef _ASM_RISCV_ENCODING_H
+#define _ASM_RISCV_ENCODING_H
+
+#include <linux/const.h>
+
+/* Status register flags */
+#define SR_SIE		_AC(0x00000002, UL) /* Supervisor Interrupt Enable */
+#define SR_SPIE		_AC(0x00000020, UL) /* Previous Supervisor IE */
+#define SR_SPP		_AC(0x00000100, UL) /* Previously Supervisor */
+#define SR_SUM		_AC(0x00040000, UL) /* Supervisor may access User Memory */
+
+#define SR_FS           _AC(0x00006000, UL) /* Floating-point Status */
+#define SR_FS_OFF       _AC(0x00000000, UL)
+#define SR_FS_INITIAL   _AC(0x00002000, UL)
+#define SR_FS_CLEAN     _AC(0x00004000, UL)
+#define SR_FS_DIRTY     _AC(0x00006000, UL)
+
+#define SR_XS           _AC(0x00018000, UL) /* Extension Status */
+#define SR_XS_OFF       _AC(0x00000000, UL)
+#define SR_XS_INITIAL   _AC(0x00008000, UL)
+#define SR_XS_CLEAN     _AC(0x00010000, UL)
+#define SR_XS_DIRTY     _AC(0x00018000, UL)
+
+#ifndef CONFIG_64BIT
+#define SR_SD		_AC(0x80000000, UL) /* FS/XS dirty */
+#else
+#define SR_SD		_AC(0x8000000000000000, UL) /* FS/XS dirty */
+#endif
+
+/* SATP flags */
+#ifndef CONFIG_64BIT
+#define SATP_PPN	_AC(0x003FFFFF, UL)
+#define SATP_MODE_32	_AC(0x80000000, UL)
+#define SATP_MODE	SATP_MODE_32
+#else
+#define SATP_PPN	_AC(0x00000FFFFFFFFFFF, UL)
+#define SATP_MODE_39	_AC(0x8000000000000000, UL)
+#define SATP_MODE	SATP_MODE_39
+#endif
+
+/* Interrupt Enable and Interrupt Pending flags */
+#define SIE_SSIE	_AC(0x00000002, UL) /* Software Interrupt Enable */
+#define SIE_STIE	_AC(0x00000020, UL) /* Timer Interrupt Enable */
+#define SIE_SEIE	_AC(0x00000200, UL) /* External Interrupt Enable */
+
+#define EXC_INST_MISALIGNED	0
+#define EXC_INST_ACCESS		1
+#define EXC_BREAKPOINT		3
+#define EXC_LOAD_ACCESS		5
+#define EXC_STORE_ACCESS	7
+#define EXC_SYSCALL		8
+#define EXC_INST_PAGE_FAULT	12
+#define EXC_LOAD_PAGE_FAULT	13
+#define EXC_STORE_PAGE_FAULT	15
+
+#endif /* _ASM_RISCV_ENCODING_H */