[PATCH 0/2] add support for Hisilicon SD5203 vector interrupt controller

[PATCH 0/2] add support for Hisilicon SD5203 vector interrupt controller

[Zhen Lei]

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".

Kefeng Wang (1):
  irqchip: add Hisilicon SD5203 vector interrupt controller

Zhen Lei (1):
  dt-bindings: interrupt-controller: add Hisilicon SD5203 vector
    interrupt controller

 .../hisilicon,sd5203-vic.txt                  |  27 ++++
 drivers/irqchip/Kconfig                       |   5 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-sd5203-vic.c              | 128 ++++++++++++++++++
 4 files changed, 161 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt
 create mode 100644 drivers/irqchip/irq-sd5203-vic.c

-- 
2.26.0.106.g9fadedd

[PATCH 1/2] dt-bindings: interrupt-controller: add Hisilicon SD5203 vector interrupt controller

[Zhen Lei]

Add DT bindings for the Hisilicon SD5203 vector interrupt controller.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 .../hisilicon,sd5203-vic.txt                  | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt

diff --git a/Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt b/Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt
new file mode 100644
index 000000000000..a08292e868b0
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt
@@ -0,0 +1,27 @@
+Hisilicon SD5203 vector interrupt controller (VIC)
+
+Hisilicon SD5203 VIC based on Synopsys DesignWare APB interrupt controller, but
+there's something special:
+1. The maximum number of irqs supported is 32. The registers ENABLE, MASK and
+   FINALSTATUS are 32 bits.
+2. There is only one VIC, it's used as primary interrupt controller.
+
+Required properties:
+- compatible: shall be "hisilicon,sd5203-vic"
+- 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
+
+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,
+
+Example:
+	vic: interrupt-controller@10130000 {
+		compatible = "hisilicon,sd5203-vic";
+		reg = <0x10130000 0x1000>;
+		interrupt-controller;
+		#interrupt-cells = <1>;
+	};
-- 
2.26.0.106.g9fadedd

Re: [PATCH 1/2] dt-bindings: interrupt-controller: add Hisilicon SD5203 vector interrupt controller

[Rob Herring]

On Thu, Sep 03, 2020 at 08:05:03PM +0800, Zhen Lei wrote:
> Add DT bindings for the Hisilicon SD5203 vector interrupt controller.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  .../hisilicon,sd5203-vic.txt                  | 27 +++++++++++++++++++

Bindings should be in DT schema format now.

>  1 file changed, 27 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt b/Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt
> new file mode 100644
> index 000000000000..a08292e868b0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt
> @@ -0,0 +1,27 @@
> +Hisilicon SD5203 vector interrupt controller (VIC)
> +
> +Hisilicon SD5203 VIC based on Synopsys DesignWare APB interrupt controller, but
> +there's something special:
> +1. The maximum number of irqs supported is 32. The registers ENABLE, MASK and
> +   FINALSTATUS are 32 bits.
> +2. There is only one VIC, it's used as primary interrupt controller.
> +
> +Required properties:
> +- compatible: shall be "hisilicon,sd5203-vic"
> +- 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
> +
> +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,
> +
> +Example:
> +	vic: interrupt-controller@10130000 {
> +		compatible = "hisilicon,sd5203-vic";
> +		reg = <0x10130000 0x1000>;
> +		interrupt-controller;
> +		#interrupt-cells = <1>;
> +	};
> -- 
> 2.26.0.106.g9fadedd
> 
> 

Re: [PATCH 1/2] dt-bindings: interrupt-controller: add Hisilicon SD5203 vector interrupt controller

[Leizhen (ThunderTown)]

On 2020/9/15 4:31, Rob Herring wrote:
> On Thu, Sep 03, 2020 at 08:05:03PM +0800, Zhen Lei wrote:
>> Add DT bindings for the Hisilicon SD5203 vector interrupt controller.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  .../hisilicon,sd5203-vic.txt                  | 27 +++++++++++++++++++
> 
> Bindings should be in DT schema format now.

Hi, Rob Herring:

As Marc Zyngier's suggestion, I discarded adding an independent SD5203-VIC
driver, but make the dw-apb-ictl irqchip driver to support hierarchy irq domain.
So this new file was also dropped. Now, I updated the descriptions in the existing
file "snps,dw-apb-ictl.txt" in the following versions.

> 
>>  1 file changed, 27 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt
>>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt b/Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt
>> new file mode 100644
>> index 000000000000..a08292e868b0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt
>> @@ -0,0 +1,27 @@
>> +Hisilicon SD5203 vector interrupt controller (VIC)
>> +
>> +Hisilicon SD5203 VIC based on Synopsys DesignWare APB interrupt controller, but
>> +there's something special:
>> +1. The maximum number of irqs supported is 32. The registers ENABLE, MASK and
>> +   FINALSTATUS are 32 bits.
>> +2. There is only one VIC, it's used as primary interrupt controller.
>> +
>> +Required properties:
>> +- compatible: shall be "hisilicon,sd5203-vic"
>> +- 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
>> +
>> +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,
>> +
>> +Example:
>> +	vic: interrupt-controller@10130000 {
>> +		compatible = "hisilicon,sd5203-vic";
>> +		reg = <0x10130000 0x1000>;
>> +		interrupt-controller;
>> +		#interrupt-cells = <1>;
>> +	};
>> -- 
>> 2.26.0.106.g9fadedd
>>
>>
> 
> .
> 

Re: [PATCH 1/2] dt-bindings: interrupt-controller: add Hisilicon SD5203 vector interrupt controller

[Leizhen (ThunderTown)]

On 2020/9/15 14:12, Leizhen (ThunderTown) wrote:
> 
> 
> On 2020/9/15 4:31, Rob Herring wrote:
>> On Thu, Sep 03, 2020 at 08:05:03PM +0800, Zhen Lei wrote:
>>> Add DT bindings for the Hisilicon SD5203 vector interrupt controller.
>>>
>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>>> ---
>>>  .../hisilicon,sd5203-vic.txt                  | 27 +++++++++++++++++++
>>
>> Bindings should be in DT schema format now.

Do I need to change the existing "snps,dw-apb-ictl.txt" to DT schema format?

> 
> Hi, Rob Herring:
> 
> As Marc Zyngier's suggestion, I discarded adding an independent SD5203-VIC
> driver, but make the dw-apb-ictl irqchip driver to support hierarchy irq domain.
> So this new file was also dropped. Now, I updated the descriptions in the existing
> file "snps,dw-apb-ictl.txt" in the following versions.
> 
>>
>>>  1 file changed, 27 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt b/Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt
>>> new file mode 100644
>>> index 000000000000..a08292e868b0
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/hisilicon,sd5203-vic.txt
>>> @@ -0,0 +1,27 @@
>>> +Hisilicon SD5203 vector interrupt controller (VIC)
>>> +
>>> +Hisilicon SD5203 VIC based on Synopsys DesignWare APB interrupt controller, but
>>> +there's something special:
>>> +1. The maximum number of irqs supported is 32. The registers ENABLE, MASK and
>>> +   FINALSTATUS are 32 bits.
>>> +2. There is only one VIC, it's used as primary interrupt controller.
>>> +
>>> +Required properties:
>>> +- compatible: shall be "hisilicon,sd5203-vic"
>>> +- 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
>>> +
>>> +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,
>>> +
>>> +Example:
>>> +	vic: interrupt-controller@10130000 {
>>> +		compatible = "hisilicon,sd5203-vic";
>>> +		reg = <0x10130000 0x1000>;
>>> +		interrupt-controller;
>>> +		#interrupt-cells = <1>;
>>> +	};
>>> -- 
>>> 2.26.0.106.g9fadedd
>>>
>>>
>>
>> .
>>

Re: [PATCH 1/2] dt-bindings: interrupt-controller: add Hisilicon SD5203 vector interrupt controller

[Rob Herring]

On Wed, Sep 16, 2020 at 9:30 PM Leizhen (ThunderTown)
<thunder.leizhen@huawei.com> wrote:
>
>
>
> On 2020/9/15 14:12, Leizhen (ThunderTown) wrote:
> >
> >
> > On 2020/9/15 4:31, Rob Herring wrote:
> >> On Thu, Sep 03, 2020 at 08:05:03PM +0800, Zhen Lei wrote:
> >>> Add DT bindings for the Hisilicon SD5203 vector interrupt controller.
> >>>
> >>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> >>> ---
> >>>  .../hisilicon,sd5203-vic.txt                  | 27 +++++++++++++++++++
> >>
> >> Bindings should be in DT schema format now.
>
> Do I need to change the existing "snps,dw-apb-ictl.txt" to DT schema format?

That would be nice, but not necessary for minor changes.

Rob

Re: [PATCH 1/2] dt-bindings: interrupt-controller: add Hisilicon SD5203 vector interrupt controller

[Leizhen (ThunderTown)]

On 2020/9/19 2:55, Rob Herring wrote:
> On Wed, Sep 16, 2020 at 9:30 PM Leizhen (ThunderTown)
> <thunder.leizhen@huawei.com> wrote:
>>
>>
>>
>> On 2020/9/15 14:12, Leizhen (ThunderTown) wrote:
>>>
>>>
>>> On 2020/9/15 4:31, Rob Herring wrote:
>>>> On Thu, Sep 03, 2020 at 08:05:03PM +0800, Zhen Lei wrote:
>>>>> Add DT bindings for the Hisilicon SD5203 vector interrupt controller.
>>>>>
>>>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>>>>> ---
>>>>>  .../hisilicon,sd5203-vic.txt                  | 27 +++++++++++++++++++
>>>>
>>>> Bindings should be in DT schema format now.
>>
>> Do I need to change the existing "snps,dw-apb-ictl.txt" to DT schema format?
> 
> That would be nice, but not necessary for minor changes.

I had done it before you replied to me. Can you check it out?

https://www.spinics.net/lists/devicetree/msg378632.html

> 
> Rob
> 
> .
> 

[PATCH 2/2] irqchip: add Hisilicon SD5203 vector interrupt controller

[Zhen Lei]

From: Kefeng Wang <wangkefeng.wang@huawei.com>

This adds an irqchip driver and corresponding devicetree binding for the
primary interrupt controller based on Hisilicon SD5203 VIC(vector
interrupt controller).

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/irqchip/Kconfig          |   5 ++
 drivers/irqchip/Makefile         |   1 +
 drivers/irqchip/irq-sd5203-vic.c | 128 +++++++++++++++++++++++++++++++
 3 files changed, 134 insertions(+)
 create mode 100644 drivers/irqchip/irq-sd5203-vic.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index bfc9719dbcdc..f64611a47cf2 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -156,6 +156,11 @@ config FARADAY_FTINTC010
 	select GENERIC_IRQ_MULTI_HANDLER
 	select SPARSE_IRQ
 
+config HISILICON_SD5203_VIC
+	bool
+	select GENERIC_IRQ_CHIP
+	select IRQ_DOMAIN_HIERARCHY
+
 config HISILICON_IRQ_MBIGEN
 	bool
 	select ARM_GIC_V3
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 133f9c45744a..94b98f881940 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -35,6 +35,7 @@ obj-$(CONFIG_ARM_GIC_V3_ITS)		+= irq-gic-v3-its.o irq-gic-v3-its-platform-msi.o
 obj-$(CONFIG_ARM_GIC_V3_ITS_PCI)	+= irq-gic-v3-its-pci-msi.o
 obj-$(CONFIG_ARM_GIC_V3_ITS_FSL_MC)	+= irq-gic-v3-its-fsl-mc-msi.o
 obj-$(CONFIG_PARTITION_PERCPU)		+= irq-partition-percpu.o
+obj-$(CONFIG_HISILICON_SD5203_VIC)	+= irq-sd5203-vic.o
 obj-$(CONFIG_HISILICON_IRQ_MBIGEN)	+= irq-mbigen.o
 obj-$(CONFIG_ARM_NVIC)			+= irq-nvic.o
 obj-$(CONFIG_ARM_VIC)			+= irq-vic.o
diff --git a/drivers/irqchip/irq-sd5203-vic.c b/drivers/irqchip/irq-sd5203-vic.c
new file mode 100644
index 000000000000..f6f8fd75f1ab
--- /dev/null
+++ b/drivers/irqchip/irq-sd5203-vic.c
@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Hisilicon SD5203 irqchip driver.
+ * Based on Synopsys DW APB ICTL irqchip driver.
+ *
+ */
+
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <asm/exception.h>
+
+#define HISI_VIC_INT_ENABLE		0x00
+#define HISI_VIC_INT_MASK		0x08
+#define HISI_VIC_INT_FINALSTATUS	0x30
+#define HISI_VIC_MAX_IRQ		32
+
+static struct irq_domain *hisi_vic_irq_domain;
+static void __iomem *hisi_vic_iobase;
+
+static void __exception_irq_entry hisi_vic_handle_irq(struct pt_regs *regs)
+{
+	u32 stat = readl_relaxed(hisi_vic_iobase + HISI_VIC_INT_FINALSTATUS);
+	u32 hwirq;
+
+	while (stat) {
+		hwirq = fls(stat) - 1;
+		handle_domain_irq(hisi_vic_irq_domain, hwirq, regs);
+		stat &= ~BIT(hwirq);
+	}
+}
+
+static int hisi_vic_irq_domain_translate(struct irq_domain *d,
+					 struct irq_fwspec *fwspec,
+					 unsigned long *hwirq,
+					 unsigned int *type)
+{
+	if (WARN_ON(fwspec->param_count < 1))
+		return -EINVAL;
+
+	*hwirq = fwspec->param[0];
+	*type = IRQ_TYPE_NONE;
+
+	return 0;
+}
+
+static int hisi_vic_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 = hisi_vic_irq_domain_translate(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 hisi_vic_irq_domain_ops = {
+	.translate = hisi_vic_irq_domain_translate,
+	.alloc = hisi_vic_irq_domain_alloc,
+	.free = irq_domain_free_irqs_top,
+};
+
+static int __init hisi_sd5203_vic_init(struct device_node *np,
+					struct device_node *parent)
+{
+	unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
+	struct irq_chip_generic *gc;
+	int ret;
+
+	if (hisi_vic_iobase) {
+		pr_err("%pOF: the irqchip has been registered.\n", np);
+		return -EBUSY;
+	}
+
+	hisi_vic_iobase = of_iomap(np, 0);
+	if (!hisi_vic_iobase) {
+		pr_err("%pOF: unable to map resource\n", np);
+		return -ENOMEM;
+	}
+
+	hisi_vic_irq_domain = irq_domain_add_linear(np, HISI_VIC_MAX_IRQ,
+				       &hisi_vic_irq_domain_ops, NULL);
+	if (!hisi_vic_irq_domain) {
+		pr_err("%pOF: unable to add irq domain\n", np);
+		ret = -ENOMEM;
+		goto err_unmap;
+	}
+
+	ret = irq_alloc_domain_generic_chips(hisi_vic_irq_domain,
+					     HISI_VIC_MAX_IRQ, 1,
+					     np->name, handle_fasteoi_irq,
+					     clr, 0, IRQ_GC_INIT_MASK_CACHE);
+	if (ret) {
+		pr_err("%pOF: unable to alloc irq domain gc\n", np);
+		goto err_unmap;
+	}
+
+	/* mask and enable all interrupts */
+	writel_relaxed(~0, hisi_vic_iobase + HISI_VIC_INT_MASK);
+	writel_relaxed(~0, hisi_vic_iobase + HISI_VIC_INT_ENABLE);
+
+	gc = irq_get_domain_generic_chip(hisi_vic_irq_domain, 0);
+	gc->reg_base = hisi_vic_iobase;
+	gc->chip_types[0].regs.mask = HISI_VIC_INT_MASK;
+	gc->chip_types[0].regs.enable = HISI_VIC_INT_ENABLE;
+	gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
+	gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
+	gc->chip_types[0].chip.irq_eoi = irq_gc_noop;
+
+	set_handle_irq(hisi_vic_handle_irq);
+	return 0;
+
+err_unmap:
+	iounmap(hisi_vic_iobase);
+	return ret;
+}
+IRQCHIP_DECLARE(hisi_sd5203_vic, "hisilicon,sd5203-vic", hisi_sd5203_vic_init);
-- 
2.26.0.106.g9fadedd

Re: [PATCH 0/2] add support for Hisilicon SD5203 vector interrupt controller

[Marc Zyngier]

On 2020-09-03 13:05, Zhen Lei wrote:
> 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.

What prevents you from improving the existing driver so that it can act
as a primary interrupt controller? It shouldn't be rocket science, 
really.

There are some examples in the tree of drivers that can be used in
both situations (GIC, VIC

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 0/2] add support for Hisilicon SD5203 vector interrupt controller

[Leizhen (ThunderTown)]

On 2020/9/4 0:46, Marc Zyngier wrote:
> On 2020-09-03 13:05, Zhen Lei wrote:
>> 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.
> 
> What prevents you from improving the existing driver so that it can act
> as a primary interrupt controller? It shouldn't be rocket science, really.
> 
> There are some examples in the tree of drivers that can be used in
> both situations (GIC, VIC

OK, thanks for the tip.

> 
> Thanks,
> 
>         M.