On 2020/9/13 23:10, Marc Zyngier wrote:
> On Wed, 09 Sep 2020 07:58:35 +0100,
> Zhen Lei <thunder.leizhen@xxxxxxxxxx> wrote:
>>
>> Add support to use dw-apb-ictl as primary interrupt controller.
>>
>> Suggested-by: Marc Zyngier <maz@xxxxxxxxxx>
>> Signed-off-by: Zhen Lei <thunder.leizhen@xxxxxxxxxx>
>> Tested-by: Haoyu Lv <lvhaoyu@xxxxxxxxxx>
>> ---
>> drivers/irqchip/Kconfig | 2 +-
>> drivers/irqchip/irq-dw-apb-ictl.c | 76 +++++++++++++++++++++++++++----
>> 2 files changed, 69 insertions(+), 9 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 5458004242e9..3c7bebe1b947 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);
>> @@ -144,10 +197,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;
>
> Again: what is that for? The level flow doesn't use any EOI callback.
OK, I will remove it. Yes, irq_eoi is only needed by handle_fasteoi_irq().
>
>> }
>>
>> - 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);
>
> This only exists on architectures that select GENERIC_IRQ_MULTI_HANDLER,
> and yet this driver is used on some arc system (AXS10x), which doesn't
> have this option selected.
>
> Please make sure this at least builds on all supported architectures.
OK, I will intsall the gcc of arc and csky, and build it.
>
> M.
>
