diff mbox series

[v1,4/9] PCI: microchip: Clean up initialisation of interrupts

Message ID 20221116135504.258687-5-daire.mcnamara@microchip.com (mailing list archive)
State Superseded
Headers show
Series PCI: microchip: Partition address translations | expand

Checks

Context Check Description
conchuod/tree_selection fail Guessing tree name failed

Commit Message

Daire McNamara Nov. 16, 2022, 1:54 p.m. UTC 
From: Daire McNamara <daire.mcnamara@microchip.com>

Refactor interrupt handling in _init() function into
disable_interrupts(), init_interrupts(), clear_sec_errors() and clear
ded_errors().  It was unwieldy and prone to bugs. Then clearly disable
interrupts as soon as possible and only enable interrupts after address
translation errors are setup to prevent spurious axi2pcie and pcie2axi
translation errors being reported

Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/pci/controller/pcie-microchip-host.c | 148 ++++++++++++-------
 1 file changed, 92 insertions(+), 56 deletions(-)

Comments

kernel test robot Nov. 16, 2022, 3:17 p.m. UTC | #1 
Hi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on 3c1f24109dfc4fb1a3730ed237e50183c6bb26b3]

url:    https://github.com/intel-lab-lkp/linux/commits/daire-mcnamara-microchip-com/PCI-microchip-Partition-address-translations/20221116-220208
base:   3c1f24109dfc4fb1a3730ed237e50183c6bb26b3
patch link:    https://lore.kernel.org/r/20221116135504.258687-5-daire.mcnamara%40microchip.com
patch subject: [PATCH v1 4/9] PCI: microchip: Clean up initialisation of interrupts
config: ia64-allyesconfig
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/95ed87687f4c8f6f3f0cd68380500fd764db9fbd
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review daire-mcnamara-microchip-com/PCI-microchip-Partition-address-translations/20221116-220208
        git checkout 95ed87687f4c8f6f3f0cd68380500fd764db9fbd
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/pci/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/pci/controller/pcie-microchip-host.c: In function 'mc_platform_init':
>> drivers/pci/controller/pcie-microchip-host.c:1103:23: warning: variable 'ctrl_base_addr' set but not used [-Wunused-but-set-variable]
    1103 |         void __iomem *ctrl_base_addr;
         |                       ^~~~~~~~~~~~~~


vim +/ctrl_base_addr +1103 drivers/pci/controller/pcie-microchip-host.c

  1096	
  1097	static int mc_platform_init(struct pci_config_window *cfg)
  1098	{
  1099		struct device *dev = cfg->parent;
  1100		struct platform_device *pdev = to_platform_device(dev);
  1101		struct mc_pcie *port;
  1102		void __iomem *bridge_base_addr;
> 1103		void __iomem *ctrl_base_addr;
  1104		int ret;
  1105	
  1106		port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
  1107		if (!port)
  1108			return -ENOMEM;
  1109		port->dev = dev;
  1110	
  1111		ret = mc_pcie_init_clks(dev);
  1112		if (ret) {
  1113			dev_err(dev, "failed to get clock resources, error %d\n", ret);
  1114			return -ENODEV;
  1115		}
  1116	
  1117		port->axi_base_addr = devm_platform_ioremap_resource(pdev, 1);
  1118		if (IS_ERR(port->axi_base_addr))
  1119			return PTR_ERR(port->axi_base_addr);
  1120	
  1121		mc_disable_interrupts(port);
  1122	
  1123		bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
  1124		ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
  1125	
  1126		port->msi.vector_phy = MSI_ADDR;
  1127		port->msi.num_vectors = MC_NUM_MSI_IRQS;
  1128	
  1129		/* Hardware doesn't setup MSI by default */
  1130		mc_pcie_enable_msi(port, cfg->win);
  1131	
  1132		/* Configure Address Translation Table 0 for PCIe config space */
  1133		mc_pcie_setup_window(bridge_base_addr, 0, cfg->res.start & 0xffffffff,
  1134				     cfg->res.start, resource_size(&cfg->res));
  1135	
  1136		ret = mc_pcie_setup_windows(pdev, port);
  1137		if (ret)
  1138			return ret;
  1139	
  1140		/* address translation is up; safe to enable interrupts */
  1141		return mc_init_interrupts(pdev, port);
  1142	}
  1143
kernel test robot Nov. 17, 2022, 6:28 p.m. UTC | #2 
Hi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on 3c1f24109dfc4fb1a3730ed237e50183c6bb26b3]

url:    https://github.com/intel-lab-lkp/linux/commits/daire-mcnamara-microchip-com/PCI-microchip-Partition-address-translations/20221116-220208
base:   3c1f24109dfc4fb1a3730ed237e50183c6bb26b3
patch link:    https://lore.kernel.org/r/20221116135504.258687-5-daire.mcnamara%40microchip.com
patch subject: [PATCH v1 4/9] PCI: microchip: Clean up initialisation of interrupts
config: x86_64-randconfig-a012
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/95ed87687f4c8f6f3f0cd68380500fd764db9fbd
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review daire-mcnamara-microchip-com/PCI-microchip-Partition-address-translations/20221116-220208
        git checkout 95ed87687f4c8f6f3f0cd68380500fd764db9fbd
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/pci/controller/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/pci/controller/pcie-microchip-host.c:1103:16: warning: variable 'ctrl_base_addr' set but not used [-Wunused-but-set-variable]
           void __iomem *ctrl_base_addr;
                         ^
   1 warning generated.


vim +/ctrl_base_addr +1103 drivers/pci/controller/pcie-microchip-host.c

  1096	
  1097	static int mc_platform_init(struct pci_config_window *cfg)
  1098	{
  1099		struct device *dev = cfg->parent;
  1100		struct platform_device *pdev = to_platform_device(dev);
  1101		struct mc_pcie *port;
  1102		void __iomem *bridge_base_addr;
> 1103		void __iomem *ctrl_base_addr;
  1104		int ret;
  1105	
  1106		port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
  1107		if (!port)
  1108			return -ENOMEM;
  1109		port->dev = dev;
  1110	
  1111		ret = mc_pcie_init_clks(dev);
  1112		if (ret) {
  1113			dev_err(dev, "failed to get clock resources, error %d\n", ret);
  1114			return -ENODEV;
  1115		}
  1116	
  1117		port->axi_base_addr = devm_platform_ioremap_resource(pdev, 1);
  1118		if (IS_ERR(port->axi_base_addr))
  1119			return PTR_ERR(port->axi_base_addr);
  1120	
  1121		mc_disable_interrupts(port);
  1122	
  1123		bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
  1124		ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
  1125	
  1126		port->msi.vector_phy = MSI_ADDR;
  1127		port->msi.num_vectors = MC_NUM_MSI_IRQS;
  1128	
  1129		/* Hardware doesn't setup MSI by default */
  1130		mc_pcie_enable_msi(port, cfg->win);
  1131	
  1132		/* Configure Address Translation Table 0 for PCIe config space */
  1133		mc_pcie_setup_window(bridge_base_addr, 0, cfg->res.start & 0xffffffff,
  1134				     cfg->res.start, resource_size(&cfg->res));
  1135	
  1136		ret = mc_pcie_setup_windows(pdev, port);
  1137		if (ret)
  1138			return ret;
  1139	
  1140		/* address translation is up; safe to enable interrupts */
  1141		return mc_init_interrupts(pdev, port);
  1142	}
  1143
Conor Dooley Nov. 23, 2022, 9:58 p.m. UTC | #3 
On Wed, Nov 16, 2022 at 01:54:59PM +0000, daire.mcnamara@microchip.com wrote:
> From: Daire McNamara <daire.mcnamara@microchip.com>
> 
> Refactor interrupt handling in _init() function into
> disable_interrupts(), init_interrupts(), clear_sec_errors() and clear
> ded_errors().  It was unwieldy and prone to bugs. Then clearly disable
> interrupts as soon as possible and only enable interrupts after address
> translation errors are setup to prevent spurious axi2pcie and pcie2axi
              ^^^^^^
Is that meant to read "after address translation is" or "after address
translation handling is"?

> translation errors being reported
> 
> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  drivers/pci/controller/pcie-microchip-host.c | 148 ++++++++++++-------
>  1 file changed, 92 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c
> index a81e6d25e347..ecd4d3f3e3d4 100644
> --- a/drivers/pci/controller/pcie-microchip-host.c
> +++ b/drivers/pci/controller/pcie-microchip-host.c
> @@ -986,39 +986,65 @@ static int mc_pcie_setup_windows(struct platform_device *pdev,
>  	return 0;
>  }
>  
> -static int mc_platform_init(struct pci_config_window *cfg)
> +static inline void mc_clear_secs(struct mc_pcie *port)
>  {
> -	struct device *dev = cfg->parent;
> -	struct platform_device *pdev = to_platform_device(dev);
> -	struct mc_pcie *port;
> -	void __iomem *bridge_base_addr;
> -	void __iomem *ctrl_base_addr;
> -	int ret;
> -	int irq;
> -	int i, intx_irq, msi_irq, event_irq;
> +	void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
> +
> +	writel_relaxed(GENMASK(15, 0), ctrl_base_addr + SEC_ERROR_INT);

I think it'd be nice if the GENMASK()s in this function and below were
#defined above somewhere.

> +	writel_relaxed(0, ctrl_base_addr + SEC_ERROR_EVENT_CNT);
> +}
> +
> +static inline void mc_clear_deds(struct mc_pcie *port)
> +{
> +	void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
> +
> +	writel_relaxed(GENMASK(15, 0), ctrl_base_addr + DED_ERROR_INT);
> +	writel_relaxed(0, ctrl_base_addr + DED_ERROR_EVENT_CNT);
> +}
> +
> +static void mc_disable_interrupts(struct mc_pcie *port)
> +{
> +	void __iomem *bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
> +	void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
>  	u32 val;
> -	int err;
>  
> -	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> -	if (!port)
> -		return -ENOMEM;
> -	port->dev = dev;
> +	/* ensure ecc bypass is enabled */
> +	val = ECC_CONTROL_TX_RAM_ECC_BYPASS | ECC_CONTROL_RX_RAM_ECC_BYPASS |
> +		ECC_CONTROL_PCIE2AXI_RAM_ECC_BYPASS | ECC_CONTROL_AXI2PCIE_RAM_ECC_BYPASS;

Pedantic maybe, but could we format this as:
		ECC_CONTROL_TX_RAM_ECC_BYPASS |
		ECC_CONTROL_RX_RAM_ECC_BYPASS |
		ECC_CONTROL_PCIE2AXI_RAM_ECC_BYPASS |
		ECC_CONTROL_AXI2PCIE_RAM_ECC_BYPASS;
And the same below for the PCIE_EVENT_FOO stuff, I think it'd make
things a bit easier on the eye.

Anyways, SEC and DED stuff that I usually see on startup are gone - at
least on the setup I have :)
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

> +	writel_relaxed(val, ctrl_base_addr + ECC_CONTROL);
>  
> -	ret = mc_pcie_init_clks(dev);
> -	if (ret) {
> -		dev_err(dev, "failed to get clock resources, error %d\n", ret);
> -		return -ENODEV;
> -	}
> +	/* disable sec errors and clear any outstanding */
> +	writel_relaxed(GENMASK(15, 0), ctrl_base_addr + SEC_ERROR_INT_MASK);
> +	mc_clear_secs(port);
>  
> -	port->axi_base_addr = devm_platform_ioremap_resource(pdev, 1);
> -	if (IS_ERR(port->axi_base_addr))
> -		return PTR_ERR(port->axi_base_addr);
> +	/* disable ded errors and clear any outstanding */
> +	writel_relaxed(GENMASK(15, 0), ctrl_base_addr + DED_ERROR_INT_MASK);
> +	mc_clear_deds(port);
>  
> -	bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
> -	ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
> +	/* disable local interrupts and clear any outstanding */
> +	writel_relaxed(0, bridge_base_addr + IMASK_LOCAL);
> +	writel_relaxed(GENMASK(31, 0), bridge_base_addr + ISTATUS_LOCAL);
> +	writel_relaxed(GENMASK(31, 0), bridge_base_addr + ISTATUS_MSI);
> +
> +	/* disable PCIe events and clear any outstanding */
> +	val = PCIE_EVENT_INT_L2_EXIT_INT | PCIE_EVENT_INT_HOTRST_EXIT_INT |
> +	      PCIE_EVENT_INT_DLUP_EXIT_INT | PCIE_EVENT_INT_L2_EXIT_INT_MASK |
> +	      PCIE_EVENT_INT_HOTRST_EXIT_INT_MASK |
> +	      PCIE_EVENT_INT_DLUP_EXIT_INT_MASK;
> +	writel_relaxed(val, ctrl_base_addr + PCIE_EVENT_INT);
> +
> +	/* disable host interrupts and clear any outstanding */
> +	writel_relaxed(0, bridge_base_addr + IMASK_HOST);
> +	writel_relaxed(GENMASK(31, 0), bridge_base_addr + ISTATUS_HOST);
> +}
> +
> +static int mc_init_interrupts(struct platform_device *pdev, struct mc_pcie *port)
> +{
> +	struct device *dev = &pdev->dev;
> +	int irq;
> +	int i, intx_irq, msi_irq, event_irq;
> +	int ret;
>  
> -	port->msi.vector_phy = MSI_ADDR;
> -	port->msi.num_vectors = MC_NUM_MSI_IRQS;
>  	ret = mc_pcie_init_irq_domains(port);
>  	if (ret) {
>  		dev_err(dev, "failed creating IRQ domains\n");
> @@ -1036,11 +1062,11 @@ static int mc_platform_init(struct pci_config_window *cfg)
>  			return -ENXIO;
>  		}
>  
> -		err = devm_request_irq(dev, event_irq, mc_event_handler,
> +		ret = devm_request_irq(dev, event_irq, mc_event_handler,
>  				       0, event_cause[i].sym, port);
> -		if (err) {
> +		if (ret) {
>  			dev_err(dev, "failed to request IRQ %d\n", event_irq);
> -			return err;
> +			return ret;
>  		}
>  	}
>  
> @@ -1065,44 +1091,54 @@ static int mc_platform_init(struct pci_config_window *cfg)
>  	/* Plug the main event chained handler */
>  	irq_set_chained_handler_and_data(irq, mc_handle_event, port);
>  
> -	/* Hardware doesn't setup MSI by default */
> -	mc_pcie_enable_msi(port, cfg->win);
> +	return 0;
> +}
>  
> -	val = readl_relaxed(bridge_base_addr + IMASK_LOCAL);
> -	val |= PM_MSI_INT_INTX_MASK;
> -	writel_relaxed(val, bridge_base_addr + IMASK_LOCAL);
> +static int mc_platform_init(struct pci_config_window *cfg)
> +{
> +	struct device *dev = cfg->parent;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct mc_pcie *port;
> +	void __iomem *bridge_base_addr;
> +	void __iomem *ctrl_base_addr;
> +	int ret;
>  
> -	writel_relaxed(val, ctrl_base_addr + ECC_CONTROL);
> +	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> +	if (!port)
> +		return -ENOMEM;
> +	port->dev = dev;
>  
> -	val = PCIE_EVENT_INT_L2_EXIT_INT |
> -	      PCIE_EVENT_INT_HOTRST_EXIT_INT |
> -	      PCIE_EVENT_INT_DLUP_EXIT_INT;
> -	writel_relaxed(val, ctrl_base_addr + PCIE_EVENT_INT);
> +	ret = mc_pcie_init_clks(dev);
> +	if (ret) {
> +		dev_err(dev, "failed to get clock resources, error %d\n", ret);
> +		return -ENODEV;
> +	}
>  
> -	val = SEC_ERROR_INT_TX_RAM_SEC_ERR_INT |
> -	      SEC_ERROR_INT_RX_RAM_SEC_ERR_INT |
> -	      SEC_ERROR_INT_PCIE2AXI_RAM_SEC_ERR_INT |
> -	      SEC_ERROR_INT_AXI2PCIE_RAM_SEC_ERR_INT;
> -	writel_relaxed(val, ctrl_base_addr + SEC_ERROR_INT);
> -	writel_relaxed(0, ctrl_base_addr + SEC_ERROR_INT_MASK);
> -	writel_relaxed(0, ctrl_base_addr + SEC_ERROR_EVENT_CNT);
> +	port->axi_base_addr = devm_platform_ioremap_resource(pdev, 1);
> +	if (IS_ERR(port->axi_base_addr))
> +		return PTR_ERR(port->axi_base_addr);
>  
> -	val = DED_ERROR_INT_TX_RAM_DED_ERR_INT |
> -	      DED_ERROR_INT_RX_RAM_DED_ERR_INT |
> -	      DED_ERROR_INT_PCIE2AXI_RAM_DED_ERR_INT |
> -	      DED_ERROR_INT_AXI2PCIE_RAM_DED_ERR_INT;
> -	writel_relaxed(val, ctrl_base_addr + DED_ERROR_INT);
> -	writel_relaxed(0, ctrl_base_addr + DED_ERROR_INT_MASK);
> -	writel_relaxed(0, ctrl_base_addr + DED_ERROR_EVENT_CNT);
> +	mc_disable_interrupts(port);
>  
> -	writel_relaxed(0, bridge_base_addr + IMASK_HOST);
> -	writel_relaxed(GENMASK(31, 0), bridge_base_addr + ISTATUS_HOST);
> +	bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
> +	ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
> +
> +	port->msi.vector_phy = MSI_ADDR;
> +	port->msi.num_vectors = MC_NUM_MSI_IRQS;
> +
> +	/* Hardware doesn't setup MSI by default */
> +	mc_pcie_enable_msi(port, cfg->win);
>  
>  	/* Configure Address Translation Table 0 for PCIe config space */
>  	mc_pcie_setup_window(bridge_base_addr, 0, cfg->res.start & 0xffffffff,
>  			     cfg->res.start, resource_size(&cfg->res));
>  
> -	return mc_pcie_setup_windows(pdev, port);
> +	ret = mc_pcie_setup_windows(pdev, port);
> +	if (ret)
> +		return ret;
> +
> +	/* address translation is up; safe to enable interrupts */
> +	return mc_init_interrupts(pdev, port);
>  }
>  
>  static const struct pci_ecam_ops mc_ecam_ops = {
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
diff mbox series

Patch

diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c
index a81e6d25e347..ecd4d3f3e3d4 100644
--- a/drivers/pci/controller/pcie-microchip-host.c
+++ b/drivers/pci/controller/pcie-microchip-host.c
@@ -986,39 +986,65 @@  static int mc_pcie_setup_windows(struct platform_device *pdev,
 	return 0;
 }
 
-static int mc_platform_init(struct pci_config_window *cfg)
+static inline void mc_clear_secs(struct mc_pcie *port)
 {
-	struct device *dev = cfg->parent;
-	struct platform_device *pdev = to_platform_device(dev);
-	struct mc_pcie *port;
-	void __iomem *bridge_base_addr;
-	void __iomem *ctrl_base_addr;
-	int ret;
-	int irq;
-	int i, intx_irq, msi_irq, event_irq;
+	void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
+
+	writel_relaxed(GENMASK(15, 0), ctrl_base_addr + SEC_ERROR_INT);
+	writel_relaxed(0, ctrl_base_addr + SEC_ERROR_EVENT_CNT);
+}
+
+static inline void mc_clear_deds(struct mc_pcie *port)
+{
+	void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
+
+	writel_relaxed(GENMASK(15, 0), ctrl_base_addr + DED_ERROR_INT);
+	writel_relaxed(0, ctrl_base_addr + DED_ERROR_EVENT_CNT);
+}
+
+static void mc_disable_interrupts(struct mc_pcie *port)
+{
+	void __iomem *bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
+	void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
 	u32 val;
-	int err;
 
-	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
-	if (!port)
-		return -ENOMEM;
-	port->dev = dev;
+	/* ensure ecc bypass is enabled */
+	val = ECC_CONTROL_TX_RAM_ECC_BYPASS | ECC_CONTROL_RX_RAM_ECC_BYPASS |
+		ECC_CONTROL_PCIE2AXI_RAM_ECC_BYPASS | ECC_CONTROL_AXI2PCIE_RAM_ECC_BYPASS;
+	writel_relaxed(val, ctrl_base_addr + ECC_CONTROL);
 
-	ret = mc_pcie_init_clks(dev);
-	if (ret) {
-		dev_err(dev, "failed to get clock resources, error %d\n", ret);
-		return -ENODEV;
-	}
+	/* disable sec errors and clear any outstanding */
+	writel_relaxed(GENMASK(15, 0), ctrl_base_addr + SEC_ERROR_INT_MASK);
+	mc_clear_secs(port);
 
-	port->axi_base_addr = devm_platform_ioremap_resource(pdev, 1);
-	if (IS_ERR(port->axi_base_addr))
-		return PTR_ERR(port->axi_base_addr);
+	/* disable ded errors and clear any outstanding */
+	writel_relaxed(GENMASK(15, 0), ctrl_base_addr + DED_ERROR_INT_MASK);
+	mc_clear_deds(port);
 
-	bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
-	ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
+	/* disable local interrupts and clear any outstanding */
+	writel_relaxed(0, bridge_base_addr + IMASK_LOCAL);
+	writel_relaxed(GENMASK(31, 0), bridge_base_addr + ISTATUS_LOCAL);
+	writel_relaxed(GENMASK(31, 0), bridge_base_addr + ISTATUS_MSI);
+
+	/* disable PCIe events and clear any outstanding */
+	val = PCIE_EVENT_INT_L2_EXIT_INT | PCIE_EVENT_INT_HOTRST_EXIT_INT |
+	      PCIE_EVENT_INT_DLUP_EXIT_INT | PCIE_EVENT_INT_L2_EXIT_INT_MASK |
+	      PCIE_EVENT_INT_HOTRST_EXIT_INT_MASK |
+	      PCIE_EVENT_INT_DLUP_EXIT_INT_MASK;
+	writel_relaxed(val, ctrl_base_addr + PCIE_EVENT_INT);
+
+	/* disable host interrupts and clear any outstanding */
+	writel_relaxed(0, bridge_base_addr + IMASK_HOST);
+	writel_relaxed(GENMASK(31, 0), bridge_base_addr + ISTATUS_HOST);
+}
+
+static int mc_init_interrupts(struct platform_device *pdev, struct mc_pcie *port)
+{
+	struct device *dev = &pdev->dev;
+	int irq;
+	int i, intx_irq, msi_irq, event_irq;
+	int ret;
 
-	port->msi.vector_phy = MSI_ADDR;
-	port->msi.num_vectors = MC_NUM_MSI_IRQS;
 	ret = mc_pcie_init_irq_domains(port);
 	if (ret) {
 		dev_err(dev, "failed creating IRQ domains\n");
@@ -1036,11 +1062,11 @@  static int mc_platform_init(struct pci_config_window *cfg)
 			return -ENXIO;
 		}
 
-		err = devm_request_irq(dev, event_irq, mc_event_handler,
+		ret = devm_request_irq(dev, event_irq, mc_event_handler,
 				       0, event_cause[i].sym, port);
-		if (err) {
+		if (ret) {
 			dev_err(dev, "failed to request IRQ %d\n", event_irq);
-			return err;
+			return ret;
 		}
 	}
 
@@ -1065,44 +1091,54 @@  static int mc_platform_init(struct pci_config_window *cfg)
 	/* Plug the main event chained handler */
 	irq_set_chained_handler_and_data(irq, mc_handle_event, port);
 
-	/* Hardware doesn't setup MSI by default */
-	mc_pcie_enable_msi(port, cfg->win);
+	return 0;
+}
 
-	val = readl_relaxed(bridge_base_addr + IMASK_LOCAL);
-	val |= PM_MSI_INT_INTX_MASK;
-	writel_relaxed(val, bridge_base_addr + IMASK_LOCAL);
+static int mc_platform_init(struct pci_config_window *cfg)
+{
+	struct device *dev = cfg->parent;
+	struct platform_device *pdev = to_platform_device(dev);
+	struct mc_pcie *port;
+	void __iomem *bridge_base_addr;
+	void __iomem *ctrl_base_addr;
+	int ret;
 
-	writel_relaxed(val, ctrl_base_addr + ECC_CONTROL);
+	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+	if (!port)
+		return -ENOMEM;
+	port->dev = dev;
 
-	val = PCIE_EVENT_INT_L2_EXIT_INT |
-	      PCIE_EVENT_INT_HOTRST_EXIT_INT |
-	      PCIE_EVENT_INT_DLUP_EXIT_INT;
-	writel_relaxed(val, ctrl_base_addr + PCIE_EVENT_INT);
+	ret = mc_pcie_init_clks(dev);
+	if (ret) {
+		dev_err(dev, "failed to get clock resources, error %d\n", ret);
+		return -ENODEV;
+	}
 
-	val = SEC_ERROR_INT_TX_RAM_SEC_ERR_INT |
-	      SEC_ERROR_INT_RX_RAM_SEC_ERR_INT |
-	      SEC_ERROR_INT_PCIE2AXI_RAM_SEC_ERR_INT |
-	      SEC_ERROR_INT_AXI2PCIE_RAM_SEC_ERR_INT;
-	writel_relaxed(val, ctrl_base_addr + SEC_ERROR_INT);
-	writel_relaxed(0, ctrl_base_addr + SEC_ERROR_INT_MASK);
-	writel_relaxed(0, ctrl_base_addr + SEC_ERROR_EVENT_CNT);
+	port->axi_base_addr = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(port->axi_base_addr))
+		return PTR_ERR(port->axi_base_addr);
 
-	val = DED_ERROR_INT_TX_RAM_DED_ERR_INT |
-	      DED_ERROR_INT_RX_RAM_DED_ERR_INT |
-	      DED_ERROR_INT_PCIE2AXI_RAM_DED_ERR_INT |
-	      DED_ERROR_INT_AXI2PCIE_RAM_DED_ERR_INT;
-	writel_relaxed(val, ctrl_base_addr + DED_ERROR_INT);
-	writel_relaxed(0, ctrl_base_addr + DED_ERROR_INT_MASK);
-	writel_relaxed(0, ctrl_base_addr + DED_ERROR_EVENT_CNT);
+	mc_disable_interrupts(port);
 
-	writel_relaxed(0, bridge_base_addr + IMASK_HOST);
-	writel_relaxed(GENMASK(31, 0), bridge_base_addr + ISTATUS_HOST);
+	bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
+	ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
+
+	port->msi.vector_phy = MSI_ADDR;
+	port->msi.num_vectors = MC_NUM_MSI_IRQS;
+
+	/* Hardware doesn't setup MSI by default */
+	mc_pcie_enable_msi(port, cfg->win);
 
 	/* Configure Address Translation Table 0 for PCIe config space */
 	mc_pcie_setup_window(bridge_base_addr, 0, cfg->res.start & 0xffffffff,
 			     cfg->res.start, resource_size(&cfg->res));
 
-	return mc_pcie_setup_windows(pdev, port);
+	ret = mc_pcie_setup_windows(pdev, port);
+	if (ret)
+		return ret;
+
+	/* address translation is up; safe to enable interrupts */
+	return mc_init_interrupts(pdev, port);
 }
 
 static const struct pci_ecam_ops mc_ecam_ops = {