On 12/11/2015 06:05 AM, Pratyush Anand wrote:

On Wed, Dec 9, 2015 at 3:53 PM, Russell King - ARM Linux
[off-list ref] wrote:

[...]

quoted

quoted

quoted

quoted

      dw_pcie_writel_rc(pp, PCIE_ATU_ENABLE, PCIE_ATU_CR2);
+     /*
+      * ensure that the ATU enable has been happaned before accessing
+      * pci configuration/io spaces through dw_pcie_cfg_[read|write].
+      */
+     wmb();
 }

My understnading is that since writel() of dw_pcie_writel_rc() in
above code and readl(), writel() of dw_pcie_cfg_[read|write]() (which
will follow) goes through same device (ie PCIe host here). So, it is
guaranteed that 1st writel() will be executed before later
readl()/writel(). If that is true then we do not need any explicit
barrier here.

Arnd, Russel: whats your opinion here.

              ^l

Sorry :(

quoted

writel() has a barrier _before_ the access but not after.

The fact is that there's nothing which guarantees that the write will hit
the hardware in a timely manner (forget any rules about PCI config space,
the PCI ordering rules apply to the PCI bus, not to the ARM buses.)

If you need this write to have hit the hardware before continuing, you
need to read back from the same register.

OK, so better to replace wmb() with read back of control register.

Would the patch be acceptable if I replace wmb with read?

-- 
regards,
Stan