Linux RISC-V AIA Support

We add a common riscv_get_intc_hartid() which help device drivers to get hartid of the HART associated with a INTC (i.e. local interrupt controller) fwnode. This new function is more generic compared to the existing riscv_of_parent_hartid() function hence we also replace use of riscv_of_parent_hartid() with riscv_get_intc_hartid().

Also, while we are here let us update riscv_of_parent_hartid() to always return the hartid irrespective whether the CPU/HART DT node is disabled or not.

Signed-off-by Anup Patel <[email protected]>

---
 arch/riscv/include/asm/processor.h |  4 +++-
 arch/riscv/kernel/cpu.c            | 26 ++++++++++++++++++++------
 drivers/irqchip/irq-riscv-intc.c   |  2 +-
 drivers/irqchip/irq-sifive-plic.c  |  3 ++-
 4 files changed, 26 insertions(+), 9 deletions(-)

arch/riscv/include/asm/processor.h +0 -0

index c950a8d9edef..662da1e112dd 100644

arch/riscv/include/asm/processor.h +0 -0

 extern void riscv_fill_hwcap(void);
 extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);

arch/riscv/kernel/cpu.c +1 -1

index a2fc952318e9..c3eaa8a55bbe 100644
@@ -81,21 +81,35 @@ int riscv_early_of_processor_hartid(struct device_node *node, unsigned long *har
  * To achieve this, we walk up the DT tree until we find an active
  * RISC-V core (HART) node and extract the cpuid from it.
  */
-int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid)
+static int riscv_of_parent_hartid(struct device_node *node,

arch/riscv/kernel/cpu.c +1 -0

+			return 0;
 		}
 	}
 
 	return -1;

arch/riscv/kernel/cpu.c +8 -0

+		if (!rc)
+			*hartid = temp;
+	} else
+		rc = riscv_of_parent_hartid(to_of_node(node), hartid);
+
+	return rc;
+}
+
 DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
 
 unsigned long riscv_cached_mvendorid(unsigned int cpu_id)

drivers/irqchip/irq-riscv-intc.c +1 -1

index 4adeee1bc391..65f4a2afb381 100644
@@ -143,7 +143,7 @@ static int __init riscv_intc_init(struct device_node *node,
 	int rc;
 	unsigned long hartid;
 
-	rc = riscv_of_parent_hartid(node, &hartid);
+	rc = riscv_get_intc_hartid(of_fwnode_handle(node), &hartid);
 	if (rc < 0) {
 		pr_warn("unable to find hart id for %pOF\n", node);
 		return 0;

drivers/irqchip/irq-sifive-plic.c +2 -1

index e1484905b7bd..56b0544b1f27 100644
@@ -477,7 +477,8 @@ static int __init __plic_init(struct device_node *node,
 			continue;
 		}
 
-		error = riscv_of_parent_hartid(parent.np, &hartid);
+		error = riscv_get_intc_hartid(of_fwnode_handle(parent.np),
+					      &hartid);
 		if (error < 0) {
 			pr_warn("failed to parse hart ID for context %d.\n", i);
 			continue;

Signature · 1 line

2.34.1

Quoted 6 lines · Anup Patel

We add a common riscv_get_intc_hartid() which help device drivers to get hartid of the HART associated with a INTC (i.e. local interrupt controller) fwnode. This new function is more generic compared to the existing riscv_of_parent_hartid() function hence we also replace use of riscv_of_parent_hartid() with riscv_get_intc_hartid().

Quoted 3 lines

Also, while we are here let us update riscv_of_parent_hartid() to always return the hartid irrespective whether the CPU/HART DT node is disabled or not.

This change should probably be a separate patch with its own justification in its commit message.

arch/riscv/kernel/cpu.c int riscv_early_of_processor_hartid(struct device_node *node, unsigned long *har · line 81

Quoted 6 lines

arch/riscv/kernel/cpu.c +0 -0

index a2fc952318e9..c3eaa8a55bbe 100644
@@ -81,21 +81,35 @@ int riscv_early_of_processor_hartid(struct device_node *node, unsigned long *har
  * To achieve this, we walk up the DT tree until we find an active

arch/riscv/kernel/cpu.c int riscv_early_of_processor_hartid(struct device_node *node, unsigned long *har · line 84

Quoted 4 lines

arch/riscv/kernel/cpu.c +1 -1

  * RISC-V core (HART) node and extract the cpuid from it.
  */
-int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid)
+static int riscv_of_parent_hartid(struct device_node *node,

Quoted 6 lines · Anup Patel

We add a common riscv_get_intc_hartid() which help device drivers to get hartid of the HART associated with a INTC (i.e. local interrupt controller) fwnode. This new function is more generic compared to the existing riscv_of_parent_hartid() function hence we also replace use of riscv_of_parent_hartid() with riscv_get_intc_hartid().

Quoted 3 lines

Also, while we are here let us update riscv_of_parent_hartid() to always return the hartid irrespective whether the CPU/HART DT node is disabled or not.

This change should probably be a separate patch with its own justification in its commit message.

Quoted 10 lines

Signed-off-by Anup Patel <[email protected]>

---
 arch/riscv/include/asm/processor.h |  4 +++-
 arch/riscv/kernel/cpu.c            | 26 ++++++++++++++++++++------
 drivers/irqchip/irq-riscv-intc.c   |  2 +-
 drivers/irqchip/irq-sifive-plic.c  |  3 ++-
 4 files changed, 26 insertions(+), 9 deletions(-)

arch/riscv/include/asm/processor.h static inline void wait_for_interrupt(void) · line 84

Quoted 12 lines

arch/riscv/include/asm/processor.h +3 -1

index c950a8d9edef..662da1e112dd 100644
@@ -79,7 +79,9 @@ static inline void wait_for_interrupt(void)
 struct device_node;
 int riscv_of_processor_hartid(struct device_node *node, unsigned long *hartid);
 int riscv_early_of_processor_hartid(struct device_node *node, unsigned long *hartid);
-int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid);
+
+struct fwnode_handle;
+int riscv_get_intc_hartid(struct fwnode_handle *node, unsigned long *hartid);

Do we want a function that is named in a way that appears to be intc-specific in processor.h?

Quoted 14 lines

arch/riscv/include/asm/processor.h +0 -0

 extern void riscv_fill_hwcap(void);
 extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);

arch/riscv/kernel/cpu.c +1 -1

index a2fc952318e9..c3eaa8a55bbe 100644
@@ -81,21 +81,35 @@ int riscv_early_of_processor_hartid(struct device_node *node, unsigned long *har
  * To achieve this, we walk up the DT tree until we find an active
  * RISC-V core (HART) node and extract the cpuid from it.
  */
-int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid)
+static int riscv_of_parent_hartid(struct device_node *node,

arch/riscv/kernel/cpu.c int riscv_early_of_processor_hartid(struct device_node *node, unsigned long *har · line 89

Quoted 10 lines

arch/riscv/kernel/cpu.c +2 -5

+                               unsigned long *hartid)
 {
-     int rc;
-
      for (; node; node = node->parent) {
              if (of_device_is_compatible(node, "riscv")) {
-                     rc = riscv_of_processor_hartid(node, hartid);
-                     if (!rc)
-                             return 0;
+                     *hartid = (unsigned long)of_get_cpu_hwid(node, 0);

Shouldn't we still do something like

   if (*hartid == ~0UL) {
       pr_warn_once("Found CPU without hart ID\n");
       return -ENODEV;
   }

Quoted 8 lines

arch/riscv/kernel/cpu.c +1 -0

+                     return 0;
              }
      }
 
      return -1;
 }
 

arch/riscv/kernel/cpu.c int riscv_early_of_processor_hartid(struct device_node *node, unsigned long *har · line 104

Quoted 8 lines

arch/riscv/kernel/cpu.c +8 -0

+/* Find hart ID of the INTC fwnode. */
+int riscv_get_intc_hartid(struct fwnode_handle *node, unsigned long *hartid)
+{
+     int rc;
+     u64 temp;
+
+     if (!is_of_node(node)) {
+             rc = fwnode_property_read_u64_array(node, "hartid", &temp, 1);

This fwnode property read call seems premature, since we don't have any way to know that "hartid" will be a property of the intc since it's not a property documented in the DT binding. (I know Sunil has a series in progress which will introduce "hartid" for ACPI, but, even then, it seems like we need some documentation to point at that says '"hartid" is the name to use'.

Quoted 33 lines

arch/riscv/kernel/cpu.c +8 -0

+             if (!rc)
+                     *hartid = temp;
+     } else
+             rc = riscv_of_parent_hartid(to_of_node(node), hartid);
+
+     return rc;
+}
+
 DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
 
 unsigned long riscv_cached_mvendorid(unsigned int cpu_id)

drivers/irqchip/irq-riscv-intc.c +1 -1

index 4adeee1bc391..65f4a2afb381 100644
@@ -143,7 +143,7 @@ static int __init riscv_intc_init(struct device_node *node,
      int rc;
      unsigned long hartid;
 
-     rc = riscv_of_parent_hartid(node, &hartid);
+     rc = riscv_get_intc_hartid(of_fwnode_handle(node), &hartid);
      if (rc < 0) {
              pr_warn("unable to find hart id for %pOF\n", node);
              return 0;

drivers/irqchip/irq-sifive-plic.c +0 -0

index e1484905b7bd..56b0544b1f27 100644
@@ -477,7 +477,8 @@ static int __init __plic_init(struct device_node *node,
                      continue;
              }
 

Quoted 9 lines

drivers/irqchip/irq-sifive-plic.c +2 -2

-             error = riscv_of_parent_hartid(parent.np, &hartid);
+             error = riscv_get_intc_hartid(of_fwnode_handle(parent.np),
+                                           &hartid);
              if (error < 0) {
                      pr_warn("failed to parse hart ID for context %d.\n", i);
                      continue;
--

2.34.1

Thanks,
drew

arch/riscv/kernel/cpu.c +4 -0

+
+	return rc;
+}
+
 DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);

arch/riscv/kernel/cpu.c int riscv_early_of_processor_hartid(struct device_node *node, unsigned long *har · line 108

Quoted 12 lines · Anup Patel

arch/riscv/kernel/cpu.c +12 -0

+/* Find hart ID of the INTC fwnode. */
+int riscv_get_intc_hartid(struct fwnode_handle *node, unsigned long *hartid)
+{
+     int rc;
+     u64 temp;
+
+     if (!is_of_node(node)) {
+             rc = fwnode_property_read_u64_array(node, "hartid", &temp, 1);
+             if (!rc)
+                     *hartid = temp;
+     } else
+             rc = riscv_of_parent_hartid(to_of_node(node), hartid);

This branch needs to be enclosed in braces too.

arch/riscv/kernel/cpu.c int riscv_early_of_processor_hartid(struct device_node *node, unsigned long *har · line 113

Quoted 6 lines

arch/riscv/kernel/cpu.c +4 -0

+
+     return rc;
+}
+
 DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);

This allows fw_devlink to create device links between consumers of a MSI and the supplier of the MSI.

Signed-off-by Anup Patel <[email protected]>

---
 drivers/of/property.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

drivers/of/property.c +5 -0

index ddc75cd50825..bc20535deed7 100644
@@ -1325,6 +1325,37 @@ static struct device_node *parse_interrupts(struct device_node *np,
 	return of_irq_parse_one(np, index, &sup_args) ? NULL : sup_args.np;
 }
 
+static struct device_node *parse_msi_parent(struct device_node *np,
+					    const char *prop_name, int index)
+{
+	struct of_phandle_args sup_args;
+	struct device_node *msi_np;

drivers/of/property.c +17 -0

+			if (index) {
+				of_node_put(msi_np);
+				return NULL;
+			}
+			return msi_np;
+		}
+		of_node_put(msi_np);
+	}
+
+	if (of_parse_phandle_with_args(np, prop_name, "#msi-cells", index,
+				       &sup_args))
+		return NULL;
+
+	return sup_args.np;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
 	{ .parse_prop = parse_clocks, },
 	{ .parse_prop = parse_interconnects, },

@@ -1359,6 +1390,7 @@ static const struct supplier_bindings of_supplier_bindings[] = {
 	{ .parse_prop = parse_regulators, },
 	{ .parse_prop = parse_gpio, },
 	{ .parse_prop = parse_gpios, },
+	{ .parse_prop = parse_msi_parent, },
 	{}
 };
 

Signature · 1 line

2.34.1

Quoted 21 lines · Anup Patel

This allows fw_devlink to create device links between consumers of a MSI and the supplier of the MSI.

Signed-off-by Anup Patel <[email protected]>

---
 drivers/of/property.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

drivers/of/property.c +5 -0

index ddc75cd50825..bc20535deed7 100644
@@ -1325,6 +1325,37 @@ static struct device_node *parse_interrupts(struct device_node *np,
      return of_irq_parse_one(np, index, &sup_args) ? NULL : sup_args.np;
 }
 
+static struct device_node *parse_msi_parent(struct device_node *np,
+                                         const char *prop_name, int index)
+{
+     struct of_phandle_args sup_args;
+     struct device_node *msi_np;

drivers/of/property.c static struct device_node *parse_interrupts(struct device_node *np, · line 1342

Quoted 10 lines

drivers/of/property.c +10 -0

+
+     if (IS_ENABLED(CONFIG_SPARC))
+             return NULL;
+
+     if (strcmp(prop_name, "msi-parent"))
+             return NULL;
+
+     msi_np = of_parse_phandle(np, prop_name, 0);
+     if (msi_np) {
+             if (!of_property_read_bool(msi_np, "#msi-cells")) {

Use of_property_present() to check presence.

However, this check is wrong. #msi-cells is optional and assumed to be 0 if not present. There's another flavor of of_parse_phandle_with_args() that allows specifying a default cell count, so I think you can get rid of all this checking.

Quoted 31 lines

drivers/of/property.c +17 -1

+                     if (index) {
+                             of_node_put(msi_np);
+                             return NULL;
+                     }
+                     return msi_np;
+             }
+             of_node_put(msi_np);
+     }
+
+     if (of_parse_phandle_with_args(np, prop_name, "#msi-cells", index,
+                                    &sup_args))
+             return NULL;
+
+     return sup_args.np;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
      { .parse_prop = parse_clocks, },
      { .parse_prop = parse_interconnects, },

@@ -1359,6 +1390,7 @@ static const struct supplier_bindings of_supplier_bindings[] = {
      { .parse_prop = parse_regulators, },
      { .parse_prop = parse_gpio, },
      { .parse_prop = parse_gpios, },
+     { .parse_prop = parse_msi_parent, },
      {}
 };
 
--

2.34.1

Quoted 21 lines · Anup Patel

This allows fw_devlink to create device links between consumers of a MSI and the supplier of the MSI.

Signed-off-by Anup Patel <[email protected]>

---
 drivers/of/property.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

drivers/of/property.c +5 -0

index ddc75cd50825..bc20535deed7 100644
@@ -1325,6 +1325,37 @@ static struct device_node *parse_interrupts(struct device_node *np,
      return of_irq_parse_one(np, index, &sup_args) ? NULL : sup_args.np;
 }
 
+static struct device_node *parse_msi_parent(struct device_node *np,
+                                         const char *prop_name, int index)
+{
+     struct of_phandle_args sup_args;
+     struct device_node *msi_np;

drivers/of/property.c static struct device_node *parse_interrupts(struct device_node *np, · line 1342

Quoted 10 lines

drivers/of/property.c +10 -0

+
+     if (IS_ENABLED(CONFIG_SPARC))
+             return NULL;
+
+     if (strcmp(prop_name, "msi-parent"))
+             return NULL;
+
+     msi_np = of_parse_phandle(np, prop_name, 0);
+     if (msi_np) {
+             if (!of_property_read_bool(msi_np, "#msi-cells")) {

Use of_property_present() to check presence.

However, this check is wrong. #msi-cells is optional and assumed to be 0 if not present. There's another flavor of of_parse_phandle_with_args() that allows specifying a default cell count, so I think you can get rid of all this checking.

Quoted 31 lines

drivers/of/property.c +17 -1

+                     if (index) {
+                             of_node_put(msi_np);
+                             return NULL;
+                     }
+                     return msi_np;
+             }
+             of_node_put(msi_np);
+     }
+
+     if (of_parse_phandle_with_args(np, prop_name, "#msi-cells", index,
+                                    &sup_args))
+             return NULL;
+
+     return sup_args.np;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
      { .parse_prop = parse_clocks, },
      { .parse_prop = parse_interconnects, },

@@ -1359,6 +1390,7 @@ static const struct supplier_bindings of_supplier_bindings[] = {
      { .parse_prop = parse_regulators, },
      { .parse_prop = parse_gpio, },
      { .parse_prop = parse_gpios, },
+     { .parse_prop = parse_msi_parent, },
      {}
 };
 
--

2.34.1

Quoted 157 lines · Anup Patel

We add DT bindings document for RISC-V advanced platform level interrupt controller (APLIC) defined by the RISC-V advanced interrupt architecture (AIA) specification.

Signed-off-by Anup Patel <[email protected]>

Reviewed-by Conor Dooley <[email protected]>

---
 .../interrupt-controller/riscv,aplic.yaml     | 172 ++++++++++++++++++
 1 file changed, 172 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml

Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +139 -0

new file mode 100644
index 000000000000..190a6499c932
@@ -0,0 +1,172 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V Advanced Platform Level Interrupt Controller (APLIC)
+
+maintainers:
+  - Anup Patel <[email protected]>
+
+description:
+  The RISC-V advanced interrupt architecture (AIA) defines an advanced
+  platform level interrupt controller (APLIC) for handling wired interrupts
+  in a RISC-V platform. The RISC-V AIA specification can be found at
+  https://github.com/riscv/riscv-aia.
+
+  The RISC-V APLIC is implemented as hierarchical APLIC domains where all
+  interrupt sources connect to the root APLIC domain and a parent APLIC
+  domain can delegate interrupt sources to it's child APLIC domains. There
+  is one device tree node for each APLIC domain.
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qemu,aplic
+      - const: riscv,aplic
+
+  reg:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+  interrupts-extended:
+    minItems: 1
+    maxItems: 16384
+    description:
+      Given APLIC domain directly injects external interrupts to a set of
+      RISC-V HARTS (or CPUs). Each node pointed to should be a riscv,cpu-intc
+      node, which has a CPU node (i.e. RISC-V HART) as parent.
+
+  msi-parent:
+    description:
+      Given APLIC domain forwards wired interrupts as MSIs to a AIA incoming
+      message signaled interrupt controller (IMSIC). If both "msi-parent" and
+      "interrupts-extended" properties are present then it means the APLIC
+      domain supports both MSI mode and Direct mode in HW. In this case, the
+      APLIC driver has to choose between MSI mode or Direct mode.
+
+  riscv,num-sources:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 1023
+    description:
+      Specifies the number of wired interrupt sources supported by this
+      APLIC domain.
+
+  riscv,children:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    minItems: 1
+    maxItems: 1024
+    items:
+      maxItems: 1
+    description:
+      A list of child APLIC domains for the given APLIC domain. Each child
+      APLIC domain is assigned a child index in increasing order, with the
+      first child APLIC domain assigned child index 0. The APLIC domain child
+      index is used by firmware to delegate interrupts from the given APLIC
+      domain to a particular child APLIC domain.
+
+  riscv,delegation:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    minItems: 1
+    maxItems: 1024
+    items:
+      items:
+        - description: child APLIC domain phandle
+        - description: first interrupt number of the parent APLIC domain (inclusive)
+        - description: last interrupt number of the parent APLIC domain (inclusive)
+    description:
+      A interrupt delegation list where each entry is a triple consisting
+      of child APLIC domain phandle, first interrupt number of the parent
+      APLIC domain, and last interrupt number of the parent APLIC domain.
+      Firmware must configure interrupt delegation registers based on
+      interrupt delegation list.
+
+dependencies:
+  riscv,delegation: [ "riscv,children" ]
+
+required:
+  - compatible
+  - reg
+  - interrupt-controller
+  - "#interrupt-cells"
+  - riscv,num-sources
+
+anyOf:
+  - required:
+      - interrupts-extended
+  - required:
+      - msi-parent
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    // Example 1 (APLIC domains directly injecting interrupt to HARTs):
+
+    interrupt-controller@c000000 {
+      compatible = "qemu,aplic", "riscv,aplic";
+      interrupts-extended = <&cpu1_intc 11>,
+                            <&cpu2_intc 11>,
+                            <&cpu3_intc 11>,
+                            <&cpu4_intc 11>;
+      reg = <0xc000000 0x4080>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      riscv,num-sources = <63>;
+      riscv,children = <&aplic1>, <&aplic2>;
+      riscv,delegation = <&aplic1 1 63>;
+    };
+
+    aplic1: interrupt-controller@d000000 {
+      compatible = "qemu,aplic", "riscv,aplic";
+      interrupts-extended = <&cpu1_intc 9>,
+                            <&cpu2_intc 9>;
+      reg = <0xd000000 0x4080>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      riscv,num-sources = <63>;
+    };
+

Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml line 149

Quoted 10 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +10 -0

+    aplic2: interrupt-controller@e000000 {
+      compatible = "qemu,aplic", "riscv,aplic";
+      interrupts-extended = <&cpu3_intc 9>,
+                            <&cpu4_intc 9>;
+      reg = <0xe000000 0x4080>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      riscv,num-sources = <63>;
+    };
+

Hi Anup,
I have some thoughts regarding the APLIC DTS node. While I understand that it might be a bit late to discuss this matter within the v7 patch (sorry for this), I hope you could still consider the following idea.

For example 1, my understanding is that each APLIC DTS node represents an interrupt domain. IIUC, in physical, these tree Interrupt domains should belong to an APLIC device so the M-mode IRQ domain can delegate interrupts to the child domain. Given this perspective, wrapping all interrupt domain DTS nodes with another DTS node seems to present the real scene more clearly. Maybe we can add "simple-bus" to the compatible property of this wrapped DTS node, so it still can be compatible with your driver implementations. Therefore, example 1 may become the following.

interrupt-controller {
        compatible = "riscv,aplics", "simple-bus";
        ranges;
        aplic0: interrupt-domain@c000000 {
                compatible = "qemu,aplic", "riscv,aplic";
                interrupts-extended = <&cpu1_intc 11>,
                                      <&cpu2_intc 11>,
                                      <&cpu3_intc 11>,
                                      <&cpu4_intc 11>;
                reg = <0xc000000 0x4080>;
                interrupt-controller;
                #interrupt-cells = <2>;
                riscv,num-sources = <63>;
                riscv,children = <&aplic1>, <&aplic2>;
                riscv,delegation = <&aplic1 1 63>;
        };

        aplic1: interrupt-domain@d000000 {
                compatible = "qemu,aplic", "riscv,aplic";
                interrupts-extended = <&cpu1_intc 9>,
                                      <&cpu2_intc 9>;
                reg = <0xd000000 0x4080>;
                interrupt-controller;
                #interrupt-cells = <2>;
                riscv,num-sources = <63>;
        };

        aplic2: interrupt-domain@e000000 {
                compatible = "qemu,aplic", "riscv,aplic";
                interrupts-extended = <&cpu3_intc 9>,
                                      <&cpu4_intc 9>;
                reg = <0xe000000 0x4080>;
                interrupt-controller;
                #interrupt-cells = <2>;
                riscv,num-sources = <63>;
        };
};

Is it feasible for you?

Thanks,
Vincent

Quoted 32 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +23 -1

+  - |
+    // Example 2 (APLIC domains forwarding interrupts as MSIs):
+
+    interrupt-controller@c000000 {
+      compatible = "qemu,aplic", "riscv,aplic";
+      msi-parent = <&imsic_mlevel>;
+      reg = <0xc000000 0x4000>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      riscv,num-sources = <63>;
+      riscv,children = <&aplic3>;
+      riscv,delegation = <&aplic3 1 63>;
+    };
+
+    aplic3: interrupt-controller@d000000 {
+      compatible = "qemu,aplic", "riscv,aplic";
+      msi-parent = <&imsic_slevel>;
+      reg = <0xd000000 0x4000>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      riscv,num-sources = <63>;
+    };
+...
--

2.34.1

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Quoted 224 lines · Anup Patel

Quoted 215 lines · Vincent Chen

Quoted 157 lines · Anup Patel

We add DT bindings document for RISC-V advanced platform level interrupt controller (APLIC) defined by the RISC-V advanced interrupt architecture (AIA) specification.

Signed-off-by Anup Patel <[email protected]>

Reviewed-by Conor Dooley <[email protected]>

---
 .../interrupt-controller/riscv,aplic.yaml     | 172 ++++++++++++++++++
 1 file changed, 172 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml

Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +139 -0

new file mode 100644
index 000000000000..190a6499c932
@@ -0,0 +1,172 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V Advanced Platform Level Interrupt Controller (APLIC)
+
+maintainers:
+  - Anup Patel <[email protected]>
+
+description:
+  The RISC-V advanced interrupt architecture (AIA) defines an advanced
+  platform level interrupt controller (APLIC) for handling wired interrupts
+  in a RISC-V platform. The RISC-V AIA specification can be found at
+  https://github.com/riscv/riscv-aia.
+
+  The RISC-V APLIC is implemented as hierarchical APLIC domains where all
+  interrupt sources connect to the root APLIC domain and a parent APLIC
+  domain can delegate interrupt sources to it's child APLIC domains. There
+  is one device tree node for each APLIC domain.
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qemu,aplic
+      - const: riscv,aplic
+
+  reg:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+  interrupts-extended:
+    minItems: 1
+    maxItems: 16384
+    description:
+      Given APLIC domain directly injects external interrupts to a set of
+      RISC-V HARTS (or CPUs). Each node pointed to should be a riscv,cpu-intc
+      node, which has a CPU node (i.e. RISC-V HART) as parent.
+
+  msi-parent:
+    description:
+      Given APLIC domain forwards wired interrupts as MSIs to a AIA incoming
+      message signaled interrupt controller (IMSIC). If both "msi-parent" and
+      "interrupts-extended" properties are present then it means the APLIC
+      domain supports both MSI mode and Direct mode in HW. In this case, the
+      APLIC driver has to choose between MSI mode or Direct mode.
+
+  riscv,num-sources:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 1023
+    description:
+      Specifies the number of wired interrupt sources supported by this
+      APLIC domain.
+
+  riscv,children:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    minItems: 1
+    maxItems: 1024
+    items:
+      maxItems: 1
+    description:
+      A list of child APLIC domains for the given APLIC domain. Each child
+      APLIC domain is assigned a child index in increasing order, with the
+      first child APLIC domain assigned child index 0. The APLIC domain child
+      index is used by firmware to delegate interrupts from the given APLIC
+      domain to a particular child APLIC domain.
+
+  riscv,delegation:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    minItems: 1
+    maxItems: 1024
+    items:
+      items:
+        - description: child APLIC domain phandle
+        - description: first interrupt number of the parent APLIC domain (inclusive)
+        - description: last interrupt number of the parent APLIC domain (inclusive)
+    description:
+      A interrupt delegation list where each entry is a triple consisting
+      of child APLIC domain phandle, first interrupt number of the parent
+      APLIC domain, and last interrupt number of the parent APLIC domain.
+      Firmware must configure interrupt delegation registers based on
+      interrupt delegation list.
+
+dependencies:
+  riscv,delegation: [ "riscv,children" ]
+
+required:
+  - compatible
+  - reg
+  - interrupt-controller
+  - "#interrupt-cells"
+  - riscv,num-sources
+
+anyOf:
+  - required:
+      - interrupts-extended
+  - required:
+      - msi-parent
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    // Example 1 (APLIC domains directly injecting interrupt to HARTs):
+
+    interrupt-controller@c000000 {
+      compatible = "qemu,aplic", "riscv,aplic";
+      interrupts-extended = <&cpu1_intc 11>,
+                            <&cpu2_intc 11>,
+                            <&cpu3_intc 11>,
+                            <&cpu4_intc 11>;
+      reg = <0xc000000 0x4080>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      riscv,num-sources = <63>;
+      riscv,children = <&aplic1>, <&aplic2>;
+      riscv,delegation = <&aplic1 1 63>;
+    };
+
+    aplic1: interrupt-controller@d000000 {
+      compatible = "qemu,aplic", "riscv,aplic";
+      interrupts-extended = <&cpu1_intc 9>,
+                            <&cpu2_intc 9>;
+      reg = <0xd000000 0x4080>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      riscv,num-sources = <63>;
+    };
+

Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml line 149

Quoted 10 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +10 -0

+    aplic2: interrupt-controller@e000000 {
+      compatible = "qemu,aplic", "riscv,aplic";
+      interrupts-extended = <&cpu3_intc 9>,
+                            <&cpu4_intc 9>;
+      reg = <0xe000000 0x4080>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      riscv,num-sources = <63>;
+    };
+

Hi Anup,
I have some thoughts regarding the APLIC DTS node. While I understand that it might be a bit late to discuss this matter within the v7 patch (sorry for this), I hope you could still consider the following idea.

For example 1, my understanding is that each APLIC DTS node represents an interrupt domain. IIUC, in physical, these tree Interrupt domains should belong to an APLIC device so the M-mode IRQ domain can delegate interrupts to the child domain. Given this perspective, wrapping all interrupt domain DTS nodes with another DTS node seems to present the real scene more clearly. Maybe we can add "simple-bus" to the compatible property of this wrapped DTS node, so it still can be compatible with your driver implementations. Therefore, example 1 may become the following.

interrupt-controller {
        compatible = "riscv,aplics", "simple-bus";
        ranges;
        aplic0: interrupt-domain@c000000 {
                compatible = "qemu,aplic", "riscv,aplic";
                interrupts-extended = <&cpu1_intc 11>,
                                      <&cpu2_intc 11>,
                                      <&cpu3_intc 11>,
                                      <&cpu4_intc 11>;
                reg = <0xc000000 0x4080>;
                interrupt-controller;
                #interrupt-cells = <2>;
                riscv,num-sources = <63>;
                riscv,children = <&aplic1>, <&aplic2>;
                riscv,delegation = <&aplic1 1 63>;
        };

        aplic1: interrupt-domain@d000000 {
                compatible = "qemu,aplic", "riscv,aplic";
                interrupts-extended = <&cpu1_intc 9>,
                                      <&cpu2_intc 9>;
                reg = <0xd000000 0x4080>;
                interrupt-controller;
                #interrupt-cells = <2>;
                riscv,num-sources = <63>;
        };

        aplic2: interrupt-domain@e000000 {
                compatible = "qemu,aplic", "riscv,aplic";
                interrupts-extended = <&cpu3_intc 9>,
                                      <&cpu4_intc 9>;
                reg = <0xe000000 0x4080>;
                interrupt-controller;
                #interrupt-cells = <2>;
                riscv,num-sources = <63>;
        };
};

Quoted 1 line

Is it feasible for you?

This clubbing of APLIC domains and placing them close to each other is a platform implementation choice. On multi-die or multi-socket systems the APIC domains can be really far apart on different physical dies so the clubbing you suggest is not always true.

Quoted 3 lines

Regards,
Anup

IIUC, I think my suggestion might be able to apply to this scenario if these interrupt domains belong to the same APLIC device. For this scenario, one thing I am concerned about is that the MMIO region of other devices locates between interrupt domains. However, currently, I have not found that the DTS specification explicitly prohibits it. Do you have the same concern?

Thanks,
Vincent

Quoted 37 lines

Quoted 37 lines

Thanks,
Vincent

Quoted 32 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +23 -1

+  - |
+    // Example 2 (APLIC domains forwarding interrupts as MSIs):
+
+    interrupt-controller@c000000 {
+      compatible = "qemu,aplic", "riscv,aplic";
+      msi-parent = <&imsic_mlevel>;
+      reg = <0xc000000 0x4000>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      riscv,num-sources = <63>;
+      riscv,children = <&aplic3>;
+      riscv,delegation = <&aplic3 1 63>;
+    };
+
+    aplic3: interrupt-controller@d000000 {
+      compatible = "qemu,aplic", "riscv,aplic";
+      msi-parent = <&imsic_slevel>;
+      reg = <0xd000000 0x4000>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      riscv,num-sources = <63>;
+    };
+...
--

2.34.1

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Quoted 228 lines · Vincent Chen

Quoted 224 lines · Anup Patel

Quoted 215 lines · Vincent Chen

Quoted 157 lines · Anup Patel

We add DT bindings document for RISC-V advanced platform level interrupt controller (APLIC) defined by the RISC-V advanced interrupt architecture (AIA) specification.

Signed-off-by Anup Patel <[email protected]>

Reviewed-by Conor Dooley <[email protected]>

---
 .../interrupt-controller/riscv,aplic.yaml     | 172 ++++++++++++++++++
 1 file changed, 172 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml

Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +139 -0

new file mode 100644
index 000000000000..190a6499c932
@@ -0,0 +1,172 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V Advanced Platform Level Interrupt Controller (APLIC)
+
+maintainers:
+  - Anup Patel <[email protected]>
+
+description:
+  The RISC-V advanced interrupt architecture (AIA) defines an advanced
+  platform level interrupt controller (APLIC) for handling wired interrupts
+  in a RISC-V platform. The RISC-V AIA specification can be found at
+  https://github.com/riscv/riscv-aia.
+
+  The RISC-V APLIC is implemented as hierarchical APLIC domains where all
+  interrupt sources connect to the root APLIC domain and a parent APLIC
+  domain can delegate interrupt sources to it's child APLIC domains. There
+  is one device tree node for each APLIC domain.
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qemu,aplic
+      - const: riscv,aplic
+
+  reg:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+  interrupts-extended:
+    minItems: 1
+    maxItems: 16384
+    description:
+      Given APLIC domain directly injects external interrupts to a set of
+      RISC-V HARTS (or CPUs). Each node pointed to should be a riscv,cpu-intc
+      node, which has a CPU node (i.e. RISC-V HART) as parent.
+
+  msi-parent:
+    description:
+      Given APLIC domain forwards wired interrupts as MSIs to a AIA incoming
+      message signaled interrupt controller (IMSIC). If both "msi-parent" and
+      "interrupts-extended" properties are present then it means the APLIC
+      domain supports both MSI mode and Direct mode in HW. In this case, the
+      APLIC driver has to choose between MSI mode or Direct mode.
+
+  riscv,num-sources:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 1023
+    description:
+      Specifies the number of wired interrupt sources supported by this
+      APLIC domain.
+
+  riscv,children:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    minItems: 1
+    maxItems: 1024
+    items:
+      maxItems: 1
+    description:
+      A list of child APLIC domains for the given APLIC domain. Each child
+      APLIC domain is assigned a child index in increasing order, with the
+      first child APLIC domain assigned child index 0. The APLIC domain child
+      index is used by firmware to delegate interrupts from the given APLIC
+      domain to a particular child APLIC domain.
+
+  riscv,delegation:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    minItems: 1
+    maxItems: 1024
+    items:
+      items:
+        - description: child APLIC domain phandle
+        - description: first interrupt number of the parent APLIC domain (inclusive)
+        - description: last interrupt number of the parent APLIC domain (inclusive)
+    description:
+      A interrupt delegation list where each entry is a triple consisting
+      of child APLIC domain phandle, first interrupt number of the parent
+      APLIC domain, and last interrupt number of the parent APLIC domain.
+      Firmware must configure interrupt delegation registers based on
+      interrupt delegation list.
+
+dependencies:
+  riscv,delegation: [ "riscv,children" ]
+
+required:
+  - compatible
+  - reg
+  - interrupt-controller
+  - "#interrupt-cells"
+  - riscv,num-sources
+
+anyOf:
+  - required:
+      - interrupts-extended
+  - required:
+      - msi-parent
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    // Example 1 (APLIC domains directly injecting interrupt to HARTs):
+
+    interrupt-controller@c000000 {
+      compatible = "qemu,aplic", "riscv,aplic";
+      interrupts-extended = <&cpu1_intc 11>,
+                            <&cpu2_intc 11>,
+                            <&cpu3_intc 11>,
+                            <&cpu4_intc 11>;
+      reg = <0xc000000 0x4080>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      riscv,num-sources = <63>;
+      riscv,children = <&aplic1>, <&aplic2>;
+      riscv,delegation = <&aplic1 1 63>;
+    };
+
+    aplic1: interrupt-controller@d000000 {
+      compatible = "qemu,aplic", "riscv,aplic";
+      interrupts-extended = <&cpu1_intc 9>,
+                            <&cpu2_intc 9>;
+      reg = <0xd000000 0x4080>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      riscv,num-sources = <63>;
+    };
+

Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml line 149

Quoted 10 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +10 -0

+    aplic2: interrupt-controller@e000000 {
+      compatible = "qemu,aplic", "riscv,aplic";
+      interrupts-extended = <&cpu3_intc 9>,
+                            <&cpu4_intc 9>;
+      reg = <0xe000000 0x4080>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      riscv,num-sources = <63>;
+    };
+

Hi Anup,
I have some thoughts regarding the APLIC DTS node. While I understand that it might be a bit late to discuss this matter within the v7 patch (sorry for this), I hope you could still consider the following idea.

For example 1, my understanding is that each APLIC DTS node represents an interrupt domain. IIUC, in physical, these tree Interrupt domains should belong to an APLIC device so the M-mode IRQ domain can delegate interrupts to the child domain. Given this perspective, wrapping all interrupt domain DTS nodes with another DTS node seems to present the real scene more clearly. Maybe we can add "simple-bus" to the compatible property of this wrapped DTS node, so it still can be compatible with your driver implementations. Therefore, example 1 may become the following.

interrupt-controller {
        compatible = "riscv,aplics", "simple-bus";
        ranges;
        aplic0: interrupt-domain@c000000 {
                compatible = "qemu,aplic", "riscv,aplic";
                interrupts-extended = <&cpu1_intc 11>,
                                      <&cpu2_intc 11>,
                                      <&cpu3_intc 11>,
                                      <&cpu4_intc 11>;
                reg = <0xc000000 0x4080>;
                interrupt-controller;
                #interrupt-cells = <2>;
                riscv,num-sources = <63>;
                riscv,children = <&aplic1>, <&aplic2>;
                riscv,delegation = <&aplic1 1 63>;
        };

        aplic1: interrupt-domain@d000000 {
                compatible = "qemu,aplic", "riscv,aplic";
                interrupts-extended = <&cpu1_intc 9>,
                                      <&cpu2_intc 9>;
                reg = <0xd000000 0x4080>;
                interrupt-controller;
                #interrupt-cells = <2>;
                riscv,num-sources = <63>;
        };

        aplic2: interrupt-domain@e000000 {
                compatible = "qemu,aplic", "riscv,aplic";
                interrupts-extended = <&cpu3_intc 9>,
                                      <&cpu4_intc 9>;
                reg = <0xe000000 0x4080>;
                interrupt-controller;
                #interrupt-cells = <2>;
                riscv,num-sources = <63>;
        };
};

Quoted 1 line

Is it feasible for you?

This clubbing of APLIC domains and placing them close to each other is a platform implementation choice. On multi-die or multi-socket systems the APIC domains can be really far apart on different physical dies so the clubbing you suggest is not always true.

Quoted 4 lines

Quoted 3 lines

Regards,
Anup

IIUC, I think my suggestion might be able to apply to this scenario if these interrupt domains belong to the same APLIC device. For this scenario, one thing I am concerned about is that the MMIO region of other devices locates between interrupt domains. However, currently, I have not found that the DTS specification explicitly prohibits it. Do you have the same concern?

One APLIC device implementing multiple APLIC domains is perfectly fine and there is nothing in this DT bindings which prevents that.

We can always group multiple APLIC domain DT nodes under another DT node but we don't need any special DT bindings for such grouping and the APLIC driver does not need to be aware of this grouping. Further, the AIA spec does not mandate grouping of APLIC domains under one APLIC device.

Like I said previously, the grouping of APLIC domains under one DT node is implementation/platform specific.

Regards,
Anup

Quoted 36 lines

Thanks,
Vincent

Quoted 33 lines

Quoted 33 lines

Thanks,
Vincent

Quoted 28 lines

Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +23 -1

+  - |
+    // Example 2 (APLIC domains forwarding interrupts as MSIs):
+
+    interrupt-controller@c000000 {
+      compatible = "qemu,aplic", "riscv,aplic";
+      msi-parent = <&imsic_mlevel>;
+      reg = <0xc000000 0x4000>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      riscv,num-sources = <63>;
+      riscv,children = <&aplic3>;
+      riscv,delegation = <&aplic3 1 63>;
+    };
+
+    aplic3: interrupt-controller@d000000 {
+      compatible = "qemu,aplic", "riscv,aplic";
+      msi-parent = <&imsic_slevel>;
+      reg = <0xd000000 0x4000>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      riscv,num-sources = <63>;
+    };
+...
--

2.34.1

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv