Re: [PATCH v2 1/3] PCI: dwc: Fix skipped index 0 in outbound ATU setup

[Niklas Cassel <cassel@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1909 bytes --]

On Thu, Jan 22, 2026 at 06:16:11PM +0000, Maciej W. Rozycki wrote:
> On Thu, 22 Jan 2026, Niklas Cassel wrote:
> 
> > Also see my series here:
> > https://lore.kernel.org/linux-pci/20260122145411.453291-4-cassel@kernel.org/T/
> > 
> > That tries to clean up this mess.
> 
>  Is your patchset referred meant to replace this one or does it apply on 
> top?

This series does no longer apply, as it collides with a commit queued on
controller/dwc branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=controller/dwc&id=e9a5415adb209f86a05e55b850127ada82e070f1

My patches are based on top of the above commit.
(I saw the big mess we currently have with regards to iatu indexing,
so I decided to clean it up, but making things consistent.)


So first of all, this series needs to be rebased.
If you ask me personally, I would prefer if Krishna could rebase on top
of my cleanups.


Patch 1/3 in this series is simply wrong, so it should be dropped.

Patch 2/3 in this series is similar to patch 1/3 in my series, but seems
to also have some extra prints that seem to be unrelated to fixing the issue
at hand. (Fine to add extra prints in some other function, but don't do it in
the fix patch itself that will be backported to stable releases.)

Patch 3/3 is the only patch that needs to be rebased, and seem to be the
patch that solves your issue.


I don't like the way that patch 3/3 is implemented.

ECAM will use two iATUs, one for PCIE_ATU_TYPE_CFG0 one for
PCIE_ATU_TYPE_CFG1. But this patch completely disregards that
this driver already reserves iATU index 0 for PCIE_ATU_TYPE_CFG0,
but for non-ECAM versions.

Now I understand why they have patch 1/3 in this series.

But it is still wrong. It has to change the indexing based on ECAM
is used or not.

Please try the attached patch on top of my series.

It avoids the need to introduce a new struct member.


Kind regards,
Niklas

[-- Attachment #2: 0001-PCI-dwc-Fix-ECAM.patch --]
[-- Type: text/plain, Size: 4398 bytes --]

From b3d345d7075a4757c10b8fbf85154da66bccfebf Mon Sep 17 00:00:00 2001
From: Niklas Cassel <cassel@kernel.org>
Date: Thu, 22 Jan 2026 22:05:39 +0100
Subject: [PATCH] PCI: dwc: Fix ECAM ...

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 .../pci/controller/dwc/pcie-designware-host.c | 34 +++++++++++--------
 drivers/pci/controller/dwc/pcie-designware.c  |  6 ++++
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index eda94db04b63..ef66a031f0bb 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -441,7 +441,7 @@ static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
 	/*
 	 * Root bus under the host bridge doesn't require any iATU configuration
 	 * as DBI region will be used to access root bus config space.
-	 * Immediate bus under Root Bus, needs type 0 iATU configuration and
+	 * Immediate bus under Root Bus needs type 0 iATU configuration and
 	 * remaining buses need type 1 iATU configuration.
 	 */
 	atu.index = 0;
@@ -641,14 +641,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
 	if (ret)
 		goto err_free_msi;
 
-	if (pp->ecam_enabled) {
-		ret = dw_pcie_config_ecam_iatu(pp);
-		if (ret) {
-			dev_err(dev, "Failed to configure iATU in ECAM mode\n");
-			goto err_free_msi;
-		}
-	}
-
 	/*
 	 * Allocate the resource for MSG TLP before programming the iATU
 	 * outbound window in dw_pcie_setup_rc(). Since the allocation depends
@@ -892,8 +884,8 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct dw_pcie_ob_atu_cfg atu = { 0 };
 	struct resource_entry *entry;
-	int ob_iatu_index_to_use = 0;
-	int ib_iatu_index_to_use = 0;
+	int ob_iatu_index_to_use;
+	int ib_iatu_index_to_use;
 	int i, ret;
 
 	if (!pci->num_ob_windows) {
@@ -915,8 +907,20 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
 	 * NOTE: For outbound address translation, outbound iATU at index 0 is
 	 * reserved for CFG IOs (dw_pcie_other_conf_map_bus()), thus start at
 	 * index 1.
+	 *
+	 * If using ECAM, outbound iATU at index 0 and index 1 is reserved for
+	 * CFG IOs.
 	 */
-	ob_iatu_index_to_use++;
+	if (pp->ecam_enabled) {
+		ob_iatu_index_to_use = 2;
+		ret = dw_pcie_config_ecam_iatu(pp);
+		if (ret) {
+			dev_err(pci->dev, "Failed to configure iATU in ECAM mode\n");
+			return ret;
+		}
+	} else {
+		ob_iatu_index_to_use = 1;
+	}
 
 	resource_list_for_each_entry(entry, &pp->bridge->windows) {
 		resource_size_t res_size;
@@ -1002,6 +1006,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
 		}
 	}
 
+	ib_iatu_index_to_use = 0;
 	resource_list_for_each_entry(entry, &pp->bridge->dma_ranges) {
 		resource_size_t res_start, res_size, window_size;
 
@@ -1157,9 +1162,10 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
 	/*
 	 * If the platform provides its own child bus config accesses, it means
 	 * the platform uses its own address translation component rather than
-	 * ATU, so we should not program the ATU here.
+	 * ATU, so we should not program the ATU here. If ECAM is enabled,
+	 * config space access goes through ATU, so set up ATU here.
 	 */
-	if (pp->bridge->child_ops == &dw_child_pcie_ops) {
+	if (pp->bridge->child_ops == &dw_child_pcie_ops || pp->ecam_enabled) {
 		ret = dw_pcie_iatu_setup(pp);
 		if (ret)
 			return ret;
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 2fa9f6ee149e..766df22fe46e 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -531,6 +531,9 @@ int dw_pcie_prog_outbound_atu(struct dw_pcie *pci,
 	u32 retries, val;
 	u64 limit_addr;
 
+	if (atu->index > pci->num_ob_windows)
+		return -ENOSPC;
+
 	limit_addr = parent_bus_addr + atu->size - 1;
 
 	if ((limit_addr & ~pci->region_limit) != (parent_bus_addr & ~pci->region_limit) ||
@@ -604,6 +607,9 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int type,
 	u64 limit_addr = pci_addr + size - 1;
 	u32 retries, val;
 
+	if (index > pci->num_ib_windows)
+		return -ENOSPC;
+
 	if ((limit_addr & ~pci->region_limit) != (pci_addr & ~pci->region_limit) ||
 	    !IS_ALIGNED(parent_bus_addr, pci->region_align) ||
 	    !IS_ALIGNED(pci_addr, pci->region_align) || !size) {
-- 
2.52.0