| Message ID | 20231011071423.249458-9-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | PCI: dwc: rcar-gen4: Add R-Car Gen4 PCIe support | expand |
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@renesas.com> > --- > 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 >
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
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
Hello Krzysztof, Mani On Wed, Oct 11, 2023 at 06:48:40PM +0530, Manivannan Sadhasivam wrote: > 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? > > I can't confirm for sure that the BARs manipulations in this patch will work on the older IP-cores (prior 5.10a) or will be required for all new controllers (5.10a and newer). Based on the BARs description posted in the IP-core HW manuals, the CSRs semantic has changed between the major releases. Old DW PCIe RC IP-core HW-manuals explicitly state that the BARs are unavailable: "The Synopsys core does not implement the optional BARs for the RC product" New DW PCIe RC IP-cores manual say that the BARs exist, but are normally unused: "Two BARs are present but are not expected to be used. You should disable them to avoid unnecessary memory assignment during device enumeration. If you do use a BAR, then you should program it to capture TLPs that are targeted to your local non-application memory space.... The BAR range must be outside of the three Base/Limit regions..." So in theory it's possible to have platforms with the BARs somehow utilized even in the Root Ports. Though currently AFAICS we don't have such devices supported in kernel. > > If Yoshihiro can confirm that his controller can work without this patch, then > I'd vote for dropping this patch and applying the rest. AFAIR Yoshihiro insisted to have the BARs reset because without it something didn't work, so he added some comment to justify it: https://lore.kernel.org/linux-pci/TYBPR01MB534104389952D87385E8745ED8879@TYBPR01MB5341.jpnprd01.prod.outlook.com/ Though based on the comment the BARs reset still seems optional. One more low-level driver which already does what is implemented in this patch is the Keystone PCI host-controller driver (see, pci-keystone.c also activates dbi_cs2 and zeros out the PCI_TYPE0_BAR0_ENABLED flag). Moreover something similar is done in the generic DW PCIe EP driver in the framework of the __dw_pcie_ep_reset_bar() method including the direct BARs zeroing out (which I questioned in my initial message in this thread). So seeing this patch would re-do what is already done for the Keystone device and would add a partly duplicated code it would be reasonable to drop the patch for now and get the BARs reset back to the Rcar host low-level driver as it was in v23. We can get back to the topic afterward and see whether the BARs reset could be done generically for the RPs. If we figure out that it's required at least for the new controllers then we'll be able to implement a generic RP/EP BARs reset method, have it utilized in both DW PCIe core drivers and drop the respective code from both Rcar and Keystone LLDDs. -Serge(y) > > 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 > > -- > மணிவண்ணன் சதாசிவம்
Hello Serge, Krzysztof, Mani, > From: Serge Semin, Sent: Wednesday, October 11, 2023 11:50 PM > > Hello Krzysztof, Mani > > On Wed, Oct 11, 2023 at 06:48:40PM +0530, Manivannan Sadhasivam wrote: > > 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? I tried to remove these setting on my environment, and then it also worked. So, as you mentioned, there is no need, I think. > > > > 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? > > > > > I can't confirm for sure that the BARs manipulations in this patch > will work on the older IP-cores (prior 5.10a) or will be required for > all new controllers (5.10a and newer). Based on the BARs description > posted in the IP-core HW manuals, the CSRs semantic has changed > between the major releases. Old DW PCIe RC IP-core HW-manuals > explicitly state that the BARs are unavailable: > > "The Synopsys core does not implement the optional BARs for the RC > product" > > New DW PCIe RC IP-cores manual say that the BARs exist, but are > normally unused: > > "Two BARs are present but are not expected to be used. You should > disable them to avoid unnecessary memory assignment during device > enumeration. If you do use a BAR, then you should program it to > capture TLPs that are targeted to your local non-application memory > space.... The BAR range must be outside of the three Base/Limit > regions..." > > So in theory it's possible to have platforms with the BARs somehow > utilized even in the Root Ports. Though currently AFAICS we don't > have such devices supported in kernel. > > > > > If Yoshihiro can confirm that his controller can work without this patch, then > > I'd vote for dropping this patch and applying the rest. > > AFAIR Yoshihiro insisted to have the BARs reset because without > it something didn't work, so he added some comment to justify it: > https://lore.kernel.org/linux-pci/TYBPR01MB534104389952D87385E8745ED8879@TYBPR01MB5341.jpnprd01.prod.outlook.com/ Yes, the settings are really needed on my environment. # I checked this again today. > Though based on the comment the BARs reset still seems optional. > > One more low-level driver which already does what is implemented in > this patch is the Keystone PCI host-controller driver (see, > pci-keystone.c also activates dbi_cs2 and zeros out the > PCI_TYPE0_BAR0_ENABLED flag). Thank you for the information. I could not find the settings... ----- static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie) { ... /* Disable BARs for inbound access */ ks_pcie_set_dbi_mode(ks_pcie); dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, 0); dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_1, 0); ks_pcie_clear_dbi_mode(ks_pcie); ----- > Moreover something similar is done in > the generic DW PCIe EP driver in the framework of the > __dw_pcie_ep_reset_bar() method including the direct BARs zeroing out > (which I questioned in my initial message in this thread). So seeing > this patch would re-do what is already done for the Keystone device > and would add a partly duplicated code it would be reasonable to drop > the patch for now and get the BARs reset back to the Rcar host > low-level driver as it was in v23. We can get back to the topic > afterward and see whether the BARs reset could be done generically for > the RPs. If we figure out that it's required at least for the new > controllers then we'll be able to implement a generic RP/EP BARs reset > method, have it utilized in both DW PCIe core drivers and drop the > respective code from both Rcar and Keystone LLDDs. If possible, I would like to have the setting on pcie-rcar-gen4.c only, for avoiding any trouble, especially on other DWC drivers. > -Serge(y) > > > > > This can be submitted later if required. > > > > - Mani > > > > > > * BTW Yoshihiro, I am sorry to see your patchset is still under review...( Serge, no worries! Thank you very much for your support again and again! > > > 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, thank you very much for your support! I hope that this patchset can be merged into v6.7-rc1... Best regards, Yoshihiro Shimoda > > > Krzysztof > > > > -- > > மணிவண்ணன் சதாசிவம்
[+cc Siddharth, Ravi, Sriramakrishnan] 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@renesas.com> > --- > 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, 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); I cc'd Siddharth and others because they are working on a Keystone issue with MSI-X that requires BAR0; see https://lore.kernel.org/r/20231011123451.34827-1-s-vadapalli@ti.com I assume any DWC controller that uses MSI-X would require BAR0 or BAR1 for the MSI-X Table. I don't have any of the DWC specs and don't know whether any controllers use MSI-X, so just heads up in case they do. This patch was recently merged and will appear in v6.7. Bjorn
Hello Bjorn, On 17/10/23 03:18, Bjorn Helgaas wrote: > [+cc Siddharth, Ravi, Sriramakrishnan] > > 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@renesas.com> >> --- >> 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, 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); > > I cc'd Siddharth and others because they are working on a Keystone > issue with MSI-X that requires BAR0; see > https://lore.kernel.org/r/20231011123451.34827-1-s-vadapalli@ti.com > > I assume any DWC controller that uses MSI-X would require BAR0 or BAR1 > for the MSI-X Table. Disabling BAR0 in this section will not affect the pci-keystone.c driver. The MSI-X setup in the pci-keystone.c driver is done via the ks_pcie_v3_65_add_bus() function which is invoked in the pci_host_probe(bridge) function call within dw_pcie_host_init(). Since pci_host_probe(bridge) is invoked *after* dw_pcie_setup_rc(), the MSI-X setup which involves enabling BAR0 will be performed after BAR0 is disabled in this patch, due to which BAR0 will remain enabled as far as MSI-X setup is concerned. Also, the DW PCIe IP version corresponding to the Keystone PCIe host controller is 4.90a. Even in the 4.90a Databook, in section 3.5.7.2 RC Mode, it is suggested that the BARs should be disabled. > > I don't have any of the DWC specs and don't know whether any > controllers use MSI-X, so just heads up in case they do. This patch > was recently merged and will appear in v6.7. > > Bjorn
Dear All, On 11.10.2023 09:14, 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@renesas.com> This patch landed in today's linux-next 20231017 as commit e308528cac3e ("PCI: dwc: Disable two BARs to avoid unnecessary memory assignment"). Unfortunately it causes the following kernel panic on Samsung Exynos5433-based TM2e board: exynos-pcie 15700000.pcie: host bridge /soc@0/pcie@15700000 ranges: exynos-pcie 15700000.pcie: IO 0x000c001000..0x000c010fff -> 0x0000000000 exynos-pcie 15700000.pcie: MEM 0x000c011000..0x000ffffffe -> 0x000c011000 exynos-pcie 15700000.pcie: iATU: unroll F, 3 ob, 5 ib, align 4K, limit 4G Unable to handle kernel paging request at virtual address ffff800084196010 Mem abort info: ... Data abort info: ... swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000022047000 [ffff800084196010] pgd=10000000df6ff003, p4d=10000000df6ff003, pud=10000000df6fe003, pmd=1000000024ad9003, pte=0000000000000000 Internal error: Oops: 0000000096000047 [#1] PREEMPT SMP Modules linked in: CPU: 4 PID: 55 Comm: kworker/u18:0 Not tainted 6.6.0-rc1+ #14129 Hardware name: Samsung TM2E board (DT) Workqueue: events_unbound deferred_probe_work_func pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : dw_pcie_write_dbi2+0xb8/0xc8 lr : dw_pcie_setup_rc+0x30/0x4e4 ... Call trace: dw_pcie_write_dbi2+0xb8/0xc8 dw_pcie_setup_rc+0x30/0x4e4 dw_pcie_host_init+0x238/0x608 exynos_pcie_probe+0x23c/0x340 platform_probe+0x68/0xd8 really_probe+0x148/0x2b4 __driver_probe_device+0x78/0x12c driver_probe_device+0xd8/0x160 __device_attach_driver+0xb8/0x138 bus_for_each_drv+0x84/0xe0 __device_attach+0xa8/0x1b0 device_initial_probe+0x14/0x20 bus_probe_device+0xb0/0xb4 deferred_probe_work_func+0x8c/0xc8 process_one_work+0x1ec/0x53c worker_thread+0x298/0x408 kthread+0x124/0x128 ret_from_fork+0x10/0x20 Code: d50332bf 79000023 17ffffe2 d50332bf (b9000023) ---[ end trace 0000000000000000 ]--- Kernel panic - not syncing: Oops: Fatal exception SMP: stopping secondary CPUs Kernel Offset: disabled CPU features: 0x8c00020e,3c020000,0000421b Memory Limit: none ---[ end Kernel panic - not syncing: Oops: Fatal exception ]--- I've observed similar issue on Qualcomm's RB5 platform with some additional not-yet merged patches enabling PCIe support. Reverting $subject on top of linux-next fixes this issue. Let me know if you need more information. > --- > 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, 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); > + > /* > * Enable DBI read-only registers for writing/updating configuration. > * Write permission gets disabled towards the end of this function. Best regards
Dear Marek, > From: Marek Szyprowski, Sent: Tuesday, October 17, 2023 6:19 PM > > Dear All, > > On 11.10.2023 09:14, 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@renesas.com> > > This patch landed in today's linux-next 20231017 as commit e308528cac3e > ("PCI: dwc: Disable two BARs to avoid unnecessary memory assignment"). > Unfortunately it causes the following kernel panic on Samsung > Exynos5433-based TM2e board: > > exynos-pcie 15700000.pcie: host bridge /soc@0/pcie@15700000 ranges: > exynos-pcie 15700000.pcie: IO 0x000c001000..0x000c010fff -> > 0x0000000000 > exynos-pcie 15700000.pcie: MEM 0x000c011000..0x000ffffffe -> > 0x000c011000 > exynos-pcie 15700000.pcie: iATU: unroll F, 3 ob, 5 ib, align 4K, limit 4G > Unable to handle kernel paging request at virtual address ffff800084196010 > Mem abort info: > ... > Data abort info: > ... > swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000022047000 > [ffff800084196010] pgd=10000000df6ff003, p4d=10000000df6ff003, > pud=10000000df6fe003, pmd=1000000024ad9003, pte=0000000000000000 > Internal error: Oops: 0000000096000047 [#1] PREEMPT SMP > Modules linked in: > CPU: 4 PID: 55 Comm: kworker/u18:0 Not tainted 6.6.0-rc1+ #14129 > Hardware name: Samsung TM2E board (DT) > Workqueue: events_unbound deferred_probe_work_func > pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > pc : dw_pcie_write_dbi2+0xb8/0xc8 > lr : dw_pcie_setup_rc+0x30/0x4e4 > ... > Call trace: > dw_pcie_write_dbi2+0xb8/0xc8 > dw_pcie_setup_rc+0x30/0x4e4 > dw_pcie_host_init+0x238/0x608 > exynos_pcie_probe+0x23c/0x340 > platform_probe+0x68/0xd8 > really_probe+0x148/0x2b4 > __driver_probe_device+0x78/0x12c > driver_probe_device+0xd8/0x160 > __device_attach_driver+0xb8/0x138 > bus_for_each_drv+0x84/0xe0 > __device_attach+0xa8/0x1b0 > device_initial_probe+0x14/0x20 > bus_probe_device+0xb0/0xb4 > deferred_probe_work_func+0x8c/0xc8 > process_one_work+0x1ec/0x53c > worker_thread+0x298/0x408 > kthread+0x124/0x128 > ret_from_fork+0x10/0x20 > Code: d50332bf 79000023 17ffffe2 d50332bf (b9000023) > ---[ end trace 0000000000000000 ]--- > Kernel panic - not syncing: Oops: Fatal exception > SMP: stopping secondary CPUs > Kernel Offset: disabled > CPU features: 0x8c00020e,3c020000,0000421b > Memory Limit: none > ---[ end Kernel panic - not syncing: Oops: Fatal exception ]--- > > I've observed similar issue on Qualcomm's RB5 platform with some > additional not-yet merged patches enabling PCIe support. Reverting > $subject on top of linux-next fixes this issue. > > Let me know if you need more information. Thank you for the report. I guess that the issue is related to out-of-range access of dbi2: - In arch/arm64/boot/dts/exynos/exynos5433.dtsi, the dbi reg size is 0x1000 like below: ----- pcie: pcie@15700000 { compatible = "samsung,exynos5433-pcie"; reg = <0x15700000 0x1000>, <0x156b0000 0x1000>, <0x0c000000 0x1000>; reg-names = "dbi", "elbi", "config"; ... ----- - In drivers/pci/controller/dwc/pcie-designware.c, "dbi2" area is calculated by the following if reg-names "dbi2" didn't exist: ----- pci->dbi_base2 = pci->dbi_base + SZ_4K; ----- - However, this is out-of-memory on exynos5433.dtsi because the "dbi" size is 0x1000 only. - And then, this patch always writes PCI_BASE_ADDRESS_[01] to dbi2 area. So, since this is out-of-range, the kernel panic happens. Perhaps, we should revert this patch at first. And, add the settings into my environment (pcie-rcar-gen4.c) only. I also have alternative solution which modifies the "dbi2" area calculation and avoid out-of-range access somehow. But, it may complicate source code... Best regards, Yoshihiro Shimoda > > --- > > 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, 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); > > + > > /* > > * Enable DBI read-only registers for writing/updating configuration. > > * Write permission gets disabled towards the end of this function. > > Best regards > -- > Marek Szyprowski, PhD > Samsung R&D Institute Poland
On Tue, Oct 17, 2023 at 12:05:12PM +0000, Yoshihiro Shimoda wrote: > Dear Marek, > > > From: Marek Szyprowski, Sent: Tuesday, October 17, 2023 6:19 PM > > > > Dear All, > > > > On 11.10.2023 09:14, 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@renesas.com> > > > > This patch landed in today's linux-next 20231017 as commit e308528cac3e > > ("PCI: dwc: Disable two BARs to avoid unnecessary memory assignment"). > > Unfortunately it causes the following kernel panic on Samsung > > Exynos5433-based TM2e board: > > > > exynos-pcie 15700000.pcie: host bridge /soc@0/pcie@15700000 ranges: > > exynos-pcie 15700000.pcie: IO 0x000c001000..0x000c010fff -> > > 0x0000000000 > > exynos-pcie 15700000.pcie: MEM 0x000c011000..0x000ffffffe -> > > 0x000c011000 > > exynos-pcie 15700000.pcie: iATU: unroll F, 3 ob, 5 ib, align 4K, limit 4G > > Unable to handle kernel paging request at virtual address ffff800084196010 > > Mem abort info: > > ... > > Data abort info: > > ... > > swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000022047000 > > [ffff800084196010] pgd=10000000df6ff003, p4d=10000000df6ff003, > > pud=10000000df6fe003, pmd=1000000024ad9003, pte=0000000000000000 > > Internal error: Oops: 0000000096000047 [#1] PREEMPT SMP > > Modules linked in: > > CPU: 4 PID: 55 Comm: kworker/u18:0 Not tainted 6.6.0-rc1+ #14129 > > Hardware name: Samsung TM2E board (DT) > > Workqueue: events_unbound deferred_probe_work_func > > pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > > pc : dw_pcie_write_dbi2+0xb8/0xc8 > > lr : dw_pcie_setup_rc+0x30/0x4e4 > > ... > > Call trace: > > dw_pcie_write_dbi2+0xb8/0xc8 > > dw_pcie_setup_rc+0x30/0x4e4 > > dw_pcie_host_init+0x238/0x608 > > exynos_pcie_probe+0x23c/0x340 > > platform_probe+0x68/0xd8 > > really_probe+0x148/0x2b4 > > __driver_probe_device+0x78/0x12c > > driver_probe_device+0xd8/0x160 > > __device_attach_driver+0xb8/0x138 > > bus_for_each_drv+0x84/0xe0 > > __device_attach+0xa8/0x1b0 > > device_initial_probe+0x14/0x20 > > bus_probe_device+0xb0/0xb4 > > deferred_probe_work_func+0x8c/0xc8 > > process_one_work+0x1ec/0x53c > > worker_thread+0x298/0x408 > > kthread+0x124/0x128 > > ret_from_fork+0x10/0x20 > > Code: d50332bf 79000023 17ffffe2 d50332bf (b9000023) > > ---[ end trace 0000000000000000 ]--- > > Kernel panic - not syncing: Oops: Fatal exception > > SMP: stopping secondary CPUs > > Kernel Offset: disabled > > CPU features: 0x8c00020e,3c020000,0000421b > > Memory Limit: none > > ---[ end Kernel panic - not syncing: Oops: Fatal exception ]--- > > > > I've observed similar issue on Qualcomm's RB5 platform with some > > additional not-yet merged patches enabling PCIe support. Reverting > > $subject on top of linux-next fixes this issue. > > > > Let me know if you need more information. > > Thank you for the report. I guess that the issue is related to > out-of-range access of dbi2: > - In arch/arm64/boot/dts/exynos/exynos5433.dtsi, the dbi reg size is 0x1000 > like below: > ----- > pcie: pcie@15700000 { > compatible = "samsung,exynos5433-pcie"; > reg = <0x15700000 0x1000>, <0x156b0000 0x1000>, > <0x0c000000 0x1000>; > reg-names = "dbi", "elbi", "config"; > ... > ----- > > - In drivers/pci/controller/dwc/pcie-designware.c, "dbi2" area is calculated > by the following if reg-names "dbi2" didn't exist: > ----- > pci->dbi_base2 = pci->dbi_base + SZ_4K; > ----- > > - However, this is out-of-memory on exynos5433.dtsi because the "dbi" size is > 0x1000 only. > - And then, this patch always writes PCI_BASE_ADDRESS_[01] to dbi2 area. > So, since this is out-of-range, the kernel panic happens. > I could reproduce the issue Marek reported on RB5. As you pointed out, it is due to not mapping dbi2 explicitly. But we were not using DBI2 on the host earlier and it looks to me that DBI2 may not be implemented on Qcom host platforms. Atleast on EP, I confirmed with Qcom that DBI=DBI2 as represented in the driver, but I couldn't confirm if it is same for host as well. > Perhaps, we should revert this patch at first. And, add the settings into > my environment (pcie-rcar-gen4.c) only. I also have alternative solution which > modifies the "dbi2" area calculation and avoid out-of-range access somehow. > But, it may complicate source code... > Yes, let's revert this patch for now and move it to rcar driver. - Mani > Best regards, > Yoshihiro Shimoda > > > > --- > > > 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, 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); > > > + > > > /* > > > * Enable DBI read-only registers for writing/updating configuration. > > > * Write permission gets disabled towards the end of this function. > > > > Best regards > > -- > > Marek Szyprowski, PhD > > Samsung R&D Institute Poland >
On Tue, Oct 17, 2023 at 11:19:21AM +0200, Marek Szyprowski wrote: > Dear All, > > On 11.10.2023 09:14, 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@renesas.com> > > This patch landed in today's linux-next 20231017 as commit e308528cac3e > ("PCI: dwc: Disable two BARs to avoid unnecessary memory assignment"). > Unfortunately it causes the following kernel panic on Samsung > Exynos5433-based TM2e board: I dropped the pci/controller/rcar branch for today until we resolve this. If you want to reorder the series like this: PCI: Add T_PVPERL macro PCI: dwc: Add dw_pcie_link_set_max_link_width() PCI: dwc: Add missing PCI_EXP_LNKCAP_MLW handling PCI: tegra194: Drop PCI_EXP_LNKSTA_NLW setting PCI: dwc: endpoint: Add multiple PFs support for dbi2 PCI: dwc: Add EDMA_UNROLL capability flag PCI: dwc: Expose dw_pcie_ep_exit() to module PCI: dwc: endpoint: Introduce .pre_init() and .deinit() PCI: dwc: Disable two BARs to avoid unnecessary memory assignment dt-bindings: PCI: dwc: Update maxItems of reg and reg-names dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint PCI: rcar-gen4: Add R-Car Gen4 PCIe controller support for host mode PCI: rcar-gen4: Add endpoint mode support MAINTAINERS: Update PCI DRIVER FOR RENESAS R-CAR for R-Car Gen4 misc: pci_endpoint_test: Add Device ID for R-Car S4-8 PCIe controller I think the first four patches could be applied because they're useful regardless of R-Car support. The rest are only needed for R-Car, and it seems like we should apply them all together as soon as this regression is solved. Bjorn
> From: mani@kernel.org, Sent: Wednesday, October 18, 2023 12:16 AM > > On Tue, Oct 17, 2023 at 12:05:12PM +0000, Yoshihiro Shimoda wrote: > > Dear Marek, > > > > > From: Marek Szyprowski, Sent: Tuesday, October 17, 2023 6:19 PM > > > > > > Dear All, > > > > > > On 11.10.2023 09:14, 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@renesas.com> > > > > > > This patch landed in today's linux-next 20231017 as commit e308528cac3e > > > ("PCI: dwc: Disable two BARs to avoid unnecessary memory assignment"). > > > Unfortunately it causes the following kernel panic on Samsung > > > Exynos5433-based TM2e board: > > > > > > exynos-pcie 15700000.pcie: host bridge /soc@0/pcie@15700000 ranges: > > > exynos-pcie 15700000.pcie: IO 0x000c001000..0x000c010fff -> > > > 0x0000000000 > > > exynos-pcie 15700000.pcie: MEM 0x000c011000..0x000ffffffe -> > > > 0x000c011000 > > > exynos-pcie 15700000.pcie: iATU: unroll F, 3 ob, 5 ib, align 4K, limit 4G > > > Unable to handle kernel paging request at virtual address ffff800084196010 > > > Mem abort info: > > > ... > > > Data abort info: > > > ... > > > swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000022047000 > > > [ffff800084196010] pgd=10000000df6ff003, p4d=10000000df6ff003, > > > pud=10000000df6fe003, pmd=1000000024ad9003, pte=0000000000000000 > > > Internal error: Oops: 0000000096000047 [#1] PREEMPT SMP > > > Modules linked in: > > > CPU: 4 PID: 55 Comm: kworker/u18:0 Not tainted 6.6.0-rc1+ #14129 > > > Hardware name: Samsung TM2E board (DT) > > > Workqueue: events_unbound deferred_probe_work_func > > > pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > > > pc : dw_pcie_write_dbi2+0xb8/0xc8 > > > lr : dw_pcie_setup_rc+0x30/0x4e4 > > > ... > > > Call trace: > > > dw_pcie_write_dbi2+0xb8/0xc8 > > > dw_pcie_setup_rc+0x30/0x4e4 > > > dw_pcie_host_init+0x238/0x608 > > > exynos_pcie_probe+0x23c/0x340 > > > platform_probe+0x68/0xd8 > > > really_probe+0x148/0x2b4 > > > __driver_probe_device+0x78/0x12c > > > driver_probe_device+0xd8/0x160 > > > __device_attach_driver+0xb8/0x138 > > > bus_for_each_drv+0x84/0xe0 > > > __device_attach+0xa8/0x1b0 > > > device_initial_probe+0x14/0x20 > > > bus_probe_device+0xb0/0xb4 > > > deferred_probe_work_func+0x8c/0xc8 > > > process_one_work+0x1ec/0x53c > > > worker_thread+0x298/0x408 > > > kthread+0x124/0x128 > > > ret_from_fork+0x10/0x20 > > > Code: d50332bf 79000023 17ffffe2 d50332bf (b9000023) > > > ---[ end trace 0000000000000000 ]--- > > > Kernel panic - not syncing: Oops: Fatal exception > > > SMP: stopping secondary CPUs > > > Kernel Offset: disabled > > > CPU features: 0x8c00020e,3c020000,0000421b > > > Memory Limit: none > > > ---[ end Kernel panic - not syncing: Oops: Fatal exception ]--- > > > > > > I've observed similar issue on Qualcomm's RB5 platform with some > > > additional not-yet merged patches enabling PCIe support. Reverting > > > $subject on top of linux-next fixes this issue. > > > > > > Let me know if you need more information. > > > > Thank you for the report. I guess that the issue is related to > > out-of-range access of dbi2: > > - In arch/arm64/boot/dts/exynos/exynos5433.dtsi, the dbi reg size is 0x1000 > > like below: > > ----- > > pcie: pcie@15700000 { > > compatible = "samsung,exynos5433-pcie"; > > reg = <0x15700000 0x1000>, <0x156b0000 0x1000>, > > <0x0c000000 0x1000>; > > reg-names = "dbi", "elbi", "config"; > > ... > > ----- > > > > - In drivers/pci/controller/dwc/pcie-designware.c, "dbi2" area is calculated > > by the following if reg-names "dbi2" didn't exist: > > ----- > > pci->dbi_base2 = pci->dbi_base + SZ_4K; > > ----- > > > > - However, this is out-of-memory on exynos5433.dtsi because the "dbi" size is > > 0x1000 only. > > - And then, this patch always writes PCI_BASE_ADDRESS_[01] to dbi2 area. > > So, since this is out-of-range, the kernel panic happens. > > > > I could reproduce the issue Marek reported on RB5. As you pointed out, it is due > to not mapping dbi2 explicitly. But we were not using DBI2 on the host earlier > and it looks to me that DBI2 may not be implemented on Qcom host platforms. > > Atleast on EP, I confirmed with Qcom that DBI=DBI2 as represented in the driver, > but I couldn't confirm if it is same for host as well. Thank you for the confirmation. I understood it. > > Perhaps, we should revert this patch at first. And, add the settings into > > my environment (pcie-rcar-gen4.c) only. I also have alternative solution which > > modifies the "dbi2" area calculation and avoid out-of-range access somehow. > > But, it may complicate source code... > > > > Yes, let's revert this patch for now and move it to rcar driver. I got it. Best regards, Yoshihiro Shimoda > - Mani > > > Best regards, > > Yoshihiro Shimoda > > > > > > --- > > > > 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, 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); > > > > + > > > > /* > > > > * Enable DBI read-only registers for writing/updating configuration. > > > > * Write permission gets disabled towards the end of this function. > > > > > > Best regards > > > -- > > > Marek Szyprowski, PhD > > > Samsung R&D Institute Poland > > > > -- > மணிவண்ணன் சதாசிவம்
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, 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); + /* * Enable DBI read-only registers for writing/updating configuration. * Write permission gets disabled towards the end of this function.
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@renesas.com> --- drivers/pci/controller/dwc/pcie-designware-host.c | 8 ++++++++ 1 file changed, 8 insertions(+)