Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH 1/3] PCI: dwc: Fix msg_atu_index assignment
@ 2026-01-22 14:54 Niklas Cassel
  2026-01-22 14:54 ` [PATCH 2/3] PCI: dwc: Improve msg_atu_index error handling Niklas Cassel
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Niklas Cassel @ 2026-01-22 14:54 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, Frank Li
  Cc: Randolph Lin, Samuel Holland, Charles Mirabile, tim609,
	Krishna Chaitanya Chundru, Niklas Cassel, stable, linux-pci

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

base-commit: e9a5415adb209f86a05e55b850127ada82e070f1
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] PCI: dwc: Improve msg_atu_index error handling
  2026-01-22 14:54 [PATCH 1/3] PCI: dwc: Fix msg_atu_index assignment Niklas Cassel
@ 2026-01-22 14:54 ` Niklas Cassel
  2026-01-22 20:33   ` Frank Li
  2026-01-22 14:54 ` [PATCH 3/3] PCI: dwc: Clean up iatu index usage in dw_pcie_iatu_setup() Niklas Cassel
  2026-01-22 20:31 ` [PATCH 1/3] PCI: dwc: Fix msg_atu_index assignment Frank Li
  2 siblings, 1 reply; 7+ messages in thread
From: Niklas Cassel @ 2026-01-22 14:54 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, Frank Li
  Cc: Randolph Lin, Samuel Holland, Charles Mirabile, tim609,
	Krishna Chaitanya Chundru, Niklas Cassel, linux-pci

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] PCI: dwc: Clean up iatu index usage in dw_pcie_iatu_setup()
  2026-01-22 14:54 [PATCH 1/3] PCI: dwc: Fix msg_atu_index assignment Niklas Cassel
  2026-01-22 14:54 ` [PATCH 2/3] PCI: dwc: Improve msg_atu_index error handling Niklas Cassel
@ 2026-01-22 14:54 ` Niklas Cassel
  2026-01-22 20:44   ` Frank Li
  2026-01-22 20:31 ` [PATCH 1/3] PCI: dwc: Fix msg_atu_index assignment Frank Li
  2 siblings, 1 reply; 7+ messages in thread
From: Niklas Cassel @ 2026-01-22 14:54 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas
  Cc: Randolph Lin, Samuel Holland, Frank Li, Charles Mirabile, tim609,
	Krishna Chaitanya Chundru, Niklas Cassel, linux-pci

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.

Because we always check if there is an index available immediately before
programming the iATU, we can also remove the useless breaks outside of
while loop.

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..eda94db04b63 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 = 0;
+	int ib_iatu_index_to_use = 0;
 	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,19 @@ 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++;
+
 	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 +941,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 +957,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 +965,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 +978,36 @@ 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;
 	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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] PCI: dwc: Fix msg_atu_index assignment
  2026-01-22 14:54 [PATCH 1/3] PCI: dwc: Fix msg_atu_index assignment Niklas Cassel
  2026-01-22 14:54 ` [PATCH 2/3] PCI: dwc: Improve msg_atu_index error handling Niklas Cassel
  2026-01-22 14:54 ` [PATCH 3/3] PCI: dwc: Clean up iatu index usage in dw_pcie_iatu_setup() Niklas Cassel
@ 2026-01-22 20:31 ` Frank Li
  2 siblings, 0 replies; 7+ messages in thread
From: Frank Li @ 2026-01-22 20:31 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Randolph Lin, Samuel Holland, Charles Mirabile, tim609,
	Krishna Chaitanya Chundru, stable, linux-pci

On Thu, Jan 22, 2026 at 03:54:12PM +0100, Niklas Cassel wrote:
> 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
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  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) {
>
> base-commit: e9a5415adb209f86a05e55b850127ada82e070f1
> --
> 2.52.0
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] PCI: dwc: Improve msg_atu_index error handling
  2026-01-22 14:54 ` [PATCH 2/3] PCI: dwc: Improve msg_atu_index error handling Niklas Cassel
@ 2026-01-22 20:33   ` Frank Li
  0 siblings, 0 replies; 7+ messages in thread
From: Frank Li @ 2026-01-22 20:33 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Randolph Lin, Samuel Holland, Charles Mirabile, tim609,
	Krishna Chaitanya Chundru, linux-pci

On Thu, Jan 22, 2026 at 03:54:13PM +0100, Niklas Cassel wrote:
> 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")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>
>  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
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] PCI: dwc: Clean up iatu index usage in dw_pcie_iatu_setup()
  2026-01-22 14:54 ` [PATCH 3/3] PCI: dwc: Clean up iatu index usage in dw_pcie_iatu_setup() Niklas Cassel
@ 2026-01-22 20:44   ` Frank Li
  2026-01-22 21:02     ` Niklas Cassel
  0 siblings, 1 reply; 7+ messages in thread
From: Frank Li @ 2026-01-22 20:44 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Randolph Lin, Samuel Holland, Charles Mirabile, tim609,
	Krishna Chaitanya Chundru, linux-pci

On Thu, Jan 22, 2026 at 03:54:14PM +0100, Niklas Cassel wrote:
> 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.
>
> Because we always check if there is an index available immediately before
> programming the iATU, we can also remove the useless breaks outside of
> while loop.

The condition is the same. So combine two paragraph

>
> 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..eda94db04b63 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 = 0;
> +	int ib_iatu_index_to_use = 0;
>  	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,19 @@ 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++;
> +
>  	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 +941,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)) {

Is it better "if (ob_iatu_index_to_use >= pci->num_ob_windows)"

Frank
> +				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 +957,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 +965,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 +978,36 @@ 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;
>  	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
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] PCI: dwc: Clean up iatu index usage in dw_pcie_iatu_setup()
  2026-01-22 20:44   ` Frank Li
@ 2026-01-22 21:02     ` Niklas Cassel
  0 siblings, 0 replies; 7+ messages in thread
From: Niklas Cassel @ 2026-01-22 21:02 UTC (permalink / raw)
  To: Frank Li
  Cc: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Randolph Lin, Samuel Holland, Charles Mirabile, tim609,
	Krishna Chaitanya Chundru, linux-pci

On Thu, Jan 22, 2026 at 03:44:24PM -0500, Frank Li wrote:
> On Thu, Jan 22, 2026 at 03:54:14PM +0100, Niklas Cassel wrote:
> > 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.
> >
> > Because we always check if there is an index available immediately before
> > programming the iATU, we can also remove the useless breaks outside of
> > while loop.
> 
> The condition is the same. So combine two paragraph

Sure.


> 
> >
> > 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..eda94db04b63 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 = 0;
> > +	int ib_iatu_index_to_use = 0;
> >  	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,19 @@ 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++;
> > +
> >  	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 +941,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)) {
> 
> Is it better "if (ob_iatu_index_to_use >= pci->num_ob_windows)"

Personally, I think no, since if you look at the condition
now used (after this patch) in:

if (pp->io_size) {
	if (ob_iatu_index_to_use < pci->num_ob_windows) {
		...

if (pp->use_atu_msg) {
	if (ob_iatu_index_to_use < pci->num_ob_windows) {
		...



The difference is that
if (pp->io_size) { and
if (pp->use_atu_msg) {

only does something if (condition)

while the loops over the memory windows have:
if (!condition)
	return -ENOMEM;

For consistency, and to try to avoid this function becoming a mess again,
I think it makes sense to use the exact same condition everywhere, and
only negate the condition if needed.


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-01-22 21:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-22 14:54 [PATCH 1/3] PCI: dwc: Fix msg_atu_index assignment Niklas Cassel
2026-01-22 14:54 ` [PATCH 2/3] PCI: dwc: Improve msg_atu_index error handling Niklas Cassel
2026-01-22 20:33   ` Frank Li
2026-01-22 14:54 ` [PATCH 3/3] PCI: dwc: Clean up iatu index usage in dw_pcie_iatu_setup() Niklas Cassel
2026-01-22 20:44   ` Frank Li
2026-01-22 21:02     ` Niklas Cassel
2026-01-22 20:31 ` [PATCH 1/3] PCI: dwc: Fix msg_atu_index assignment Frank Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox