The RISC-V ISA mandantes the presence of a simple, per-hart (hardware
thread) interrupt controller availiable to supervisor mode. This patch
set adds a driver for this interrupt controller.
The patch set itself has been around in various flavors for a while, but
as far as I remember it's never been properly cleaned up and submitted
before. As it currently stands it's essentially three seperate patches,
but as they're all intertwined I'm keeping them together. Sorry if
that's the wrong thing to do.
The patches are:
* A cleanup to arch/riscv to remove a bit of the old initialization
mechanism that snuck in to our arch port. This patch is trivial, so
unless there's any feedback on it specificly I'll include it in my
next pull request.
* The addition of device tree bindings to describe "riscv,cpu-intc".
* The actual irqchip driver. If this gets merged before the arch/riscv
patch then it'll cause our build to fail, but I'm assuming this will
be targeted at the next merge window so we should be safe.
From: Palmer Dabbelt <[email protected]>
This patch adds a driver that manages the local interrupts on each
RISC-V hart, as specifiec by the RISC-V supervisor level ISA manual.
The local interrupt controller manages software interrupts, timer
interrupts, and hardware interrupts (which are routed via the
platform level interrupt controller). Per-hart local interrupt
controllers are found on all RISC-V systems.
CC: Michael Clark <[email protected]>
Signed-off-by: Palmer Dabbelt <[email protected]>
---
drivers/irqchip/Kconfig | 13 +++
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-riscv-intc.c | 215 +++++++++++++++++++++++++++++++++++++++
3 files changed, 229 insertions(+)
create mode 100644 drivers/irqchip/irq-riscv-intc.c
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index e9233db16e03..bf7fc86673b1 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -372,3 +372,16 @@ config QCOM_PDC
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.
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 15f268f646bf..74e333cc274c 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_RISCV_INTC) += irq-riscv-intc.o
diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
new file mode 100644
index 000000000000..7ca3060e76b4
--- /dev/null
+++ b/drivers/irqchip/irq-riscv-intc.c
@@ -0,0 +1,215 @@
+/*
+ * Copyright (C) 2012 Regents of the University of California
+ * Copyright (C) 2017-2018 SiFive
+ */
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>
+#include <linux/interrupt.h>
+#include <linux/ftrace.h>
+#include <linux/of.h>
+#include <linux/seq_file.h>
+
+#include <asm/ptrace.h>
+#include <asm/sbi.h>
+#include <asm/smp.h>
+
+#define PTR_BITS (8 * sizeof(uintptr_t))
+
+struct riscv_irq_data {
+ struct irq_chip chip;
+ struct irq_domain *domain;
+ int hart;
+ char name[20];
+};
+DEFINE_PER_CPU(struct riscv_irq_data, riscv_irq_data);
+
+static void riscv_software_interrupt(void)
+{
+#ifdef CONFIG_SMP
+ irqreturn_t ret;
+
+ ret = handle_ipi();
+
+ WARN_ON(ret == IRQ_NONE);
+#else
+ /*
+ * We currently only use software interrupts to pass inter-processor
+ * interrupts, so if a non-SMP system gets a software interrupt then we
+ * don't know what to do.
+ */
+ pr_warning("Software Interrupt without CONFIG_SMP\n");
+#endif
+}
+
+static void riscv_intc_irq(struct pt_regs *regs)
+{
+ struct pt_regs *old_regs = set_irq_regs(regs);
+ struct irq_domain *domain;
+ unsigned long cause = csr_read(scause);
+
+ /*
+ * 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 here.
+ */
+ WARN_ON((cause & (1UL << (PTR_BITS - 1))) == 0);
+ cause = cause & ~(1UL << (PTR_BITS - 1));
+
+ irq_enter();
+
+ /*
+ * There are three classes of interrupt: timer, software, and
+ * external devices. We dispatch between them here. External
+ * device interrupts use the generic IRQ mechanisms.
+ */
+ switch (cause) {
+ case INTERRUPT_CAUSE_TIMER:
+ riscv_timer_interrupt();
+ break;
+ case INTERRUPT_CAUSE_SOFTWARE:
+ riscv_software_interrupt();
+ break;
+ default:
+ domain = per_cpu(riscv_irq_data, smp_processor_id()).domain;
+ generic_handle_irq(irq_find_mapping(domain, cause));
+ break;
+ }
+
+ irq_exit();
+ set_irq_regs(old_regs);
+}
+
+static int riscv_irqdomain_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ struct riscv_irq_data *data = d->host_data;
+
+ irq_set_chip_and_handler(irq, &data->chip, handle_simple_irq);
+ irq_set_chip_data(irq, data);
+ irq_set_noprobe(irq);
+ irq_set_affinity(irq, cpumask_of(data->hart));
+
+ return 0;
+}
+
+static const struct irq_domain_ops riscv_irqdomain_ops = {
+ .map = riscv_irqdomain_map,
+ .xlate = irq_domain_xlate_onecell,
+};
+
+/*
+ * 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_irq_mask(struct irq_data *d)
+{
+ struct riscv_irq_data *data = irq_data_get_irq_chip_data(d);
+
+ BUG_ON(smp_processor_id() != data->hart);
+ csr_clear(sie, 1 << (long)d->hwirq);
+}
+
+static void riscv_irq_unmask(struct irq_data *d)
+{
+ struct riscv_irq_data *data = irq_data_get_irq_chip_data(d);
+
+ BUG_ON(smp_processor_id() != data->hart);
+ csr_set(sie, 1 << (long)d->hwirq);
+}
+
+/* Callbacks for twiddling SIE on another hart. */
+static void riscv_irq_enable_helper(void *d)
+{
+ riscv_irq_unmask(d);
+}
+
+static void riscv_irq_disable_helper(void *d)
+{
+ riscv_irq_mask(d);
+}
+
+static void riscv_remote_ctrl(unsigned int cpu, void (*fn)(void *d),
+ struct irq_data *data)
+{
+ smp_call_function_single(cpu, fn, data, true);
+}
+
+static void riscv_irq_enable(struct irq_data *d)
+{
+ struct riscv_irq_data *data = irq_data_get_irq_chip_data(d);
+
+ /*
+ * It's only possible to write SIE on the current hart. This jumps
+ * over to the target hart if it's not the current one. It's invalid
+ * to write SIE on a hart that's not currently running.
+ */
+ if (data->hart == smp_processor_id())
+ riscv_irq_unmask(d);
+ else if (cpu_online(data->hart))
+ riscv_remote_ctrl(data->hart, riscv_irq_enable_helper, d);
+ else
+ WARN_ON_ONCE(1);
+}
+
+static void riscv_irq_disable(struct irq_data *d)
+{
+ struct riscv_irq_data *data = irq_data_get_irq_chip_data(d);
+
+ /*
+ * It's only possible to write SIE on the current hart. This jumps
+ * over to the target hart if it's not the current one. It's invalid
+ * to write SIE on a hart that's not currently running.
+ */
+ if (data->hart == smp_processor_id())
+ riscv_irq_mask(d);
+ else if (cpu_online(data->hart))
+ riscv_remote_ctrl(data->hart, riscv_irq_disable_helper, d);
+ else
+ WARN_ON_ONCE(1);
+}
+
+static int __init riscv_intc_init(struct device_node *node, struct device_node *parent)
+{
+ int hart;
+ struct riscv_irq_data *data;
+
+ if (parent)
+ return 0;
+
+ hart = riscv_of_processor_hart(node->parent);
+ if (hart < 0)
+ return -EIO;
+
+ data = &per_cpu(riscv_irq_data, hart);
+ snprintf(data->name, sizeof(data->name), "riscv,cpu_intc,%d", hart);
+ data->hart = hart;
+ data->chip.name = data->name;
+ data->chip.irq_mask = riscv_irq_mask;
+ data->chip.irq_unmask = riscv_irq_unmask;
+ data->chip.irq_enable = riscv_irq_enable;
+ data->chip.irq_disable = riscv_irq_disable;
+ data->domain = irq_domain_add_linear(node, PTR_BITS,
+ &riscv_irqdomain_ops, data);
+ if (!data->domain)
+ goto error_add_linear;
+
+ set_handle_irq(&riscv_intc_irq);
+
+ pr_info("%s: %lu local interrupts mapped\n", data->name, PTR_BITS);
+ return 0;
+
+error_add_linear:
+ pr_warning("%s: unable to add IRQ domain\n",
+ data->name);
+ return -ENXIO;
+}
+
+IRQCHIP_DECLARE(riscv, "riscv,cpu-intc", riscv_intc_init);
--
2.16.4
This file has never existed in the upstream kernel, but it's guarded by
an #ifdef that's also never existed in the upstream kernel. As a part
of our interrupt controller refactoring this header is no longer
necessary, but this reference managed to sneak in anyway.
Signed-off-by: Palmer Dabbelt <[email protected]>
---
arch/riscv/kernel/irq.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
index b74cbfbce2d0..7bcdaed15703 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -16,10 +16,6 @@
#include <linux/irqchip.h>
#include <linux/irqdomain.h>
-#ifdef CONFIG_RISCV_INTC
-#include <linux/irqchip/irq-riscv-intc.h>
-#endif
-
void __init init_IRQ(void)
{
irqchip_init();
--
2.16.4
From: Palmer Dabbelt <[email protected]>
This patch adds 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 <[email protected]>
---
.../interrupt-controller/riscv,cpu-intc.txt | 41 ++++++++++++++++++++++
1 file changed, 41 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..61900e2e3868
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
@@ -0,0 +1,41 @@
+RISC-V Hart-Level Interrupt Controller (HLIC)
+---------------------------------------------
+
+RISC-V cores include Control Status Registers (CSRs) which are local to each
+hart 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 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.
+
+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";
+ interrupt-controller;
+ };
+ };
--
2.16.4
On 06/22/2018 04:20 PM, Palmer Dabbelt wrote:
> From: Palmer Dabbelt <[email protected]>
>
> This patch adds a driver that manages the local interrupts on each
> RISC-V hart, as specifiec by the RISC-V supervisor level ISA manual.
> The local interrupt controller manages software interrupts, timer
> interrupts, and hardware interrupts (which are routed via the
> platform level interrupt controller). Per-hart local interrupt
> controllers are found on all RISC-V systems.
>
> CC: Michael Clark <[email protected]>
> Signed-off-by: Palmer Dabbelt <[email protected]>
> ---
> drivers/irqchip/Kconfig | 13 +++
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-riscv-intc.c | 215 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 229 insertions(+)
> create mode 100644 drivers/irqchip/irq-riscv-intc.c
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index e9233db16e03..bf7fc86673b1 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -372,3 +372,16 @@ config QCOM_PDC
> IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
>
> endmenu
> +
> +config RISCV_INTC
> + #bool "RISC-V Interrupt Controller"
Hi,
What does the leading '#' do?
> + 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.
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 15f268f646bf..74e333cc274c 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_RISCV_INTC) += irq-riscv-intc.o
--
~Randy
On Fri, Jun 22, 2018 at 04:20:04PM -0700, Palmer Dabbelt wrote:
> This file has never existed in the upstream kernel, but it's guarded by
> an #ifdef that's also never existed in the upstream kernel. As a part
> of our interrupt controller refactoring this header is no longer
> necessary, but this reference managed to sneak in anyway.
>
> Signed-off-by: Palmer Dabbelt <[email protected]>
Looks good:
Reviewed-by: Christoph Hellwig <[email protected]>
> +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.
This can just be:
config RISCV_INTC
def_bool y if RISCV
depends on RISCV
as this isn't a user selectable option.
> index 000000000000..7ca3060e76b4
> --- /dev/null
> +++ b/drivers/irqchip/irq-riscv-intc.c
> @@ -0,0 +1,215 @@
> +/*
> + * Copyright (C) 2012 Regents of the University of California
> + * Copyright (C) 2017-2018 SiFive
> + */
> +/* SPDX-License-Identifier: GPL-2.0 */
SPDX tags need to be the first thing in the file, and use // -style
comments.
FYI, I've got a version with this and a few other cleanups here:
http://git.infradead.org/users/hch/riscv.git/commitdiff/d0789843f663ba8a6573ac5f73d8b6f999c8e6ed
On Fri, 22 Jun 2018, Palmer Dabbelt wrote:
> +struct riscv_irq_data {
> + struct irq_chip chip;
> + struct irq_domain *domain;
> + int hart;
> + char name[20];
> +};
> +DEFINE_PER_CPU(struct riscv_irq_data, riscv_irq_data);
static ?
> +static void riscv_intc_irq(struct pt_regs *regs)
> +{
> + struct pt_regs *old_regs = set_irq_regs(regs);
> + struct irq_domain *domain;
> + unsigned long cause = csr_read(scause);
> +
> + /*
> + * 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 here.
> + */
> + WARN_ON((cause & (1UL << (PTR_BITS - 1))) == 0);
So what's the point of continuing here?
> + cause = cause & ~(1UL << (PTR_BITS - 1));
Please define a proper CAUSE_MASK or such as this is really hard to read.
> +/*
> + * 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_irq_mask(struct irq_data *d)
> +{
> + struct riscv_irq_data *data = irq_data_get_irq_chip_data(d);
> +
> + BUG_ON(smp_processor_id() != data->hart);
> + csr_clear(sie, 1 << (long)d->hwirq);
What's the point of this type cast? Whether you shift by unsigned long or by
long does not really matter, right? I'd rather expected to see 1U << d->hwirq
> +static int __init riscv_intc_init(struct device_node *node, struct device_node *parent)
> +{
> + int hart;
> + struct riscv_irq_data *data;
Nit. Reverse fir tree ordering please
struct riscv_irq_data *data;
int hart;
Simpler to parse.
> +
> + if (parent)
> + return 0;
Hmm, that wants a comment because it's not clear why this is done for the
casual reader.
> +
> + hart = riscv_of_processor_hart(node->parent);
> + if (hart < 0)
> + return -EIO;
> +
> + data = &per_cpu(riscv_irq_data, hart);
> + snprintf(data->name, sizeof(data->name), "riscv,cpu_intc,%d", hart);
> + data->hart = hart;
> + data->chip.name = data->name;
> + data->chip.irq_mask = riscv_irq_mask;
> + data->chip.irq_unmask = riscv_irq_unmask;
> + data->chip.irq_enable = riscv_irq_enable;
> + data->chip.irq_disable = riscv_irq_disable;
> + data->domain = irq_domain_add_linear(node, PTR_BITS,
> + &riscv_irqdomain_ops, data);
> + if (!data->domain)
> + goto error_add_linear;
> +
> + set_handle_irq(&riscv_intc_irq);
> +
> + pr_info("%s: %lu local interrupts mapped\n", data->name, PTR_BITS);
> + return 0;
> +
> +error_add_linear:
> + pr_warning("%s: unable to add IRQ domain\n",
> + data->name);
One line please.
> + return -ENXIO;
Thanks,
tglx
On Fri, Jun 22, 2018 at 04:20:05PM -0700, Palmer Dabbelt wrote:
> From: Palmer Dabbelt <[email protected]>
>
> This patch adds 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 <[email protected]>
> ---
> .../interrupt-controller/riscv,cpu-intc.txt | 41 ++++++++++++++++++++++
> 1 file changed, 41 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..61900e2e3868
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
> @@ -0,0 +1,41 @@
> +RISC-V Hart-Level Interrupt Controller (HLIC)
> +---------------------------------------------
> +
> +RISC-V cores include Control Status Registers (CSRs) which are local to each
> +hart 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 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.
> +
> +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.
Spotted a typo here, "show" -> "shown".
> +
> + cpu1: cpu@1 {
> + compatible = "riscv";
> + ...
> + cpu1-intc: interrupt-controller {
> + #interrupt-cells = <1>;
> + compatible = "riscv,cpu-intc";
> + interrupt-controller;
> + };
> + };
> --
> 2.16.4
Also, I've noticed that double spaces after punctuation are used pretty
inconsistently throughout the document. Is that intended?
--
Regards,
Christoph
On Fri, Jun 22, 2018 at 04:20:05PM -0700, Palmer Dabbelt wrote:
> From: Palmer Dabbelt <[email protected]>
>
> This patch adds 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 <[email protected]>
> ---
> .../interrupt-controller/riscv,cpu-intc.txt | 41 ++++++++++++++++++++++
> 1 file changed, 41 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..61900e2e3868
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
> @@ -0,0 +1,41 @@
> +RISC-V Hart-Level Interrupt Controller (HLIC)
> +---------------------------------------------
> +
> +RISC-V cores include Control Status Registers (CSRs) which are local to each
> +hart 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 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.
What are the PLIC to HLIC connections you need to support? 1-to-1 is
easy. But I would imagine you'd want the PLIC irq to go to all the HLICs
which can't really be described unless you list every CPU's HLIC in
PLIC "interrupts" property.
We avoid this problem on ARM because a single DT interrupt controller
node represents even per cpu interrupts and interrupt cells are used to
indicate which CPU interrupts are routed too. That's not perfect either,
but seems to be good enough (though maybe Marc Z or Mark R have more
thoughts on that).
> +
> +Required properties:
> +- compatible : "riscv,cpu-intc"
Kind of vague. There's only one implementation and one set of bugs?
> +- #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";
> + interrupt-controller;
> + };
> + };
> --
> 2.16.4
>
On Fri, 22 Jun 2018 17:08:58 PDT (-0700), [email protected] wrote:
> On 06/22/2018 04:20 PM, Palmer Dabbelt wrote:
>> From: Palmer Dabbelt <[email protected]>
>>
>> This patch adds a driver that manages the local interrupts on each
>> RISC-V hart, as specifiec by the RISC-V supervisor level ISA manual.
>> The local interrupt controller manages software interrupts, timer
>> interrupts, and hardware interrupts (which are routed via the
>> platform level interrupt controller). Per-hart local interrupt
>> controllers are found on all RISC-V systems.
>>
>> CC: Michael Clark <[email protected]>
>> Signed-off-by: Palmer Dabbelt <[email protected]>
>> ---
>> drivers/irqchip/Kconfig | 13 +++
>> drivers/irqchip/Makefile | 1 +
>> drivers/irqchip/irq-riscv-intc.c | 215 +++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 229 insertions(+)
>> create mode 100644 drivers/irqchip/irq-riscv-intc.c
>>
>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>> index e9233db16e03..bf7fc86673b1 100644
>> --- a/drivers/irqchip/Kconfig
>> +++ b/drivers/irqchip/Kconfig
>> @@ -372,3 +372,16 @@ config QCOM_PDC
>> IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
>>
>> endmenu
>> +
>> +config RISCV_INTC
>> + #bool "RISC-V Interrupt Controller"
>
> Hi,
> What does the leading '#' do?
It's just a comment. I'd seen this idiom used before to say "here's what this
option would say if it was optional, which it may be at some point in the
future, but for now it's just always enabled."
>> + 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.
>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>> index 15f268f646bf..74e333cc274c 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_RISCV_INTC) += irq-riscv-intc.o
On Mon, 25 Jun 2018 13:04:48 PDT (-0700), [email protected] wrote:
> On Fri, Jun 22, 2018 at 04:20:05PM -0700, Palmer Dabbelt wrote:
>> From: Palmer Dabbelt <[email protected]>
>>
>> This patch adds 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 <[email protected]>
>> ---
>> .../interrupt-controller/riscv,cpu-intc.txt | 41 ++++++++++++++++++++++
>> 1 file changed, 41 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..61900e2e3868
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,cpu-intc.txt
>> @@ -0,0 +1,41 @@
>> +RISC-V Hart-Level Interrupt Controller (HLIC)
>> +---------------------------------------------
>> +
>> +RISC-V cores include Control Status Registers (CSRs) which are local to each
>> +hart 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 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.
>> +
>> +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.
>
> Spotted a typo here, "show" -> "shown".
Thanks. It looks like we're actually dropping this binding and integrating
this first-level interrupt controller into the core RISC-V arch code as keeping
it split out results in too many inefficiencies.
>> +
>> + cpu1: cpu@1 {
>> + compatible = "riscv";
>> + ...
>> + cpu1-intc: interrupt-controller {
>> + #interrupt-cells = <1>;
>> + compatible = "riscv,cpu-intc";
>> + interrupt-controller;
>> + };
>> + };
>> --
>> 2.16.4
>
> Also, I've noticed that double spaces after punctuation are used pretty
> inconsistently throughout the document. Is that intended?
No. I noticed this in the PLIC document as well, I'll fix that one up.
Thanks!