diff mbox series

[v3,2/3] irqchip: dw-apb-ictl: support hierarchy irq domain

Message ID 20200909065836.2631-3-thunder.leizhen@huawei.com
State New
Headers show
Series irqchip: dw-apb-ictl: support hierarchy irq domain | expand

Commit Message

Leizhen (ThunderTown) Sept. 9, 2020, 6:58 a.m. UTC
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 | 76 +++++++++++++++++++++++++++----
 2 files changed, 69 insertions(+), 9 deletions(-)

-- 
2.26.0.106.g9fadedd

Comments

Marc Zyngier Sept. 13, 2020, 3:10 p.m. UTC | #1
On Wed, 09 Sep 2020 07:58:35 +0100,
Zhen Lei <thunder.leizhen@huawei.com> wrote:
> 

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

>  	}

>  

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

	M.

-- 
Without deviation from the norm, progress is not possible.
Leizhen (ThunderTown) Sept. 14, 2020, 9:07 a.m. UTC | #2
On 2020/9/13 23:10, Marc Zyngier wrote:
> On Wed, 09 Sep 2020 07:58:35 +0100,

> Zhen Lei <thunder.leizhen@huawei.com> wrote:

>>

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

>
diff mbox series

Patch

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