Re: [PATCH v7 06/10] PCI: dwc: Implement endpoint DMA channel delegation

[Koichiro Den]

On Tue, Aug 25, 2026 at 11:36:54AM +0900, Koichiro Den wrote:
> On Thu, Aug 13, 2026 at 02:09:26PM -0500, Frank Li wrote:
> > On Thu, Aug 13, 2026 at 03:37:53PM +0900, Koichiro Den wrote:
> > > Implement the EPC DMA channel delegation operations for DesignWare
> > > endpoint controllers. Verify that the DMA engine channel reserved by the
> > > endpoint function belongs to the integrated DesignWare eDMA instance,
> > > then hand its interrupt routing to the host.
> > >
> > > The DWC eDMA/HDMA engine generates DMA requests with a programmable
> > > requester function number. For delegated channels, the host-side
> > > dw-edma-pcie instance bound to the exposed DMA function programs its own
> > > PCI_FUNC() into the per-channel requester field; the endpoint-side chip
> > > func_no does
> > > not participate in that handoff.
> > >
> > > Reject VF requests because the RC-programmable DWC eDMA/HDMA register
> > > window is assigned to a PF BAR only.
> > >
> > > Reclaim returns interrupt routing to endpoint ownership. Propagate the
> > > EPC quiesce request so bind failure paths can reclaim unexposed channels
> > > without quiescing the DMA engine.
> > >
> > > Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> > > ---
> > > Changes in v7:
> > >   - Delegate the DMA engine channel already reserved by the endpoint
> > >     function, matching the API and helper changes in patches 4 and 5.
> > >
> > >  .../pci/controller/dwc/pcie-designware-ep.c   | 42 +++++++++++++++++++
> > >  1 file changed, 42 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > index 0b915824963a..a74d3896e436 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > @@ -858,6 +858,18 @@ dw_pcie_ep_find_bar_rsvd_region(struct dw_pcie_ep *ep,
> > >  	return NULL;
> > >  }
> > >
> > > +static int dw_pcie_ep_check_edma_vfunc(u8 vfunc_no)
> > > +{
> > > +	/*
> > > +	 * The DWC endpoint databook says it is not possible to assign the
> > > +	 * DMA/HDMA registers to any Virtual Function.
> > > +	 */
> > > +	if (vfunc_no)
> > > +		return -EOPNOTSUPP;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  static int
> > >  dw_pcie_ep_get_aux_resources_count(struct pci_epc *epc, u8 func_no,
> > >  				   u8 vfunc_no)
> > > @@ -933,6 +945,34 @@ dw_pcie_ep_get_aux_resources(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > >  	return 0;
> > >  }
> > >
> > > +static int dw_pcie_ep_delegate_dma_chan(struct pci_epc *epc, u8 func_no,
> > > +					u8 vfunc_no, struct dma_chan *chan)
> > > +{
> > > +	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > > +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > +	struct dw_edma_chip *edma = &pci->edma;
> > > +	int ret;
> > > +
> > > +	ret = dw_pcie_ep_check_edma_vfunc(vfunc_no);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	if (!edma->dw)
> > > +		return -ENODEV;
> > > +
> > > +	if (!chan || chan->device->dev != edma->dev)
> > > +		return -EINVAL;
> > > +
> > > +	return dw_edma_delegate_chan(chan);
> > > +}
> > > +
> > > +static void dw_pcie_ep_reclaim_dma_chan(struct pci_epc *epc, u8 func_no,
> > > +					u8 vfunc_no, struct dma_chan *chan,
> > > +					bool quiesce)
> > > +{
> > > +	dw_edma_reclaim_chan(chan, quiesce);
> > > +}
> > > +
> > 
> > After use fixed chan_ids, needn't these functins. EPF driver already
> > find expected dma channel and config it remote only by standard API.
> 
> (I've been somewhat holding this thread, sorry. I'm still unsure whether the
> separate PCI DMA EPF idea will survive, or whether making it much smaller and
> initially limiting it to a test endpoint for dw-edma-pcie would make it
> acceptable. AFAICT, Mani has not answered that point yet. Still, I think I
> should reply here in case the idea survives in some form and v8 becomes much
> simpler.)
> 
> I agree fixed chan_id removes the private channel lookup. In v7, the EPF already
> reserves the channels to delegate through dma_request_channel(), so Patch 4 may
> look like a leftover. Its remaining benefit is to hide dw-edma-specific
> interrupt-routing semantics from the generic EPF.

Let me correct myself.

On second thought, that "benefit" is really superficial. If we ever want a
generic abstraction for that purpose, it should belong in dmaengine, not PCI
EPC.

Now that the EPF reserves channels through standard dma_request_channel(), it
owns them until dma_release_channel(). Going thought PCI EPC just to change
their interrupt routing (in the dw-edma case) is odd.

> 
> The EPF could instead do something like this in pci_epf_dma_claim_channel()
> after extending dw-edma to accept the setting:
> 
>   config.peripheral_config = ... DW_EDMA_CH_IRQ_REMOTE ...;
>   dmaengine_slave_config(chan, &config);
> 
> That would make the EPF depend on dw-edma-specific interrupt routing. However,
> if the goal is to make the PCI DMA EPF as small as possible and accept that it
> initially only serves the dw-edma-pcie path, I think that is fine. I think
> that's your point, but if not, please correct me.

So even in the vNTB-embedded design, your suggestion to use prepheral_config
fits well. I've now implemented and tested it there, and it works. I'm preparing
v2 on that basis and hope to send it soon.

Best regards,
Koichiro

> 
> Thanks for reviewing.
> Koichiro
> 
> > 
> > Frank
> > 
> > >  static const struct pci_epc_ops epc_ops = {
> > >  	.write_header		= dw_pcie_ep_write_header,
> > >  	.set_bar		= dw_pcie_ep_set_bar,
> > > @@ -950,6 +990,8 @@ static const struct pci_epc_ops epc_ops = {
> > >  	.get_features		= dw_pcie_ep_get_features,
> > >  	.get_aux_resources_count	= dw_pcie_ep_get_aux_resources_count,
> > >  	.get_aux_resources	= dw_pcie_ep_get_aux_resources,
> > > +	.delegate_dma_chan	= dw_pcie_ep_delegate_dma_chan,
> > > +	.reclaim_dma_chan	= dw_pcie_ep_reclaim_dma_chan,
> > >  };
> > >
> > >  /**
> > > --
> > > 2.51.0
> > >
>