On Mon, Jun 10, 2024 at 04:50:48PM -0700, Shyam Saini wrote:
> 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.
>
> 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@xxxxxxxxxxxxxxxxxxx>
> ---
> .../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));
Sorry, but the change doesn't look correct.
First of all, you suggest to split up _all_ the PCIe ranges greater
than 4GB. As I already mentioned several times the DW PCIe controller
of v4.60a and younger may have the maximum limit address greater than
4GB. So the change is wrong for the modern IP-core which have the
CX_ATU_MAX_REGION_SIZE synthesis parameter set with a number greater
than 4GB.
Secondly the dw_pcie_iatu_setup() method is looping around all the
PCIe memory ranges and allocates the iATU regions one after another.
So should the dw_pcie_prog_outbound_atu_multi() allocated more than
one region, the second, third, etc iATU regions setup will be
overwritten with the subsequent PCIe memory ranges data.
Thirdly your dw_pcie_prog_outbound_atu_multi() initializes the iATU
regions starting from zero each time it's called. So each next call
will override the initialization performed in the framework of the
previous one.
AFAICS it's not that easy as it seemed at the first glance. One of the
possible solution is to fix the __dw_pcie_prog_outbound_atu() method
in a way so instead of failing on the
((limit_addr & ~pci->region_limit) != (pci_addr & ~pci->region_limit))
conditional statement the method would initialize the requested region from
cpu_addr
up to
limit_addr = clamp(limit_addr, cpu_addr, cpu_addr | pci->region_limit)
and returned (limit_addr - cpu_addr + 1) from __dw_pcie_prog_outbound_atu().
The dw_pcie_prog_outbound_atu() caller shall check the returned value.
If it's negative then error happened. If the size is the same as the
requested one, then the memory window region was successfully
initialized. If the returned value is smaller than the requested one,
then the memory window only partly covers the requested region and the
next free outbound iATU region needs to be allocated and initialized.
Note 1. The suggested semantics need to be propagated to all the
dw_pcie_prog_outbound_atu() and dw_pcie_prog_ep_outbound_atu()
callers.
Note 2. The same procedure can be implemented for the inbound iATU
regions initialization procedure in dw_pcie_prog_inbound_atu(). The
difference is only in the untranslated address argument. In
dw_pcie_prog_inbound_atu() it's pci_addr. So the initialization would
be based on the PCI-address space from
pci_addr
up to
limit_addr = clamp(limit_addr, pci_addr, pci_addr | pci->region_limit)
with returning (limit_addr - pci_addr + 1).
-Serge(y)
> if (ret) {
> dev_err(pci->dev, "Failed to set MEM range %pr\n",
> entry->res);
> --
> 2.34.1
>
