PCI: microchip: Clean up initialisation of interrupts

Refactor interrupt handling in _init() function into
disable_interrupts(), init_interrupts(), clear_sec_errors() and clear
ded_errors() because current code is unwieldy and prone to bugs.

Disable interrupts as soon as possible and only enable interrupts after
address translation is setup to prevent spurious axi2pcie and pcie2axi
translation errors being reported.

Link: https://lore.kernel.org/r/20230728131401.1615724-6-daire.mcnamara@microchip.com
Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c
index a81e6d2..7b212ee 100644 (file)
--- a/drivers/pci/controller/pcie-microchip-host.c
+++ b/drivers/pci/controller/pcie-microchip-host.c
@@ -112,6 +112,7 @@
 #define  SEC_ERROR_INT_RX_RAM_SEC_ERR_INT      GENMASK(7, 4)
 #define  SEC_ERROR_INT_PCIE2AXI_RAM_SEC_ERR_INT        GENMASK(11, 8)
 #define  SEC_ERROR_INT_AXI2PCIE_RAM_SEC_ERR_INT        GENMASK(15, 12)
+#define  SEC_ERROR_INT_ALL_RAM_SEC_ERR_INT     GENMASK(15, 0)
 #define  NUM_SEC_ERROR_INTS                    (4)
 #define SEC_ERROR_INT_MASK                     0x2c
 #define DED_ERROR_INT                          0x30
@@ -119,6 +120,7 @@
 #define  DED_ERROR_INT_RX_RAM_DED_ERR_INT      GENMASK(7, 4)
 #define  DED_ERROR_INT_PCIE2AXI_RAM_DED_ERR_INT        GENMASK(11, 8)
 #define  DED_ERROR_INT_AXI2PCIE_RAM_DED_ERR_INT        GENMASK(15, 12)
+#define  DED_ERROR_INT_ALL_RAM_DED_ERR_INT     GENMASK(15, 0)
 #define  NUM_DED_ERROR_INTS                    (4)
 #define DED_ERROR_INT_MASK                     0x34
 #define ECC_CONTROL                            0x38
@@ -986,39 +988,73 @@ 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(SEC_ERROR_INT_ALL_RAM_SEC_ERR_INT, 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(DED_ERROR_INT_ALL_RAM_DED_ERR_INT, 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(SEC_ERROR_INT_ALL_RAM_SEC_ERR_INT, 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(DED_ERROR_INT_ALL_RAM_DED_ERR_INT, 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 +1072,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 +1101,52 @@ 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;
+       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;
+
+       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 = {