[PATCH v2 0/4] PCI: dwc: Clean up iatu index mess

[PATCH v2 0/4] PCI: dwc: Clean up iatu index mess

[Niklas Cassel]

This series tries to clean up the iatu index mess, by treating
iatu indexing for outbound and inbound address translation in
a consistent way (increment the index after assignment, and only
if the function call succeeded).


Changes since v1:
-Added patch 4/4 from Krishna which fixes iatu indexing when using ECAM.
 I've adapted the patch to my clean up series and fixed some bugs.


Link to v1:
https://lore.kernel.org/linux-pci/20260122145411.453291-4-cassel@kernel.org/


Krishna Chaitanya Chundru (1):
  PCI: dwc: Fix missing iATU setup when ECAM is enabled

Niklas Cassel (3):
  PCI: dwc: Fix msg_atu_index assignment
  PCI: dwc: Improve msg_atu_index error handling
  PCI: dwc: Clean up iatu index usage in dw_pcie_iatu_setup()

 .../pci/controller/dwc/pcie-designware-host.c | 92 +++++++++++--------
 drivers/pci/controller/dwc/pcie-designware.c  |  6 ++
 2 files changed, 61 insertions(+), 37 deletions(-)


base-commit: e9a5415adb209f86a05e55b850127ada82e070f1
-- 
2.52.0

[PATCH v2 1/4] PCI: dwc: Fix msg_atu_index assignment

[Niklas Cassel]

When dw_pcie_iatu_setup() configures outbound address translation
for both type PCIE_ATU_TYPE_MEM and PCIE_ATU_TYPE_IO, the iatu index
to use is incremented before calling dw_pcie_prog_outbound_atu().

However, for msg_atu_index the index is not incremented before use,
causing the iATU index to be the same as the last configured iatu
index, which means that it will incorrectly use the same iatu index
that is already in use, breaking outbound address translation.

Fixes: e1a4ec1a9520 ("PCI: dwc: Add generic MSG TLP support for sending PME_Turn_Off when system suspend")
Cc: stable@vger.kernel.org
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index ab17549af518..cca5fc886409 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -982,7 +982,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
 		dev_warn(pci->dev, "Ranges exceed outbound iATU size (%d)\n",
 			 pci->num_ob_windows);
 
-	pp->msg_atu_index = i;
+	pp->msg_atu_index = ++i;
 
 	i = 0;
 	resource_list_for_each_entry(entry, &pp->bridge->dma_ranges) {
-- 
2.52.0

[PATCH v2 2/4] PCI: dwc: Improve msg_atu_index error handling

[Niklas Cassel]

Only try to dedicate an outbound iATU to MSG TLP if use_atu_msg is set.
If use_atu_msg is not set, it is completely useless to bump the index.

Additionally, if use_atu_msg is set, and there is no available outbound
iatu available, return an error.

Fixes: e1a4ec1a9520 ("PCI: dwc: Add generic MSG TLP support for sending PME_Turn_Off when system suspend")
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index cca5fc886409..991fe5b9a7b3 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -982,7 +982,14 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
 		dev_warn(pci->dev, "Ranges exceed outbound iATU size (%d)\n",
 			 pci->num_ob_windows);
 
-	pp->msg_atu_index = ++i;
+	if (pp->use_atu_msg) {
+		if (pci->num_ob_windows > ++i) {
+			pp->msg_atu_index = i;
+		} else {
+			dev_err(pci->dev, "Cannot add outbound window for MSG TLP\n");
+			return -ENOMEM;
+		}
+	}
 
 	i = 0;
 	resource_list_for_each_entry(entry, &pp->bridge->dma_ranges) {
-- 
2.52.0

[PATCH v2 3/4] PCI: dwc: Clean up iatu index usage in dw_pcie_iatu_setup()

[Niklas Cassel]

The current iatu index usage in dw_pcie_iatu_setup() is a mess.

For outbound address translation the index is incremented before usage.
For inbound address translation the index is incremented after usage.

Incrementing the index after usage make much more sense, and:
Make the index usage consistent for both outbound and inbound address
translation.

Most likely, the overly complicated logic for the outbound address
translation is because the iatu at index 0 is reserved for CFG IOs
(dw_pcie_other_conf_map_bus()), however, we should be able to use the
exact same logic for the indexing of the outbound and inbound iatus.
(Only the starting index should be different.)

Create two new variables ob_iatu_index_to_use, ib_iatu_index_to_use,
which makes it clear from the name itself that it is the index before
increment.

Since we always check if there is an index available immediately before
programming the iATU, we can remove the useless "ranges exceed outbound
iATU size" warnings, as the code is already unreachable. For the same
reason, we can also remove the useless breaks outside of the while loops.

Also switch to use the more logical, but equivalent check if index is
smaller than length, which is the most common pattern when e.g. looping
through an array which has length items (0 to length-1), such that it is
even clearer to the reader that this is a zeroes based index.

No functional changes intended.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 .../pci/controller/dwc/pcie-designware-host.c | 59 ++++++++++---------
 1 file changed, 32 insertions(+), 27 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 991fe5b9a7b3..76be24af7cfd 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -892,9 +892,10 @@ 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;
+	int ib_iatu_index_to_use;
 	int i, ret;
 
-	/* Note the very first outbound ATU is used for CFG IOs */
 	if (!pci->num_ob_windows) {
 		dev_err(pci->dev, "No outbound iATU found\n");
 		return -EINVAL;
@@ -910,16 +911,18 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
 	for (i = 0; i < pci->num_ib_windows; i++)
 		dw_pcie_disable_atu(pci, PCIE_ATU_REGION_DIR_IB, i);
 
-	i = 0;
+	/*
+	 * 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.
+	 */
+	ob_iatu_index_to_use = 1;
 	resource_list_for_each_entry(entry, &pp->bridge->windows) {
 		resource_size_t res_size;
 
 		if (resource_type(entry->res) != IORESOURCE_MEM)
 			continue;
 
-		if (pci->num_ob_windows <= i + 1)
-			break;
-
 		atu.type = PCIE_ATU_TYPE_MEM;
 		atu.parent_bus_addr = entry->res->start - pci->parent_bus_offset;
 		atu.pci_addr = entry->res->start - entry->offset;
@@ -937,13 +940,13 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
 			 * middle. Otherwise, we would end up only partially
 			 * mapping a single resource.
 			 */
-			if (pci->num_ob_windows <= ++i) {
-				dev_err(pci->dev, "Exhausted outbound windows for region: %pr\n",
+			if (!(ob_iatu_index_to_use < pci->num_ob_windows)) {
+				dev_err(pci->dev, "Cannot add outbound window for region: %pr\n",
 					entry->res);
 				return -ENOMEM;
 			}
 
-			atu.index = i;
+			atu.index = ob_iatu_index_to_use;
 			atu.size = MIN(pci->region_limit + 1, res_size);
 
 			ret = dw_pcie_prog_outbound_atu(pci, &atu);
@@ -953,6 +956,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
 				return ret;
 			}
 
+			ob_iatu_index_to_use++;
 			atu.parent_bus_addr += atu.size;
 			atu.pci_addr += atu.size;
 			res_size -= atu.size;
@@ -960,8 +964,8 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
 	}
 
 	if (pp->io_size) {
-		if (pci->num_ob_windows > ++i) {
-			atu.index = i;
+		if (ob_iatu_index_to_use < pci->num_ob_windows) {
+			atu.index = ob_iatu_index_to_use;
 			atu.type = PCIE_ATU_TYPE_IO;
 			atu.parent_bus_addr = pp->io_base - pci->parent_bus_offset;
 			atu.pci_addr = pp->io_bus_addr;
@@ -973,34 +977,37 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
 					entry->res);
 				return ret;
 			}
+			ob_iatu_index_to_use++;
 		} else {
+			/*
+			 * If there are not enough outbound windows to give I/O
+			 * space its own iATU, the outbound iATU at index 0 will
+			 * be shared between I/O space and CFG IOs, by
+			 * temporarily reconfiguring the iATU to CFG space, in
+			 * order to do a CFG IO, and then immediately restoring
+			 * it to I/O space.
+			 */
 			pp->cfg0_io_shared = true;
 		}
 	}
 
-	if (pci->num_ob_windows <= i)
-		dev_warn(pci->dev, "Ranges exceed outbound iATU size (%d)\n",
-			 pci->num_ob_windows);
-
 	if (pp->use_atu_msg) {
-		if (pci->num_ob_windows > ++i) {
-			pp->msg_atu_index = i;
+		if (ob_iatu_index_to_use < pci->num_ob_windows) {
+			pp->msg_atu_index = ob_iatu_index_to_use;
+			ob_iatu_index_to_use++;
 		} else {
 			dev_err(pci->dev, "Cannot add outbound window for MSG TLP\n");
 			return -ENOMEM;
 		}
 	}
 
-	i = 0;
+	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;
 
 		if (resource_type(entry->res) != IORESOURCE_MEM)
 			continue;
 
-		if (pci->num_ib_windows <= i)
-			break;
-
 		res_size = resource_size(entry->res);
 		res_start = entry->res->start;
 		while (res_size > 0) {
@@ -1009,14 +1016,15 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
 			 * middle. Otherwise, we would end up only partially
 			 * mapping a single resource.
 			 */
-			if (pci->num_ib_windows <= i) {
-				dev_err(pci->dev, "Exhausted inbound windows for region: %pr\n",
+			if (!(ib_iatu_index_to_use < pci->num_ib_windows)) {
+				dev_err(pci->dev, "Cannot add inbound window for region: %pr\n",
 					entry->res);
 				return -ENOMEM;
 			}
 
 			window_size = MIN(pci->region_limit + 1, res_size);
-			ret = dw_pcie_prog_inbound_atu(pci, i++, PCIE_ATU_TYPE_MEM, res_start,
+			ret = dw_pcie_prog_inbound_atu(pci, ib_iatu_index_to_use,
+						       PCIE_ATU_TYPE_MEM, res_start,
 						       res_start - entry->offset, window_size);
 			if (ret) {
 				dev_err(pci->dev, "Failed to set DMA range %pr\n",
@@ -1024,15 +1032,12 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
 				return ret;
 			}
 
+			ib_iatu_index_to_use++;
 			res_start += window_size;
 			res_size -= window_size;
 		}
 	}
 
-	if (pci->num_ib_windows <= i)
-		dev_warn(pci->dev, "Dma-ranges exceed inbound iATU size (%u)\n",
-			 pci->num_ib_windows);
-
 	return 0;
 }
 
-- 
2.52.0

[PATCH v2 4/4] PCI: dwc: Fix missing iATU setup when ECAM is enabled

[Niklas Cassel]

From: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

When ECAM is enabled, the driver skipped calling dw_pcie_iatu_setup()
before configuring ECAM iATU entries. This left IO and MEM outbound
windows unprogrammed, resulting in broken IO transactions. Additionally,
dw_pcie_config_ecam_iatu() was only called during host initialization,
so ECAM-related iATU entries were not restored after suspend/resume,
leading to failures in configuration space access

To resolve these issues, the ECAM iATU configuration is moved into
dw_pcie_iatu_setup(), and dw_pcie_iatu_setup() is invoked when ECAM is
enabled.

Furthermore, error checks are added in dw_pcie_prog_outbound_atu() and
dw_pcie_prog_inbound_atu() so that an error is returned if trying to
programming an iATU that is outside the number of iATUs provided by
the controller.

Fixes: f6fd357f7afb ("PCI: dwc: Prepare the driver for enabling ECAM mechanism using iATU 'CFG Shift Feature'")
Reported-by: Maciej W. Rozycki <macro@orcam.me.uk>
Closes: https://lore.kernel.org/all/alpine.DEB.2.21.2511280256260.36486@angie.orcam.me.uk/
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Co-developed-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 .../pci/controller/dwc/pcie-designware-host.c | 30 +++++++++++--------
 drivers/pci/controller/dwc/pcie-designware.c  |  6 ++++
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 76be24af7cfd..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
@@ -915,8 +907,21 @@ 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 = 1;
+	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;
 
@@ -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..225897c87c49 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

Re: [PATCH v2 3/4] PCI: dwc: Clean up iatu index usage in dw_pcie_iatu_setup()

[Niklas Cassel]

On Thu, Jan 22, 2026 at 11:29:17PM +0100, Niklas Cassel wrote:

(snip)

>  	if (pp->use_atu_msg) {
> -		if (pci->num_ob_windows > ++i) {
> -			pp->msg_atu_index = i;
> +		if (ob_iatu_index_to_use < pci->num_ob_windows) {
> +			pp->msg_atu_index = ob_iatu_index_to_use;
> +			ob_iatu_index_to_use++;
>  		} else {
>  			dev_err(pci->dev, "Cannot add outbound window for MSG TLP\n");
>  			return -ENOMEM;
>  		}
>  	}


To be even more consistent with the while loops,
this part should probably instead be written as:

 	if (pp->use_atu_msg) {
-		if (ob_iatu_index_to_use < pci->num_ob_windows) {
-			pp->msg_atu_index = ob_iatu_index_to_use;
-			ob_iatu_index_to_use++;
-		} else {
+		if (!(ob_iatu_index_to_use < pci->num_ob_windows)) {
 			dev_err(pci->dev, "Cannot add outbound window for MSG TLP\n");
 			return -ENOMEM;
 		}
+		pp->msg_atu_index = ob_iatu_index_to_use;
+		ob_iatu_index_to_use++;
 	}


Kind regards,
Niklas

Re: [PATCH v2 1/4] PCI: dwc: Fix msg_atu_index assignment

[Shawn Lin]

在 2026/01/23 星期五 6:29, Niklas Cassel 写道:
> When dw_pcie_iatu_setup() configures outbound address translation
> for both type PCIE_ATU_TYPE_MEM and PCIE_ATU_TYPE_IO, the iatu index
> to use is incremented before calling dw_pcie_prog_outbound_atu().
> 
> However, for msg_atu_index the index is not incremented before use,
> causing the iATU index to be the same as the last configured iatu
> index, which means that it will incorrectly use the same iatu index
> that is already in use, breaking outbound address translation.
> 

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> Fixes: e1a4ec1a9520 ("PCI: dwc: Add generic MSG TLP support for sending PME_Turn_Off when system suspend")
> Cc: stable@vger.kernel.org
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>   drivers/pci/controller/dwc/pcie-designware-host.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index ab17549af518..cca5fc886409 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -982,7 +982,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
>   		dev_warn(pci->dev, "Ranges exceed outbound iATU size (%d)\n",
>   			 pci->num_ob_windows);
>   
> -	pp->msg_atu_index = i;
> +	pp->msg_atu_index = ++i;
>   
>   	i = 0;
>   	resource_list_for_each_entry(entry, &pp->bridge->dma_ranges) {

Re: [PATCH v2 2/4] PCI: dwc: Improve msg_atu_index error handling

[Shawn Lin]

在 2026/01/23 星期五 6:29, Niklas Cassel 写道:
> Only try to dedicate an outbound iATU to MSG TLP if use_atu_msg is set.
> If use_atu_msg is not set, it is completely useless to bump the index.
> 
> Additionally, if use_atu_msg is set, and there is no available outbound
> iatu available, return an error.
> 

Nit: there is no outbound iATU available....

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> Fixes: e1a4ec1a9520 ("PCI: dwc: Add generic MSG TLP support for sending PME_Turn_Off when system suspend")
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>   drivers/pci/controller/dwc/pcie-designware-host.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index cca5fc886409..991fe5b9a7b3 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -982,7 +982,14 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
>   		dev_warn(pci->dev, "Ranges exceed outbound iATU size (%d)\n",
>   			 pci->num_ob_windows);
>   
> -	pp->msg_atu_index = ++i;
> +	if (pp->use_atu_msg) {
> +		if (pci->num_ob_windows > ++i) {
> +			pp->msg_atu_index = i;
> +		} else {
> +			dev_err(pci->dev, "Cannot add outbound window for MSG TLP\n");
> +			return -ENOMEM;
> +		}
> +	}
>   
>   	i = 0;
>   	resource_list_for_each_entry(entry, &pp->bridge->dma_ranges) {

Re: [PATCH v2 2/4] PCI: dwc: Improve msg_atu_index error handling

[Niklas Cassel]

On Fri, Jan 23, 2026 at 10:07:31AM +0800, Shawn Lin wrote:
> 在 2026/01/23 星期五 6:29, Niklas Cassel 写道:
> > Only try to dedicate an outbound iATU to MSG TLP if use_atu_msg is set.
> > If use_atu_msg is not set, it is completely useless to bump the index.
> > 
> > Additionally, if use_atu_msg is set, and there is no available outbound
> > iatu available, return an error.
> > 
> 
> Nit: there is no outbound iATU available....
> 
> Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

Thank you, I will wait to send V3 until Maciej has had time to test.


Kind regards,
Niklas

Re: [PATCH v2 3/4] PCI: dwc: Clean up iatu index usage in dw_pcie_iatu_setup()

[Manivannan Sadhasivam]

On Thu, Jan 22, 2026 at 11:29:17PM +0100, Niklas Cassel wrote:
> The current iatu index usage in dw_pcie_iatu_setup() is a mess.
> 

s/iatu/iATU here and below.

> For outbound address translation the index is incremented before usage.
> For inbound address translation the index is incremented after usage.
> 
> Incrementing the index after usage make much more sense, and:
> Make the index usage consistent for both outbound and inbound address
> translation.
> 
> Most likely, the overly complicated logic for the outbound address
> translation is because the iatu at index 0 is reserved for CFG IOs
> (dw_pcie_other_conf_map_bus()), however, we should be able to use the
> exact same logic for the indexing of the outbound and inbound iatus.
> (Only the starting index should be different.)
> 
> Create two new variables ob_iatu_index_to_use, ib_iatu_index_to_use,
> which makes it clear from the name itself that it is the index before
> increment.
> 
> Since we always check if there is an index available immediately before
> programming the iATU, we can remove the useless "ranges exceed outbound
> iATU size" warnings, as the code is already unreachable. For the same
> reason, we can also remove the useless breaks outside of the while loops.
> 
> Also switch to use the more logical, but equivalent check if index is
> smaller than length, which is the most common pattern when e.g. looping
> through an array which has length items (0 to length-1), such that it is
> even clearer to the reader that this is a zeroes based index.
> 
> No functional changes intended.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 59 ++++++++++---------
>  1 file changed, 32 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 991fe5b9a7b3..76be24af7cfd 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -892,9 +892,10 @@ 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;
> +	int ib_iatu_index_to_use;

I'd prefer ob_iatu_index, ib_iatu_index

>  	int i, ret;
>  
> -	/* Note the very first outbound ATU is used for CFG IOs */
>  	if (!pci->num_ob_windows) {
>  		dev_err(pci->dev, "No outbound iATU found\n");
>  		return -EINVAL;
> @@ -910,16 +911,18 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
>  	for (i = 0; i < pci->num_ib_windows; i++)
>  		dw_pcie_disable_atu(pci, PCIE_ATU_REGION_DIR_IB, i);
>  
> -	i = 0;
> +	/*
> +	 * 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.
> +	 */
> +	ob_iatu_index_to_use = 1;
>  	resource_list_for_each_entry(entry, &pp->bridge->windows) {
>  		resource_size_t res_size;
>  
>  		if (resource_type(entry->res) != IORESOURCE_MEM)
>  			continue;
>  
> -		if (pci->num_ob_windows <= i + 1)
> -			break;
> -
>  		atu.type = PCIE_ATU_TYPE_MEM;
>  		atu.parent_bus_addr = entry->res->start - pci->parent_bus_offset;
>  		atu.pci_addr = entry->res->start - entry->offset;
> @@ -937,13 +940,13 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
>  			 * middle. Otherwise, we would end up only partially
>  			 * mapping a single resource.
>  			 */
> -			if (pci->num_ob_windows <= ++i) {
> -				dev_err(pci->dev, "Exhausted outbound windows for region: %pr\n",
> +			if (!(ob_iatu_index_to_use < pci->num_ob_windows)) {

			if (ob_iatu_index >= pci->num_ob_windows) ?

> +				dev_err(pci->dev, "Cannot add outbound window for region: %pr\n",
>  					entry->res);
>  				return -ENOMEM;
>  			}
>  
> -			atu.index = i;
> +			atu.index = ob_iatu_index_to_use;
>  			atu.size = MIN(pci->region_limit + 1, res_size);
>  
>  			ret = dw_pcie_prog_outbound_atu(pci, &atu);
> @@ -953,6 +956,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
>  				return ret;
>  			}
>  
> +			ob_iatu_index_to_use++;
>  			atu.parent_bus_addr += atu.size;
>  			atu.pci_addr += atu.size;
>  			res_size -= atu.size;
> @@ -960,8 +964,8 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
>  	}
>  
>  	if (pp->io_size) {
> -		if (pci->num_ob_windows > ++i) {
> -			atu.index = i;
> +		if (ob_iatu_index_to_use < pci->num_ob_windows) {
> +			atu.index = ob_iatu_index_to_use;
>  			atu.type = PCIE_ATU_TYPE_IO;
>  			atu.parent_bus_addr = pp->io_base - pci->parent_bus_offset;
>  			atu.pci_addr = pp->io_bus_addr;
> @@ -973,34 +977,37 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
>  					entry->res);
>  				return ret;
>  			}
> +			ob_iatu_index_to_use++;
>  		} else {
> +			/*
> +			 * If there are not enough outbound windows to give I/O
> +			 * space its own iATU, the outbound iATU at index 0 will
> +			 * be shared between I/O space and CFG IOs, by
> +			 * temporarily reconfiguring the iATU to CFG space, in
> +			 * order to do a CFG IO, and then immediately restoring
> +			 * it to I/O space.
> +			 */
>  			pp->cfg0_io_shared = true;
>  		}
>  	}
>  
> -	if (pci->num_ob_windows <= i)
> -		dev_warn(pci->dev, "Ranges exceed outbound iATU size (%d)\n",
> -			 pci->num_ob_windows);
> -
>  	if (pp->use_atu_msg) {
> -		if (pci->num_ob_windows > ++i) {
> -			pp->msg_atu_index = i;
> +		if (ob_iatu_index_to_use < pci->num_ob_windows) {
> +			pp->msg_atu_index = ob_iatu_index_to_use;
> +			ob_iatu_index_to_use++;
>  		} else {
>  			dev_err(pci->dev, "Cannot add outbound window for MSG TLP\n");
>  			return -ENOMEM;

Let's also be consistent with error checking as well:

		if (ob_iatu_index >= pci->num_ob_windows) {
			dev_err();
			return -ENOMEM;
		}

		pp->msg_atu_index = ob_iatu_index++;

>  		}
>  	}
>  
> -	i = 0;
> +	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;
>  
>  		if (resource_type(entry->res) != IORESOURCE_MEM)
>  			continue;
>  
> -		if (pci->num_ib_windows <= i)
> -			break;
> -
>  		res_size = resource_size(entry->res);
>  		res_start = entry->res->start;
>  		while (res_size > 0) {
> @@ -1009,14 +1016,15 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
>  			 * middle. Otherwise, we would end up only partially
>  			 * mapping a single resource.
>  			 */
> -			if (pci->num_ib_windows <= i) {
> -				dev_err(pci->dev, "Exhausted inbound windows for region: %pr\n",
> +			if (!(ib_iatu_index_to_use < pci->num_ib_windows)) {

			if (ib_iatu_index >= pci->num_ib_windows) ?

- Mani

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

Re: [PATCH v2 4/4] PCI: dwc: Fix missing iATU setup when ECAM is enabled

[Manivannan Sadhasivam]

On Thu, Jan 22, 2026 at 11:29:18PM +0100, Niklas Cassel wrote:
> From: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> 
> When ECAM is enabled, the driver skipped calling dw_pcie_iatu_setup()
> before configuring ECAM iATU entries. This left IO and MEM outbound
> windows unprogrammed, resulting in broken IO transactions. Additionally,
> dw_pcie_config_ecam_iatu() was only called during host initialization,
> so ECAM-related iATU entries were not restored after suspend/resume,
> leading to failures in configuration space access
> 
> To resolve these issues, the ECAM iATU configuration is moved into
> dw_pcie_iatu_setup(), and dw_pcie_iatu_setup() is invoked when ECAM is
> enabled.
> 
> Furthermore, error checks are added in dw_pcie_prog_outbound_atu() and
> dw_pcie_prog_inbound_atu() so that an error is returned if trying to
> programming an iATU that is outside the number of iATUs provided by
> the controller.
> 
> Fixes: f6fd357f7afb ("PCI: dwc: Prepare the driver for enabling ECAM mechanism using iATU 'CFG Shift Feature'")
> Reported-by: Maciej W. Rozycki <macro@orcam.me.uk>
> Closes: https://lore.kernel.org/all/alpine.DEB.2.21.2511280256260.36486@angie.orcam.me.uk/
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Co-developed-by: Niklas Cassel <cassel@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 30 +++++++++++--------
>  drivers/pci/controller/dwc/pcie-designware.c  |  6 ++++
>  2 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 76be24af7cfd..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

Spurious change.

>  	 * 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
> @@ -915,8 +907,21 @@ 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 = 1;
> +	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;
>  
> @@ -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.

Spurious change.

>  	 */
> -	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..225897c87c49 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))

	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))

	if (index >= pci->num_ib_windows)

- Mani

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

Re: [PATCH v2 3/4] PCI: dwc: Clean up iatu index usage in dw_pcie_iatu_setup()

[Niklas Cassel]

On Fri, Jan 23, 2026 at 01:45:07PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Jan 22, 2026 at 11:29:17PM +0100, Niklas Cassel wrote:
> > The current iatu index usage in dw_pcie_iatu_setup() is a mess.
> > 
> 
> s/iatu/iATU here and below.
> 
> > For outbound address translation the index is incremented before usage.
> > For inbound address translation the index is incremented after usage.
> > 
> > Incrementing the index after usage make much more sense, and:
> > Make the index usage consistent for both outbound and inbound address
> > translation.
> > 
> > Most likely, the overly complicated logic for the outbound address
> > translation is because the iatu at index 0 is reserved for CFG IOs
> > (dw_pcie_other_conf_map_bus()), however, we should be able to use the
> > exact same logic for the indexing of the outbound and inbound iatus.
> > (Only the starting index should be different.)
> > 
> > Create two new variables ob_iatu_index_to_use, ib_iatu_index_to_use,
> > which makes it clear from the name itself that it is the index before
> > increment.
> > 
> > Since we always check if there is an index available immediately before
> > programming the iATU, we can remove the useless "ranges exceed outbound
> > iATU size" warnings, as the code is already unreachable. For the same
> > reason, we can also remove the useless breaks outside of the while loops.
> > 
> > Also switch to use the more logical, but equivalent check if index is
> > smaller than length, which is the most common pattern when e.g. looping
> > through an array which has length items (0 to length-1), such that it is
> > even clearer to the reader that this is a zeroes based index.
> > 
> > No functional changes intended.
> > 
> > Signed-off-by: Niklas Cassel <cassel@kernel.org>
> > ---
> >  .../pci/controller/dwc/pcie-designware-host.c | 59 ++++++++++---------
> >  1 file changed, 32 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 991fe5b9a7b3..76be24af7cfd 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -892,9 +892,10 @@ 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;
> > +	int ib_iatu_index_to_use;
> 
> I'd prefer ob_iatu_index, ib_iatu_index

Ok.

> 
> >  	int i, ret;
> >  
> > -	/* Note the very first outbound ATU is used for CFG IOs */
> >  	if (!pci->num_ob_windows) {
> >  		dev_err(pci->dev, "No outbound iATU found\n");
> >  		return -EINVAL;
> > @@ -910,16 +911,18 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> >  	for (i = 0; i < pci->num_ib_windows; i++)
> >  		dw_pcie_disable_atu(pci, PCIE_ATU_REGION_DIR_IB, i);
> >  
> > -	i = 0;
> > +	/*
> > +	 * 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.
> > +	 */
> > +	ob_iatu_index_to_use = 1;
> >  	resource_list_for_each_entry(entry, &pp->bridge->windows) {
> >  		resource_size_t res_size;
> >  
> >  		if (resource_type(entry->res) != IORESOURCE_MEM)
> >  			continue;
> >  
> > -		if (pci->num_ob_windows <= i + 1)
> > -			break;
> > -
> >  		atu.type = PCIE_ATU_TYPE_MEM;
> >  		atu.parent_bus_addr = entry->res->start - pci->parent_bus_offset;
> >  		atu.pci_addr = entry->res->start - entry->offset;
> > @@ -937,13 +940,13 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> >  			 * middle. Otherwise, we would end up only partially
> >  			 * mapping a single resource.
> >  			 */
> > -			if (pci->num_ob_windows <= ++i) {
> > -				dev_err(pci->dev, "Exhausted outbound windows for region: %pr\n",
> > +			if (!(ob_iatu_index_to_use < pci->num_ob_windows)) {
> 
> 			if (ob_iatu_index >= pci->num_ob_windows) ?

Since both you and Frank have commented on this,
I guess I am the weird one.

I will change it.


> 
> > +				dev_err(pci->dev, "Cannot add outbound window for region: %pr\n",
> >  					entry->res);
> >  				return -ENOMEM;
> >  			}
> >  
> > -			atu.index = i;
> > +			atu.index = ob_iatu_index_to_use;
> >  			atu.size = MIN(pci->region_limit + 1, res_size);
> >  
> >  			ret = dw_pcie_prog_outbound_atu(pci, &atu);
> > @@ -953,6 +956,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> >  				return ret;
> >  			}
> >  
> > +			ob_iatu_index_to_use++;
> >  			atu.parent_bus_addr += atu.size;
> >  			atu.pci_addr += atu.size;
> >  			res_size -= atu.size;
> > @@ -960,8 +964,8 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> >  	}
> >  
> >  	if (pp->io_size) {
> > -		if (pci->num_ob_windows > ++i) {
> > -			atu.index = i;
> > +		if (ob_iatu_index_to_use < pci->num_ob_windows) {
> > +			atu.index = ob_iatu_index_to_use;
> >  			atu.type = PCIE_ATU_TYPE_IO;
> >  			atu.parent_bus_addr = pp->io_base - pci->parent_bus_offset;
> >  			atu.pci_addr = pp->io_bus_addr;
> > @@ -973,34 +977,37 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> >  					entry->res);
> >  				return ret;
> >  			}
> > +			ob_iatu_index_to_use++;
> >  		} else {
> > +			/*
> > +			 * If there are not enough outbound windows to give I/O
> > +			 * space its own iATU, the outbound iATU at index 0 will
> > +			 * be shared between I/O space and CFG IOs, by
> > +			 * temporarily reconfiguring the iATU to CFG space, in
> > +			 * order to do a CFG IO, and then immediately restoring
> > +			 * it to I/O space.
> > +			 */
> >  			pp->cfg0_io_shared = true;
> >  		}
> >  	}
> >  
> > -	if (pci->num_ob_windows <= i)
> > -		dev_warn(pci->dev, "Ranges exceed outbound iATU size (%d)\n",
> > -			 pci->num_ob_windows);
> > -
> >  	if (pp->use_atu_msg) {
> > -		if (pci->num_ob_windows > ++i) {
> > -			pp->msg_atu_index = i;
> > +		if (ob_iatu_index_to_use < pci->num_ob_windows) {
> > +			pp->msg_atu_index = ob_iatu_index_to_use;
> > +			ob_iatu_index_to_use++;
> >  		} else {
> >  			dev_err(pci->dev, "Cannot add outbound window for MSG TLP\n");
> >  			return -ENOMEM;
> 
> Let's also be consistent with error checking as well:
> 
> 		if (ob_iatu_index >= pci->num_ob_windows) {
> 			dev_err();
> 			return -ENOMEM;
> 		}
> 
> 		pp->msg_atu_index = ob_iatu_index++;


Yes, I replied this to myself as well:
https://lore.kernel.org/linux-pci/aXKpHS20vHDyh4fL@ryzen/

> 
> >  		}
> >  	}
> >  
> > -	i = 0;
> > +	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;
> >  
> >  		if (resource_type(entry->res) != IORESOURCE_MEM)
> >  			continue;
> >  
> > -		if (pci->num_ib_windows <= i)
> > -			break;
> > -
> >  		res_size = resource_size(entry->res);
> >  		res_start = entry->res->start;
> >  		while (res_size > 0) {
> > @@ -1009,14 +1016,15 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> >  			 * middle. Otherwise, we would end up only partially
> >  			 * mapping a single resource.
> >  			 */
> > -			if (pci->num_ib_windows <= i) {
> > -				dev_err(pci->dev, "Exhausted inbound windows for region: %pr\n",
> > +			if (!(ib_iatu_index_to_use < pci->num_ib_windows)) {
> 
> 			if (ib_iatu_index >= pci->num_ib_windows) ?

Yes, if I change it in one place, I will change it everywhere.



Kind regards,
Niklas