Re: [PATCH 5/7] irqchip/irq-bcm2836: Add support for the 7211 interrupt controller
On Wed, 2 Oct 2019 10:06:31 -0700 Florian Fainelli wrote: > On 10/2/19 5:40 AM, Marc Zyngier wrote: > > On Tue, 1 Oct 2019 15:48:40 -0700 > > Florian Fainelli 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? > > The nominal configuration on 7211 is to have all interrupts go through > the ARM GIC. It is possible however, to fallback to the legacy 2836 mode > whereby the root interrupt controller for peripheral interrupts is this > ARMCTL IC. There is a mux that the firmware can control which will > dictate which root interrupt controller is used for peripherals. > > I have used this mostly for silicon verification and since those are > fairly harmless patches, just decided to send them out to avoid > maintaining them out of tree. This doesn't really answer my question. What I understand is that your system is laid out like this: DEVICES -> ARMCTL -> CPUs ^ GIC How are the various GIC outputs mapped into the ARMCTL? It has 4 of them per CPU (IRQ/FIQ + vIRQ/vFIQ), which the ARMCTL must somehow map to its own interrupts, specially if you want to signal IPIs using the GIC's SGIs (to which you hint in the commit log). There is a link I'm missing here. > We have a plan to use those as an "alternate" interrupt domain for low > power modes and use the fact that peripheral interrupts could be active > in both domains (GIC and ARMCTRL IC) to help support configuring and > identifying wake-up sources fro m within Linux. That's usually done with a hierarchy, where the ARMCTL IC would be a child of the GIC and see all interrupt configuration calls before they reach the GIC driver. We have plenty of examples in the tree already. Thanks, M. -- Without deviation from the norm, progress is not possible.
Re: [PATCH 5/7] irqchip/irq-bcm2836: Add support for the 7211 interrupt controller
On 10/2/19 5:40 AM, Marc Zyngier wrote:
> On Tue, 1 Oct 2019 15:48:40 -0700
> Florian Fainelli 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?
The nominal configuration on 7211 is to have all interrupts go through
the ARM GIC. It is possible however, to fallback to the legacy 2836 mode
whereby the root interrupt controller for peripheral interrupts is this
ARMCTL IC. There is a mux that the firmware can control which will
dictate which root interrupt controller is used for peripherals.
I have used this mostly for silicon verification and since those are
fairly harmless patches, just decided to send them out to avoid
maintaining them out of tree.
We have a plan to use those as an "alternate" interrupt domain for low
power modes and use the fact that peripheral interrupts could be active
in both domains (GIC and ARMCTRL IC) to help support configuring and
identifying wake-up sources fro m within Linux.
Thanks!
>
>>
>> Signed-off-by: Florian Fainelli
>> ---
>> 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(0x8000, 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?
Good idea, will do.
--
Florian
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 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
> ---
> 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(0x8000, 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.
[PATCH 5/7] irqchip/irq-bcm2836: Add support for the 7211 interrupt controller
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.
Signed-off-by: Florian Fainelli
---
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(0x8000, 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();
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);
--
2.17.1
[PATCH 5/7] irqchip/irq-bcm2836: Add support for the 7211 interrupt controller
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.
Signed-off-by: Florian Fainelli
---
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(0x8000, 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();
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);
--
2.17.1
