On Wed, Oct 11, 2023 at 10:07:27PM +0900, Krzysztof Wilczyński wrote: > Hello, > > [...] > > > + /* > > > + * According to the section 3.5.7.2 "RC Mode" in DWC PCIe Dual Mode > > > + * Rev.5.20a, > > > > and 3.5.6.1 "RC mode" in DWC PCIe RC databook v5.20a. > > OK. I can fix this citation later. > > > > + ... we should disable two BARs to avoid unnecessary memory > > > + * assignment during device enumeration. > > > + */ > > > + dw_pcie_writel_dbi2(pci, PCI_BASE_ADDRESS_0, 0x0); > > > + dw_pcie_writel_dbi2(pci, PCI_BASE_ADDRESS_1, 0x0); > > > + > > > > What's the point in doing this > > dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, 0x00000004); > > dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_1, 0x00000000); > > ... > > dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, 0); > > afterward? > > > > I guess if the BARs are disabled there is no need in having them > > touched. Am I wrong? > > > > BTW I failed to understand why the BARs inits was originally needed: > > first merging the BAR0 and BAR1 into a single 64-bit BAR, then > > switching it back to two 32-bit BARs. Moreover here is what prior DW > > PCIe RC v5.x databooks say about the BARs: > > > > "3.5.6 BAR Details > > Base Address Registers (Offset: 0x10-x14) > > The Synopsys core does not implement the optional BARs for the RC > > product. This is based on the assumption that the RC host probably has > > registers on some other internal bus and has knowledge and setup > > access to these registers already." > > > > I am not sure I fully understand what it means, but it seems as DW > > PCIe cores didn't have anything behind the RC BARs even back then. So > > it seems to me that the BARs manipulation was the Exinos PCIe host > > specific, from which driver they are originating - commit 340cba6092c2 > > ("pci: Add PCIe driver for Samsung Exynos"). > > Would any of the above be something we need to address before this series > can be successfully merged? I am asking if this is a show stopper, > something we can fix later, or even something I could address once I take > this series again. > > Thoughts? > If Yoshihiro can confirm that his controller can work without this patch, then I'd vote for dropping this patch and applying the rest. This can be submitted later if required. - Mani > > * BTW Yoshihiro, I am sorry to see your patchset is still under review...( > > Yes, we need to draw a line somewhere. :) I am happy to take this series > so we don't miss another merge window. We can always fix other bits and > pieces later and iron out any kinks that might have fallen through the > cracks, so to speak. > > Krzysztof -- மணிவண்ணன் சதாசிவம்