simplified RISC-V interrupt and clocksource handling v4

simplified RISC-V interrupt and clocksource handling v4

[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).  The series is against the riscv for-next
tree, so using the git tree is advised

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

Gitweb:

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

Changes since v3:
 - more improvements to the DT bindings
 - add back the patch adding the riscv,cpu-intc binding documentation
   as it is referenced from others

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 v4 1/3] dt-bindings: interrupt-controller: RISC-V local interrupt controller

[Christoph Hellwig]

From: Palmer Dabbelt <palmer@dabbelt.com>

Add documentation on the RISC-V local interrupt controller, which is a
per-hart interrupt controller that manages all interrupts entering a
RISC-V hart.  This interrupt controller is present on all RISC-V systems.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
[hch: minor cleanups]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 .../interrupt-controller/riscv,cpu-intc.txt   | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt

diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
new file mode 100644
index 000000000000..b0a8af51c388
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
@@ -0,0 +1,44 @@
+RISC-V Hart-Level Interrupt Controller (HLIC)
+---------------------------------------------
+
+RISC-V cores include Control Status Registers (CSRs) which are local to each
+CPU core (HART in RISC-V terminology) and can be read or written by software.
+Some of these CSRs are used to control local interrupts connected to the core.
+Every interrupt is ultimately routed through a hart's HLIC before it
+interrupts that hart.
+
+The RISC-V supervisor ISA manual specifies three interrupt sources that are
+attached to every HLIC: software interrupts, the timer interrupt, and external
+interrupts.  Software interrupts are used to send IPIs between cores.  The
+timer interrupt comes from an architecturally mandated real-time timer that is
+controller via Supervisor Binary Interface (SBI) calls and CSR reads.  External
+interrupts connect all other device interrupts to the HLIC, which are routed
+via the platform-level interrupt controller (PLIC).
+
+All RISC-V systems that conform to the supervisor ISA specification are
+required to have a HLIC with these three interrupt sources present.  Since the
+interrupt map is defined by the ISA it's not listed in the HLIC's device tree
+entry, though external interrupt controllers (like the PLIC, for example) will
+need to define how their interrupts map to the relevant HLICs.  This means
+a PLIC interrupt property will typically list the HLICs for all present HARTs
+in the system.
+
+Required properties:
+- compatible : "riscv,cpu-intc"
+- #interrupt-cells : should be <1>
+- interrupt-controller : Identifies the node as an interrupt controller
+
+Furthermore, this interrupt-controller MUST be embedded inside the cpu
+definition of the hart whose CSRs control these local interrupts.
+
+An example device tree entry for a HLIC is show below.
+
+	cpu1: cpu@1 {
+		compatible = "riscv";
+		...
+		cpu1-intc: interrupt-controller {
+			#interrupt-cells = <1>;
+			compatible = "riscv,cpu-intc", "sifive,fu540-c000-cpu-intc";
+			interrupt-controller;
+		};
+	};
-- 
2.18.0

[PATCH v4 2/3] dt-bindings: interrupt-controller: SiFive Plaform Level Interrupt Controller

[Christoph Hellwig]

From: Palmer Dabbelt <palmer@dabbelt.com>

Add documentation 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.

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

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic.txt b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic.txt
new file mode 100644
index 000000000000..fe7ccd944c12
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic.txt
@@ -0,0 +1,58 @@
+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,plic" 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,plic" and a string identifying the actual detailed
+  implementation in case that specific bugs need to be worked around.
+- #address-cells : should be <0> or more.
+- #interrupt-cells : should be <1> or more.
+- 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.  Each node pointed
+  to should be a riscv,cpu-intc node, which has a riscv node as parent.
+- 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,plic", "sifive,fu540-c000-plic";
+		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

[PATCH v4 3/3] irqchip: add a SiFive PLIC driver

[Christoph Hellwig]

Add 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>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
---
 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..298685e56676
--- /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 an 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_plic, "sifive,plic", plic_init);
+IRQCHIP_DECLARE(riscv_plic0, "riscv,plic0", plic_init); /* for legacy systems */
-- 
2.18.0

Re: [PATCH v4 1/3] dt-bindings: interrupt-controller: RISC-V local interrupt controller

[Rob Herring]

On Thu, Aug 09, 2018 at 09:56:00AM +0200, Christoph Hellwig wrote:
> From: Palmer Dabbelt <palmer@dabbelt.com>
> 
> Add documentation on the RISC-V local interrupt controller, which is a
> per-hart interrupt controller that manages all interrupts entering a
> RISC-V hart.  This interrupt controller is present on all RISC-V systems.
> 
> Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
> [hch: minor cleanups]
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  .../interrupt-controller/riscv,cpu-intc.txt   | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
> new file mode 100644
> index 000000000000..b0a8af51c388
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
> @@ -0,0 +1,44 @@
> +RISC-V Hart-Level Interrupt Controller (HLIC)
> +---------------------------------------------
> +
> +RISC-V cores include Control Status Registers (CSRs) which are local to each
> +CPU core (HART in RISC-V terminology) and can be read or written by software.
> +Some of these CSRs are used to control local interrupts connected to the core.
> +Every interrupt is ultimately routed through a hart's HLIC before it
> +interrupts that hart.
> +
> +The RISC-V supervisor ISA manual specifies three interrupt sources that are
> +attached to every HLIC: software interrupts, the timer interrupt, and external
> +interrupts.  Software interrupts are used to send IPIs between cores.  The
> +timer interrupt comes from an architecturally mandated real-time timer that is
> +controller via Supervisor Binary Interface (SBI) calls and CSR reads.  External

s/controller/controlled/

> +interrupts connect all other device interrupts to the HLIC, which are routed
> +via the platform-level interrupt controller (PLIC).
> +
> +All RISC-V systems that conform to the supervisor ISA specification are
> +required to have a HLIC with these three interrupt sources present.  Since the
> +interrupt map is defined by the ISA it's not listed in the HLIC's device tree
> +entry, though external interrupt controllers (like the PLIC, for example) will
> +need to define how their interrupts map to the relevant HLICs.  This means
> +a PLIC interrupt property will typically list the HLICs for all present HARTs
> +in the system.
> +
> +Required properties:
> +- compatible : "riscv,cpu-intc"
> +- #interrupt-cells : should be <1>

Given that there's only 3 sources and they are fixed(?), you should 
define the numbering of them here.

> +- interrupt-controller : Identifies the node as an interrupt controller
> +
> +Furthermore, this interrupt-controller MUST be embedded inside the cpu
> +definition of the hart whose CSRs control these local interrupts.
> +
> +An example device tree entry for a HLIC is show below.
> +
> +	cpu1: cpu@1 {
> +		compatible = "riscv";
> +		...
> +		cpu1-intc: interrupt-controller {
> +			#interrupt-cells = <1>;
> +			compatible = "riscv,cpu-intc", "sifive,fu540-c000-cpu-intc";

The order here should be reversed (most specific first) and 
"sifive,fu540-c000-cpu-intc" needs to be listed above.

> +			interrupt-controller;
> +		};
> +	};
> -- 
> 2.18.0
> 

Re: [PATCH v4 3/3] irqchip: add a SiFive PLIC driver

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 7871 bytes --]

Hi Christoph,

I love your patch! Yet something to improve:

[auto build test ERROR on tip/irq/core]
[also build test ERROR on v4.18]
[cannot apply to next-20180816]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Christoph-Hellwig/dt-bindings-interrupt-controller-RISC-V-local-interrupt-controller/20180810-014110
config: riscv-defconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 8.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.1.0 make.cross ARCH=riscv 

All errors (new ones prefixed by >>):

   In file included from arch/riscv/include/asm/ptrace.h:18,
                    from arch/riscv/include/asm/processor.h:19,
                    from arch/riscv/include/asm/irqflags.h:18,
                    from include/linux/irqflags.h:16,
                    from arch/riscv/include/asm/bitops.h:22,
                    from include/linux/bitops.h:38,
                    from include/linux/kernel.h:11,
                    from include/linux/interrupt.h:6,
                    from drivers/irqchip/irq-sifive-plic.c:7:
   drivers/irqchip/irq-sifive-plic.c: In function 'plic_handle_irq':
>> drivers/irqchip/irq-sifive-plic.c:156:17: error: 'SIE_SEIE' undeclared (first use in this function); did you mean 'SIE_STIE'?
     csr_clear(sie, SIE_SEIE);
                    ^~~~~~~~
   arch/riscv/include/asm/csr.h:124:38: note: in definition of macro 'csr_clear'
     unsigned long __v = (unsigned long)(val);  \
                                         ^~~
   drivers/irqchip/irq-sifive-plic.c:156:17: note: each undeclared identifier is reported only once for each function it appears in
     csr_clear(sie, SIE_SEIE);
                    ^~~~~~~~
   arch/riscv/include/asm/csr.h:124:38: note: in definition of macro 'csr_clear'
     unsigned long __v = (unsigned long)(val);  \
                                         ^~~

vim +156 drivers/irqchip/irq-sifive-plic.c

   > 7	#include <linux/interrupt.h>
     8	#include <linux/io.h>
     9	#include <linux/irq.h>
    10	#include <linux/irqchip.h>
    11	#include <linux/irqdomain.h>
    12	#include <linux/module.h>
    13	#include <linux/of.h>
    14	#include <linux/of_address.h>
    15	#include <linux/of_irq.h>
    16	#include <linux/platform_device.h>
    17	#include <linux/spinlock.h>
    18	
    19	/*
    20	 * This driver implements a version of the RISC-V PLIC with the actual layout
    21	 * specified in chapter 8 of the SiFive U5 Coreplex Series Manual:
    22	 *
    23	 *     https://static.dev.sifive.com/U54-MC-RVCoreIP.pdf
    24	 *
    25	 * The largest number supported by devices marked as 'riscv,plic0', is 1024, of
    26	 * which device 0 is defined as non-existent by the RISC-V Privileged Spec.
    27	 */
    28	
    29	#define MAX_DEVICES			1024
    30	#define MAX_CONTEXTS			15872
    31	
    32	/*
    33	 * Each interrupt source has a priority register associated with it.
    34	 * We always hardwire it to one in Linux.
    35	 */
    36	#define PRIORITY_BASE			0
    37	#define     PRIORITY_PER_ID		4
    38	
    39	/*
    40	 * Each hart context has a vector of interrupt enable bits associated with it.
    41	 * There's one bit for each interrupt source.
    42	 */
    43	#define ENABLE_BASE			0x2000
    44	#define     ENABLE_PER_HART		0x80
    45	
    46	/*
    47	 * Each hart context has a set of control registers associated with it.  Right
    48	 * now there's only two: a source priority threshold over which the hart will
    49	 * take an interrupt, and a register to claim interrupts.
    50	 */
    51	#define CONTEXT_BASE			0x200000
    52	#define     CONTEXT_PER_HART		0x1000
    53	#define     CONTEXT_THRESHOLD		0x00
    54	#define     CONTEXT_CLAIM		0x04
    55	
    56	static void __iomem *plic_regs;
    57	
    58	struct plic_handler {
    59		bool			present;
    60		int			ctxid;
    61	};
    62	static DEFINE_PER_CPU(struct plic_handler, plic_handlers);
    63	
    64	static inline void __iomem *plic_hart_offset(int ctxid)
    65	{
    66		return plic_regs + CONTEXT_BASE + ctxid * CONTEXT_PER_HART;
    67	}
    68	
    69	static inline u32 __iomem *plic_enable_base(int ctxid)
    70	{
    71		return plic_regs + ENABLE_BASE + ctxid * ENABLE_PER_HART;
    72	}
    73	
    74	/*
    75	 * Protect mask operations on the registers given that we can't assume that
    76	 * atomic memory operations work on them.
    77	 */
    78	static DEFINE_RAW_SPINLOCK(plic_toggle_lock);
    79	
    80	static inline void plic_toggle(int ctxid, int hwirq, int enable)
    81	{
    82		u32 __iomem *reg = plic_enable_base(ctxid) + (hwirq / 32);
    83		u32 hwirq_mask = 1 << (hwirq % 32);
    84	
    85		raw_spin_lock(&plic_toggle_lock);
    86		if (enable)
    87			writel(readl(reg) | hwirq_mask, reg);
    88		else
    89			writel(readl(reg) & ~hwirq_mask, reg);
    90		raw_spin_unlock(&plic_toggle_lock);
    91	}
    92	
    93	static inline void plic_irq_toggle(struct irq_data *d, int enable)
    94	{
    95		int cpu;
    96	
    97		writel(enable, plic_regs + PRIORITY_BASE + d->hwirq * PRIORITY_PER_ID);
    98		for_each_cpu(cpu, irq_data_get_affinity_mask(d)) {
    99			struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu);
   100	
   101			if (handler->present)
   102				plic_toggle(handler->ctxid, d->hwirq, enable);
   103		}
   104	}
   105	
   106	static void plic_irq_enable(struct irq_data *d)
   107	{
   108		plic_irq_toggle(d, 1);
   109	}
   110	
   111	static void plic_irq_disable(struct irq_data *d)
   112	{
   113		plic_irq_toggle(d, 0);
   114	}
   115	
   116	static struct irq_chip plic_chip = {
   117		.name		= "SiFive PLIC",
   118		/*
   119		 * There is no need to mask/unmask PLIC interrupts.  They are "masked"
   120		 * by reading claim and "unmasked" when writing it back.
   121		 */
   122		.irq_enable	= plic_irq_enable,
   123		.irq_disable	= plic_irq_disable,
   124	};
   125	
   126	static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq,
   127				      irq_hw_number_t hwirq)
   128	{
   129		irq_set_chip_and_handler(irq, &plic_chip, handle_simple_irq);
   130		irq_set_chip_data(irq, NULL);
   131		irq_set_noprobe(irq);
   132		return 0;
   133	}
   134	
   135	static const struct irq_domain_ops plic_irqdomain_ops = {
   136		.map		= plic_irqdomain_map,
   137		.xlate		= irq_domain_xlate_onecell,
   138	};
   139	
   140	static struct irq_domain *plic_irqdomain;
   141	
   142	/*
   143	 * Handling an interrupt is a two-step process: first you claim the interrupt
   144	 * by reading the claim register, then you complete the interrupt by writing
   145	 * that source ID back to the same claim register.  This automatically enables
   146	 * and disables the interrupt, so there's nothing else to do.
   147	 */
   148	static void plic_handle_irq(struct pt_regs *regs)
   149	{
   150		struct plic_handler *handler = this_cpu_ptr(&plic_handlers);
   151		void __iomem *claim = plic_hart_offset(handler->ctxid) + CONTEXT_CLAIM;
   152		irq_hw_number_t hwirq;
   153	
   154		WARN_ON_ONCE(!handler->present);
   155	
 > 156		csr_clear(sie, SIE_SEIE);
   157		while ((hwirq = readl(claim))) {
   158			int irq = irq_find_mapping(plic_irqdomain, hwirq);
   159	
   160			if (unlikely(irq <= 0))
   161				pr_warn_ratelimited("can't find mapping for hwirq %lu\n",
   162						hwirq);
   163			else
   164				generic_handle_irq(irq);
   165			writel(hwirq, claim);
   166		}
   167		csr_set(sie, SIE_SEIE);
   168	}
   169	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 16337 bytes --]