[PATCH 0/2] PCI: dwc: Fix missing iATU setup when ECAM is enabled

[PATCH 0/2] PCI: dwc: Fix missing iATU setup when ECAM is enabled

[Krishna Chaitanya Chundru]

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_setup_rc(). At the same time, dw_pcie_iatu_setup() is invoked
when ECAM is enabled.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
Krishna Chaitanya Chundru (2):
      PCI: dwc: Correct iATU index increment for MSG TLP region
      PCI: dwc: Fix missing iATU setup when ECAM is enabled

 drivers/pci/controller/dwc/pcie-designware-host.c | 37 ++++++++++++++---------
 drivers/pci/controller/dwc/pcie-designware.c      |  3 ++
 drivers/pci/controller/dwc/pcie-designware.h      |  2 +-
 3 files changed, 26 insertions(+), 16 deletions(-)
---
base-commit: 3f9f0252130e7dd60d41be0802bf58f6471c691d
change-id: 20251203-ecam_io_fix-6e060fecd3b8

Best regards,
-- 
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

[PATCH 1/2] PCI: dwc: Correct iATU index increment for MSG TLP region

[Krishna Chaitanya Chundru]

Commit e1a4ec1a9520 ("PCI: dwc: Add generic MSG TLP support for sending
PME_Turn_Off when system suspend") introduced a mechanism to reserve an
iATU window for MSG TLP transactions. However, the code incorrectly
assigned pp->msg_atu_index = i without incrementing i first, causing
the MSG TLP region to reuse the last configured outbound window instead
of the next available one. This can cause issue with IO transfers as
this can over write iATU configured for IO memory.

Fix this by incrementing i before assigning it to msg_atu_index so
that the MSG TLP region uses a dedicated iATU entry.

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: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.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 e92513c5bda51bde3a7157033ddbd73afa370d78..4fb6331fbc2b322c1a1b6a8e4fe08f92e170da19 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -942,7 +942,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.34.1

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

[Krishna Chaitanya Chundru]

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_setup_rc(). At the same time, dw_pcie_iatu_setup() is invoked
when ECAM is enabled.

Rename msg_atu_index to ob_atu_index to track the next available outbound
iATU index for ECAM and MSG TLP windows. Furthermore, an error check is
added in dw_pcie_prog_outbound_atu() to avoid programming beyond
num_ob_windows.

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>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 37 ++++++++++++++---------
 drivers/pci/controller/dwc/pcie-designware.c      |  3 ++
 drivers/pci/controller/dwc/pcie-designware.h      |  2 +-
 3 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 4fb6331fbc2b322c1a1b6a8e4fe08f92e170da19..22c6ec7bff8840d935803f0b96749413b1c3f905 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -433,7 +433,7 @@ static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
 	 * Immediate bus under Root Bus, needs type 0 iATU configuration and
 	 * remaining buses need type 1 iATU configuration.
 	 */
-	atu.index = 0;
+	atu.index = pp->ob_atu_index;
 	atu.type = PCIE_ATU_TYPE_CFG0;
 	atu.parent_bus_addr = pp->cfg0_base + SZ_1M;
 	/* 1MiB is to cover 1 (bus) * 32 (devices) * 8 (functions) */
@@ -443,6 +443,8 @@ static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
 	if (ret)
 		return ret;
 
+	pp->ob_atu_index++;
+
 	bus_range_max = resource_size(bus->res);
 
 	if (bus_range_max < 2)
@@ -455,7 +457,11 @@ static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
 	atu.size = (SZ_1M * bus_range_max) - SZ_2M;
 	atu.ctrl2 = PCIE_ATU_CFG_SHIFT_MODE_ENABLE;
 
-	return dw_pcie_prog_outbound_atu(pci, &atu);
+	ret = dw_pcie_prog_outbound_atu(pci, &atu);
+	if (!ret)
+		pp->ob_atu_index++;
+
+	return ret;
 }
 
 static int dw_pcie_create_ecam_window(struct dw_pcie_rp *pp, struct resource *res)
@@ -630,14 +636,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
@@ -942,7 +940,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->ob_atu_index = ++i;
 
 	i = 0;
 	resource_list_for_each_entry(entry, &pp->bridge->dma_ranges) {
@@ -1084,14 +1082,23 @@ 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 setup 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;
 	}
 
+	if (pp->ecam_enabled) {
+		ret = dw_pcie_config_ecam_iatu(pp);
+		if (ret) {
+			dev_err(pci->dev, "Failed to configure iATU in ECAM mode\n");
+			return ret;
+		}
+	}
+
 	dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, 0);
 
 	/* Program correct class for RC */
@@ -1113,7 +1120,7 @@ static int dw_pcie_pme_turn_off(struct dw_pcie *pci)
 	void __iomem *mem;
 	int ret;
 
-	if (pci->num_ob_windows <= pci->pp.msg_atu_index)
+	if (pci->num_ob_windows <= pci->pp.ob_atu_index)
 		return -ENOSPC;
 
 	if (!pci->pp.msg_res)
@@ -1123,7 +1130,7 @@ static int dw_pcie_pme_turn_off(struct dw_pcie *pci)
 	atu.routing = PCIE_MSG_TYPE_R_BC;
 	atu.type = PCIE_ATU_TYPE_MSG;
 	atu.size = resource_size(pci->pp.msg_res);
-	atu.index = pci->pp.msg_atu_index;
+	atu.index = pci->pp.ob_atu_index;
 
 	atu.parent_bus_addr = pci->pp.msg_res->start - pci->parent_bus_offset;
 
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index c644216995f69cbf065e61a0392bf1e5e32cf56e..f9f3c2f3532e0d0e9f8e4f42d8c5c9db68d55272 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -476,6 +476,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) ||
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index e995f692a1ecd10130d3be3358827f801811387f..69d0bd8b3c57353763f98999e5d39527861fe1a7 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -423,8 +423,8 @@ struct dw_pcie_rp {
 	struct pci_host_bridge  *bridge;
 	raw_spinlock_t		lock;
 	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
+	int			ob_atu_index;
 	bool			use_atu_msg;
-	int			msg_atu_index;
 	struct resource		*msg_res;
 	bool			use_linkup_irq;
 	struct pci_eq_presets	presets;

-- 
2.34.1

Re: [PATCH 1/2] PCI: dwc: Correct iATU index increment for MSG TLP region

[Frank Li]

On Wed, Dec 03, 2025 at 11:50:14AM +0530, Krishna Chaitanya Chundru wrote:
> Commit e1a4ec1a9520 ("PCI: dwc: Add generic MSG TLP support for sending
> PME_Turn_Off when system suspend") introduced a mechanism to reserve an
> iATU window for MSG TLP transactions. However, the code incorrectly
> assigned pp->msg_atu_index = i without incrementing i first, causing
> the MSG TLP region to reuse the last configured outbound window instead
> of the next available one. This can cause issue with IO transfers as
> this can over write iATU configured for IO memory.
>
> Fix this by incrementing i before assigning it to msg_atu_index so
> that the MSG TLP region uses a dedicated iATU entry.
>
> 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: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

Thank you fix it.

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 e92513c5bda51bde3a7157033ddbd73afa370d78..4fb6331fbc2b322c1a1b6a8e4fe08f92e170da19 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -942,7 +942,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.34.1
>

Re: [PATCH 1/2] PCI: dwc: Correct iATU index increment for MSG TLP region

[Maciej W. Rozycki]

On Wed, 3 Dec 2025, Krishna Chaitanya Chundru wrote:

> Fix this by incrementing i before assigning it to msg_atu_index so
> that the MSG TLP region uses a dedicated iATU entry.

Tested-by: Maciej W. Rozycki <macro@orcam.me.uk>

  Maciej

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

[Maciej W. Rozycki]

On Wed, 3 Dec 2025, Krishna Chaitanya Chundru wrote:

> To resolve these issues, the ECAM iATU configuration is moved into
> dw_pcie_setup_rc(). At the same time, dw_pcie_iatu_setup() is invoked
> when ECAM is enabled.

Tested-by: Maciej W. Rozycki <macro@orcam.me.uk>

  Maciej

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

[Manivannan Sadhasivam]

On Wed, 03 Dec 2025 11:50:13 +0530, Krishna Chaitanya Chundru wrote:
> 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.
> 
> [...]

Applied, thanks!

[1/2] PCI: dwc: Correct iATU index increment for MSG TLP region
      commit: 3c364c9b96f1a0629a29363cdc6239c1ad2f68ad
[2/2] PCI: dwc: Fix missing iATU setup when ECAM is enabled
      commit: 37781eb814e16c75abb78dec2f9412d2e4d88298

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>

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

[Bjorn Helgaas]

On Wed, Dec 03, 2025 at 11:50:15AM +0530, Krishna Chaitanya Chundru wrote:
> 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

Thanks for fixing this bug.

> To resolve these issues, the ECAM iATU configuration is moved into
> dw_pcie_setup_rc(). At the same time, dw_pcie_iatu_setup() is invoked
> when ECAM is enabled.
> 
> Rename msg_atu_index to ob_atu_index to track the next available outbound
> iATU index for ECAM and MSG TLP windows. Furthermore, an error check is
> added in dw_pcie_prog_outbound_atu() to avoid programming beyond
> num_ob_windows.
> 
> 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>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 37 ++++++++++++++---------
>  drivers/pci/controller/dwc/pcie-designware.c      |  3 ++
>  drivers/pci/controller/dwc/pcie-designware.h      |  2 +-
>  3 files changed, 26 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 4fb6331fbc2b322c1a1b6a8e4fe08f92e170da19..22c6ec7bff8840d935803f0b96749413b1c3f905 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -433,7 +433,7 @@ static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
>  	 * Immediate bus under Root Bus, needs type 0 iATU configuration and
>  	 * remaining buses need type 1 iATU configuration.

Tangent: I think this comment should say "Immediate bus under Root
Port needs type 0" or "Root bus needs type 0".

>  	 */
> -	atu.index = 0;
> +	atu.index = pp->ob_atu_index;

Previously we used entries 0 and 1 for config space accesses; now we
program the entries for bridge->windows first and use later entries for
config space.  But the implicit reservation of entry 0 persists below.

>  	atu.type = PCIE_ATU_TYPE_CFG0;
>  	atu.parent_bus_addr = pp->cfg0_base + SZ_1M;
>  	/* 1MiB is to cover 1 (bus) * 32 (devices) * 8 (functions) */
> @@ -443,6 +443,8 @@ static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
>  	if (ret)
>  		return ret;
>  
> +	pp->ob_atu_index++;
> +
>  	bus_range_max = resource_size(bus->res);
>  
>  	if (bus_range_max < 2)
> @@ -455,7 +457,11 @@ static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
>  	atu.size = (SZ_1M * bus_range_max) - SZ_2M;
>  	atu.ctrl2 = PCIE_ATU_CFG_SHIFT_MODE_ENABLE;
>  
> -	return dw_pcie_prog_outbound_atu(pci, &atu);
> +	ret = dw_pcie_prog_outbound_atu(pci, &atu);
> +	if (!ret)
> +		pp->ob_atu_index++;
> +
> +	return ret;

This would be better like this to match the type 0 case:

    ret = dw_pcie_prog_outbound_atu(pci, &atu);
    if (ret)
        return ret;

    pp->ob_atu_index++;
    return 0;

>  }
>  
>  static int dw_pcie_create_ecam_window(struct dw_pcie_rp *pp, struct resource *res)
> @@ -630,14 +636,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
> @@ -942,7 +940,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)

Earlier in dw_pcie_iatu_setup(), we have this:

    i = 0;
    resource_list_for_each_entry(entry, &pp->bridge->windows) {
        if (pci->num_ob_windows <= ++i)
            break;

        atu.index = i;
        dw_pcie_prog_outbound_atu(pci, &atu);

I think this starts at 1 because of the implicit reservation of entry
0 for config access, so now I think entry 0 is never used.

Since dw_pcie_prog_outbound_atu() now checks against
pci->num_ob_windows, I don't think we should also do it here.

The use of preincrement also makes this harder to read than it should
be.  You could do something like this:

    i = 0;
    resource_list_for_each_entry(entry, &pp->bridge->windows) {
        atu.index = i++;   # i++ better matches inbound ATU setup
        ret = dw_pcie_prog_outbound_atu(pci, &atu);
        if (ret)
            return ret;
        pci->ob_atu_index = i;
    }

>  		dev_warn(pci->dev, "Ranges exceed outbound iATU size (%d)\n",
>  			 pci->num_ob_windows);
>  
> -	pp->msg_atu_index = ++i;
> +	pp->ob_atu_index = ++i;
>  
>  	i = 0;
>  	resource_list_for_each_entry(entry, &pp->bridge->dma_ranges) {
> @@ -1084,14 +1082,23 @@ 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 setup ATU here.

Wrap comment to fit in 80 columns.

s/setup ATU/set up ATU/

>  	 */
> -	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;
>  	}
>  
> +	if (pp->ecam_enabled) {
> +		ret = dw_pcie_config_ecam_iatu(pp);
> +		if (ret) {
> +			dev_err(pci->dev, "Failed to configure iATU in ECAM mode\n");
> +			return ret;
> +		}
> +	}
> +
>  	dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, 0);
>  
>  	/* Program correct class for RC */
> @@ -1113,7 +1120,7 @@ static int dw_pcie_pme_turn_off(struct dw_pcie *pci)
>  	void __iomem *mem;
>  	int ret;
>  
> -	if (pci->num_ob_windows <= pci->pp.msg_atu_index)
> +	if (pci->num_ob_windows <= pci->pp.ob_atu_index)
>  		return -ENOSPC;

You added basically the same check in dw_pcie_prog_outbound_atu(), so
I don't think this check is needed, is it?

>  	if (!pci->pp.msg_res)
> @@ -1123,7 +1130,7 @@ static int dw_pcie_pme_turn_off(struct dw_pcie *pci)
>  	atu.routing = PCIE_MSG_TYPE_R_BC;
>  	atu.type = PCIE_ATU_TYPE_MSG;
>  	atu.size = resource_size(pci->pp.msg_res);
> -	atu.index = pci->pp.msg_atu_index;
> +	atu.index = pci->pp.ob_atu_index;
>  
>  	atu.parent_bus_addr = pci->pp.msg_res->start - pci->parent_bus_offset;

It worries me a bit that we don't log any messages when this function
fails.  If it ever does fail, whether we failed to allocate pp.msg_res
or there are no available ATU entries, it looks like suspend might
silently fail with no clue in the logs, which sounds like a hassle to
debug.

> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index c644216995f69cbf065e61a0392bf1e5e32cf56e..f9f3c2f3532e0d0e9f8e4f42d8c5c9db68d55272 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -476,6 +476,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) ||
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index e995f692a1ecd10130d3be3358827f801811387f..69d0bd8b3c57353763f98999e5d39527861fe1a7 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -423,8 +423,8 @@ struct dw_pcie_rp {
>  	struct pci_host_bridge  *bridge;
>  	raw_spinlock_t		lock;
>  	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
> +	int			ob_atu_index;

The number of outbound ATU entries (num_ob_windows) is apparently a
Root Complex property, not a Root Port property, so it's in struct
dw_pcie.  If ob_atu_index tracks how many of those entries are in use,
I think it should be in struct dw_pci too instead of being in struct
dw_pcie_rp.

>  	bool			use_atu_msg;
> -	int			msg_atu_index;
>  	struct resource		*msg_res;
>  	bool			use_linkup_irq;
>  	struct pci_eq_presets	presets;
> 
> -- 
> 2.34.1
> 

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

[Krishna Chaitanya Chundru]

On 12/27/2025 2:22 AM, Bjorn Helgaas wrote:
> On Wed, Dec 03, 2025 at 11:50:15AM +0530, Krishna Chaitanya Chundru wrote:
>> 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
> Thanks for fixing this bug.
>
>> To resolve these issues, the ECAM iATU configuration is moved into
>> dw_pcie_setup_rc(). At the same time, dw_pcie_iatu_setup() is invoked
>> when ECAM is enabled.
>>
>> Rename msg_atu_index to ob_atu_index to track the next available outbound
>> iATU index for ECAM and MSG TLP windows. Furthermore, an error check is
>> added in dw_pcie_prog_outbound_atu() to avoid programming beyond
>> num_ob_windows.
>>
>> 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>
>> ---
>>   drivers/pci/controller/dwc/pcie-designware-host.c | 37 ++++++++++++++---------
>>   drivers/pci/controller/dwc/pcie-designware.c      |  3 ++
>>   drivers/pci/controller/dwc/pcie-designware.h      |  2 +-
>>   3 files changed, 26 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>> index 4fb6331fbc2b322c1a1b6a8e4fe08f92e170da19..22c6ec7bff8840d935803f0b96749413b1c3f905 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>> @@ -433,7 +433,7 @@ static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
>>   	 * Immediate bus under Root Bus, needs type 0 iATU configuration and
>>   	 * remaining buses need type 1 iATU configuration.
> Tangent: I think this comment should say "Immediate bus under Root
> Port needs type 0" or "Root bus needs type 0".
>
>>   	 */
>> -	atu.index = 0;
>> +	atu.index = pp->ob_atu_index;
> Previously we used entries 0 and 1 for config space accesses; now we
> program the entries for bridge->windows first and use later entries for
> config space.  But the implicit reservation of entry 0 persists below.
>
>>   	atu.type = PCIE_ATU_TYPE_CFG0;
>>   	atu.parent_bus_addr = pp->cfg0_base + SZ_1M;
>>   	/* 1MiB is to cover 1 (bus) * 32 (devices) * 8 (functions) */
>> @@ -443,6 +443,8 @@ static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
>>   	if (ret)
>>   		return ret;
>>   
>> +	pp->ob_atu_index++;
>> +
>>   	bus_range_max = resource_size(bus->res);
>>   
>>   	if (bus_range_max < 2)
>> @@ -455,7 +457,11 @@ static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
>>   	atu.size = (SZ_1M * bus_range_max) - SZ_2M;
>>   	atu.ctrl2 = PCIE_ATU_CFG_SHIFT_MODE_ENABLE;
>>   
>> -	return dw_pcie_prog_outbound_atu(pci, &atu);
>> +	ret = dw_pcie_prog_outbound_atu(pci, &atu);
>> +	if (!ret)
>> +		pp->ob_atu_index++;
>> +
>> +	return ret;
> This would be better like this to match the type 0 case:
>
>      ret = dw_pcie_prog_outbound_atu(pci, &atu);
>      if (ret)
>          return ret;
>
>      pp->ob_atu_index++;
>      return 0;
>
>>   }
>>   
>>   static int dw_pcie_create_ecam_window(struct dw_pcie_rp *pp, struct resource *res)
>> @@ -630,14 +636,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
>> @@ -942,7 +940,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> Earlier in dw_pcie_iatu_setup(), we have this:
>
>      i = 0;
>      resource_list_for_each_entry(entry, &pp->bridge->windows) {
>          if (pci->num_ob_windows <= ++i)
>              break;
>
>          atu.index = i;
>          dw_pcie_prog_outbound_atu(pci, &atu);
>
> I think this starts at 1 because of the implicit reservation of entry
> 0 for config access, so now I think entry 0 is never used.
>
> Since dw_pcie_prog_outbound_atu() now checks against
> pci->num_ob_windows, I don't think we should also do it here.
>
> The use of preincrement also makes this harder to read than it should
> be.  You could do something like this:
>
>      i = 0;
>      resource_list_for_each_entry(entry, &pp->bridge->windows) {
>          atu.index = i++;   # i++ better matches inbound ATU setup
>          ret = dw_pcie_prog_outbound_atu(pci, &atu);
>          if (ret)
>              return ret;
>          pci->ob_atu_index = i;
>      }
Yeah, this problem still present, this patches actually aimed for 6.19, 
so I skipped fixing
this index 0 reservation issue explicitly, since this needs separate  
patch as this needs to
be back ported separately.  I will raise separate patch for this.
>>   		dev_warn(pci->dev, "Ranges exceed outbound iATU size (%d)\n",
>>   			 pci->num_ob_windows);
>>   
>> -	pp->msg_atu_index = ++i;
>> +	pp->ob_atu_index = ++i;
>>   
>>   	i = 0;
>>   	resource_list_for_each_entry(entry, &pp->bridge->dma_ranges) {
>> @@ -1084,14 +1082,23 @@ 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 setup ATU here.
> Wrap comment to fit in 80 columns.
>
> s/setup ATU/set up ATU/
>
>>   	 */
>> -	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;
>>   	}
>>   
>> +	if (pp->ecam_enabled) {
>> +		ret = dw_pcie_config_ecam_iatu(pp);
>> +		if (ret) {
>> +			dev_err(pci->dev, "Failed to configure iATU in ECAM mode\n");
>> +			return ret;
>> +		}
>> +	}
>> +
>>   	dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, 0);
>>   
>>   	/* Program correct class for RC */
>> @@ -1113,7 +1120,7 @@ static int dw_pcie_pme_turn_off(struct dw_pcie *pci)
>>   	void __iomem *mem;
>>   	int ret;
>>   
>> -	if (pci->num_ob_windows <= pci->pp.msg_atu_index)
>> +	if (pci->num_ob_windows <= pci->pp.ob_atu_index)
>>   		return -ENOSPC;
> You added basically the same check in dw_pcie_prog_outbound_atu(), so
> I don't think this check is needed, is it?

dw_pcie_create_ecam_window, is not checking if the index is in the num_ob_windows
limit or not. Instead of having two checks in each iATU created, I added single
check here.

>>   	if (!pci->pp.msg_res)
>> @@ -1123,7 +1130,7 @@ static int dw_pcie_pme_turn_off(struct dw_pcie *pci)
>>   	atu.routing = PCIE_MSG_TYPE_R_BC;
>>   	atu.type = PCIE_ATU_TYPE_MSG;
>>   	atu.size = resource_size(pci->pp.msg_res);
>> -	atu.index = pci->pp.msg_atu_index;
>> +	atu.index = pci->pp.ob_atu_index;
>>   
>>   	atu.parent_bus_addr = pci->pp.msg_res->start - pci->parent_bus_offset;
> It worries me a bit that we don't log any messages when this function
> fails.  If it ever does fail, whether we failed to allocate pp.msg_res
> or there are no available ATU entries, it looks like suspend might
> silently fail with no clue in the logs, which sounds like a hassle to
> debug.
Ack.
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>> index c644216995f69cbf065e61a0392bf1e5e32cf56e..f9f3c2f3532e0d0e9f8e4f42d8c5c9db68d55272 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>> @@ -476,6 +476,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) ||
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>> index e995f692a1ecd10130d3be3358827f801811387f..69d0bd8b3c57353763f98999e5d39527861fe1a7 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>> @@ -423,8 +423,8 @@ struct dw_pcie_rp {
>>   	struct pci_host_bridge  *bridge;
>>   	raw_spinlock_t		lock;
>>   	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
>> +	int			ob_atu_index;
> The number of outbound ATU entries (num_ob_windows) is apparently a
> Root Complex property, not a Root Port property, so it's in struct
> dw_pcie.  If ob_atu_index tracks how many of those entries are in use,
> I think it should be in struct dw_pci too instead of being in struct
> dw_pcie_rp.
ack.

- Krishna Chaitanya.
>>   	bool			use_atu_msg;
>> -	int			msg_atu_index;
>>   	struct resource		*msg_res;
>>   	bool			use_linkup_irq;
>>   	struct pci_eq_presets	presets;
>>
>> -- 
>> 2.34.1
>>