lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Mon, 14 Sep 2020 17:07:20 +0800
From:   "Leizhen (ThunderTown)" <thunder.leizhen@...wei.com>
To:     Marc Zyngier <maz@...nel.org>
CC:     Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        Rob Herring <robh+dt@...nel.org>,
        devicetree <devicetree@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
        Haoyu Lv <lvhaoyu@...wei.com>, Libin <huawei.libin@...wei.com>,
        Kefeng Wang <wangkefeng.wang@...wei.com>
Subject: Re: [PATCH v3 2/3] irqchip: dw-apb-ictl: support hierarchy irq domain



On 2020/9/13 23:10, Marc Zyngier wrote:
> On Wed, 09 Sep 2020 07:58:35 +0100,
> Zhen Lei <thunder.leizhen@...wei.com> wrote:
>>
>> Add support to use dw-apb-ictl as primary interrupt controller.
>>
>> Suggested-by: Marc Zyngier <maz@...nel.org>
>> Signed-off-by: Zhen Lei <thunder.leizhen@...wei.com>
>> Tested-by: Haoyu Lv <lvhaoyu@...wei.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.
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ