simplified RISC-V interrupt and clocksource handling v3

simplified RISC-V interrupt and clocksource handling v3

[Christoph Hellwig]

This series tries adds support for interrupt handling and timers
for the RISC-V architecture.

The basic per-hart interrupt handling implemented by the scause
and sie CSRs is extremely simple and implemented directly in
arch/riscv/kernel/irq.c.  In addition there is a irqchip driver
for the PLIC external interrupt controller, which is called through
the set_handle_irq API, and a clocksource driver that gets its
timer interrupt directly from the low-level interrupt handling.

Compared to previous iterations this version does not try to use an
irqchip driver for the low-level interrupt handling.  This saves
a couple indirect calls and an additional read of the scause CSR
in the hot path, makes the code much simpler and last but not least
avoid the dependency on a device tree for a mandatory architectural
feature.

A git tree is available here (contains a few more patches before
the ones in this series)

    git://git.infradead.org/users/hch/riscv.git riscv-irq-simple.3

Gitweb:

    http://git.infradead.org/users/hch/riscv.git/shortlog/refs/heads/riscv-irq-simple.3

Changes since v2:
 - actually use SEIE instead of STIE in the plic driver
 - rename the default compat string for the plic to sifive,u5-plic
 - various spelling fixes
 - drop a superflous derefence in the plic driver that is taken care of
   by the following loop
 - drop the patch to document the enable method - not relevant for the
   rest of the series
 - drop the patches for the per-hart timebase frequency - not relevant
   for the rest of the series.
 - use riscv_of_processor_hart in the timer driver

Changes since v1:
 - rename the plic driver to irq-sifive-plic
 - switch to a default compatible of sifive,plic0 (still supporting the
   riscv,plic0 name for compatibility)
 - add a reference for the SiFive PLIC register layout
 - fix plic_toggle addressing for large numbers of hwirqs
 - remove the call to ack_bad_irq
 - use a raw spinlock for plic_toggle_lock
 - use the irq_desc cpumask in the plic enable/disable methods
 - add back OF contexid parsing in the plic driver
 - don't allow COMPILE_TEST builds of the clocksource driver, as it
   depends on <asm/sbi.h>
 - default the clocksource driver to y
 - clean up naming in the clocksource driver
 - remove the MINDELTA and MAXDELTA #defines
 - various DT binding fixes

[PATCH 1/8] RISC-V: remove timer leftovers

[Christoph Hellwig]

This code is currently unused and will be added back later in a different
place with the real interrupt and clocksource support.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/riscv/kernel/time.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/arch/riscv/kernel/time.c b/arch/riscv/kernel/time.c
index 2463fcca719e..0df9b2cbd645 100644
--- a/arch/riscv/kernel/time.c
+++ b/arch/riscv/kernel/time.c
@@ -13,32 +13,11 @@
  */
 
 #include <linux/clocksource.h>
-#include <linux/clockchips.h>
 #include <linux/delay.h>
-
-#ifdef CONFIG_RISCV_TIMER
-#include <linux/timer_riscv.h>
-#endif
-
 #include <asm/sbi.h>
 
 unsigned long riscv_timebase;
 
-DECLARE_PER_CPU(struct clock_event_device, riscv_clock_event);
-
-void riscv_timer_interrupt(void)
-{
-#ifdef CONFIG_RISCV_TIMER
-	/*
-	 * FIXME: This needs to be cleaned up along with the rest of the IRQ
-	 * handling cleanup.  See irq.c for more details.
-	 */
-	struct clock_event_device *evdev = this_cpu_ptr(&riscv_clock_event);
-
-	evdev->event_handler(evdev);
-#endif
-}
-
 void __init init_clockevent(void)
 {
 	timer_probe();
-- 
2.18.0

[PATCH 2/8] RISC-V: simplify software interrupt / IPI code

[Christoph Hellwig]

Rename handle_ipi to riscv_software_interrupt, drop the unused return
value and move the prototype to irq.h together with riscv_timer_interupt.
This allows simplifying the upcoming interrupt handling support.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/riscv/include/asm/irq.h | 1 +
 arch/riscv/include/asm/smp.h | 3 ---
 arch/riscv/kernel/smp.c      | 6 ++----
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/riscv/include/asm/irq.h b/arch/riscv/include/asm/irq.h
index 4dee9d4c13c0..c871661c9df4 100644
--- a/arch/riscv/include/asm/irq.h
+++ b/arch/riscv/include/asm/irq.h
@@ -22,6 +22,7 @@
 #define INTERRUPT_CAUSE_EXTERNAL    9
 
 void riscv_timer_interrupt(void);
+void riscv_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 85e4220839b0..c9395fff246f 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -44,9 +44,6 @@ void arch_send_call_function_single_ipi(int cpu);
  */
 #define raw_smp_processor_id() (*((int*)((char*)get_current() + TASK_TI_CPU)))
 
-/* Interprocessor interrupt handler */
-irqreturn_t handle_ipi(void);
-
 #endif /* CONFIG_SMP */
 
 #endif /* _ASM_RISCV_SMP_H */
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index 6d3962435720..906fe21ea21b 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -45,7 +45,7 @@ int setup_profiling_timer(unsigned int multiplier)
 	return -EINVAL;
 }
 
-irqreturn_t handle_ipi(void)
+void riscv_software_interrupt(void)
 {
 	unsigned long *pending_ipis = &ipi_data[smp_processor_id()].bits;
 
@@ -60,7 +60,7 @@ irqreturn_t handle_ipi(void)
 
 		ops = xchg(pending_ipis, 0);
 		if (ops == 0)
-			return IRQ_HANDLED;
+			return;
 
 		if (ops & (1 << IPI_RESCHEDULE))
 			scheduler_ipi();
@@ -73,8 +73,6 @@ irqreturn_t handle_ipi(void)
 		/* Order data access and bit testing. */
 		mb();
 	}
-
-	return IRQ_HANDLED;
 }
 
 static void
-- 
2.18.0

[PATCH 3/8] RISC-V: remove INTERRUPT_CAUSE_* defines from asm/irq.h

[Christoph Hellwig]

These are only of use to the local irq controller driver, so add them in
that driver implementation instead, which will be submitted soon.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/riscv/include/asm/irq.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/riscv/include/asm/irq.h b/arch/riscv/include/asm/irq.h
index c871661c9df4..996b6fbe17a6 100644
--- a/arch/riscv/include/asm/irq.h
+++ b/arch/riscv/include/asm/irq.h
@@ -17,10 +17,6 @@
 
 #define NR_IRQS         0
 
-#define INTERRUPT_CAUSE_SOFTWARE    1
-#define INTERRUPT_CAUSE_TIMER       5
-#define INTERRUPT_CAUSE_EXTERNAL    9
-
 void riscv_timer_interrupt(void);
 void riscv_software_interrupt(void);
 
-- 
2.18.0

[PATCH 4/8] RISC-V: add a definition for the SIE SEIE bit

[Christoph Hellwig]

This mirrors the SIE_SSIE and SETE bits that are used in a similar
fashion.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/riscv/include/asm/csr.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index 421fa3585798..28a0d1cb374c 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -54,6 +54,7 @@
 /* Interrupt Enable and Interrupt Pending flags */
 #define SIE_SSIE _AC(0x00000002, UL) /* Software Interrupt Enable */
 #define SIE_STIE _AC(0x00000020, UL) /* Timer Interrupt Enable */
+#define SIE_SEIE _AC(0x00000200, UL) /* External Interrupt Enable */
 
 #define EXC_INST_MISALIGNED     0
 #define EXC_INST_ACCESS         1
-- 
2.18.0

[PATCH 5/8] RISC-V: implement low-level interrupt handling

[Christoph Hellwig]

Add support for a routine that dispatches exceptions with the interrupt
flags set to either the IPI or irqdomain code (and the clock source in the
future).

Loosely based on the irq-riscv-int.c irqchip driver from the RISC-V tree.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/riscv/kernel/entry.S |  4 +--
 arch/riscv/kernel/irq.c   | 52 ++++++++++++++++++++++++++++++++-------
 2 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 9aaf6c986771..fa2c08e3c05e 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -168,8 +168,8 @@ ENTRY(handle_exception)
 
 	/* Handle interrupts */
 	move a0, sp /* pt_regs */
-	REG_L a1, handle_arch_irq
-	jr a1
+	move a1, s4 /* scause */
+	tail do_IRQ
 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 7bcdaed15703..ab5f3e22c7cc 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -1,21 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright (C) 2012 Regents of the University of California
  * Copyright (C) 2017 SiFive
- *
- *   This program is free software; you can redistribute it and/or
- *   modify it under the terms of the GNU General Public License
- *   as published by the Free Software Foundation, version 2.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY; without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *   GNU General Public License for more details.
+ * Copyright (C) 2018 Christoph Hellwig
  */
 
 #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, unsigned long cause)
+{
+	struct pt_regs *old_regs = set_irq_regs(regs);
+
+	irq_enter();
+	switch (cause & ~INTERRUPT_CAUSE_FLAG) {
+#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.
+		 */
+		riscv_software_interrupt();
+		break;
+#endif
+	case INTERRUPT_CAUSE_EXTERNAL:
+		handle_arch_irq(regs);
+		break;
+	default:
+		panic("unexpected interrupt cause");
+	}
+	irq_exit();
+
+	set_irq_regs(old_regs);
+}
+
 void __init init_IRQ(void)
 {
 	irqchip_init();
-- 
2.18.0

[PATCH 6/8] dt-bindings: interrupt-controller: RISC-V PLIC documentation

[Christoph Hellwig]

From: Palmer Dabbelt <palmer@dabbelt.com>

This patch adds documentation for the platform-level interrupt
controller (PLIC) found in all RISC-V systems.  This interrupt
controller routes interrupts from all the devices in the system to each
hart-local interrupt controller.

Note: the DTS bindings for the PLIC aren't set in stone yet, as we might
want to change how we're specifying holes in the hart list.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
[hch: various fixes and updates]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 .../interrupt-controller/sifive,plic0.txt     | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
new file mode 100644
index 000000000000..bbfa61cf8d3f
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
@@ -0,0 +1,57 @@
+SiFive Platform-Level Interrupt Controller (PLIC)
+-------------------------------------------------
+
+SiFive SOCs include an implementation of the Platform-Level Interrupt Controller
+(PLIC) high-level specification in the RISC-V Privileged Architecture
+specification.  The PLIC connects all external interrupts in the system to all
+hart contexts in the system, via the external interrupt source in each hart.
+
+A hart context is a privilege mode in a hardware execution thread.  For example,
+in an 4 core system with 2-way SMT, you have 8 harts and probably at least two
+privilege modes per hart; machine mode and supervisor mode.
+
+Each interrupt can be enabled on per-context basis.  Any context can claim
+a pending enabled interrupt and then release it once it has been handled.
+
+Each interrupt has a configurable priority.  Higher priority interrupts are
+serviced first.  Each context can specify a priority threshold. Interrupts
+with priority below this threshold will not cause the PLIC to raise its
+interrupt line leading to the context.
+
+While the PLIC supports both edge-triggered and level-triggered interrupts,
+interrupt handlers are oblivious to this distinction and therefore it is not
+specified in the PLIC device-tree binding.
+
+While the RISC-V ISA doesn't specify a memory layout for the PLIC, the
+"sifive,plic0" device is a concrete implementation of the PLIC that contains a
+specific memory layout, which is documented in chapter 8 of the SiFive U5
+Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
+
+Required properties:
+- compatible : "sifive,plic0".
+- #address-cells : should be <0>.
+- #interrupt-cells : should be <1>.
+- interrupt-controller : Identifies the node as an interrupt controller.
+- reg : Should contain 1 register range (address and length).
+- interrupts-extended : Specifies which contexts are connected to the PLIC,
+  with "-1" specifying that a context is not present.  The nodes pointed
+  to should be "riscv" HART nodes, or eventually be parented by such nodes.
+- riscv,ndev: Specifies how many external interrupts are supported by
+  this controller.
+
+Example:
+
+	plic: interrupt-controller@c000000 {
+		#address-cells = <0>;
+		#interrupt-cells = <1>;
+		compatible = "riscv,plic0";
+		interrupt-controller;
+		interrupts-extended = <
+			&cpu0-intc 11
+			&cpu1-intc 11 &cpu1-intc 9
+			&cpu2-intc 11 &cpu2-intc 9
+			&cpu3-intc 11 &cpu3-intc 9
+			&cpu4-intc 11 &cpu4-intc 9>;
+		reg = <0xc000000 0x4000000>;
+		riscv,ndev = <10>;
+	};
-- 
2.18.0

Re: [PATCH 6/8] dt-bindings: interrupt-controller: RISC-V PLIC documentation

[Rob Herring]

On Sat, Aug 4, 2018 at 2:23 AM Christoph Hellwig <hch@lst.de> wrote:
>
> From: Palmer Dabbelt <palmer@dabbelt.com>

Version numbers on the individual patches would be nice...

> This patch adds documentation for the platform-level interrupt
> controller (PLIC) found in all RISC-V systems.  This interrupt
> controller routes interrupts from all the devices in the system to each
> hart-local interrupt controller.
>
> Note: the DTS bindings for the PLIC aren't set in stone yet, as we might
> want to change how we're specifying holes in the hart list.
>
> Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
> [hch: various fixes and updates]
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  .../interrupt-controller/sifive,plic0.txt     | 57 +++++++++++++++++++
>  1 file changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
> new file mode 100644
> index 000000000000..bbfa61cf8d3f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic0.txt
> @@ -0,0 +1,57 @@
> +SiFive Platform-Level Interrupt Controller (PLIC)
> +-------------------------------------------------
> +
> +SiFive SOCs include an implementation of the Platform-Level Interrupt Controller
> +(PLIC) high-level specification in the RISC-V Privileged Architecture
> +specification.  The PLIC connects all external interrupts in the system to all
> +hart contexts in the system, via the external interrupt source in each hart.
> +
> +A hart context is a privilege mode in a hardware execution thread.  For example,
> +in an 4 core system with 2-way SMT, you have 8 harts and probably at least two
> +privilege modes per hart; machine mode and supervisor mode.
> +
> +Each interrupt can be enabled on per-context basis.  Any context can claim
> +a pending enabled interrupt and then release it once it has been handled.
> +
> +Each interrupt has a configurable priority.  Higher priority interrupts are
> +serviced first.  Each context can specify a priority threshold. Interrupts
> +with priority below this threshold will not cause the PLIC to raise its
> +interrupt line leading to the context.
> +
> +While the PLIC supports both edge-triggered and level-triggered interrupts,
> +interrupt handlers are oblivious to this distinction and therefore it is not
> +specified in the PLIC device-tree binding.
> +
> +While the RISC-V ISA doesn't specify a memory layout for the PLIC, the
> +"sifive,plic0" device is a concrete implementation of the PLIC that contains a
> +specific memory layout, which is documented in chapter 8 of the SiFive U5
> +Coreplex Series Manual <https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf>.
> +
> +Required properties:
> +- compatible : "sifive,plic0".
> +- #address-cells : should be <0>.
> +- #interrupt-cells : should be <1>.
> +- interrupt-controller : Identifies the node as an interrupt controller.
> +- reg : Should contain 1 register range (address and length).
> +- interrupts-extended : Specifies which contexts are connected to the PLIC,
> +  with "-1" specifying that a context is not present.  The nodes pointed
> +  to should be "riscv" HART nodes, or eventually be parented by such nodes.
> +- riscv,ndev: Specifies how many external interrupts are supported by
> +  this controller.
> +
> +Example:
> +
> +       plic: interrupt-controller@c000000 {
> +               #address-cells = <0>;
> +               #interrupt-cells = <1>;
> +               compatible = "riscv,plic0";
> +               interrupt-controller;
> +               interrupts-extended = <
> +                       &cpu0-intc 11
> +                       &cpu1-intc 11 &cpu1-intc 9
> +                       &cpu2-intc 11 &cpu2-intc 9
> +                       &cpu3-intc 11 &cpu3-intc 9
> +                       &cpu4-intc 11 &cpu4-intc 9>;

I'm confused why this is still here if you are dropping the cpu intc binding?

I also noticed the cpu binding refers to "riscv,cpu-intc" as well.
That needs to be fixed too if there's a change.

Rob

Re: [PATCH 6/8] dt-bindings: interrupt-controller: RISC-V PLIC documentation

[Christoph Hellwig]

On Wed, Aug 08, 2018 at 08:29:50AM -0600, Rob Herring wrote:
> Version numbers on the individual patches would be nice...

We've never done these in the subsystems I'm involved with.  Too
much clutter in the subject lines for information that is easily
deductable.

> > +Example:
> > +
> > +       plic: interrupt-controller@c000000 {
> > +               #address-cells = <0>;
> > +               #interrupt-cells = <1>;
> > +               compatible = "riscv,plic0";
> > +               interrupt-controller;
> > +               interrupts-extended = <
> > +                       &cpu0-intc 11
> > +                       &cpu1-intc 11 &cpu1-intc 9
> > +                       &cpu2-intc 11 &cpu2-intc 9
> > +                       &cpu3-intc 11 &cpu3-intc 9
> > +                       &cpu4-intc 11 &cpu4-intc 9>;
> 
> I'm confused why this is still here if you are dropping the cpu intc binding?

We need some parent that identifies the core (hart in RISC-V terminology).
The way the code now works is that it just walks up the parent chain
until it finds a CPU node, so it either accepts the legacy intc node
inbetween, or it accepts the cpu node directly as the intc node is pointless.

I guess for the documentation we should instead just point to the
"riscv" cpu nodes instead?

> I also noticed the cpu binding refers to "riscv,cpu-intc" as well.
> That needs to be fixed too if there's a change.

Only in the examples.  I'd be fine with dropping them, but let's keep
that separate from the interrupt support.

Re: [PATCH 6/8] dt-bindings: interrupt-controller: RISC-V PLIC documentation

[Rob Herring]

On Wed, Aug 8, 2018 at 8:59 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Wed, Aug 08, 2018 at 08:29:50AM -0600, Rob Herring wrote:
> > Version numbers on the individual patches would be nice...
>
> We've never done these in the subsystems I'm involved with.  Too
> much clutter in the subject lines for information that is easily
> deductable.

Unfortunately not in Gmail which doesn't thread properly. But
patchwork also provides the version tag which I use to sort my
reviews.

> > > +Example:
> > > +
> > > +       plic: interrupt-controller@c000000 {
> > > +               #address-cells = <0>;
> > > +               #interrupt-cells = <1>;
> > > +               compatible = "riscv,plic0";
> > > +               interrupt-controller;
> > > +               interrupts-extended = <
> > > +                       &cpu0-intc 11
> > > +                       &cpu1-intc 11 &cpu1-intc 9
> > > +                       &cpu2-intc 11 &cpu2-intc 9
> > > +                       &cpu3-intc 11 &cpu3-intc 9
> > > +                       &cpu4-intc 11 &cpu4-intc 9>;
> >
> > I'm confused why this is still here if you are dropping the cpu intc binding?
>
> We need some parent that identifies the core (hart in RISC-V terminology).
> The way the code now works is that it just walks up the parent chain
> until it finds a CPU node, so it either accepts the legacy intc node
> inbetween, or it accepts the cpu node directly as the intc node is pointless.
>
> I guess for the documentation we should instead just point to the
> "riscv" cpu nodes instead?

That's not valid and dtc will tell you that. 'interrupts' (via
interrupt-parent) or 'interrupts-extended' has to point to an
'interrupt-controller' node. I guess you could make the cpu nodes
interrupt-controllers. That's a bit strange, but I can't think of a
reason why that wouldn't work.

Just because the cpu-intc is not made to be an irqchip in the kernel
doesn't mean it can't still be represented as an interrupt-controller
in DT. It shouldn't be designed just around how some OS happens to
implement things.

> > I also noticed the cpu binding refers to "riscv,cpu-intc" as well.
> > That needs to be fixed too if there's a change.
>
> Only in the examples.  I'd be fine with dropping them, but let's keep
> that separate from the interrupt support.

You need to sort out how this is all tied together and works because
right now you are supporting 2 ways and one is undocumented and the
other is invalid. Changing things later is only going to be more
painful.

Rob

Re: [PATCH 6/8] dt-bindings: interrupt-controller: RISC-V PLIC documentation

[Christoph Hellwig]

On Wed, Aug 08, 2018 at 10:15:58AM -0600, Rob Herring wrote:
> 'interrupts' (via
> interrupt-parent) or 'interrupts-extended' has to point to an
> 'interrupt-controller' node. I guess you could make the cpu nodes
> interrupt-controllers. That's a bit strange, but I can't think of a
> reason why that wouldn't work.

It could work, and would actually match how the hardware works
fairly closely.

> Just because the cpu-intc is not made to be an irqchip in the kernel
> doesn't mean it can't still be represented as an interrupt-controller
> in DT. It shouldn't be designed just around how some OS happens to
> implement things.

Independent of how you implement it, there isn't really such a thing
as the cpu-intc.  The CPU itself has a number of exceptions, that
are all handled the same way.  One of them just happens to be
the connection to an external interrupt controller.

That being said I'm fine with keeping up the pretence (at least in
DT) that it is a separate entity and resubmit the cpu-intc docs
given how widespread they exist already.

Re: [PATCH 6/8] dt-bindings: interrupt-controller: RISC-V PLIC documentation

[Palmer Dabbelt]

On Wed, 08 Aug 2018 09:15:58 PDT (-0700), robh+dt@kernel.org wrote:
> On Wed, Aug 8, 2018 at 8:59 AM Christoph Hellwig <hch@lst.de> wrote:
>>
>> On Wed, Aug 08, 2018 at 08:29:50AM -0600, Rob Herring wrote:
>> > Version numbers on the individual patches would be nice...
>>
>> We've never done these in the subsystems I'm involved with.  Too
>> much clutter in the subject lines for information that is easily
>> deductable.
>
> Unfortunately not in Gmail which doesn't thread properly. But
> patchwork also provides the version tag which I use to sort my
> reviews.
>
>> > > +Example:
>> > > +
>> > > +       plic: interrupt-controller@c000000 {
>> > > +               #address-cells = <0>;
>> > > +               #interrupt-cells = <1>;
>> > > +               compatible = "riscv,plic0";
>> > > +               interrupt-controller;
>> > > +               interrupts-extended = <
>> > > +                       &cpu0-intc 11
>> > > +                       &cpu1-intc 11 &cpu1-intc 9
>> > > +                       &cpu2-intc 11 &cpu2-intc 9
>> > > +                       &cpu3-intc 11 &cpu3-intc 9
>> > > +                       &cpu4-intc 11 &cpu4-intc 9>;
>> >
>> > I'm confused why this is still here if you are dropping the cpu intc binding?
>>
>> We need some parent that identifies the core (hart in RISC-V terminology).
>> The way the code now works is that it just walks up the parent chain
>> until it finds a CPU node, so it either accepts the legacy intc node
>> inbetween, or it accepts the cpu node directly as the intc node is pointless.
>>
>> I guess for the documentation we should instead just point to the
>> "riscv" cpu nodes instead?
>
> That's not valid and dtc will tell you that. 'interrupts' (via
> interrupt-parent) or 'interrupts-extended' has to point to an
> 'interrupt-controller' node. I guess you could make the cpu nodes
> interrupt-controllers. That's a bit strange, but I can't think of a
> reason why that wouldn't work.
>
> Just because the cpu-intc is not made to be an irqchip in the kernel
> doesn't mean it can't still be represented as an interrupt-controller
> in DT. It shouldn't be designed just around how some OS happens to
> implement things.

FWIW, I like this approach.  There is an interrupt widget in the hardware, so 
having the device tree represent it seems like a good idea.

>> > I also noticed the cpu binding refers to "riscv,cpu-intc" as well.
>> > That needs to be fixed too if there's a change.
>>
>> Only in the examples.  I'd be fine with dropping them, but let's keep
>> that separate from the interrupt support.
>
> You need to sort out how this is all tied together and works because
> right now you are supporting 2 ways and one is undocumented and the
> other is invalid. Changing things later is only going to be more
> painful.
>
> Rob

[PATCH 7/8] irqchip: add a SiFive PLIC driver

[Christoph Hellwig]

Adds a driver for the SiFive implementation of the RISC-V Platform Level
Interrupt Controller (PLIC).  The PLIC connects global interrupt sources
to the local interrupt controller on each hart.

This driver is based on the driver in the RISC-V tree from Palmer Dabbelt,
but has been almost entirely rewritten since, and includes many fixes
from Atish Patra.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/riscv/configs/defconfig      |   1 +
 drivers/irqchip/Kconfig           |  12 ++
 drivers/irqchip/Makefile          |   1 +
 drivers/irqchip/irq-sifive-plic.c | 259 ++++++++++++++++++++++++++++++
 4 files changed, 273 insertions(+)
 create mode 100644 drivers/irqchip/irq-sifive-plic.c

diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index 07326466871b..36473d7dbaac 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -76,3 +76,4 @@ CONFIG_ROOT_NFS=y
 CONFIG_CRYPTO_USER_API_HASH=y
 CONFIG_MODULES=y
 CONFIG_MODULE_UNLOAD=y
+CONFIG_SIFIVE_PLIC=y
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index e9233db16e03..df345b878ac2 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -372,3 +372,15 @@ config QCOM_PDC
 	  IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
 
 endmenu
+
+config SIFIVE_PLIC
+	bool "SiFive Platform-Level Interrupt Controller"
+	depends on RISCV
+	help
+	   This enables support for the PLIC chip found in SiFive (and
+	   potentially other) RISC-V systems.  The PLIC controls devices
+	   interrupts and connects them to each core's local interrupt
+	   controller.  Aside from timer and software interrupts, all other
+	   interrupt sources are subordinate to the PLIC.
+
+	   If you don't know what to do here, say Y.
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 15f268f646bf..fbd1ec8070ef 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -87,3 +87,4 @@ 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_SIFIVE_PLIC)		+= irq-sifive-plic.o
diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
new file mode 100644
index 000000000000..faacf428e250
--- /dev/null
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -0,0 +1,259 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2017 SiFive
+ * Copyright (C) 2018 Christoph Hellwig
+ */
+#define pr_fmt(fmt) "plic: " fmt
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+/*
+ * This driver implements a version of the RISC-V PLIC with the actual layout
+ * specified in chapter 8 of the SiFive U5 Coreplex Series Manual:
+ *
+ *     https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf
+ *
+ * The largest number supported by devices marked as 'riscv,plic0', is 1024, of
+ * which device 0 is defined as non-existent by the RISC-V Privileged Spec.
+ */
+
+#define MAX_DEVICES			1024
+#define MAX_CONTEXTS			15872
+
+/*
+ * Each interrupt source has a priority register associated with it.
+ * We always hardwire it to one in Linux.
+ */
+#define PRIORITY_BASE			0
+#define     PRIORITY_PER_ID		4
+
+/*
+ * Each hart context has a vector of interrupt enable bits associated with it.
+ * There's one bit for each interrupt source.
+ */
+#define ENABLE_BASE			0x2000
+#define     ENABLE_PER_HART		0x80
+
+/*
+ * Each hart context has a set of control registers associated with it.  Right
+ * now there's only two: a source priority threshold over which the hart will
+ * take an interrupt, and a register to claim interrupts.
+ */
+#define CONTEXT_BASE			0x200000
+#define     CONTEXT_PER_HART		0x1000
+#define     CONTEXT_THRESHOLD		0x00
+#define     CONTEXT_CLAIM		0x04
+
+static void __iomem *plic_regs;
+
+struct plic_handler {
+	bool			present;
+	int			ctxid;
+};
+static DEFINE_PER_CPU(struct plic_handler, plic_handlers);
+
+static inline void __iomem *plic_hart_offset(int ctxid)
+{
+	return plic_regs + CONTEXT_BASE + ctxid * CONTEXT_PER_HART;
+}
+
+static inline u32 __iomem *plic_enable_base(int ctxid)
+{
+	return plic_regs + ENABLE_BASE + ctxid * ENABLE_PER_HART;
+}
+
+/*
+ * Protect mask operations on the registers given that we can't assume that
+ * atomic memory operations work on them.
+ */
+static DEFINE_RAW_SPINLOCK(plic_toggle_lock);
+
+static inline void plic_toggle(int ctxid, int hwirq, int enable)
+{
+	u32 __iomem *reg = plic_enable_base(ctxid) + (hwirq / 32);
+	u32 hwirq_mask = 1 << (hwirq % 32);
+
+	raw_spin_lock(&plic_toggle_lock);
+	if (enable)
+		writel(readl(reg) | hwirq_mask, reg);
+	else
+		writel(readl(reg) & ~hwirq_mask, reg);
+	raw_spin_unlock(&plic_toggle_lock);
+}
+
+static inline void plic_irq_toggle(struct irq_data *d, int enable)
+{
+	int cpu;
+
+	writel(enable, plic_regs + PRIORITY_BASE + d->hwirq * PRIORITY_PER_ID);
+	for_each_cpu(cpu, irq_data_get_affinity_mask(d)) {
+		struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu);
+
+		if (handler->present)
+			plic_toggle(handler->ctxid, d->hwirq, enable);
+	}
+}
+
+static void plic_irq_enable(struct irq_data *d)
+{
+	plic_irq_toggle(d, 1);
+}
+
+static void plic_irq_disable(struct irq_data *d)
+{
+	plic_irq_toggle(d, 0);
+}
+
+static struct irq_chip plic_chip = {
+	.name		= "SiFive PLIC",
+	/*
+	 * There is no need to mask/unmask PLIC interrupts.  They are "masked"
+	 * by reading claim and "unmasked" when writing it back.
+	 */
+	.irq_enable	= plic_irq_enable,
+	.irq_disable	= plic_irq_disable,
+};
+
+static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq,
+			      irq_hw_number_t hwirq)
+{
+	irq_set_chip_and_handler(irq, &plic_chip, handle_simple_irq);
+	irq_set_chip_data(irq, NULL);
+	irq_set_noprobe(irq);
+	return 0;
+}
+
+static const struct irq_domain_ops plic_irqdomain_ops = {
+	.map		= plic_irqdomain_map,
+	.xlate		= irq_domain_xlate_onecell,
+};
+
+static struct irq_domain *plic_irqdomain;
+
+/*
+ * Handling an interrupt is a two-step process: first you claim the interrupt
+ * by reading the claim register, then you complete the interrupt by writing
+ * 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)
+{
+	struct plic_handler *handler = this_cpu_ptr(&plic_handlers);
+	void __iomem *claim = plic_hart_offset(handler->ctxid) + CONTEXT_CLAIM;
+	irq_hw_number_t hwirq;
+
+	WARN_ON_ONCE(!handler->present);
+
+	csr_clear(sie, SIE_SEIE);
+	while ((hwirq = readl(claim))) {
+		int irq = irq_find_mapping(plic_irqdomain, hwirq);
+
+		if (unlikely(irq <= 0))
+			pr_warn_ratelimited("can't find mapping for hwirq %lu\n",
+					hwirq);
+		else
+			generic_handle_irq(irq);
+		writel(hwirq, claim);
+	}
+	csr_set(sie, SIE_SEIE);
+}
+
+/*
+ * Walk up the DT tree until we find a active RISC-V core (HART) node and
+ * extract the cpuid from it.
+ */
+static int plic_find_hart_id(struct device_node *node)
+{
+	for (; node; node = node->parent) {
+		if (of_device_is_compatible(node, "riscv"))
+			return riscv_of_processor_hart(node);
+	}
+
+	return -1;
+}
+
+static int __init plic_init(struct device_node *node,
+		struct device_node *parent)
+{
+	int error = 0, nr_handlers, nr_mapped = 0, i;
+	u32 nr_irqs;
+
+	if (plic_regs) {
+		pr_warn("PLIC already present.\n");
+		return -ENXIO;
+	}
+
+	plic_regs = of_iomap(node, 0);
+	if (WARN_ON(!plic_regs))
+		return -EIO;
+
+	error = -EINVAL;
+	of_property_read_u32(node, "riscv,ndev", &nr_irqs);
+	if (WARN_ON(!nr_irqs))
+		goto out_iounmap;
+
+	nr_handlers = of_irq_count(node);
+	if (WARN_ON(!nr_handlers))
+		goto out_iounmap;
+	if (WARN_ON(nr_handlers < num_possible_cpus()))
+		goto out_iounmap;
+
+	error = -ENOMEM;
+	plic_irqdomain = irq_domain_add_linear(node, nr_irqs + 1,
+			&plic_irqdomain_ops, NULL);
+	if (WARN_ON(!plic_irqdomain))
+		goto out_iounmap;
+
+	for (i = 0; i < nr_handlers; i++) {
+		struct of_phandle_args parent;
+		struct plic_handler *handler;
+		irq_hw_number_t hwirq;
+		int cpu;
+
+		if (of_irq_parse_one(node, i, &parent)) {
+			pr_err("failed to parse parent for context %d.\n", i);
+			continue;
+		}
+
+		/* skip context holes */
+		if (parent.args[0] == -1)
+			continue;
+
+		cpu = plic_find_hart_id(parent.np);
+		if (cpu < 0) {
+			pr_warn("failed to parse hart ID for context %d.\n", i);
+			continue;
+		}
+
+		handler = per_cpu_ptr(&plic_handlers, cpu);
+		handler->present = true;
+		handler->ctxid = i;
+
+		/* priority must be > threshold to trigger an interrupt */
+		writel(0, plic_hart_offset(i) + CONTEXT_THRESHOLD);
+		for (hwirq = 1; hwirq <= nr_irqs; hwirq++)
+			plic_toggle(i, hwirq, 0);
+		nr_mapped++;
+	}
+
+	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:
+	iounmap(plic_regs);
+	return error;
+}
+
+IRQCHIP_DECLARE(sifive_plic0, "sifive,u5-plic", plic_init);
+IRQCHIP_DECLARE(riscv_plic0, "riscv,plic0", plic_init); /* for legacy systems */
-- 
2.18.0

Re: [PATCH 7/8] irqchip: add a SiFive PLIC driver

[Atish Patra]

On 8/4/18 1:23 AM, Christoph Hellwig wrote:
> Adds a driver for the SiFive implementation of the RISC-V Platform Level
> Interrupt Controller (PLIC).  The PLIC connects global interrupt sources
> to the local interrupt controller on each hart.
> 
> This driver is based on the driver in the RISC-V tree from Palmer Dabbelt,
> but has been almost entirely rewritten since, and includes many fixes
> from Atish Patra.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>   arch/riscv/configs/defconfig      |   1 +
>   drivers/irqchip/Kconfig           |  12 ++
>   drivers/irqchip/Makefile          |   1 +
>   drivers/irqchip/irq-sifive-plic.c | 259 ++++++++++++++++++++++++++++++
>   4 files changed, 273 insertions(+)
>   create mode 100644 drivers/irqchip/irq-sifive-plic.c
> 
> diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
> index 07326466871b..36473d7dbaac 100644
> --- a/arch/riscv/configs/defconfig
> +++ b/arch/riscv/configs/defconfig
> @@ -76,3 +76,4 @@ CONFIG_ROOT_NFS=y
>   CONFIG_CRYPTO_USER_API_HASH=y
>   CONFIG_MODULES=y
>   CONFIG_MODULE_UNLOAD=y
> +CONFIG_SIFIVE_PLIC=y
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index e9233db16e03..df345b878ac2 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -372,3 +372,15 @@ config QCOM_PDC
>   	  IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
>   
>   endmenu
> +
> +config SIFIVE_PLIC
> +	bool "SiFive Platform-Level Interrupt Controller"
> +	depends on RISCV
> +	help
> +	   This enables support for the PLIC chip found in SiFive (and
> +	   potentially other) RISC-V systems.  The PLIC controls devices
> +	   interrupts and connects them to each core's local interrupt
> +	   controller.  Aside from timer and software interrupts, all other
> +	   interrupt sources are subordinate to the PLIC.
> +
> +	   If you don't know what to do here, say Y.
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 15f268f646bf..fbd1ec8070ef 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -87,3 +87,4 @@ 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_SIFIVE_PLIC)		+= irq-sifive-plic.o
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> new file mode 100644
> index 000000000000..faacf428e250
> --- /dev/null
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -0,0 +1,259 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2017 SiFive
> + * Copyright (C) 2018 Christoph Hellwig
> + */
> +#define pr_fmt(fmt) "plic: " fmt
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +
> +/*
> + * This driver implements a version of the RISC-V PLIC with the actual layout
> + * specified in chapter 8 of the SiFive U5 Coreplex Series Manual:
> + *
> + *     https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf
> + *
> + * The largest number supported by devices marked as 'riscv,plic0', is 1024, of
> + * which device 0 is defined as non-existent by the RISC-V Privileged Spec.
> + */
> +
> +#define MAX_DEVICES			1024
> +#define MAX_CONTEXTS			15872
> +
> +/*
> + * Each interrupt source has a priority register associated with it.
> + * We always hardwire it to one in Linux.
> + */
> +#define PRIORITY_BASE			0
> +#define     PRIORITY_PER_ID		4
> +
> +/*
> + * Each hart context has a vector of interrupt enable bits associated with it.
> + * There's one bit for each interrupt source.
> + */
> +#define ENABLE_BASE			0x2000
> +#define     ENABLE_PER_HART		0x80
> +
> +/*
> + * Each hart context has a set of control registers associated with it.  Right
> + * now there's only two: a source priority threshold over which the hart will
> + * take an interrupt, and a register to claim interrupts.
> + */
> +#define CONTEXT_BASE			0x200000
> +#define     CONTEXT_PER_HART		0x1000
> +#define     CONTEXT_THRESHOLD		0x00
> +#define     CONTEXT_CLAIM		0x04
> +
> +static void __iomem *plic_regs;
> +
> +struct plic_handler {
> +	bool			present;
> +	int			ctxid;
> +};
> +static DEFINE_PER_CPU(struct plic_handler, plic_handlers);
> +
> +static inline void __iomem *plic_hart_offset(int ctxid)
> +{
> +	return plic_regs + CONTEXT_BASE + ctxid * CONTEXT_PER_HART;
> +}
> +
> +static inline u32 __iomem *plic_enable_base(int ctxid)
> +{
> +	return plic_regs + ENABLE_BASE + ctxid * ENABLE_PER_HART;
> +}
> +
> +/*
> + * Protect mask operations on the registers given that we can't assume that
> + * atomic memory operations work on them.
> + */
> +static DEFINE_RAW_SPINLOCK(plic_toggle_lock);
> +
> +static inline void plic_toggle(int ctxid, int hwirq, int enable)
> +{
> +	u32 __iomem *reg = plic_enable_base(ctxid) + (hwirq / 32);
> +	u32 hwirq_mask = 1 << (hwirq % 32);
> +
> +	raw_spin_lock(&plic_toggle_lock);
> +	if (enable)
> +		writel(readl(reg) | hwirq_mask, reg);
> +	else
> +		writel(readl(reg) & ~hwirq_mask, reg);
> +	raw_spin_unlock(&plic_toggle_lock);
> +}
> +
> +static inline void plic_irq_toggle(struct irq_data *d, int enable)
> +{
> +	int cpu;
> +
> +	writel(enable, plic_regs + PRIORITY_BASE + d->hwirq * PRIORITY_PER_ID);
> +	for_each_cpu(cpu, irq_data_get_affinity_mask(d)) {
> +		struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu);
> +
> +		if (handler->present)
> +			plic_toggle(handler->ctxid, d->hwirq, enable);
> +	}
> +}
> +
> +static void plic_irq_enable(struct irq_data *d)
> +{
> +	plic_irq_toggle(d, 1);
> +}
> +
> +static void plic_irq_disable(struct irq_data *d)
> +{
> +	plic_irq_toggle(d, 0);
> +}
> +
> +static struct irq_chip plic_chip = {
> +	.name		= "SiFive PLIC",
> +	/*
> +	 * There is no need to mask/unmask PLIC interrupts.  They are "masked"
> +	 * by reading claim and "unmasked" when writing it back.
> +	 */
> +	.irq_enable	= plic_irq_enable,
> +	.irq_disable	= plic_irq_disable,
> +};
> +
> +static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq,
> +			      irq_hw_number_t hwirq)
> +{
> +	irq_set_chip_and_handler(irq, &plic_chip, handle_simple_irq);
> +	irq_set_chip_data(irq, NULL);
> +	irq_set_noprobe(irq);
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops plic_irqdomain_ops = {
> +	.map		= plic_irqdomain_map,
> +	.xlate		= irq_domain_xlate_onecell,
> +};
> +
> +static struct irq_domain *plic_irqdomain;
> +
> +/*
> + * Handling an interrupt is a two-step process: first you claim the interrupt
> + * by reading the claim register, then you complete the interrupt by writing
> + * 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)
> +{
> +	struct plic_handler *handler = this_cpu_ptr(&plic_handlers);
> +	void __iomem *claim = plic_hart_offset(handler->ctxid) + CONTEXT_CLAIM;
> +	irq_hw_number_t hwirq;
> +
> +	WARN_ON_ONCE(!handler->present);
> +
> +	csr_clear(sie, SIE_SEIE);
> +	while ((hwirq = readl(claim))) {
> +		int irq = irq_find_mapping(plic_irqdomain, hwirq);
> +
> +		if (unlikely(irq <= 0))
> +			pr_warn_ratelimited("can't find mapping for hwirq %lu\n",
> +					hwirq);
> +		else
> +			generic_handle_irq(irq);
> +		writel(hwirq, claim);
> +	}
> +	csr_set(sie, SIE_SEIE);
> +}
> +
> +/*
> + * Walk up the DT tree until we find a active RISC-V core (HART) node and
/s/a/an

> + * extract the cpuid from it.
> + */
> +static int plic_find_hart_id(struct device_node *node)
> +{
> +	for (; node; node = node->parent) {
> +		if (of_device_is_compatible(node, "riscv"))
> +			return riscv_of_processor_hart(node);
> +	}
> +
> +	return -1;
> +}
> +
> +static int __init plic_init(struct device_node *node,
> +		struct device_node *parent)
> +{
> +	int error = 0, nr_handlers, nr_mapped = 0, i;
> +	u32 nr_irqs;
> +
> +	if (plic_regs) {
> +		pr_warn("PLIC already present.\n");
> +		return -ENXIO;
> +	}
> +
> +	plic_regs = of_iomap(node, 0);
> +	if (WARN_ON(!plic_regs))
> +		return -EIO;
> +
> +	error = -EINVAL;
> +	of_property_read_u32(node, "riscv,ndev", &nr_irqs);
> +	if (WARN_ON(!nr_irqs))
> +		goto out_iounmap;
> +
> +	nr_handlers = of_irq_count(node);
> +	if (WARN_ON(!nr_handlers))
> +		goto out_iounmap;
> +	if (WARN_ON(nr_handlers < num_possible_cpus()))
> +		goto out_iounmap;
> +
> +	error = -ENOMEM;
> +	plic_irqdomain = irq_domain_add_linear(node, nr_irqs + 1,
> +			&plic_irqdomain_ops, NULL);
> +	if (WARN_ON(!plic_irqdomain))
> +		goto out_iounmap;
> +
> +	for (i = 0; i < nr_handlers; i++) {
> +		struct of_phandle_args parent;
> +		struct plic_handler *handler;
> +		irq_hw_number_t hwirq;
> +		int cpu;
> +
> +		if (of_irq_parse_one(node, i, &parent)) {
> +			pr_err("failed to parse parent for context %d.\n", i);
> +			continue;
> +		}
> +
> +		/* skip context holes */
> +		if (parent.args[0] == -1)
> +			continue;
> +
> +		cpu = plic_find_hart_id(parent.np);
> +		if (cpu < 0) {
> +			pr_warn("failed to parse hart ID for context %d.\n", i);
> +			continue;
> +		}
> +
> +		handler = per_cpu_ptr(&plic_handlers, cpu);
> +		handler->present = true;
> +		handler->ctxid = i;
> +
> +		/* priority must be > threshold to trigger an interrupt */
> +		writel(0, plic_hart_offset(i) + CONTEXT_THRESHOLD);
> +		for (hwirq = 1; hwirq <= nr_irqs; hwirq++)
> +			plic_toggle(i, hwirq, 0);
> +		nr_mapped++;
> +	}
> +
> +	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:
> +	iounmap(plic_regs);
> +	return error;
> +}
> +
> +IRQCHIP_DECLARE(sifive_plic0, "sifive,u5-plic", plic_init);
Should it be sifive,u54-plic ?

> +IRQCHIP_DECLARE(riscv_plic0, "riscv,plic0", plic_init); /* for legacy systems */
> 

Otherwise, Looks good to me. FWIW..
Reviewed-by: Atish Patra <atish.patra@wdc.com>

Regards,
Atish

[PATCH 8/8] clocksource: new RISC-V SBI timer driver

[Christoph Hellwig]

From: Palmer Dabbelt <palmer@dabbelt.com>

The RISC-V ISA defines a per-hart real-time clock and timer, which is
present on all systems.  The clock is accessed via the 'rdtime'
pseudo-instruction (which reads a CSR), and the timer is set via an SBI
call.

Contains various improvements from Atish Patra <atish.patra@wdc.com>.

Signed-off-by: Dmitriy Cherkasov <dmitriy@oss-tech.org>
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
[hch: remove dead code, add SPDX tags, used riscv_of_processor_hart(),
 minor cleanups, merged  hotplug cpu support and other improvements
 from Atish]
Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/include/asm/smp.h      |   3 -
 arch/riscv/kernel/irq.c           |   3 +
 arch/riscv/kernel/smpboot.c       |   1 -
 arch/riscv/kernel/time.c          |   9 +--
 drivers/clocksource/Kconfig       |  11 ++++
 drivers/clocksource/Makefile      |   1 +
 drivers/clocksource/riscv_timer.c | 105 ++++++++++++++++++++++++++++++
 include/linux/cpuhotplug.h        |   1 +
 8 files changed, 122 insertions(+), 12 deletions(-)
 create mode 100644 drivers/clocksource/riscv_timer.c

diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
index c9395fff246f..36016845461d 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -24,9 +24,6 @@
 
 #ifdef CONFIG_SMP
 
-/* SMP initialization hook for setup_arch */
-void __init init_clockevent(void);
-
 /* SMP initialization hook for setup_arch */
 void __init setup_smp(void);
 
diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
index ab5f3e22c7cc..0cfac48a1272 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -30,6 +30,9 @@ asmlinkage void __irq_entry do_IRQ(struct pt_regs *regs, unsigned long cause)
 
 	irq_enter();
 	switch (cause & ~INTERRUPT_CAUSE_FLAG) {
+	case INTERRUPT_CAUSE_TIMER:
+		riscv_timer_interrupt();
+		break;
 #ifdef CONFIG_SMP
 	case INTERRUPT_CAUSE_SOFTWARE:
 		/*
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index f741458c5a3f..56abab6a9812 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -104,7 +104,6 @@ asmlinkage void __init smp_callin(void)
 	current->active_mm = mm;
 
 	trap_init();
-	init_clockevent();
 	notify_cpu_starting(smp_processor_id());
 	set_cpu_online(smp_processor_id(), 1);
 	local_flush_tlb_all();
diff --git a/arch/riscv/kernel/time.c b/arch/riscv/kernel/time.c
index 0df9b2cbd645..1911c8f6b8a6 100644
--- a/arch/riscv/kernel/time.c
+++ b/arch/riscv/kernel/time.c
@@ -18,12 +18,6 @@
 
 unsigned long riscv_timebase;
 
-void __init init_clockevent(void)
-{
-	timer_probe();
-	csr_set(sie, SIE_STIE);
-}
-
 void __init time_init(void)
 {
 	struct device_node *cpu;
@@ -35,6 +29,5 @@ void __init time_init(void)
 	riscv_timebase = prop;
 
 	lpj_fine = riscv_timebase / HZ;
-
-	init_clockevent();
+	timer_probe();
 }
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index dec0dd88ec15..a11f4ba98b05 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -609,4 +609,15 @@ config ATCPIT100_TIMER
 	help
 	  This option enables support for the Andestech ATCPIT100 timers.
 
+config RISCV_TIMER
+	bool "Timer for the RISC-V platform"
+	depends on RISCV
+	default y
+	select TIMER_PROBE
+	select TIMER_OF
+	help
+	  This enables the per-hart timer built into all RISC-V systems, which
+	  is accessed via both the SBI and the rdcycle instruction.  This is
+	  required for all RISC-V systems.
+
 endmenu
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 00caf37e52f9..ded31f720bd9 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -78,3 +78,4 @@ obj-$(CONFIG_H8300_TPU)			+= h8300_tpu.o
 obj-$(CONFIG_CLKSRC_ST_LPC)		+= clksrc_st_lpc.o
 obj-$(CONFIG_X86_NUMACHIP)		+= numachip.o
 obj-$(CONFIG_ATCPIT100_TIMER)		+= timer-atcpit100.o
+obj-$(CONFIG_RISCV_TIMER)		+= riscv_timer.o
diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c
new file mode 100644
index 000000000000..4e8b347e43e2
--- /dev/null
+++ b/drivers/clocksource/riscv_timer.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2012 Regents of the University of California
+ * Copyright (C) 2017 SiFive
+ */
+#include <linux/clocksource.h>
+#include <linux/clockchips.h>
+#include <linux/cpu.h>
+#include <linux/delay.h>
+#include <linux/irq.h>
+#include <asm/sbi.h>
+
+/*
+ * All RISC-V systems have a timer attached to every hart.  These timers can be
+ * read by the 'rdcycle' pseudo instruction, and can use the SBI to setup
+ * events.  In order to abstract the architecture-specific timer reading and
+ * setting functions away from the clock event insertion code, we provide
+ * function pointers to the clockevent subsystem that perform two basic
+ * operations: rdtime() reads the timer on the current CPU, and
+ * next_event(delta) sets the next timer event to 'delta' cycles in the future.
+ * As the timers are inherently a per-cpu resource, these callbacks perform
+ * operations on the current hart.  There is guaranteed to be exactly one timer
+ * per hart on all RISC-V systems.
+ */
+
+static int riscv_clock_next_event(unsigned long delta,
+		struct clock_event_device *ce)
+{
+	csr_set(sie, SIE_STIE);
+	sbi_set_timer(get_cycles64() + delta);
+	return 0;
+}
+
+static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = {
+	.name			= "riscv_timer_clockevent",
+	.features		= CLOCK_EVT_FEAT_ONESHOT,
+	.rating			= 100,
+	.set_next_event		= riscv_clock_next_event,
+};
+
+/*
+ * 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
+ * backwards when hopping between CPUs, practically it won't happen.
+ */
+static unsigned long long riscv_clocksource_rdtime(struct clocksource *cs)
+{
+	return get_cycles64();
+}
+
+static DEFINE_PER_CPU(struct clocksource, riscv_clocksource) = {
+	.name		= "riscv_clocksource",
+	.rating		= 300,
+	.mask		= CLOCKSOURCE_MASK(BITS_PER_LONG),
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+	.read		= riscv_clocksource_rdtime,
+};
+
+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);
+	clockevents_config_and_register(ce, riscv_timebase, 100, 0x7fffffff);
+
+	csr_set(sie, SIE_STIE);
+	return 0;
+}
+
+static int riscv_timer_dying_cpu(unsigned int cpu)
+{
+	csr_clear(sie, SIE_STIE);
+	return 0;
+}
+
+/* called directly from the low-level interrupt handler */
+void riscv_timer_interrupt(void)
+{
+	struct clock_event_device *evdev = this_cpu_ptr(&riscv_clock_event);
+
+	csr_clear(sie, SIE_STIE);
+	evdev->event_handler(evdev);
+}
+
+static int __init riscv_timer_init_dt(struct device_node *n)
+{
+	int cpu_id = riscv_of_processor_hart(n), error;
+	struct clocksource *cs;
+
+	if (cpu_id != smp_processor_id())
+		return 0;
+
+	cs = per_cpu_ptr(&riscv_clocksource, cpu_id);
+	clocksource_register_hz(cs, riscv_timebase);
+
+	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, cpu_id);
+	return error;
+}
+
+TIMER_OF_DECLARE(riscv_timer, "riscv", riscv_timer_init_dt);
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 8796ba387152..554c27f6cfbd 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -125,6 +125,7 @@ enum cpuhp_state {
 	CPUHP_AP_MARCO_TIMER_STARTING,
 	CPUHP_AP_MIPS_GIC_TIMER_STARTING,
 	CPUHP_AP_ARC_TIMER_STARTING,
+	CPUHP_AP_RISCV_TIMER_STARTING,
 	CPUHP_AP_KVM_STARTING,
 	CPUHP_AP_KVM_ARM_VGIC_INIT_STARTING,
 	CPUHP_AP_KVM_ARM_VGIC_STARTING,
-- 
2.18.0

Re: simplified RISC-V interrupt and clocksource handling v3

[Palmer Dabbelt]

On Sat, 04 Aug 2018 01:23:11 PDT (-0700), Christoph Hellwig wrote:
> This series tries adds support for interrupt handling and timers
> for the RISC-V architecture.
>
> The basic per-hart interrupt handling implemented by the scause
> and sie CSRs is extremely simple and implemented directly in
> arch/riscv/kernel/irq.c.  In addition there is a irqchip driver
> for the PLIC external interrupt controller, which is called through
> the set_handle_irq API, and a clocksource driver that gets its
> timer interrupt directly from the low-level interrupt handling.
>
> Compared to previous iterations this version does not try to use an
> irqchip driver for the low-level interrupt handling.  This saves
> a couple indirect calls and an additional read of the scause CSR
> in the hot path, makes the code much simpler and last but not least
> avoid the dependency on a device tree for a mandatory architectural
> feature.
>
> A git tree is available here (contains a few more patches before
> the ones in this series)
>
>     git://git.infradead.org/users/hch/riscv.git riscv-irq-simple.3
>
> Gitweb:
>
>     http://git.infradead.org/users/hch/riscv.git/shortlog/refs/heads/riscv-irq-simple.3
>
> Changes since v2:
>  - actually use SEIE instead of STIE in the plic driver
>  - rename the default compat string for the plic to sifive,u5-plic
>  - various spelling fixes
>  - drop a superflous derefence in the plic driver that is taken care of
>    by the following loop
>  - drop the patch to document the enable method - not relevant for the
>    rest of the series
>  - drop the patches for the per-hart timebase frequency - not relevant
>    for the rest of the series.
>  - use riscv_of_processor_hart in the timer driver
>
> Changes since v1:
>  - rename the plic driver to irq-sifive-plic
>  - switch to a default compatible of sifive,plic0 (still supporting the
>    riscv,plic0 name for compatibility)
>  - add a reference for the SiFive PLIC register layout
>  - fix plic_toggle addressing for large numbers of hwirqs
>  - remove the call to ack_bad_irq
>  - use a raw spinlock for plic_toggle_lock
>  - use the irq_desc cpumask in the plic enable/disable methods
>  - add back OF contexid parsing in the plic driver
>  - don't allow COMPILE_TEST builds of the clocksource driver, as it
>    depends on <asm/sbi.h>
>  - default the clocksource driver to y
>  - clean up naming in the clocksource driver
>  - remove the MINDELTA and MAXDELTA #defines
>  - various DT binding fixes

Thanks!  Modulo the one device tree issue I replied to in patch 3 this looks 
great!  We've already gotten the ACKs to take this through the RISC-V tree, so 
I'm going to put this along with the queued RISC-V patches on our for-next 
branch, including my proposed change for "sifive,plic-1.0" but leaving the 
device tree bindings with #{address,size}-cells=1.

We can always change this, but I'd like to get this out so people can start 
playing with it earlier rather than later.

Thanks to everyone for all the help!

Re: simplified RISC-V interrupt and clocksource handling v3

[Christoph Hellwig]

[attention Rob: Palmer said he is going to pull it in, and I'd really
 like to have your ACK on the DT bindings, can you chime in if
 everything is ok for you?]

On Tue, Aug 07, 2018 at 07:23:19PM -0700, Palmer Dabbelt wrote:
> Thanks!  Modulo the one device tree issue I replied to in patch 3 this 
> looks great!  We've already gotten the ACKs to take this through the RISC-V 
> tree, so I'm going to put this along with the queued RISC-V patches on our 
> for-next branch, including my proposed change for "sifive,plic-1.0" but 
> leaving the device tree bindings with #{address,size}-cells=1.

Note that I saw a branch that only has the actual driver patch, this
needs to be in the documentation as well of couse.  I don't really
care which name we settle on as long we agree on it, and document it
properly.