Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1371173 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 4/4] irqchip: bcm2836: Use a more generic memory barrier call Eric Anholt <eric@anholt.net> - 2016-04-05 05:50 +0200
    Re: [PATCH 4/4] irqchip: bcm2836: Use a more generic memory barrier  call Stephen Warren <swarren@wwwdotorg.org> - 2016-04-06 07:10 +0200
      Re: [PATCH 4/4] irqchip: bcm2836: Use a more generic memory barrier call Eric Anholt <eric@anholt.net> - 2016-04-08 20:30 +0200
        Re: [PATCH 4/4] irqchip: bcm2836: Use a more generic memory barrier  call Stephen Warren <swarren@wwwdotorg.org> - 2016-04-09 07:30 +0200
          Re: [PATCH 4/4] irqchip: bcm2836: Use a more generic memory barrier call Eric Anholt <eric@anholt.net> - 2016-04-10 20:40 +0200
            Re: [PATCH 4/4] irqchip: bcm2836: Use a more generic memory barrier  call Stephen Warren <swarren@wwwdotorg.org> - 2016-04-11 18:00 +0200

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

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

[toc] | [next] | [standalone]

#1372185 — Re: [PATCH 4/4] irqchip: bcm2836: Use a more generic memory barrier call

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>

[toc] | [prev] | [next] | [standalone]

#1374404

[Multipart message — attachments visible in raw view] — view raw

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.

[toc] | [prev] | [next] | [standalone]

#1374574 — Re: [PATCH 4/4] irqchip: bcm2836: Use a more generic memory barrier call

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.

[toc] | [prev] | [next] | [standalone]

#1374831

[Multipart message — attachments visible in raw view] — view raw

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.

[toc] | [prev] | [next] | [standalone]

#1376134 — Re: [PATCH 4/4] irqchip: bcm2836: Use a more generic memory barrier call

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web