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 -- மணிவண்ணன் சதாசிவம்
