Re: [PATCH V2] drivers: pci: dwc: configure multiple atu regions

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Mon, Jun 10, 2024 at 04:50:48PM -0700, Shyam Saini wrote:

Subject should be 'PCI: dwc: ...'

> Before this change, the dwc PCIe driver configures only 1 ATU region,
> which is sufficient for the devices with PCIe memory <= 4GB. However,
> the driver probe fails when device uses more than 4GB of pcie memory.
> 

Something is not clear... This commit message implies that the driver used to
work on your hardware (you haven't mentioned which one it is) and broken by the
commit from Sergey.

As per Sergey's commit, we auto detect the dw_pcie::region_limit. If the IP is <
4.60, then the limit is 4G, otherwise depends on CX_ATU_MAX_REGION_SIZE set in
hw.

So if your IP is < 4.60, you cannot map > 4GB of outbound memory anyway. But if
it is > 4.60, you shouldn't see the failure that you reported for > 4G space
(well you can see the failure if the limit is less than the region size). In the
previous thread you mentioned that dw_pcie::region_limit is set to 4G. So how
come your driver was working previously?

PS: When reporting issue like this, please add info about your hardware also
(platform, controller driver used etc...).

- Mani

> Fix this by configuring multiple ATU regions for the devices which
> use more than 4GB of PCIe memory.
> 
> Given each 4GB block of memory requires a new ATU region, the total
> number of ATU regions are calculated using the size of PCIe device
> tree node's MEM64 pref range size.
> 
> Signed-off-by: Shyam Saini <shyamsaini@linux.microsoft.com>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 38 +++++++++++++++++--
>  1 file changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index d15a5c2d5b48..bed0b189b6ad 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -652,6 +652,33 @@ static struct pci_ops dw_pcie_ops = {
>  	.write = pci_generic_config_write,
>  };
>  
> +static int dw_pcie_num_atu_regions(struct resource_entry *entry)
> +{
> +	return DIV_ROUND_UP(resource_size(entry->res), SZ_4G);
> +}
> +
> +static int dw_pcie_prog_outbound_atu_multi(struct dw_pcie *pci, int type,
> +						struct resource_entry *entry)
> +{
> +	int idx, ret, num_regions;
> +
> +	num_regions = dw_pcie_num_atu_regions(entry);
> +
> +	for (idx = 0; idx < num_regions; idx++) {
> +		dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT, idx);
> +		ret = dw_pcie_prog_outbound_atu(pci, idx, PCIE_ATU_TYPE_MEM,
> +						entry->res->start,
> +						entry->res->start - entry->offset,
> +						resource_size(entry->res)/4);
> +
> +		if (ret)
> +			goto err;
> +	}
> +
> +err:
> +	return ret;
> +}
> +
>  static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -682,10 +709,13 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
>  		if (pci->num_ob_windows <= ++i)
>  			break;
>  
> -		ret = dw_pcie_prog_outbound_atu(pci, i, PCIE_ATU_TYPE_MEM,
> -						entry->res->start,
> -						entry->res->start - entry->offset,
> -						resource_size(entry->res));
> +		if (resource_size(entry->res) > SZ_4G)
> +			ret = dw_pcie_prog_outbound_atu_multi(pci, PCIE_ATU_TYPE_MEM, entry);
> +		else
> +			ret = dw_pcie_prog_outbound_atu(pci, i, PCIE_ATU_TYPE_MEM,
> +							entry->res->start,
> +							entry->res->start - entry->offset,
> +							resource_size(entry->res));
>  		if (ret) {
>  			dev_err(pci->dev, "Failed to set MEM range %pr\n",
>  				entry->res);
> -- 
> 2.34.1
> 

-- 
மணிவண்ணன் சதாசிவம்