Message-ID: <zqcu3awadvqbtil3vudcmgjyjpku7divrhqyox72k43nfzcoo7@hflaengfjy27>
Date: Fri, 30 Jan 2026 16:16:11 +0900
From: Koichiro Den <den@...inux.co.jp>
To: Frank Li <Frank.li@....com>
Cc: mani@...nel.org, vkoul@...nel.org, jingoohan1@...il.com, 
	lpieralisi@...nel.org, kwilczynski@...nel.org, robh@...nel.org, bhelgaas@...gle.com, 
	dmaengine@...r.kernel.org, linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 7/7] PCI: dwc: Add helper to query integrated dw-edma
 linked-list region

On Wed, Jan 28, 2026 at 10:42:29PM -0500, Frank Li wrote:
> On Thu, Jan 29, 2026 at 01:25:30AM +0900, Koichiro Den wrote:
> > On Wed, Jan 28, 2026 at 10:48:04AM -0500, Frank Li wrote:
> > > On Tue, Jan 27, 2026 at 12:34:20PM +0900, Koichiro Den wrote:
> > > > Some DesignWare PCIe endpoint controllers integrate a DesignWare eDMA
> > > > instance and allocate per-channel linked-list (LL) regions. Remote eDMA
> > > > providers may need to expose those LL regions to the host so it can
> > > > build descriptors against the remote view.
> > > >
> > > > Export dwc_pcie_edma_get_ll_region() to allow higher-level code to query
> > > > the LL region (base/size) for a given EPC, transfer direction
> > > > (DMA_DEV_TO_MEM / DMA_MEM_TO_DEV) and hardware channel identifier. The
> > > > helper maps the request to the appropriate read/write LL region
> > > > depending on whether the integrated eDMA is the local or the remote
> > > > view.
> > > >
> > > > Signed-off-by: Koichiro Den <den@...inux.co.jp>
> > > > ---
> > > >  drivers/pci/controller/dwc/pcie-designware.c | 45 ++++++++++++++++++++
> > > >  include/linux/pcie-dwc-edma.h                | 33 ++++++++++++++
> > > >  2 files changed, 78 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > > index bbaeecce199a..e8617873e832 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > > @@ -1369,3 +1369,48 @@ int dwc_pcie_edma_get_reg_window(struct pci_epc *epc, phys_addr_t *phys,
> > > >  	return 0;
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(dwc_pcie_edma_get_reg_window);
> > > > +
> > > > +int dwc_pcie_edma_get_ll_region(struct pci_epc *epc,
> > > > +				enum dma_transfer_direction dir, int hw_id,
> > > > +				struct dw_edma_region *region)
> > >
> > > These APIs need an user, A simple method is that add one test case in
> > > pci-epf-test.
> >
> > Thanks for the feedback.
> >
> > I'm unsure whether adding DesignWare-specific coverage to pci-epf-test
> > would be acceptable. I'd appreciate your guidance on the preferred
> > approach.
> >
> > One possible direction I had in mind is to keep pci-epf-test.c generic and
> 
> Add a EPC/EPF API, so dynatmic check if support DMA region or other feature.

Thank you for the comment.

Ok, I have drafted an API ([1] below).

One thing I'm unsure about is how far the pci-epf-test validation should
go. Since the API (pci_epc_get_remote_resources() in [1]) is generic and
only returns a list of (type, phys_addr, size, and type-specific fields), a
fully end-to-end validation (i.e. "are these resources actually usable from
the host?") would require controller-specific (DW-eDMA-specific) knowledge
and also depends on how those resources are exposed (e.g. for a eDMA LL
region, it may be sufficient to inform the host of EP-local address, while
the eDMA register block would need to be fully exposed via BAR mapping).

Do you think a "smoke test" would be sufficient for now (e.g. testing
whether the new API returns a sane / well-formed list of resources), or are
you expecting the test to actually program the DMA eingine from the host?
Personally, I think the former would suffice, since the latter would be
very close to re-implementing the real user (e.g. ntb_dw_edma [2]) itself.

[1]:

diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index c021c7af175f..4cb5e634daba 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -61,6 +61,35 @@ struct pci_epc_map {
        void __iomem    *virt_addr;
 };

+/**
+ * enum pci_epc_remote_resource_type - remote resource type identifiers
+ */
+enum pci_epc_remote_resource_type {
+       PCI_EPC_RR_DMA_CTRL_MMIO,
+       PCI_EPC_RR_DMA_CHAN_DESC,
+};
+
+/**
+ * struct pci_epc_remote_resource - a physical resource that can be exposed
+ * @type:      resource type, see enum pci_epc_remote_resource_type
+ * @phys_addr: physical base address of the resource
+ * @size:      size of the resource in bytes
+ * @u:         type-specific metadata
+ */
+struct pci_epc_remote_resource {
+       enum pci_epc_remote_resource_type type;
+       phys_addr_t phys_addr;
+       size_t size;
+
+       union {
+               /* PCI_EPC_RR_DMA_CHAN_DESC */
+               struct {
+                       u16 hw_chan_id;
+                       bool ep2rc;
+               } dma_chan_desc;
+       } u;
+};
+
 /**
  * struct pci_epc_ops - set of function pointers for performing EPC operations
  * @write_header: ops to populate configuration space header
@@ -84,6 +113,8 @@ struct pci_epc_map {
  * @start: ops to start the PCI link
  * @stop: ops to stop the PCI link
  * @get_features: ops to get the features supported by the EPC
+ * @get_remote_resources: ops to retrieve controller-owned resources that can be
+ *                       exposed to the remote host (optional)
  * @owner: the module owner containing the ops
  */
 struct pci_epc_ops {
@@ -115,6 +146,9 @@ struct pci_epc_ops {
        void    (*stop)(struct pci_epc *epc);
        const struct pci_epc_features* (*get_features)(struct pci_epc *epc,
                                                       u8 func_no, u8 vfunc_no);
+       int     (*get_remote_resources)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
+                                       struct pci_epc_remote_resource *resources,
+                                       int num_resources);
        struct module *owner;
 };

@@ -309,6 +343,9 @@ int pci_epc_start(struct pci_epc *epc);
 void pci_epc_stop(struct pci_epc *epc);
 const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
                                                    u8 func_no, u8 vfunc_no);
+int pci_epc_get_remote_resources(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
+                                struct pci_epc_remote_resource *resources,
+                                int num_resources);
 enum pci_barno
 pci_epc_get_first_free_bar(const struct pci_epc_features *epc_features);
 enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features

---8<---

Notes:
* The naming ("pci_epc_get_remote_resources") is still TBD, but the intent
  is to return everything needed to support remote use of an EPC-integraed
  DMA engine.
* With this API, [PATCH v2 1/7] and [PATCH v2 2/7] are no longer needed,
  since API caller does not need to know the low-level DMA channel id
  (hw_id).

[2] https://lore.kernel.org/all/20260118135440.1958279-26-den@valinux.co.jp/

Thanks for the review,
Koichiro

> 
> Frank
> 
> > add an optional DesignWare-specific extension, selected via Kconfig, to
> > exercise these helpers. Likewise on the host side
> > (drivers/misc/pci_endpoint_test.c and kselftest pci_endpoint_test.c).
> >
> > Koichiro
> >
> > >
> > > Frank
> > >
> > > > +{
> > > > +	struct dw_edma_chip *chip;
> > > > +	struct dw_pcie_ep *ep;
> > > > +	struct dw_pcie *pci;
> > > > +	bool dir_read;
> > > > +
> > > > +	if (!epc || !region)
> > > > +		return -EINVAL;
> > > > +	if (dir != DMA_DEV_TO_MEM && dir != DMA_MEM_TO_DEV)
> > > > +		return -EINVAL;
> > > > +	if (hw_id < 0)
> > > > +		return -EINVAL;
> > > > +
> > > > +	ep = epc_get_drvdata(epc);
> > > > +	if (!ep)
> > > > +		return -ENODEV;
> > > > +
> > > > +	pci = to_dw_pcie_from_ep(ep);
> > > > +	chip = &pci->edma;
> > > > +
> > > > +	if (!chip->dev)
> > > > +		return -ENODEV;
> > > > +
> > > > +	if (chip->flags & DW_EDMA_CHIP_LOCAL)
> > > > +		dir_read = (dir == DMA_DEV_TO_MEM);
> > > > +	else
> > > > +		dir_read = (dir == DMA_MEM_TO_DEV);
> > > > +
> > > > +	if (dir_read) {
> > > > +		if (hw_id >= chip->ll_rd_cnt)
> > > > +			return -EINVAL;
> > > > +		*region = chip->ll_region_rd[hw_id];
> > > > +	} else {
> > > > +		if (hw_id >= chip->ll_wr_cnt)
> > > > +			return -EINVAL;
> > > > +		*region = chip->ll_region_wr[hw_id];
> > > > +	}
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(dwc_pcie_edma_get_ll_region);
> > > > diff --git a/include/linux/pcie-dwc-edma.h b/include/linux/pcie-dwc-edma.h
> > > > index a5b0595603f4..36afb4df1998 100644
> > > > --- a/include/linux/pcie-dwc-edma.h
> > > > +++ b/include/linux/pcie-dwc-edma.h
> > > > @@ -6,6 +6,8 @@
> > > >  #ifndef LINUX_PCIE_DWC_EDMA_H
> > > >  #define LINUX_PCIE_DWC_EDMA_H
> > > >
> > > > +#include <linux/dma/edma.h>
> > > > +#include <linux/dmaengine.h>
> > > >  #include <linux/errno.h>
> > > >  #include <linux/kconfig.h>
> > > >  #include <linux/pci-epc.h>
> > > > @@ -27,6 +29,29 @@
> > > >   */
> > > >  int dwc_pcie_edma_get_reg_window(struct pci_epc *epc, phys_addr_t *phys,
> > > >  				 resource_size_t *sz);
> > > > +
> > > > +/**
> > > > + * dwc_pcie_edma_get_ll_region() - get linked-list (LL) region for a HW channel
> > > > + * @epc:    EPC device associated with the integrated eDMA instance
> > > > + * @dir:    DMA transfer direction (%DMA_DEV_TO_MEM or %DMA_MEM_TO_DEV)
> > > > + * @hw_id:  hardware channel identifier (equals to dw_edma_chan.id)
> > > > + * @region: pointer to a region descriptor to fill in
> > > > + *
> > > > + * Some integrated DesignWare eDMA instances allocate per-channel linked-list
> > > > + * (LL) regions for descriptor storage. This helper returns the LL region
> > > > + * corresponding to @dir and @hw_id.
> > > > + *
> > > > + * The mapping between @dir and the underlying eDMA read/write LL region
> > > > + * depends on whether the integrated eDMA instance represents a local or a
> > > > + * remote view.
> > > > + *
> > > > + * Return: 0 on success, -EINVAL on invalid arguments (including out-of-range
> > > > + *         @hw_id), or -ENODEV if the integrated eDMA instance is not present
> > > > + *         or not initialized.
> > > > + */
> > > > +int dwc_pcie_edma_get_ll_region(struct pci_epc *epc,
> > > > +				enum dma_transfer_direction dir, int hw_id,
> > > > +				struct dw_edma_region *region);
> > > >  #else
> > > >  static inline int
> > > >  dwc_pcie_edma_get_reg_window(struct pci_epc *epc, phys_addr_t *phys,
> > > > @@ -34,6 +59,14 @@ dwc_pcie_edma_get_reg_window(struct pci_epc *epc, phys_addr_t *phys,
> > > >  {
> > > >  	return -ENODEV;
> > > >  }
> > > > +
> > > > +static inline int
> > > > +dwc_pcie_edma_get_ll_region(struct pci_epc *epc,
> > > > +			    enum dma_transfer_direction dir, int hw_id,
> > > > +			    struct dw_edma_region *region)
> > > > +{
> > > > +	return -ENODEV;
> > > > +}
> > > >  #endif /* CONFIG_PCIE_DW */
> > > >
> > > >  #endif /* LINUX_PCIE_DWC_EDMA_H */
> > > > --
> > > > 2.51.0
> > > >

Powered by blists - more mailing lists