DWC PCIe eDMA irqs

DWC PCIe eDMA irqs

[Niklas Cassel]

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

Re: DWC PCIe eDMA irqs

[Manivannan Sadhasivam]

On Thu, Sep 04, 2025 at 02:48:50PM GMT, Niklas Cassel wrote:
> 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.
> 

Agree.

> 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()
> 

I think we could also get rid of dw_pcie_edma_irq_verify(), since kernel is not
a devicetree validator and it should just trust DT (atleast for these kind of
resources). The binding should make sure that the DTBs are correct.

Moreover, dw_edma_irq_request() will look for only 2 combinations:

1. Single IRQ for all channels
2. Separate IRQ per channel

So if the platform doesn't supply enough IRQs for each channel,
dw_edma_irq_request() will fail anyway. Precisely, the irq_vector() callback.

But for removing dw_pcie_edma_irq_verify() and dw_edma_chip::nr_irqs, we also
need corresponding change in dw_edma_irq_request(). So I'm not asking you to
implement it. It is more of a note to myself or someone interested :)

> 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.
> 

Ack, for both changes.

- Mani

-- 
மணிவண்ணன் சதாசிவம்