On Wed, Oct 11, 2023 at 04:14:15PM +0900, Yoshihiro Shimoda wrote:
> According to the section 3.5.7.2 "RC Mode" in DWC PCIe Dual Mode
> Rev.5.20a, we should disable two BARs to avoid unnecessary memory
> assignment during device enumeration. Otherwise, Renesas R-Car Gen4
> PCIe controllers cannot work correctly in host mode.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> ---
> drivers/pci/controller/dwc/pcie-designware-host.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index a7170fd0e847..56cc7ff6d508 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -737,6 +737,14 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
> u32 val, ctrl, num_ctrls;
> int ret;
>
> + /*
> + * 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.
> + ... 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").
* BTW Yoshihiro, I am sorry to see your patchset is still under review...(
-Serge(y)
> /*
> * Enable DBI read-only registers for writing/updating configuration.
> * Write permission gets disabled towards the end of this function.
> --
> 2.25.1
>
