[PATCH v5 0/6] irqchip: dw-apb-ictl: support hierarchy irq domain

[PATCH v5 0/6] irqchip: dw-apb-ictl: support hierarchy irq domain

[Zhen Lei]

v4 --> v5:
1. Add WARN_ON(1) in set_handle_irq() if !GENERIC_IRQ_MULTI_HANDLER
2. Convert "snps,dw-apb-ictl.txt" to "snps,dw-apb-ictl.yaml"
3. Fix the errors detected by "snps,dw-apb-ictl.yaml" on arch/arc

v3 --> v4:
1. remove "gc->chip_types[0].chip.irq_eoi = irq_gc_noop;", the "chip.irq_eoi" hook
   is not needed by handle_level_irq(). Thanks for Marc Zyngier's review.
2. Add a new patch: define an empty function set_handle_irq() if !GENERIC_IRQ_MULTI_HANDLER
   to avoid compilation error on arch/arc system.

v2 --> v3:
1. change (1 << hwirq) to BIT(hwirq).
2. change __exception_irq_entry to __irq_entry, so we can "#include <linux/interrupt.h>"
   instead of "#include <asm/exception.h>". Ohterwise, an compilation error will be
   reported on arch/csky.
   drivers/irqchip/irq-dw-apb-ictl.c:20:10: fatal error: asm/exception.h: No such file or directory
3. use "if (!parent || (np == parent))" to determine whether it is primary interrupt controller.
4. make the primary interrupt controller case also use function handle_level_irq(), I used 
   handle_fasteoi_irq() as flow_handler before.
5. Other minor changes are not detailed.

v1 --> v2:
According to Marc Zyngier's suggestion, discard adding an independent SD5203-VIC
driver, but make the dw-apb-ictl irqchip driver to support hierarchy irq domain.
It was originally available only for secondary interrupt controller, now it can
also be used as primary interrupt controller. The related dt-bindings is updated
appropriately.

Add "Suggested-by: Marc Zyngier <maz@kernel.org>".
Add "Tested-by: Haoyu Lv <lvhaoyu@huawei.com>".


v1:
The interrupt controller of SD5203 SoC is VIC(vector interrupt controller), it's
based on Synopsys DesignWare APB interrupt controller (dw_apb_ictl) IP, but it
can not directly use dw_apb_ictl driver. The main reason is that VIC is used as
primary interrupt controller and dw_apb_ictl driver worked for secondary
interrupt controller. So add a new driver: "hisilicon,sd5203-vic".

Zhen Lei (6):
  genirq: define an empty function set_handle_irq() if
    !GENERIC_IRQ_MULTI_HANDLER
  irqchip: dw-apb-ictl: prepare for support hierarchy irq domain
  irqchip: dw-apb-ictl: support hierarchy irq domain
  dt-bindings: dw-apb-ictl: support hierarchy irq domain
  dt-bindings: dw-apb-ictl: convert to json-schema
  ARC: [dts] fix the errors detected by dtbs_check

 .../interrupt-controller/snps,dw-apb-ictl.txt      | 31 --------
 .../interrupt-controller/snps,dw-apb-ictl.yaml     | 75 +++++++++++++++++++
 arch/arc/boot/dts/axc001.dtsi                      |  2 +-
 arch/arc/boot/dts/axc003.dtsi                      |  2 +-
 arch/arc/boot/dts/axc003_idu.dtsi                  |  2 +-
 arch/arc/boot/dts/vdk_axc003.dtsi                  |  2 +-
 arch/arc/boot/dts/vdk_axc003_idu.dtsi              |  2 +-
 drivers/irqchip/Kconfig                            |  2 +-
 drivers/irqchip/irq-dw-apb-ictl.c                  | 83 ++++++++++++++++++----
 include/linux/irq.h                                |  6 ++
 10 files changed, 158 insertions(+), 49 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml

-- 
1.8.3

[PATCH v5 1/6] genirq: define an empty function set_handle_irq() if !GENERIC_IRQ_MULTI_HANDLER

[Zhen Lei]

To avoid compilation error if an irqchip driver references the function
set_handle_irq() but may not select GENERIC_IRQ_MULTI_HANDLER on some
systems.

For example, the Synopsys DesignWare APB interrupt controller
(dw_apb_ictl) is used as the secondary interrupt controller on arc, csky,
arm64, and most arm32 SoCs, and it's also used as the primary interrupt
controller on Hisilicon SD5203 (an arm32 SoC). The latter need to use
set_handle_irq() to register the top-level IRQ handler, but this multi
irq handler registration mechanism is not implemented on arc system.

The input parameter "handle_irq" maybe defined as static and only
set_handle_irq() references it. This will trigger "defined but not used"
warning. So add "(void)handle_irq" to suppress it.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 include/linux/irq.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 1b7f4dfee35b397..b167baef88c0b43 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1252,6 +1252,12 @@ void irq_matrix_free(struct irq_matrix *m, unsigned int cpu,
  * top-level IRQ handler.
  */
 extern void (*handle_arch_irq)(struct pt_regs *) __ro_after_init;
+#else
+#define set_handle_irq(handle_irq)		\
+	do {					\
+		(void)handle_irq;		\
+		WARN_ON(1);			\
+	} while (0)
 #endif
 
 #endif /* _LINUX_IRQ_H */
-- 
1.8.3

[PATCH v5 2/6] irqchip: dw-apb-ictl: prepare for support hierarchy irq domain

[Zhen Lei]

Rename some functions and variables in advance, to make the next patch
looks more clear. The details are as follows:
1. rename dw_apb_ictl_handler() to dw_apb_ictl_handle_irq_cascaded().
2. change (1 << hwirq) to BIT(hwirq).

In function dw_apb_ictl_init():
1. rename local variable irq to parent_irq.
2. add "const struct irq_domain_ops *domain_ops = &irq_generic_chip_ops",
   then replace &irq_generic_chip_ops in other places with domain_ops.

No functional change.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
Tested-by: Haoyu Lv <lvhaoyu@huawei.com>
---
 drivers/irqchip/irq-dw-apb-ictl.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
index e4550e9c810ba94..5458004242e9d20 100644
--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c
@@ -26,7 +26,7 @@
 #define APB_INT_FINALSTATUS_H	0x34
 #define APB_INT_BASE_OFFSET	0x04
 
-static void dw_apb_ictl_handler(struct irq_desc *desc)
+static void dw_apb_ictl_handle_irq_cascaded(struct irq_desc *desc)
 {
 	struct irq_domain *d = irq_desc_get_handler_data(desc);
 	struct irq_chip *chip = irq_desc_get_chip(desc);
@@ -43,7 +43,7 @@ static void dw_apb_ictl_handler(struct irq_desc *desc)
 			u32 virq = irq_find_mapping(d, gc->irq_base + hwirq);
 
 			generic_handle_irq(virq);
-			stat &= ~(1 << hwirq);
+			stat &= ~BIT(hwirq);
 		}
 	}
 
@@ -73,12 +73,13 @@ static int __init dw_apb_ictl_init(struct device_node *np,
 	struct irq_domain *domain;
 	struct irq_chip_generic *gc;
 	void __iomem *iobase;
-	int ret, nrirqs, irq, i;
+	int ret, nrirqs, parent_irq, i;
 	u32 reg;
+	const struct irq_domain_ops *domain_ops = &irq_generic_chip_ops;
 
 	/* Map the parent interrupt for the chained handler */
-	irq = irq_of_parse_and_map(np, 0);
-	if (irq <= 0) {
+	parent_irq = irq_of_parse_and_map(np, 0);
+	if (parent_irq <= 0) {
 		pr_err("%pOF: unable to parse irq\n", np);
 		return -EINVAL;
 	}
@@ -120,8 +121,7 @@ static int __init dw_apb_ictl_init(struct device_node *np,
 	else
 		nrirqs = fls(readl_relaxed(iobase + APB_INT_ENABLE_L));
 
-	domain = irq_domain_add_linear(np, nrirqs,
-				       &irq_generic_chip_ops, NULL);
+	domain = irq_domain_add_linear(np, nrirqs, domain_ops, NULL);
 	if (!domain) {
 		pr_err("%pOF: unable to add irq domain\n", np);
 		ret = -ENOMEM;
@@ -146,7 +146,8 @@ static int __init dw_apb_ictl_init(struct device_node *np,
 		gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
 	}
 
-	irq_set_chained_handler_and_data(irq, dw_apb_ictl_handler, domain);
+	irq_set_chained_handler_and_data(parent_irq,
+				dw_apb_ictl_handle_irq_cascaded, domain);
 
 	return 0;
 
-- 
1.8.3

[PATCH v5 3/6] irqchip: dw-apb-ictl: support hierarchy irq domain

[Zhen Lei]

Add support to use dw-apb-ictl as primary interrupt controller.

Suggested-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
Tested-by: Haoyu Lv <lvhaoyu@huawei.com>
---
 drivers/irqchip/Kconfig           |  2 +-
 drivers/irqchip/irq-dw-apb-ictl.c | 74 ++++++++++++++++++++++++++++++++++-----
 2 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index bfc9719dbcdc31c..7c2d1c8fa551a66 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -148,7 +148,7 @@ config DAVINCI_CP_INTC
 config DW_APB_ICTL
 	bool
 	select GENERIC_IRQ_CHIP
-	select IRQ_DOMAIN
+	select IRQ_DOMAIN_HIERARCHY
 
 config FARADAY_FTINTC010
 	bool
diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
index 5458004242e9d20..418183b9983dfad 100644
--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c
@@ -17,6 +17,7 @@
 #include <linux/irqchip/chained_irq.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
+#include <linux/interrupt.h>
 
 #define APB_INT_ENABLE_L	0x00
 #define APB_INT_ENABLE_H	0x04
@@ -26,6 +27,27 @@
 #define APB_INT_FINALSTATUS_H	0x34
 #define APB_INT_BASE_OFFSET	0x04
 
+/* irq domain of the primary interrupt controller. */
+static struct irq_domain *dw_apb_ictl_irq_domain;
+
+static void __irq_entry dw_apb_ictl_handle_irq(struct pt_regs *regs)
+{
+	struct irq_domain *d = dw_apb_ictl_irq_domain;
+	int n;
+
+	for (n = 0; n < d->revmap_size; n += 32) {
+		struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, n);
+		u32 stat = readl_relaxed(gc->reg_base + APB_INT_FINALSTATUS_L);
+
+		while (stat) {
+			u32 hwirq = ffs(stat) - 1;
+
+			handle_domain_irq(d, hwirq, regs);
+			stat &= ~BIT(hwirq);
+		}
+	}
+}
+
 static void dw_apb_ictl_handle_irq_cascaded(struct irq_desc *desc)
 {
 	struct irq_domain *d = irq_desc_get_handler_data(desc);
@@ -50,6 +72,30 @@ static void dw_apb_ictl_handle_irq_cascaded(struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
+static int dw_apb_ictl_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
+				unsigned int nr_irqs, void *arg)
+{
+	int i, ret;
+	irq_hw_number_t hwirq;
+	unsigned int type = IRQ_TYPE_NONE;
+	struct irq_fwspec *fwspec = arg;
+
+	ret = irq_domain_translate_onecell(domain, fwspec, &hwirq, &type);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < nr_irqs; i++)
+		irq_map_generic_chip(domain, virq + i, hwirq + i);
+
+	return 0;
+}
+
+static const struct irq_domain_ops dw_apb_ictl_irq_domain_ops = {
+	.translate = irq_domain_translate_onecell,
+	.alloc = dw_apb_ictl_irq_domain_alloc,
+	.free = irq_domain_free_irqs_top,
+};
+
 #ifdef CONFIG_PM
 static void dw_apb_ictl_resume(struct irq_data *d)
 {
@@ -75,13 +121,20 @@ static int __init dw_apb_ictl_init(struct device_node *np,
 	void __iomem *iobase;
 	int ret, nrirqs, parent_irq, i;
 	u32 reg;
-	const struct irq_domain_ops *domain_ops = &irq_generic_chip_ops;
-
-	/* Map the parent interrupt for the chained handler */
-	parent_irq = irq_of_parse_and_map(np, 0);
-	if (parent_irq <= 0) {
-		pr_err("%pOF: unable to parse irq\n", np);
-		return -EINVAL;
+	const struct irq_domain_ops *domain_ops;
+
+	if (!parent || (np == parent)) {
+		/* It's used as the primary interrupt controller */
+		parent_irq = 0;
+		domain_ops = &dw_apb_ictl_irq_domain_ops;
+	} else {
+		/* Map the parent interrupt for the chained handler */
+		parent_irq = irq_of_parse_and_map(np, 0);
+		if (parent_irq <= 0) {
+			pr_err("%pOF: unable to parse irq\n", np);
+			return -EINVAL;
+		}
+		domain_ops = &irq_generic_chip_ops;
 	}
 
 	ret = of_address_to_resource(np, 0, &r);
@@ -146,8 +199,13 @@ static int __init dw_apb_ictl_init(struct device_node *np,
 		gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
 	}
 
-	irq_set_chained_handler_and_data(parent_irq,
+	if (parent_irq) {
+		irq_set_chained_handler_and_data(parent_irq,
 				dw_apb_ictl_handle_irq_cascaded, domain);
+	} else {
+		dw_apb_ictl_irq_domain = domain;
+		set_handle_irq(dw_apb_ictl_handle_irq);
+	}
 
 	return 0;
 
-- 
1.8.3

[PATCH v5 4/6] dt-bindings: dw-apb-ictl: support hierarchy irq domain

[Zhen Lei]

Add support to use dw-apb-ictl as primary interrupt controller.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 .../bindings/interrupt-controller/snps,dw-apb-ictl.txt     | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
index 086ff08322db94f..2db59df9408f4c6 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
@@ -2,7 +2,8 @@ Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
 
 Synopsys DesignWare provides interrupt controller IP for APB known as
 dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs with
-APB bus, e.g. Marvell Armada 1500.
+APB bus, e.g. Marvell Armada 1500. It can also be used as primary interrupt
+controller in some SoCs, e.g. Hisilicon SD5203.
 
 Required properties:
 - compatible: shall be "snps,dw-apb-ictl"
@@ -10,6 +11,8 @@ Required properties:
   region starting with ENABLE_LOW register
 - interrupt-controller: identifies the node as an interrupt controller
 - #interrupt-cells: number of cells to encode an interrupt-specifier, shall be 1
+
+Additional required property when it's used as secondary interrupt controller:
 - interrupts: interrupt reference to primary interrupt controller
 
 The interrupt sources map to the corresponding bits in the interrupt
@@ -21,6 +24,7 @@ registers, i.e.
 - (optional) fast interrupts start at 64.
 
 Example:
+	/* dw_apb_ictl is used as secondary interrupt controller */
 	aic: interrupt-controller@3000 {
 		compatible = "snps,dw-apb-ictl";
 		reg = <0x3000 0xc00>;
@@ -29,3 +33,11 @@ Example:
 		interrupt-parent = <&gic>;
 		interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
 	};
+
+	/* dw_apb_ictl is used as primary interrupt controller */
+	vic: interrupt-controller@10130000 {
+		compatible = "snps,dw-apb-ictl";
+		reg = <0x10130000 0x1000>;
+		interrupt-controller;
+		#interrupt-cells = <1>;
+	};
-- 
1.8.3

[PATCH v5 5/6] dt-bindings: dw-apb-ictl: convert to json-schema

[Zhen Lei]

Convert the Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
binding to DT schema format using json-schema.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 .../interrupt-controller/snps,dw-apb-ictl.txt      | 43 -------------
 .../interrupt-controller/snps,dw-apb-ictl.yaml     | 75 ++++++++++++++++++++++
 2 files changed, 75 insertions(+), 43 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml

diff --git a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
deleted file mode 100644
index 2db59df9408f4c6..000000000000000
--- a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
+++ /dev/null
@@ -1,43 +0,0 @@
-Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
-
-Synopsys DesignWare provides interrupt controller IP for APB known as
-dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs with
-APB bus, e.g. Marvell Armada 1500. It can also be used as primary interrupt
-controller in some SoCs, e.g. Hisilicon SD5203.
-
-Required properties:
-- compatible: shall be "snps,dw-apb-ictl"
-- reg: physical base address of the controller and length of memory mapped
-  region starting with ENABLE_LOW register
-- interrupt-controller: identifies the node as an interrupt controller
-- #interrupt-cells: number of cells to encode an interrupt-specifier, shall be 1
-
-Additional required property when it's used as secondary interrupt controller:
-- interrupts: interrupt reference to primary interrupt controller
-
-The interrupt sources map to the corresponding bits in the interrupt
-registers, i.e.
-- 0 maps to bit 0 of low interrupts,
-- 1 maps to bit 1 of low interrupts,
-- 32 maps to bit 0 of high interrupts,
-- 33 maps to bit 1 of high interrupts,
-- (optional) fast interrupts start at 64.
-
-Example:
-	/* dw_apb_ictl is used as secondary interrupt controller */
-	aic: interrupt-controller@3000 {
-		compatible = "snps,dw-apb-ictl";
-		reg = <0x3000 0xc00>;
-		interrupt-controller;
-		#interrupt-cells = <1>;
-		interrupt-parent = <&gic>;
-		interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
-	};
-
-	/* dw_apb_ictl is used as primary interrupt controller */
-	vic: interrupt-controller@10130000 {
-		compatible = "snps,dw-apb-ictl";
-		reg = <0x10130000 0x1000>;
-		interrupt-controller;
-		#interrupt-cells = <1>;
-	};
diff --git a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
new file mode 100644
index 000000000000000..70c12979c843bf0
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/snps,dw-apb-ictl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
+
+maintainers:
+  - Marc Zyngier <marc.zyngier@arm.com>
+
+description:
+  Synopsys DesignWare provides interrupt controller IP for APB known as
+  dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs
+  with APB bus, e.g. Marvell Armada 1500. It can also be used as primary
+  interrupt controller in some SoCs, e.g. Hisilicon SD5203.
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+  compatible:
+    const: snps,dw-apb-ictl
+
+  interrupt-controller: true
+
+  reg:
+    description:
+      Physical base address of the controller and length of memory mapped
+      region starting with ENABLE_LOW register.
+
+  interrupts:
+    description:
+      Interrupt reference to primary interrupt controller.
+
+      The interrupt sources map to the corresponding bits in the interrupt
+      registers, i.e.
+      - 0 maps to bit 0 of low interrupts,
+      - 1 maps to bit 1 of low interrupts,
+      - 32 maps to bit 0 of high interrupts,
+      - 33 maps to bit 1 of high interrupts,
+      - (optional) fast interrupts start at 64.
+    minItems: 1
+    maxItems: 65
+
+  "#interrupt-cells":
+    description:
+      Number of cells to encode an interrupt-specifier.
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - interrupt-controller
+  - '#interrupt-cells'
+
+examples:
+  - |
+    /* dw_apb_ictl is used as secondary interrupt controller */
+    aic: interrupt-controller@3000 {
+        compatible = "snps,dw-apb-ictl";
+        reg = <0x3000 0xc00>;
+        interrupt-controller;
+        #interrupt-cells = <1>;
+        interrupt-parent = <&gic>;
+        interrupts = <0 3 4>;
+    };
+
+    /* dw_apb_ictl is used as primary interrupt controller */
+    vic: interrupt-controller@10130000 {
+        compatible = "snps,dw-apb-ictl";
+        reg = <0x10130000 0x1000>;
+        interrupt-controller;
+        #interrupt-cells = <1>;
+    };
-- 
1.8.3

[PATCH v5 6/6] ARC: [dts] fix the errors detected by dtbs_check

[Zhen Lei]

xxx/arc/boot/dts/axs101.dt.yaml: dw-apb-ictl@e0012000: $nodename:0: \
'dw-apb-ictl@e0012000' does not match '^interrupt-controller(@[0-9a-f,]+)*$'
From schema: xxx/interrupt-controller/snps,dw-apb-ictl.yaml

The node name of the interrupt controller must start with
"interrupt-controller" instead of "dw-apb-ictl".

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 arch/arc/boot/dts/axc001.dtsi         | 2 +-
 arch/arc/boot/dts/axc003.dtsi         | 2 +-
 arch/arc/boot/dts/axc003_idu.dtsi     | 2 +-
 arch/arc/boot/dts/vdk_axc003.dtsi     | 2 +-
 arch/arc/boot/dts/vdk_axc003_idu.dtsi | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arc/boot/dts/axc001.dtsi b/arch/arc/boot/dts/axc001.dtsi
index 79ec27c043c1da7..2a151607b08057c 100644
--- a/arch/arc/boot/dts/axc001.dtsi
+++ b/arch/arc/boot/dts/axc001.dtsi
@@ -91,7 +91,7 @@
 	 * avoid duplicating the MB dtsi file given that IRQ from
 	 * this intc to cpu intc are different for axs101 and axs103
 	 */
-	mb_intc: dw-apb-ictl@e0012000 {
+	mb_intc: interrupt-controller@e0012000 {
 		#interrupt-cells = <1>;
 		compatible = "snps,dw-apb-ictl";
 		reg = < 0x0 0xe0012000 0x0 0x200 >;
diff --git a/arch/arc/boot/dts/axc003.dtsi b/arch/arc/boot/dts/axc003.dtsi
index ac8e1b463a70992..cd1edcf4f95efe6 100644
--- a/arch/arc/boot/dts/axc003.dtsi
+++ b/arch/arc/boot/dts/axc003.dtsi
@@ -129,7 +129,7 @@
 	 * avoid duplicating the MB dtsi file given that IRQ from
 	 * this intc to cpu intc are different for axs101 and axs103
 	 */
-	mb_intc: dw-apb-ictl@e0012000 {
+	mb_intc: interrupt-controller@e0012000 {
 		#interrupt-cells = <1>;
 		compatible = "snps,dw-apb-ictl";
 		reg = < 0x0 0xe0012000 0x0 0x200 >;
diff --git a/arch/arc/boot/dts/axc003_idu.dtsi b/arch/arc/boot/dts/axc003_idu.dtsi
index 9da21e7fd246f9f..70779386ca7963a 100644
--- a/arch/arc/boot/dts/axc003_idu.dtsi
+++ b/arch/arc/boot/dts/axc003_idu.dtsi
@@ -135,7 +135,7 @@
 	 * avoid duplicating the MB dtsi file given that IRQ from
 	 * this intc to cpu intc are different for axs101 and axs103
 	 */
-	mb_intc: dw-apb-ictl@e0012000 {
+	mb_intc: interrupt-controller@e0012000 {
 		#interrupt-cells = <1>;
 		compatible = "snps,dw-apb-ictl";
 		reg = < 0x0 0xe0012000 0x0 0x200 >;
diff --git a/arch/arc/boot/dts/vdk_axc003.dtsi b/arch/arc/boot/dts/vdk_axc003.dtsi
index f8be7ba8dad499c..c21d0eb07bf6737 100644
--- a/arch/arc/boot/dts/vdk_axc003.dtsi
+++ b/arch/arc/boot/dts/vdk_axc003.dtsi
@@ -46,7 +46,7 @@
 
 	};
 
-	mb_intc: dw-apb-ictl@e0012000 {
+	mb_intc: interrupt-controller@e0012000 {
 		#interrupt-cells = <1>;
 		compatible = "snps,dw-apb-ictl";
 		reg = < 0xe0012000 0x200 >;
diff --git a/arch/arc/boot/dts/vdk_axc003_idu.dtsi b/arch/arc/boot/dts/vdk_axc003_idu.dtsi
index 0afa3e53a4e3932..4d348853ac7c5dc 100644
--- a/arch/arc/boot/dts/vdk_axc003_idu.dtsi
+++ b/arch/arc/boot/dts/vdk_axc003_idu.dtsi
@@ -54,7 +54,7 @@
 
 	};
 
-	mb_intc: dw-apb-ictl@e0012000 {
+	mb_intc: interrupt-controller@e0012000 {
 		#interrupt-cells = <1>;
 		compatible = "snps,dw-apb-ictl";
 		reg = < 0xe0012000 0x200 >;
-- 
1.8.3

Re: [PATCH v5 5/6] dt-bindings: dw-apb-ictl: convert to json-schema

[Rob Herring]

On Fri, Sep 18, 2020 at 07:22:01PM +0800, Zhen Lei wrote:
> Convert the Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
> binding to DT schema format using json-schema.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  .../interrupt-controller/snps,dw-apb-ictl.txt      | 43 -------------
>  .../interrupt-controller/snps,dw-apb-ictl.yaml     | 75 ++++++++++++++++++++++
>  2 files changed, 75 insertions(+), 43 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
> deleted file mode 100644
> index 2db59df9408f4c6..000000000000000
> --- a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
> -
> -Synopsys DesignWare provides interrupt controller IP for APB known as
> -dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs with
> -APB bus, e.g. Marvell Armada 1500. It can also be used as primary interrupt
> -controller in some SoCs, e.g. Hisilicon SD5203.
> -
> -Required properties:
> -- compatible: shall be "snps,dw-apb-ictl"
> -- reg: physical base address of the controller and length of memory mapped
> -  region starting with ENABLE_LOW register
> -- interrupt-controller: identifies the node as an interrupt controller
> -- #interrupt-cells: number of cells to encode an interrupt-specifier, shall be 1
> -
> -Additional required property when it's used as secondary interrupt controller:
> -- interrupts: interrupt reference to primary interrupt controller
> -
> -The interrupt sources map to the corresponding bits in the interrupt
> -registers, i.e.
> -- 0 maps to bit 0 of low interrupts,
> -- 1 maps to bit 1 of low interrupts,
> -- 32 maps to bit 0 of high interrupts,
> -- 33 maps to bit 1 of high interrupts,
> -- (optional) fast interrupts start at 64.
> -
> -Example:
> -	/* dw_apb_ictl is used as secondary interrupt controller */
> -	aic: interrupt-controller@3000 {
> -		compatible = "snps,dw-apb-ictl";
> -		reg = <0x3000 0xc00>;
> -		interrupt-controller;
> -		#interrupt-cells = <1>;
> -		interrupt-parent = <&gic>;
> -		interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> -	};
> -
> -	/* dw_apb_ictl is used as primary interrupt controller */
> -	vic: interrupt-controller@10130000 {
> -		compatible = "snps,dw-apb-ictl";
> -		reg = <0x10130000 0x1000>;
> -		interrupt-controller;
> -		#interrupt-cells = <1>;
> -	};
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
> new file mode 100644
> index 000000000000000..70c12979c843bf0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
> @@ -0,0 +1,75 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/snps,dw-apb-ictl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
> +
> +maintainers:
> +  - Marc Zyngier <marc.zyngier@arm.com>

Usually this would be an owner for this IP block, not the subsystem 
maintainer.

> +
> +description:
> +  Synopsys DesignWare provides interrupt controller IP for APB known as
> +  dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs
> +  with APB bus, e.g. Marvell Armada 1500. It can also be used as primary
> +  interrupt controller in some SoCs, e.g. Hisilicon SD5203.
> +
> +allOf:
> +  - $ref: /schemas/interrupt-controller.yaml#

You can drop this, it's already applied based on node name.

> +
> +properties:
> +  compatible:
> +    const: snps,dw-apb-ictl
> +
> +  interrupt-controller: true
> +
> +  reg:
> +    description:
> +      Physical base address of the controller and length of memory mapped
> +      region starting with ENABLE_LOW register.

Need to define how many reg regions (maxItems: 1).

> +
> +  interrupts:
> +    description:
> +      Interrupt reference to primary interrupt controller.
> +
> +      The interrupt sources map to the corresponding bits in the interrupt
> +      registers, i.e.
> +      - 0 maps to bit 0 of low interrupts,
> +      - 1 maps to bit 1 of low interrupts,
> +      - 32 maps to bit 0 of high interrupts,
> +      - 33 maps to bit 1 of high interrupts,
> +      - (optional) fast interrupts start at 64.
> +    minItems: 1
> +    maxItems: 65

65 connections to the primary interrupt controller? I think this is for 
the child interrupts? If so, move to #interrupt-cells description 
instead.

> +
> +  "#interrupt-cells":
> +    description:
> +      Number of cells to encode an interrupt-specifier.

Drop. No need to redefine common properties.

> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupt-controller
> +  - '#interrupt-cells'
> +
> +examples:
> +  - |
> +    /* dw_apb_ictl is used as secondary interrupt controller */
> +    aic: interrupt-controller@3000 {
> +        compatible = "snps,dw-apb-ictl";
> +        reg = <0x3000 0xc00>;
> +        interrupt-controller;
> +        #interrupt-cells = <1>;
> +        interrupt-parent = <&gic>;
> +        interrupts = <0 3 4>;
> +    };
> +
> +    /* dw_apb_ictl is used as primary interrupt controller */
> +    vic: interrupt-controller@10130000 {
> +        compatible = "snps,dw-apb-ictl";
> +        reg = <0x10130000 0x1000>;
> +        interrupt-controller;
> +        #interrupt-cells = <1>;
> +    };
> -- 
> 1.8.3
> 
> 

Re: [PATCH v5 4/6] dt-bindings: dw-apb-ictl: support hierarchy irq domain

[Rob Herring]

On Fri, 18 Sep 2020 19:22:00 +0800, Zhen Lei wrote:
> Add support to use dw-apb-ictl as primary interrupt controller.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  .../bindings/interrupt-controller/snps,dw-apb-ictl.txt     | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v5 5/6] dt-bindings: dw-apb-ictl: convert to json-schema

[Leizhen (ThunderTown)]

On 2020/9/24 4:49, Rob Herring wrote:
> On Fri, Sep 18, 2020 at 07:22:01PM +0800, Zhen Lei wrote:
>> Convert the Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
>> binding to DT schema format using json-schema.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  .../interrupt-controller/snps,dw-apb-ictl.txt      | 43 -------------
>>  .../interrupt-controller/snps,dw-apb-ictl.yaml     | 75 ++++++++++++++++++++++
>>  2 files changed, 75 insertions(+), 43 deletions(-)
>>  delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
>>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
>> deleted file mode 100644
>> index 2db59df9408f4c6..000000000000000
>> --- a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
>> +++ /dev/null
>> @@ -1,43 +0,0 @@
>> -Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
>> -
>> -Synopsys DesignWare provides interrupt controller IP for APB known as
>> -dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs with
>> -APB bus, e.g. Marvell Armada 1500. It can also be used as primary interrupt
>> -controller in some SoCs, e.g. Hisilicon SD5203.
>> -
>> -Required properties:
>> -- compatible: shall be "snps,dw-apb-ictl"
>> -- reg: physical base address of the controller and length of memory mapped
>> -  region starting with ENABLE_LOW register
>> -- interrupt-controller: identifies the node as an interrupt controller
>> -- #interrupt-cells: number of cells to encode an interrupt-specifier, shall be 1
>> -
>> -Additional required property when it's used as secondary interrupt controller:
>> -- interrupts: interrupt reference to primary interrupt controller
>> -
>> -The interrupt sources map to the corresponding bits in the interrupt
>> -registers, i.e.
>> -- 0 maps to bit 0 of low interrupts,
>> -- 1 maps to bit 1 of low interrupts,
>> -- 32 maps to bit 0 of high interrupts,
>> -- 33 maps to bit 1 of high interrupts,
>> -- (optional) fast interrupts start at 64.
>> -
>> -Example:
>> -	/* dw_apb_ictl is used as secondary interrupt controller */
>> -	aic: interrupt-controller@3000 {
>> -		compatible = "snps,dw-apb-ictl";
>> -		reg = <0x3000 0xc00>;
>> -		interrupt-controller;
>> -		#interrupt-cells = <1>;
>> -		interrupt-parent = <&gic>;
>> -		interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
>> -	};
>> -
>> -	/* dw_apb_ictl is used as primary interrupt controller */
>> -	vic: interrupt-controller@10130000 {
>> -		compatible = "snps,dw-apb-ictl";
>> -		reg = <0x10130000 0x1000>;
>> -		interrupt-controller;
>> -		#interrupt-cells = <1>;
>> -	};
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
>> new file mode 100644
>> index 000000000000000..70c12979c843bf0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
>> @@ -0,0 +1,75 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/interrupt-controller/snps,dw-apb-ictl.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
>> +
>> +maintainers:
>> +  - Marc Zyngier <marc.zyngier@arm.com>
> 
> Usually this would be an owner for this IP block, not the subsystem 
> maintainer.

OK, I will change it to the author of the file "snps,dw-apb-ictl.txt".


> 
>> +
>> +description:
>> +  Synopsys DesignWare provides interrupt controller IP for APB known as
>> +  dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs
>> +  with APB bus, e.g. Marvell Armada 1500. It can also be used as primary
>> +  interrupt controller in some SoCs, e.g. Hisilicon SD5203.
>> +
>> +allOf:
>> +  - $ref: /schemas/interrupt-controller.yaml#
> 
> You can drop this, it's already applied based on node name.
But if we drop this, the "snps,dw-apb-ictl.yaml" can not require that the node name
must match '^interrupt-controller(@[0-9a-f,]+)*$'. The problem of Patch 6/6 was
discovered by this.

> 
>> +
>> +properties:
>> +  compatible:
>> +    const: snps,dw-apb-ictl
>> +
>> +  interrupt-controller: true
>> +
>> +  reg:
>> +    description:
>> +      Physical base address of the controller and length of memory mapped
>> +      region starting with ENABLE_LOW register.
> 
> Need to define how many reg regions (maxItems: 1).

OK, I will add it.

> 
>> +
>> +  interrupts:
>> +    description:
>> +      Interrupt reference to primary interrupt controller.
>> +
>> +      The interrupt sources map to the corresponding bits in the interrupt
>> +      registers, i.e.
>> +      - 0 maps to bit 0 of low interrupts,
>> +      - 1 maps to bit 1 of low interrupts,
>> +      - 32 maps to bit 0 of high interrupts,
>> +      - 33 maps to bit 1 of high interrupts,
>> +      - (optional) fast interrupts start at 64.
>> +    minItems: 1
>> +    maxItems: 65
> 
> 65 connections to the primary interrupt controller? I think this is for 
> the child interrupts? If so, move to #interrupt-cells description 
> instead.
Oh, yes. The property "interrupts" here describes which interrupts are used in the
primary interrupt controller. We don't known how many will be connected but at least one.
I will remove "maxItems: 65", it's my mistake.

Property "#interrupt-cells" describes how many cells of one "interrupts" item. So I will
move these descriptions under the top property "properties:"

> 
>> +
>> +  "#interrupt-cells":
>> +    description:
>> +      Number of cells to encode an interrupt-specifier.
> 
> Drop. No need to redefine common properties.

This is useful. See below, I add "const: 1" to require that the value of
"#interrupt-cells" can only be one.

#interrupt-cells = <1>;

I followed the description in "snps,dw-apb-ictl.txt".
-- #interrupt-cells: number of cells to encode an interrupt-specifier, shall be 1

> 
>> +    const: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupt-controller
>> +  - '#interrupt-cells'
>> +
>> +examples:
>> +  - |
>> +    /* dw_apb_ictl is used as secondary interrupt controller */
>> +    aic: interrupt-controller@3000 {
>> +        compatible = "snps,dw-apb-ictl";
>> +        reg = <0x3000 0xc00>;
>> +        interrupt-controller;
>> +        #interrupt-cells = <1>;
>> +        interrupt-parent = <&gic>;
>> +        interrupts = <0 3 4>;
>> +    };
>> +
>> +    /* dw_apb_ictl is used as primary interrupt controller */
>> +    vic: interrupt-controller@10130000 {
>> +        compatible = "snps,dw-apb-ictl";
>> +        reg = <0x10130000 0x1000>;
>> +        interrupt-controller;
>> +        #interrupt-cells = <1>;
>> +    };
>> -- 
>> 1.8.3
>>
>>
> 
> .
> 

Re: [PATCH v5 5/6] dt-bindings: dw-apb-ictl: convert to json-schema

[Leizhen (ThunderTown)]

On 2020/9/24 11:26, Leizhen (ThunderTown) wrote:
> 
> 
> On 2020/9/24 4:49, Rob Herring wrote:
>> On Fri, Sep 18, 2020 at 07:22:01PM +0800, Zhen Lei wrote:
>>> Convert the Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
>>> binding to DT schema format using json-schema.
>>>
>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>>> ---
>>>  .../interrupt-controller/snps,dw-apb-ictl.txt      | 43 -------------
>>>  .../interrupt-controller/snps,dw-apb-ictl.yaml     | 75 ++++++++++++++++++++++
>>>  2 files changed, 75 insertions(+), 43 deletions(-)
>>>  delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
>>>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
>>> deleted file mode 100644
>>> index 2db59df9408f4c6..000000000000000
>>> --- a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
>>> +++ /dev/null
>>> @@ -1,43 +0,0 @@
>>> -Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
>>> -
>>> -Synopsys DesignWare provides interrupt controller IP for APB known as
>>> -dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs with
>>> -APB bus, e.g. Marvell Armada 1500. It can also be used as primary interrupt
>>> -controller in some SoCs, e.g. Hisilicon SD5203.
>>> -
>>> -Required properties:
>>> -- compatible: shall be "snps,dw-apb-ictl"
>>> -- reg: physical base address of the controller and length of memory mapped
>>> -  region starting with ENABLE_LOW register
>>> -- interrupt-controller: identifies the node as an interrupt controller
>>> -- #interrupt-cells: number of cells to encode an interrupt-specifier, shall be 1
>>> -
>>> -Additional required property when it's used as secondary interrupt controller:
>>> -- interrupts: interrupt reference to primary interrupt controller
>>> -
>>> -The interrupt sources map to the corresponding bits in the interrupt
>>> -registers, i.e.
>>> -- 0 maps to bit 0 of low interrupts,
>>> -- 1 maps to bit 1 of low interrupts,
>>> -- 32 maps to bit 0 of high interrupts,
>>> -- 33 maps to bit 1 of high interrupts,
>>> -- (optional) fast interrupts start at 64.
>>> -
>>> -Example:
>>> -	/* dw_apb_ictl is used as secondary interrupt controller */
>>> -	aic: interrupt-controller@3000 {
>>> -		compatible = "snps,dw-apb-ictl";
>>> -		reg = <0x3000 0xc00>;
>>> -		interrupt-controller;
>>> -		#interrupt-cells = <1>;
>>> -		interrupt-parent = <&gic>;
>>> -		interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
>>> -	};
>>> -
>>> -	/* dw_apb_ictl is used as primary interrupt controller */
>>> -	vic: interrupt-controller@10130000 {
>>> -		compatible = "snps,dw-apb-ictl";
>>> -		reg = <0x10130000 0x1000>;
>>> -		interrupt-controller;
>>> -		#interrupt-cells = <1>;
>>> -	};
>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
>>> new file mode 100644
>>> index 000000000000000..70c12979c843bf0
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
>>> @@ -0,0 +1,75 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/interrupt-controller/snps,dw-apb-ictl.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
>>> +
>>> +maintainers:
>>> +  - Marc Zyngier <marc.zyngier@arm.com>
>>
>> Usually this would be an owner for this IP block, not the subsystem 
>> maintainer.
> 
> OK, I will change it to the author of the file "snps,dw-apb-ictl.txt".
> 
> 
>>
>>> +
>>> +description:
>>> +  Synopsys DesignWare provides interrupt controller IP for APB known as
>>> +  dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs
>>> +  with APB bus, e.g. Marvell Armada 1500. It can also be used as primary
>>> +  interrupt controller in some SoCs, e.g. Hisilicon SD5203.
>>> +
>>> +allOf:
>>> +  - $ref: /schemas/interrupt-controller.yaml#
>>
>> You can drop this, it's already applied based on node name.
> But if we drop this, the "snps,dw-apb-ictl.yaml" can not require that the node name
> must match '^interrupt-controller(@[0-9a-f,]+)*$'. The problem of Patch 6/6 was
> discovered by this.
> 
>>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: snps,dw-apb-ictl
>>> +
>>> +  interrupt-controller: true
>>> +
>>> +  reg:
>>> +    description:
>>> +      Physical base address of the controller and length of memory mapped
>>> +      region starting with ENABLE_LOW register.
>>
>> Need to define how many reg regions (maxItems: 1).
> 
> OK, I will add it.
> 
>>
>>> +
>>> +  interrupts:
>>> +    description:
>>> +      Interrupt reference to primary interrupt controller.
>>> +
>>> +      The interrupt sources map to the corresponding bits in the interrupt
>>> +      registers, i.e.
>>> +      - 0 maps to bit 0 of low interrupts,
>>> +      - 1 maps to bit 1 of low interrupts,
>>> +      - 32 maps to bit 0 of high interrupts,
>>> +      - 33 maps to bit 1 of high interrupts,
>>> +      - (optional) fast interrupts start at 64.
>>> +    minItems: 1
>>> +    maxItems: 65
>>
>> 65 connections to the primary interrupt controller? I think this is for 
>> the child interrupts? If so, move to #interrupt-cells description 
>> instead.
> Oh, yes. The property "interrupts" here describes which interrupts are used in the
> primary interrupt controller. We don't known how many will be connected but at least one.
> I will remove "maxItems: 65", it's my mistake.

Oh, I think I should change "minItems: 1" to "maxItems: 1" further， because there is only one
connection currently.

> 
> Property "#interrupt-cells" describes how many cells of one "interrupts" item. So I will
> move these descriptions under the top property "properties:"
> 
>>
>>> +
>>> +  "#interrupt-cells":
>>> +    description:
>>> +      Number of cells to encode an interrupt-specifier.
>>
>> Drop. No need to redefine common properties.
> 
> This is useful. See below, I add "const: 1" to require that the value of
> "#interrupt-cells" can only be one.
> 
> #interrupt-cells = <1>;
> 
> I followed the description in "snps,dw-apb-ictl.txt".
> -- #interrupt-cells: number of cells to encode an interrupt-specifier, shall be 1
> 
>>
>>> +    const: 1
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - interrupt-controller
>>> +  - '#interrupt-cells'
>>> +
>>> +examples:
>>> +  - |
>>> +    /* dw_apb_ictl is used as secondary interrupt controller */
>>> +    aic: interrupt-controller@3000 {
>>> +        compatible = "snps,dw-apb-ictl";
>>> +        reg = <0x3000 0xc00>;
>>> +        interrupt-controller;
>>> +        #interrupt-cells = <1>;
>>> +        interrupt-parent = <&gic>;
>>> +        interrupts = <0 3 4>;
>>> +    };
>>> +
>>> +    /* dw_apb_ictl is used as primary interrupt controller */
>>> +    vic: interrupt-controller@10130000 {
>>> +        compatible = "snps,dw-apb-ictl";
>>> +        reg = <0x10130000 0x1000>;
>>> +        interrupt-controller;
>>> +        #interrupt-cells = <1>;
>>> +    };
>>> -- 
>>> 1.8.3
>>>
>>>
>>
>> .
>>