Raspberry Pi 2 interrupt controller support (v2) Second round

Raspberry Pi 2 interrupt controller support (v2) Second round

[Eric Anholt]

As before, the full series you can build and test can be found at:

https://github.com/anholt/linux/tree/bcm2836-irqchip

Raspberry Pi 2 interrupt controller support (v2) Second round

[Eric Anholt]

As before, the full series you can build and test can be found at:

https://github.com/anholt/linux/tree/bcm2836-irqchip

Raspberry Pi 2 interrupt controller support (v2) Second round

[Eric Anholt]

As before, the full series you can build and test can be found at:

https://github.com/anholt/linux/tree/bcm2836-irqchip

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/4] irqchip: bcm2835: Refactor handle_IRQ() calls out of MAKE_HWIRQ.

[Eric Anholt]

For BCM2836, we want to chain into this IRQ chip from the root
controller, and for chaining we need to do something else instead of
handle_IRQ() once we have decoded the IRQ.

Note that this changes the behavior a little bit: Previously for a
non-shortcut IRQ, we'd loop reading and handling the second level IRQ
status until it was cleared before returning to the loop reading the
top level IRQ status (Note that the top level bit is just an OR of the
low level bits).  For the expected case of just one interrupt to be
handled, this was an extra register read, so we're down from 4 to 3
reads.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/irqchip/irq-bcm2835.c | 57 ++++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 28 deletions(-)

diff --git a/drivers/irqchip/irq-bcm2835.c b/drivers/irqchip/irq-bcm2835.c
index e68c3b6..382450a 100644
--- a/drivers/irqchip/irq-bcm2835.c
+++ b/drivers/irqchip/irq-bcm2835.c
@@ -179,44 +179,45 @@ static int __init armctrl_of_init(struct device_node *node,
  * handle_IRQ may briefly re-enable interrupts for soft IRQ handling.
  */
 
-static void armctrl_handle_bank(int bank, struct pt_regs *regs)
+static u32 armctrl_translate_bank(int bank)
 {
-	u32 stat, irq;
+	u32 stat = readl_relaxed(intc.pending[bank]);
 
-	while ((stat = readl_relaxed(intc.pending[bank]))) {
-		irq = MAKE_HWIRQ(bank, ffs(stat) - 1);
-		handle_IRQ(irq_linear_revmap(intc.domain, irq), regs);
-	}
+	return MAKE_HWIRQ(bank, ffs(stat) - 1);
+}
+
+static u32 armctrl_translate_shortcut(int bank, u32 stat)
+{
+	return MAKE_HWIRQ(bank, shortcuts[ffs(stat >> SHORTCUT_SHIFT) - 1]);
 }
 
-static void armctrl_handle_shortcut(int bank, struct pt_regs *regs,
-	u32 stat)
+static u32 get_next_armctrl_hwirq(void)
 {
-	u32 irq = MAKE_HWIRQ(bank, shortcuts[ffs(stat >> SHORTCUT_SHIFT) - 1]);
-	handle_IRQ(irq_linear_revmap(intc.domain, irq), regs);
+	u32 stat = readl_relaxed(intc.pending[0]) & BANK0_VALID_MASK;
+
+	if (stat == 0)
+		return ~0;
+	else if (stat & BANK0_HWIRQ_MASK)
+		return MAKE_HWIRQ(0, ffs(stat & BANK0_HWIRQ_MASK) - 1);
+	else if (stat & SHORTCUT1_MASK)
+		return armctrl_translate_shortcut(1, stat & SHORTCUT1_MASK);
+	else if (stat & SHORTCUT2_MASK)
+		return armctrl_translate_shortcut(2, stat & SHORTCUT2_MASK);
+	else if (stat & BANK1_HWIRQ)
+		return armctrl_translate_bank(1);
+	else if (stat & BANK2_HWIRQ)
+		return armctrl_translate_bank(2);
+	else
+		BUG();
 }
 
 static void __exception_irq_entry bcm2835_handle_irq(
 	struct pt_regs *regs)
 {
-	u32 stat, irq;
-
-	while ((stat = readl_relaxed(intc.pending[0]) & BANK0_VALID_MASK)) {
-		if (stat & BANK0_HWIRQ_MASK) {
-			irq = MAKE_HWIRQ(0, ffs(stat & BANK0_HWIRQ_MASK) - 1);
-			handle_IRQ(irq_linear_revmap(intc.domain, irq), regs);
-		} else if (stat & SHORTCUT1_MASK) {
-			armctrl_handle_shortcut(1, regs, stat & SHORTCUT1_MASK);
-		} else if (stat & SHORTCUT2_MASK) {
-			armctrl_handle_shortcut(2, regs, stat & SHORTCUT2_MASK);
-		} else if (stat & BANK1_HWIRQ) {
-			armctrl_handle_bank(1, regs);
-		} else if (stat & BANK2_HWIRQ) {
-			armctrl_handle_bank(2, regs);
-		} else {
-			BUG();
-		}
-	}
+	u32 hwirq;
+
+	while ((hwirq = get_next_armctrl_hwirq()) != ~0)
+		handle_IRQ(irq_linear_revmap(intc.domain, hwirq), regs);
 }
 
 IRQCHIP_DECLARE(bcm2835_armctrl_ic, "brcm,bcm2835-armctrl-ic", armctrl_of_init);
-- 
2.1.4

[PATCH v2 1/4] irqchip: bcm2835: Refactor handle_IRQ() calls out of MAKE_HWIRQ.

[Eric Anholt]

For BCM2836, we want to chain into this IRQ chip from the root
controller, and for chaining we need to do something else instead of
handle_IRQ() once we have decoded the IRQ.

Note that this changes the behavior a little bit: Previously for a
non-shortcut IRQ, we'd loop reading and handling the second level IRQ
status until it was cleared before returning to the loop reading the
top level IRQ status (Note that the top level bit is just an OR of the
low level bits).  For the expected case of just one interrupt to be
handled, this was an extra register read, so we're down from 4 to 3
reads.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/irqchip/irq-bcm2835.c | 57 ++++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 28 deletions(-)

diff --git a/drivers/irqchip/irq-bcm2835.c b/drivers/irqchip/irq-bcm2835.c
index e68c3b6..382450a 100644
--- a/drivers/irqchip/irq-bcm2835.c
+++ b/drivers/irqchip/irq-bcm2835.c
@@ -179,44 +179,45 @@ static int __init armctrl_of_init(struct device_node *node,
  * handle_IRQ may briefly re-enable interrupts for soft IRQ handling.
  */
 
-static void armctrl_handle_bank(int bank, struct pt_regs *regs)
+static u32 armctrl_translate_bank(int bank)
 {
-	u32 stat, irq;
+	u32 stat = readl_relaxed(intc.pending[bank]);
 
-	while ((stat = readl_relaxed(intc.pending[bank]))) {
-		irq = MAKE_HWIRQ(bank, ffs(stat) - 1);
-		handle_IRQ(irq_linear_revmap(intc.domain, irq), regs);
-	}
+	return MAKE_HWIRQ(bank, ffs(stat) - 1);
+}
+
+static u32 armctrl_translate_shortcut(int bank, u32 stat)
+{
+	return MAKE_HWIRQ(bank, shortcuts[ffs(stat >> SHORTCUT_SHIFT) - 1]);
 }
 
-static void armctrl_handle_shortcut(int bank, struct pt_regs *regs,
-	u32 stat)
+static u32 get_next_armctrl_hwirq(void)
 {
-	u32 irq = MAKE_HWIRQ(bank, shortcuts[ffs(stat >> SHORTCUT_SHIFT) - 1]);
-	handle_IRQ(irq_linear_revmap(intc.domain, irq), regs);
+	u32 stat = readl_relaxed(intc.pending[0]) & BANK0_VALID_MASK;
+
+	if (stat == 0)
+		return ~0;
+	else if (stat & BANK0_HWIRQ_MASK)
+		return MAKE_HWIRQ(0, ffs(stat & BANK0_HWIRQ_MASK) - 1);
+	else if (stat & SHORTCUT1_MASK)
+		return armctrl_translate_shortcut(1, stat & SHORTCUT1_MASK);
+	else if (stat & SHORTCUT2_MASK)
+		return armctrl_translate_shortcut(2, stat & SHORTCUT2_MASK);
+	else if (stat & BANK1_HWIRQ)
+		return armctrl_translate_bank(1);
+	else if (stat & BANK2_HWIRQ)
+		return armctrl_translate_bank(2);
+	else
+		BUG();
 }
 
 static void __exception_irq_entry bcm2835_handle_irq(
 	struct pt_regs *regs)
 {
-	u32 stat, irq;
-
-	while ((stat = readl_relaxed(intc.pending[0]) & BANK0_VALID_MASK)) {
-		if (stat & BANK0_HWIRQ_MASK) {
-			irq = MAKE_HWIRQ(0, ffs(stat & BANK0_HWIRQ_MASK) - 1);
-			handle_IRQ(irq_linear_revmap(intc.domain, irq), regs);
-		} else if (stat & SHORTCUT1_MASK) {
-			armctrl_handle_shortcut(1, regs, stat & SHORTCUT1_MASK);
-		} else if (stat & SHORTCUT2_MASK) {
-			armctrl_handle_shortcut(2, regs, stat & SHORTCUT2_MASK);
-		} else if (stat & BANK1_HWIRQ) {
-			armctrl_handle_bank(1, regs);
-		} else if (stat & BANK2_HWIRQ) {
-			armctrl_handle_bank(2, regs);
-		} else {
-			BUG();
-		}
-	}
+	u32 hwirq;
+
+	while ((hwirq = get_next_armctrl_hwirq()) != ~0)
+		handle_IRQ(irq_linear_revmap(intc.domain, hwirq), regs);
 }
 
 IRQCHIP_DECLARE(bcm2835_armctrl_ic, "brcm,bcm2835-armctrl-ic", armctrl_of_init);
-- 
2.1.4

[PATCH v2 2/4] irqchip: bcm2835: If a parent interrupt is registered, chain from it.

[Eric Anholt]

The BCM2836 (Raspberry Pi 2) uses two levels of interrupt handling
with the CPU-local interrupts being the root, so we need to register
ours as chained off of the CPU's local interrupt.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 .../brcm,bcm2835-armctrl-ic.txt                    | 22 ++++++++++++++++++++++
 drivers/irqchip/irq-bcm2835.c                      | 20 ++++++++++++++++++--
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
index 7da578d..8363bc4 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
@@ -5,6 +5,10 @@ The BCM2835 contains a custom top-level interrupt controller, which supports
 controller, or the HW block containing it, is referred to occasionally
 as "armctrl" in the SoC documentation, hence naming of this binding.
 
+The BCM2836 contains the same interrupt controller with the same
+interrupts, but the per-CPU interrupt controller is the root, and an
+interrupt there indicates that the ARMCTRL has an interrupt to handle.
+
 Required properties:
 
 - compatible : should be "brcm,bcm2835-armctrl-ic"
@@ -20,6 +24,12 @@ Required properties:
   The 2nd cell contains the interrupt number within the bank. Valid values
   are 0..7 for bank 0, and 0..31 for bank 1.
 
+Optional properties:
+- interrupt-parent : Specifies the parent interrupt controller when this
+  controller is the second level.
+- interrupts : Specifies the interrupt on the parent for this interrupt
+  controller to handle.
+
 The interrupt sources are as follows:
 
 Bank 0:
@@ -102,9 +112,21 @@ Bank 2:
 
 Example:
 
+/* BCM2835, first level */
+intc: interrupt-controller {
+	compatible = "brcm,bcm2835-armctrl-ic";
+	reg = <0x7e00b200 0x200>;
+	interrupt-controller;
+	#interrupt-cells = <2>;
+};
+
+/* BCM2836, second level */
 intc: interrupt-controller {
 	compatible = "brcm,bcm2835-armctrl-ic";
 	reg = <0x7e00b200 0x200>;
 	interrupt-controller;
 	#interrupt-cells = <2>;
+
+	interrupt-parent = <&local_intc>;
+	interrupts = <8>;
 };
diff --git a/drivers/irqchip/irq-bcm2835.c b/drivers/irqchip/irq-bcm2835.c
index 382450a..dc6b159 100644
--- a/drivers/irqchip/irq-bcm2835.c
+++ b/drivers/irqchip/irq-bcm2835.c
@@ -97,6 +97,7 @@ struct armctrl_ic {
 static struct armctrl_ic intc __read_mostly;
 static void __exception_irq_entry bcm2835_handle_irq(
 	struct pt_regs *regs);
+static void bcm2835_chained_handle_irq(unsigned int irq, struct irq_desc *desc);
 
 static void armctrl_mask_irq(struct irq_data *d)
 {
@@ -143,7 +144,7 @@ static int __init armctrl_of_init(struct device_node *node,
 	struct device_node *parent)
 {
 	void __iomem *base;
-	int irq, b, i;
+	int irq, parent_irq, b, i;
 
 	base = of_iomap(node, 0);
 	if (!base)
@@ -169,7 +170,14 @@ static int __init armctrl_of_init(struct device_node *node,
 		}
 	}
 
-	set_handle_irq(bcm2835_handle_irq);
+	parent_irq = irq_of_parse_and_map(node, 0);
+	if (!parent_irq) {
+		/* No parent IRQ, so we're the root interrupt controller */
+		set_handle_irq(bcm2835_handle_irq);
+	} else {
+		irq_set_chained_handler(parent_irq, bcm2835_chained_handle_irq);
+	}
+
 	return 0;
 }
 
@@ -220,4 +228,12 @@ static void __exception_irq_entry bcm2835_handle_irq(
 		handle_IRQ(irq_linear_revmap(intc.domain, hwirq), regs);
 }
 
+static void bcm2835_chained_handle_irq(unsigned int irq, struct irq_desc *desc)
+{
+	u32 hwirq;
+
+	while ((hwirq = get_next_armctrl_hwirq()) != ~0)
+		generic_handle_irq(irq_linear_revmap(intc.domain, hwirq));
+}
+
 IRQCHIP_DECLARE(bcm2835_armctrl_ic, "brcm,bcm2835-armctrl-ic", armctrl_of_init);
-- 
2.1.4

[PATCH v2 2/4] irqchip: bcm2835: If a parent interrupt is registered, chain from it.

[Eric Anholt]

The BCM2836 (Raspberry Pi 2) uses two levels of interrupt handling
with the CPU-local interrupts being the root, so we need to register
ours as chained off of the CPU's local interrupt.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 .../brcm,bcm2835-armctrl-ic.txt                    | 22 ++++++++++++++++++++++
 drivers/irqchip/irq-bcm2835.c                      | 20 ++++++++++++++++++--
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
index 7da578d..8363bc4 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
@@ -5,6 +5,10 @@ The BCM2835 contains a custom top-level interrupt controller, which supports
 controller, or the HW block containing it, is referred to occasionally
 as "armctrl" in the SoC documentation, hence naming of this binding.
 
+The BCM2836 contains the same interrupt controller with the same
+interrupts, but the per-CPU interrupt controller is the root, and an
+interrupt there indicates that the ARMCTRL has an interrupt to handle.
+
 Required properties:
 
 - compatible : should be "brcm,bcm2835-armctrl-ic"
@@ -20,6 +24,12 @@ Required properties:
   The 2nd cell contains the interrupt number within the bank. Valid values
   are 0..7 for bank 0, and 0..31 for bank 1.
 
+Optional properties:
+- interrupt-parent : Specifies the parent interrupt controller when this
+  controller is the second level.
+- interrupts : Specifies the interrupt on the parent for this interrupt
+  controller to handle.
+
 The interrupt sources are as follows:
 
 Bank 0:
@@ -102,9 +112,21 @@ Bank 2:
 
 Example:
 
+/* BCM2835, first level */
+intc: interrupt-controller {
+	compatible = "brcm,bcm2835-armctrl-ic";
+	reg = <0x7e00b200 0x200>;
+	interrupt-controller;
+	#interrupt-cells = <2>;
+};
+
+/* BCM2836, second level */
 intc: interrupt-controller {
 	compatible = "brcm,bcm2835-armctrl-ic";
 	reg = <0x7e00b200 0x200>;
 	interrupt-controller;
 	#interrupt-cells = <2>;
+
+	interrupt-parent = <&local_intc>;
+	interrupts = <8>;
 };
diff --git a/drivers/irqchip/irq-bcm2835.c b/drivers/irqchip/irq-bcm2835.c
index 382450a..dc6b159 100644
--- a/drivers/irqchip/irq-bcm2835.c
+++ b/drivers/irqchip/irq-bcm2835.c
@@ -97,6 +97,7 @@ struct armctrl_ic {
 static struct armctrl_ic intc __read_mostly;
 static void __exception_irq_entry bcm2835_handle_irq(
 	struct pt_regs *regs);
+static void bcm2835_chained_handle_irq(unsigned int irq, struct irq_desc *desc);
 
 static void armctrl_mask_irq(struct irq_data *d)
 {
@@ -143,7 +144,7 @@ static int __init armctrl_of_init(struct device_node *node,
 	struct device_node *parent)
 {
 	void __iomem *base;
-	int irq, b, i;
+	int irq, parent_irq, b, i;
 
 	base = of_iomap(node, 0);
 	if (!base)
@@ -169,7 +170,14 @@ static int __init armctrl_of_init(struct device_node *node,
 		}
 	}
 
-	set_handle_irq(bcm2835_handle_irq);
+	parent_irq = irq_of_parse_and_map(node, 0);
+	if (!parent_irq) {
+		/* No parent IRQ, so we're the root interrupt controller */
+		set_handle_irq(bcm2835_handle_irq);
+	} else {
+		irq_set_chained_handler(parent_irq, bcm2835_chained_handle_irq);
+	}
+
 	return 0;
 }
 
@@ -220,4 +228,12 @@ static void __exception_irq_entry bcm2835_handle_irq(
 		handle_IRQ(irq_linear_revmap(intc.domain, hwirq), regs);
 }
 
+static void bcm2835_chained_handle_irq(unsigned int irq, struct irq_desc *desc)
+{
+	u32 hwirq;
+
+	while ((hwirq = get_next_armctrl_hwirq()) != ~0)
+		generic_handle_irq(irq_linear_revmap(intc.domain, hwirq));
+}
+
 IRQCHIP_DECLARE(bcm2835_armctrl_ic, "brcm,bcm2835-armctrl-ic", armctrl_of_init);
-- 
2.1.4

[PATCH v2 2/4] irqchip: bcm2835: If a parent interrupt is registered, chain from it.

[Stephen Warren]

On 07/13/2015 07:35 PM, Eric Anholt wrote:
> The BCM2836 (Raspberry Pi 2) uses two levels of interrupt handling
> with the CPU-local interrupts being the root, so we need to register
> ours as chained off of the CPU's local interrupt.

Sorry for the slow review; laziness after vacation!

> diff --git a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt

> +The BCM2836 contains the same interrupt controller with the same
> +interrupts, but the per-CPU interrupt controller is the root, and an
> +interrupt there indicates that the ARMCTRL has an interrupt to handle.
> +
>  Required properties:
>  
>  - compatible : should be "brcm,bcm2835-armctrl-ic"

Since there are some differences between the bcm2835 and bcm2836 HW
blocks, I'd expect the compatible value to be different for each. In
particular...

> +Optional properties:
> +- interrupt-parent : Specifies the parent interrupt controller when this
> +  controller is the second level.
> +- interrupts : Specifies the interrupt on the parent for this interrupt
> +  controller to handle.

I'd classify that as "additional required properties for
brcm,bcm2836-armctrl-ic"

... and with different compatible values for the two chips, you would
know when probe() should require vs. reject the property.

Re: [PATCH v2 2/4] irqchip: bcm2835: If a parent interrupt is registered, chain from it.

[Stephen Warren]

On 07/13/2015 07:35 PM, Eric Anholt wrote:
> The BCM2836 (Raspberry Pi 2) uses two levels of interrupt handling
> with the CPU-local interrupts being the root, so we need to register
> ours as chained off of the CPU's local interrupt.

Sorry for the slow review; laziness after vacation!

> diff --git a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt

> +The BCM2836 contains the same interrupt controller with the same
> +interrupts, but the per-CPU interrupt controller is the root, and an
> +interrupt there indicates that the ARMCTRL has an interrupt to handle.
> +
>  Required properties:
>  
>  - compatible : should be "brcm,bcm2835-armctrl-ic"

Since there are some differences between the bcm2835 and bcm2836 HW
blocks, I'd expect the compatible value to be different for each. In
particular...

> +Optional properties:
> +- interrupt-parent : Specifies the parent interrupt controller when this
> +  controller is the second level.
> +- interrupts : Specifies the interrupt on the parent for this interrupt
> +  controller to handle.

I'd classify that as "additional required properties for
brcm,bcm2836-armctrl-ic"

... and with different compatible values for the two chips, you would
know when probe() should require vs. reject the property.

Re: [PATCH v2 2/4] irqchip: bcm2835: If a parent interrupt is registered, chain from it.

[Stephen Warren]

On 07/13/2015 07:35 PM, Eric Anholt wrote:
> The BCM2836 (Raspberry Pi 2) uses two levels of interrupt handling
> with the CPU-local interrupts being the root, so we need to register
> ours as chained off of the CPU's local interrupt.

Sorry for the slow review; laziness after vacation!

> diff --git a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt

> +The BCM2836 contains the same interrupt controller with the same
> +interrupts, but the per-CPU interrupt controller is the root, and an
> +interrupt there indicates that the ARMCTRL has an interrupt to handle.
> +
>  Required properties:
>  
>  - compatible : should be "brcm,bcm2835-armctrl-ic"

Since there are some differences between the bcm2835 and bcm2836 HW
blocks, I'd expect the compatible value to be different for each. In
particular...

> +Optional properties:
> +- interrupt-parent : Specifies the parent interrupt controller when this
> +  controller is the second level.
> +- interrupts : Specifies the interrupt on the parent for this interrupt
> +  controller to handle.

I'd classify that as "additional required properties for
brcm,bcm2836-armctrl-ic"

... and with different compatible values for the two chips, you would
know when probe() should require vs. reject the property.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/4] irqchip: bcm2835: If a parent interrupt is registered, chain from it.

[Eric Anholt]

Stephen Warren <swarren@wwwdotorg.org> writes:

> On 07/13/2015 07:35 PM, Eric Anholt wrote:
>> The BCM2836 (Raspberry Pi 2) uses two levels of interrupt handling
>> with the CPU-local interrupts being the root, so we need to register
>> ours as chained off of the CPU's local interrupt.
>
> Sorry for the slow review; laziness after vacation!
>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>
>> +The BCM2836 contains the same interrupt controller with the same
>> +interrupts, but the per-CPU interrupt controller is the root, and an
>> +interrupt there indicates that the ARMCTRL has an interrupt to handle.
>> +
>>  Required properties:
>>  
>>  - compatible : should be "brcm,bcm2835-armctrl-ic"
>
> Since there are some differences between the bcm2835 and bcm2836 HW
> blocks, I'd expect the compatible value to be different for each. In
> particular...

Well, there are actually no differences within this block of the HW (HDL
is unmodified), it's just where the output interrupt line gets consumed.
But it's not much extra to add a new compatible value, so sure.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150722/0c8559aa/attachment.sig>

Re: [PATCH v2 2/4] irqchip: bcm2835: If a parent interrupt is registered, chain from it.

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 1235 bytes --]

Stephen Warren <swarren@wwwdotorg.org> writes:

> On 07/13/2015 07:35 PM, Eric Anholt wrote:
>> The BCM2836 (Raspberry Pi 2) uses two levels of interrupt handling
>> with the CPU-local interrupts being the root, so we need to register
>> ours as chained off of the CPU's local interrupt.
>
> Sorry for the slow review; laziness after vacation!
>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>
>> +The BCM2836 contains the same interrupt controller with the same
>> +interrupts, but the per-CPU interrupt controller is the root, and an
>> +interrupt there indicates that the ARMCTRL has an interrupt to handle.
>> +
>>  Required properties:
>>  
>>  - compatible : should be "brcm,bcm2835-armctrl-ic"
>
> Since there are some differences between the bcm2835 and bcm2836 HW
> blocks, I'd expect the compatible value to be different for each. In
> particular...

Well, there are actually no differences within this block of the HW (HDL
is unmodified), it's just where the output interrupt line gets consumed.
But it's not much extra to add a new compatible value, so sure.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

[PATCH v2 2/4] irqchip: bcm2835: If a parent interrupt is registered, chain from it.

[Stephen Warren]

On 07/22/2015 12:17 PM, Eric Anholt wrote:
> Stephen Warren <swarren@wwwdotorg.org> writes:
> 
>> On 07/13/2015 07:35 PM, Eric Anholt wrote:
>>> The BCM2836 (Raspberry Pi 2) uses two levels of interrupt
>>> handling with the CPU-local interrupts being the root, so we
>>> need to register ours as chained off of the CPU's local
>>> interrupt.
>> 
>> Sorry for the slow review; laziness after vacation!
>> 
>>> diff --git
>>> a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>>> b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>>
>>>
>>> 
+The BCM2836 contains the same interrupt controller with the same
>>> +interrupts, but the per-CPU interrupt controller is the root,
>>> and an +interrupt there indicates that the ARMCTRL has an
>>> interrupt to handle. + Required properties:
>>> 
>>> - compatible : should be "brcm,bcm2835-armctrl-ic"
>> 
>> Since there are some differences between the bcm2835 and bcm2836
>> HW blocks, I'd expect the compatible value to be different for
>> each. In particular...
> 
> Well, there are actually no differences within this block of the HW
> (HDL is unmodified), it's just where the output interrupt line gets
> consumed. But it's not much extra to add a new compatible value, so
> sure.

Mmm. I suppose that's true indeed.

So, I guess either of the following is fine for bcm2836 by me:

compatible = "brcm,bcm2836-armctrl-ic";
compatible = "brcm,bcm2836-armctrl-ic", "brcm,bcm2835-armctrl-ic";

The 2836 value is always needed since DT should contain the most
specific compatible value for the implementation. The 2835 value is
optional based on whether the HW block is 100% backwards-compatible
with the older HW block; a driver for the old block can run unmodified
against the new block. It's debatable whether that's true here; the
interface to this HW block itself is unchanged between
implementations, yet the way the driver for it integrates into the
system differs since it either is/isn't a top-level IRQ chip.

Re: [PATCH v2 2/4] irqchip: bcm2835: If a parent interrupt is registered, chain from it.

[Stephen Warren]

On 07/22/2015 12:17 PM, Eric Anholt wrote:
> Stephen Warren <swarren@wwwdotorg.org> writes:
> 
>> On 07/13/2015 07:35 PM, Eric Anholt wrote:
>>> The BCM2836 (Raspberry Pi 2) uses two levels of interrupt
>>> handling with the CPU-local interrupts being the root, so we
>>> need to register ours as chained off of the CPU's local
>>> interrupt.
>> 
>> Sorry for the slow review; laziness after vacation!
>> 
>>> diff --git
>>> a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>>> b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>>
>>>
>>> 
+The BCM2836 contains the same interrupt controller with the same
>>> +interrupts, but the per-CPU interrupt controller is the root,
>>> and an +interrupt there indicates that the ARMCTRL has an
>>> interrupt to handle. + Required properties:
>>> 
>>> - compatible : should be "brcm,bcm2835-armctrl-ic"
>> 
>> Since there are some differences between the bcm2835 and bcm2836
>> HW blocks, I'd expect the compatible value to be different for
>> each. In particular...
> 
> Well, there are actually no differences within this block of the HW
> (HDL is unmodified), it's just where the output interrupt line gets
> consumed. But it's not much extra to add a new compatible value, so
> sure.

Mmm. I suppose that's true indeed.

So, I guess either of the following is fine for bcm2836 by me:

compatible = "brcm,bcm2836-armctrl-ic";
compatible = "brcm,bcm2836-armctrl-ic", "brcm,bcm2835-armctrl-ic";

The 2836 value is always needed since DT should contain the most
specific compatible value for the implementation. The 2835 value is
optional based on whether the HW block is 100% backwards-compatible
with the older HW block; a driver for the old block can run unmodified
against the new block. It's debatable whether that's true here; the
interface to this HW block itself is unchanged between
implementations, yet the way the driver for it integrates into the
system differs since it either is/isn't a top-level IRQ chip.

Re: [PATCH v2 2/4] irqchip: bcm2835: If a parent interrupt is registered, chain from it.

[Stephen Warren]

On 07/22/2015 12:17 PM, Eric Anholt wrote:
> Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> writes:
> 
>> On 07/13/2015 07:35 PM, Eric Anholt wrote:
>>> The BCM2836 (Raspberry Pi 2) uses two levels of interrupt
>>> handling with the CPU-local interrupts being the root, so we
>>> need to register ours as chained off of the CPU's local
>>> interrupt.
>> 
>> Sorry for the slow review; laziness after vacation!
>> 
>>> diff --git
>>> a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>>> b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>>
>>>
>>> 
+The BCM2836 contains the same interrupt controller with the same
>>> +interrupts, but the per-CPU interrupt controller is the root,
>>> and an +interrupt there indicates that the ARMCTRL has an
>>> interrupt to handle. + Required properties:
>>> 
>>> - compatible : should be "brcm,bcm2835-armctrl-ic"
>> 
>> Since there are some differences between the bcm2835 and bcm2836
>> HW blocks, I'd expect the compatible value to be different for
>> each. In particular...
> 
> Well, there are actually no differences within this block of the HW
> (HDL is unmodified), it's just where the output interrupt line gets
> consumed. But it's not much extra to add a new compatible value, so
> sure.

Mmm. I suppose that's true indeed.

So, I guess either of the following is fine for bcm2836 by me:

compatible = "brcm,bcm2836-armctrl-ic";
compatible = "brcm,bcm2836-armctrl-ic", "brcm,bcm2835-armctrl-ic";

The 2836 value is always needed since DT should contain the most
specific compatible value for the implementation. The 2835 value is
optional based on whether the HW block is 100% backwards-compatible
with the older HW block; a driver for the old block can run unmodified
against the new block. It's debatable whether that's true here; the
interface to this HW block itself is unchanged between
implementations, yet the way the driver for it integrates into the
system differs since it either is/isn't a top-level IRQ chip.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 3/4] irqchip: Add documentation for the bcm2836 interrupt controller.

[Eric Anholt]

This is a new per-cpu root interrupt controller on the Raspberry Pi 2,
which will chain to the bcm2835 interrupt controller for peripheral
interrupts.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 .../interrupt-controller/brcm,bcm2836-l1-intc.txt  | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt

diff --git a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt
new file mode 100644
index 0000000..f320dcd
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt
@@ -0,0 +1,37 @@
+BCM2836 per-CPU interrupt controller
+
+The BCM2836 has a per-cpu interrupt controller for the timer, PMU
+events, and SMP IPIs.  One of the CPUs may receive interrupts for the
+peripheral (GPU) events, which chain to the BCM2835-style interrupt
+controller.
+
+Required properties:
+
+- compatible:	 	Should be "brcm,bcm2836-l1-intc"
+- reg:			Specifies base physical address and size of the
+			  registers
+- interrupt-controller:	Identifies the node as an interrupt controller
+- #interrupt-cells:	Specifies the number of cells needed to encode an
+			  interrupt source. The value shall be 1
+
+Please refer to interrupts.txt in this directory for details of the common
+Interrupt Controllers bindings used by client devices.
+
+The interrupt sources are as follows:
+
+0: CNTPSIRQ
+1: CNTPNSIRQ
+2: CNTHPIRQ
+3: CNTVIRQ
+8: GPU_FAST
+9: PMU_FAST
+
+Example:
+
+local_intc: local_intc {
+	compatible = "brcm,bcm2836-l1-intc";
+	reg = <0x40000000 0x100>;
+	interrupt-controller;
+	#interrupt-cells = <1>;
+	interrupt-parent = <&local_intc>;
+};
-- 
2.1.4

[PATCH v2 3/4] irqchip: Add documentation for the bcm2836 interrupt controller.

[Eric Anholt]

This is a new per-cpu root interrupt controller on the Raspberry Pi 2,
which will chain to the bcm2835 interrupt controller for peripheral
interrupts.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 .../interrupt-controller/brcm,bcm2836-l1-intc.txt  | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt

diff --git a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt
new file mode 100644
index 0000000..f320dcd
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt
@@ -0,0 +1,37 @@
+BCM2836 per-CPU interrupt controller
+
+The BCM2836 has a per-cpu interrupt controller for the timer, PMU
+events, and SMP IPIs.  One of the CPUs may receive interrupts for the
+peripheral (GPU) events, which chain to the BCM2835-style interrupt
+controller.
+
+Required properties:
+
+- compatible:	 	Should be "brcm,bcm2836-l1-intc"
+- reg:			Specifies base physical address and size of the
+			  registers
+- interrupt-controller:	Identifies the node as an interrupt controller
+- #interrupt-cells:	Specifies the number of cells needed to encode an
+			  interrupt source. The value shall be 1
+
+Please refer to interrupts.txt in this directory for details of the common
+Interrupt Controllers bindings used by client devices.
+
+The interrupt sources are as follows:
+
+0: CNTPSIRQ
+1: CNTPNSIRQ
+2: CNTHPIRQ
+3: CNTVIRQ
+8: GPU_FAST
+9: PMU_FAST
+
+Example:
+
+local_intc: local_intc {
+	compatible = "brcm,bcm2836-l1-intc";
+	reg = <0x40000000 0x100>;
+	interrupt-controller;
+	#interrupt-cells = <1>;
+	interrupt-parent = <&local_intc>;
+};
-- 
2.1.4

[PATCH v2 3/4] irqchip: Add documentation for the bcm2836 interrupt controller.

[Eric Anholt]

This is a new per-cpu root interrupt controller on the Raspberry Pi 2,
which will chain to the bcm2835 interrupt controller for peripheral
interrupts.

Signed-off-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
---
 .../interrupt-controller/brcm,bcm2836-l1-intc.txt  | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt

diff --git a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt
new file mode 100644
index 0000000..f320dcd
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt
@@ -0,0 +1,37 @@
+BCM2836 per-CPU interrupt controller
+
+The BCM2836 has a per-cpu interrupt controller for the timer, PMU
+events, and SMP IPIs.  One of the CPUs may receive interrupts for the
+peripheral (GPU) events, which chain to the BCM2835-style interrupt
+controller.
+
+Required properties:
+
+- compatible:	 	Should be "brcm,bcm2836-l1-intc"
+- reg:			Specifies base physical address and size of the
+			  registers
+- interrupt-controller:	Identifies the node as an interrupt controller
+- #interrupt-cells:	Specifies the number of cells needed to encode an
+			  interrupt source. The value shall be 1
+
+Please refer to interrupts.txt in this directory for details of the common
+Interrupt Controllers bindings used by client devices.
+
+The interrupt sources are as follows:
+
+0: CNTPSIRQ
+1: CNTPNSIRQ
+2: CNTHPIRQ
+3: CNTVIRQ
+8: GPU_FAST
+9: PMU_FAST
+
+Example:
+
+local_intc: local_intc {
+	compatible = "brcm,bcm2836-l1-intc";
+	reg = <0x40000000 0x100>;
+	interrupt-controller;
+	#interrupt-cells = <1>;
+	interrupt-parent = <&local_intc>;
+};
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 4/4] irqchip: Add bcm2836 interrupt controller for Raspberry Pi 2.

[Eric Anholt]

This interrupt controller is the new root interrupt controller with
the timer, PMU events, and IPIs, and the bcm2835's interrupt
controller is chained off of it to handle the peripherals.

I wrote the interrupt chip support, while Andrea Merello wrote the IPI
code.

Signed-off-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
---

v2: Rename functions/globals to bcm2836_arm_local.  Treat per-cpu
    interrupts as per-cpu in mask/unmask.  Add comments about how
    registers work.

 drivers/irqchip/Makefile      |   1 +
 drivers/irqchip/irq-bcm2836.c | 256 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 257 insertions(+)
 create mode 100644 drivers/irqchip/irq-bcm2836.c

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index b8d4e96..9727681 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -1,6 +1,7 @@
 obj-$(CONFIG_IRQCHIP)			+= irqchip.o
 
 obj-$(CONFIG_ARCH_BCM2835)		+= irq-bcm2835.o
+obj-$(CONFIG_ARCH_BCM2835)		+= irq-bcm2836.o
 obj-$(CONFIG_ARCH_EXYNOS)		+= exynos-combiner.o
 obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
 obj-$(CONFIG_ARCH_MMP)			+= irq-mmp.o
diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
new file mode 100644
index 0000000..fecbbc2
--- /dev/null
+++ b/drivers/irqchip/irq-bcm2836.c
@@ -0,0 +1,256 @@
+/*
+ * Root interrupt controller for the BCM2836 (Raspberry Pi 2).
+ *
+ * Copyright 2015 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>
+#include <asm/exception.h>
+
+/*
+ * The low 2 bits identify the CPU that the GPU IRQ goes to, and the
+ * next 2 bits identify the CPU that the GPU FIQ goes to.
+ */
+#define LOCAL_GPU_ROUTING		0x00c
+/* When setting bits 0-3, enables PMU interrupts on that CPU. */
+#define LOCAL_PM_ROUTING_SET		0x010
+/* When setting bits 0-3, disables PMU interrupts on that CPU. */
+#define LOCAL_PM_ROUTING_CLR		0x014
+/*
+ * The low 4 bits of this are the CPU's timer IRQ enables, and the
+ * next 4 bits are the CPU's timer FIQ enables (which override the IRQ
+ * bits).
+ */
+#define LOCAL_TIMER_INT_CONTROL0	0x040
+/*
+ * The low 4 bits of this are the CPU's per-mailbox IRQ enables, and
+ * the next 4 bits are the CPU's per-mailbox FIQ enables (which
+ * override the IRQ bits).
+ */
+#define LOCAL_MAILBOX_INT_CONTROL0	0x050
+/*
+ * The CPU's interrupt status register.  Bits are defined by the the
+ * LOCAL_IRQ_* bits below.
+ */
+#define LOCAL_IRQ_PENDING0		0x060
+/* Same status bits as above, but for FIQ. */
+#define LOCAL_FIQ_PENDING0		0x070
+/*
+ * Mailbox0 write-to-set bits.  There are 16 mailboxes, 4 per CPU, and
+ * these bits are organized by mailbox number and then CPU number.  We
+ * use mailbox 0 for IPIs.  The mailbox's interrupt is raised while
+ * any bit is set.
+ */
+#define LOCAL_MAILBOX0_SET0		0x080
+/* Mailbox0 write-to-clear bits. */
+#define LOCAL_MAILBOX0_CLR0		0x0c0
+
+#define LOCAL_IRQ_CNTPSIRQ	0
+#define LOCAL_IRQ_CNTPNSIRQ	1
+#define LOCAL_IRQ_CNTHPIRQ	2
+#define LOCAL_IRQ_CNTVIRQ	3
+#define LOCAL_IRQ_MAILBOX0	4
+#define LOCAL_IRQ_MAILBOX1	5
+#define LOCAL_IRQ_MAILBOX2	6
+#define LOCAL_IRQ_MAILBOX3	7
+#define LOCAL_IRQ_GPU_FAST	8
+#define LOCAL_IRQ_PMU_FAST	9
+#define LAST_IRQ		LOCAL_IRQ_PMU_FAST
+
+struct bcm2836_arm_irqchip_intc {
+	struct irq_domain *domain;
+	void __iomem *base;
+};
+
+static struct bcm2836_arm_irqchip_intc intc  __read_mostly;
+
+static void bcm2836_arm_irqchip_mask_per_cpu_irq(unsigned int reg_offset,
+						 unsigned int bit,
+						 int cpu)
+{
+	void __iomem *reg = intc.base + reg_offset + 4 * cpu;
+
+	writel(readl(reg) & ~BIT(bit), reg);
+}
+
+static void bcm2836_arm_irqchip_unmask_per_cpu_irq(unsigned int reg_offset,
+						   unsigned int bit,
+						 int cpu)
+{
+	void __iomem *reg = intc.base + reg_offset + 4 * cpu;
+
+	writel(readl(reg) | BIT(bit), reg);
+}
+
+static void bcm2836_arm_irqchip_mask_timer_irq(struct irq_data *d)
+{
+	bcm2836_arm_irqchip_mask_per_cpu_irq(LOCAL_TIMER_INT_CONTROL0,
+					     d->hwirq - LOCAL_IRQ_CNTPSIRQ,
+					     smp_processor_id());
+}
+
+static void bcm2836_arm_irqchip_unmask_timer_irq(struct irq_data *d)
+{
+	bcm2836_arm_irqchip_unmask_per_cpu_irq(LOCAL_TIMER_INT_CONTROL0,
+					       d->hwirq - LOCAL_IRQ_CNTPSIRQ,
+					       smp_processor_id());
+}
+
+static struct irq_chip bcm2836_arm_irqchip_timer = {
+	.name = "bcm2836-timer",
+	.irq_mask = bcm2836_arm_irqchip_mask_timer_irq,
+	.irq_unmask = bcm2836_arm_irqchip_unmask_timer_irq
+};
+
+static void bcm2836_arm_irqchip_mask_pmu_irq(struct irq_data *d)
+{
+	pr_err("%d: mask PMU\n", smp_processor_id());
+	writel(1 << smp_processor_id(), intc.base + LOCAL_PM_ROUTING_CLR);
+}
+
+static void bcm2836_arm_irqchip_unmask_pmu_irq(struct irq_data *d)
+{
+	pr_err("%d: unmask PMU\n", smp_processor_id());
+	writel(1 << smp_processor_id(), intc.base + LOCAL_PM_ROUTING_SET);
+}
+
+static struct irq_chip bcm2836_arm_irqchip_pmu = {
+	.name = "bcm2836-pmu",
+	.irq_mask = bcm2836_arm_irqchip_mask_pmu_irq,
+	.irq_unmask = bcm2836_arm_irqchip_unmask_pmu_irq
+};
+
+static void bcm2836_arm_irqchip_mask_gpu_irq(struct irq_data *d)
+{
+}
+
+static void bcm2836_arm_irqchip_unmask_gpu_irq(struct irq_data *d)
+{
+}
+
+static struct irq_chip bcm2836_arm_irqchip_gpu = {
+	.name = "bcm2836-gpu",
+	.irq_mask = bcm2836_arm_irqchip_mask_gpu_irq,
+	.irq_unmask = bcm2836_arm_irqchip_unmask_gpu_irq
+};
+
+static void bcm2836_arm_irqchip_register_irq(int hwirq, struct irq_chip *chip)
+{
+	int irq = irq_create_mapping(intc.domain, hwirq);
+
+	irq_set_percpu_devid(irq);
+	irq_set_chip_and_handler(irq, chip, handle_percpu_devid_irq);
+	irq_set_status_flags(irq, IRQ_NOAUTOEN);
+}
+
+static void __exception_irq_entry bcm2836_arm_irqchip_handle_irq(
+	struct pt_regs *regs)
+{
+	int cpu = smp_processor_id();
+	u32 stat;
+
+	stat = readl_relaxed(intc.base + LOCAL_IRQ_PENDING0 + 4 * cpu);
+	if (stat & 0x10) {
+#ifdef CONFIG_SMP
+		void __iomem *mailbox0 = (intc.base +
+					  LOCAL_MAILBOX0_CLR0 + 16 * cpu);
+		u32 mbox_val = readl(mailbox0);
+		u32 ipi = ffs(mbox_val) - 1;
+
+		writel(1 << ipi, mailbox0);
+		handle_IPI(ipi, regs);
+#endif
+	} else {
+		u32 hwirq = ffs(stat) - 1;
+
+		handle_IRQ(irq_linear_revmap(intc.domain, hwirq), regs);
+	}
+}
+
+#ifdef CONFIG_SMP
+static void bcm2836_arm_irqchip_send_ipi(const struct cpumask *mask,
+					 unsigned int ipi)
+{
+	int cpu;
+	void __iomem *mailbox0_base = intc.base + LOCAL_MAILBOX0_SET0;
+
+	/*
+	 * Ensure that stores to normal memory are visible to the
+	 * other CPUs before issuing the IPI.
+	 */
+	dsb();
+
+	for_each_cpu(cpu, mask)	{
+		writel(1 << ipi, mailbox0_base + 16 * cpu);
+	}
+}
+#endif
+
+static const struct irq_domain_ops bcm2836_arm_irqchip_intc_ops = {
+	.xlate = irq_domain_xlate_onecell
+};
+
+static void
+bcm2836_arm_irqchip_smp_init(void)
+{
+#ifdef CONFIG_SMP
+	int i;
+
+	/* unmask IPIs */
+	for_each_possible_cpu(i) {
+		bcm2836_arm_irqchip_unmask_per_cpu_irq(
+			LOCAL_MAILBOX_INT_CONTROL0, 0, i);
+	}
+	set_smp_cross_call(bcm2836_arm_irqchip_send_ipi);
+#endif
+}
+
+static int __init bcm2836_arm_irqchip_l1_intc_of_init(struct device_node *node,
+						      struct device_node *parent)
+{
+	intc.base = of_iomap(node, 0);
+	if (!intc.base) {
+		panic("%s: unable to map local interrupt registers\n",
+			node->full_name);
+	}
+
+	intc.domain = irq_domain_add_linear(node, LAST_IRQ + 1,
+					    &bcm2836_arm_irqchip_intc_ops,
+					    NULL);
+	if (!intc.domain)
+		panic("%s: unable to create IRQ domain\n", node->full_name);
+
+	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_CNTPSIRQ,
+					 &bcm2836_arm_irqchip_timer);
+	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_CNTPNSIRQ,
+					 &bcm2836_arm_irqchip_timer);
+	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_CNTHPIRQ,
+					 &bcm2836_arm_irqchip_timer);
+	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_CNTVIRQ,
+					 &bcm2836_arm_irqchip_timer);
+	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_GPU_FAST,
+					 &bcm2836_arm_irqchip_gpu);
+	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_PMU_FAST,
+					 &bcm2836_arm_irqchip_pmu);
+
+	bcm2836_arm_irqchip_smp_init();
+
+	set_handle_irq(bcm2836_arm_irqchip_handle_irq);
+	return 0;
+}
+
+IRQCHIP_DECLARE(bcm2836_arm_irqchip_l1_intc, "brcm,bcm2836-l1-intc",
+		bcm2836_arm_irqchip_l1_intc_of_init);
-- 
2.1.4

[PATCH v2 4/4] irqchip: Add bcm2836 interrupt controller for Raspberry Pi 2.

[Eric Anholt]

This interrupt controller is the new root interrupt controller with
the timer, PMU events, and IPIs, and the bcm2835's interrupt
controller is chained off of it to handle the peripherals.

I wrote the interrupt chip support, while Andrea Merello wrote the IPI
code.

Signed-off-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
---

v2: Rename functions/globals to bcm2836_arm_local.  Treat per-cpu
    interrupts as per-cpu in mask/unmask.  Add comments about how
    registers work.

 drivers/irqchip/Makefile      |   1 +
 drivers/irqchip/irq-bcm2836.c | 256 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 257 insertions(+)
 create mode 100644 drivers/irqchip/irq-bcm2836.c

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index b8d4e96..9727681 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -1,6 +1,7 @@
 obj-$(CONFIG_IRQCHIP)			+= irqchip.o
 
 obj-$(CONFIG_ARCH_BCM2835)		+= irq-bcm2835.o
+obj-$(CONFIG_ARCH_BCM2835)		+= irq-bcm2836.o
 obj-$(CONFIG_ARCH_EXYNOS)		+= exynos-combiner.o
 obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
 obj-$(CONFIG_ARCH_MMP)			+= irq-mmp.o
diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
new file mode 100644
index 0000000..fecbbc2
--- /dev/null
+++ b/drivers/irqchip/irq-bcm2836.c
@@ -0,0 +1,256 @@
+/*
+ * Root interrupt controller for the BCM2836 (Raspberry Pi 2).
+ *
+ * Copyright 2015 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>
+#include <asm/exception.h>
+
+/*
+ * The low 2 bits identify the CPU that the GPU IRQ goes to, and the
+ * next 2 bits identify the CPU that the GPU FIQ goes to.
+ */
+#define LOCAL_GPU_ROUTING		0x00c
+/* When setting bits 0-3, enables PMU interrupts on that CPU. */
+#define LOCAL_PM_ROUTING_SET		0x010
+/* When setting bits 0-3, disables PMU interrupts on that CPU. */
+#define LOCAL_PM_ROUTING_CLR		0x014
+/*
+ * The low 4 bits of this are the CPU's timer IRQ enables, and the
+ * next 4 bits are the CPU's timer FIQ enables (which override the IRQ
+ * bits).
+ */
+#define LOCAL_TIMER_INT_CONTROL0	0x040
+/*
+ * The low 4 bits of this are the CPU's per-mailbox IRQ enables, and
+ * the next 4 bits are the CPU's per-mailbox FIQ enables (which
+ * override the IRQ bits).
+ */
+#define LOCAL_MAILBOX_INT_CONTROL0	0x050
+/*
+ * The CPU's interrupt status register.  Bits are defined by the the
+ * LOCAL_IRQ_* bits below.
+ */
+#define LOCAL_IRQ_PENDING0		0x060
+/* Same status bits as above, but for FIQ. */
+#define LOCAL_FIQ_PENDING0		0x070
+/*
+ * Mailbox0 write-to-set bits.  There are 16 mailboxes, 4 per CPU, and
+ * these bits are organized by mailbox number and then CPU number.  We
+ * use mailbox 0 for IPIs.  The mailbox's interrupt is raised while
+ * any bit is set.
+ */
+#define LOCAL_MAILBOX0_SET0		0x080
+/* Mailbox0 write-to-clear bits. */
+#define LOCAL_MAILBOX0_CLR0		0x0c0
+
+#define LOCAL_IRQ_CNTPSIRQ	0
+#define LOCAL_IRQ_CNTPNSIRQ	1
+#define LOCAL_IRQ_CNTHPIRQ	2
+#define LOCAL_IRQ_CNTVIRQ	3
+#define LOCAL_IRQ_MAILBOX0	4
+#define LOCAL_IRQ_MAILBOX1	5
+#define LOCAL_IRQ_MAILBOX2	6
+#define LOCAL_IRQ_MAILBOX3	7
+#define LOCAL_IRQ_GPU_FAST	8
+#define LOCAL_IRQ_PMU_FAST	9
+#define LAST_IRQ		LOCAL_IRQ_PMU_FAST
+
+struct bcm2836_arm_irqchip_intc {
+	struct irq_domain *domain;
+	void __iomem *base;
+};
+
+static struct bcm2836_arm_irqchip_intc intc  __read_mostly;
+
+static void bcm2836_arm_irqchip_mask_per_cpu_irq(unsigned int reg_offset,
+						 unsigned int bit,
+						 int cpu)
+{
+	void __iomem *reg = intc.base + reg_offset + 4 * cpu;
+
+	writel(readl(reg) & ~BIT(bit), reg);
+}
+
+static void bcm2836_arm_irqchip_unmask_per_cpu_irq(unsigned int reg_offset,
+						   unsigned int bit,
+						 int cpu)
+{
+	void __iomem *reg = intc.base + reg_offset + 4 * cpu;
+
+	writel(readl(reg) | BIT(bit), reg);
+}
+
+static void bcm2836_arm_irqchip_mask_timer_irq(struct irq_data *d)
+{
+	bcm2836_arm_irqchip_mask_per_cpu_irq(LOCAL_TIMER_INT_CONTROL0,
+					     d->hwirq - LOCAL_IRQ_CNTPSIRQ,
+					     smp_processor_id());
+}
+
+static void bcm2836_arm_irqchip_unmask_timer_irq(struct irq_data *d)
+{
+	bcm2836_arm_irqchip_unmask_per_cpu_irq(LOCAL_TIMER_INT_CONTROL0,
+					       d->hwirq - LOCAL_IRQ_CNTPSIRQ,
+					       smp_processor_id());
+}
+
+static struct irq_chip bcm2836_arm_irqchip_timer = {
+	.name = "bcm2836-timer",
+	.irq_mask = bcm2836_arm_irqchip_mask_timer_irq,
+	.irq_unmask = bcm2836_arm_irqchip_unmask_timer_irq
+};
+
+static void bcm2836_arm_irqchip_mask_pmu_irq(struct irq_data *d)
+{
+	pr_err("%d: mask PMU\n", smp_processor_id());
+	writel(1 << smp_processor_id(), intc.base + LOCAL_PM_ROUTING_CLR);
+}
+
+static void bcm2836_arm_irqchip_unmask_pmu_irq(struct irq_data *d)
+{
+	pr_err("%d: unmask PMU\n", smp_processor_id());
+	writel(1 << smp_processor_id(), intc.base + LOCAL_PM_ROUTING_SET);
+}
+
+static struct irq_chip bcm2836_arm_irqchip_pmu = {
+	.name = "bcm2836-pmu",
+	.irq_mask = bcm2836_arm_irqchip_mask_pmu_irq,
+	.irq_unmask = bcm2836_arm_irqchip_unmask_pmu_irq
+};
+
+static void bcm2836_arm_irqchip_mask_gpu_irq(struct irq_data *d)
+{
+}
+
+static void bcm2836_arm_irqchip_unmask_gpu_irq(struct irq_data *d)
+{
+}
+
+static struct irq_chip bcm2836_arm_irqchip_gpu = {
+	.name = "bcm2836-gpu",
+	.irq_mask = bcm2836_arm_irqchip_mask_gpu_irq,
+	.irq_unmask = bcm2836_arm_irqchip_unmask_gpu_irq
+};
+
+static void bcm2836_arm_irqchip_register_irq(int hwirq, struct irq_chip *chip)
+{
+	int irq = irq_create_mapping(intc.domain, hwirq);
+
+	irq_set_percpu_devid(irq);
+	irq_set_chip_and_handler(irq, chip, handle_percpu_devid_irq);
+	irq_set_status_flags(irq, IRQ_NOAUTOEN);
+}
+
+static void __exception_irq_entry bcm2836_arm_irqchip_handle_irq(
+	struct pt_regs *regs)
+{
+	int cpu = smp_processor_id();
+	u32 stat;
+
+	stat = readl_relaxed(intc.base + LOCAL_IRQ_PENDING0 + 4 * cpu);
+	if (stat & 0x10) {
+#ifdef CONFIG_SMP
+		void __iomem *mailbox0 = (intc.base +
+					  LOCAL_MAILBOX0_CLR0 + 16 * cpu);
+		u32 mbox_val = readl(mailbox0);
+		u32 ipi = ffs(mbox_val) - 1;
+
+		writel(1 << ipi, mailbox0);
+		handle_IPI(ipi, regs);
+#endif
+	} else {
+		u32 hwirq = ffs(stat) - 1;
+
+		handle_IRQ(irq_linear_revmap(intc.domain, hwirq), regs);
+	}
+}
+
+#ifdef CONFIG_SMP
+static void bcm2836_arm_irqchip_send_ipi(const struct cpumask *mask,
+					 unsigned int ipi)
+{
+	int cpu;
+	void __iomem *mailbox0_base = intc.base + LOCAL_MAILBOX0_SET0;
+
+	/*
+	 * Ensure that stores to normal memory are visible to the
+	 * other CPUs before issuing the IPI.
+	 */
+	dsb();
+
+	for_each_cpu(cpu, mask)	{
+		writel(1 << ipi, mailbox0_base + 16 * cpu);
+	}
+}
+#endif
+
+static const struct irq_domain_ops bcm2836_arm_irqchip_intc_ops = {
+	.xlate = irq_domain_xlate_onecell
+};
+
+static void
+bcm2836_arm_irqchip_smp_init(void)
+{
+#ifdef CONFIG_SMP
+	int i;
+
+	/* unmask IPIs */
+	for_each_possible_cpu(i) {
+		bcm2836_arm_irqchip_unmask_per_cpu_irq(
+			LOCAL_MAILBOX_INT_CONTROL0, 0, i);
+	}
+	set_smp_cross_call(bcm2836_arm_irqchip_send_ipi);
+#endif
+}
+
+static int __init bcm2836_arm_irqchip_l1_intc_of_init(struct device_node *node,
+						      struct device_node *parent)
+{
+	intc.base = of_iomap(node, 0);
+	if (!intc.base) {
+		panic("%s: unable to map local interrupt registers\n",
+			node->full_name);
+	}
+
+	intc.domain = irq_domain_add_linear(node, LAST_IRQ + 1,
+					    &bcm2836_arm_irqchip_intc_ops,
+					    NULL);
+	if (!intc.domain)
+		panic("%s: unable to create IRQ domain\n", node->full_name);
+
+	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_CNTPSIRQ,
+					 &bcm2836_arm_irqchip_timer);
+	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_CNTPNSIRQ,
+					 &bcm2836_arm_irqchip_timer);
+	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_CNTHPIRQ,
+					 &bcm2836_arm_irqchip_timer);
+	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_CNTVIRQ,
+					 &bcm2836_arm_irqchip_timer);
+	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_GPU_FAST,
+					 &bcm2836_arm_irqchip_gpu);
+	bcm2836_arm_irqchip_register_irq(LOCAL_IRQ_PMU_FAST,
+					 &bcm2836_arm_irqchip_pmu);
+
+	bcm2836_arm_irqchip_smp_init();
+
+	set_handle_irq(bcm2836_arm_irqchip_handle_irq);
+	return 0;
+}
+
+IRQCHIP_DECLARE(bcm2836_arm_irqchip_l1_intc, "brcm,bcm2836-l1-intc",
+		bcm2836_arm_irqchip_l1_intc_of_init);
-- 
2.1.4

[PATCH v2 4/4] irqchip: Add bcm2836 interrupt controller for Raspberry Pi 2.

[Stephen Warren]

On 07/13/2015 07:35 PM, Eric Anholt wrote:
> This interrupt controller is the new root interrupt controller with
> the timer, PMU events, and IPIs, and the bcm2835's interrupt
> controller is chained off of it to handle the peripherals.

> diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c

> +static void bcm2836_arm_irqchip_mask_pmu_irq(struct irq_data *d)
> +{
> +	pr_err("%d: mask PMU\n", smp_processor_id());
> +	writel(1 << smp_processor_id(), intc.base + LOCAL_PM_ROUTING_CLR);
> +}
> +
> +static void bcm2836_arm_irqchip_unmask_pmu_irq(struct irq_data *d)
> +{
> +	pr_err("%d: unmask PMU\n", smp_processor_id());
> +	writel(1 << smp_processor_id(), intc.base + LOCAL_PM_ROUTING_SET);
> +}

Are those pr_err() calls left-over debug, or is there some reason it's
an error to call those functions?

Aside from this and the other minor comment, the series,
Acked-by: Stephen Warren <swarren@wwwdotorg.org>

Re: [PATCH v2 4/4] irqchip: Add bcm2836 interrupt controller for Raspberry Pi 2.

[Stephen Warren]

On 07/13/2015 07:35 PM, Eric Anholt wrote:
> This interrupt controller is the new root interrupt controller with
> the timer, PMU events, and IPIs, and the bcm2835's interrupt
> controller is chained off of it to handle the peripherals.

> diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c

> +static void bcm2836_arm_irqchip_mask_pmu_irq(struct irq_data *d)
> +{
> +	pr_err("%d: mask PMU\n", smp_processor_id());
> +	writel(1 << smp_processor_id(), intc.base + LOCAL_PM_ROUTING_CLR);
> +}
> +
> +static void bcm2836_arm_irqchip_unmask_pmu_irq(struct irq_data *d)
> +{
> +	pr_err("%d: unmask PMU\n", smp_processor_id());
> +	writel(1 << smp_processor_id(), intc.base + LOCAL_PM_ROUTING_SET);
> +}

Are those pr_err() calls left-over debug, or is there some reason it's
an error to call those functions?

Aside from this and the other minor comment, the series,
Acked-by: Stephen Warren <swarren@wwwdotorg.org>

Re: [PATCH v2 4/4] irqchip: Add bcm2836 interrupt controller for Raspberry Pi 2.

[Stephen Warren]

On 07/13/2015 07:35 PM, Eric Anholt wrote:
> This interrupt controller is the new root interrupt controller with
> the timer, PMU events, and IPIs, and the bcm2835's interrupt
> controller is chained off of it to handle the peripherals.

> diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c

> +static void bcm2836_arm_irqchip_mask_pmu_irq(struct irq_data *d)
> +{
> +	pr_err("%d: mask PMU\n", smp_processor_id());
> +	writel(1 << smp_processor_id(), intc.base + LOCAL_PM_ROUTING_CLR);
> +}
> +
> +static void bcm2836_arm_irqchip_unmask_pmu_irq(struct irq_data *d)
> +{
> +	pr_err("%d: unmask PMU\n", smp_processor_id());
> +	writel(1 << smp_processor_id(), intc.base + LOCAL_PM_ROUTING_SET);
> +}

Are those pr_err() calls left-over debug, or is there some reason it's
an error to call those functions?

Aside from this and the other minor comment, the series,
Acked-by: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 4/4] irqchip: Add bcm2836 interrupt controller for Raspberry Pi 2.

[Eric Anholt]

Stephen Warren <swarren@wwwdotorg.org> writes:

> On 07/13/2015 07:35 PM, Eric Anholt wrote:
>> This interrupt controller is the new root interrupt controller with
>> the timer, PMU events, and IPIs, and the bcm2835's interrupt
>> controller is chained off of it to handle the peripherals.
>
>> diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
>
>> +static void bcm2836_arm_irqchip_mask_pmu_irq(struct irq_data *d)
>> +{
>> +	pr_err("%d: mask PMU\n", smp_processor_id());
>> +	writel(1 << smp_processor_id(), intc.base + LOCAL_PM_ROUTING_CLR);
>> +}
>> +
>> +static void bcm2836_arm_irqchip_unmask_pmu_irq(struct irq_data *d)
>> +{
>> +	pr_err("%d: unmask PMU\n", smp_processor_id());
>> +	writel(1 << smp_processor_id(), intc.base + LOCAL_PM_ROUTING_SET);
>> +}
>
> Are those pr_err() calls left-over debug, or is there some reason it's
> an error to call those functions?

Yeah, I was trying to figure out whether the PMU bits worked.  We're
failing before the point of IRQ probe, though (I don't see anything that
would set arm_pmu->supported_cpus for us).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150722/77c9390a/attachment.sig>

Re: [PATCH v2 4/4] irqchip: Add bcm2836 interrupt controller for Raspberry Pi 2.

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 1086 bytes --]

Stephen Warren <swarren@wwwdotorg.org> writes:

> On 07/13/2015 07:35 PM, Eric Anholt wrote:
>> This interrupt controller is the new root interrupt controller with
>> the timer, PMU events, and IPIs, and the bcm2835's interrupt
>> controller is chained off of it to handle the peripherals.
>
>> diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
>
>> +static void bcm2836_arm_irqchip_mask_pmu_irq(struct irq_data *d)
>> +{
>> +	pr_err("%d: mask PMU\n", smp_processor_id());
>> +	writel(1 << smp_processor_id(), intc.base + LOCAL_PM_ROUTING_CLR);
>> +}
>> +
>> +static void bcm2836_arm_irqchip_unmask_pmu_irq(struct irq_data *d)
>> +{
>> +	pr_err("%d: unmask PMU\n", smp_processor_id());
>> +	writel(1 << smp_processor_id(), intc.base + LOCAL_PM_ROUTING_SET);
>> +}
>
> Are those pr_err() calls left-over debug, or is there some reason it's
> an error to call those functions?

Yeah, I was trying to figure out whether the PMU bits worked.  We're
failing before the point of IRQ probe, though (I don't see anything that
would set arm_pmu->supported_cpus for us).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH v2 4/4] irqchip: Add bcm2836 interrupt controller for Raspberry Pi 2.

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 1112 bytes --]

Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> writes:

> On 07/13/2015 07:35 PM, Eric Anholt wrote:
>> This interrupt controller is the new root interrupt controller with
>> the timer, PMU events, and IPIs, and the bcm2835's interrupt
>> controller is chained off of it to handle the peripherals.
>
>> diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
>
>> +static void bcm2836_arm_irqchip_mask_pmu_irq(struct irq_data *d)
>> +{
>> +	pr_err("%d: mask PMU\n", smp_processor_id());
>> +	writel(1 << smp_processor_id(), intc.base + LOCAL_PM_ROUTING_CLR);
>> +}
>> +
>> +static void bcm2836_arm_irqchip_unmask_pmu_irq(struct irq_data *d)
>> +{
>> +	pr_err("%d: unmask PMU\n", smp_processor_id());
>> +	writel(1 << smp_processor_id(), intc.base + LOCAL_PM_ROUTING_SET);
>> +}
>
> Are those pr_err() calls left-over debug, or is there some reason it's
> an error to call those functions?

Yeah, I was trying to figure out whether the PMU bits worked.  We're
failing before the point of IRQ probe, though (I don't see anything that
would set arm_pmu->supported_cpus for us).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

[PATCH v2 4/4] irqchip: Add bcm2836 interrupt controller for Raspberry Pi 2.

[Thomas Gleixner]

On Mon, 13 Jul 2015, Eric Anholt wrote:
> +static void
> +bcm2836_arm_irqchip_smp_init(void)
> +{
> +#ifdef CONFIG_SMP
> +	int i;
> +
> +	/* unmask IPIs */
> +	for_each_possible_cpu(i) {
> +		bcm2836_arm_irqchip_unmask_per_cpu_irq(
> +			LOCAL_MAILBOX_INT_CONTROL0, 0, i);

Do you really want to enable these interrupts on all possible cpus? Why so?

Shouldn't that be done at the point where a hotplugged CPU is starting ?

Thanks,

	tglx

Re: [PATCH v2 4/4] irqchip: Add bcm2836 interrupt controller for Raspberry Pi 2.

[Thomas Gleixner]

On Mon, 13 Jul 2015, Eric Anholt wrote:
> +static void
> +bcm2836_arm_irqchip_smp_init(void)
> +{
> +#ifdef CONFIG_SMP
> +	int i;
> +
> +	/* unmask IPIs */
> +	for_each_possible_cpu(i) {
> +		bcm2836_arm_irqchip_unmask_per_cpu_irq(
> +			LOCAL_MAILBOX_INT_CONTROL0, 0, i);

Do you really want to enable these interrupts on all possible cpus? Why so?

Shouldn't that be done at the point where a hotplugged CPU is starting ?

Thanks,

	tglx

Re: [PATCH v2 4/4] irqchip: Add bcm2836 interrupt controller for Raspberry Pi 2.

[Thomas Gleixner]

On Mon, 13 Jul 2015, Eric Anholt wrote:
> +static void
> +bcm2836_arm_irqchip_smp_init(void)
> +{
> +#ifdef CONFIG_SMP
> +	int i;
> +
> +	/* unmask IPIs */
> +	for_each_possible_cpu(i) {
> +		bcm2836_arm_irqchip_unmask_per_cpu_irq(
> +			LOCAL_MAILBOX_INT_CONTROL0, 0, i);

Do you really want to enable these interrupts on all possible cpus? Why so?

Shouldn't that be done at the point where a hotplugged CPU is starting ?

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] irq-bcm2836: Move SMP boot to the irqchip code.

[Eric Anholt]

Signed-off-by: Eric Anholt <eric@anholt.net>
---

Thomas: The problem with delaying IPI unmasking until secondary boot
is that it means we need the secondary boot process to integrate with
the irqchip code, which seems unusual given the dearth of includes I
could find between arch/arm/mach-* SMP boot code and drivers/irqchip/
to get function prototypes.  However, since the irqchip is most of
this register space already, it might make sense to just have the SMP
boot live in drivers/irqchip/.  Here's a patch that would do that,
that could be squashed into my change.

 drivers/irqchip/irq-bcm2836.c | 57 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 50 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
index 87340b0..5f2a40e 100644
--- a/drivers/irqchip/irq-bcm2836.c
+++ b/drivers/irqchip/irq-bcm2836.c
@@ -49,14 +49,16 @@
 /* Same status bits as above, but for FIQ. */
 #define LOCAL_FIQ_PENDING0		0x070
 /*
- * Mailbox0 write-to-set bits.  There are 16 mailboxes, 4 per CPU, and
+ * Mailbox write-to-set bits.  There are 16 mailboxes, 4 per CPU, and
  * these bits are organized by mailbox number and then CPU number.  We
  * use mailbox 0 for IPIs.  The mailbox's interrupt is raised while
  * any bit is set.
  */
 #define LOCAL_MAILBOX0_SET0		0x080
+#define LOCAL_MAILBOX3_SET0		0x08c
 /* Mailbox0 write-to-clear bits. */
 #define LOCAL_MAILBOX0_CLR0		0x0c0
+#define LOCAL_MAILBOX3_CLR0		0x0cc
 
 #define LOCAL_IRQ_CNTPSIRQ	0
 #define LOCAL_IRQ_CNTPNSIRQ	1
@@ -195,6 +197,46 @@ static void bcm2836_arm_irqchip_send_ipi(const struct cpumask *mask,
 		writel(1 << ipi, mailbox0_base + 16 * cpu);
 	}
 }
+
+/* Requests boot of a secondary CPU.
+ *
+ * The Raspberry Pi firmware has already started up the CPU and set it
+ * spinning in a loop in low memory waiting for a value in mailbox 3
+ * indicating what OS code it should jump to.
+ */
+int __init bcm2836_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+	int timeout = 20;
+	void __iomem *mailbox3_set_base = intc.base + LOCAL_MAILBOX3_SET0;
+	void __iomem *mailbox3_clr_base = intc.base + LOCAL_MAILBOX3_CLR0;
+	unsigned long secondary_startup_phys =
+		(unsigned long) virt_to_phys((void *)secondary_startup);
+
+	/* Unmask IPIs to the target cpu. */
+	bcm2836_arm_irqchip_unmask_per_cpu_irq(LOCAL_MAILBOX_INT_CONTROL0, 0,
+					       cpu);
+
+	dsb();
+
+	writel(secondary_startup_phys, mailbox3_set_base + 16 * cpu);
+
+	while (true) {
+		int val = readl(mailbox3_clr_base + 16 * cpu);
+
+		if (val == 0)
+			return 0;
+		if (timeout-- == 0)
+			return -ETIMEDOUT;
+		cpu_relax();
+	}
+
+	return 0;
+}
+
+static struct smp_operations bcm2836_smp_ops __initdata = {
+	.smp_boot_secondary	= bcm2836_smp_boot_secondary,
+};
+
 #endif
 
 static const struct irq_domain_ops bcm2836_arm_irqchip_intc_ops = {
@@ -205,14 +247,15 @@ static void
 bcm2836_arm_irqchip_smp_init(void)
 {
 #ifdef CONFIG_SMP
-	int i;
+	/* Unmask IPIs to the boot CPU. Other CPUs will be unmasked as
+	 * they're brought up.
+	 */
+	bcm2836_arm_irqchip_unmask_per_cpu_irq(LOCAL_MAILBOX_INT_CONTROL0, 0,
+					       smp_processor_id());
 
-	/* unmask IPIs */
-	for_each_possible_cpu(i) {
-		bcm2836_arm_irqchip_unmask_per_cpu_irq(
-			LOCAL_MAILBOX_INT_CONTROL0, 0, i);
-	}
 	set_smp_cross_call(bcm2836_arm_irqchip_send_ipi);
+
+	smp_set_ops(&bcm2836_smp_ops);
 #endif
 }
 
-- 
2.1.4

[PATCH] irq-bcm2836: Move SMP boot to the irqchip code.

[Eric Anholt]

Signed-off-by: Eric Anholt <eric@anholt.net>
---

Thomas: The problem with delaying IPI unmasking until secondary boot
is that it means we need the secondary boot process to integrate with
the irqchip code, which seems unusual given the dearth of includes I
could find between arch/arm/mach-* SMP boot code and drivers/irqchip/
to get function prototypes.  However, since the irqchip is most of
this register space already, it might make sense to just have the SMP
boot live in drivers/irqchip/.  Here's a patch that would do that,
that could be squashed into my change.

 drivers/irqchip/irq-bcm2836.c | 57 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 50 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
index 87340b0..5f2a40e 100644
--- a/drivers/irqchip/irq-bcm2836.c
+++ b/drivers/irqchip/irq-bcm2836.c
@@ -49,14 +49,16 @@
 /* Same status bits as above, but for FIQ. */
 #define LOCAL_FIQ_PENDING0		0x070
 /*
- * Mailbox0 write-to-set bits.  There are 16 mailboxes, 4 per CPU, and
+ * Mailbox write-to-set bits.  There are 16 mailboxes, 4 per CPU, and
  * these bits are organized by mailbox number and then CPU number.  We
  * use mailbox 0 for IPIs.  The mailbox's interrupt is raised while
  * any bit is set.
  */
 #define LOCAL_MAILBOX0_SET0		0x080
+#define LOCAL_MAILBOX3_SET0		0x08c
 /* Mailbox0 write-to-clear bits. */
 #define LOCAL_MAILBOX0_CLR0		0x0c0
+#define LOCAL_MAILBOX3_CLR0		0x0cc
 
 #define LOCAL_IRQ_CNTPSIRQ	0
 #define LOCAL_IRQ_CNTPNSIRQ	1
@@ -195,6 +197,46 @@ static void bcm2836_arm_irqchip_send_ipi(const struct cpumask *mask,
 		writel(1 << ipi, mailbox0_base + 16 * cpu);
 	}
 }
+
+/* Requests boot of a secondary CPU.
+ *
+ * The Raspberry Pi firmware has already started up the CPU and set it
+ * spinning in a loop in low memory waiting for a value in mailbox 3
+ * indicating what OS code it should jump to.
+ */
+int __init bcm2836_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+	int timeout = 20;
+	void __iomem *mailbox3_set_base = intc.base + LOCAL_MAILBOX3_SET0;
+	void __iomem *mailbox3_clr_base = intc.base + LOCAL_MAILBOX3_CLR0;
+	unsigned long secondary_startup_phys =
+		(unsigned long) virt_to_phys((void *)secondary_startup);
+
+	/* Unmask IPIs to the target cpu. */
+	bcm2836_arm_irqchip_unmask_per_cpu_irq(LOCAL_MAILBOX_INT_CONTROL0, 0,
+					       cpu);
+
+	dsb();
+
+	writel(secondary_startup_phys, mailbox3_set_base + 16 * cpu);
+
+	while (true) {
+		int val = readl(mailbox3_clr_base + 16 * cpu);
+
+		if (val == 0)
+			return 0;
+		if (timeout-- == 0)
+			return -ETIMEDOUT;
+		cpu_relax();
+	}
+
+	return 0;
+}
+
+static struct smp_operations bcm2836_smp_ops __initdata = {
+	.smp_boot_secondary	= bcm2836_smp_boot_secondary,
+};
+
 #endif
 
 static const struct irq_domain_ops bcm2836_arm_irqchip_intc_ops = {
@@ -205,14 +247,15 @@ static void
 bcm2836_arm_irqchip_smp_init(void)
 {
 #ifdef CONFIG_SMP
-	int i;
+	/* Unmask IPIs to the boot CPU. Other CPUs will be unmasked as
+	 * they're brought up.
+	 */
+	bcm2836_arm_irqchip_unmask_per_cpu_irq(LOCAL_MAILBOX_INT_CONTROL0, 0,
+					       smp_processor_id());
 
-	/* unmask IPIs */
-	for_each_possible_cpu(i) {
-		bcm2836_arm_irqchip_unmask_per_cpu_irq(
-			LOCAL_MAILBOX_INT_CONTROL0, 0, i);
-	}
 	set_smp_cross_call(bcm2836_arm_irqchip_send_ipi);
+
+	smp_set_ops(&bcm2836_smp_ops);
 #endif
 }
 
-- 
2.1.4

[PATCH] irq-bcm2836: Move SMP boot to the irqchip code.

[Florian Fainelli]

On 27/07/15 11:42, Eric Anholt wrote:
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
> 
> Thomas: The problem with delaying IPI unmasking until secondary boot
> is that it means we need the secondary boot process to integrate with
> the irqchip code, which seems unusual given the dearth of includes I
> could find between arch/arm/mach-* SMP boot code and drivers/irqchip/
> to get function prototypes.  However, since the irqchip is most of
> this register space already, it might make sense to just have the SMP
> boot live in drivers/irqchip/.  Here's a patch that would do that,
> that could be squashed into my change.

I do not think this patch is going to work, there are CPU notifiers that
allows you to listen for events (CPU_ONLINE, CPU_DEAD...) as to when
(re)configuration of an interrupt controller can become necessary in
other areas of the kernel.

drivers/irqchip/irq-gic*.c contains code that deals with this for instance.

> 
>  drivers/irqchip/irq-bcm2836.c | 57 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 50 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
> index 87340b0..5f2a40e 100644
> --- a/drivers/irqchip/irq-bcm2836.c
> +++ b/drivers/irqchip/irq-bcm2836.c
> @@ -49,14 +49,16 @@
>  /* Same status bits as above, but for FIQ. */
>  #define LOCAL_FIQ_PENDING0		0x070
>  /*
> - * Mailbox0 write-to-set bits.  There are 16 mailboxes, 4 per CPU, and
> + * Mailbox write-to-set bits.  There are 16 mailboxes, 4 per CPU, and
>   * these bits are organized by mailbox number and then CPU number.  We
>   * use mailbox 0 for IPIs.  The mailbox's interrupt is raised while
>   * any bit is set.
>   */
>  #define LOCAL_MAILBOX0_SET0		0x080
> +#define LOCAL_MAILBOX3_SET0		0x08c
>  /* Mailbox0 write-to-clear bits. */
>  #define LOCAL_MAILBOX0_CLR0		0x0c0
> +#define LOCAL_MAILBOX3_CLR0		0x0cc
>  
>  #define LOCAL_IRQ_CNTPSIRQ	0
>  #define LOCAL_IRQ_CNTPNSIRQ	1
> @@ -195,6 +197,46 @@ static void bcm2836_arm_irqchip_send_ipi(const struct cpumask *mask,
>  		writel(1 << ipi, mailbox0_base + 16 * cpu);
>  	}
>  }
> +
> +/* Requests boot of a secondary CPU.
> + *
> + * The Raspberry Pi firmware has already started up the CPU and set it
> + * spinning in a loop in low memory waiting for a value in mailbox 3
> + * indicating what OS code it should jump to.
> + */
> +int __init bcm2836_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
> +{
> +	int timeout = 20;
> +	void __iomem *mailbox3_set_base = intc.base + LOCAL_MAILBOX3_SET0;
> +	void __iomem *mailbox3_clr_base = intc.base + LOCAL_MAILBOX3_CLR0;
> +	unsigned long secondary_startup_phys =
> +		(unsigned long) virt_to_phys((void *)secondary_startup);
> +
> +	/* Unmask IPIs to the target cpu. */
> +	bcm2836_arm_irqchip_unmask_per_cpu_irq(LOCAL_MAILBOX_INT_CONTROL0, 0,
> +					       cpu);
> +
> +	dsb();
> +
> +	writel(secondary_startup_phys, mailbox3_set_base + 16 * cpu);
> +
> +	while (true) {
> +		int val = readl(mailbox3_clr_base + 16 * cpu);
> +
> +		if (val == 0)
> +			return 0;
> +		if (timeout-- == 0)
> +			return -ETIMEDOUT;
> +		cpu_relax();
> +	}
> +
> +	return 0;
> +}
> +
> +static struct smp_operations bcm2836_smp_ops __initdata = {
> +	.smp_boot_secondary	= bcm2836_smp_boot_secondary,
> +};
> +
>  #endif
>  
>  static const struct irq_domain_ops bcm2836_arm_irqchip_intc_ops = {
> @@ -205,14 +247,15 @@ static void
>  bcm2836_arm_irqchip_smp_init(void)
>  {
>  #ifdef CONFIG_SMP
> -	int i;
> +	/* Unmask IPIs to the boot CPU. Other CPUs will be unmasked as
> +	 * they're brought up.
> +	 */
> +	bcm2836_arm_irqchip_unmask_per_cpu_irq(LOCAL_MAILBOX_INT_CONTROL0, 0,
> +					       smp_processor_id());
>  
> -	/* unmask IPIs */
> -	for_each_possible_cpu(i) {
> -		bcm2836_arm_irqchip_unmask_per_cpu_irq(
> -			LOCAL_MAILBOX_INT_CONTROL0, 0, i);
> -	}
>  	set_smp_cross_call(bcm2836_arm_irqchip_send_ipi);
> +
> +	smp_set_ops(&bcm2836_smp_ops);
>  #endif
>  }
>  
> 


-- 
Florian

Re: [PATCH] irq-bcm2836: Move SMP boot to the irqchip code.

[Florian Fainelli]

On 27/07/15 11:42, Eric Anholt wrote:
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
> 
> Thomas: The problem with delaying IPI unmasking until secondary boot
> is that it means we need the secondary boot process to integrate with
> the irqchip code, which seems unusual given the dearth of includes I
> could find between arch/arm/mach-* SMP boot code and drivers/irqchip/
> to get function prototypes.  However, since the irqchip is most of
> this register space already, it might make sense to just have the SMP
> boot live in drivers/irqchip/.  Here's a patch that would do that,
> that could be squashed into my change.

I do not think this patch is going to work, there are CPU notifiers that
allows you to listen for events (CPU_ONLINE, CPU_DEAD...) as to when
(re)configuration of an interrupt controller can become necessary in
other areas of the kernel.

drivers/irqchip/irq-gic*.c contains code that deals with this for instance.

> 
>  drivers/irqchip/irq-bcm2836.c | 57 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 50 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
> index 87340b0..5f2a40e 100644
> --- a/drivers/irqchip/irq-bcm2836.c
> +++ b/drivers/irqchip/irq-bcm2836.c
> @@ -49,14 +49,16 @@
>  /* Same status bits as above, but for FIQ. */
>  #define LOCAL_FIQ_PENDING0		0x070
>  /*
> - * Mailbox0 write-to-set bits.  There are 16 mailboxes, 4 per CPU, and
> + * Mailbox write-to-set bits.  There are 16 mailboxes, 4 per CPU, and
>   * these bits are organized by mailbox number and then CPU number.  We
>   * use mailbox 0 for IPIs.  The mailbox's interrupt is raised while
>   * any bit is set.
>   */
>  #define LOCAL_MAILBOX0_SET0		0x080
> +#define LOCAL_MAILBOX3_SET0		0x08c
>  /* Mailbox0 write-to-clear bits. */
>  #define LOCAL_MAILBOX0_CLR0		0x0c0
> +#define LOCAL_MAILBOX3_CLR0		0x0cc
>  
>  #define LOCAL_IRQ_CNTPSIRQ	0
>  #define LOCAL_IRQ_CNTPNSIRQ	1
> @@ -195,6 +197,46 @@ static void bcm2836_arm_irqchip_send_ipi(const struct cpumask *mask,
>  		writel(1 << ipi, mailbox0_base + 16 * cpu);
>  	}
>  }
> +
> +/* Requests boot of a secondary CPU.
> + *
> + * The Raspberry Pi firmware has already started up the CPU and set it
> + * spinning in a loop in low memory waiting for a value in mailbox 3
> + * indicating what OS code it should jump to.
> + */
> +int __init bcm2836_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
> +{
> +	int timeout = 20;
> +	void __iomem *mailbox3_set_base = intc.base + LOCAL_MAILBOX3_SET0;
> +	void __iomem *mailbox3_clr_base = intc.base + LOCAL_MAILBOX3_CLR0;
> +	unsigned long secondary_startup_phys =
> +		(unsigned long) virt_to_phys((void *)secondary_startup);
> +
> +	/* Unmask IPIs to the target cpu. */
> +	bcm2836_arm_irqchip_unmask_per_cpu_irq(LOCAL_MAILBOX_INT_CONTROL0, 0,
> +					       cpu);
> +
> +	dsb();
> +
> +	writel(secondary_startup_phys, mailbox3_set_base + 16 * cpu);
> +
> +	while (true) {
> +		int val = readl(mailbox3_clr_base + 16 * cpu);
> +
> +		if (val == 0)
> +			return 0;
> +		if (timeout-- == 0)
> +			return -ETIMEDOUT;
> +		cpu_relax();
> +	}
> +
> +	return 0;
> +}
> +
> +static struct smp_operations bcm2836_smp_ops __initdata = {
> +	.smp_boot_secondary	= bcm2836_smp_boot_secondary,
> +};
> +
>  #endif
>  
>  static const struct irq_domain_ops bcm2836_arm_irqchip_intc_ops = {
> @@ -205,14 +247,15 @@ static void
>  bcm2836_arm_irqchip_smp_init(void)
>  {
>  #ifdef CONFIG_SMP
> -	int i;
> +	/* Unmask IPIs to the boot CPU. Other CPUs will be unmasked as
> +	 * they're brought up.
> +	 */
> +	bcm2836_arm_irqchip_unmask_per_cpu_irq(LOCAL_MAILBOX_INT_CONTROL0, 0,
> +					       smp_processor_id());
>  
> -	/* unmask IPIs */
> -	for_each_possible_cpu(i) {
> -		bcm2836_arm_irqchip_unmask_per_cpu_irq(
> -			LOCAL_MAILBOX_INT_CONTROL0, 0, i);
> -	}
>  	set_smp_cross_call(bcm2836_arm_irqchip_send_ipi);
> +
> +	smp_set_ops(&bcm2836_smp_ops);
>  #endif
>  }
>  
> 


-- 
Florian

[PATCH] irqchip: bcm2836: Use a CPU notifier enable IPIs.

[Eric Anholt]

Signed-off-by: Eric Anholt <eric@anholt.net>
---

Florian: Thanks!  That looks like what we need to squash for Thomas's
feedback.  It feels a bit silly to me to avoid enabling IPIs on
not-yet-booted CPUs, despite the fact that Linux itself is what
generates IPIs.  However, I've tested it on the 2836 series and it
does work.

 drivers/irqchip/irq-bcm2836.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
index 87340b0..0bb0552 100644
--- a/drivers/irqchip/irq-bcm2836.c
+++ b/drivers/irqchip/irq-bcm2836.c
@@ -14,6 +14,7 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/cpu.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/irqchip.h>
@@ -195,6 +196,25 @@ static void bcm2836_arm_irqchip_send_ipi(const struct cpumask *mask,
 		writel(1 << ipi, mailbox0_base + 16 * cpu);
 	}
 }
+
+/* Unmasks the IPI on the CPU wen it's first brought online. */
+static int bcm2836_arm_irqchip_cpu_notify(struct notifier_block *nfb,
+					  unsigned long action, void *hcpu)
+{
+	unsigned int cpu = (unsigned long)hcpu;
+	unsigned int int_reg = LOCAL_MAILBOX_INT_CONTROL0;
+	unsigned int mailbox = 0;
+
+	if (action == CPU_STARTING || action == CPU_STARTING_FROZEN)
+		bcm2836_arm_irqchip_unmask_per_cpu_irq(int_reg, mailbox, cpu);
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block bcm2836_arm_irqchip_cpu_notifier = {
+	.notifier_call = bcm2836_arm_irqchip_cpu_notify,
+	.priority = 100,
+};
 #endif
 
 static const struct irq_domain_ops bcm2836_arm_irqchip_intc_ops = {
@@ -205,13 +225,12 @@ static void
 bcm2836_arm_irqchip_smp_init(void)
 {
 #ifdef CONFIG_SMP
-	int i;
+	/* Unmask IPIs to the boot CPU. */
+	bcm2836_arm_irqchip_cpu_notify(&bcm2836_arm_irqchip_cpu_notifier,
+				       CPU_STARTING,
+				       (void *)smp_processor_id());
+	register_cpu_notifier(&bcm2836_arm_irqchip_cpu_notifier);
 
-	/* unmask IPIs */
-	for_each_possible_cpu(i) {
-		bcm2836_arm_irqchip_unmask_per_cpu_irq(
-			LOCAL_MAILBOX_INT_CONTROL0, 0, i);
-	}
 	set_smp_cross_call(bcm2836_arm_irqchip_send_ipi);
 #endif
 }
-- 
2.1.4

[PATCH] irqchip: bcm2836: Use a CPU notifier enable IPIs.

[Eric Anholt]

Signed-off-by: Eric Anholt <eric@anholt.net>
---

Florian: Thanks!  That looks like what we need to squash for Thomas's
feedback.  It feels a bit silly to me to avoid enabling IPIs on
not-yet-booted CPUs, despite the fact that Linux itself is what
generates IPIs.  However, I've tested it on the 2836 series and it
does work.

 drivers/irqchip/irq-bcm2836.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
index 87340b0..0bb0552 100644
--- a/drivers/irqchip/irq-bcm2836.c
+++ b/drivers/irqchip/irq-bcm2836.c
@@ -14,6 +14,7 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/cpu.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/irqchip.h>
@@ -195,6 +196,25 @@ static void bcm2836_arm_irqchip_send_ipi(const struct cpumask *mask,
 		writel(1 << ipi, mailbox0_base + 16 * cpu);
 	}
 }
+
+/* Unmasks the IPI on the CPU wen it's first brought online. */
+static int bcm2836_arm_irqchip_cpu_notify(struct notifier_block *nfb,
+					  unsigned long action, void *hcpu)
+{
+	unsigned int cpu = (unsigned long)hcpu;
+	unsigned int int_reg = LOCAL_MAILBOX_INT_CONTROL0;
+	unsigned int mailbox = 0;
+
+	if (action == CPU_STARTING || action == CPU_STARTING_FROZEN)
+		bcm2836_arm_irqchip_unmask_per_cpu_irq(int_reg, mailbox, cpu);
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block bcm2836_arm_irqchip_cpu_notifier = {
+	.notifier_call = bcm2836_arm_irqchip_cpu_notify,
+	.priority = 100,
+};
 #endif
 
 static const struct irq_domain_ops bcm2836_arm_irqchip_intc_ops = {
@@ -205,13 +225,12 @@ static void
 bcm2836_arm_irqchip_smp_init(void)
 {
 #ifdef CONFIG_SMP
-	int i;
+	/* Unmask IPIs to the boot CPU. */
+	bcm2836_arm_irqchip_cpu_notify(&bcm2836_arm_irqchip_cpu_notifier,
+				       CPU_STARTING,
+				       (void *)smp_processor_id());
+	register_cpu_notifier(&bcm2836_arm_irqchip_cpu_notifier);
 
-	/* unmask IPIs */
-	for_each_possible_cpu(i) {
-		bcm2836_arm_irqchip_unmask_per_cpu_irq(
-			LOCAL_MAILBOX_INT_CONTROL0, 0, i);
-	}
 	set_smp_cross_call(bcm2836_arm_irqchip_send_ipi);
 #endif
 }
-- 
2.1.4

[PATCH] irqchip: bcm2836: Use a CPU notifier enable IPIs.

[Thomas Gleixner]

On Mon, 27 Jul 2015, Eric Anholt wrote:
> +/* Unmasks the IPI on the CPU wen it's first brought online. */

when

> +static int bcm2836_arm_irqchip_cpu_notify(struct notifier_block *nfb,
> +					  unsigned long action, void *hcpu)
> +{
> +	unsigned int cpu = (unsigned long)hcpu;
> +	unsigned int int_reg = LOCAL_MAILBOX_INT_CONTROL0;
> +	unsigned int mailbox = 0;
> +
> +	if (action == CPU_STARTING || action == CPU_STARTING_FROZEN)
> +		bcm2836_arm_irqchip_unmask_per_cpu_irq(int_reg, mailbox, cpu);

Shouldn't you mask the irq on CPU_DYING?

Thanks,

	tglx

Re: [PATCH] irqchip: bcm2836: Use a CPU notifier enable IPIs.

[Thomas Gleixner]

On Mon, 27 Jul 2015, Eric Anholt wrote:
> +/* Unmasks the IPI on the CPU wen it's first brought online. */

when

> +static int bcm2836_arm_irqchip_cpu_notify(struct notifier_block *nfb,
> +					  unsigned long action, void *hcpu)
> +{
> +	unsigned int cpu = (unsigned long)hcpu;
> +	unsigned int int_reg = LOCAL_MAILBOX_INT_CONTROL0;
> +	unsigned int mailbox = 0;
> +
> +	if (action == CPU_STARTING || action == CPU_STARTING_FROZEN)
> +		bcm2836_arm_irqchip_unmask_per_cpu_irq(int_reg, mailbox, cpu);

Shouldn't you mask the irq on CPU_DYING?

Thanks,

	tglx

Re: [PATCH] irqchip: bcm2836: Use a CPU notifier enable IPIs.

[Thomas Gleixner]

On Mon, 27 Jul 2015, Eric Anholt wrote:
> +/* Unmasks the IPI on the CPU wen it's first brought online. */

when

> +static int bcm2836_arm_irqchip_cpu_notify(struct notifier_block *nfb,
> +					  unsigned long action, void *hcpu)
> +{
> +	unsigned int cpu = (unsigned long)hcpu;
> +	unsigned int int_reg = LOCAL_MAILBOX_INT_CONTROL0;
> +	unsigned int mailbox = 0;
> +
> +	if (action == CPU_STARTING || action == CPU_STARTING_FROZEN)
> +		bcm2836_arm_irqchip_unmask_per_cpu_irq(int_reg, mailbox, cpu);

Shouldn't you mask the irq on CPU_DYING?

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] irqchip: bcm2836: Use a CPU notifier enable IPIs.

[Eric Anholt]

Thomas Gleixner <tglx@linutronix.de> writes:

> On Mon, 27 Jul 2015, Eric Anholt wrote:
>> +/* Unmasks the IPI on the CPU wen it's first brought online. */
>
> when
>
>> +static int bcm2836_arm_irqchip_cpu_notify(struct notifier_block *nfb,
>> +					  unsigned long action, void *hcpu)
>> +{
>> +	unsigned int cpu = (unsigned long)hcpu;
>> +	unsigned int int_reg = LOCAL_MAILBOX_INT_CONTROL0;
>> +	unsigned int mailbox = 0;
>> +
>> +	if (action == CPU_STARTING || action == CPU_STARTING_FROZEN)
>> +		bcm2836_arm_irqchip_unmask_per_cpu_irq(int_reg, mailbox, cpu);
>
> Shouldn't you mask the irq on CPU_DYING?

I was just following what other drivers were doing.  Is CPU_DYING the
only thing that needs masking?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150803/f573d5f4/attachment.sig>

Re: [PATCH] irqchip: bcm2836: Use a CPU notifier enable IPIs.

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 711 bytes --]

Thomas Gleixner <tglx@linutronix.de> writes:

> On Mon, 27 Jul 2015, Eric Anholt wrote:
>> +/* Unmasks the IPI on the CPU wen it's first brought online. */
>
> when
>
>> +static int bcm2836_arm_irqchip_cpu_notify(struct notifier_block *nfb,
>> +					  unsigned long action, void *hcpu)
>> +{
>> +	unsigned int cpu = (unsigned long)hcpu;
>> +	unsigned int int_reg = LOCAL_MAILBOX_INT_CONTROL0;
>> +	unsigned int mailbox = 0;
>> +
>> +	if (action == CPU_STARTING || action == CPU_STARTING_FROZEN)
>> +		bcm2836_arm_irqchip_unmask_per_cpu_irq(int_reg, mailbox, cpu);
>
> Shouldn't you mask the irq on CPU_DYING?

I was just following what other drivers were doing.  Is CPU_DYING the
only thing that needs masking?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] irqchip: bcm2836: Use a CPU notifier enable IPIs.

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 737 bytes --]

Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> writes:

> On Mon, 27 Jul 2015, Eric Anholt wrote:
>> +/* Unmasks the IPI on the CPU wen it's first brought online. */
>
> when
>
>> +static int bcm2836_arm_irqchip_cpu_notify(struct notifier_block *nfb,
>> +					  unsigned long action, void *hcpu)
>> +{
>> +	unsigned int cpu = (unsigned long)hcpu;
>> +	unsigned int int_reg = LOCAL_MAILBOX_INT_CONTROL0;
>> +	unsigned int mailbox = 0;
>> +
>> +	if (action == CPU_STARTING || action == CPU_STARTING_FROZEN)
>> +		bcm2836_arm_irqchip_unmask_per_cpu_irq(int_reg, mailbox, cpu);
>
> Shouldn't you mask the irq on CPU_DYING?

I was just following what other drivers were doing.  Is CPU_DYING the
only thing that needs masking?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

[PATCH] irqchip: bcm2836: Use a CPU notifier enable IPIs.

[Thomas Gleixner]

On Mon, 3 Aug 2015, Eric Anholt wrote:

> Thomas Gleixner <tglx@linutronix.de> writes:
> 
> > On Mon, 27 Jul 2015, Eric Anholt wrote:
> >> +/* Unmasks the IPI on the CPU wen it's first brought online. */
> >
> > when
> >
> >> +static int bcm2836_arm_irqchip_cpu_notify(struct notifier_block *nfb,
> >> +					  unsigned long action, void *hcpu)
> >> +{
> >> +	unsigned int cpu = (unsigned long)hcpu;
> >> +	unsigned int int_reg = LOCAL_MAILBOX_INT_CONTROL0;
> >> +	unsigned int mailbox = 0;
> >> +
> >> +	if (action == CPU_STARTING || action == CPU_STARTING_FROZEN)
> >> +		bcm2836_arm_irqchip_unmask_per_cpu_irq(int_reg, mailbox, cpu);
> >
> > Shouldn't you mask the irq on CPU_DYING?
> 
> I was just following what other drivers were doing.  Is CPU_DYING the
> only thing that needs masking?

CPPU_DYING is the counterpart of CPU_STARTING. It's called from the
CPU which goes down.

Thanks,

	tglx

Re: [PATCH] irqchip: bcm2836: Use a CPU notifier enable IPIs.

[Thomas Gleixner]

On Mon, 3 Aug 2015, Eric Anholt wrote:

> Thomas Gleixner <tglx@linutronix.de> writes:
> 
> > On Mon, 27 Jul 2015, Eric Anholt wrote:
> >> +/* Unmasks the IPI on the CPU wen it's first brought online. */
> >
> > when
> >
> >> +static int bcm2836_arm_irqchip_cpu_notify(struct notifier_block *nfb,
> >> +					  unsigned long action, void *hcpu)
> >> +{
> >> +	unsigned int cpu = (unsigned long)hcpu;
> >> +	unsigned int int_reg = LOCAL_MAILBOX_INT_CONTROL0;
> >> +	unsigned int mailbox = 0;
> >> +
> >> +	if (action == CPU_STARTING || action == CPU_STARTING_FROZEN)
> >> +		bcm2836_arm_irqchip_unmask_per_cpu_irq(int_reg, mailbox, cpu);
> >
> > Shouldn't you mask the irq on CPU_DYING?
> 
> I was just following what other drivers were doing.  Is CPU_DYING the
> only thing that needs masking?

CPPU_DYING is the counterpart of CPU_STARTING. It's called from the
CPU which goes down.

Thanks,

	tglx

Re: [PATCH] irqchip: bcm2836: Use a CPU notifier enable IPIs.

[Thomas Gleixner]

On Mon, 3 Aug 2015, Eric Anholt wrote:

> Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> writes:
> 
> > On Mon, 27 Jul 2015, Eric Anholt wrote:
> >> +/* Unmasks the IPI on the CPU wen it's first brought online. */
> >
> > when
> >
> >> +static int bcm2836_arm_irqchip_cpu_notify(struct notifier_block *nfb,
> >> +					  unsigned long action, void *hcpu)
> >> +{
> >> +	unsigned int cpu = (unsigned long)hcpu;
> >> +	unsigned int int_reg = LOCAL_MAILBOX_INT_CONTROL0;
> >> +	unsigned int mailbox = 0;
> >> +
> >> +	if (action == CPU_STARTING || action == CPU_STARTING_FROZEN)
> >> +		bcm2836_arm_irqchip_unmask_per_cpu_irq(int_reg, mailbox, cpu);
> >
> > Shouldn't you mask the irq on CPU_DYING?
> 
> I was just following what other drivers were doing.  Is CPU_DYING the
> only thing that needs masking?

CPPU_DYING is the counterpart of CPU_STARTING. It's called from the
CPU which goes down.

Thanks,

	tglx

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html