On Friday, December 11, 2015 2:49 PM, Jisheng Zhang wrote: > > On Fri, 11 Dec 2015 09:35:10 +0530 Pratyush Anand wrote: > > > On Wed, Dec 9, 2015 at 3:53 PM, Russell King - ARM Linux 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. > > > > > > > > I'm just looking at this driver, trying to decipher what it's doing. It > > > _looks_ to me like it's reprogramming one of the outbound windows (IO?) > > > so that configuration space can be accessed. Doesn't this have the > > > effect of disabling access to the IO segment of the PCI bus from the > > > host CPU? > > > > > > What protections are there against other CPUs in the system issuing a > > > PCI I/O read/write while this outbound window is programmed as > > > configuration space? > > > > > > Yes, that is an issue with this driver. Most of the host controller > > has 4 or more viewpoints, and it is very easy to handle for them. But > > there are few which has only two viewpoints. Do not know how to solve > > it, so that it works for all. > > > > The default outbound iATU number is two, this may be the reason why the driver > is written in current style. And two outbound iATUs may be common for pcie dw > users because ASIC people just follow the default configuration ;). > > In our case, Marvell Berlin SoCs have two outbound iATUs. Hmm, we need to add new DT property to handle the number of outbound iATUs. Then, 'pcie-designware.c' should configure registers according to the number. Anyway, we should add this agenda to ToDo list. Best regards, Jingoo Han > > Thanks, > Jisheng -- 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
