linux-kernel - Re: [PATCH 5/7] irqchip/irq-bcm2836: Add support for the 7211 interrupt controller

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191002134041.5a181d96@why>
Date:   Wed, 2 Oct 2019 13:40:41 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Ray Jui <rjui@...adcom.com>,
        Scott Branden <sbranden@...adcom.com>,
        bcm-kernel-feedback-list@...adcom.com (maintainer:BROADCOM
        BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...),
        Eric Anholt <eric@...olt.net>,
        Stefan Wahren <wahrenst@....net>,
        devicetree@...r.kernel.org (open list:OPEN FIRMWARE AND FLATTENED
        DEVICE TREE BINDINGS),
        linux-rpi-kernel@...ts.infradead.org (moderated list:BROADCOM BCM2835
        ARM ARCHITECTURE),
        linux-arm-kernel@...ts.infradead.org (moderated list:BROADCOM BCM2835
        ARM ARCHITECTURE)
Subject: Re: [PATCH 5/7] irqchip/irq-bcm2836: Add support for the 7211
 interrupt controller

On Tue,  1 Oct 2019 15:48:40 -0700
Florian Fainelli <f.fainelli@...il.com> wrote:

> The root interrupt controller on 7211 is about identical to the one
> existing on BCM2836, except that the SMP cross call are done through the
> standard ARM GIC-400 interrupt controller. This interrupt controller is
> used for side band wake-up signals though.

I don't fully grasp how this thing works.

If the 7211 interrupt controller is root and the GIC is used for SGIs,
this means that the GIC outputs (IRQ/FIQ/VIRQ/VFIQ, times eight) are
connected to individual inputs to the 7211 controller. Seems totally
braindead, and unexpectedly so.

If the GIC is root and the 7211 outputs into the GIC all of its
interrupts as a secondary irqchip, it would at least match an existing
(and pretty bad) pattern.

So which one of the two is it?

> 
> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
> ---
>  drivers/irqchip/irq-bcm2836.c | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
> index 2038693f074c..77fa395c8f6b 100644
> --- a/drivers/irqchip/irq-bcm2836.c
> +++ b/drivers/irqchip/irq-bcm2836.c
> @@ -112,6 +112,8 @@ static int bcm2836_map(struct irq_domain *d, unsigned int irq,
>  		return -EINVAL;
>  	}
>  
> +	chip->flags |= IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_SKIP_SET_WAKE;
> +
>  	irq_set_percpu_devid(irq);
>  	irq_domain_set_info(d, irq, hw, chip, d->host_data,
>  			    handle_percpu_devid_irq, NULL, NULL);
> @@ -216,8 +218,9 @@ static void bcm2835_init_local_timer_frequency(void)
>  	writel(0x80000000, intc.base + LOCAL_PRESCALER);
>  }
>  
> -static int __init bcm2836_arm_irqchip_l1_intc_of_init(struct device_node *node,
> -						      struct device_node *parent)
> +static int __init arm_irqchip_l1_intc_of_init_smp(struct device_node *node,
> +						  struct device_node *parent,
> +						  bool smp_init)
>  {
>  	intc.base = of_iomap(node, 0);
>  	if (!intc.base) {
> @@ -232,11 +235,27 @@ static int __init bcm2836_arm_irqchip_l1_intc_of_init(struct device_node *node,
>  	if (!intc.domain)
>  		panic("%pOF: unable to create IRQ domain\n", node);
>  
> -	bcm2836_arm_irqchip_smp_init();
> +	if (smp_init)
> +		bcm2836_arm_irqchip_smp_init();

Instead of the additional parameter and this check, why don't you just
move the smp_init() call to bcm2836_arm_irqchip_l1_intc_of_init()
instead?

>  
>  	set_handle_irq(bcm2836_arm_irqchip_handle_irq);
> +
>  	return 0;
>  }
>  
> +static int __init bcm2836_arm_irqchip_l1_intc_of_init(struct device_node *node,
> +						      struct device_node *parent)
> +{
> +	return arm_irqchip_l1_intc_of_init_smp(node, parent, true);
> +}
> +
> +static int __init bcm7211_arm_irqchip_l1_intc_of_init(struct device_node *node,
> +						      struct device_node *parent)
> +{
> +	return arm_irqchip_l1_intc_of_init_smp(node, parent, false);
> +}
> +
>  IRQCHIP_DECLARE(bcm2836_arm_irqchip_l1_intc, "brcm,bcm2836-l1-intc",
>  		bcm2836_arm_irqchip_l1_intc_of_init);
> +IRQCHIP_DECLARE(bcm7211_arm_irqchip_l1_intc, "brcm,bcm7211-l1-intc",
> +		bcm7211_arm_irqchip_l1_intc_of_init);


Thanks,

	M.
-- 
Without deviation from the norm, progress is not possible.