| 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
| ||
|
Date: Sun, 13 Sep 2020 16:10:15 +0100
From: Marc Zyngier <maz@...nel.org>
To: Zhen Lei <thunder.leizhen@...wei.com>
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 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.
> }
>
> - 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.
Powered by blists - more mailing lists