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
