On Thu, Dec 17, 2015 at 9:15 PM, Stanimir Varbanov <stanimir.varbanov@xxxxxxxxxx> wrote: > > 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? For me it would be fine. ~Pratyush -- 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
