Hi:
On 2016/5/27 20:25, Marc Zyngier Wrote:
[+Lorenzo]
On 20/05/16 11:29, Shawn Lin wrote:
RK3399 has a PCIe controller which can be used as Root Complex.
This driver supports a PCIe controller as Root Complex mode.
[....]
+static int rockchip_pcie_init_irq_domain(struct rockchip_pcie_port *pp)
+{
+ struct device *dev = pp->dev;
+ struct device_node *node = dev->of_node;
+ struct device_node *pcie_intc_node = of_get_next_child(node, NULL);
That's really ugly, as it depends on the layout of your DT.
+
+ if (!pcie_intc_node) {
+ dev_err(dev, "No PCIe Intc node found\n");
+ return PTR_ERR(pcie_intc_node);
+ }
+ pp->irq_domain = irq_domain_add_linear(pcie_intc_node, 4,
+ &intx_domain_ops, pp);
Why can't you just register your host controller as the interrupt
controller? You don't need an intermediate node for that.
OK, the child node is really no need here, we will use the host
controller as interrupt controller next patch. Thanks!
+ if (!pp->irq_domain) {
+ dev_err(dev, "Failed to get a INTx IRQ domain\n");
+ return PTR_ERR(pp->irq_domain);
+ }
+
+ return 0;
+}
+
+static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg)
+{
+ struct rockchip_pcie_port *pp = arg;
+ u32 reg;
+ u32 sub_reg;
+
+ reg = pcie_read(pp, PCIE_CLIENT_INT_STATUS);
+ if (reg & LOC_INT) {
+ dev_dbg(pp->dev, "local interrupt recived\n");
+ sub_reg = pcie_read(pp, PCIE_CORE_INT_STATUS);
+ if (sub_reg & PRFPE)
+ dev_dbg(pp->dev, "Parity error detected while reading from the PNP Receive FIFO RAM\n");
+
+ if (sub_reg & CRFPE)
+ dev_dbg(pp->dev, "Parity error detected while reading from the Completion Receive FIFO RAM\n");
+
+ if (sub_reg & RRPE)
+ dev_dbg(pp->dev, "Parity error detected while reading from Replay Buffer RAM\n");
+
+ if (sub_reg & PRFO)
+ dev_dbg(pp->dev, "Overflow occurred in the PNP Receive FIFO\n");
+
+ if (sub_reg & CRFO)
+ dev_dbg(pp->dev, "Overflow occurred in the Completion Receive FIFO\n");
+
+ if (sub_reg & RT)
+ dev_dbg(pp->dev, "Replay timer timed out\n");
+
+ if (sub_reg & RTR)
+ dev_dbg(pp->dev, "Replay timer rolled over after 4 transmissions of the same TLP\n");
+
+ if (sub_reg & PE)
+ dev_dbg(pp->dev, "Phy error detected on receive side\n");
+
+ if (sub_reg & MTR)
+ dev_dbg(pp->dev, "Malformed TLP received from the link\n");
+
+ if (sub_reg & UCR)
+ dev_dbg(pp->dev, "Malformed TLP received from the link\n");
+
+ if (sub_reg & FCE)
+ dev_dbg(pp->dev, "An error was observed in the flow control advertisements from the other side\n");
+
+ if (sub_reg & CT)
+ dev_dbg(pp->dev, "A request timed out waiting for completion\n");
+
+ if (sub_reg & UTC)
+ dev_dbg(pp->dev, "Unmapped TC error\n");
+
+ if (sub_reg & MMVC)
+ dev_dbg(pp->dev, "MSI mask register changes\n");
+
+ pcie_write(pp, sub_reg, PCIE_CORE_INT_STATUS);
+ }
+
+ pcie_write(pp, reg, PCIE_CLIENT_INT_STATUS);
+
+ return IRQ_HANDLED;
+}
[....]
+static irqreturn_t rockchip_pcie_legacy_int_handler(int irq, void *arg)
+{
+ struct rockchip_pcie_port *pp = arg;
+ u32 reg;
+
+ reg = pcie_read(pp, PCIE_CLIENT_INT_STATUS);
+ reg = (reg & ROCKCHIP_PCIE_RPIFR1_INTR_MASK) >>
+ ROCKCHIP_PCIE_RPIFR1_INTR_SHIFT;
+ generic_handle_irq(irq_find_mapping(pp->irq_domain, ffs(reg)));
NAK. What you have here is a chained interrupt controller, please
implement it as such.
Your mean is use handle_nested_irq instead of generic_handle_irq here?
But, I found all other pci host controller drivers use this api.
+
+ pcie_write(pp, reg, PCIE_CLIENT_INT_STATUS);
+ return IRQ_HANDLED;
+}
+
[...]
+static struct platform_driver rockchip_pcie_driver = {
+ .driver = {
+ .name = "rockchip-pcie",
+ .of_match_table = rockchip_pcie_of_match,
+ .suppress_bind_attrs = true,
+ },
+ .probe = rockchip_pcie_probe,
+ .remove = rockchip_pcie_remove,
+};
+module_platform_driver(rockchip_pcie_driver);
+
+MODULE_AUTHOR("Rockchip Inc");
+MODULE_DESCRIPTION("Rockchip AXI PCIe driver");
+MODULE_LICENSE("GPL v2");
This definitely requires some rework for both the interrupt and MSI
parts. I'll leave Lorenzo to comment on the PCI side of things.
Thanks,
M.
Best Regards,
Li
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
