DWC PCIe eDMA irqs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Mani,


Looking at dw_pcie_edma_irq_verify()
https://github.com/torvalds/linux/blob/v6.17-rc4/drivers/pci/controller/dwc/pcie-designware.c#L1048-L1049

We can see that it does an early return if "pci->edma.nr_irqs == 1".

I.e. if the glue driver has set "pci->edma.nr_irqs == 1", we never even
bother to fetch "dma"/"dmaX" from device tree.

This suggests that we support a single IRQ for all eDMA channels,
and since we don't even parse anything for DT, it suggests that this
IRQ could be the same IRQ as the "sys IRQ" for the PCI controller.
(E.g. tegra has a single IRQ for the PCI controller and the eDMA.)



However, looking at dw_pcie_edma_irq_vector(), which will be called by
dw_edma_probe():
https://github.com/torvalds/linux/blob/v6.17-rc4/drivers/pci/controller/dwc/pcie-designware.c#L945-L947

We can see that we need either "dma" or "dmaX" in DT.

Which suggests that the code currently only supports either a
separate IRQ for each eDMA per channel, or a combined IRQ for
each eDMA channel, but the combined IRQ cannot be the same as
the "sys IRQ".


This seems inconsistent to me.

Since dw_pcie_edma_irq_vector() requires either "dma" or "dmaX" in DT,
I don't see why we shouldn't have the same requirement for
dw_pcie_edma_irq_verify()

Looking at pcie-ep@1c08000 in arch/arm64/boot/dts/qcom/sm8450.dtsi,
it does also specify "dma" IRQ in DT.


So, should I submit a patch that does:


diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 89aad5a08928..c7a2cf5e886f 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -1045,9 +1045,7 @@ static int dw_pcie_edma_irq_verify(struct dw_pcie *pci)
 	char name[15];
 	int ret;
 
-	if (pci->edma.nr_irqs == 1)
-		return 0;
-	else if (pci->edma.nr_irqs > 1)
+	if (pci->edma.nr_irqs > 1)
 		return pci->edma.nr_irqs != ch_cnt ? -EINVAL : 0;
 
 	ret = platform_get_irq_byname_optional(pdev, "dma");
diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index bf7c6ac0f3e3..ad98598bb522 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -874,7 +874,6 @@ static int qcom_pcie_ep_probe(struct platform_device *pdev)
 	pcie_ep->pci.dev = dev;
 	pcie_ep->pci.ops = &pci_ops;
 	pcie_ep->pci.ep.ops = &pci_ep_ops;
-	pcie_ep->pci.edma.nr_irqs = 1;
 
 	pcie_ep->cfg = of_device_get_match_data(dev);
 	if (pcie_ep->cfg && pcie_ep->cfg->hdma_support) {


Because, since sm8450 (and all other platforms) currently must have
either "dma" or "dmaX" in DT, all currently supported platform should
still work.


If sm8450 case, this code:
https://github.com/torvalds/linux/blob/v6.17-rc4/drivers/pci/controller/dwc/pcie-designware.c#L1053-L1057

should set pcie_ep->pci.edma.nr_irqs = 1, because sdm8450 has "dma" in DT.



Kind regards,
Niklas




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux