[PATCH] ath10k: Replace ioread with wmb for data sync
Peter Oh
poh at qca.qualcomm.com
Tue Jan 27 21:39:09 PST 2015
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
More information about the ath10k
mailing list