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

[Damien Le Moal <dlemoal@kernel.org>]

On 2026/01/24 5:28, 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() such that an error is returned if trying to
> program 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>

It looks like this patch depends on patch 3, which does not have a Fixes tag. So
why not squash together patches 3 and 4 ?

> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 33 ++++++++++++-------
>  drivers/pci/controller/dwc/pcie-designware.c  |  6 ++++
>  2 files changed, 28 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 87e6a32dbb9a..bc2e08ec515e 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -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 = 1;
> +	if (pp->ecam_enabled) {
> +		ob_iatu_index = 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 = 1;
> +	}
> +
>  	resource_list_for_each_entry(entry, &pp->bridge->windows) {
>  		resource_size_t res_size;
>  
> @@ -985,8 +990,14 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
>  			 * 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.
> +			 * it to I/O space. This is only implemented when using
> +			 * dw_pcie_other_conf_map_bus(), which is not the case
> +			 * when using ECAM.
>  			 */
> +			if (pp->ecam_enabled) {
> +				dev_err(pci->dev, "Cannot add outbound window for I/O\n");
> +				return -ENOMEM;
> +			}
>  			pp->cfg0_io_shared = true;
>  		}
>  	}
> @@ -1157,7 +1168,7 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
>  	 * the platform uses its own address translation component rather than
>  	 * ATU, so we should not program the 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 18331d9e85be..5741c09dde7f 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -532,6 +532,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) ||
> @@ -605,6 +608,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) {


-- 
Damien Le Moal
Western Digital Research