Re: [PATCH v24 08/16] PCI: dwc: Disable two BARs to avoid unnecessary memory assignment

["Krzysztof Wilczyński" <kw@linux.com>]

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?

> * 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