On 2020-09-08 08:11, Zhen Lei wrote:
Add support to use dw-apb-ictl as primary interrupt controller.

Suggested-by: Marc Zyngier <m...@kernel.org>
Signed-off-by: Zhen Lei <thunder.leiz...@huawei.com>
Tested-by: Haoyu Lv <lvha...@huawei.com>
---
 drivers/irqchip/Kconfig           |  2 +-
 drivers/irqchip/irq-dw-apb-ictl.c | 75 +++++++++++++++++++++++++++++--
 2 files changed, 73 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index bfc9719dbcdc..7c2d1c8fa551 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 aa6214da0b1f..405861322596 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 <asm/exception.h>

 #define APB_INT_ENABLE_L       0x00
 #define APB_INT_ENABLE_H       0x04
@@ -26,6 +27,30 @@
 #define APB_INT_FINALSTATUS_H  0x34
 #define APB_INT_BASE_OFFSET    0x04

+/*
+ * irq domain of the primary interrupt controller. Currently, only one is
+ * supported.

By definition, there is only one primary interrupt controller.

+ */
+static struct irq_domain *dw_apb_ictl_irq_domain;
+
+static void __exception_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 &= ~(1 << hwirq);

nit: prefer 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 +75,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)
 {
@@ -78,11 +127,24 @@ static int __init dw_apb_ictl_init(struct device_node *np,
        const struct irq_domain_ops *domain_ops = &irq_generic_chip_ops;
        irq_flow_handler_t flow_handler = handle_level_irq;

+       if (dw_apb_ictl_irq_domain) {
+               pr_err("%pOF: a hierarchy irq domain is already exist.\n", np);
+               return -EBUSY;

How can this happen?

+       }
+
        /* 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

Checking for an output interrupt is not the way to check for a chained
interrupt controller. That's what the parent device_node is for (no
parent or parent == self denotes a primary controller).
;
+               /* It's used as secondary interrupt controller */
+               if (of_find_property(np, "interrupts", NULL)) {
+                       pr_err("%pOF: unable to parse irq\n", np);
+                       return -EINVAL;
+               }
+
+               /* It's used as the primary interrupt controller */
+               parent_irq = 0;
+               domain_ops = &dw_apb_ictl_irq_domain_ops;
+               flow_handler = handle_fasteoi_irq;

Why? This irqchip obviously doesn't support an EOI method since you
setting it to a NOP callback below. From what I understand, this
controller should use handle_level_irq, just like its chained version.

        }

        ret = of_address_to_resource(np, 0, &r);
@@ -145,10 +207,17 @@ static int __init dw_apb_ictl_init(struct device_node *np,
                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_resume = dw_apb_ictl_resume;
+               if (!parent_irq)
+                       gc->chip_types[0].chip.irq_eoi = irq_gc_noop;
        }

-       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;

Thanks,

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

Reply via email to