Re: [PATCH] ath10k: Replace ioread with wmb for data sync

[Peter Oh <poh@qca.qualcomm.com>]

On 01/27/2015 01:33 PM, Bob Copeland wrote:
> On Mon, Jan 26, 2015 at 02:25:18PM -0800, Peter Oh wrote:
>> Using ioread() to perform data sync is excessive.
>> Use compact API, wmb(), that intended to be used for the case.
>> It reduces total 14 CPU clocks per interrupt.
> Hi,
>
>>   	ath10k_pci_write32(ar, SOC_CORE_BASE_ADDRESS + PCIE_INTR_CLR_ADDRESS,
>>   			   PCIE_INTR_FIRMWARE_MASK | PCIE_INTR_CE_MASK_ALL);
>>   
>> -	/* 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.
when wmb is used, it guarantees all memory accesses complete before wmb 
command completes.
for instance dsb is the corresponding command for wmb in arm and arm 
instruction guide says
"The DSB instruction completes when all explicit memory accesses before 
it complete."
Which means DSB instruction will hold the bus (usually AXI bus) and 
never returns until any memory or memory mapped I/O access complete
(dsb is for ARM and other platforms have the equivalent instruction such 
as sfence, sync, mf, and dcs).
Thanks,
Peter