[PATCH 0/2] Add AMD MDB Endpoint and non-LL mode Support

[PATCH 0/2] Add AMD MDB Endpoint and non-LL mode Support

[Devendra K Verma]

This series of patch support the following:

 - AMD MDB Endpoint Support, as part of this patch following are
   added:
   o AMD supported device ID and vendor ID (Xilinx)
   o AMD MDB specific driver data
   o AMD specific VSEC capabilities to retrieve the base of
     phys address of MDB side DDR

 - Addition of non-LL mode
   o The IP supported non-LL mode functions
   o Flexibility to choose non-LL mode via dma_slave_config
     param peripheral_config, by the client
   o Allow IP utilization if LL mode is not available


Devendra K Verma (2):
  dmaengine: dw-edma: Add AMD MDB Endpoint Support
  dmaengine: dw-edma: Add non-LL mode

 drivers/dma/dw-edma/dw-edma-core.c    |  38 ++++++++++--
 drivers/dma/dw-edma/dw-edma-core.h    |   1 +
 drivers/dma/dw-edma/dw-edma-pcie.c    | 112 +++++++++++++++++++++++++++++++---
 drivers/dma/dw-edma/dw-hdma-v0-core.c |  62 ++++++++++++++++++-
 include/linux/dma/edma.h              |   1 +
 include/linux/pci_ids.h               |   1 +
 6 files changed, 201 insertions(+), 14 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] dmaengine: dw-edma: Add AMD MDB Endpoint Support

[Devendra K Verma]

AMD MDB PCIe endpoint support. For AMD specific support
added the following
  - AMD supported PCIe Device IDs and Vendor ID (Xilinx).
  - AMD MDB specific driver data
  - AMD MDB specific VSEC capability to retrieve the device DDR
    base address.

Signed-off-by: Devendra K Verma <devendra.verma@amd.com>
---
 drivers/dma/dw-edma/dw-edma-pcie.c | 83 +++++++++++++++++++++++++++++++++++++-
 include/linux/pci_ids.h            |  1 +
 2 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 3371e0a7..749067b 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -18,10 +18,12 @@
 #include "dw-edma-core.h"
 
 #define DW_PCIE_VSEC_DMA_ID			0x6
+#define DW_PCIE_AMD_MDB_VSEC_ID			0x20
 #define DW_PCIE_VSEC_DMA_BAR			GENMASK(10, 8)
 #define DW_PCIE_VSEC_DMA_MAP			GENMASK(2, 0)
 #define DW_PCIE_VSEC_DMA_WR_CH			GENMASK(9, 0)
 #define DW_PCIE_VSEC_DMA_RD_CH			GENMASK(25, 16)
+#define DW_PCIE_AMD_MDB_INVALID_ADDR		(~0ULL)
 
 #define DW_BLOCK(a, b, c) \
 	{ \
@@ -50,6 +52,7 @@ struct dw_edma_pcie_data {
 	u8				irqs;
 	u16				wr_ch_cnt;
 	u16				rd_ch_cnt;
+	u64				phys_addr;
 };
 
 static const struct dw_edma_pcie_data snps_edda_data = {
@@ -90,6 +93,44 @@ struct dw_edma_pcie_data {
 	.rd_ch_cnt			= 2,
 };
 
+static const struct dw_edma_pcie_data amd_mdb_data = {
+	/* MDB registers location */
+	.rg.bar				= BAR_0,
+	.rg.off				= 0x00001000,	/*  4 Kbytes */
+	.rg.sz				= 0x00002000,	/*  8 Kbytes */
+	/* MDB memory linked list location */
+	.ll_wr = {
+		/* Channel 0 - BAR 2, offset 0 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00000000, 0x00000800)
+		/* Channel 1 - BAR 2, offset 2 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00200000, 0x00000800)
+	},
+	.ll_rd = {
+		/* Channel 0 - BAR 2, offset 4 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00400000, 0x00000800)
+		/* Channel 1 - BAR 2, offset 6 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00600000, 0x00000800)
+	},
+	/* MDB memory data location */
+	.dt_wr = {
+		/* Channel 0 - BAR 2, offset 8 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00800000, 0x00000800)
+		/* Channel 1 - BAR 2, offset 9 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00900000, 0x00000800)
+	},
+	.dt_rd = {
+		/* Channel 0 - BAR 2, offset 10 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00a00000, 0x00000800)
+		/* Channel 1 - BAR 2, offset 11 Mbytes, size 2 Kbytes */
+		DW_BLOCK(BAR_2, 0x00b00000, 0x00000800)
+	},
+	/* Other */
+	.mf				= EDMA_MF_HDMA_NATIVE,
+	.irqs				= 1,
+	.wr_ch_cnt			= 2,
+	.rd_ch_cnt			= 2,
+};
+
 static int dw_edma_pcie_irq_vector(struct device *dev, unsigned int nr)
 {
 	return pci_irq_vector(to_pci_dev(dev), nr);
@@ -120,9 +161,14 @@ static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
 	u32 val, map;
 	u16 vsec;
 	u64 off;
+	u16 vendor;
 
-	vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_SYNOPSYS,
-					DW_PCIE_VSEC_DMA_ID);
+	vendor = pdev->vendor;
+	if (vendor != PCI_VENDOR_ID_SYNOPSYS &&
+	    vendor != PCI_VENDOR_ID_XILINX)
+		return;
+
+	vsec = pci_find_vsec_capability(pdev, vendor, DW_PCIE_VSEC_DMA_ID);
 	if (!vsec)
 		return;
 
@@ -155,6 +201,27 @@ static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
 	off <<= 32;
 	off |= val;
 	pdata->rg.off = off;
+
+	/* AMD specific VSEC capability */
+	if (vendor != PCI_VENDOR_ID_XILINX)
+		return;
+
+	vsec = pci_find_vsec_capability(pdev, vendor,
+					DW_PCIE_AMD_MDB_VSEC_ID);
+	if (!vsec)
+		return;
+
+	pci_read_config_dword(pdev, vsec + PCI_VNDR_HEADER, &val);
+	if (PCI_VNDR_HEADER_ID(val) != 0x20 ||
+	    PCI_VNDR_HEADER_REV(val) != 0x1)
+		return;
+
+	pci_read_config_dword(pdev, vsec + 0xc, &val);
+	off = val;
+	pci_read_config_dword(pdev, vsec + 0x8, &val);
+	off <<= 32;
+	off |= val;
+	pdata->phys_addr = off;
 }
 
 static int dw_edma_pcie_probe(struct pci_dev *pdev,
@@ -179,6 +246,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	}
 
 	memcpy(vsec_data, pdata, sizeof(struct dw_edma_pcie_data));
+	vsec_data->phys_addr = DW_PCIE_AMD_MDB_INVALID_ADDR;
 
 	/*
 	 * Tries to find if exists a PCIe Vendor-Specific Extended Capability
@@ -186,6 +254,15 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	 */
 	dw_edma_pcie_get_vsec_dma_data(pdev, vsec_data);
 
+	if (pdev->vendor == PCI_VENDOR_ID_XILINX) {
+		/*
+		 * There is no valid address found for the LL memory
+		 * space on the device side.
+		 */
+		if (vsec_data->phys_addr == DW_PCIE_AMD_MDB_INVALID_ADDR)
+			return -EINVAL;
+	}
+
 	/* Mapping PCI BAR regions */
 	mask = BIT(vsec_data->rg.bar);
 	for (i = 0; i < vsec_data->wr_ch_cnt; i++) {
@@ -367,6 +444,8 @@ static void dw_edma_pcie_remove(struct pci_dev *pdev)
 
 static const struct pci_device_id dw_edma_pcie_id_table[] = {
 	{ PCI_DEVICE_DATA(SYNOPSYS, EDDA, &snps_edda_data) },
+	{ PCI_VDEVICE(XILINX, PCI_DEVICE_ID_AMD_MDB_B054),
+	  (kernel_ulong_t)&amd_mdb_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(pci, dw_edma_pcie_id_table);
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 92ffc43..c15607d 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -636,6 +636,7 @@
 #define PCI_DEVICE_ID_AMD_HUDSON2_SMBUS		0x780b
 #define PCI_DEVICE_ID_AMD_HUDSON2_IDE		0x780c
 #define PCI_DEVICE_ID_AMD_KERNCZ_SMBUS  0x790b
+#define PCI_DEVICE_ID_AMD_MDB_B054	0xb054
 
 #define PCI_VENDOR_ID_TRIDENT		0x1023
 #define PCI_DEVICE_ID_TRIDENT_4DWAVE_DX	0x2000
-- 
1.8.3.1

[PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

[Devendra K Verma]

AMD MDB IP supports Linked List (LL) mode as well as non-LL mode.
The current code does not have the mechanisms to enable the
DMA transactions using the non-LL mode. The following two cases
are added with this patch:
- When a valid physical base address is not configured via the
  Xilinx VSEC capability then the IP can still be used in non-LL
  mode. The default mode for all the DMA transactions and for all
  the DMA channels then is non-LL mode.
- When a valid physical base address is configured but the client
  wants to use the non-LL mode for DMA transactions then also the
  flexibility is provided via the peripheral_config struct member of
  dma_slave_config. In this case the channels can be individually
  configured in non-LL mode. This use case is desirable for single
  DMA transfer of a chunk, this saves the effort of preparing the
  Link List.

Signed-off-by: Devendra K Verma <devendra.verma@amd.com>
---
 drivers/dma/dw-edma/dw-edma-core.c    | 38 ++++++++++++++++++---
 drivers/dma/dw-edma/dw-edma-core.h    |  1 +
 drivers/dma/dw-edma/dw-edma-pcie.c    | 33 ++++++++++++++-----
 drivers/dma/dw-edma/dw-hdma-v0-core.c | 62 ++++++++++++++++++++++++++++++++++-
 include/linux/dma/edma.h              |  1 +
 5 files changed, 121 insertions(+), 14 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index b43255f..dbef571 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -223,8 +223,28 @@ static int dw_edma_device_config(struct dma_chan *dchan,
 				 struct dma_slave_config *config)
 {
 	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+	int nollp = 0;
+
+	if (WARN_ON(config->peripheral_config &&
+		    config->peripheral_size != sizeof(int)))
+		return -EINVAL;
 
 	memcpy(&chan->config, config, sizeof(*config));
+
+	/*
+	 * When there is no valid LLP base address available
+	 * then the default DMA ops will use the non-LL mode.
+	 * Cases where LL mode is enabled and client wants
+	 * to use the non-LL mode then also client can do
+	 * so via the providing the peripheral_config param.
+	 */
+	if (config->peripheral_config)
+		nollp = *(int *)config->peripheral_config;
+
+	chan->nollp = false;
+	if (chan->dw->chip->nollp || (!chan->dw->chip->nollp && nollp))
+		chan->nollp = true;
+
 	chan->configured = true;
 
 	return 0;
@@ -353,7 +373,7 @@ static void dw_edma_device_issue_pending(struct dma_chan *dchan)
 	struct dw_edma_chan *chan = dchan2dw_edma_chan(xfer->dchan);
 	enum dma_transfer_direction dir = xfer->direction;
 	struct scatterlist *sg = NULL;
-	struct dw_edma_chunk *chunk;
+	struct dw_edma_chunk *chunk = NULL;
 	struct dw_edma_burst *burst;
 	struct dw_edma_desc *desc;
 	u64 src_addr, dst_addr;
@@ -419,9 +439,11 @@ static void dw_edma_device_issue_pending(struct dma_chan *dchan)
 	if (unlikely(!desc))
 		goto err_alloc;
 
-	chunk = dw_edma_alloc_chunk(desc);
-	if (unlikely(!chunk))
-		goto err_alloc;
+	if (!chan->nollp) {
+		chunk = dw_edma_alloc_chunk(desc);
+		if (unlikely(!chunk))
+			goto err_alloc;
+	}
 
 	if (xfer->type == EDMA_XFER_INTERLEAVED) {
 		src_addr = xfer->xfer.il->src_start;
@@ -450,7 +472,13 @@ static void dw_edma_device_issue_pending(struct dma_chan *dchan)
 		if (xfer->type == EDMA_XFER_SCATTER_GATHER && !sg)
 			break;
 
-		if (chunk->bursts_alloc == chan->ll_max) {
+		/*
+		 * For non-LL mode, only a single burst can be handled
+		 * in a single chunk unlike LL mode where multiple bursts
+		 * can be configured in a single chunk.
+		 */
+		if ((chunk && chunk->bursts_alloc == chan->ll_max) ||
+		    chan->nollp) {
 			chunk = dw_edma_alloc_chunk(desc);
 			if (unlikely(!chunk))
 				goto err_alloc;
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 71894b9..2a4ad45 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -86,6 +86,7 @@ struct dw_edma_chan {
 	u8				configured;
 
 	struct dma_slave_config		config;
+	bool				nollp;
 };
 
 struct dw_edma_irq {
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 749067b..0d6254f 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -224,6 +224,15 @@ static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
 	pdata->phys_addr = off;
 }
 
+static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
+				 struct dw_edma_pcie_data *pdata,
+				 enum pci_barno bar)
+{
+	if (pdev->vendor == PCI_VENDOR_ID_XILINX)
+		return pdata->phys_addr;
+	return pci_bus_address(pdev, bar);
+}
+
 static int dw_edma_pcie_probe(struct pci_dev *pdev,
 			      const struct pci_device_id *pid)
 {
@@ -233,6 +242,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	struct dw_edma_chip *chip;
 	int err, nr_irqs;
 	int i, mask;
+	bool nollp = false;
 
 	vsec_data = kmalloc(sizeof(*vsec_data), GFP_KERNEL);
 	if (!vsec_data)
@@ -257,10 +267,12 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	if (pdev->vendor == PCI_VENDOR_ID_XILINX) {
 		/*
 		 * There is no valid address found for the LL memory
-		 * space on the device side.
+		 * space on the device side. In the absence of LL base
+		 * address use the non-LL mode or simple mode supported by
+		 * the HDMA IP.
 		 */
 		if (vsec_data->phys_addr == DW_PCIE_AMD_MDB_INVALID_ADDR)
-			return -EINVAL;
+			nollp = true;
 	}
 
 	/* Mapping PCI BAR regions */
@@ -308,6 +320,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	chip->mf = vsec_data->mf;
 	chip->nr_irqs = nr_irqs;
 	chip->ops = &dw_edma_pcie_plat_ops;
+	chip->nollp = nollp;
 
 	chip->ll_wr_cnt = vsec_data->wr_ch_cnt;
 	chip->ll_rd_cnt = vsec_data->rd_ch_cnt;
@@ -316,7 +329,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	if (!chip->reg_base)
 		return -ENOMEM;
 
-	for (i = 0; i < chip->ll_wr_cnt; i++) {
+	for (i = 0; i < chip->ll_wr_cnt && !nollp; i++) {
 		struct dw_edma_region *ll_region = &chip->ll_region_wr[i];
 		struct dw_edma_region *dt_region = &chip->dt_region_wr[i];
 		struct dw_edma_block *ll_block = &vsec_data->ll_wr[i];
@@ -327,7 +340,8 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 			return -ENOMEM;
 
 		ll_region->vaddr.io += ll_block->off;
-		ll_region->paddr = pci_bus_address(pdev, ll_block->bar);
+		ll_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
+							 ll_block->bar);
 		ll_region->paddr += ll_block->off;
 		ll_region->sz = ll_block->sz;
 
@@ -336,12 +350,13 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 			return -ENOMEM;
 
 		dt_region->vaddr.io += dt_block->off;
-		dt_region->paddr = pci_bus_address(pdev, dt_block->bar);
+		dt_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
+							 dt_block->bar);
 		dt_region->paddr += dt_block->off;
 		dt_region->sz = dt_block->sz;
 	}
 
-	for (i = 0; i < chip->ll_rd_cnt; i++) {
+	for (i = 0; i < chip->ll_rd_cnt && !nollp; i++) {
 		struct dw_edma_region *ll_region = &chip->ll_region_rd[i];
 		struct dw_edma_region *dt_region = &chip->dt_region_rd[i];
 		struct dw_edma_block *ll_block = &vsec_data->ll_rd[i];
@@ -352,7 +367,8 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 			return -ENOMEM;
 
 		ll_region->vaddr.io += ll_block->off;
-		ll_region->paddr = pci_bus_address(pdev, ll_block->bar);
+		ll_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
+							 ll_block->bar);
 		ll_region->paddr += ll_block->off;
 		ll_region->sz = ll_block->sz;
 
@@ -361,7 +377,8 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 			return -ENOMEM;
 
 		dt_region->vaddr.io += dt_block->off;
-		dt_region->paddr = pci_bus_address(pdev, dt_block->bar);
+		dt_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
+							 dt_block->bar);
 		dt_region->paddr += dt_block->off;
 		dt_region->sz = dt_block->sz;
 	}
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index e3f8db4..befb9e0 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -225,7 +225,7 @@ static void dw_hdma_v0_sync_ll_data(struct dw_edma_chunk *chunk)
 		readl(chunk->ll_region.vaddr.io);
 }
 
-static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
+static void dw_hdma_v0_core_ll_start(struct dw_edma_chunk *chunk, bool first)
 {
 	struct dw_edma_chan *chan = chunk->chan;
 	struct dw_edma *dw = chan->dw;
@@ -263,6 +263,66 @@ static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
 	SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
 }
 
+static void dw_hdma_v0_core_non_ll_start(struct dw_edma_chunk *chunk)
+{
+	struct dw_edma_chan *chan = chunk->chan;
+	struct dw_edma *dw = chan->dw;
+	struct dw_edma_burst *child;
+	u32 val;
+
+	list_for_each_entry(child, &chunk->burst->list, list) {
+		SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
+
+		/* Source address */
+		SET_CH_32(dw, chan->dir, chan->id, sar.lsb,
+			  lower_32_bits(child->sar));
+		SET_CH_32(dw, chan->dir, chan->id, sar.msb,
+			  upper_32_bits(child->sar));
+
+		/* Destination address */
+		SET_CH_32(dw, chan->dir, chan->id, dar.lsb,
+			  lower_32_bits(child->dar));
+		SET_CH_32(dw, chan->dir, chan->id, dar.msb,
+			  upper_32_bits(child->dar));
+
+		/* Transfer size */
+		SET_CH_32(dw, chan->dir, chan->id, transfer_size, child->sz);
+
+		/* Interrupt setup */
+		val = GET_CH_32(dw, chan->dir, chan->id, int_setup) |
+				HDMA_V0_STOP_INT_MASK |
+				HDMA_V0_ABORT_INT_MASK |
+				HDMA_V0_LOCAL_STOP_INT_EN |
+				HDMA_V0_LOCAL_ABORT_INT_EN;
+
+		if (!(dw->chip->flags & DW_EDMA_CHIP_LOCAL)) {
+			val |= HDMA_V0_REMOTE_STOP_INT_EN |
+			       HDMA_V0_REMOTE_ABORT_INT_EN;
+		}
+
+		SET_CH_32(dw, chan->dir, chan->id, int_setup, val);
+
+		/* Channel control setup */
+		val = GET_CH_32(dw, chan->dir, chan->id, control1);
+		val &= ~HDMA_V0_LINKLIST_EN;
+		SET_CH_32(dw, chan->dir, chan->id, control1, val);
+
+		/* Ring the doorbell */
+		SET_CH_32(dw, chan->dir, chan->id, doorbell,
+			  HDMA_V0_DOORBELL_START);
+	}
+}
+
+static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
+{
+	struct dw_edma_chan *chan = chunk->chan;
+
+	if (!chan->nollp)
+		dw_hdma_v0_core_ll_start(chunk, first);
+	else
+		dw_hdma_v0_core_non_ll_start(chunk);
+}
+
 static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
 {
 	struct dw_edma *dw = chan->dw;
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index 3080747..e14e16f 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -99,6 +99,7 @@ struct dw_edma_chip {
 	enum dw_edma_map_format	mf;
 
 	struct dw_edma		*dw;
+	bool			nollp;
 };
 
 /* Export to the platform drivers */
-- 
1.8.3.1

Re: [PATCH 1/2] dmaengine: dw-edma: Add AMD MDB Endpoint Support

[Bjorn Helgaas]

On Fri, Sep 05, 2025 at 03:46:58PM +0530, Devendra K Verma wrote:
> AMD MDB PCIe endpoint support. For AMD specific support
> added the following
>   - AMD supported PCIe Device IDs and Vendor ID (Xilinx).
>   - AMD MDB specific driver data
>   - AMD MDB specific VSEC capability to retrieve the device DDR
>     base address.
> 
> Signed-off-by: Devendra K Verma <devendra.verma@amd.com>
> ---
>  drivers/dma/dw-edma/dw-edma-pcie.c | 83 +++++++++++++++++++++++++++++++++++++-
>  include/linux/pci_ids.h            |  1 +
>  2 files changed, 82 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index 3371e0a7..749067b 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -18,10 +18,12 @@
>  #include "dw-edma-core.h"
>  
>  #define DW_PCIE_VSEC_DMA_ID			0x6
> +#define DW_PCIE_AMD_MDB_VSEC_ID			0x20
>  #define DW_PCIE_VSEC_DMA_BAR			GENMASK(10, 8)
>  #define DW_PCIE_VSEC_DMA_MAP			GENMASK(2, 0)
>  #define DW_PCIE_VSEC_DMA_WR_CH			GENMASK(9, 0)
>  #define DW_PCIE_VSEC_DMA_RD_CH			GENMASK(25, 16)
> +#define DW_PCIE_AMD_MDB_INVALID_ADDR		(~0ULL)
>  
>  #define DW_BLOCK(a, b, c) \
>  	{ \
> @@ -50,6 +52,7 @@ struct dw_edma_pcie_data {
>  	u8				irqs;
>  	u16				wr_ch_cnt;
>  	u16				rd_ch_cnt;
> +	u64				phys_addr;
>  };
>  
>  static const struct dw_edma_pcie_data snps_edda_data = {
> @@ -90,6 +93,44 @@ struct dw_edma_pcie_data {
>  	.rd_ch_cnt			= 2,
>  };
>  
> +static const struct dw_edma_pcie_data amd_mdb_data = {
> +	/* MDB registers location */
> +	.rg.bar				= BAR_0,
> +	.rg.off				= 0x00001000,	/*  4 Kbytes */
> +	.rg.sz				= 0x00002000,	/*  8 Kbytes */
> +	/* MDB memory linked list location */
> +	.ll_wr = {
> +		/* Channel 0 - BAR 2, offset 0 Mbytes, size 2 Kbytes */
> +		DW_BLOCK(BAR_2, 0x00000000, 0x00000800)
> +		/* Channel 1 - BAR 2, offset 2 Mbytes, size 2 Kbytes */
> +		DW_BLOCK(BAR_2, 0x00200000, 0x00000800)
> +	},
> +	.ll_rd = {
> +		/* Channel 0 - BAR 2, offset 4 Mbytes, size 2 Kbytes */
> +		DW_BLOCK(BAR_2, 0x00400000, 0x00000800)
> +		/* Channel 1 - BAR 2, offset 6 Mbytes, size 2 Kbytes */
> +		DW_BLOCK(BAR_2, 0x00600000, 0x00000800)
> +	},
> +	/* MDB memory data location */
> +	.dt_wr = {
> +		/* Channel 0 - BAR 2, offset 8 Mbytes, size 2 Kbytes */
> +		DW_BLOCK(BAR_2, 0x00800000, 0x00000800)
> +		/* Channel 1 - BAR 2, offset 9 Mbytes, size 2 Kbytes */
> +		DW_BLOCK(BAR_2, 0x00900000, 0x00000800)
> +	},
> +	.dt_rd = {
> +		/* Channel 0 - BAR 2, offset 10 Mbytes, size 2 Kbytes */
> +		DW_BLOCK(BAR_2, 0x00a00000, 0x00000800)
> +		/* Channel 1 - BAR 2, offset 11 Mbytes, size 2 Kbytes */
> +		DW_BLOCK(BAR_2, 0x00b00000, 0x00000800)
> +	},
> +	/* Other */
> +	.mf				= EDMA_MF_HDMA_NATIVE,
> +	.irqs				= 1,
> +	.wr_ch_cnt			= 2,
> +	.rd_ch_cnt			= 2,
> +};
> +
>  static int dw_edma_pcie_irq_vector(struct device *dev, unsigned int nr)
>  {
>  	return pci_irq_vector(to_pci_dev(dev), nr);
> @@ -120,9 +161,14 @@ static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
>  	u32 val, map;
>  	u16 vsec;
>  	u64 off;
> +	u16 vendor;
>  
> -	vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_SYNOPSYS,
> -					DW_PCIE_VSEC_DMA_ID);
> +	vendor = pdev->vendor;
> +	if (vendor != PCI_VENDOR_ID_SYNOPSYS &&
> +	    vendor != PCI_VENDOR_ID_XILINX)
> +		return;
> +
> +	vsec = pci_find_vsec_capability(pdev, vendor, DW_PCIE_VSEC_DMA_ID);

The vendor of a device assigns VSEC IDs and determines what each ID
means, so the semantics of a VSEC Capability are determined by the
tuple of (device Vendor ID, VSEC ID), where the Vendor ID is the value
at 0x00 in config space.

This code assumes that Synopsys and Xilinx agreed on the same VSEC ID
(6) and semantics of that Capability.

I assume you know this is true in this particular case, but it is not
safe for in general because even if other vendors incorporate this
same IP into their devices, they may choose different VSEC IDs because
they may have already assigned the VSEC ID 6 for something else.

So you should add a comment to the effect that "Synopsys and Xilinx
happened to use the same VSEC ID and semantics.  This may vary for
other vendors."

The DVSEC Capability is a more generic solution to this problem.  The
VSEC ID namespace is determined by the Vendor ID of the *device*.

By contrast, the DVSEC ID namespace is determined by a Vendor ID in
the DVSEC Capability itself, not by the Vendor ID of the device.

So AMD could define a DVSEC ID, e.g., 6, and define the semantics of
that DVSEC.  Then devices from *any* vendor could include a DVSEC
Capability with (PCI_VENDOR_ID_AMD, 6), and generic code could look
for that DVSEC independent of what is at 0x00 in config space.

>  	if (!vsec)
>  		return;
>  
> @@ -155,6 +201,27 @@ static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
>  	off <<= 32;
>  	off |= val;
>  	pdata->rg.off = off;
> +
> +	/* AMD specific VSEC capability */
> +	if (vendor != PCI_VENDOR_ID_XILINX)
> +		return;
> +
> +	vsec = pci_find_vsec_capability(pdev, vendor,
> +					DW_PCIE_AMD_MDB_VSEC_ID);

pci_find_vsec_capability() finds a Vendor-Specific Extended Capability
defined by the vendor of the device (Xilinx in this case).

pci_find_vsec_capability() already checks that dev->vendor matches the
vendor argument so you don't need the "vendor != PCI_VENDOR_ID_XILINX"
check above.

DW_PCIE_AMD_MDB_VSEC_ID should include "XILINX" because this ID is in
the namespace created by PCI_VENDOR_ID_XILINX, i.e., it's somewhere in
the (PCI_VENDOR_ID_XILINX, x) space.

This code should look something like this (you could add "MDB" or
something if it makes sense):

  #define PCI_VSEC_ID_XILINX_DMA_DATA               0x20

  vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_XILINX,
                                  PCI_VSEC_ID_XILINX_DMA_DATA);

> +	if (!vsec)
> +		return;
> +
> +	pci_read_config_dword(pdev, vsec + PCI_VNDR_HEADER, &val);
> +	if (PCI_VNDR_HEADER_ID(val) != 0x20 ||
> +	    PCI_VNDR_HEADER_REV(val) != 0x1)
> +		return;
> +
> +	pci_read_config_dword(pdev, vsec + 0xc, &val);
> +	off = val;
> +	pci_read_config_dword(pdev, vsec + 0x8, &val);
> +	off <<= 32;
> +	off |= val;
> +	pdata->phys_addr = off;
>  }
>  
>  static int dw_edma_pcie_probe(struct pci_dev *pdev,
> @@ -179,6 +246,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>  	}
>  
>  	memcpy(vsec_data, pdata, sizeof(struct dw_edma_pcie_data));
> +	vsec_data->phys_addr = DW_PCIE_AMD_MDB_INVALID_ADDR;
>  
>  	/*
>  	 * Tries to find if exists a PCIe Vendor-Specific Extended Capability
> @@ -186,6 +254,15 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>  	 */
>  	dw_edma_pcie_get_vsec_dma_data(pdev, vsec_data);
>  
> +	if (pdev->vendor == PCI_VENDOR_ID_XILINX) {
> +		/*
> +		 * There is no valid address found for the LL memory
> +		 * space on the device side.
> +		 */
> +		if (vsec_data->phys_addr == DW_PCIE_AMD_MDB_INVALID_ADDR)
> +			return -EINVAL;
> +	}
> +
>  	/* Mapping PCI BAR regions */
>  	mask = BIT(vsec_data->rg.bar);
>  	for (i = 0; i < vsec_data->wr_ch_cnt; i++) {
> @@ -367,6 +444,8 @@ static void dw_edma_pcie_remove(struct pci_dev *pdev)
>  
>  static const struct pci_device_id dw_edma_pcie_id_table[] = {
>  	{ PCI_DEVICE_DATA(SYNOPSYS, EDDA, &snps_edda_data) },
> +	{ PCI_VDEVICE(XILINX, PCI_DEVICE_ID_AMD_MDB_B054),
> +	  (kernel_ulong_t)&amd_mdb_data },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(pci, dw_edma_pcie_id_table);
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 92ffc43..c15607d 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -636,6 +636,7 @@
>  #define PCI_DEVICE_ID_AMD_HUDSON2_SMBUS		0x780b
>  #define PCI_DEVICE_ID_AMD_HUDSON2_IDE		0x780c
>  #define PCI_DEVICE_ID_AMD_KERNCZ_SMBUS  0x790b
> +#define PCI_DEVICE_ID_AMD_MDB_B054	0xb054

Unless PCI_DEVICE_ID_AMD_MDB_B054 is used in more than one driver,
move the #define into the file that uses it.  See the note at the top
of pci_ids.h.

Re: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

[Bjorn Helgaas]

On Fri, Sep 05, 2025 at 03:46:59PM +0530, Devendra K Verma wrote:
> AMD MDB IP supports Linked List (LL) mode as well as non-LL mode.
> The current code does not have the mechanisms to enable the
> DMA transactions using the non-LL mode. The following two cases
> are added with this patch:
> - When a valid physical base address is not configured via the
>   Xilinx VSEC capability then the IP can still be used in non-LL
>   mode. The default mode for all the DMA transactions and for all
>   the DMA channels then is non-LL mode.
> - When a valid physical base address is configured but the client
>   wants to use the non-LL mode for DMA transactions then also the
>   flexibility is provided via the peripheral_config struct member of
>   dma_slave_config. In this case the channels can be individually
>   configured in non-LL mode. This use case is desirable for single
>   DMA transfer of a chunk, this saves the effort of preparing the
>   Link List.

> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -223,8 +223,28 @@ static int dw_edma_device_config(struct dma_chan *dchan,
>  				 struct dma_slave_config *config)
>  {
>  	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> +	int nollp = 0;
> +
> +	if (WARN_ON(config->peripheral_config &&
> +		    config->peripheral_size != sizeof(int)))
> +		return -EINVAL;
>  
>  	memcpy(&chan->config, config, sizeof(*config));
> +
> +	/*
> +	 * When there is no valid LLP base address available
> +	 * then the default DMA ops will use the non-LL mode.
> +	 * Cases where LL mode is enabled and client wants
> +	 * to use the non-LL mode then also client can do
> +	 * so via the providing the peripheral_config param.

s/via the/via/

> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -224,6 +224,15 @@ static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
>  	pdata->phys_addr = off;
>  }
>  
> +static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
> +				 struct dw_edma_pcie_data *pdata,
> +				 enum pci_barno bar)
> +{
> +	if (pdev->vendor == PCI_VENDOR_ID_XILINX)
> +		return pdata->phys_addr;
> +	return pci_bus_address(pdev, bar);

This doesn't seem right.  pci_bus_address() returns pci_bus_addr_t, so
pdata->phys_addr should also be a pci_bus_addr_t, and the function
should return pci_bus_addr_t.

A pci_bus_addr_t is not a "phys_addr"; it is an address that is valid
on the PCI side of a PCI host bridge, which may be different than the
CPU physical address on the CPU side of the bridge because of things
like IOMMUs.

Seems like the struct dw_edma_region.paddr should be renamed to
something like "bus_addr" and made into a pci_bus_addr_t.

Bjorn

RE: [PATCH 1/2] dmaengine: dw-edma: Add AMD MDB Endpoint Support

[Verma, Devendra]

[AMD Official Use Only - AMD Internal Distribution Only]

Thank you, Bjorn for reviewing the code!
Please check my response inline.

Regards,
Devendra

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Friday, September 5, 2025 22:24
> To: Verma, Devendra <Devendra.Verma@amd.com>
> Cc: bhelgaas@google.com; mani@kernel.org; vkoul@kernel.org;
> dmaengine@vger.kernel.org; linux-pci@vger.kernel.org; linux-
> kernel@vger.kernel.org; Simek, Michal <michal.simek@amd.com>
> Subject: Re: [PATCH 1/2] dmaengine: dw-edma: Add AMD MDB Endpoint Support
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Fri, Sep 05, 2025 at 03:46:58PM +0530, Devendra K Verma wrote:
> > AMD MDB PCIe endpoint support. For AMD specific support added the
> > following
> >   - AMD supported PCIe Device IDs and Vendor ID (Xilinx).
> >   - AMD MDB specific driver data
> >   - AMD MDB specific VSEC capability to retrieve the device DDR
> >     base address.
> >
> > Signed-off-by: Devendra K Verma <devendra.verma@amd.com>
> > ---
> >  drivers/dma/dw-edma/dw-edma-pcie.c | 83
> +++++++++++++++++++++++++++++++++++++-
> >  include/linux/pci_ids.h            |  1 +
> >  2 files changed, 82 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c
> > b/drivers/dma/dw-edma/dw-edma-pcie.c
> > index 3371e0a7..749067b 100644
> > --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> > @@ -18,10 +18,12 @@
> >  #include "dw-edma-core.h"
> >
> >  #define DW_PCIE_VSEC_DMA_ID                  0x6
> > +#define DW_PCIE_AMD_MDB_VSEC_ID                      0x20
> >  #define DW_PCIE_VSEC_DMA_BAR                 GENMASK(10, 8)
> >  #define DW_PCIE_VSEC_DMA_MAP                 GENMASK(2, 0)
> >  #define DW_PCIE_VSEC_DMA_WR_CH                       GENMASK(9, 0)
> >  #define DW_PCIE_VSEC_DMA_RD_CH                       GENMASK(25, 16)
> > +#define DW_PCIE_AMD_MDB_INVALID_ADDR         (~0ULL)
> >
> >  #define DW_BLOCK(a, b, c) \
> >       { \
> > @@ -50,6 +52,7 @@ struct dw_edma_pcie_data {
> >       u8                              irqs;
> >       u16                             wr_ch_cnt;
> >       u16                             rd_ch_cnt;
> > +     u64                             phys_addr;
> >  };
> >
> >  static const struct dw_edma_pcie_data snps_edda_data = { @@ -90,6
> > +93,44 @@ struct dw_edma_pcie_data {
> >       .rd_ch_cnt                      = 2,
> >  };
> >
> > +static const struct dw_edma_pcie_data amd_mdb_data = {
> > +     /* MDB registers location */
> > +     .rg.bar                         = BAR_0,
> > +     .rg.off                         = 0x00001000,   /*  4 Kbytes */
> > +     .rg.sz                          = 0x00002000,   /*  8 Kbytes */
> > +     /* MDB memory linked list location */
> > +     .ll_wr = {
> > +             /* Channel 0 - BAR 2, offset 0 Mbytes, size 2 Kbytes */
> > +             DW_BLOCK(BAR_2, 0x00000000, 0x00000800)
> > +             /* Channel 1 - BAR 2, offset 2 Mbytes, size 2 Kbytes */
> > +             DW_BLOCK(BAR_2, 0x00200000, 0x00000800)
> > +     },
> > +     .ll_rd = {
> > +             /* Channel 0 - BAR 2, offset 4 Mbytes, size 2 Kbytes */
> > +             DW_BLOCK(BAR_2, 0x00400000, 0x00000800)
> > +             /* Channel 1 - BAR 2, offset 6 Mbytes, size 2 Kbytes */
> > +             DW_BLOCK(BAR_2, 0x00600000, 0x00000800)
> > +     },
> > +     /* MDB memory data location */
> > +     .dt_wr = {
> > +             /* Channel 0 - BAR 2, offset 8 Mbytes, size 2 Kbytes */
> > +             DW_BLOCK(BAR_2, 0x00800000, 0x00000800)
> > +             /* Channel 1 - BAR 2, offset 9 Mbytes, size 2 Kbytes */
> > +             DW_BLOCK(BAR_2, 0x00900000, 0x00000800)
> > +     },
> > +     .dt_rd = {
> > +             /* Channel 0 - BAR 2, offset 10 Mbytes, size 2 Kbytes */
> > +             DW_BLOCK(BAR_2, 0x00a00000, 0x00000800)
> > +             /* Channel 1 - BAR 2, offset 11 Mbytes, size 2 Kbytes */
> > +             DW_BLOCK(BAR_2, 0x00b00000, 0x00000800)
> > +     },
> > +     /* Other */
> > +     .mf                             = EDMA_MF_HDMA_NATIVE,
> > +     .irqs                           = 1,
> > +     .wr_ch_cnt                      = 2,
> > +     .rd_ch_cnt                      = 2,
> > +};
> > +
> >  static int dw_edma_pcie_irq_vector(struct device *dev, unsigned int
> > nr)  {
> >       return pci_irq_vector(to_pci_dev(dev), nr); @@ -120,9 +161,14 @@
> > static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
> >       u32 val, map;
> >       u16 vsec;
> >       u64 off;
> > +     u16 vendor;
> >
> > -     vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_SYNOPSYS,
> > -                                     DW_PCIE_VSEC_DMA_ID);
> > +     vendor = pdev->vendor;
> > +     if (vendor != PCI_VENDOR_ID_SYNOPSYS &&
> > +         vendor != PCI_VENDOR_ID_XILINX)
> > +             return;
> > +
> > +     vsec = pci_find_vsec_capability(pdev, vendor,
> > + DW_PCIE_VSEC_DMA_ID);
>
> The vendor of a device assigns VSEC IDs and determines what each ID means, so
> the semantics of a VSEC Capability are determined by the tuple of (device Vendor
> ID, VSEC ID), where the Vendor ID is the value at 0x00 in config space.
>

As AMD is a vendor for this device, it is determined as VSEC capability to support
some of the functionality not supported by the other vendor Synopsys.

> This code assumes that Synopsys and Xilinx agreed on the same VSEC ID
> (6) and semantics of that Capability.
>
> I assume you know this is true in this particular case, but it is not safe for in general
> because even if other vendors incorporate this same IP into their devices, they may
> choose different VSEC IDs because they may have already assigned the VSEC ID 6
> for something else.
>
> So you should add a comment to the effect that "Synopsys and Xilinx happened to
> use the same VSEC ID and semantics.  This may vary for other vendors."
>

Agree with the above suggestion. Will add this in next review.

> The DVSEC Capability is a more generic solution to this problem.  The VSEC ID
> namespace is determined by the Vendor ID of the *device*.
>
> By contrast, the DVSEC ID namespace is determined by a Vendor ID in the DVSEC
> Capability itself, not by the Vendor ID of the device.
>
> So AMD could define a DVSEC ID, e.g., 6, and define the semantics of that DVSEC.
> Then devices from *any* vendor could include a DVSEC Capability with
> (PCI_VENDOR_ID_AMD, 6), and generic code could look for that DVSEC
> independent of what is at 0x00 in config space.
>

As AMD itself becomes the vendor for this device, VSEC capability is chosen to support
the functionality missing in the code.

> >       if (!vsec)
> >               return;
> >
> > @@ -155,6 +201,27 @@ static void dw_edma_pcie_get_vsec_dma_data(struct
> pci_dev *pdev,
> >       off <<= 32;
> >       off |= val;
> >       pdata->rg.off = off;
> > +
> > +     /* AMD specific VSEC capability */
> > +     if (vendor != PCI_VENDOR_ID_XILINX)
> > +             return;
> > +
> > +     vsec = pci_find_vsec_capability(pdev, vendor,
> > +                                     DW_PCIE_AMD_MDB_VSEC_ID);
>
> pci_find_vsec_capability() finds a Vendor-Specific Extended Capability defined by
> the vendor of the device (Xilinx in this case).
>
> pci_find_vsec_capability() already checks that dev->vendor matches the vendor
> argument so you don't need the "vendor != PCI_VENDOR_ID_XILINX"
> check above.
>
> DW_PCIE_AMD_MDB_VSEC_ID should include "XILINX" because this ID is in the
> namespace created by PCI_VENDOR_ID_XILINX, i.e., it's somewhere in the
> (PCI_VENDOR_ID_XILINX, x) space.
>
> This code should look something like this (you could add "MDB" or something if it
> makes sense):
>
>   #define PCI_VSEC_ID_XILINX_DMA_DATA               0x20
>

Thank you for the suggestion, will go with the DW_PCIE_XILINX_MDB_VSEC_ID
similar to the format already been used in the code.

>   vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_XILINX,
>                                   PCI_VSEC_ID_XILINX_DMA_DATA);
>
> > +     if (!vsec)
> > +             return;
> > +
> > +     pci_read_config_dword(pdev, vsec + PCI_VNDR_HEADER, &val);
> > +     if (PCI_VNDR_HEADER_ID(val) != 0x20 ||
> > +         PCI_VNDR_HEADER_REV(val) != 0x1)
> > +             return;
> > +
> > +     pci_read_config_dword(pdev, vsec + 0xc, &val);
> > +     off = val;
> > +     pci_read_config_dword(pdev, vsec + 0x8, &val);
> > +     off <<= 32;
> > +     off |= val;
> > +     pdata->phys_addr = off;
> >  }
> >
> >  static int dw_edma_pcie_probe(struct pci_dev *pdev, @@ -179,6 +246,7
> > @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >       }
> >
> >       memcpy(vsec_data, pdata, sizeof(struct dw_edma_pcie_data));
> > +     vsec_data->phys_addr = DW_PCIE_AMD_MDB_INVALID_ADDR;
> >
> >       /*
> >        * Tries to find if exists a PCIe Vendor-Specific Extended
> > Capability @@ -186,6 +254,15 @@ static int dw_edma_pcie_probe(struct pci_dev
> *pdev,
> >        */
> >       dw_edma_pcie_get_vsec_dma_data(pdev, vsec_data);
> >
> > +     if (pdev->vendor == PCI_VENDOR_ID_XILINX) {
> > +             /*
> > +              * There is no valid address found for the LL memory
> > +              * space on the device side.
> > +              */
> > +             if (vsec_data->phys_addr == DW_PCIE_AMD_MDB_INVALID_ADDR)
> > +                     return -EINVAL;
> > +     }
> > +
> >       /* Mapping PCI BAR regions */
> >       mask = BIT(vsec_data->rg.bar);
> >       for (i = 0; i < vsec_data->wr_ch_cnt; i++) { @@ -367,6 +444,8 @@
> > static void dw_edma_pcie_remove(struct pci_dev *pdev)
> >
> >  static const struct pci_device_id dw_edma_pcie_id_table[] = {
> >       { PCI_DEVICE_DATA(SYNOPSYS, EDDA, &snps_edda_data) },
> > +     { PCI_VDEVICE(XILINX, PCI_DEVICE_ID_AMD_MDB_B054),
> > +       (kernel_ulong_t)&amd_mdb_data },
> >       { }
> >  };
> >  MODULE_DEVICE_TABLE(pci, dw_edma_pcie_id_table); diff --git
> > a/include/linux/pci_ids.h b/include/linux/pci_ids.h index
> > 92ffc43..c15607d 100644
> > --- a/include/linux/pci_ids.h
> > +++ b/include/linux/pci_ids.h
> > @@ -636,6 +636,7 @@
> >  #define PCI_DEVICE_ID_AMD_HUDSON2_SMBUS              0x780b
> >  #define PCI_DEVICE_ID_AMD_HUDSON2_IDE                0x780c
> >  #define PCI_DEVICE_ID_AMD_KERNCZ_SMBUS  0x790b
> > +#define PCI_DEVICE_ID_AMD_MDB_B054   0xb054
>
> Unless PCI_DEVICE_ID_AMD_MDB_B054 is used in more than one driver, move
> the #define into the file that uses it.  See the note at the top of pci_ids.h.

Agreed. Will move this to the file where it is being used as no other driver is using it now.

RE: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

[Verma, Devendra]

[AMD Official Use Only - AMD Internal Distribution Only]

Thank you Bjorn for reviewing the patch.
Please check my response inline.

Regards,
Devendra

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Saturday, September 6, 2025 00:36
> To: Verma, Devendra <Devendra.Verma@amd.com>
> Cc: bhelgaas@google.com; mani@kernel.org; vkoul@kernel.org;
> dmaengine@vger.kernel.org; linux-pci@vger.kernel.org; linux-
> kernel@vger.kernel.org; Simek, Michal <michal.simek@amd.com>
> Subject: Re: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Fri, Sep 05, 2025 at 03:46:59PM +0530, Devendra K Verma wrote:
> > AMD MDB IP supports Linked List (LL) mode as well as non-LL mode.
> > The current code does not have the mechanisms to enable the DMA
> > transactions using the non-LL mode. The following two cases are added
> > with this patch:
> > - When a valid physical base address is not configured via the
> >   Xilinx VSEC capability then the IP can still be used in non-LL
> >   mode. The default mode for all the DMA transactions and for all
> >   the DMA channels then is non-LL mode.
> > - When a valid physical base address is configured but the client
> >   wants to use the non-LL mode for DMA transactions then also the
> >   flexibility is provided via the peripheral_config struct member of
> >   dma_slave_config. In this case the channels can be individually
> >   configured in non-LL mode. This use case is desirable for single
> >   DMA transfer of a chunk, this saves the effort of preparing the
> >   Link List.
>
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -223,8 +223,28 @@ static int dw_edma_device_config(struct dma_chan
> *dchan,
> >                                struct dma_slave_config *config)  {
> >       struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > +     int nollp = 0;
> > +
> > +     if (WARN_ON(config->peripheral_config &&
> > +                 config->peripheral_size != sizeof(int)))
> > +             return -EINVAL;
> >
> >       memcpy(&chan->config, config, sizeof(*config));
> > +
> > +     /*
> > +      * When there is no valid LLP base address available
> > +      * then the default DMA ops will use the non-LL mode.
> > +      * Cases where LL mode is enabled and client wants
> > +      * to use the non-LL mode then also client can do
> > +      * so via the providing the peripheral_config param.
>
> s/via the/via/
>
> > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> > @@ -224,6 +224,15 @@ static void dw_edma_pcie_get_vsec_dma_data(struct
> pci_dev *pdev,
> >       pdata->phys_addr = off;
> >  }
> >
> > +static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
> > +                              struct dw_edma_pcie_data *pdata,
> > +                              enum pci_barno bar) {
> > +     if (pdev->vendor == PCI_VENDOR_ID_XILINX)
> > +             return pdata->phys_addr;
> > +     return pci_bus_address(pdev, bar);
>
> This doesn't seem right.  pci_bus_address() returns pci_bus_addr_t, so
> pdata->phys_addr should also be a pci_bus_addr_t, and the function
> should return pci_bus_addr_t.
>
> A pci_bus_addr_t is not a "phys_addr"; it is an address that is valid on the PCI side
> of a PCI host bridge, which may be different than the CPU physical address on the
> CPU side of the bridge because of things like IOMMUs.
>
> Seems like the struct dw_edma_region.paddr should be renamed to something like
> "bus_addr" and made into a pci_bus_addr_t.
>
> Bjorn

In case of AMD, it is not an address that is accessible from host via PCI, it is the device
side DDR offset of physical address which is not known to host,that is why the VSEC capability
is used to let know host of the DDR offset to correctly programming the LLP of DMA controller.
Without programming the LLP controller will not know from where to start reading the LL for
DMA processing. DMA controller requires the physical address of LL present on its side of DDR.

Re: [PATCH 1/2] dmaengine: dw-edma: Add AMD MDB Endpoint Support

[Bjorn Helgaas]

On Wed, Sep 10, 2025 at 12:28:40PM +0000, Verma, Devendra wrote:
> > From: Bjorn Helgaas <helgaas@kernel.org>

[redundant quoted headers removed]

> > On Fri, Sep 05, 2025 at 03:46:58PM +0530, Devendra K Verma wrote:
> > > AMD MDB PCIe endpoint support. For AMD specific support added the
> > > following
> > >   - AMD supported PCIe Device IDs and Vendor ID (Xilinx).
> > >   - AMD MDB specific driver data
> > >   - AMD MDB specific VSEC capability to retrieve the device DDR
> > >     base address.

> > > static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
> > >       u32 val, map;
> > >       u16 vsec;
> > >       u64 off;
> > > +     u16 vendor;
> > >
> > > -     vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_SYNOPSYS,
> > > -                                     DW_PCIE_VSEC_DMA_ID);
> > > +     vendor = pdev->vendor;
> > > +     if (vendor != PCI_VENDOR_ID_SYNOPSYS &&
> > > +         vendor != PCI_VENDOR_ID_XILINX)
> > > +             return;
> > > +
> > > +     vsec = pci_find_vsec_capability(pdev, vendor,
> > > + DW_PCIE_VSEC_DMA_ID);
> >
> > The vendor of a device assigns VSEC IDs and determines what each
> > ID means, so the semantics of a VSEC Capability are determined by
> > the tuple of (device Vendor ID, VSEC ID), where the Vendor ID is
> > the value at 0x00 in config space.
> 
> As AMD is a vendor for this device, it is determined as VSEC
> capability to support some of the functionality not supported by the
> other vendor Synopsys.

Based on your code, the vendor of this device (the value at 0x00 in
config space) is either PCI_VENDOR_ID_SYNOPSYS or
PCI_VENDOR_ID_XILINX.  Whoever controls those Vendor IDs also
controls the VSEC ID allocations for those Vendor IDs.

> > The DVSEC Capability is a more generic solution to this problem.
> > The VSEC ID namespace is determined by the Vendor ID of the
> > *device*.
> >
> > By contrast, the DVSEC ID namespace is determined by a Vendor ID
> > in the DVSEC Capability itself, not by the Vendor ID of the
> > device.
> >
> > So AMD could define a DVSEC ID, e.g., 6, and define the semantics
> > of that DVSEC.  Then devices from *any* vendor could include a
> > DVSEC Capability with (PCI_VENDOR_ID_AMD, 6), and generic code
> > could look for that DVSEC independent of what is at 0x00 in config
> > space.
> 
> As AMD itself becomes the vendor for this device, VSEC capability is
> chosen to support the functionality missing in the code.

For VSEC, it doesn't matter who *sells* the device or who supplies IP
contained in the device.  What matters is the Vendor ID at 0x00 in
config space.

Bjorn

Re: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

[Bjorn Helgaas]

On Wed, Sep 10, 2025 at 12:30:39PM +0000, Verma, Devendra wrote:
> > From: Bjorn Helgaas <helgaas@kernel.org>

[redundant headers removed]

> > On Fri, Sep 05, 2025 at 03:46:59PM +0530, Devendra K Verma wrote:
> > > AMD MDB IP supports Linked List (LL) mode as well as non-LL mode.
> > > The current code does not have the mechanisms to enable the DMA
> > > transactions using the non-LL mode. The following two cases are added
> > > with this patch:

> > > +static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
> > > +                              struct dw_edma_pcie_data *pdata,
> > > +                              enum pci_barno bar) {
> > > +     if (pdev->vendor == PCI_VENDOR_ID_XILINX)
> > > +             return pdata->phys_addr;
> > > +     return pci_bus_address(pdev, bar);
> >
> > This doesn't seem right.  pci_bus_address() returns
> > pci_bus_addr_t, so pdata->phys_addr should also be a
> > pci_bus_addr_t, and the function should return pci_bus_addr_t.
> >
> > A pci_bus_addr_t is not a "phys_addr"; it is an address that is
> > valid on the PCI side of a PCI host bridge, which may be different
> > than the CPU physical address on the CPU side of the bridge
> > because of things like IOMMUs.
> >
> > Seems like the struct dw_edma_region.paddr should be renamed to
> > something like "bus_addr" and made into a pci_bus_addr_t.
> 
> In case of AMD, it is not an address that is accessible from host
> via PCI, it is the device side DDR offset of physical address which
> is not known to host,that is why the VSEC capability is used to let
> know host of the DDR offset to correctly programming the LLP of DMA
> controller.  Without programming the LLP controller will not know
> from where to start reading the LL for DMA processing. DMA
> controller requires the physical address of LL present on its side
> of DDR.

I guess "device side DDR offset" means this Xilinx device has some DDR
internal to the PCI device, and the CPU cannot access it via a BAR?

But you need addresses inside that device DDR even though the CPU
can't access it, and the VSEC gives you the base address of the DDR?

This makes me wonder about how dw_edma_region is used elsewhere
because some of those places seem like they assume the CPU *can*
access this area.

dw_pcie_edma_ll_alloc() uses dmam_alloc_coherent(), which allocates
RAM and gives you a CPU virtual address (ll->vaddr.mem) and a DMA
address (ll->paddr).  dw_edma_pcie_probe() will later overwrite the
ll->paddr with the DDR offset based on VSEC.

But it sounds like ll->vaddr.mem is useless for Xilinx devices since
the CPU can't access the DDR?

If the CPU can't use ll->vaddr.mem, what happens in places like
dw_hdma_v0_write_ll_data() where we access it?

Bjorn

RE: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

[Verma, Devendra]

[AMD Official Use Only - AMD Internal Distribution Only]

Thank you Bjorn for your inputs!
Please check my comments inline.

Regards,
Devendra

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Wednesday, September 10, 2025 23:27
> To: Verma, Devendra <Devendra.Verma@amd.com>
> Cc: bhelgaas@google.com; mani@kernel.org; vkoul@kernel.org;
> dmaengine@vger.kernel.org; linux-pci@vger.kernel.org; linux-
> kernel@vger.kernel.org; Simek, Michal <michal.simek@amd.com>
> Subject: Re: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Wed, Sep 10, 2025 at 12:30:39PM +0000, Verma, Devendra wrote:
> > > From: Bjorn Helgaas <helgaas@kernel.org>
>
> [redundant headers removed]
>
> > > On Fri, Sep 05, 2025 at 03:46:59PM +0530, Devendra K Verma wrote:
> > > > AMD MDB IP supports Linked List (LL) mode as well as non-LL mode.
> > > > The current code does not have the mechanisms to enable the DMA
> > > > transactions using the non-LL mode. The following two cases are
> > > > added with this patch:
>
> > > > +static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
> > > > +                              struct dw_edma_pcie_data *pdata,
> > > > +                              enum pci_barno bar) {
> > > > +     if (pdev->vendor == PCI_VENDOR_ID_XILINX)
> > > > +             return pdata->phys_addr;
> > > > +     return pci_bus_address(pdev, bar);
> > >
> > > This doesn't seem right.  pci_bus_address() returns pci_bus_addr_t,
> > > so pdata->phys_addr should also be a pci_bus_addr_t, and the
> > > function should return pci_bus_addr_t.
> > >
> > > A pci_bus_addr_t is not a "phys_addr"; it is an address that is
> > > valid on the PCI side of a PCI host bridge, which may be different
> > > than the CPU physical address on the CPU side of the bridge because
> > > of things like IOMMUs.
> > >
> > > Seems like the struct dw_edma_region.paddr should be renamed to
> > > something like "bus_addr" and made into a pci_bus_addr_t.
> >
> > In case of AMD, it is not an address that is accessible from host via
> > PCI, it is the device side DDR offset of physical address which is not
> > known to host,that is why the VSEC capability is used to let know host
> > of the DDR offset to correctly programming the LLP of DMA controller.
> > Without programming the LLP controller will not know from where to
> > start reading the LL for DMA processing. DMA controller requires the
> > physical address of LL present on its side of DDR.
>
> I guess "device side DDR offset" means this Xilinx device has some DDR internal to
> the PCI device, and the CPU cannot access it via a BAR?
>

The host can access the DDR internal to the PCI device via BAR, but it involves
an iATU translation. The host can use the virtual / physical address to access that DDR.
The issue is not with the host rather DMA controller which does not understand the
physical address provided by the host, eg, the address returned
by pci_bus_addr(pdev, barno).

> But you need addresses inside that device DDR even though the CPU can't access
> it, and the VSEC gives you the base address of the DDR?
>
> This makes me wonder about how dw_edma_region is used elsewhere because
> some of those places seem like they assume the CPU *can* access this area.
>
> dw_pcie_edma_ll_alloc() uses dmam_alloc_coherent(), which allocates RAM and
> gives you a CPU virtual address (ll->vaddr.mem) and a DMA address (ll->paddr).
> dw_edma_pcie_probe() will later overwrite the
> ll->paddr with the DDR offset based on VSEC.
>
> But it sounds like ll->vaddr.mem is useless for Xilinx devices since the CPU can't
> access the DDR?
>
> If the CPU can't use ll->vaddr.mem, what happens in places like
> dw_hdma_v0_write_ll_data() where we access it?
>
> Bjorn

As explained above, the host can access the device side internal DDR via one of
the BARs but involves some translation (iATU) configuration on the device side.
The issue is with the DMA controller which does not understand the host side
virt / phys addresses thus need to be programmed correctly to the device side
DDR offset which is a valid physical address on the device side. Host needs the
Information of the device side DDR offset to program the LLP register using which
DMA controller starts reading the DDR on its side.
Hope this clarifies.

Re: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

[Bjorn Helgaas]

On Thu, Sep 11, 2025 at 11:42:31AM +0000, Verma, Devendra wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@kernel.org>

> > On Wed, Sep 10, 2025 at 12:30:39PM +0000, Verma, Devendra wrote:
> > > > From: Bjorn Helgaas <helgaas@kernel.org>
> >
> > [redundant headers removed]
> >
> > > > On Fri, Sep 05, 2025 at 03:46:59PM +0530, Devendra K Verma wrote:
> > > > > AMD MDB IP supports Linked List (LL) mode as well as non-LL mode.
> > > > > The current code does not have the mechanisms to enable the DMA
> > > > > transactions using the non-LL mode. The following two cases are
> > > > > added with this patch:
> >
> > > > > +static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
> > > > > +                              struct dw_edma_pcie_data *pdata,
> > > > > +                              enum pci_barno bar) {
> > > > > +     if (pdev->vendor == PCI_VENDOR_ID_XILINX)
> > > > > +             return pdata->phys_addr;
> > > > > +     return pci_bus_address(pdev, bar);
> > > >
> > > > This doesn't seem right.  pci_bus_address() returns pci_bus_addr_t,
> > > > so pdata->phys_addr should also be a pci_bus_addr_t, and the
> > > > function should return pci_bus_addr_t.
> > > >
> > > > A pci_bus_addr_t is not a "phys_addr"; it is an address that is
> > > > valid on the PCI side of a PCI host bridge, which may be different
> > > > than the CPU physical address on the CPU side of the bridge because
> > > > of things like IOMMUs.
> > > >
> > > > Seems like the struct dw_edma_region.paddr should be renamed to
> > > > something like "bus_addr" and made into a pci_bus_addr_t.
> > >
> > > In case of AMD, it is not an address that is accessible from host via
> > > PCI, it is the device side DDR offset of physical address which is not
> > > known to host,that is why the VSEC capability is used to let know host
> > > of the DDR offset to correctly programming the LLP of DMA controller.
> > > Without programming the LLP controller will not know from where to
> > > start reading the LL for DMA processing. DMA controller requires the
> > > physical address of LL present on its side of DDR.
> >
> > I guess "device side DDR offset" means this Xilinx device has some
> > DDR internal to the PCI device, and the CPU cannot access it via a
> > BAR?
> 
> The host can access the DDR internal to the PCI device via BAR, but
> it involves an iATU translation. The host can use the virtual /
> physical address to access that DDR.  The issue is not with the host
> rather DMA controller which does not understand the physical address
> provided by the host, eg, the address returned by pci_bus_addr(pdev,
> barno).

Does this mean dw_edma_get_phys_addr() depends on iATU programming
done by the PCI controller driver?  Is it possible for that driver to
change the iATU programming after dw_edma_get_phys_addr() in a way
that breaks this?

Bjorn

RE: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode

[Verma, Devendra]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Bjorn

Please check my response inline.
Regards,
Devendra

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Friday, September 12, 2025 02:14
> To: Verma, Devendra <Devendra.Verma@amd.com>
> Cc: bhelgaas@google.com; mani@kernel.org; vkoul@kernel.org;
> dmaengine@vger.kernel.org; linux-pci@vger.kernel.org; linux-
> kernel@vger.kernel.org; Simek, Michal <michal.simek@amd.com>
> Subject: Re: [PATCH 2/2] dmaengine: dw-edma: Add non-LL mode
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Thu, Sep 11, 2025 at 11:42:31AM +0000, Verma, Devendra wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@kernel.org>
>
> > > On Wed, Sep 10, 2025 at 12:30:39PM +0000, Verma, Devendra wrote:
> > > > > From: Bjorn Helgaas <helgaas@kernel.org>
> > >
> > > [redundant headers removed]
> > >
> > > > > On Fri, Sep 05, 2025 at 03:46:59PM +0530, Devendra K Verma wrote:
> > > > > > AMD MDB IP supports Linked List (LL) mode as well as non-LL mode.
> > > > > > The current code does not have the mechanisms to enable the
> > > > > > DMA transactions using the non-LL mode. The following two
> > > > > > cases are added with this patch:
> > >
> > > > > > +static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
> > > > > > +                              struct dw_edma_pcie_data *pdata,
> > > > > > +                              enum pci_barno bar) {
> > > > > > +     if (pdev->vendor == PCI_VENDOR_ID_XILINX)
> > > > > > +             return pdata->phys_addr;
> > > > > > +     return pci_bus_address(pdev, bar);
> > > > >
> > > > > This doesn't seem right.  pci_bus_address() returns
> > > > > pci_bus_addr_t, so pdata->phys_addr should also be a
> > > > > pci_bus_addr_t, and the function should return pci_bus_addr_t.
> > > > >
> > > > > A pci_bus_addr_t is not a "phys_addr"; it is an address that is
> > > > > valid on the PCI side of a PCI host bridge, which may be
> > > > > different than the CPU physical address on the CPU side of the
> > > > > bridge because of things like IOMMUs.
> > > > >
> > > > > Seems like the struct dw_edma_region.paddr should be renamed to
> > > > > something like "bus_addr" and made into a pci_bus_addr_t.
> > > >
> > > > In case of AMD, it is not an address that is accessible from host
> > > > via PCI, it is the device side DDR offset of physical address
> > > > which is not known to host,that is why the VSEC capability is used
> > > > to let know host of the DDR offset to correctly programming the LLP of DMA
> controller.
> > > > Without programming the LLP controller will not know from where to
> > > > start reading the LL for DMA processing. DMA controller requires
> > > > the physical address of LL present on its side of DDR.
> > >
> > > I guess "device side DDR offset" means this Xilinx device has some
> > > DDR internal to the PCI device, and the CPU cannot access it via a
> > > BAR?
> >
> > The host can access the DDR internal to the PCI device via BAR, but it
> > involves an iATU translation. The host can use the virtual / physical
> > address to access that DDR.  The issue is not with the host rather DMA
> > controller which does not understand the physical address provided by
> > the host, eg, the address returned by pci_bus_addr(pdev, barno).
>
> Does this mean dw_edma_get_phys_addr() depends on iATU programming done by
> the PCI controller driver?  Is it possible for that driver to change the iATU
> programming after dw_edma_get_phys_addr() in a way that breaks this?
>

No, driver can neither program nor modify the iATU. Once device is configured using
AMD Programmable Device Image (PDI) which contains these configurations.
Once programmed the configuration can not be changed by the driver.

> Bjorn