On 2020-06-04 10:43, Kunihiko Hayashi wrote:
[...]
-static void uniphier_pcie_irq_handler(struct irq_desc *desc)
+static void uniphier_pcie_misc_isr(struct pcie_port *pp)
{
- 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 (pci_msi_enabled()) {
This checks whether the kernel supports MSIs. Not that they are
enabled in your controller. Is that really what you want to do?
The below two status bits are valid when the interrupt for MSI is
asserted.
That is, pci_msi_enabled() is wrong.
I'll modify the function to check the two bits only if this function is
called from MSI handler.
+ if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS) {
+ dev_dbg(pci->dev, "Root Error Status\n");
+ virq = irq_linear_revmap(pp->irq_domain, 0);
+ generic_handle_irq(virq);
+ }
+
+ if (val & PCL_CFG_PME_MSI_STATUS) {
+ dev_dbg(pci->dev, "PME Interrupt\n");
+ virq = irq_linear_revmap(pp->irq_domain, 0);
+ generic_handle_irq(virq);
+ }
These two cases do the exact same thing, calling the same interrupt.
What is the point of dealing with them independently?
Both PME and AER are asserted from MSI-0, and each handler checks its
own
status bit in the PCIe register (aer_irq() in pcie/aer.c and
pcie_pme_irq()
in pcie/pme.c).
So I think this handler calls generic_handle_irq() for the same MSI-0.
So what is wrong with
if (val & (PCL_CFG_AER_RC_ERR_MSI_STATUS |
PCL_CFG_PME_MSI_STATUS)) {
// handle interrupt
}
?
If you have two handlers for the same interrupt, this is a shared
interrupt and each handler will be called in turn.
M.
--
Jazz is not dead. It just smells funny...
