lkml.org 
[lkml]   [2020]   [Mar]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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

\
 
 \ /
  Last update: 2020-03-26 15:39    [W:0.097 / U:12.976 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site