On 01/27/2015 08:30 PM, Bob Copeland wrote:
On Tue, Jan 27, 2015 at 03:53:00PM -0800, Peter Oh wrote:
- /* IMPORTANT: this extra read transaction is required to
- * flush the posted write buffer. */
- (void)ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS +
- PCIE_INTR_ENABLE_ADDRESS);
+ /* invoke data sync barrier */
+ wmb();
}
I am no expert in arcane PCI matters, but that looks suspicious to me. I seem
to recall wmb() only enforced ordering, and maybe not even memory-IO ordering
on all platforms. If you want to disable an irq, it really seems like you
would want to flush posted writes so you know the hardware has seen it.
enforced ordering is happened by flush write buffer and wmb is
commonly used to flush write buffer.
so that wmb guarantees ordering by flush write buffer. That's why
it's called a memory barrier.
Ok, sure, but I/O ordering two different writes, and ensuring device
has seen a posted write, are related but different things, no?
yes, they are different and wmb guarantees both.
So if
you are disabling an interrupt and want to be really sure interrupts
are off when function returns, I think you still want the read.
the mechanism that read is using to make sure write work done is the
same mechanism that wmb is using.
Anyway, it's your driver, just pointing out that the "IMPORTANT"
read might still be important to someone.
I really appreciate your opinion.
Regards,
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
