Messages in this thread | | | From | Andy Shevchenko <> | Date | Thu, 26 Mar 2020 16:37:15 +0200 | Subject | Re: [PATCH v5 2/6] PCI: iproc: Add INTx support with better modeling |
| |
On Thu, Mar 26, 2020 at 8:49 AM Srinath Mannam <srinath.mannam@broadcom.com> wrote: > > From: Ray Jui <ray.jui@broadcom.com> > > Add PCIe legacy interrupt INTx support to the iProc PCIe driver by > modeling it with its own IRQ domain. All 4 interrupts INTA, INTB, INTC, > INTD share the same interrupt line connected to the GIC in the system, > while the status of each INTx can be obtained through the INTX CSR > register.
... > + val &= ~(BIT(irqd_to_hwirq(d)));
Too many parentheses.
...
> + val |= (BIT(irqd_to_hwirq(d)));
Ditto.
...
> + /* go through INTx A, B, C, D until all interrupts are handled */ > + do { > + status = iproc_pcie_read_reg(pcie, IPROC_PCIE_INTX_CSR); > + for_each_set_bit(bit, &status, PCI_NUM_INTX) { > + virq = irq_find_mapping(pcie->irq_domain, bit); > + if (virq) > + generic_handle_irq(virq);
> + else > + dev_err(dev, "unexpected INTx%u\n", bit);
Any guarantee it will be no storm of undesired messages here?
> + }
> + } while ((status & SYS_RC_INTX_MASK) != 0);
' != 0' part is not needed.
If there an interrupt storm the handler will never end, right? Is it the idea by design?
...
> + node = of_get_compatible_child(dev->of_node, "brcm,iproc-intc"); > + if (node) > + pcie->irq = of_irq_get(node, 0); > + > + if (!node || pcie->irq <= 0) > + return 0;
Perhaps node = of_get_compatible_child(dev->of_node, "brcm,iproc-intc"); if (!node) return 0;
pcie->irq = of_irq_get(node, 0); if (pcie->irq <= 0) return 0; ?
-- With Best Regards, Andy Shevchenko
| |