[PATCH + an old question] firewire: ohci: use memory barriers toorder descriptor updates

From: Stefan Richter
Date: Tue Jul 27 2010 - 07:20:50 EST

When we append to a DMA program, we need to ensure that the PCI device
sees the updates in the intended order. We need:

1. a write memory barrier between initialization of new descriptors and
the update of the last old descriptor to point to the new
descriptors (i.e. branch_address update),

2. a write memory barrier between branch_address update and wake-up of
the DMA unit by MMIO register write.

This patch adds only barrier 1.

Barrier 2 is implicit in writel() on most machines --- or at least I
think it is. See this from arch/alpha/include/asm/io.h:

#define build_mmio_write(name, size, type, reg, barrier) \
static inline void name(type val, volatile void __iomem *addr) \
{ asm volatile("mov" size " %0,%1": :reg (val), \
"m" (*(volatile type __force *)addr) barrier); }

build_mmio_write(writel, "l", unsigned int, "r", :"memory")

Does this order the mmio write relative to previous memory writes?

I am not so sure about barrier semantics of writel() on some less
popular architectures. From arch/alpha/include/asm/io.h:

extern inline void writel(u32 b, volatile void __iomem *addr)
__raw_writel(b, addr);

This mb() is nice but we need a barrier in front of the __raw_writel.
Somebody who cares might want to add it in the architecture code or in
hundreds of drivers.

Signed-off-by: Stefan Richter <stefanr@xxxxxxxxxxxxxxxxx>
drivers/firewire/ohci.c | 3 +++
1 file changed, 3 insertions(+)

Index: b/drivers/firewire/ohci.c
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -595,6 +595,7 @@ static int ar_context_add_page(struct ar
ab->descriptor.res_count = cpu_to_le16(PAGE_SIZE - offset);
ab->descriptor.branch_address = 0;

+ wmb(); /* finish init of new descriptors before branch_address update */
ctx->last_buffer->descriptor.branch_address = cpu_to_le32(ab_bus | 1);
ctx->last_buffer->next = ab;
ctx->last_buffer = ab;
@@ -982,6 +983,8 @@ static void context_append(struct contex
d_bus = desc->buffer_bus + (d - desc->buffer) * sizeof(*d);

desc->used += (z + extra) * sizeof(*d);
+ wmb(); /* finish init of new descriptors before branch_address update */
ctx->prev->branch_address = cpu_to_le32(d_bus | z);
ctx->prev = find_branch_descriptor(d, z);

Stefan Richter
-=====-==-=- -=== ==-==

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/