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

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

[Bjorn Helgaas]

[+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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Siddharth Vadapalli]

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

-- 
Regards,
Siddharth.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel