On 12/11/2015 06:05 AM, Pratyush Anand wrote: > On Wed, Dec 9, 2015 at 3:53 PM, Russell King - ARM Linux > <linux@xxxxxxxxxxxxxxxx> wrote: > > [...] > >>>>> 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 :( > >> >> 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
