[PATCH 0/4] irqchip: bcm2835: arm64 port

[PATCH 0/4] irqchip: bcm2835: arm64 port

[Eric Anholt]

Here's the series for irqchip-bcm283x on arm64 for the Pi3.  Since I'd
like to land Makefile changes that would enable building
irqchip-bcm283x on arm64, it would be nice if I had either a stable
branch to merge from, or an ack to merge them through the arm64 tree.

Eric Anholt (4):
  irqchip: bcm2835: Avoid arch/arm-specific handle_IRQ
  irqchip: bcm2836: Drop smp_set_ops on arm64 builds
  irqchip: bcm2836: Fix compiler warning on 64-bit build
  irqchip: bcm2836: Use a more generic memory barrier call

 drivers/irqchip/irq-bcm2835.c |  3 +--
 drivers/irqchip/irq-bcm2836.c | 12 ++++++++----
 2 files changed, 9 insertions(+), 6 deletions(-)

-- 
2.7.0

[PATCH 1/4] irqchip: bcm2835: Avoid arch/arm-specific handle_IRQ

[Eric Anholt]

This is equivalent and works for arm64 as well.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/irqchip/irq-bcm2835.c | 3 +--
 drivers/irqchip/irq-bcm2836.c | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-bcm2835.c b/drivers/irqchip/irq-bcm2835.c
index bf9cc5f..44d7c38 100644
--- a/drivers/irqchip/irq-bcm2835.c
+++ b/drivers/irqchip/irq-bcm2835.c
@@ -52,7 +52,6 @@
 #include <linux/irqdomain.h>
 
 #include <asm/exception.h>
-#include <asm/mach/irq.h>
 
 /* Put the bank and irq (32 bits) into the hwirq */
 #define MAKE_HWIRQ(b, n)	((b << 5) | (n))
@@ -242,7 +241,7 @@ static void __exception_irq_entry bcm2835_handle_irq(
 	u32 hwirq;
 
 	while ((hwirq = get_next_armctrl_hwirq()) != ~0)
-		handle_IRQ(irq_linear_revmap(intc.domain, hwirq), regs);
+		handle_domain_irq(intc.domain, hwirq, regs);
 }
 
 static void bcm2836_chained_handle_irq(struct irq_desc *desc)
diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
index b6e950d..233ccdd 100644
--- a/drivers/irqchip/irq-bcm2836.c
+++ b/drivers/irqchip/irq-bcm2836.c
@@ -180,7 +180,7 @@ __exception_irq_entry bcm2836_arm_irqchip_handle_irq(struct pt_regs *regs)
 	} else if (stat) {
 		u32 hwirq = ffs(stat) - 1;
 
-		handle_IRQ(irq_linear_revmap(intc.domain, hwirq), regs);
+		handle_domain_irq(intc.domain, hwirq, regs);
 	}
 }
 
-- 
2.7.0

[PATCH 2/4] irqchip: bcm2836: Drop smp_set_ops on arm64 builds

[Eric Anholt]

For arm64, the bootloader will instead be implementing the spin-table
enable method.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/irqchip/irq-bcm2836.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
index 233ccdd..4ae9f76 100644
--- a/drivers/irqchip/irq-bcm2836.c
+++ b/drivers/irqchip/irq-bcm2836.c
@@ -223,6 +223,7 @@ static struct notifier_block bcm2836_arm_irqchip_cpu_notifier = {
 	.priority = 100,
 };
 
+#ifdef ARM
 int __init bcm2836_smp_boot_secondary(unsigned int cpu,
 				      struct task_struct *idle)
 {
@@ -238,7 +239,7 @@ int __init bcm2836_smp_boot_secondary(unsigned int cpu,
 static const struct smp_operations bcm2836_smp_ops __initconst = {
 	.smp_boot_secondary	= bcm2836_smp_boot_secondary,
 };
-
+#endif
 #endif
 
 static const struct irq_domain_ops bcm2836_arm_irqchip_intc_ops = {
@@ -256,8 +257,11 @@ bcm2836_arm_irqchip_smp_init(void)
 	register_cpu_notifier(&bcm2836_arm_irqchip_cpu_notifier);
 
 	set_smp_cross_call(bcm2836_arm_irqchip_send_ipi);
+
+#ifdef ARM
 	smp_set_ops(&bcm2836_smp_ops);
 #endif
+#endif
 }
 
 /*
-- 
2.7.0

[PATCH 2/4] irqchip: bcm2836: Drop smp_set_ops on arm64 builds

[Marc Zyngier]

Eric,

On 05/04/16 04:44, Eric Anholt wrote:
> For arm64, the bootloader will instead be implementing the spin-table
> enable method.

You may also want to add that SMP ops simply do not exist on arm64,
hence the bootloader [...].

> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  drivers/irqchip/irq-bcm2836.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
> index 233ccdd..4ae9f76 100644
> --- a/drivers/irqchip/irq-bcm2836.c
> +++ b/drivers/irqchip/irq-bcm2836.c
> @@ -223,6 +223,7 @@ static struct notifier_block bcm2836_arm_irqchip_cpu_notifier = {
>  	.priority = 100,
>  };
>  
> +#ifdef ARM

That should really be CONFIG_ARM.

>  int __init bcm2836_smp_boot_secondary(unsigned int cpu,
>  				      struct task_struct *idle)
>  {
> @@ -238,7 +239,7 @@ int __init bcm2836_smp_boot_secondary(unsigned int cpu,
>  static const struct smp_operations bcm2836_smp_ops __initconst = {
>  	.smp_boot_secondary	= bcm2836_smp_boot_secondary,
>  };
> -
> +#endif
>  #endif
>  
>  static const struct irq_domain_ops bcm2836_arm_irqchip_intc_ops = {
> @@ -256,8 +257,11 @@ bcm2836_arm_irqchip_smp_init(void)
>  	register_cpu_notifier(&bcm2836_arm_irqchip_cpu_notifier);
>  
>  	set_smp_cross_call(bcm2836_arm_irqchip_send_ipi);
> +
> +#ifdef ARM
>  	smp_set_ops(&bcm2836_smp_ops);
>  #endif
> +#endif
>  }
>  
>  /*
> 

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 2/4] irqchip: bcm2836: Drop smp_set_ops on arm64 builds

[Arnd Bergmann]

On Tuesday 05 April 2016, Eric Anholt wrote:
> For arm64, the bootloader will instead be implementing the spin-table
> enable method.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  drivers/irqchip/irq-bcm2836.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
> index 233ccdd..4ae9f76 100644
> --- a/drivers/irqchip/irq-bcm2836.c
> +++ b/drivers/irqchip/irq-bcm2836.c
> @@ -223,6 +223,7 @@ static struct notifier_block bcm2836_arm_irqchip_cpu_notifier = {
>  	.priority = 100,
>  };
>  
> +#ifdef ARM
>  int __init bcm2836_smp_boot_secondary(unsigned int cpu,
>  				      struct task_struct *idle)
>  {
> @@ -238,7 +239,7 @@ int __init bcm2836_smp_boot_secondary(unsigned int cpu,
>  static const struct smp_operations bcm2836_smp_ops __initconst = {
>  	.smp_boot_secondary	= bcm2836_smp_boot_secondary,
>  };
> -
> +#endif
>  #endif
>  
>  static const struct irq_domain_ops bcm2836_arm_irqchip_intc_ops = {
> @@ -256,8 +257,11 @@ bcm2836_arm_irqchip_smp_init(void)
>  	register_cpu_notifier(&bcm2836_arm_irqchip_cpu_notifier);
>  
>  	set_smp_cross_call(bcm2836_arm_irqchip_send_ipi);
> +
> +#ifdef ARM
>  	smp_set_ops(&bcm2836_smp_ops);
>  #endif
> +#endif
>  }

I'd suggest instead using CPU_METHOD_OF_DECLARE and moving the SMP code
to arch/arm/mach-bcm/platsmp-bcm2835.c. It doesn't really belong in the
irqchip code.

	Arnd

[PATCH 2/4] irqchip: bcm2836: Drop smp_set_ops on arm64 builds

[Eric Anholt]

Arnd Bergmann <arnd@arndb.de> writes:

> On Tuesday 05 April 2016, Eric Anholt wrote:
>> For arm64, the bootloader will instead be implementing the spin-table
>> enable method.
>> 
>> Signed-off-by: Eric Anholt <eric@anholt.net>
>> ---
>>  drivers/irqchip/irq-bcm2836.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
>> index 233ccdd..4ae9f76 100644
>> --- a/drivers/irqchip/irq-bcm2836.c
>> +++ b/drivers/irqchip/irq-bcm2836.c
>> @@ -223,6 +223,7 @@ static struct notifier_block bcm2836_arm_irqchip_cpu_notifier = {
>>  	.priority = 100,
>>  };
>>  
>> +#ifdef ARM
>>  int __init bcm2836_smp_boot_secondary(unsigned int cpu,
>>  				      struct task_struct *idle)
>>  {
>> @@ -238,7 +239,7 @@ int __init bcm2836_smp_boot_secondary(unsigned int cpu,
>>  static const struct smp_operations bcm2836_smp_ops __initconst = {
>>  	.smp_boot_secondary	= bcm2836_smp_boot_secondary,
>>  };
>> -
>> +#endif
>>  #endif
>>  
>>  static const struct irq_domain_ops bcm2836_arm_irqchip_intc_ops = {
>> @@ -256,8 +257,11 @@ bcm2836_arm_irqchip_smp_init(void)
>>  	register_cpu_notifier(&bcm2836_arm_irqchip_cpu_notifier);
>>  
>>  	set_smp_cross_call(bcm2836_arm_irqchip_send_ipi);
>> +
>> +#ifdef ARM
>>  	smp_set_ops(&bcm2836_smp_ops);
>>  #endif
>> +#endif
>>  }
>
> I'd suggest instead using CPU_METHOD_OF_DECLARE and moving the SMP code
> to arch/arm/mach-bcm/platsmp-bcm2835.c. It doesn't really belong in the
> irqchip code.

I think because of DT ABI (sigh) we're stuck with the current
implementation.

FWIW, I had started with the SMP bits in a platsmp.c, but moved it into
the irqchip because that was so much simpler than reaching over into the
irqchip node to map its registers.
-------------- 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/20160413/714a26ec/attachment.sig>

[PATCH 3/4] irqchip: bcm2836: Fix compiler warning on 64-bit build

[Eric Anholt]

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/irqchip/irq-bcm2836.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
index 4ae9f76..ee62413 100644
--- a/drivers/irqchip/irq-bcm2836.c
+++ b/drivers/irqchip/irq-bcm2836.c
@@ -253,7 +253,7 @@ bcm2836_arm_irqchip_smp_init(void)
 	/* Unmask IPIs to the boot CPU. */
 	bcm2836_arm_irqchip_cpu_notify(&bcm2836_arm_irqchip_cpu_notifier,
 				       CPU_STARTING,
-				       (void *)smp_processor_id());
+				       (void *)(uintptr_t)smp_processor_id());
 	register_cpu_notifier(&bcm2836_arm_irqchip_cpu_notifier);
 
 	set_smp_cross_call(bcm2836_arm_irqchip_send_ipi);
-- 
2.7.0

[PATCH 4/4] irqchip: bcm2836: Use a more generic memory barrier call

[Eric Anholt]

dsb() requires an argument on arm64, so we needed to add "sy".
Instead, take this opportunity to switch to the same smp_wmb() call
that gic uses for its IPIs.  This is a less strong barrier than we
were doing before (dmb(ishst) compared to dsb(sy)), but it seems to be
the correct one.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/irqchip/irq-bcm2836.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
index ee62413..a99b630 100644
--- a/drivers/irqchip/irq-bcm2836.c
+++ b/drivers/irqchip/irq-bcm2836.c
@@ -195,7 +195,7 @@ static void bcm2836_arm_irqchip_send_ipi(const struct cpumask *mask,
 	 * Ensure that stores to normal memory are visible to the
 	 * other CPUs before issuing the IPI.
 	 */
-	dsb();
+	smp_wmb();
 
 	for_each_cpu(cpu, mask)	{
 		writel(1 << ipi, mailbox0_base + 16 * cpu);
-- 
2.7.0

[PATCH 4/4] irqchip: bcm2836: Use a more generic memory barrier call

[Stephen Warren]

On 04/04/2016 09:44 PM, Eric Anholt wrote:
> dsb() requires an argument on arm64, so we needed to add "sy".
> Instead, take this opportunity to switch to the same smp_wmb() call
> that gic uses for its IPIs.  This is a less strong barrier than we
> were doing before (dmb(ishst) compared to dsb(sy)), but it seems to be
> the correct one.

I assume all MMIO is part of the ish domain?

If so, the series,
Acked-by: Stephen Warren <swarren@wwwdotorg.org>

[PATCH 4/4] irqchip: bcm2836: Use a more generic memory barrier call

[Eric Anholt]

Stephen Warren <swarren@wwwdotorg.org> writes:

> On 04/04/2016 09:44 PM, Eric Anholt wrote:
>> dsb() requires an argument on arm64, so we needed to add "sy".
>> Instead, take this opportunity to switch to the same smp_wmb() call
>> that gic uses for its IPIs.  This is a less strong barrier than we
>> were doing before (dmb(ishst) compared to dsb(sy)), but it seems to be
>> the correct one.
>
> I assume all MMIO is part of the ish domain?
>
> If so, the series,
> Acked-by: Stephen Warren <swarren@wwwdotorg.org>

I don't know if this barrier implies ordering all the way out to AXI on
this HW, but I don't think that's a requirement of this function.
-------------- 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/20160408/bd4f2397/attachment.sig>

[PATCH 4/4] irqchip: bcm2836: Use a more generic memory barrier call

[Stephen Warren]

On 04/08/2016 12:20 PM, Eric Anholt wrote:
> Stephen Warren <swarren@wwwdotorg.org> writes:
>
>> On 04/04/2016 09:44 PM, Eric Anholt wrote:
>>> dsb() requires an argument on arm64, so we needed to add "sy".
>>> Instead, take this opportunity to switch to the same smp_wmb() call
>>> that gic uses for its IPIs.  This is a less strong barrier than we
>>> were doing before (dmb(ishst) compared to dsb(sy)), but it seems to be
>>> the correct one.
>>
>> I assume all MMIO is part of the ish domain?
>>
>> If so, the series,
>> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
>
> I don't know if this barrier implies ordering all the way out to AXI on
> this HW, but I don't think that's a requirement of this function.

My understanding was that the barrier was explicitly to work around a 
bug in the bus fabric of the SoC, and hence the barrier very much does 
have to affect the transaction all the way out to AXI. Re-reading 
BCM2835-ARM-Peripherals.pdf section 1.3 "Peripheral access precautions 
for correct memory ordering" seems to confirm this.

[PATCH 4/4] irqchip: bcm2836: Use a more generic memory barrier call

[Eric Anholt]

Stephen Warren <swarren@wwwdotorg.org> writes:

> On 04/08/2016 12:20 PM, Eric Anholt wrote:
>> Stephen Warren <swarren@wwwdotorg.org> writes:
>>
>>> On 04/04/2016 09:44 PM, Eric Anholt wrote:
>>>> dsb() requires an argument on arm64, so we needed to add "sy".
>>>> Instead, take this opportunity to switch to the same smp_wmb() call
>>>> that gic uses for its IPIs.  This is a less strong barrier than we
>>>> were doing before (dmb(ishst) compared to dsb(sy)), but it seems to be
>>>> the correct one.
>>>
>>> I assume all MMIO is part of the ish domain?
>>>
>>> If so, the series,
>>> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
>>
>> I don't know if this barrier implies ordering all the way out to AXI on
>> this HW, but I don't think that's a requirement of this function.
>
> My understanding was that the barrier was explicitly to work around a 
> bug in the bus fabric of the SoC, and hence the barrier very much does 
> have to affect the transaction all the way out to AXI. Re-reading 
> BCM2835-ARM-Peripherals.pdf section 1.3 "Peripheral access precautions 
> for correct memory ordering" seems to confirm this.

My understanding of the explicit barrier here, which was copied from
other irqchips, is "Make sure that normal memory writes before our IPI
on this CPU appear on the other CPUs before they get the IPI" (like the
comment says).  This barrier was not put in to deal with the
283x-specific weird AXI behavior.

Note that we had previously decided that the weird AXI ordering
behavior, which is about repeated reads or repeated writes from the same
CPU across different peripherals, is already covered by the barriers
present in readl() and writel().  The writel() barrier happens to be a
dsb() as well, so this explicit barrier is actually redundant.
-------------- 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/20160410/cdaf3077/attachment.sig>

[PATCH 4/4] irqchip: bcm2836: Use a more generic memory barrier call

[Stephen Warren]

On 04/10/2016 12:32 PM, Eric Anholt wrote:
> Stephen Warren <swarren@wwwdotorg.org> writes:
>
>> On 04/08/2016 12:20 PM, Eric Anholt wrote:
>>> Stephen Warren <swarren@wwwdotorg.org> writes:
>>>
>>>> On 04/04/2016 09:44 PM, Eric Anholt wrote:
>>>>> dsb() requires an argument on arm64, so we needed to add "sy".
>>>>> Instead, take this opportunity to switch to the same smp_wmb() call
>>>>> that gic uses for its IPIs.  This is a less strong barrier than we
>>>>> were doing before (dmb(ishst) compared to dsb(sy)), but it seems to be
>>>>> the correct one.
>>>>
>>>> I assume all MMIO is part of the ish domain?
>>>>
>>>> If so, the series,
>>>> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
>>>
>>> I don't know if this barrier implies ordering all the way out to AXI on
>>> this HW, but I don't think that's a requirement of this function.
>>
>> My understanding was that the barrier was explicitly to work around a
>> bug in the bus fabric of the SoC, and hence the barrier very much does
>> have to affect the transaction all the way out to AXI. Re-reading
>> BCM2835-ARM-Peripherals.pdf section 1.3 "Peripheral access precautions
>> for correct memory ordering" seems to confirm this.
>
> My understanding of the explicit barrier here, which was copied from
> other irqchips, is "Make sure that normal memory writes before our IPI
> on this CPU appear on the other CPUs before they get the IPI" (like the
> comment says).  This barrier was not put in to deal with the
> 283x-specific weird AXI behavior.
>
> Note that we had previously decided that the weird AXI ordering
> behavior, which is about repeated reads or repeated writes from the same
> CPU across different peripherals, is already covered by the barriers
> present in readl() and writel().  The writel() barrier happens to be a
> dsb() as well, so this explicit barrier is actually redundant.

Ah OK. In that case, the change seems fine.