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? 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. Anyway, it's your driver, just pointing out that the "IMPORTANT" read might still be important to someone. -- Bob Copeland %% http://bobcopeland.com/ -- 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
