[PATCH v2 0/5] New RISC-V Local Interrupt Controller Driver

[PATCH v2 0/5] New RISC-V Local Interrupt Controller Driver

[Anup Patel]

This patchset provides a new RISC-V Local Interrupt Controller Driver
for managing per-CPU local interrupts. The overall approach is inspired
from the way per-CPU local interrupts are handled by Linux ARM64 and
ARM GICv3 driver.

Few advantages of having this new driver are as follows:
1. It registers all local interrupts as per-CPU interrupts
2. We can develop drivers for devices with per-CPU local interrupts
without changing arch code or this driver
3. It allows local interrupt controller DT node under each CPU DT node
as well as single system-wide DT node for local interrupt controller.

With this patchset, output of "cat /proc/interrupts" looks as follows:
           CPU0       CPU1       CPU2       CPU3       
  5:       1256       1269       1276       1262  RISC-V INTC   5  riscv_timer
  8:          6         15          7         16  SiFive PLIC   8  virtio0
 10:         22         12         12         21  SiFive PLIC  10  ttyS0

The patchset is based up Linux-4.19-rc2 and can be found at
riscv_intc_v2 branch of:
https://github.com/avpatel/linux.git

Changes since v1:
 - Removed changes related to puggable IPI triggering
 - Separate patch for self-contained IPI handling routine
 - Removed patch for GENERIC_IRQ kconfig options
 - Added patch to remove do_IRQ() function
 - Rebased upon Atish's SMP patches

Anup Patel (5):
  RISC-V: self-contained IPI handling routine
  RISC-V: No need to pass scause as arg to do_IRQ()
  irqchip: RISC-V Local Interrupt Controller Driver
  clocksource: riscv_timer: Make timer interrupt as a per-CPU interrupt
  RISC-V: Remove do_IRQ() function

 arch/riscv/Kconfig                |   1 +
 arch/riscv/include/asm/irq.h      |  17 +++-
 arch/riscv/include/asm/smp.h      |   3 +
 arch/riscv/kernel/entry.S         |   5 +-
 arch/riscv/kernel/irq.c           |  47 +---------
 arch/riscv/kernel/smp.c           |  11 ++-
 drivers/clocksource/riscv_timer.c |  78 ++++++++++++----
 drivers/irqchip/Kconfig           |  15 ++-
 drivers/irqchip/Makefile          |   1 +
 drivers/irqchip/irq-riscv-intc.c  | 148 ++++++++++++++++++++++++++++++
 drivers/irqchip/irq-sifive-plic.c |  20 +++-
 include/linux/cpuhotplug.h        |   1 +
 12 files changed, 273 insertions(+), 74 deletions(-)
 create mode 100644 drivers/irqchip/irq-riscv-intc.c

-- 
2.17.1

[PATCH v2 0/5] New RISC-V Local Interrupt Controller Driver

[Anup Patel]

This patchset provides a new RISC-V Local Interrupt Controller Driver
for managing per-CPU local interrupts. The overall approach is inspired
from the way per-CPU local interrupts are handled by Linux ARM64 and
ARM GICv3 driver.

Few advantages of having this new driver are as follows:
1. It registers all local interrupts as per-CPU interrupts
2. We can develop drivers for devices with per-CPU local interrupts
without changing arch code or this driver
3. It allows local interrupt controller DT node under each CPU DT node
as well as single system-wide DT node for local interrupt controller.

With this patchset, output of "cat /proc/interrupts" looks as follows:
           CPU0       CPU1       CPU2       CPU3       
  5:       1256       1269       1276       1262  RISC-V INTC   5  riscv_timer
  8:          6         15          7         16  SiFive PLIC   8  virtio0
 10:         22         12         12         21  SiFive PLIC  10  ttyS0

The patchset is based up Linux-4.19-rc2 and can be found at
riscv_intc_v2 branch of:
https://github.com/avpatel/linux.git

Changes since v1:
 - Removed changes related to puggable IPI triggering
 - Separate patch for self-contained IPI handling routine
 - Removed patch for GENERIC_IRQ kconfig options
 - Added patch to remove do_IRQ() function
 - Rebased upon Atish's SMP patches

Anup Patel (5):
  RISC-V: self-contained IPI handling routine
  RISC-V: No need to pass scause as arg to do_IRQ()
  irqchip: RISC-V Local Interrupt Controller Driver
  clocksource: riscv_timer: Make timer interrupt as a per-CPU interrupt
  RISC-V: Remove do_IRQ() function

 arch/riscv/Kconfig                |   1 +
 arch/riscv/include/asm/irq.h      |  17 +++-
 arch/riscv/include/asm/smp.h      |   3 +
 arch/riscv/kernel/entry.S         |   5 +-
 arch/riscv/kernel/irq.c           |  47 +---------
 arch/riscv/kernel/smp.c           |  11 ++-
 drivers/clocksource/riscv_timer.c |  78 ++++++++++++----
 drivers/irqchip/Kconfig           |  15 ++-
 drivers/irqchip/Makefile          |   1 +
 drivers/irqchip/irq-riscv-intc.c  | 148 ++++++++++++++++++++++++++++++
 drivers/irqchip/irq-sifive-plic.c |  20 +++-
 include/linux/cpuhotplug.h        |   1 +
 12 files changed, 273 insertions(+), 74 deletions(-)
 create mode 100644 drivers/irqchip/irq-riscv-intc.c

-- 
2.17.1

[PATCH v2 1/5] RISC-V: self-contained IPI handling routine

[Anup Patel]

Currently, the IPI handling routine riscv_software_interrupt() does
not take any argument and also does not perform irq_enter()/irq_exit().

This patch makes IPI handling routine more self-contained by:
1. Passing "pt_regs *" argument
2. Explicitly doing irq_enter()/irq_exit()
3. Explicitly save/restore "pt_regs *" using set_irq_regs()

With above changes, IPI handling routine does not depend on caller
function to perform irq_enter()/irq_exit() and save/restore of
"pt_regs *" hence its more self-contained. This also enables us
to call IPI handling routine from IRQCHIP drivers.

Signed-off-by: Anup Patel <anup@brainfault.org>
---
 arch/riscv/include/asm/irq.h |  1 -
 arch/riscv/include/asm/smp.h |  3 +++
 arch/riscv/kernel/irq.c      | 16 ++++++++++------
 arch/riscv/kernel/smp.c      | 11 +++++++++--
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/include/asm/irq.h b/arch/riscv/include/asm/irq.h
index a873a72d0f00..8d5d1a9f7fae 100644
--- a/arch/riscv/include/asm/irq.h
+++ b/arch/riscv/include/asm/irq.h
@@ -18,7 +18,6 @@
 #define NR_IRQS         0
 
 void riscv_timer_interrupt(void);
-void riscv_software_interrupt(void);
 void wait_for_software_interrupt(void);
 
 #include <asm-generic/irq.h>
diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
index 8145b8657d20..d7c3da05f200 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -51,6 +51,9 @@ void send_ipi_message(const struct cpumask *to_whom,
 /* SMP initialization hook for setup_arch */
 void __init setup_smp(void);
 
+/* Called from C code, this handles an IPI. */
+void handle_IPI(struct pt_regs *regs);
+
 /* Hook for the generic smp_call_function_many() routine. */
 void arch_send_call_function_ipi_mask(struct cpumask *mask);
 
diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
index 7e14b0d9a71d..f5073dcbc560 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -26,12 +26,15 @@
 
 asmlinkage void __irq_entry do_IRQ(struct pt_regs *regs, unsigned long cause)
 {
-	struct pt_regs *old_regs = set_irq_regs(regs);
+	struct pt_regs *old_regs;
 
-	irq_enter();
 	switch (cause & ~INTERRUPT_CAUSE_FLAG) {
 	case INTERRUPT_CAUSE_TIMER:
+		old_regs = set_irq_regs(regs);
+		irq_enter();
 		riscv_timer_interrupt();
+		irq_exit();
+		set_irq_regs(old_regs);
 		break;
 #ifdef CONFIG_SMP
 	case INTERRUPT_CAUSE_SOFTWARE:
@@ -39,18 +42,19 @@ asmlinkage void __irq_entry do_IRQ(struct pt_regs *regs, unsigned long cause)
 		 * We only use software interrupts to pass IPIs, so if a non-SMP
 		 * system gets one, then we don't know what to do.
 		 */
-		riscv_software_interrupt();
+		handle_IPI(regs);
 		break;
 #endif
 	case INTERRUPT_CAUSE_EXTERNAL:
+		old_regs = set_irq_regs(regs);
+		irq_enter();
 		handle_arch_irq(regs);
+		irq_exit();
+		set_irq_regs(old_regs);
 		break;
 	default:
 		panic("unexpected interrupt cause");
 	}
-	irq_exit();
-
-	set_irq_regs(old_regs);
 }
 
 /*
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index 629456bb6122..9e7bcf705946 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -58,10 +58,13 @@ int setup_profiling_timer(unsigned int multiplier)
 	return -EINVAL;
 }
 
-void riscv_software_interrupt(void)
+void handle_IPI(struct pt_regs *regs)
 {
+	struct pt_regs *old_regs = set_irq_regs(regs);
 	unsigned long *pending_ipis = &ipi_data[smp_processor_id()].bits;
 
+	irq_enter();
+
 	/* Clear pending IPI */
 	csr_clear(sip, SIE_SSIE);
 
@@ -73,7 +76,7 @@ void riscv_software_interrupt(void)
 
 		ops = xchg(pending_ipis, 0);
 		if (ops == 0)
-			return;
+			goto done;
 
 		if (ops & (1 << IPI_RESCHEDULE))
 			scheduler_ipi();
@@ -86,6 +89,10 @@ void riscv_software_interrupt(void)
 		/* Order data access and bit testing. */
 		mb();
 	}
+
+done:
+	irq_exit();
+	set_irq_regs(old_regs);
 }
 
 void
-- 
2.17.1

[PATCH v2 1/5] RISC-V: self-contained IPI handling routine

[Anup Patel]

Currently, the IPI handling routine riscv_software_interrupt() does
not take any argument and also does not perform irq_enter()/irq_exit().

This patch makes IPI handling routine more self-contained by:
1. Passing "pt_regs *" argument
2. Explicitly doing irq_enter()/irq_exit()
3. Explicitly save/restore "pt_regs *" using set_irq_regs()

With above changes, IPI handling routine does not depend on caller
function to perform irq_enter()/irq_exit() and save/restore of
"pt_regs *" hence its more self-contained. This also enables us
to call IPI handling routine from IRQCHIP drivers.

Signed-off-by: Anup Patel <anup@brainfault.org>
---
 arch/riscv/include/asm/irq.h |  1 -
 arch/riscv/include/asm/smp.h |  3 +++
 arch/riscv/kernel/irq.c      | 16 ++++++++++------
 arch/riscv/kernel/smp.c      | 11 +++++++++--
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/include/asm/irq.h b/arch/riscv/include/asm/irq.h
index a873a72d0f00..8d5d1a9f7fae 100644
--- a/arch/riscv/include/asm/irq.h
+++ b/arch/riscv/include/asm/irq.h
@@ -18,7 +18,6 @@
 #define NR_IRQS         0
 
 void riscv_timer_interrupt(void);
-void riscv_software_interrupt(void);
 void wait_for_software_interrupt(void);
 
 #include <asm-generic/irq.h>
diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
index 8145b8657d20..d7c3da05f200 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -51,6 +51,9 @@ void send_ipi_message(const struct cpumask *to_whom,
 /* SMP initialization hook for setup_arch */
 void __init setup_smp(void);
 
+/* Called from C code, this handles an IPI. */
+void handle_IPI(struct pt_regs *regs);
+
 /* Hook for the generic smp_call_function_many() routine. */
 void arch_send_call_function_ipi_mask(struct cpumask *mask);
 
diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
index 7e14b0d9a71d..f5073dcbc560 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -26,12 +26,15 @@
 
 asmlinkage void __irq_entry do_IRQ(struct pt_regs *regs, unsigned long cause)
 {
-	struct pt_regs *old_regs = set_irq_regs(regs);
+	struct pt_regs *old_regs;
 
-	irq_enter();
 	switch (cause & ~INTERRUPT_CAUSE_FLAG) {
 	case INTERRUPT_CAUSE_TIMER:
+		old_regs = set_irq_regs(regs);
+		irq_enter();
 		riscv_timer_interrupt();
+		irq_exit();
+		set_irq_regs(old_regs);
 		break;
 #ifdef CONFIG_SMP
 	case INTERRUPT_CAUSE_SOFTWARE:
@@ -39,18 +42,19 @@ asmlinkage void __irq_entry do_IRQ(struct pt_regs *regs, unsigned long cause)
 		 * We only use software interrupts to pass IPIs, so if a non-SMP
 		 * system gets one, then we don't know what to do.
 		 */
-		riscv_software_interrupt();
+		handle_IPI(regs);
 		break;
 #endif
 	case INTERRUPT_CAUSE_EXTERNAL:
+		old_regs = set_irq_regs(regs);
+		irq_enter();
 		handle_arch_irq(regs);
+		irq_exit();
+		set_irq_regs(old_regs);
 		break;
 	default:
 		panic("unexpected interrupt cause");
 	}
-	irq_exit();
-
-	set_irq_regs(old_regs);
 }
 
 /*
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index 629456bb6122..9e7bcf705946 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -58,10 +58,13 @@ int setup_profiling_timer(unsigned int multiplier)
 	return -EINVAL;
 }
 
-void riscv_software_interrupt(void)
+void handle_IPI(struct pt_regs *regs)
 {
+	struct pt_regs *old_regs = set_irq_regs(regs);
 	unsigned long *pending_ipis = &ipi_data[smp_processor_id()].bits;
 
+	irq_enter();
+
 	/* Clear pending IPI */
 	csr_clear(sip, SIE_SSIE);
 
@@ -73,7 +76,7 @@ void riscv_software_interrupt(void)
 
 		ops = xchg(pending_ipis, 0);
 		if (ops == 0)
-			return;
+			goto done;
 
 		if (ops & (1 << IPI_RESCHEDULE))
 			scheduler_ipi();
@@ -86,6 +89,10 @@ void riscv_software_interrupt(void)
 		/* Order data access and bit testing. */
 		mb();
 	}
+
+done:
+	irq_exit();
+	set_irq_regs(old_regs);
 }
 
 void
-- 
2.17.1

[PATCH v2 2/5] RISC-V: No need to pass scause as arg to do_IRQ()

[Anup Patel]

The scause is already part of pt_regs so no need to pass
scause as separate arg to do_IRQ().

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anup Patel <anup@brainfault.org>
---
 arch/riscv/kernel/entry.S | 1 -
 arch/riscv/kernel/irq.c   | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index fa2c08e3c05e..6eaacfa5b63d 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -168,7 +168,6 @@ ENTRY(handle_exception)
 
 	/* Handle interrupts */
 	move a0, sp /* pt_regs */
-	move a1, s4 /* scause */
 	tail do_IRQ
 1:
 	/* Exceptions run with interrupts enabled */
diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
index f5073dcbc560..d0de40e1e7f3 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -24,11 +24,11 @@
  */
 #define INTERRUPT_CAUSE_FLAG	(1UL << (__riscv_xlen - 1))
 
-asmlinkage void __irq_entry do_IRQ(struct pt_regs *regs, unsigned long cause)
+asmlinkage void __irq_entry do_IRQ(struct pt_regs *regs)
 {
 	struct pt_regs *old_regs;
 
-	switch (cause & ~INTERRUPT_CAUSE_FLAG) {
+	switch (regs->scause & ~INTERRUPT_CAUSE_FLAG) {
 	case INTERRUPT_CAUSE_TIMER:
 		old_regs = set_irq_regs(regs);
 		irq_enter();
-- 
2.17.1

[PATCH v2 2/5] RISC-V: No need to pass scause as arg to do_IRQ()

[Anup Patel]

The scause is already part of pt_regs so no need to pass
scause as separate arg to do_IRQ().

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anup Patel <anup@brainfault.org>
---
 arch/riscv/kernel/entry.S | 1 -
 arch/riscv/kernel/irq.c   | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index fa2c08e3c05e..6eaacfa5b63d 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -168,7 +168,6 @@ ENTRY(handle_exception)
 
 	/* Handle interrupts */
 	move a0, sp /* pt_regs */
-	move a1, s4 /* scause */
 	tail do_IRQ
 1:
 	/* Exceptions run with interrupts enabled */
diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
index f5073dcbc560..d0de40e1e7f3 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -24,11 +24,11 @@
  */
 #define INTERRUPT_CAUSE_FLAG	(1UL << (__riscv_xlen - 1))
 
-asmlinkage void __irq_entry do_IRQ(struct pt_regs *regs, unsigned long cause)
+asmlinkage void __irq_entry do_IRQ(struct pt_regs *regs)
 {
 	struct pt_regs *old_regs;
 
-	switch (cause & ~INTERRUPT_CAUSE_FLAG) {
+	switch (regs->scause & ~INTERRUPT_CAUSE_FLAG) {
 	case INTERRUPT_CAUSE_TIMER:
 		old_regs = set_irq_regs(regs);
 		irq_enter();
-- 
2.17.1

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

The RISC-V local interrupt controller manages software interrupts,
timer interrupts, external interrupts (which are routed via the
platform level interrupt controller) and per-HART local interrupts.

This patch add a driver for RISC-V local interrupt controller. It's
a major re-write over perviously submitted version.
(Refer, https://www.spinics.net/lists/devicetree/msg241230.html)

Few advantages of this new driver over previous one are:
1. It registers all local interrupts as per-CPU interrupts
2. We can develop drivers for devices with per-CPU local interrupts
without changing arch code or this driver
3. It allows local interrupt controller DT node under each CPU DT node
as well as single system-wide DT node for local interrupt controller.

The RISC-V INTC driver is compliant with RISC-V Hart-Level Interrupt
Controller DT bindings located at:
Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt

Signed-off-by: Anup Patel <anup@brainfault.org>
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
---
 arch/riscv/Kconfig                |   1 +
 arch/riscv/include/asm/irq.h      |  15 ++-
 arch/riscv/kernel/irq.c           |  47 +--------
 drivers/irqchip/Kconfig           |  15 ++-
 drivers/irqchip/Makefile          |   1 +
 drivers/irqchip/irq-riscv-intc.c  | 156 ++++++++++++++++++++++++++++++
 drivers/irqchip/irq-sifive-plic.c |  20 +++-
 include/linux/cpuhotplug.h        |   1 +
 8 files changed, 206 insertions(+), 50 deletions(-)
 create mode 100644 drivers/irqchip/irq-riscv-intc.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index ea2e617eff72..16bb33cc145f 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -32,6 +32,7 @@ config RISCV
 	select HAVE_DMA_CONTIGUOUS
 	select HAVE_GENERIC_DMA_COHERENT
 	select HAVE_PERF_EVENTS
+	select HANDLE_DOMAIN_IRQ
 	select IRQ_DOMAIN
 	select NO_BOOTMEM
 	select RISCV_ISA_A if SMP
diff --git a/arch/riscv/include/asm/irq.h b/arch/riscv/include/asm/irq.h
index 8d5d1a9f7fae..2ad610802689 100644
--- a/arch/riscv/include/asm/irq.h
+++ b/arch/riscv/include/asm/irq.h
@@ -15,7 +15,20 @@
 #ifndef _ASM_RISCV_IRQ_H
 #define _ASM_RISCV_IRQ_H
 
-#define NR_IRQS         0
+/*
+ * Possible interrupt causes:
+ */
+#define INTERRUPT_CAUSE_SOFTWARE    1
+#define INTERRUPT_CAUSE_TIMER       5
+#define INTERRUPT_CAUSE_EXTERNAL    9
+
+/*
+ * The high order bit of the trap cause register is always set for
+ * interrupts, which allows us to differentiate them from exceptions
+ * quickly.  The INTERRUPT_CAUSE_* macros don't contain that bit, so we
+ * need to mask it off.
+ */
+#define INTERRUPT_CAUSE_FLAG	(1UL << (__riscv_xlen - 1))
 
 void riscv_timer_interrupt(void);
 void wait_for_software_interrupt(void);
diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
index d0de40e1e7f3..9ced284da30b 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -7,54 +7,11 @@
 
 #include <linux/interrupt.h>
 #include <linux/irqchip.h>
-#include <linux/irqdomain.h>
-
-/*
- * Possible interrupt causes:
- */
-#define INTERRUPT_CAUSE_SOFTWARE    1
-#define INTERRUPT_CAUSE_TIMER       5
-#define INTERRUPT_CAUSE_EXTERNAL    9
-
-/*
- * The high order bit of the trap cause register is always set for
- * interrupts, which allows us to differentiate them from exceptions
- * quickly.  The INTERRUPT_CAUSE_* macros don't contain that bit, so we
- * need to mask it off.
- */
-#define INTERRUPT_CAUSE_FLAG	(1UL << (__riscv_xlen - 1))
 
 asmlinkage void __irq_entry do_IRQ(struct pt_regs *regs)
 {
-	struct pt_regs *old_regs;
-
-	switch (regs->scause & ~INTERRUPT_CAUSE_FLAG) {
-	case INTERRUPT_CAUSE_TIMER:
-		old_regs = set_irq_regs(regs);
-		irq_enter();
-		riscv_timer_interrupt();
-		irq_exit();
-		set_irq_regs(old_regs);
-		break;
-#ifdef CONFIG_SMP
-	case INTERRUPT_CAUSE_SOFTWARE:
-		/*
-		 * We only use software interrupts to pass IPIs, so if a non-SMP
-		 * system gets one, then we don't know what to do.
-		 */
-		handle_IPI(regs);
-		break;
-#endif
-	case INTERRUPT_CAUSE_EXTERNAL:
-		old_regs = set_irq_regs(regs);
-		irq_enter();
+	if (handle_arch_irq)
 		handle_arch_irq(regs);
-		irq_exit();
-		set_irq_regs(old_regs);
-		break;
-	default:
-		panic("unexpected interrupt cause");
-	}
 }
 
 /*
@@ -84,4 +41,6 @@ void wait_for_software_interrupt(void)
 void __init init_IRQ(void)
 {
 	irqchip_init();
+	if (!handle_arch_irq)
+		panic("No interrupt controller found.");
 }
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 383e7b70221d..885e182586f4 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -371,7 +371,18 @@ config QCOM_PDC
 	  Power Domain Controller driver to manage and configure wakeup
 	  IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
 
-endmenu
+config RISCV_INTC
+	bool "RISC-V Interrupt Controller"
+	depends on RISCV
+	default y
+	help
+	   This enables support for the local interrupt controller found in
+	   standard RISC-V systems.  The local interrupt controller handles
+	   timer interrupts, software interrupts, and hardware interrupts.
+	   Without a local interrupt controller the system will be unable to
+	   handle any interrupts, including those passed via the PLIC.
+
+	   If you don't know what to do here, say Y.
 
 config SIFIVE_PLIC
 	bool "SiFive Platform-Level Interrupt Controller"
@@ -384,3 +395,5 @@ config SIFIVE_PLIC
 	   interrupt sources are subordinate to the PLIC.
 
 	   If you don't know what to do here, say Y.
+
+endmenu
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index fbd1ec8070ef..e638ee5c4452 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -87,4 +87,5 @@ obj-$(CONFIG_MESON_IRQ_GPIO)		+= irq-meson-gpio.o
 obj-$(CONFIG_GOLDFISH_PIC) 		+= irq-goldfish-pic.o
 obj-$(CONFIG_NDS32)			+= irq-ativic32.o
 obj-$(CONFIG_QCOM_PDC)			+= qcom-pdc.o
+obj-$(CONFIG_RISCV_INTC)		+= irq-riscv-intc.o
 obj-$(CONFIG_SIFIVE_PLIC)		+= irq-sifive-plic.o
diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
new file mode 100644
index 000000000000..7dea2297daaa
--- /dev/null
+++ b/drivers/irqchip/irq-riscv-intc.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2012 Regents of the University of California
+ * Copyright (C) 2017-2018 SiFive
+ * Copyright (C) 2018 Anup Patel
+ */
+
+#define pr_fmt(fmt) "riscv-intc: " fmt
+#include <linux/atomic.h>
+#include <linux/bits.h>
+#include <linux/cpu.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>
+#include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/smp.h>
+
+static struct irq_domain *intc_domain;
+static atomic_t intc_init = ATOMIC_INIT(0);
+
+static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
+{
+	struct pt_regs *old_regs;
+	unsigned long cause = regs->scause & ~INTERRUPT_CAUSE_FLAG;
+
+	if (unlikely(cause >= BITS_PER_LONG))
+		panic("unexpected interrupt cause");
+
+	switch (cause) {
+	case INTERRUPT_CAUSE_TIMER:
+		old_regs = set_irq_regs(regs);
+		irq_enter();
+		riscv_timer_interrupt();
+		irq_exit();
+		set_irq_regs(old_regs);
+		break;
+#ifdef CONFIG_SMP
+	case INTERRUPT_CAUSE_SOFTWARE:
+		/*
+		 * We only use software interrupts to pass IPIs, so if a non-SMP
+		 * system gets one, then we don't know what to do.
+		 */
+		handle_IPI(regs);
+		break;
+#endif
+	default:
+		handle_domain_irq(intc_domain, cause, regs);
+		break;
+	}
+}
+
+/*
+ * On RISC-V systems local interrupts are masked or unmasked by writing the SIE
+ * (Supervisor Interrupt Enable) CSR.  As CSRs can only be written on the local
+ * hart, these functions can only be called on the hart that corresponds to the
+ * IRQ chip.  They are only called internally to this module, so they BUG_ON if
+ * this condition is violated rather than attempting to handle the error by
+ * forwarding to the target hart, as that's already expected to have been done.
+ */
+static void riscv_intc_irq_mask(struct irq_data *d)
+{
+	csr_clear(sie, 1 << (long)d->hwirq);
+}
+
+static void riscv_intc_irq_unmask(struct irq_data *d)
+{
+	csr_set(sie, 1 << (long)d->hwirq);
+}
+
+#ifdef CONFIG_SMP
+static int riscv_intc_cpu_starting(unsigned int cpu)
+{
+	csr_set(sie, 1 << INTERRUPT_CAUSE_SOFTWARE);
+	return 0;
+}
+
+static int riscv_intc_cpu_dying(unsigned int cpu)
+{
+	csr_clear(sie, 1 << INTERRUPT_CAUSE_SOFTWARE);
+	return 0;
+}
+
+static void riscv_intc_smp_init(void)
+{
+	csr_write(sie, 0);
+	csr_write(sip, 0);
+
+	cpuhp_setup_state(CPUHP_AP_IRQ_RISCV_STARTING,
+			  "irqchip/riscv/intc:starting",
+			  riscv_intc_cpu_starting,
+			  riscv_intc_cpu_dying);
+
+}
+#else
+static void riscv_intc_smp_init(void)
+{
+	csr_write(sie, 0);
+	csr_write(sip, 0);
+}
+#endif
+
+static struct irq_chip riscv_intc_chip = {
+	.name = "RISC-V INTC",
+	.irq_mask = riscv_intc_irq_mask,
+	.irq_unmask = riscv_intc_irq_unmask,
+};
+
+static int riscv_intc_domain_map(struct irq_domain *d, unsigned int irq,
+				 irq_hw_number_t hwirq)
+{
+	irq_set_percpu_devid(irq);
+	irq_domain_set_info(d, irq, hwirq, &riscv_intc_chip, d->host_data,
+			    handle_percpu_devid_irq, NULL, NULL);
+	irq_set_status_flags(irq, IRQ_NOAUTOEN);
+
+	return 0;
+}
+
+static const struct irq_domain_ops riscv_intc_domain_ops = {
+	.map	= riscv_intc_domain_map,
+	.xlate	= irq_domain_xlate_onecell,
+};
+
+static int __init riscv_intc_init(struct device_node *node,
+				  struct device_node *parent)
+{
+	/*
+	 * RISC-V device trees can have one INTC DT node under
+	 * each CPU DT node so INTC init function will be called
+	 * once for each INTC DT node. We only need to do INTC
+	 * init once for boot CPU so we use atomic counter to
+	 * achieve this.
+	 */
+	if (atomic_inc_return(&intc_init) > 1)
+		return 0;
+
+	intc_domain = irq_domain_add_linear(node, BITS_PER_LONG,
+					    &riscv_intc_domain_ops, NULL);
+	if (!intc_domain)
+		goto error_add_linear;
+
+	set_handle_irq(&riscv_intc_irq);
+
+	riscv_intc_smp_init();
+
+	pr_info("%lu local interrupts mapped\n", (long)BITS_PER_LONG);
+
+	return 0;
+
+error_add_linear:
+	pr_warn("unable to add IRQ domain\n");
+	return -ENXIO;
+}
+
+IRQCHIP_DECLARE(riscv, "riscv,cpu-intc", riscv_intc_init);
diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
index 357e9daf94ae..630a8c28e715 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -8,6 +8,7 @@
 #include <linux/io.h>
 #include <linux/irq.h>
 #include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -147,14 +148,17 @@ static struct irq_domain *plic_irqdomain;
  * that source ID back to the same claim register.  This automatically enables
  * and disables the interrupt, so there's nothing else to do.
  */
-static void plic_handle_irq(struct pt_regs *regs)
+static void plic_handle_irq(struct irq_desc *desc)
 {
 	struct plic_handler *handler = this_cpu_ptr(&plic_handlers);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
 	void __iomem *claim = plic_hart_offset(handler->ctxid) + CONTEXT_CLAIM;
 	irq_hw_number_t hwirq;
 
 	WARN_ON_ONCE(!handler->present);
 
+	chained_irq_enter(chip, desc);
+
 	csr_clear(sie, SIE_SEIE);
 	while ((hwirq = readl(claim))) {
 		int irq = irq_find_mapping(plic_irqdomain, hwirq);
@@ -167,6 +171,8 @@ static void plic_handle_irq(struct pt_regs *regs)
 		writel(hwirq, claim);
 	}
 	csr_set(sie, SIE_SEIE);
+
+	chained_irq_exit(chip, desc);
 }
 
 /*
@@ -184,7 +190,7 @@ static int plic_find_hart_id(struct device_node *node)
 }
 
 static int __init plic_init(struct device_node *node,
-		struct device_node *parent)
+			    struct device_node *parent)
 {
 	int error = 0, nr_handlers, nr_mapped = 0, i;
 	u32 nr_irqs;
@@ -219,7 +225,7 @@ static int __init plic_init(struct device_node *node,
 		struct of_phandle_args parent;
 		struct plic_handler *handler;
 		irq_hw_number_t hwirq;
-		int cpu, hartid;
+		int cpu, hartid, parent_irq;
 
 		if (of_irq_parse_one(node, i, &parent)) {
 			pr_err("failed to parse parent for context %d.\n", i);
@@ -230,6 +236,13 @@ static int __init plic_init(struct device_node *node,
 		if (parent.args[0] == -1)
 			continue;
 
+		if (irq_find_host(parent.np)) {
+			parent_irq = irq_of_parse_and_map(node, i);
+			if (parent_irq)
+				irq_set_chained_handler(parent_irq,
+							plic_handle_irq);
+		}
+
 		hartid = plic_find_hart_id(parent.np);
 		if (hartid < 0) {
 			pr_warn("failed to parse hart ID for context %d.\n", i);
@@ -250,7 +263,6 @@ static int __init plic_init(struct device_node *node,
 
 	pr_info("mapped %d interrupts to %d (out of %d) handlers.\n",
 		nr_irqs, nr_mapped, nr_handlers);
-	set_handle_irq(plic_handle_irq);
 	return 0;
 
 out_iounmap:
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index caf40ad0bbc6..ca7268a38cef 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -100,6 +100,7 @@ enum cpuhp_state {
 	CPUHP_AP_IRQ_ARMADA_XP_STARTING,
 	CPUHP_AP_IRQ_BCM2836_STARTING,
 	CPUHP_AP_IRQ_MIPS_GIC_STARTING,
+	CPUHP_AP_IRQ_RISCV_STARTING,
 	CPUHP_AP_ARM_MVEBU_COHERENCY,
 	CPUHP_AP_PERF_X86_AMD_UNCORE_STARTING,
 	CPUHP_AP_PERF_X86_STARTING,
-- 
2.17.1

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

The RISC-V local interrupt controller manages software interrupts,
timer interrupts, external interrupts (which are routed via the
platform level interrupt controller) and per-HART local interrupts.

This patch add a driver for RISC-V local interrupt controller. It's
a major re-write over perviously submitted version.
(Refer, https://www.spinics.net/lists/devicetree/msg241230.html)

Few advantages of this new driver over previous one are:
1. It registers all local interrupts as per-CPU interrupts
2. We can develop drivers for devices with per-CPU local interrupts
without changing arch code or this driver
3. It allows local interrupt controller DT node under each CPU DT node
as well as single system-wide DT node for local interrupt controller.

The RISC-V INTC driver is compliant with RISC-V Hart-Level Interrupt
Controller DT bindings located at:
Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt

Signed-off-by: Anup Patel <anup@brainfault.org>
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
---
 arch/riscv/Kconfig                |   1 +
 arch/riscv/include/asm/irq.h      |  15 ++-
 arch/riscv/kernel/irq.c           |  47 +--------
 drivers/irqchip/Kconfig           |  15 ++-
 drivers/irqchip/Makefile          |   1 +
 drivers/irqchip/irq-riscv-intc.c  | 156 ++++++++++++++++++++++++++++++
 drivers/irqchip/irq-sifive-plic.c |  20 +++-
 include/linux/cpuhotplug.h        |   1 +
 8 files changed, 206 insertions(+), 50 deletions(-)
 create mode 100644 drivers/irqchip/irq-riscv-intc.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index ea2e617eff72..16bb33cc145f 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -32,6 +32,7 @@ config RISCV
 	select HAVE_DMA_CONTIGUOUS
 	select HAVE_GENERIC_DMA_COHERENT
 	select HAVE_PERF_EVENTS
+	select HANDLE_DOMAIN_IRQ
 	select IRQ_DOMAIN
 	select NO_BOOTMEM
 	select RISCV_ISA_A if SMP
diff --git a/arch/riscv/include/asm/irq.h b/arch/riscv/include/asm/irq.h
index 8d5d1a9f7fae..2ad610802689 100644
--- a/arch/riscv/include/asm/irq.h
+++ b/arch/riscv/include/asm/irq.h
@@ -15,7 +15,20 @@
 #ifndef _ASM_RISCV_IRQ_H
 #define _ASM_RISCV_IRQ_H
 
-#define NR_IRQS         0
+/*
+ * Possible interrupt causes:
+ */
+#define INTERRUPT_CAUSE_SOFTWARE    1
+#define INTERRUPT_CAUSE_TIMER       5
+#define INTERRUPT_CAUSE_EXTERNAL    9
+
+/*
+ * The high order bit of the trap cause register is always set for
+ * interrupts, which allows us to differentiate them from exceptions
+ * quickly.  The INTERRUPT_CAUSE_* macros don't contain that bit, so we
+ * need to mask it off.
+ */
+#define INTERRUPT_CAUSE_FLAG	(1UL << (__riscv_xlen - 1))
 
 void riscv_timer_interrupt(void);
 void wait_for_software_interrupt(void);
diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
index d0de40e1e7f3..9ced284da30b 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -7,54 +7,11 @@
 
 #include <linux/interrupt.h>
 #include <linux/irqchip.h>
-#include <linux/irqdomain.h>
-
-/*
- * Possible interrupt causes:
- */
-#define INTERRUPT_CAUSE_SOFTWARE    1
-#define INTERRUPT_CAUSE_TIMER       5
-#define INTERRUPT_CAUSE_EXTERNAL    9
-
-/*
- * The high order bit of the trap cause register is always set for
- * interrupts, which allows us to differentiate them from exceptions
- * quickly.  The INTERRUPT_CAUSE_* macros don't contain that bit, so we
- * need to mask it off.
- */
-#define INTERRUPT_CAUSE_FLAG	(1UL << (__riscv_xlen - 1))
 
 asmlinkage void __irq_entry do_IRQ(struct pt_regs *regs)
 {
-	struct pt_regs *old_regs;
-
-	switch (regs->scause & ~INTERRUPT_CAUSE_FLAG) {
-	case INTERRUPT_CAUSE_TIMER:
-		old_regs = set_irq_regs(regs);
-		irq_enter();
-		riscv_timer_interrupt();
-		irq_exit();
-		set_irq_regs(old_regs);
-		break;
-#ifdef CONFIG_SMP
-	case INTERRUPT_CAUSE_SOFTWARE:
-		/*
-		 * We only use software interrupts to pass IPIs, so if a non-SMP
-		 * system gets one, then we don't know what to do.
-		 */
-		handle_IPI(regs);
-		break;
-#endif
-	case INTERRUPT_CAUSE_EXTERNAL:
-		old_regs = set_irq_regs(regs);
-		irq_enter();
+	if (handle_arch_irq)
 		handle_arch_irq(regs);
-		irq_exit();
-		set_irq_regs(old_regs);
-		break;
-	default:
-		panic("unexpected interrupt cause");
-	}
 }
 
 /*
@@ -84,4 +41,6 @@ void wait_for_software_interrupt(void)
 void __init init_IRQ(void)
 {
 	irqchip_init();
+	if (!handle_arch_irq)
+		panic("No interrupt controller found.");
 }
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 383e7b70221d..885e182586f4 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -371,7 +371,18 @@ config QCOM_PDC
 	  Power Domain Controller driver to manage and configure wakeup
 	  IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
 
-endmenu
+config RISCV_INTC
+	bool "RISC-V Interrupt Controller"
+	depends on RISCV
+	default y
+	help
+	   This enables support for the local interrupt controller found in
+	   standard RISC-V systems.  The local interrupt controller handles
+	   timer interrupts, software interrupts, and hardware interrupts.
+	   Without a local interrupt controller the system will be unable to
+	   handle any interrupts, including those passed via the PLIC.
+
+	   If you don't know what to do here, say Y.
 
 config SIFIVE_PLIC
 	bool "SiFive Platform-Level Interrupt Controller"
@@ -384,3 +395,5 @@ config SIFIVE_PLIC
 	   interrupt sources are subordinate to the PLIC.
 
 	   If you don't know what to do here, say Y.
+
+endmenu
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index fbd1ec8070ef..e638ee5c4452 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -87,4 +87,5 @@ obj-$(CONFIG_MESON_IRQ_GPIO)		+= irq-meson-gpio.o
 obj-$(CONFIG_GOLDFISH_PIC) 		+= irq-goldfish-pic.o
 obj-$(CONFIG_NDS32)			+= irq-ativic32.o
 obj-$(CONFIG_QCOM_PDC)			+= qcom-pdc.o
+obj-$(CONFIG_RISCV_INTC)		+= irq-riscv-intc.o
 obj-$(CONFIG_SIFIVE_PLIC)		+= irq-sifive-plic.o
diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
new file mode 100644
index 000000000000..7dea2297daaa
--- /dev/null
+++ b/drivers/irqchip/irq-riscv-intc.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2012 Regents of the University of California
+ * Copyright (C) 2017-2018 SiFive
+ * Copyright (C) 2018 Anup Patel
+ */
+
+#define pr_fmt(fmt) "riscv-intc: " fmt
+#include <linux/atomic.h>
+#include <linux/bits.h>
+#include <linux/cpu.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>
+#include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/smp.h>
+
+static struct irq_domain *intc_domain;
+static atomic_t intc_init = ATOMIC_INIT(0);
+
+static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
+{
+	struct pt_regs *old_regs;
+	unsigned long cause = regs->scause & ~INTERRUPT_CAUSE_FLAG;
+
+	if (unlikely(cause >= BITS_PER_LONG))
+		panic("unexpected interrupt cause");
+
+	switch (cause) {
+	case INTERRUPT_CAUSE_TIMER:
+		old_regs = set_irq_regs(regs);
+		irq_enter();
+		riscv_timer_interrupt();
+		irq_exit();
+		set_irq_regs(old_regs);
+		break;
+#ifdef CONFIG_SMP
+	case INTERRUPT_CAUSE_SOFTWARE:
+		/*
+		 * We only use software interrupts to pass IPIs, so if a non-SMP
+		 * system gets one, then we don't know what to do.
+		 */
+		handle_IPI(regs);
+		break;
+#endif
+	default:
+		handle_domain_irq(intc_domain, cause, regs);
+		break;
+	}
+}
+
+/*
+ * On RISC-V systems local interrupts are masked or unmasked by writing the SIE
+ * (Supervisor Interrupt Enable) CSR.  As CSRs can only be written on the local
+ * hart, these functions can only be called on the hart that corresponds to the
+ * IRQ chip.  They are only called internally to this module, so they BUG_ON if
+ * this condition is violated rather than attempting to handle the error by
+ * forwarding to the target hart, as that's already expected to have been done.
+ */
+static void riscv_intc_irq_mask(struct irq_data *d)
+{
+	csr_clear(sie, 1 << (long)d->hwirq);
+}
+
+static void riscv_intc_irq_unmask(struct irq_data *d)
+{
+	csr_set(sie, 1 << (long)d->hwirq);
+}
+
+#ifdef CONFIG_SMP
+static int riscv_intc_cpu_starting(unsigned int cpu)
+{
+	csr_set(sie, 1 << INTERRUPT_CAUSE_SOFTWARE);
+	return 0;
+}
+
+static int riscv_intc_cpu_dying(unsigned int cpu)
+{
+	csr_clear(sie, 1 << INTERRUPT_CAUSE_SOFTWARE);
+	return 0;
+}
+
+static void riscv_intc_smp_init(void)
+{
+	csr_write(sie, 0);
+	csr_write(sip, 0);
+
+	cpuhp_setup_state(CPUHP_AP_IRQ_RISCV_STARTING,
+			  "irqchip/riscv/intc:starting",
+			  riscv_intc_cpu_starting,
+			  riscv_intc_cpu_dying);
+
+}
+#else
+static void riscv_intc_smp_init(void)
+{
+	csr_write(sie, 0);
+	csr_write(sip, 0);
+}
+#endif
+
+static struct irq_chip riscv_intc_chip = {
+	.name = "RISC-V INTC",
+	.irq_mask = riscv_intc_irq_mask,
+	.irq_unmask = riscv_intc_irq_unmask,
+};
+
+static int riscv_intc_domain_map(struct irq_domain *d, unsigned int irq,
+				 irq_hw_number_t hwirq)
+{
+	irq_set_percpu_devid(irq);
+	irq_domain_set_info(d, irq, hwirq, &riscv_intc_chip, d->host_data,
+			    handle_percpu_devid_irq, NULL, NULL);
+	irq_set_status_flags(irq, IRQ_NOAUTOEN);
+
+	return 0;
+}
+
+static const struct irq_domain_ops riscv_intc_domain_ops = {
+	.map	= riscv_intc_domain_map,
+	.xlate	= irq_domain_xlate_onecell,
+};
+
+static int __init riscv_intc_init(struct device_node *node,
+				  struct device_node *parent)
+{
+	/*
+	 * RISC-V device trees can have one INTC DT node under
+	 * each CPU DT node so INTC init function will be called
+	 * once for each INTC DT node. We only need to do INTC
+	 * init once for boot CPU so we use atomic counter to
+	 * achieve this.
+	 */
+	if (atomic_inc_return(&intc_init) > 1)
+		return 0;
+
+	intc_domain = irq_domain_add_linear(node, BITS_PER_LONG,
+					    &riscv_intc_domain_ops, NULL);
+	if (!intc_domain)
+		goto error_add_linear;
+
+	set_handle_irq(&riscv_intc_irq);
+
+	riscv_intc_smp_init();
+
+	pr_info("%lu local interrupts mapped\n", (long)BITS_PER_LONG);
+
+	return 0;
+
+error_add_linear:
+	pr_warn("unable to add IRQ domain\n");
+	return -ENXIO;
+}
+
+IRQCHIP_DECLARE(riscv, "riscv,cpu-intc", riscv_intc_init);
diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
index 357e9daf94ae..630a8c28e715 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -8,6 +8,7 @@
 #include <linux/io.h>
 #include <linux/irq.h>
 #include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -147,14 +148,17 @@ static struct irq_domain *plic_irqdomain;
  * that source ID back to the same claim register.  This automatically enables
  * and disables the interrupt, so there's nothing else to do.
  */
-static void plic_handle_irq(struct pt_regs *regs)
+static void plic_handle_irq(struct irq_desc *desc)
 {
 	struct plic_handler *handler = this_cpu_ptr(&plic_handlers);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
 	void __iomem *claim = plic_hart_offset(handler->ctxid) + CONTEXT_CLAIM;
 	irq_hw_number_t hwirq;
 
 	WARN_ON_ONCE(!handler->present);
 
+	chained_irq_enter(chip, desc);
+
 	csr_clear(sie, SIE_SEIE);
 	while ((hwirq = readl(claim))) {
 		int irq = irq_find_mapping(plic_irqdomain, hwirq);
@@ -167,6 +171,8 @@ static void plic_handle_irq(struct pt_regs *regs)
 		writel(hwirq, claim);
 	}
 	csr_set(sie, SIE_SEIE);
+
+	chained_irq_exit(chip, desc);
 }
 
 /*
@@ -184,7 +190,7 @@ static int plic_find_hart_id(struct device_node *node)
 }
 
 static int __init plic_init(struct device_node *node,
-		struct device_node *parent)
+			    struct device_node *parent)
 {
 	int error = 0, nr_handlers, nr_mapped = 0, i;
 	u32 nr_irqs;
@@ -219,7 +225,7 @@ static int __init plic_init(struct device_node *node,
 		struct of_phandle_args parent;
 		struct plic_handler *handler;
 		irq_hw_number_t hwirq;
-		int cpu, hartid;
+		int cpu, hartid, parent_irq;
 
 		if (of_irq_parse_one(node, i, &parent)) {
 			pr_err("failed to parse parent for context %d.\n", i);
@@ -230,6 +236,13 @@ static int __init plic_init(struct device_node *node,
 		if (parent.args[0] == -1)
 			continue;
 
+		if (irq_find_host(parent.np)) {
+			parent_irq = irq_of_parse_and_map(node, i);
+			if (parent_irq)
+				irq_set_chained_handler(parent_irq,
+							plic_handle_irq);
+		}
+
 		hartid = plic_find_hart_id(parent.np);
 		if (hartid < 0) {
 			pr_warn("failed to parse hart ID for context %d.\n", i);
@@ -250,7 +263,6 @@ static int __init plic_init(struct device_node *node,
 
 	pr_info("mapped %d interrupts to %d (out of %d) handlers.\n",
 		nr_irqs, nr_mapped, nr_handlers);
-	set_handle_irq(plic_handle_irq);
 	return 0;
 
 out_iounmap:
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index caf40ad0bbc6..ca7268a38cef 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -100,6 +100,7 @@ enum cpuhp_state {
 	CPUHP_AP_IRQ_ARMADA_XP_STARTING,
 	CPUHP_AP_IRQ_BCM2836_STARTING,
 	CPUHP_AP_IRQ_MIPS_GIC_STARTING,
+	CPUHP_AP_IRQ_RISCV_STARTING,
 	CPUHP_AP_ARM_MVEBU_COHERENCY,
 	CPUHP_AP_PERF_X86_AMD_UNCORE_STARTING,
 	CPUHP_AP_PERF_X86_STARTING,
-- 
2.17.1

[PATCH v2 4/5] clocksource: riscv_timer: Make timer interrupt as a per-CPU interrupt

[Anup Patel]

Instead of directly calling RISC-V timer interrupt handler from
RISC-V local interrupt conntroller driver, this patch implements
RISC-V timer interrupt as a per-CPU interrupt using per-CPU APIs
of Linux IRQ subsystem.

Signed-off-by: Anup Patel <anup@brainfault.org>
---
 arch/riscv/include/asm/irq.h      |  1 -
 drivers/clocksource/riscv_timer.c | 78 ++++++++++++++++++++++++-------
 drivers/irqchip/irq-riscv-intc.c  |  8 ----
 3 files changed, 61 insertions(+), 26 deletions(-)

diff --git a/arch/riscv/include/asm/irq.h b/arch/riscv/include/asm/irq.h
index 2ad610802689..11c6dd365643 100644
--- a/arch/riscv/include/asm/irq.h
+++ b/arch/riscv/include/asm/irq.h
@@ -30,7 +30,6 @@
  */
 #define INTERRUPT_CAUSE_FLAG	(1UL << (__riscv_xlen - 1))
 
-void riscv_timer_interrupt(void);
 void wait_for_software_interrupt(void);
 
 #include <asm-generic/irq.h>
diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c
index 084e97dc10ed..c7ac60a82754 100644
--- a/drivers/clocksource/riscv_timer.c
+++ b/drivers/clocksource/riscv_timer.c
@@ -3,12 +3,17 @@
  * Copyright (C) 2012 Regents of the University of California
  * Copyright (C) 2017 SiFive
  */
+#define pr_fmt(fmt) "riscv-timer: " fmt
 #include <linux/clocksource.h>
 #include <linux/clockchips.h>
 #include <linux/cpu.h>
 #include <linux/delay.h>
 #include <linux/irq.h>
-#include <asm/smp.h>
+#include <linux/irqdomain.h>
+#include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/sched_clock.h>
 #include <asm/sbi.h>
 
 /*
@@ -32,6 +37,7 @@ static int riscv_clock_next_event(unsigned long delta,
 	return 0;
 }
 
+static unsigned int riscv_clock_event_irq;
 static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
 	.name			= "riscv_timer_clockevent",
 	.features		= CLOCK_EVT_FEAT_ONESHOT,
@@ -39,6 +45,11 @@ static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
 	.set_next_event		= riscv_clock_next_event,
 };
 
+static u64 riscv_sched_clock(void)
+{
+	return get_cycles64();
+}
+
 /*
  * It is guaranteed that all the timers across all the harts are synchronized
  * within one tick of each other, so while this could technically go
@@ -49,7 +60,7 @@ static unsigned long long riscv_clocksource_rdtime(struct clocksource *cs)
 	return get_cycles64();
 }
 
-static DEFINE_PER_CPU(struct clocksource, riscv_clocksource) = {
+static struct clocksource riscv_clocksource = {
 	.name		= "riscv_clocksource",
 	.rating		= 300,
 	.mask		= CLOCKSOURCE_MASK(BITS_PER_LONG),
@@ -62,48 +73,81 @@ static int riscv_timer_starting_cpu(unsigned int cpu)
 	struct clock_event_device *ce = per_cpu_ptr(&riscv_clock_event, cpu);
 
 	ce->cpumask = cpumask_of(cpu);
+	ce->irq = riscv_clock_event_irq;
 	clockevents_config_and_register(ce, riscv_timebase, 100, 0x7fffffff);
 
-	csr_set(sie, SIE_STIE);
+	enable_percpu_irq(riscv_clock_event_irq, IRQ_TYPE_NONE);
 	return 0;
 }
 
 static int riscv_timer_dying_cpu(unsigned int cpu)
 {
-	csr_clear(sie, SIE_STIE);
+	disable_percpu_irq(riscv_clock_event_irq);
 	return 0;
 }
 
-/* called directly from the low-level interrupt handler */
-void riscv_timer_interrupt(void)
+static irqreturn_t riscv_timer_interrupt(int irq, void *dev_id)
 {
 	struct clock_event_device *evdev = this_cpu_ptr(&riscv_clock_event);
 
 	csr_clear(sie, SIE_STIE);
 	evdev->event_handler(evdev);
+
+	return IRQ_HANDLED;
 }
 
 static int __init riscv_timer_init_dt(struct device_node *n)
 {
-	int cpuid, hartid, error;
-	struct clocksource *cs;
+	int error;
+	struct irq_domain *domain;
+	struct of_phandle_args oirq;
+
+	/*
+	 * Either we have one INTC DT node under each CPU DT node
+	 * or a single system wide INTC DT node. Irrespective to
+	 * number of INTC DT nodes, we only proceed if we are able
+	 * to find irq_domain of INTC.
+	 *
+	 * Once we have INTC irq_domain, we create mapping for timer
+	 * interrupt HWIRQ and request_percpu_irq() on it.
+	 */
+
+	if (riscv_clock_event_irq)
+		return 0;
 
-	hartid = riscv_of_processor_hartid(n);
-	cpuid = riscv_hartid_to_cpuid(hartid);
+	oirq.np = n;
+	oirq.args_count = 1;
+	oirq.args[0] = INTERRUPT_CAUSE_TIMER;
 
-	if (cpuid != smp_processor_id())
-		return 0;
+	domain = irq_find_host(oirq.np);
+	if (!domain)
+		return -ENODEV;
 
-	cs = per_cpu_ptr(&riscv_clocksource, cpuid);
-	clocksource_register_hz(cs, riscv_timebase);
+	riscv_clock_event_irq = irq_create_of_mapping(&oirq);
+	if (!riscv_clock_event_irq)
+		return -ENODEV;
+
+	clocksource_register_hz(&riscv_clocksource, riscv_timebase);
+	sched_clock_register(riscv_sched_clock,
+			     BITS_PER_LONG, riscv_timebase);
+
+	error = request_percpu_irq(riscv_clock_event_irq,
+				   riscv_timer_interrupt,
+				   "riscv_timer", &riscv_clock_event);
+	if (error)
+		return error;
 
 	error = cpuhp_setup_state(CPUHP_AP_RISCV_TIMER_STARTING,
 			 "clockevents/riscv/timer:starting",
 			 riscv_timer_starting_cpu, riscv_timer_dying_cpu);
 	if (error)
-		pr_err("RISCV timer register failed [%d] for cpu = [%d]\n",
-		       error, cpuid);
+		pr_err("RISCV timer register failed error %d\n", error);
+
+	pr_info("running at %lu.%02luMHz frequency\n",
+		(unsigned long)riscv_timebase / 1000000,
+		(unsigned long)(riscv_timebase / 10000) % 100);
+
 	return error;
 }
 
-TIMER_OF_DECLARE(riscv_timer, "riscv", riscv_timer_init_dt);
+TIMER_OF_DECLARE(riscv_timer, "riscv,cpu-intc", riscv_timer_init_dt);
diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
index 7dea2297daaa..259524145b1f 100644
--- a/drivers/irqchip/irq-riscv-intc.c
+++ b/drivers/irqchip/irq-riscv-intc.c
@@ -21,20 +21,12 @@ static atomic_t intc_init = ATOMIC_INIT(0);
 
 static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
 {
-	struct pt_regs *old_regs;
 	unsigned long cause = regs->scause & ~INTERRUPT_CAUSE_FLAG;
 
 	if (unlikely(cause >= BITS_PER_LONG))
 		panic("unexpected interrupt cause");
 
 	switch (cause) {
-	case INTERRUPT_CAUSE_TIMER:
-		old_regs = set_irq_regs(regs);
-		irq_enter();
-		riscv_timer_interrupt();
-		irq_exit();
-		set_irq_regs(old_regs);
-		break;
 #ifdef CONFIG_SMP
 	case INTERRUPT_CAUSE_SOFTWARE:
 		/*
-- 
2.17.1

[PATCH v2 4/5] clocksource: riscv_timer: Make timer interrupt as a per-CPU interrupt

[Anup Patel]

Instead of directly calling RISC-V timer interrupt handler from
RISC-V local interrupt conntroller driver, this patch implements
RISC-V timer interrupt as a per-CPU interrupt using per-CPU APIs
of Linux IRQ subsystem.

Signed-off-by: Anup Patel <anup@brainfault.org>
---
 arch/riscv/include/asm/irq.h      |  1 -
 drivers/clocksource/riscv_timer.c | 78 ++++++++++++++++++++++++-------
 drivers/irqchip/irq-riscv-intc.c  |  8 ----
 3 files changed, 61 insertions(+), 26 deletions(-)

diff --git a/arch/riscv/include/asm/irq.h b/arch/riscv/include/asm/irq.h
index 2ad610802689..11c6dd365643 100644
--- a/arch/riscv/include/asm/irq.h
+++ b/arch/riscv/include/asm/irq.h
@@ -30,7 +30,6 @@
  */
 #define INTERRUPT_CAUSE_FLAG	(1UL << (__riscv_xlen - 1))
 
-void riscv_timer_interrupt(void);
 void wait_for_software_interrupt(void);
 
 #include <asm-generic/irq.h>
diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c
index 084e97dc10ed..c7ac60a82754 100644
--- a/drivers/clocksource/riscv_timer.c
+++ b/drivers/clocksource/riscv_timer.c
@@ -3,12 +3,17 @@
  * Copyright (C) 2012 Regents of the University of California
  * Copyright (C) 2017 SiFive
  */
+#define pr_fmt(fmt) "riscv-timer: " fmt
 #include <linux/clocksource.h>
 #include <linux/clockchips.h>
 #include <linux/cpu.h>
 #include <linux/delay.h>
 #include <linux/irq.h>
-#include <asm/smp.h>
+#include <linux/irqdomain.h>
+#include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/sched_clock.h>
 #include <asm/sbi.h>
 
 /*
@@ -32,6 +37,7 @@ static int riscv_clock_next_event(unsigned long delta,
 	return 0;
 }
 
+static unsigned int riscv_clock_event_irq;
 static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
 	.name			= "riscv_timer_clockevent",
 	.features		= CLOCK_EVT_FEAT_ONESHOT,
@@ -39,6 +45,11 @@ static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
 	.set_next_event		= riscv_clock_next_event,
 };
 
+static u64 riscv_sched_clock(void)
+{
+	return get_cycles64();
+}
+
 /*
  * It is guaranteed that all the timers across all the harts are synchronized
  * within one tick of each other, so while this could technically go
@@ -49,7 +60,7 @@ static unsigned long long riscv_clocksource_rdtime(struct clocksource *cs)
 	return get_cycles64();
 }
 
-static DEFINE_PER_CPU(struct clocksource, riscv_clocksource) = {
+static struct clocksource riscv_clocksource = {
 	.name		= "riscv_clocksource",
 	.rating		= 300,
 	.mask		= CLOCKSOURCE_MASK(BITS_PER_LONG),
@@ -62,48 +73,81 @@ static int riscv_timer_starting_cpu(unsigned int cpu)
 	struct clock_event_device *ce = per_cpu_ptr(&riscv_clock_event, cpu);
 
 	ce->cpumask = cpumask_of(cpu);
+	ce->irq = riscv_clock_event_irq;
 	clockevents_config_and_register(ce, riscv_timebase, 100, 0x7fffffff);
 
-	csr_set(sie, SIE_STIE);
+	enable_percpu_irq(riscv_clock_event_irq, IRQ_TYPE_NONE);
 	return 0;
 }
 
 static int riscv_timer_dying_cpu(unsigned int cpu)
 {
-	csr_clear(sie, SIE_STIE);
+	disable_percpu_irq(riscv_clock_event_irq);
 	return 0;
 }
 
-/* called directly from the low-level interrupt handler */
-void riscv_timer_interrupt(void)
+static irqreturn_t riscv_timer_interrupt(int irq, void *dev_id)
 {
 	struct clock_event_device *evdev = this_cpu_ptr(&riscv_clock_event);
 
 	csr_clear(sie, SIE_STIE);
 	evdev->event_handler(evdev);
+
+	return IRQ_HANDLED;
 }
 
 static int __init riscv_timer_init_dt(struct device_node *n)
 {
-	int cpuid, hartid, error;
-	struct clocksource *cs;
+	int error;
+	struct irq_domain *domain;
+	struct of_phandle_args oirq;
+
+	/*
+	 * Either we have one INTC DT node under each CPU DT node
+	 * or a single system wide INTC DT node. Irrespective to
+	 * number of INTC DT nodes, we only proceed if we are able
+	 * to find irq_domain of INTC.
+	 *
+	 * Once we have INTC irq_domain, we create mapping for timer
+	 * interrupt HWIRQ and request_percpu_irq() on it.
+	 */
+
+	if (riscv_clock_event_irq)
+		return 0;
 
-	hartid = riscv_of_processor_hartid(n);
-	cpuid = riscv_hartid_to_cpuid(hartid);
+	oirq.np = n;
+	oirq.args_count = 1;
+	oirq.args[0] = INTERRUPT_CAUSE_TIMER;
 
-	if (cpuid != smp_processor_id())
-		return 0;
+	domain = irq_find_host(oirq.np);
+	if (!domain)
+		return -ENODEV;
 
-	cs = per_cpu_ptr(&riscv_clocksource, cpuid);
-	clocksource_register_hz(cs, riscv_timebase);
+	riscv_clock_event_irq = irq_create_of_mapping(&oirq);
+	if (!riscv_clock_event_irq)
+		return -ENODEV;
+
+	clocksource_register_hz(&riscv_clocksource, riscv_timebase);
+	sched_clock_register(riscv_sched_clock,
+			     BITS_PER_LONG, riscv_timebase);
+
+	error = request_percpu_irq(riscv_clock_event_irq,
+				   riscv_timer_interrupt,
+				   "riscv_timer", &riscv_clock_event);
+	if (error)
+		return error;
 
 	error = cpuhp_setup_state(CPUHP_AP_RISCV_TIMER_STARTING,
 			 "clockevents/riscv/timer:starting",
 			 riscv_timer_starting_cpu, riscv_timer_dying_cpu);
 	if (error)
-		pr_err("RISCV timer register failed [%d] for cpu = [%d]\n",
-		       error, cpuid);
+		pr_err("RISCV timer register failed error %d\n", error);
+
+	pr_info("running at %lu.%02luMHz frequency\n",
+		(unsigned long)riscv_timebase / 1000000,
+		(unsigned long)(riscv_timebase / 10000) % 100);
+
 	return error;
 }
 
-TIMER_OF_DECLARE(riscv_timer, "riscv", riscv_timer_init_dt);
+TIMER_OF_DECLARE(riscv_timer, "riscv,cpu-intc", riscv_timer_init_dt);
diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
index 7dea2297daaa..259524145b1f 100644
--- a/drivers/irqchip/irq-riscv-intc.c
+++ b/drivers/irqchip/irq-riscv-intc.c
@@ -21,20 +21,12 @@ static atomic_t intc_init = ATOMIC_INIT(0);
 
 static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
 {
-	struct pt_regs *old_regs;
 	unsigned long cause = regs->scause & ~INTERRUPT_CAUSE_FLAG;
 
 	if (unlikely(cause >= BITS_PER_LONG))
 		panic("unexpected interrupt cause");
 
 	switch (cause) {
-	case INTERRUPT_CAUSE_TIMER:
-		old_regs = set_irq_regs(regs);
-		irq_enter();
-		riscv_timer_interrupt();
-		irq_exit();
-		set_irq_regs(old_regs);
-		break;
 #ifdef CONFIG_SMP
 	case INTERRUPT_CAUSE_SOFTWARE:
 		/*
-- 
2.17.1

[PATCH v2 5/5] RISC-V: Remove do_IRQ() function

[Anup Patel]

The only thing do_IRQ() does is call handle_arch_irq function
pointer. We can very well call handle_arch_irq function pointer
directly from assembly and remove do_IRQ() function hence this
patch.

Signed-off-by: Anup Patel <anup@brainfault.org>
---
 arch/riscv/kernel/entry.S | 4 +++-
 arch/riscv/kernel/irq.c   | 6 ------
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 6eaacfa5b63d..69fbe9dd9e0c 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -168,7 +168,9 @@ ENTRY(handle_exception)
 
 	/* Handle interrupts */
 	move a0, sp /* pt_regs */
-	tail do_IRQ
+	la a1, handle_arch_irq
+	REG_L a1, (a1)
+	jr a1
 1:
 	/* Exceptions run with interrupts enabled */
 	csrs sstatus, SR_SIE
diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
index 9ced284da30b..695e241d0402 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -8,12 +8,6 @@
 #include <linux/interrupt.h>
 #include <linux/irqchip.h>
 
-asmlinkage void __irq_entry do_IRQ(struct pt_regs *regs)
-{
-	if (handle_arch_irq)
-		handle_arch_irq(regs);
-}
-
 /*
  * This function doesn't return until a software interrupt is sent via IPI.
  * Obviously, all the interrupts except software interrupt should be disabled
-- 
2.17.1

[PATCH v2 5/5] RISC-V: Remove do_IRQ() function

[Anup Patel]

The only thing do_IRQ() does is call handle_arch_irq function
pointer. We can very well call handle_arch_irq function pointer
directly from assembly and remove do_IRQ() function hence this
patch.

Signed-off-by: Anup Patel <anup@brainfault.org>
---
 arch/riscv/kernel/entry.S | 4 +++-
 arch/riscv/kernel/irq.c   | 6 ------
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 6eaacfa5b63d..69fbe9dd9e0c 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -168,7 +168,9 @@ ENTRY(handle_exception)
 
 	/* Handle interrupts */
 	move a0, sp /* pt_regs */
-	tail do_IRQ
+	la a1, handle_arch_irq
+	REG_L a1, (a1)
+	jr a1
 1:
 	/* Exceptions run with interrupts enabled */
 	csrs sstatus, SR_SIE
diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
index 9ced284da30b..695e241d0402 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -8,12 +8,6 @@
 #include <linux/interrupt.h>
 #include <linux/irqchip.h>
 
-asmlinkage void __irq_entry do_IRQ(struct pt_regs *regs)
-{
-	if (handle_arch_irq)
-		handle_arch_irq(regs);
-}
-
 /*
  * This function doesn't return until a software interrupt is sent via IPI.
  * Obviously, all the interrupts except software interrupt should be disabled
-- 
2.17.1

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

Just as before:  NAK to entirely pointless abstractions.  Please stop
beating the dead horse.

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

Just as before:  NAK to entirely pointless abstractions.  Please stop
beating the dead horse.

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Thu, Sep 6, 2018 at 7:36 PM, Christoph Hellwig <hch@infradead.org> wrote:
> Just as before:  NAK to entirely pointless abstractions.  Please stop
> beating the dead horse.

That's just your opinion without any concrete reasoning.

Even after explaining in various ways, you fail to understand the
flexibility brought by this driver.

Regards,
Anup

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Thu, Sep 6, 2018 at 7:36 PM, Christoph Hellwig <hch@infradead.org> wrote:
> Just as before:  NAK to entirely pointless abstractions.  Please stop
> beating the dead horse.

That's just your opinion without any concrete reasoning.

Even after explaining in various ways, you fail to understand the
flexibility brought by this driver.

Regards,
Anup

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Thomas Gleixner]

On Thu, 6 Sep 2018, Christoph Hellwig wrote:

> Just as before:  NAK to entirely pointless abstractions.  Please stop
> beating the dead horse.

I disagree. These interrupts very well fit into the percpu interupt
mechanics and that allows them to be handled by all the generic mechanisms
as any other interrupt.

The extra short cuts have their own set of problems and we really should
avoid them where ever we can. I cursed the direct vector mess x86 has more
than once in the past, but never had cycles to clean it up.

Thanks,

	tglx

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Thomas Gleixner]

On Thu, 6 Sep 2018, Christoph Hellwig wrote:

> Just as before:  NAK to entirely pointless abstractions.  Please stop
> beating the dead horse.

I disagree. These interrupts very well fit into the percpu interupt
mechanics and that allows them to be handled by all the generic mechanisms
as any other interrupt.

The extra short cuts have their own set of problems and we really should
avoid them where ever we can. I cursed the direct vector mess x86 has more
than once in the past, but never had cycles to clean it up.

Thanks,

	tglx

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Sat, Sep 08, 2018 at 12:46:35PM +0200, Thomas Gleixner wrote:
> On Thu, 6 Sep 2018, Christoph Hellwig wrote:
> 
> > Just as before:  NAK to entirely pointless abstractions.  Please stop
> > beating the dead horse.
> 
> I disagree. These interrupts very well fit into the percpu interupt
> mechanics and that allows them to be handled by all the generic mechanisms
> as any other interrupt.

Just a few weeks ago you said the contrary:

http://lists.infradead.org/pipermail/linux-riscv/2018-August/000943.html

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Sat, Sep 08, 2018 at 12:46:35PM +0200, Thomas Gleixner wrote:
> On Thu, 6 Sep 2018, Christoph Hellwig wrote:
> 
> > Just as before:  NAK to entirely pointless abstractions.  Please stop
> > beating the dead horse.
> 
> I disagree. These interrupts very well fit into the percpu interupt
> mechanics and that allows them to be handled by all the generic mechanisms
> as any other interrupt.

Just a few weeks ago you said the contrary:

http://lists.infradead.org/pipermail/linux-riscv/2018-August/000943.html

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Thomas Gleixner]

On Mon, 10 Sep 2018, Christoph Hellwig wrote:
> On Sat, Sep 08, 2018 at 12:46:35PM +0200, Thomas Gleixner wrote:
> > On Thu, 6 Sep 2018, Christoph Hellwig wrote:
> > 
> > > Just as before:  NAK to entirely pointless abstractions.  Please stop
> > > beating the dead horse.
> > 
> > I disagree. These interrupts very well fit into the percpu interupt
> > mechanics and that allows them to be handled by all the generic mechanisms
> > as any other interrupt.
> 
> Just a few weeks ago you said the contrary:
> 
> http://lists.infradead.org/pipermail/linux-riscv/2018-August/000943.html

Sigh. Yes. Now that you remind me. 

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Thomas Gleixner]

On Mon, 10 Sep 2018, Christoph Hellwig wrote:
> On Sat, Sep 08, 2018 at 12:46:35PM +0200, Thomas Gleixner wrote:
> > On Thu, 6 Sep 2018, Christoph Hellwig wrote:
> > 
> > > Just as before:  NAK to entirely pointless abstractions.  Please stop
> > > beating the dead horse.
> > 
> > I disagree. These interrupts very well fit into the percpu interupt
> > mechanics and that allows them to be handled by all the generic mechanisms
> > as any other interrupt.
> 
> Just a few weeks ago you said the contrary:
> 
> http://lists.infradead.org/pipermail/linux-riscv/2018-August/000943.html

Sigh. Yes. Now that you remind me. 

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Thomas Gleixner]

On Mon, 10 Sep 2018, Thomas Gleixner wrote:
> On Mon, 10 Sep 2018, Christoph Hellwig wrote:
> > On Sat, Sep 08, 2018 at 12:46:35PM +0200, Thomas Gleixner wrote:
> > > On Thu, 6 Sep 2018, Christoph Hellwig wrote:
> > > 
> > > > Just as before:  NAK to entirely pointless abstractions.  Please stop
> > > > beating the dead horse.
> > > 
> > > I disagree. These interrupts very well fit into the percpu interupt
> > > mechanics and that allows them to be handled by all the generic mechanisms
> > > as any other interrupt.
> > 
> > Just a few weeks ago you said the contrary:
> > 
> > http://lists.infradead.org/pipermail/linux-riscv/2018-August/000943.html
> 
> Sigh. Yes. Now that you remind me. 

Just for clarification. I had the impression that Anup was trying to wire
up more than just the timer interrupt, but that doesn't seem to be the
case.

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Thomas Gleixner]

On Mon, 10 Sep 2018, Thomas Gleixner wrote:
> On Mon, 10 Sep 2018, Christoph Hellwig wrote:
> > On Sat, Sep 08, 2018 at 12:46:35PM +0200, Thomas Gleixner wrote:
> > > On Thu, 6 Sep 2018, Christoph Hellwig wrote:
> > > 
> > > > Just as before:  NAK to entirely pointless abstractions.  Please stop
> > > > beating the dead horse.
> > > 
> > > I disagree. These interrupts very well fit into the percpu interupt
> > > mechanics and that allows them to be handled by all the generic mechanisms
> > > as any other interrupt.
> > 
> > Just a few weeks ago you said the contrary:
> > 
> > http://lists.infradead.org/pipermail/linux-riscv/2018-August/000943.html
> 
> Sigh. Yes. Now that you remind me. 

Just for clarification. I had the impression that Anup was trying to wire
up more than just the timer interrupt, but that doesn't seem to be the
case.

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 03:37:31PM +0200, Thomas Gleixner wrote:
> > > Just a few weeks ago you said the contrary:
> > > 
> > > http://lists.infradead.org/pipermail/linux-riscv/2018-August/000943.html
> > 
> > Sigh. Yes. Now that you remind me. 
> 
> Just for clarification. I had the impression that Anup was trying to wire
> up more than just the timer interrupt, but that doesn't seem to be the
> case.

He has an irqchip that is called from the RISC-V exception handler
when the interrupt flag is set in scause and then dispatches to one
of:  IPI, timer, actual irqchip.

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 03:37:31PM +0200, Thomas Gleixner wrote:
> > > Just a few weeks ago you said the contrary:
> > > 
> > > http://lists.infradead.org/pipermail/linux-riscv/2018-August/000943.html
> > 
> > Sigh. Yes. Now that you remind me. 
> 
> Just for clarification. I had the impression that Anup was trying to wire
> up more than just the timer interrupt, but that doesn't seem to be the
> case.

He has an irqchip that is called from the RISC-V exception handler
when the interrupt flag is set in scause and then dispatches to one
of:  IPI, timer, actual irqchip.

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Thomas Gleixner]

On Mon, 10 Sep 2018, Christoph Hellwig wrote:
> On Mon, Sep 10, 2018 at 03:37:31PM +0200, Thomas Gleixner wrote:
> > > > Just a few weeks ago you said the contrary:
> > > > 
> > > > http://lists.infradead.org/pipermail/linux-riscv/2018-August/000943.html
> > > 
> > > Sigh. Yes. Now that you remind me. 
> > 
> > Just for clarification. I had the impression that Anup was trying to wire
> > up more than just the timer interrupt, but that doesn't seem to be the
> > case.
> 
> He has an irqchip that is called from the RISC-V exception handler
> when the interrupt flag is set in scause and then dispatches to one
> of:  IPI, timer, actual irqchip.

So the per cpu timer is the only per cpu interrupt and that thing is used
unconditionally, right?

Thanks,

	tglx

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Thomas Gleixner]

On Mon, 10 Sep 2018, Christoph Hellwig wrote:
> On Mon, Sep 10, 2018 at 03:37:31PM +0200, Thomas Gleixner wrote:
> > > > Just a few weeks ago you said the contrary:
> > > > 
> > > > http://lists.infradead.org/pipermail/linux-riscv/2018-August/000943.html
> > > 
> > > Sigh. Yes. Now that you remind me. 
> > 
> > Just for clarification. I had the impression that Anup was trying to wire
> > up more than just the timer interrupt, but that doesn't seem to be the
> > case.
> 
> He has an irqchip that is called from the RISC-V exception handler
> when the interrupt flag is set in scause and then dispatches to one
> of:  IPI, timer, actual irqchip.

So the per cpu timer is the only per cpu interrupt and that thing is used
unconditionally, right?

Thanks,

	tglx

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 03:45:42PM +0200, Thomas Gleixner wrote:
> > He has an irqchip that is called from the RISC-V exception handler
> > when the interrupt flag is set in scause and then dispatches to one
> > of:  IPI, timer, actual irqchip.
> 
> So the per cpu timer is the only per cpu interrupt and that thing is used
> unconditionally, right?

Yes.  external is chained and IPI is still handled explicitly.

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 03:45:42PM +0200, Thomas Gleixner wrote:
> > He has an irqchip that is called from the RISC-V exception handler
> > when the interrupt flag is set in scause and then dispatches to one
> > of:  IPI, timer, actual irqchip.
> 
> So the per cpu timer is the only per cpu interrupt and that thing is used
> unconditionally, right?

Yes.  external is chained and IPI is still handled explicitly.

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Mon, Sep 10, 2018 at 7:19 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Sep 10, 2018 at 03:45:42PM +0200, Thomas Gleixner wrote:
>> > He has an irqchip that is called from the RISC-V exception handler
>> > when the interrupt flag is set in scause and then dispatches to one
>> > of:  IPI, timer, actual irqchip.
>>
>> So the per cpu timer is the only per cpu interrupt and that thing is used
>> unconditionally, right?
>
> Yes.  external is chained and IPI is still handled explicitly.

On riscv64, there are 64 local interrupts (i.e. per-CPU interrupts).

Three of these local interrupts have clearly defined use:
1. Software interrupt (inter-processor interrupt)
2. External interrupt (interrupt from PLIC)
3. Timer interrupt (interrupt from per-CPU timer)

Other local interrupts are available for future use.

Taking inspiration from ARM world, I had give quite a few
examples how these RISC-V local interrupts can be used
for other purposes:
1. per-CPU interrupt for per-HART performance monitoring unit
2. interrupt controller virtualizaton extension can use per-CPU
interrupts for managing resources (just like ARM GICv2/v3 virt
extensions)
3. bus errors can be reported as per-CPU interrupts

Considering above, it is better to have a distinct irqchip and
irq_domain for all local interrupts (just like this patch).

Regards,
Anup

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Mon, Sep 10, 2018 at 7:19 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Sep 10, 2018 at 03:45:42PM +0200, Thomas Gleixner wrote:
>> > He has an irqchip that is called from the RISC-V exception handler
>> > when the interrupt flag is set in scause and then dispatches to one
>> > of:  IPI, timer, actual irqchip.
>>
>> So the per cpu timer is the only per cpu interrupt and that thing is used
>> unconditionally, right?
>
> Yes.  external is chained and IPI is still handled explicitly.

On riscv64, there are 64 local interrupts (i.e. per-CPU interrupts).

Three of these local interrupts have clearly defined use:
1. Software interrupt (inter-processor interrupt)
2. External interrupt (interrupt from PLIC)
3. Timer interrupt (interrupt from per-CPU timer)

Other local interrupts are available for future use.

Taking inspiration from ARM world, I had give quite a few
examples how these RISC-V local interrupts can be used
for other purposes:
1. per-CPU interrupt for per-HART performance monitoring unit
2. interrupt controller virtualizaton extension can use per-CPU
interrupts for managing resources (just like ARM GICv2/v3 virt
extensions)
3. bus errors can be reported as per-CPU interrupts

Considering above, it is better to have a distinct irqchip and
irq_domain for all local interrupts (just like this patch).

Regards,
Anup

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Thomas Gleixner]

On Mon, 10 Sep 2018, Anup Patel wrote:

> On Mon, Sep 10, 2018 at 7:19 PM, Christoph Hellwig <hch@infradead.org> wrote:
> > On Mon, Sep 10, 2018 at 03:45:42PM +0200, Thomas Gleixner wrote:
> >> > He has an irqchip that is called from the RISC-V exception handler
> >> > when the interrupt flag is set in scause and then dispatches to one
> >> > of:  IPI, timer, actual irqchip.
> >>
> >> So the per cpu timer is the only per cpu interrupt and that thing is used
> >> unconditionally, right?
> >
> > Yes.  external is chained and IPI is still handled explicitly.
> 
> On riscv64, there are 64 local interrupts (i.e. per-CPU interrupts).
> 
> Three of these local interrupts have clearly defined use:
> 1. Software interrupt (inter-processor interrupt)
> 2. External interrupt (interrupt from PLIC)
> 3. Timer interrupt (interrupt from per-CPU timer)
> 
> Other local interrupts are available for future use.
> 
> Taking inspiration from ARM world, I had give quite a few
> examples how these RISC-V local interrupts can be used
> for other purposes:
> 1. per-CPU interrupt for per-HART performance monitoring unit
> 2. interrupt controller virtualizaton extension can use per-CPU
> interrupts for managing resources (just like ARM GICv2/v3 virt
> extensions)
> 3. bus errors can be reported as per-CPU interrupts
> 
> Considering above, it is better to have a distinct irqchip and
> irq_domain for all local interrupts (just like this patch).

If that's the future usage and that's what my impression was, under which I
changed my mind, yes, then having a domain model is certainly of advantage
especially when those things end up being different per SoC.

Thanks,

	tglx

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Thomas Gleixner]

On Mon, 10 Sep 2018, Anup Patel wrote:

> On Mon, Sep 10, 2018 at 7:19 PM, Christoph Hellwig <hch@infradead.org> wrote:
> > On Mon, Sep 10, 2018 at 03:45:42PM +0200, Thomas Gleixner wrote:
> >> > He has an irqchip that is called from the RISC-V exception handler
> >> > when the interrupt flag is set in scause and then dispatches to one
> >> > of:  IPI, timer, actual irqchip.
> >>
> >> So the per cpu timer is the only per cpu interrupt and that thing is used
> >> unconditionally, right?
> >
> > Yes.  external is chained and IPI is still handled explicitly.
> 
> On riscv64, there are 64 local interrupts (i.e. per-CPU interrupts).
> 
> Three of these local interrupts have clearly defined use:
> 1. Software interrupt (inter-processor interrupt)
> 2. External interrupt (interrupt from PLIC)
> 3. Timer interrupt (interrupt from per-CPU timer)
> 
> Other local interrupts are available for future use.
> 
> Taking inspiration from ARM world, I had give quite a few
> examples how these RISC-V local interrupts can be used
> for other purposes:
> 1. per-CPU interrupt for per-HART performance monitoring unit
> 2. interrupt controller virtualizaton extension can use per-CPU
> interrupts for managing resources (just like ARM GICv2/v3 virt
> extensions)
> 3. bus errors can be reported as per-CPU interrupts
> 
> Considering above, it is better to have a distinct irqchip and
> irq_domain for all local interrupts (just like this patch).

If that's the future usage and that's what my impression was, under which I
changed my mind, yes, then having a domain model is certainly of advantage
especially when those things end up being different per SoC.

Thanks,

	tglx

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 06:07:12PM +0200, Thomas Gleixner wrote:
> > Considering above, it is better to have a distinct irqchip and
> > irq_domain for all local interrupts (just like this patch).
> 
> If that's the future usage

It's not, at least there has been no proposal for that so far, and I
don't really think it is how the architecture was intended.

> and that's what my impression was, under which I
> changed my mind, yes, then having a domain model is certainly of advantage
> especially when those things end up being different per SoC.

And even if we went down the way of using the other bits it would
be architectureal in the RISC-V spec - these are not available for
vendor specific uses.

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 06:07:12PM +0200, Thomas Gleixner wrote:
> > Considering above, it is better to have a distinct irqchip and
> > irq_domain for all local interrupts (just like this patch).
> 
> If that's the future usage

It's not, at least there has been no proposal for that so far, and I
don't really think it is how the architecture was intended.

> and that's what my impression was, under which I
> changed my mind, yes, then having a domain model is certainly of advantage
> especially when those things end up being different per SoC.

And even if we went down the way of using the other bits it would
be architectureal in the RISC-V spec - these are not available for
vendor specific uses.

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 07:59:15PM +0530, Anup Patel wrote:
> > Yes.  external is chained and IPI is still handled explicitly.
> 
> On riscv64, there are 64 local interrupts (i.e. per-CPU interrupts).

There aren't.  There are 9 right now, which are your three below:

> Three of these local interrupts have clearly defined use:
> 1. Software interrupt (inter-processor interrupt)
> 2. External interrupt (interrupt from PLIC)
> 3. Timer interrupt (interrupt from per-CPU timer)

multiplied by 3 for machine, supervisor, user.

> Other local interrupts are available for future use.

The others aren't even defined as other interrupts, but just reserved
fields.  And only one bit per privilege level would even fit into the
encoding scheme used right now.

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 07:59:15PM +0530, Anup Patel wrote:
> > Yes.  external is chained and IPI is still handled explicitly.
> 
> On riscv64, there are 64 local interrupts (i.e. per-CPU interrupts).

There aren't.  There are 9 right now, which are your three below:

> Three of these local interrupts have clearly defined use:
> 1. Software interrupt (inter-processor interrupt)
> 2. External interrupt (interrupt from PLIC)
> 3. Timer interrupt (interrupt from per-CPU timer)

multiplied by 3 for machine, supervisor, user.

> Other local interrupts are available for future use.

The others aren't even defined as other interrupts, but just reserved
fields.  And only one bit per privilege level would even fit into the
encoding scheme used right now.

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Mon, Sep 10, 2018 at 9:43 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Sep 10, 2018 at 07:59:15PM +0530, Anup Patel wrote:
>> > Yes.  external is chained and IPI is still handled explicitly.
>>
>> On riscv64, there are 64 local interrupts (i.e. per-CPU interrupts).
>
> There aren't.  There are 9 right now, which are your three below:

You are thinking very much in-context of SiFive CPUs only.

Lot of SOC vendors are trying to come-up with their own CPUs
and RISC-V spec does not restrict the use of local interrupts.

The mie/mip/sie/sip/uie/uip are all machine word size so on
riscv64 we can theoretically have maximum 64 local interrupts.

>
>> Three of these local interrupts have clearly defined use:
>> 1. Software interrupt (inter-processor interrupt)
>> 2. External interrupt (interrupt from PLIC)
>> 3. Timer interrupt (interrupt from per-CPU timer)
>
> multiplied by 3 for machine, supervisor, user.

Yes, 3 per-CPU interrupts for each CPU mode which
makes it 9 per-CPU interrupts.

In fact, initial intention was to have these separate for
each mode but there is not restriction as explained by
Andrew Waterman on ISA-DEV:

"One key point is that UIRQ, SIRQ, and MIRQ are just names; the letters
U, S, and M don't actually connote anything about privilege.  UIRQ,
SIRQ, and MIRQ are three different interrupt lines on equal footing.
The prefix letters merely reflect the intended use case.  So only one
version of this description is necessary.  I'll try to rewrite a
version with a few corrections."

>
>> Other local interrupts are available for future use.
>
> The others aren't even defined as other interrupts, but just reserved
> fields.  And only one bit per privilege level would even fit into the
> encoding scheme used right now.

That's how it is implemented on SiFive CPUs.

This does not mean we will not have more per-CPU interrupts
in-future.

Regards,
Anup

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Mon, Sep 10, 2018 at 9:43 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Sep 10, 2018 at 07:59:15PM +0530, Anup Patel wrote:
>> > Yes.  external is chained and IPI is still handled explicitly.
>>
>> On riscv64, there are 64 local interrupts (i.e. per-CPU interrupts).
>
> There aren't.  There are 9 right now, which are your three below:

You are thinking very much in-context of SiFive CPUs only.

Lot of SOC vendors are trying to come-up with their own CPUs
and RISC-V spec does not restrict the use of local interrupts.

The mie/mip/sie/sip/uie/uip are all machine word size so on
riscv64 we can theoretically have maximum 64 local interrupts.

>
>> Three of these local interrupts have clearly defined use:
>> 1. Software interrupt (inter-processor interrupt)
>> 2. External interrupt (interrupt from PLIC)
>> 3. Timer interrupt (interrupt from per-CPU timer)
>
> multiplied by 3 for machine, supervisor, user.

Yes, 3 per-CPU interrupts for each CPU mode which
makes it 9 per-CPU interrupts.

In fact, initial intention was to have these separate for
each mode but there is not restriction as explained by
Andrew Waterman on ISA-DEV:

"One key point is that UIRQ, SIRQ, and MIRQ are just names; the letters
U, S, and M don't actually connote anything about privilege.  UIRQ,
SIRQ, and MIRQ are three different interrupt lines on equal footing.
The prefix letters merely reflect the intended use case.  So only one
version of this description is necessary.  I'll try to rewrite a
version with a few corrections."

>
>> Other local interrupts are available for future use.
>
> The others aren't even defined as other interrupts, but just reserved
> fields.  And only one bit per privilege level would even fit into the
> encoding scheme used right now.

That's how it is implemented on SiFive CPUs.

This does not mean we will not have more per-CPU interrupts
in-future.

Regards,
Anup

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Mon, Sep 10, 2018 at 9:41 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Sep 10, 2018 at 06:07:12PM +0200, Thomas Gleixner wrote:
>> > Considering above, it is better to have a distinct irqchip and
>> > irq_domain for all local interrupts (just like this patch).
>>
>> If that's the future usage
>
> It's not, at least there has been no proposal for that so far, and I
> don't really think it is how the architecture was intended.
>
>> and that's what my impression was, under which I
>> changed my mind, yes, then having a domain model is certainly of advantage
>> especially when those things end up being different per SoC.
>
> And even if we went down the way of using the other bits it would
> be architectureal in the RISC-V spec - these are not available for
> vendor specific uses.

I am quite sure RISC-V spec does not restrict the use of other
local interrupts. Different CPU implementations can have their
own local interrupts.

Regards,
Anup

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Mon, Sep 10, 2018 at 9:41 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Sep 10, 2018 at 06:07:12PM +0200, Thomas Gleixner wrote:
>> > Considering above, it is better to have a distinct irqchip and
>> > irq_domain for all local interrupts (just like this patch).
>>
>> If that's the future usage
>
> It's not, at least there has been no proposal for that so far, and I
> don't really think it is how the architecture was intended.
>
>> and that's what my impression was, under which I
>> changed my mind, yes, then having a domain model is certainly of advantage
>> especially when those things end up being different per SoC.
>
> And even if we went down the way of using the other bits it would
> be architectureal in the RISC-V spec - these are not available for
> vendor specific uses.

I am quite sure RISC-V spec does not restrict the use of other
local interrupts. Different CPU implementations can have their
own local interrupts.

Regards,
Anup

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 10:02:09PM +0530, Anup Patel wrote:
> You are thinking very much in-context of SiFive CPUs only.

No.  I think in terms of the RISC-V spec.  I could care less about
SiFive to be honest.

> Lot of SOC vendors are trying to come-up with their own CPUs
> and RISC-V spec does not restrict the use of local interrupts.

Yes, it does.

> The mie/mip/sie/sip/uie/uip are all machine word size so on
> riscv64 we can theoretically have maximum 64 local interrupts.

They could in theory IFF someone actually get the use case through
the riscv privileged spec working group.

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 10:02:09PM +0530, Anup Patel wrote:
> You are thinking very much in-context of SiFive CPUs only.

No.  I think in terms of the RISC-V spec.  I could care less about
SiFive to be honest.

> Lot of SOC vendors are trying to come-up with their own CPUs
> and RISC-V spec does not restrict the use of local interrupts.

Yes, it does.

> The mie/mip/sie/sip/uie/uip are all machine word size so on
> riscv64 we can theoretically have maximum 64 local interrupts.

They could in theory IFF someone actually get the use case through
the riscv privileged spec working group.

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Mon, Sep 10, 2018 at 10:05 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Sep 10, 2018 at 10:02:09PM +0530, Anup Patel wrote:
>> You are thinking very much in-context of SiFive CPUs only.
>
> No.  I think in terms of the RISC-V spec.  I could care less about
> SiFive to be honest.
>
>> Lot of SOC vendors are trying to come-up with their own CPUs
>> and RISC-V spec does not restrict the use of local interrupts.
>
> Yes, it does.
>
>> The mie/mip/sie/sip/uie/uip are all machine word size so on
>> riscv64 we can theoretically have maximum 64 local interrupts.
>
> They could in theory IFF someone actually get the use case through
> the riscv privileged spec working group.

Their is no point in having each and every possible local interrupts
defined by RISC-V spec because some of these will be CPU
implementation specific in which case these local interrupts will
be described in platform specific DT passed to Linux.

Regards,
Anup

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Mon, Sep 10, 2018 at 10:05 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Sep 10, 2018 at 10:02:09PM +0530, Anup Patel wrote:
>> You are thinking very much in-context of SiFive CPUs only.
>
> No.  I think in terms of the RISC-V spec.  I could care less about
> SiFive to be honest.
>
>> Lot of SOC vendors are trying to come-up with their own CPUs
>> and RISC-V spec does not restrict the use of local interrupts.
>
> Yes, it does.
>
>> The mie/mip/sie/sip/uie/uip are all machine word size so on
>> riscv64 we can theoretically have maximum 64 local interrupts.
>
> They could in theory IFF someone actually get the use case through
> the riscv privileged spec working group.

Their is no point in having each and every possible local interrupts
defined by RISC-V spec because some of these will be CPU
implementation specific in which case these local interrupts will
be described in platform specific DT passed to Linux.

Regards,
Anup

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 10:05:42PM +0530, Anup Patel wrote:
> I am quite sure RISC-V spec does not restrict the use of other
> local interrupts. Different CPU implementations can have their
> own local interrupts.

Please take a look at sections 3.1.14 and 4.1.1 of the RISC-V privileged
spec 1.10.

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 10:05:42PM +0530, Anup Patel wrote:
> I am quite sure RISC-V spec does not restrict the use of other
> local interrupts. Different CPU implementations can have their
> own local interrupts.

Please take a look at sections 3.1.14 and 4.1.1 of the RISC-V privileged
spec 1.10.

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Mon, Sep 10, 2018 at 10:09 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Sep 10, 2018 at 10:05:42PM +0530, Anup Patel wrote:
>> I am quite sure RISC-V spec does not restrict the use of other
>> local interrupts. Different CPU implementations can have their
>> own local interrupts.
>
> Please take a look at sections 3.1.14 and 4.1.1 of the RISC-V privileged
> spec 1.10.

RISC-V priv spec 1.10 defines the 9 bits in MIE and MIP registers and
other bits are reserved.

The unused bits in MIP are WIRI (reserved write ignored and read ignored)
and unused bits in MIE are WPRI (reserved write preserve values and
read ignored).

The RISC-V priv spec 1.10 does not tell that unused reserved bits in
MIE/MIP cannot be used for:
1. CPU implementation specific local interrupts
2. Per-CPU device interrupts.

The RISC-V priv spec 1.10 tries to only describe MIE/MIP bits which
are mandatory on any RISC-V 1.10 compliant CPU but it possible to
used other reserved bits for implementation specific local interrupts.

Regards,
Anup

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Mon, Sep 10, 2018 at 10:09 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Sep 10, 2018 at 10:05:42PM +0530, Anup Patel wrote:
>> I am quite sure RISC-V spec does not restrict the use of other
>> local interrupts. Different CPU implementations can have their
>> own local interrupts.
>
> Please take a look at sections 3.1.14 and 4.1.1 of the RISC-V privileged
> spec 1.10.

RISC-V priv spec 1.10 defines the 9 bits in MIE and MIP registers and
other bits are reserved.

The unused bits in MIP are WIRI (reserved write ignored and read ignored)
and unused bits in MIE are WPRI (reserved write preserve values and
read ignored).

The RISC-V priv spec 1.10 does not tell that unused reserved bits in
MIE/MIP cannot be used for:
1. CPU implementation specific local interrupts
2. Per-CPU device interrupts.

The RISC-V priv spec 1.10 tries to only describe MIE/MIP bits which
are mandatory on any RISC-V 1.10 compliant CPU but it possible to
used other reserved bits for implementation specific local interrupts.

Regards,
Anup

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Thomas Gleixner]

On Mon, 10 Sep 2018, Anup Patel wrote:
> On Mon, Sep 10, 2018 at 10:09 PM, Christoph Hellwig <hch@infradead.org> wrote:
> > On Mon, Sep 10, 2018 at 10:05:42PM +0530, Anup Patel wrote:
> >> I am quite sure RISC-V spec does not restrict the use of other
> >> local interrupts. Different CPU implementations can have their
> >> own local interrupts.
> >
> > Please take a look at sections 3.1.14 and 4.1.1 of the RISC-V privileged
> > spec 1.10.
> 
> RISC-V priv spec 1.10 defines the 9 bits in MIE and MIP registers and
> other bits are reserved.
> 
> The unused bits in MIP are WIRI (reserved write ignored and read ignored)
> and unused bits in MIE are WPRI (reserved write preserve values and
> read ignored).
> 
> The RISC-V priv spec 1.10 does not tell that unused reserved bits in
> MIE/MIP cannot be used for:
> 1. CPU implementation specific local interrupts
> 2. Per-CPU device interrupts.
> 
> The RISC-V priv spec 1.10 tries to only describe MIE/MIP bits which
> are mandatory on any RISC-V 1.10 compliant CPU but it possible to
> used other reserved bits for implementation specific local interrupts.

Processor local interrupts really should be architected and there are
really not that many of them.

But well, RISC-V decided obvsiouly not to learn from mistakes made by
others.

That said, if your cpu local interrupts are not architectural and cannot be
made software configured architectural, then you will end up sooner than
later with the need for an irqdomain in order to handle the implementer
specific crap.

Thanks,

	tglx

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Thomas Gleixner]

On Mon, 10 Sep 2018, Anup Patel wrote:
> On Mon, Sep 10, 2018 at 10:09 PM, Christoph Hellwig <hch@infradead.org> wrote:
> > On Mon, Sep 10, 2018 at 10:05:42PM +0530, Anup Patel wrote:
> >> I am quite sure RISC-V spec does not restrict the use of other
> >> local interrupts. Different CPU implementations can have their
> >> own local interrupts.
> >
> > Please take a look at sections 3.1.14 and 4.1.1 of the RISC-V privileged
> > spec 1.10.
> 
> RISC-V priv spec 1.10 defines the 9 bits in MIE and MIP registers and
> other bits are reserved.
> 
> The unused bits in MIP are WIRI (reserved write ignored and read ignored)
> and unused bits in MIE are WPRI (reserved write preserve values and
> read ignored).
> 
> The RISC-V priv spec 1.10 does not tell that unused reserved bits in
> MIE/MIP cannot be used for:
> 1. CPU implementation specific local interrupts
> 2. Per-CPU device interrupts.
> 
> The RISC-V priv spec 1.10 tries to only describe MIE/MIP bits which
> are mandatory on any RISC-V 1.10 compliant CPU but it possible to
> used other reserved bits for implementation specific local interrupts.

Processor local interrupts really should be architected and there are
really not that many of them.

But well, RISC-V decided obvsiouly not to learn from mistakes made by
others.

That said, if your cpu local interrupts are not architectural and cannot be
made software configured architectural, then you will end up sooner than
later with the need for an irqdomain in order to handle the implementer
specific crap.

Thanks,

	tglx

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 10:41:22PM +0530, Anup Patel wrote:
> RISC-V priv spec 1.10 defines the 9 bits in MIE and MIP registers and
> other bits are reserved.
> 
> The unused bits in MIP are WIRI (reserved write ignored and read ignored)
> and unused bits in MIE are WPRI (reserved write preserve values and
> read ignored).
> 
> The RISC-V priv spec 1.10 does not tell that unused reserved bits in
> MIE/MIP cannot be used for:
> 1. CPU implementation specific local interrupts
> 2. Per-CPU device interrupts.
> 
> The RISC-V priv spec 1.10 tries to only describe MIE/MIP bits which
> are mandatory on any RISC-V 1.10 compliant CPU but it possible to
> used other reserved bits for implementation specific local interrupts.

Reserved means reserved for future versions of the spec, not for vendor
specific bad ideas.

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 10:41:22PM +0530, Anup Patel wrote:
> RISC-V priv spec 1.10 defines the 9 bits in MIE and MIP registers and
> other bits are reserved.
> 
> The unused bits in MIP are WIRI (reserved write ignored and read ignored)
> and unused bits in MIE are WPRI (reserved write preserve values and
> read ignored).
> 
> The RISC-V priv spec 1.10 does not tell that unused reserved bits in
> MIE/MIP cannot be used for:
> 1. CPU implementation specific local interrupts
> 2. Per-CPU device interrupts.
> 
> The RISC-V priv spec 1.10 tries to only describe MIE/MIP bits which
> are mandatory on any RISC-V 1.10 compliant CPU but it possible to
> used other reserved bits for implementation specific local interrupts.

Reserved means reserved for future versions of the spec, not for vendor
specific bad ideas.

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 09:37:59PM +0200, Thomas Gleixner wrote:
> Processor local interrupts really should be architected and there are
> really not that many of them.

And that is what they are.

> But well, RISC-V decided obvsiouly not to learn from mistakes made by
> others.

I don't think that is the case.  I think Atup misreads what reserved
means - if you look at section 2.3 of the RISC-V privileged spec
it clearly states that reserved fields are for future use and not
for vendor specific use.

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 09:37:59PM +0200, Thomas Gleixner wrote:
> Processor local interrupts really should be architected and there are
> really not that many of them.

And that is what they are.

> But well, RISC-V decided obvsiouly not to learn from mistakes made by
> others.

I don't think that is the case.  I think Atup misreads what reserved
means - if you look at section 2.3 of the RISC-V privileged spec
it clearly states that reserved fields are for future use and not
for vendor specific use.

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Tue, Sep 11, 2018 at 3:49 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Sep 10, 2018 at 09:37:59PM +0200, Thomas Gleixner wrote:
>> Processor local interrupts really should be architected and there are
>> really not that many of them.
>
> And that is what they are.
>
>> But well, RISC-V decided obvsiouly not to learn from mistakes made by
>> others.
>
> I don't think that is the case.  I think Atup misreads what reserved
> means - if you look at section 2.3 of the RISC-V privileged spec
> it clearly states that reserved fields are for future use and not
> for vendor specific use.

I think I understood what reserved means here. If reserved bits are
not for vendor specific or implementation specific stuff then it should
be mentioned clearly which is not the case.

The list of currently defined RISC-V local interrupts will definitely grow
based on my experience from ARM/ARM64 world.

Like Thomas mentioned, we will definitely end-up having separate
irqchip and irq_domain for RISC-V local interrupts for flexibility. Better
do it now with separate RISC-V INTC driver.

Regards,
Anup

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Tue, Sep 11, 2018 at 3:49 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Sep 10, 2018 at 09:37:59PM +0200, Thomas Gleixner wrote:
>> Processor local interrupts really should be architected and there are
>> really not that many of them.
>
> And that is what they are.
>
>> But well, RISC-V decided obvsiouly not to learn from mistakes made by
>> others.
>
> I don't think that is the case.  I think Atup misreads what reserved
> means - if you look at section 2.3 of the RISC-V privileged spec
> it clearly states that reserved fields are for future use and not
> for vendor specific use.

I think I understood what reserved means here. If reserved bits are
not for vendor specific or implementation specific stuff then it should
be mentioned clearly which is not the case.

The list of currently defined RISC-V local interrupts will definitely grow
based on my experience from ARM/ARM64 world.

Like Thomas mentioned, we will definitely end-up having separate
irqchip and irq_domain for RISC-V local interrupts for flexibility. Better
do it now with separate RISC-V INTC driver.

Regards,
Anup

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Tue, Sep 11, 2018 at 09:27:45AM +0530, Anup Patel wrote:
> The list of currently defined RISC-V local interrupts will definitely grow
> based on my experience from ARM/ARM64 world.
> 
> Like Thomas mentioned, we will definitely end-up having separate
> irqchip and irq_domain for RISC-V local interrupts for flexibility. Better
> do it now with separate RISC-V INTC driver.

I disagree.  Just because arm made a mess of their irq handling there
is no need to repeat mistakes by paving their way.  IFF we end up
with a convoluted mess like arm in the end we'll need an irqchip,
and as your series has shown that is fairly easily doable.  But I'd
rather spend my effort on it not becoming a mess in the first place
rather than helping the mess.

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Tue, Sep 11, 2018 at 09:27:45AM +0530, Anup Patel wrote:
> The list of currently defined RISC-V local interrupts will definitely grow
> based on my experience from ARM/ARM64 world.
> 
> Like Thomas mentioned, we will definitely end-up having separate
> irqchip and irq_domain for RISC-V local interrupts for flexibility. Better
> do it now with separate RISC-V INTC driver.

I disagree.  Just because arm made a mess of their irq handling there
is no need to repeat mistakes by paving their way.  IFF we end up
with a convoluted mess like arm in the end we'll need an irqchip,
and as your series has shown that is fairly easily doable.  But I'd
rather spend my effort on it not becoming a mess in the first place
rather than helping the mess.

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 10:08:58PM +0530, Anup Patel wrote:
> > They could in theory IFF someone actually get the use case through
> > the riscv privileged spec working group.
> 
> Their is no point in having each and every possible local interrupts
> defined by RISC-V spec because some of these will be CPU
> implementation specific in which case these local interrupts will
> be described in platform specific DT passed to Linux.

Again, to legally have implementation specific local interrupt types
you'll first need to convice the spec to change the status for those
fields from reserved to implementation specific.

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 10:08:58PM +0530, Anup Patel wrote:
> > They could in theory IFF someone actually get the use case through
> > the riscv privileged spec working group.
> 
> Their is no point in having each and every possible local interrupts
> defined by RISC-V spec because some of these will be CPU
> implementation specific in which case these local interrupts will
> be described in platform specific DT passed to Linux.

Again, to legally have implementation specific local interrupt types
you'll first need to convice the spec to change the status for those
fields from reserved to implementation specific.

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Mon, Sep 17, 2018 at 7:44 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Sep 10, 2018 at 10:08:58PM +0530, Anup Patel wrote:
> > > They could in theory IFF someone actually get the use case through
> > > the riscv privileged spec working group.
> >
> > Their is no point in having each and every possible local interrupts
> > defined by RISC-V spec because some of these will be CPU
> > implementation specific in which case these local interrupts will
> > be described in platform specific DT passed to Linux.
>
> Again, to legally have implementation specific local interrupt types
> you'll first need to convice the spec to change the status for those
> fields from reserved to implementation specific.

I agree, this needs to be first clarified in RISC-V spec. May be this is
a good topic for discussion in any upcoming RISC-V meetup.

Until then anyone can try these patches from riscv_intc_v2 branch of
https://github.com/avpatel/linux

Regards,
Anup

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Mon, Sep 17, 2018 at 7:44 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Sep 10, 2018 at 10:08:58PM +0530, Anup Patel wrote:
> > > They could in theory IFF someone actually get the use case through
> > > the riscv privileged spec working group.
> >
> > Their is no point in having each and every possible local interrupts
> > defined by RISC-V spec because some of these will be CPU
> > implementation specific in which case these local interrupts will
> > be described in platform specific DT passed to Linux.
>
> Again, to legally have implementation specific local interrupt types
> you'll first need to convice the spec to change the status for those
> fields from reserved to implementation specific.

I agree, this needs to be first clarified in RISC-V spec. May be this is
a good topic for discussion in any upcoming RISC-V meetup.

Until then anyone can try these patches from riscv_intc_v2 branch of
https://github.com/avpatel/linux

Regards,
Anup

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Mon, Sep 17, 2018 at 7:58 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Mon, Sep 17, 2018 at 7:44 PM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Mon, Sep 10, 2018 at 10:08:58PM +0530, Anup Patel wrote:
> > > > They could in theory IFF someone actually get the use case through
> > > > the riscv privileged spec working group.
> > >
> > > Their is no point in having each and every possible local interrupts
> > > defined by RISC-V spec because some of these will be CPU
> > > implementation specific in which case these local interrupts will
> > > be described in platform specific DT passed to Linux.
> >
> > Again, to legally have implementation specific local interrupt types
> > you'll first need to convice the spec to change the status for those
> > fields from reserved to implementation specific.
>
> I agree, this needs to be first clarified in RISC-V spec. May be this is
> a good topic for discussion in any upcoming RISC-V meetup.
>
> Until then anyone can try these patches from riscv_intc_v2 branch of
> https://github.com/avpatel/linux
>

I released that CLIC is going to be available for both M-mode and S-mode.
Software can choose to use HLIC or CLIC based on it's own preference.

If we are going to support both HLIC and CLIC in Linux kernel for per-CPU
local interrupts then we should definitely have irqdomain and irqchip for
per-CPU local interrupts. The selection between HLIC and CLIC will be
based on which driver gets probed via DT.

Regards,
Anup

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Mon, Sep 17, 2018 at 7:58 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Mon, Sep 17, 2018 at 7:44 PM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Mon, Sep 10, 2018 at 10:08:58PM +0530, Anup Patel wrote:
> > > > They could in theory IFF someone actually get the use case through
> > > > the riscv privileged spec working group.
> > >
> > > Their is no point in having each and every possible local interrupts
> > > defined by RISC-V spec because some of these will be CPU
> > > implementation specific in which case these local interrupts will
> > > be described in platform specific DT passed to Linux.
> >
> > Again, to legally have implementation specific local interrupt types
> > you'll first need to convice the spec to change the status for those
> > fields from reserved to implementation specific.
>
> I agree, this needs to be first clarified in RISC-V spec. May be this is
> a good topic for discussion in any upcoming RISC-V meetup.
>
> Until then anyone can try these patches from riscv_intc_v2 branch of
> https://github.com/avpatel/linux
>

I released that CLIC is going to be available for both M-mode and S-mode.
Software can choose to use HLIC or CLIC based on it's own preference.

If we are going to support both HLIC and CLIC in Linux kernel for per-CPU
local interrupts then we should definitely have irqdomain and irqchip for
per-CPU local interrupts. The selection between HLIC and CLIC will be
based on which driver gets probed via DT.

Regards,
Anup

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Anup Patel]

On Mon, Sep 17, 2018 at 7:58 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Mon, Sep 17, 2018 at 7:44 PM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Mon, Sep 10, 2018 at 10:08:58PM +0530, Anup Patel wrote:
> > > > They could in theory IFF someone actually get the use case through
> > > > the riscv privileged spec working group.
> > >
> > > Their is no point in having each and every possible local interrupts
> > > defined by RISC-V spec because some of these will be CPU
> > > implementation specific in which case these local interrupts will
> > > be described in platform specific DT passed to Linux.
> >
> > Again, to legally have implementation specific local interrupt types
> > you'll first need to convice the spec to change the status for those
> > fields from reserved to implementation specific.
>
> I agree, this needs to be first clarified in RISC-V spec. May be this is
> a good topic for discussion in any upcoming RISC-V meetup.
>
> Until then anyone can try these patches from riscv_intc_v2 branch of
> https://github.com/avpatel/linux
>

I released that CLIC is going to be available for both M-mode and S-mode.
Software can choose to use HLIC or CLIC based on it's own preference.

If we are going to support both HLIC and CLIC in Linux kernel for per-CPU
local interrupts then we should definitely have irqdomain and irqchip for
per-CPU local interrupts. The selection between HLIC and CLIC will be
based on which driver gets probed via DT.

Regards,
Anup

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Palmer Dabbelt]

On Tue, 25 Sep 2018 22:54:48 PDT (-0700), anup@brainfault.org wrote:
> On Mon, Sep 17, 2018 at 7:58 PM Anup Patel <anup@brainfault.org> wrote:
>>
>> On Mon, Sep 17, 2018 at 7:44 PM Christoph Hellwig <hch@infradead.org> wrote:
>> >
>> > On Mon, Sep 10, 2018 at 10:08:58PM +0530, Anup Patel wrote:
>> > > > They could in theory IFF someone actually get the use case through
>> > > > the riscv privileged spec working group.
>> > >
>> > > Their is no point in having each and every possible local interrupts
>> > > defined by RISC-V spec because some of these will be CPU
>> > > implementation specific in which case these local interrupts will
>> > > be described in platform specific DT passed to Linux.
>> >
>> > Again, to legally have implementation specific local interrupt types
>> > you'll first need to convice the spec to change the status for those
>> > fields from reserved to implementation specific.
>>
>> I agree, this needs to be first clarified in RISC-V spec. May be this is
>> a good topic for discussion in any upcoming RISC-V meetup.
>>
>> Until then anyone can try these patches from riscv_intc_v2 branch of
>> https://github.com/avpatel/linux
>>
>
> I released that CLIC is going to be available for both M-mode and S-mode.
> Software can choose to use HLIC or CLIC based on it's own preference.
>
> If we are going to support both HLIC and CLIC in Linux kernel for per-CPU
> local interrupts then we should definitely have irqdomain and irqchip for
> per-CPU local interrupts. The selection between HLIC and CLIC will be
> based on which driver gets probed via DT.

Yes, and note that the CLIC is actually an extension of what is currently 
available.  Thus we could produce a single driver that supports both, with 
features selected based on a DT entry.

I'm not sure if a native CLIC driver buys us anything in Linux, though, as 
IIRC there aren't that many CLIC features that will be particularly useful in 
our space.

[PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Palmer Dabbelt]

On Tue, 25 Sep 2018 22:54:48 PDT (-0700), anup at brainfault.org wrote:
> On Mon, Sep 17, 2018 at 7:58 PM Anup Patel <anup@brainfault.org> wrote:
>>
>> On Mon, Sep 17, 2018 at 7:44 PM Christoph Hellwig <hch@infradead.org> wrote:
>> >
>> > On Mon, Sep 10, 2018 at 10:08:58PM +0530, Anup Patel wrote:
>> > > > They could in theory IFF someone actually get the use case through
>> > > > the riscv privileged spec working group.
>> > >
>> > > Their is no point in having each and every possible local interrupts
>> > > defined by RISC-V spec because some of these will be CPU
>> > > implementation specific in which case these local interrupts will
>> > > be described in platform specific DT passed to Linux.
>> >
>> > Again, to legally have implementation specific local interrupt types
>> > you'll first need to convice the spec to change the status for those
>> > fields from reserved to implementation specific.
>>
>> I agree, this needs to be first clarified in RISC-V spec. May be this is
>> a good topic for discussion in any upcoming RISC-V meetup.
>>
>> Until then anyone can try these patches from riscv_intc_v2 branch of
>> https://github.com/avpatel/linux
>>
>
> I released that CLIC is going to be available for both M-mode and S-mode.
> Software can choose to use HLIC or CLIC based on it's own preference.
>
> If we are going to support both HLIC and CLIC in Linux kernel for per-CPU
> local interrupts then we should definitely have irqdomain and irqchip for
> per-CPU local interrupts. The selection between HLIC and CLIC will be
> based on which driver gets probed via DT.

Yes, and note that the CLIC is actually an extension of what is currently 
available.  Thus we could produce a single driver that supports both, with 
features selected based on a DT entry.

I'm not sure if a native CLIC driver buys us anything in Linux, though, as 
IIRC there aren't that many CLIC features that will be particularly useful in 
our space.

Re: [PATCH v2 3/5] irqchip: RISC-V Local Interrupt Controller Driver

[Palmer Dabbelt]

On Tue, 25 Sep 2018 22:54:48 PDT (-0700), anup@brainfault.org wrote:
> On Mon, Sep 17, 2018 at 7:58 PM Anup Patel <anup@brainfault.org> wrote:
>>
>> On Mon, Sep 17, 2018 at 7:44 PM Christoph Hellwig <hch@infradead.org> wrote:
>> >
>> > On Mon, Sep 10, 2018 at 10:08:58PM +0530, Anup Patel wrote:
>> > > > They could in theory IFF someone actually get the use case through
>> > > > the riscv privileged spec working group.
>> > >
>> > > Their is no point in having each and every possible local interrupts
>> > > defined by RISC-V spec because some of these will be CPU
>> > > implementation specific in which case these local interrupts will
>> > > be described in platform specific DT passed to Linux.
>> >
>> > Again, to legally have implementation specific local interrupt types
>> > you'll first need to convice the spec to change the status for those
>> > fields from reserved to implementation specific.
>>
>> I agree, this needs to be first clarified in RISC-V spec. May be this is
>> a good topic for discussion in any upcoming RISC-V meetup.
>>
>> Until then anyone can try these patches from riscv_intc_v2 branch of
>> https://github.com/avpatel/linux
>>
>
> I released that CLIC is going to be available for both M-mode and S-mode.
> Software can choose to use HLIC or CLIC based on it's own preference.
>
> If we are going to support both HLIC and CLIC in Linux kernel for per-CPU
> local interrupts then we should definitely have irqdomain and irqchip for
> per-CPU local interrupts. The selection between HLIC and CLIC will be
> based on which driver gets probed via DT.

Yes, and note that the CLIC is actually an extension of what is currently 
available.  Thus we could produce a single driver that supports both, with 
features selected based on a DT entry.

I'm not sure if a native CLIC driver buys us anything in Linux, though, as 
IIRC there aren't that many CLIC features that will be particularly useful in 
our space.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv