On Mon, Jun 08, 2020 at 06:48:58PM +0530, Bharat Kumar Gogada wrote:
> - Add support for Versal CPM as Root Port.
> - The Versal ACAP devices include CCIX-PCIe Module (CPM). The integrated
> block for CPM along with the integrated bridge can function
> as PCIe Root Port.
> - Bridge error and legacy interrupts in Versal CPM are handled using
> Versal CPM specific interrupt line.
> +static irqreturn_t xilinx_cpm_pcie_intr_handler(int irq, void *dev_id)
> +{
> + struct xilinx_cpm_pcie_port *port = dev_id;
> + struct device *dev = port->dev;
> + struct irq_data *d;
> +
> + d = irq_domain_get_irq_data(port->cpm_domain, irq);
> +
> + switch (d->hwirq) {
> + case XILINX_CPM_PCIE_INTR_CORRECTABLE:
> + case XILINX_CPM_PCIE_INTR_NONFATAL:
> + case XILINX_CPM_PCIE_INTR_FATAL:
> + cpm_pcie_clear_err_interrupts(port);
> + fallthrough;
> +
> + default:
> + if (intr_cause[d->hwirq].str)
> + dev_warn(dev, "%s\n", intr_cause[d->hwirq].str);
> + else
> + dev_warn(dev, "Unknown interrupt\n");
Maybe include d->hwirq in the "Unknown interrupt" message?
I assume if we take the default case, there's no need to clear the
interrupt?
> + }
> +
> + return IRQ_HANDLED;
> +}
> +static int xilinx_cpm_setup_irq(struct xilinx_cpm_pcie_port *port)
> +{
> + struct device *dev = port->dev;
> + struct platform_device *pdev = to_platform_device(dev);
> + int i, irq;
> +
> + port->irq = platform_get_irq(pdev, 0);
> + if (port->irq < 0) {
> + dev_err(dev, "Unable to find misc IRQ line\n");
platform_get_irq() already prints an error message; you probably don't
need another.
> + return port->irq;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(intr_cause); i++) {
> + int err;
> +
> + if (!intr_cause[i].str)
> + continue;
> +
> + irq = irq_create_mapping(port->cpm_domain, i);
> + if (WARN_ON(irq <= 0))
I'm not a huge fan of WARN_ON() inside "if" statements, but ... OK.
Why do these all need to be WARN_ON() instead of dev_warn()?
> + return -ENXIO;
> +
> + err = devm_request_irq(dev, irq, xilinx_cpm_pcie_intr_handler,
> + 0, intr_cause[i].sym, port);
> + if (WARN_ON(err))
> + return err;
> + }
> +
> + port->intx_irq = irq_create_mapping(port->cpm_domain,
> + XILINX_CPM_PCIE_INTR_INTX);
> + if (WARN_ON(port->intx_irq <= 0))
> + return -ENXIO;
> +
> + /* Plug the INTx chained handler */
> + irq_set_chained_handler_and_data(port->intx_irq,
> + xilinx_cpm_pcie_intx_flow, port);
