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

[Bob Copeland <me@bobcopeland.com>]

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.

-- 
Bob Copeland %% http://bobcopeland.com/