On Thu, 18 Jun 2020 09:38:09 +0100, Kunihiko Hayashi <hayashi.kunihiko@xxxxxxxxxxxxx> wrote: > > The misc interrupts consisting of PME, AER, and Link event, is handled > by INTx handler, however, these interrupts should be also handled by > MSI handler. > > This adds the function uniphier_pcie_misc_isr() that handles misc > interrupts, which is called from both INTx and MSI handlers. > This function detects PME and AER interrupts with the status register, > and invoke PME and AER drivers related to MSI. > > And this sets the mask for misc interrupts from INTx if MSI is enabled > and sets the mask for misc interrupts from MSI if MSI is disabled. > > Cc: Marc Zyngier <maz@xxxxxxxxxx> > Cc: Jingoo Han <jingoohan1@xxxxxxxxx> > Cc: Gustavo Pimentel <gustavo.pimentel@xxxxxxxxxxxx> > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@xxxxxxxxxxxxx> > --- > drivers/pci/controller/dwc/pcie-uniphier.c | 57 ++++++++++++++++++++++++------ > 1 file changed, 46 insertions(+), 11 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c > index a5401a0..5ce2479 100644 > --- a/drivers/pci/controller/dwc/pcie-uniphier.c > +++ b/drivers/pci/controller/dwc/pcie-uniphier.c > @@ -44,7 +44,9 @@ > #define PCL_SYS_AUX_PWR_DET BIT(8) > > #define PCL_RCV_INT 0x8108 > +#define PCL_RCV_INT_ALL_INT_MASK GENMASK(28, 25) > #define PCL_RCV_INT_ALL_ENABLE GENMASK(20, 17) > +#define PCL_RCV_INT_ALL_MSI_MASK GENMASK(12, 9) > #define PCL_CFG_BW_MGT_STATUS BIT(4) > #define PCL_CFG_LINK_AUTO_BW_STATUS BIT(3) > #define PCL_CFG_AER_RC_ERR_MSI_STATUS BIT(2) > @@ -167,7 +169,15 @@ static void uniphier_pcie_stop_link(struct dw_pcie *pci) > > static void uniphier_pcie_irq_enable(struct uniphier_pcie_priv *priv) > { > - writel(PCL_RCV_INT_ALL_ENABLE, priv->base + PCL_RCV_INT); > + u32 val; > + > + val = PCL_RCV_INT_ALL_ENABLE; > + if (pci_msi_enabled()) > + val |= PCL_RCV_INT_ALL_INT_MASK; > + else > + val |= PCL_RCV_INT_ALL_MSI_MASK; Does this affect endpoints? Or just the RC itself? > + > + writel(val, priv->base + PCL_RCV_INT); > writel(PCL_RCV_INTX_ALL_ENABLE, priv->base + PCL_RCV_INTX); > } > > @@ -231,32 +241,56 @@ static const struct irq_domain_ops uniphier_intx_domain_ops = { > .map = uniphier_pcie_intx_map, > }; > > -static void uniphier_pcie_irq_handler(struct irq_desc *desc) > +static void uniphier_pcie_misc_isr(struct pcie_port *pp, bool is_msi) > { > - struct pcie_port *pp = irq_desc_get_handler_data(desc); > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci); > - struct irq_chip *chip = irq_desc_get_chip(desc); > - unsigned long reg; > - u32 val, bit, virq; > + u32 val, virq; > > - /* INT for debug */ > val = readl(priv->base + PCL_RCV_INT); > > if (val & PCL_CFG_BW_MGT_STATUS) > dev_dbg(pci->dev, "Link Bandwidth Management Event\n"); > + > if (val & PCL_CFG_LINK_AUTO_BW_STATUS) > dev_dbg(pci->dev, "Link Autonomous Bandwidth Event\n"); > - if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS) > - dev_dbg(pci->dev, "Root Error\n"); > - if (val & PCL_CFG_PME_MSI_STATUS) > - dev_dbg(pci->dev, "PME Interrupt\n"); > + > + if (is_msi) { > + if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS) > + dev_dbg(pci->dev, "Root Error Status\n"); > + > + if (val & PCL_CFG_PME_MSI_STATUS) > + dev_dbg(pci->dev, "PME Interrupt\n"); > + > + if (val & (PCL_CFG_AER_RC_ERR_MSI_STATUS | > + PCL_CFG_PME_MSI_STATUS)) { > + virq = irq_linear_revmap(pp->irq_domain, 0); > + generic_handle_irq(virq); > + } > + } Please have two handlers: one for interrupts that are from the RC, another for interrupts coming from the endpoints. M. -- Without deviation from the norm, progress is not possible.