All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/6] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally
@ 2022-03-07 16:24 Frank Li
  2022-03-07 16:24 ` [PATCH v3 2/6] dmaengine: dw-edma-pcie: don't touch internal struct dw_edma Frank Li
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Frank Li @ 2022-03-07 16:24 UTC (permalink / raw)
  To: gustavo.pimentel, hongxing.zhu, l.stach, linux-imx, linux-pci, lznuaa
  Cc: vkoul, lorenzo.pieralisi, robh, kw, bhelgaas, shawnguo,
	manivannan.sadhasivam

"struct dw_edma_chip" contains an internal structure "struct dw_edma" that is
used by the eDMA core internally. This structure should not be touched
by the eDMA controller drivers themselves. But currently, the eDMA
controller drivers like "dw-edma-pci" allocates and populates this
internal structure then passes it on to eDMA core. The eDMA core further
populates the structure and uses it. This is wrong!

Hence, move all the "struct dw_edma" specifics from controller drivers
to the eDMA core.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v2 to v3
 - none
Change from v1 to v2
 - rework commit message
 - remove duplicate field in struct dw_edma

 drivers/dma/dw-edma/dw-edma-core.c       |  91 +++++-----
 drivers/dma/dw-edma/dw-edma-core.h       |  30 +---
 drivers/dma/dw-edma/dw-edma-v0-core.c    | 206 ++++++++++++-----------
 drivers/dma/dw-edma/dw-edma-v0-core.h    |   8 +-
 drivers/dma/dw-edma/dw-edma-v0-debugfs.c |  37 ++--
 include/linux/dma/edma.h                 |  47 +++++-
 6 files changed, 230 insertions(+), 189 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 53289927dd0d6..0cb66434f9e14 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -65,7 +65,7 @@ static struct dw_edma_burst *dw_edma_alloc_burst(struct dw_edma_chunk *chunk)
 static struct dw_edma_chunk *dw_edma_alloc_chunk(struct dw_edma_desc *desc)
 {
 	struct dw_edma_chan *chan = desc->chan;
-	struct dw_edma *dw = chan->chip->dw;
+	struct dw_edma_chip *chip = chan->chip;
 	struct dw_edma_chunk *chunk;
 
 	chunk = kzalloc(sizeof(*chunk), GFP_NOWAIT);
@@ -82,11 +82,11 @@ static struct dw_edma_chunk *dw_edma_alloc_chunk(struct dw_edma_desc *desc)
 	 */
 	chunk->cb = !(desc->chunks_alloc % 2);
 	if (chan->dir == EDMA_DIR_WRITE) {
-		chunk->ll_region.paddr = dw->ll_region_wr[chan->id].paddr;
-		chunk->ll_region.vaddr = dw->ll_region_wr[chan->id].vaddr;
+		chunk->ll_region.paddr = chip->ll_region_wr[chan->id].paddr;
+		chunk->ll_region.vaddr = chip->ll_region_wr[chan->id].vaddr;
 	} else {
-		chunk->ll_region.paddr = dw->ll_region_rd[chan->id].paddr;
-		chunk->ll_region.vaddr = dw->ll_region_rd[chan->id].vaddr;
+		chunk->ll_region.paddr = chip->ll_region_rd[chan->id].paddr;
+		chunk->ll_region.vaddr = chip->ll_region_rd[chan->id].vaddr;
 	}
 
 	if (desc->chunk) {
@@ -601,7 +601,8 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
 static irqreturn_t dw_edma_interrupt(int irq, void *data, bool write)
 {
 	struct dw_edma_irq *dw_irq = data;
-	struct dw_edma *dw = dw_irq->dw;
+	struct dw_edma_chip *chip = dw_irq->chip;
+	struct dw_edma *dw = chip->dw;
 	unsigned long total, pos, val;
 	unsigned long off;
 	u32 mask;
@@ -616,7 +617,7 @@ static irqreturn_t dw_edma_interrupt(int irq, void *data, bool write)
 		mask = dw_irq->rd_mask;
 	}
 
-	val = dw_edma_v0_core_status_done_int(dw, write ?
+	val = dw_edma_v0_core_status_done_int(chip, write ?
 							  EDMA_DIR_WRITE :
 							  EDMA_DIR_READ);
 	val &= mask;
@@ -626,7 +627,7 @@ static irqreturn_t dw_edma_interrupt(int irq, void *data, bool write)
 		dw_edma_done_interrupt(chan);
 	}
 
-	val = dw_edma_v0_core_status_abort_int(dw, write ?
+	val = dw_edma_v0_core_status_abort_int(chip, write ?
 							   EDMA_DIR_WRITE :
 							   EDMA_DIR_READ);
 	val &= mask;
@@ -718,7 +719,7 @@ static int dw_edma_channel_setup(struct dw_edma_chip *chip, bool write,
 	}
 
 	INIT_LIST_HEAD(&dma->channels);
-	for (j = 0; (alloc || dw->nr_irqs == 1) && j < cnt; j++, i++) {
+	for (j = 0; (alloc || chip->nr_irqs == 1) && j < cnt; j++, i++) {
 		chan = &dw->chan[i];
 
 		dt_region = devm_kzalloc(dev, sizeof(*dt_region), GFP_KERNEL);
@@ -735,15 +736,15 @@ static int dw_edma_channel_setup(struct dw_edma_chip *chip, bool write,
 		chan->status = EDMA_ST_IDLE;
 
 		if (write)
-			chan->ll_max = (dw->ll_region_wr[j].sz / EDMA_LL_SZ);
+			chan->ll_max = (chip->ll_region_wr[j].sz / EDMA_LL_SZ);
 		else
-			chan->ll_max = (dw->ll_region_rd[j].sz / EDMA_LL_SZ);
+			chan->ll_max = (chip->ll_region_rd[j].sz / EDMA_LL_SZ);
 		chan->ll_max -= 1;
 
 		dev_vdbg(dev, "L. List:\tChannel %s[%u] max_cnt=%u\n",
 			 write ? "write" : "read", j, chan->ll_max);
 
-		if (dw->nr_irqs == 1)
+		if (chip->nr_irqs == 1)
 			pos = 0;
 		else
 			pos = off_alloc + (j % alloc);
@@ -755,7 +756,7 @@ static int dw_edma_channel_setup(struct dw_edma_chip *chip, bool write,
 		else
 			irq->rd_mask |= BIT(j);
 
-		irq->dw = dw;
+		irq->chip = chip;
 		memcpy(&chan->msi, &irq->msi, sizeof(chan->msi));
 
 		dev_vdbg(dev, "MSI:\t\tChannel %s[%u] addr=0x%.8x%.8x, data=0x%.8x\n",
@@ -767,13 +768,13 @@ static int dw_edma_channel_setup(struct dw_edma_chip *chip, bool write,
 		vchan_init(&chan->vc, dma);
 
 		if (write) {
-			dt_region->paddr = dw->dt_region_wr[j].paddr;
-			dt_region->vaddr = dw->dt_region_wr[j].vaddr;
-			dt_region->sz = dw->dt_region_wr[j].sz;
+			dt_region->paddr = chip->dt_region_wr[j].paddr;
+			dt_region->vaddr = chip->dt_region_wr[j].vaddr;
+			dt_region->sz = chip->dt_region_wr[j].sz;
 		} else {
-			dt_region->paddr = dw->dt_region_rd[j].paddr;
-			dt_region->vaddr = dw->dt_region_rd[j].vaddr;
-			dt_region->sz = dw->dt_region_rd[j].sz;
+			dt_region->paddr = chip->dt_region_rd[j].paddr;
+			dt_region->vaddr = chip->dt_region_rd[j].vaddr;
+			dt_region->sz = chip->dt_region_rd[j].sz;
 		}
 
 		dw_edma_v0_core_device_config(chan);
@@ -840,16 +841,16 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
 
 	ch_cnt = dw->wr_ch_cnt + dw->rd_ch_cnt;
 
-	if (dw->nr_irqs < 1)
+	if (chip->nr_irqs < 1)
 		return -EINVAL;
 
-	if (dw->nr_irqs == 1) {
+	if (chip->nr_irqs == 1) {
 		/* Common IRQ shared among all channels */
-		irq = dw->ops->irq_vector(dev, 0);
+		irq = chip->ops->irq_vector(dev, 0);
 		err = request_irq(irq, dw_edma_interrupt_common,
 				  IRQF_SHARED, dw->name, &dw->irq[0]);
 		if (err) {
-			dw->nr_irqs = 0;
+			chip->nr_irqs = 0;
 			return err;
 		}
 
@@ -857,7 +858,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
 			get_cached_msi_msg(irq, &dw->irq[0].msi);
 	} else {
 		/* Distribute IRQs equally among all channels */
-		int tmp = dw->nr_irqs;
+		int tmp = chip->nr_irqs;
 
 		while (tmp && (*wr_alloc + *rd_alloc) < ch_cnt) {
 			dw_edma_dec_irq_alloc(&tmp, wr_alloc, dw->wr_ch_cnt);
@@ -868,7 +869,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
 		dw_edma_add_irq_mask(&rd_mask, *rd_alloc, dw->rd_ch_cnt);
 
 		for (i = 0; i < (*wr_alloc + *rd_alloc); i++) {
-			irq = dw->ops->irq_vector(dev, i);
+			irq = chip->ops->irq_vector(dev, i);
 			err = request_irq(irq,
 					  i < *wr_alloc ?
 						dw_edma_interrupt_write :
@@ -876,7 +877,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
 					  IRQF_SHARED, dw->name,
 					  &dw->irq[i]);
 			if (err) {
-				dw->nr_irqs = i;
+				chip->nr_irqs = i;
 				return err;
 			}
 
@@ -884,7 +885,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
 				get_cached_msi_msg(irq, &dw->irq[i].msi);
 		}
 
-		dw->nr_irqs = i;
+		chip->nr_irqs = i;
 	}
 
 	return err;
@@ -905,18 +906,24 @@ int dw_edma_probe(struct dw_edma_chip *chip)
 	if (!dev)
 		return -EINVAL;
 
-	dw = chip->dw;
-	if (!dw || !dw->irq || !dw->ops || !dw->ops->irq_vector)
+	dw = devm_kzalloc(dev, sizeof(*dw), GFP_KERNEL);
+	if (!dw)
+		return -ENOMEM;
+
+	chip->dw = dw;
+
+	if (!chip->nr_irqs || !chip->ops)
 		return -EINVAL;
 
 	raw_spin_lock_init(&dw->lock);
 
-	dw->wr_ch_cnt = min_t(u16, dw->wr_ch_cnt,
-			      dw_edma_v0_core_ch_count(dw, EDMA_DIR_WRITE));
+
+	dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt,
+			      dw_edma_v0_core_ch_count(chip, EDMA_DIR_WRITE));
 	dw->wr_ch_cnt = min_t(u16, dw->wr_ch_cnt, EDMA_MAX_WR_CH);
 
-	dw->rd_ch_cnt = min_t(u16, dw->rd_ch_cnt,
-			      dw_edma_v0_core_ch_count(dw, EDMA_DIR_READ));
+	dw->rd_ch_cnt = min_t(u16, chip->ll_rd_cnt,
+			      dw_edma_v0_core_ch_count(chip, EDMA_DIR_READ));
 	dw->rd_ch_cnt = min_t(u16, dw->rd_ch_cnt, EDMA_MAX_RD_CH);
 
 	if (!dw->wr_ch_cnt && !dw->rd_ch_cnt)
@@ -934,7 +941,11 @@ int dw_edma_probe(struct dw_edma_chip *chip)
 	snprintf(dw->name, sizeof(dw->name), "dw-edma-core:%d", chip->id);
 
 	/* Disable eDMA, only to establish the ideal initial conditions */
-	dw_edma_v0_core_off(dw);
+	dw_edma_v0_core_off(chip);
+
+	dw->irq = devm_kcalloc(dev, chip->nr_irqs, sizeof(*dw->irq), GFP_KERNEL);
+	if (!dw->irq)
+		return -ENOMEM;
 
 	/* Request IRQs */
 	err = dw_edma_irq_request(chip, &wr_alloc, &rd_alloc);
@@ -960,10 +971,10 @@ int dw_edma_probe(struct dw_edma_chip *chip)
 	return 0;
 
 err_irq_free:
-	for (i = (dw->nr_irqs - 1); i >= 0; i--)
-		free_irq(dw->ops->irq_vector(dev, i), &dw->irq[i]);
+	for (i = (chip->nr_irqs - 1); i >= 0; i--)
+		free_irq(chip->ops->irq_vector(dev, i), &dw->irq[i]);
 
-	dw->nr_irqs = 0;
+	chip->nr_irqs = 0;
 
 	return err;
 }
@@ -977,11 +988,11 @@ int dw_edma_remove(struct dw_edma_chip *chip)
 	int i;
 
 	/* Disable eDMA */
-	dw_edma_v0_core_off(dw);
+	dw_edma_v0_core_off(chip);
 
 	/* Free irqs */
-	for (i = (dw->nr_irqs - 1); i >= 0; i--)
-		free_irq(dw->ops->irq_vector(dev, i), &dw->irq[i]);
+	for (i = (chip->nr_irqs - 1); i >= 0; i--)
+		free_irq(chip->ops->irq_vector(dev, i), &dw->irq[i]);
 
 	/* Power management */
 	pm_runtime_disable(dev);
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 60316d408c3e0..885f6719c9462 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -15,20 +15,12 @@
 #include "../virt-dma.h"
 
 #define EDMA_LL_SZ					24
-#define EDMA_MAX_WR_CH					8
-#define EDMA_MAX_RD_CH					8
 
 enum dw_edma_dir {
 	EDMA_DIR_WRITE = 0,
 	EDMA_DIR_READ
 };
 
-enum dw_edma_map_format {
-	EDMA_MF_EDMA_LEGACY = 0x0,
-	EDMA_MF_EDMA_UNROLL = 0x1,
-	EDMA_MF_HDMA_COMPAT = 0x5
-};
-
 enum dw_edma_request {
 	EDMA_REQ_NONE = 0,
 	EDMA_REQ_STOP,
@@ -57,12 +49,6 @@ struct dw_edma_burst {
 	u32				sz;
 };
 
-struct dw_edma_region {
-	phys_addr_t			paddr;
-	void				__iomem *vaddr;
-	size_t				sz;
-};
-
 struct dw_edma_chunk {
 	struct list_head		list;
 	struct dw_edma_chan		*chan;
@@ -106,11 +92,7 @@ struct dw_edma_irq {
 	struct msi_msg                  msi;
 	u32				wr_mask;
 	u32				rd_mask;
-	struct dw_edma			*dw;
-};
-
-struct dw_edma_core_ops {
-	int	(*irq_vector)(struct device *dev, unsigned int nr);
+	struct dw_edma_chip		*chip;
 };
 
 struct dw_edma {
@@ -122,19 +104,9 @@ struct dw_edma {
 	struct dma_device		rd_edma;
 	u16				rd_ch_cnt;
 
-	struct dw_edma_region		rg_region;	/* Registers */
-	struct dw_edma_region		ll_region_wr[EDMA_MAX_WR_CH];
-	struct dw_edma_region		ll_region_rd[EDMA_MAX_RD_CH];
-	struct dw_edma_region		dt_region_wr[EDMA_MAX_WR_CH];
-	struct dw_edma_region		dt_region_rd[EDMA_MAX_RD_CH];
-
 	struct dw_edma_irq		*irq;
-	int				nr_irqs;
-
-	enum dw_edma_map_format		mf;
 
 	struct dw_edma_chan		*chan;
-	const struct dw_edma_core_ops	*ops;
 
 	raw_spinlock_t			lock;		/* Only for legacy */
 #ifdef CONFIG_DEBUG_FS
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index 329fc2e57b703..6e2f83e31a03a 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -23,92 +23,94 @@ enum dw_edma_control {
 	DW_EDMA_V0_LLE					= BIT(9),
 };
 
-static inline struct dw_edma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
+static inline struct dw_edma_v0_regs __iomem *__dw_regs(struct dw_edma_chip *chip)
 {
-	return dw->rg_region.vaddr;
+	return chip->reg_base;
 }
 
-#define SET_32(dw, name, value)				\
-	writel(value, &(__dw_regs(dw)->name))
+#define SET_32(chip, name, value)				\
+	writel(value, &(__dw_regs(chip)->name))
 
-#define GET_32(dw, name)				\
-	readl(&(__dw_regs(dw)->name))
+#define GET_32(chip, name)				\
+	readl(&(__dw_regs(chip)->name))
 
-#define SET_RW_32(dw, dir, name, value)			\
+#define SET_RW_32(chip, dir, name, value)			\
 	do {						\
 		if ((dir) == EDMA_DIR_WRITE)		\
-			SET_32(dw, wr_##name, value);	\
+			SET_32(chip, wr_##name, value);	\
 		else					\
-			SET_32(dw, rd_##name, value);	\
+			SET_32(chip, rd_##name, value);	\
 	} while (0)
 
-#define GET_RW_32(dw, dir, name)			\
+#define GET_RW_32(chip, dir, name)			\
 	((dir) == EDMA_DIR_WRITE			\
-	  ? GET_32(dw, wr_##name)			\
-	  : GET_32(dw, rd_##name))
+	  ? GET_32(chip, wr_##name)			\
+	  : GET_32(chip, rd_##name))
 
-#define SET_BOTH_32(dw, name, value)			\
+#define SET_BOTH_32(chip, name, value)			\
 	do {						\
-		SET_32(dw, wr_##name, value);		\
-		SET_32(dw, rd_##name, value);		\
+		SET_32(chip, wr_##name, value);		\
+		SET_32(chip, rd_##name, value);		\
 	} while (0)
 
 #ifdef CONFIG_64BIT
 
-#define SET_64(dw, name, value)				\
-	writeq(value, &(__dw_regs(dw)->name))
+#define SET_64(chip, name, value)				\
+	writeq(value, &(__dw_regs(chip)->name))
 
-#define GET_64(dw, name)				\
-	readq(&(__dw_regs(dw)->name))
+#define GET_64(chip, name)				\
+	readq(&(__dw_regs(chip)->name))
 
-#define SET_RW_64(dw, dir, name, value)			\
+#define SET_RW_64(chip, dir, name, value)			\
 	do {						\
 		if ((dir) == EDMA_DIR_WRITE)		\
-			SET_64(dw, wr_##name, value);	\
+			SET_64(chip, wr_##name, value);	\
 		else					\
-			SET_64(dw, rd_##name, value);	\
+			SET_64(chip, rd_##name, value);	\
 	} while (0)
 
-#define GET_RW_64(dw, dir, name)			\
+#define GET_RW_64(chip, dir, name)			\
 	((dir) == EDMA_DIR_WRITE			\
-	  ? GET_64(dw, wr_##name)			\
-	  : GET_64(dw, rd_##name))
+	  ? GET_64(chip, wr_##name)			\
+	  : GET_64(chip, rd_##name))
 
-#define SET_BOTH_64(dw, name, value)			\
+#define SET_BOTH_64(chip, name, value)			\
 	do {						\
-		SET_64(dw, wr_##name, value);		\
-		SET_64(dw, rd_##name, value);		\
+		SET_64(chip, wr_##name, value);		\
+		SET_64(chip, rd_##name, value);		\
 	} while (0)
 
 #endif /* CONFIG_64BIT */
 
-#define SET_COMPAT(dw, name, value)			\
-	writel(value, &(__dw_regs(dw)->type.unroll.name))
+#define SET_COMPAT(chip, name, value)			\
+	writel(value, &(__dw_regs(chip)->type.unroll.name))
 
-#define SET_RW_COMPAT(dw, dir, name, value)		\
+#define SET_RW_COMPAT(chip, dir, name, value)		\
 	do {						\
 		if ((dir) == EDMA_DIR_WRITE)		\
-			SET_COMPAT(dw, wr_##name, value); \
+			SET_COMPAT(chip, wr_##name, value); \
 		else					\
-			SET_COMPAT(dw, rd_##name, value); \
+			SET_COMPAT(chip, rd_##name, value); \
 	} while (0)
 
 static inline struct dw_edma_v0_ch_regs __iomem *
-__dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
+__dw_ch_regs(struct dw_edma_chip *chip, enum dw_edma_dir dir, u16 ch)
 {
-	if (dw->mf == EDMA_MF_EDMA_LEGACY)
-		return &(__dw_regs(dw)->type.legacy.ch);
+	if (chip->mf == EDMA_MF_EDMA_LEGACY)
+		return &(__dw_regs(chip)->type.legacy.ch);
 
 	if (dir == EDMA_DIR_WRITE)
-		return &__dw_regs(dw)->type.unroll.ch[ch].wr;
+		return &__dw_regs(chip)->type.unroll.ch[ch].wr;
 
-	return &__dw_regs(dw)->type.unroll.ch[ch].rd;
+	return &__dw_regs(chip)->type.unroll.ch[ch].rd;
 }
 
-static inline void writel_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
+static inline void writel_ch(struct dw_edma_chip *chip, enum dw_edma_dir dir, u16 ch,
 			     u32 value, void __iomem *addr)
 {
-	if (dw->mf == EDMA_MF_EDMA_LEGACY) {
+	struct dw_edma *dw = chip->dw;
+
+	if (chip->mf == EDMA_MF_EDMA_LEGACY) {
 		u32 viewport_sel;
 		unsigned long flags;
 
@@ -119,7 +121,7 @@ static inline void writel_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
 			viewport_sel |= BIT(31);
 
 		writel(viewport_sel,
-		       &(__dw_regs(dw)->type.legacy.viewport_sel));
+		       &(__dw_regs(chip)->type.legacy.viewport_sel));
 		writel(value, addr);
 
 		raw_spin_unlock_irqrestore(&dw->lock, flags);
@@ -128,12 +130,13 @@ static inline void writel_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
 	}
 }
 
-static inline u32 readl_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
+static inline u32 readl_ch(struct dw_edma_chip *chip, enum dw_edma_dir dir, u16 ch,
 			   const void __iomem *addr)
 {
+	struct dw_edma *dw = chip->dw;
 	u32 value;
 
-	if (dw->mf == EDMA_MF_EDMA_LEGACY) {
+	if (chip->mf == EDMA_MF_EDMA_LEGACY) {
 		u32 viewport_sel;
 		unsigned long flags;
 
@@ -144,7 +147,7 @@ static inline u32 readl_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
 			viewport_sel |= BIT(31);
 
 		writel(viewport_sel,
-		       &(__dw_regs(dw)->type.legacy.viewport_sel));
+		       &(__dw_regs(chip)->type.legacy.viewport_sel));
 		value = readl(addr);
 
 		raw_spin_unlock_irqrestore(&dw->lock, flags);
@@ -166,10 +169,12 @@ static inline u32 readl_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
 
 #ifdef CONFIG_64BIT
 
-static inline void writeq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
+static inline void writeq_ch(struct dw_edma_chip *chip, enum dw_edma_dir dir, u16 ch,
 			     u64 value, void __iomem *addr)
 {
-	if (dw->mf == EDMA_MF_EDMA_LEGACY) {
+	struct dw_edma *dw = chip->dw;
+
+	if (chip->mf == EDMA_MF_EDMA_LEGACY) {
 		u32 viewport_sel;
 		unsigned long flags;
 
@@ -180,7 +185,7 @@ static inline void writeq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
 			viewport_sel |= BIT(31);
 
 		writel(viewport_sel,
-		       &(__dw_regs(dw)->type.legacy.viewport_sel));
+		       &(__dw_regs(chip)->type.legacy.viewport_sel));
 		writeq(value, addr);
 
 		raw_spin_unlock_irqrestore(&dw->lock, flags);
@@ -189,12 +194,13 @@ static inline void writeq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
 	}
 }
 
-static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
+static inline u64 readq_ch(struct dw_edma_chip *chip, enum dw_edma_dir dir, u16 ch,
 			   const void __iomem *addr)
 {
+	struct dw_edma *dw = chip->dw;
 	u32 value;
 
-	if (dw->mf == EDMA_MF_EDMA_LEGACY) {
+	if (chip->mf == EDMA_MF_EDMA_LEGACY) {
 		u32 viewport_sel;
 		unsigned long flags;
 
@@ -205,7 +211,7 @@ static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
 			viewport_sel |= BIT(31);
 
 		writel(viewport_sel,
-		       &(__dw_regs(dw)->type.legacy.viewport_sel));
+		       &(__dw_regs(chip)->type.legacy.viewport_sel));
 		value = readq(addr);
 
 		raw_spin_unlock_irqrestore(&dw->lock, flags);
@@ -228,25 +234,25 @@ static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
 #endif /* CONFIG_64BIT */
 
 /* eDMA management callbacks */
-void dw_edma_v0_core_off(struct dw_edma *dw)
+void dw_edma_v0_core_off(struct dw_edma_chip *chip)
 {
-	SET_BOTH_32(dw, int_mask,
+	SET_BOTH_32(chip, int_mask,
 		    EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
-	SET_BOTH_32(dw, int_clear,
+	SET_BOTH_32(chip, int_clear,
 		    EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
-	SET_BOTH_32(dw, engine_en, 0);
+	SET_BOTH_32(chip, engine_en, 0);
 }
 
-u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
+u16 dw_edma_v0_core_ch_count(struct dw_edma_chip *chip, enum dw_edma_dir dir)
 {
 	u32 num_ch;
 
 	if (dir == EDMA_DIR_WRITE)
 		num_ch = FIELD_GET(EDMA_V0_WRITE_CH_COUNT_MASK,
-				   GET_32(dw, ctrl));
+				   GET_32(chip, ctrl));
 	else
 		num_ch = FIELD_GET(EDMA_V0_READ_CH_COUNT_MASK,
-				   GET_32(dw, ctrl));
+				   GET_32(chip, ctrl));
 
 	if (num_ch > EDMA_V0_MAX_NR_CH)
 		num_ch = EDMA_V0_MAX_NR_CH;
@@ -256,11 +262,11 @@ u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
 
 enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
 {
-	struct dw_edma *dw = chan->chip->dw;
+	struct dw_edma_chip *chip = chan->chip;
 	u32 tmp;
 
 	tmp = FIELD_GET(EDMA_V0_CH_STATUS_MASK,
-			GET_CH_32(dw, chan->dir, chan->id, ch_control1));
+			GET_CH_32(chip, chan->dir, chan->id, ch_control1));
 
 	if (tmp == 1)
 		return DMA_IN_PROGRESS;
@@ -272,30 +278,30 @@ enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
 
 void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
 {
-	struct dw_edma *dw = chan->chip->dw;
+	struct dw_edma_chip *chip = chan->chip;
 
-	SET_RW_32(dw, chan->dir, int_clear,
+	SET_RW_32(chip, chan->dir, int_clear,
 		  FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id)));
 }
 
 void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
 {
-	struct dw_edma *dw = chan->chip->dw;
+	struct dw_edma_chip *chip = chan->chip;
 
-	SET_RW_32(dw, chan->dir, int_clear,
+	SET_RW_32(chip, chan->dir, int_clear,
 		  FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id)));
 }
 
-u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
+u32 dw_edma_v0_core_status_done_int(struct dw_edma_chip *chip, enum dw_edma_dir dir)
 {
 	return FIELD_GET(EDMA_V0_DONE_INT_MASK,
-			 GET_RW_32(dw, dir, int_status));
+			 GET_RW_32(chip, dir, int_status));
 }
 
-u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
+u32 dw_edma_v0_core_status_abort_int(struct dw_edma_chip *chip, enum dw_edma_dir dir)
 {
 	return FIELD_GET(EDMA_V0_ABORT_INT_MASK,
-			 GET_RW_32(dw, dir, int_status));
+			 GET_RW_32(chip, dir, int_status));
 }
 
 static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
@@ -357,109 +363,109 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
 void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
 {
 	struct dw_edma_chan *chan = chunk->chan;
-	struct dw_edma *dw = chan->chip->dw;
+	struct dw_edma_chip *chip = chan->chip;
 	u32 tmp;
 
 	dw_edma_v0_core_write_chunk(chunk);
 
 	if (first) {
 		/* Enable engine */
-		SET_RW_32(dw, chan->dir, engine_en, BIT(0));
-		if (dw->mf == EDMA_MF_HDMA_COMPAT) {
+		SET_RW_32(chip, chan->dir, engine_en, BIT(0));
+		if (chip->mf == EDMA_MF_HDMA_COMPAT) {
 			switch (chan->id) {
 			case 0:
-				SET_RW_COMPAT(dw, chan->dir, ch0_pwr_en,
+				SET_RW_COMPAT(chip, chan->dir, ch0_pwr_en,
 					      BIT(0));
 				break;
 			case 1:
-				SET_RW_COMPAT(dw, chan->dir, ch1_pwr_en,
+				SET_RW_COMPAT(chip, chan->dir, ch1_pwr_en,
 					      BIT(0));
 				break;
 			case 2:
-				SET_RW_COMPAT(dw, chan->dir, ch2_pwr_en,
+				SET_RW_COMPAT(chip, chan->dir, ch2_pwr_en,
 					      BIT(0));
 				break;
 			case 3:
-				SET_RW_COMPAT(dw, chan->dir, ch3_pwr_en,
+				SET_RW_COMPAT(chip, chan->dir, ch3_pwr_en,
 					      BIT(0));
 				break;
 			case 4:
-				SET_RW_COMPAT(dw, chan->dir, ch4_pwr_en,
+				SET_RW_COMPAT(chip, chan->dir, ch4_pwr_en,
 					      BIT(0));
 				break;
 			case 5:
-				SET_RW_COMPAT(dw, chan->dir, ch5_pwr_en,
+				SET_RW_COMPAT(chip, chan->dir, ch5_pwr_en,
 					      BIT(0));
 				break;
 			case 6:
-				SET_RW_COMPAT(dw, chan->dir, ch6_pwr_en,
+				SET_RW_COMPAT(chip, chan->dir, ch6_pwr_en,
 					      BIT(0));
 				break;
 			case 7:
-				SET_RW_COMPAT(dw, chan->dir, ch7_pwr_en,
+				SET_RW_COMPAT(chip, chan->dir, ch7_pwr_en,
 					      BIT(0));
 				break;
 			}
 		}
 		/* Interrupt unmask - done, abort */
-		tmp = GET_RW_32(dw, chan->dir, int_mask);
+		tmp = GET_RW_32(chip, chan->dir, int_mask);
 		tmp &= ~FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id));
 		tmp &= ~FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id));
-		SET_RW_32(dw, chan->dir, int_mask, tmp);
+		SET_RW_32(chip, chan->dir, int_mask, tmp);
 		/* Linked list error */
-		tmp = GET_RW_32(dw, chan->dir, linked_list_err_en);
+		tmp = GET_RW_32(chip, chan->dir, linked_list_err_en);
 		tmp |= FIELD_PREP(EDMA_V0_LINKED_LIST_ERR_MASK, BIT(chan->id));
-		SET_RW_32(dw, chan->dir, linked_list_err_en, tmp);
+		SET_RW_32(chip, chan->dir, linked_list_err_en, tmp);
 		/* Channel control */
-		SET_CH_32(dw, chan->dir, chan->id, ch_control1,
+		SET_CH_32(chip, chan->dir, chan->id, ch_control1,
 			  (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
 		/* Linked list */
 		#ifdef CONFIG_64BIT
-			SET_CH_64(dw, chan->dir, chan->id, llp.reg,
+			SET_CH_64(chip, chan->dir, chan->id, llp.reg,
 				  chunk->ll_region.paddr);
 		#else /* CONFIG_64BIT */
-			SET_CH_32(dw, chan->dir, chan->id, llp.lsb,
+			SET_CH_32(chip, chan->dir, chan->id, llp.lsb,
 				  lower_32_bits(chunk->ll_region.paddr));
-			SET_CH_32(dw, chan->dir, chan->id, llp.msb,
+			SET_CH_32(chip, chan->dir, chan->id, llp.msb,
 				  upper_32_bits(chunk->ll_region.paddr));
 		#endif /* CONFIG_64BIT */
 	}
 	/* Doorbell */
-	SET_RW_32(dw, chan->dir, doorbell,
+	SET_RW_32(chip, chan->dir, doorbell,
 		  FIELD_PREP(EDMA_V0_DOORBELL_CH_MASK, chan->id));
 }
 
 int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
 {
-	struct dw_edma *dw = chan->chip->dw;
+	struct dw_edma_chip *chip = chan->chip;
 	u32 tmp = 0;
 
 	/* MSI done addr - low, high */
-	SET_RW_32(dw, chan->dir, done_imwr.lsb, chan->msi.address_lo);
-	SET_RW_32(dw, chan->dir, done_imwr.msb, chan->msi.address_hi);
+	SET_RW_32(chip, chan->dir, done_imwr.lsb, chan->msi.address_lo);
+	SET_RW_32(chip, chan->dir, done_imwr.msb, chan->msi.address_hi);
 	/* MSI abort addr - low, high */
-	SET_RW_32(dw, chan->dir, abort_imwr.lsb, chan->msi.address_lo);
-	SET_RW_32(dw, chan->dir, abort_imwr.msb, chan->msi.address_hi);
+	SET_RW_32(chip, chan->dir, abort_imwr.lsb, chan->msi.address_lo);
+	SET_RW_32(chip, chan->dir, abort_imwr.msb, chan->msi.address_hi);
 	/* MSI data - low, high */
 	switch (chan->id) {
 	case 0:
 	case 1:
-		tmp = GET_RW_32(dw, chan->dir, ch01_imwr_data);
+		tmp = GET_RW_32(chip, chan->dir, ch01_imwr_data);
 		break;
 
 	case 2:
 	case 3:
-		tmp = GET_RW_32(dw, chan->dir, ch23_imwr_data);
+		tmp = GET_RW_32(chip, chan->dir, ch23_imwr_data);
 		break;
 
 	case 4:
 	case 5:
-		tmp = GET_RW_32(dw, chan->dir, ch45_imwr_data);
+		tmp = GET_RW_32(chip, chan->dir, ch45_imwr_data);
 		break;
 
 	case 6:
 	case 7:
-		tmp = GET_RW_32(dw, chan->dir, ch67_imwr_data);
+		tmp = GET_RW_32(chip, chan->dir, ch67_imwr_data);
 		break;
 	}
 
@@ -478,22 +484,22 @@ int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
 	switch (chan->id) {
 	case 0:
 	case 1:
-		SET_RW_32(dw, chan->dir, ch01_imwr_data, tmp);
+		SET_RW_32(chip, chan->dir, ch01_imwr_data, tmp);
 		break;
 
 	case 2:
 	case 3:
-		SET_RW_32(dw, chan->dir, ch23_imwr_data, tmp);
+		SET_RW_32(chip, chan->dir, ch23_imwr_data, tmp);
 		break;
 
 	case 4:
 	case 5:
-		SET_RW_32(dw, chan->dir, ch45_imwr_data, tmp);
+		SET_RW_32(chip, chan->dir, ch45_imwr_data, tmp);
 		break;
 
 	case 6:
 	case 7:
-		SET_RW_32(dw, chan->dir, ch67_imwr_data, tmp);
+		SET_RW_32(chip, chan->dir, ch67_imwr_data, tmp);
 		break;
 	}
 
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.h b/drivers/dma/dw-edma/dw-edma-v0-core.h
index 2afa626b8300c..01a29c74c0c43 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.h
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.h
@@ -12,13 +12,13 @@
 #include <linux/dma/edma.h>
 
 /* eDMA management callbacks */
-void dw_edma_v0_core_off(struct dw_edma *chan);
-u16 dw_edma_v0_core_ch_count(struct dw_edma *chan, enum dw_edma_dir dir);
+void dw_edma_v0_core_off(struct dw_edma_chip *chip);
+u16 dw_edma_v0_core_ch_count(struct dw_edma_chip *chip, enum dw_edma_dir dir);
 enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan);
 void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan);
 void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan);
-u32 dw_edma_v0_core_status_done_int(struct dw_edma *chan, enum dw_edma_dir dir);
-u32 dw_edma_v0_core_status_abort_int(struct dw_edma *chan, enum dw_edma_dir dir);
+u32 dw_edma_v0_core_status_done_int(struct dw_edma_chip *chip, enum dw_edma_dir dir);
+u32 dw_edma_v0_core_status_abort_int(struct dw_edma_chip *chip, enum dw_edma_dir dir);
 void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first);
 int dw_edma_v0_core_device_config(struct dw_edma_chan *chan);
 /* eDMA debug fs callbacks */
diff --git a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
index 4b3bcffd15ef1..5819a64aceb0f 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
@@ -38,7 +38,7 @@
 #define CHANNEL_STR				"channel"
 #define REGISTERS_STR				"registers"
 
-static struct dw_edma				*dw;
+static struct dw_edma_chip			*chip;
 static struct dw_edma_v0_regs			__iomem *regs;
 
 static struct {
@@ -53,8 +53,10 @@ struct debugfs_entries {
 
 static int dw_edma_debugfs_u32_get(void *data, u64 *val)
 {
+	struct dw_edma *dw = chip->dw;
+
 	void __iomem *reg = (void __force __iomem *)data;
-	if (dw->mf == EDMA_MF_EDMA_LEGACY &&
+	if (chip->mf == EDMA_MF_EDMA_LEGACY &&
 	    reg >= (void __iomem *)&regs->type.legacy.ch) {
 		void __iomem *ptr = &regs->type.legacy.ch;
 		u32 viewport_sel = 0;
@@ -127,6 +129,8 @@ static void dw_edma_debugfs_regs_ch(struct dw_edma_v0_ch_regs __iomem *regs,
 
 static void dw_edma_debugfs_regs_wr(struct dentry *dir)
 {
+	struct dw_edma *dw = chip->dw;
+
 	const struct debugfs_entries debugfs_regs[] = {
 		/* eDMA global registers */
 		WR_REGISTER(engine_en),
@@ -173,7 +177,7 @@ static void dw_edma_debugfs_regs_wr(struct dentry *dir)
 	nr_entries = ARRAY_SIZE(debugfs_regs);
 	dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dir);
 
-	if (dw->mf == EDMA_MF_HDMA_COMPAT) {
+	if (chip->mf == EDMA_MF_HDMA_COMPAT) {
 		nr_entries = ARRAY_SIZE(debugfs_unroll_regs);
 		dw_edma_debugfs_create_x32(debugfs_unroll_regs, nr_entries,
 					   regs_dir);
@@ -195,6 +199,8 @@ static void dw_edma_debugfs_regs_wr(struct dentry *dir)
 
 static void dw_edma_debugfs_regs_rd(struct dentry *dir)
 {
+	struct dw_edma *dw = chip->dw;
+
 	const struct debugfs_entries debugfs_regs[] = {
 		/* eDMA global registers */
 		RD_REGISTER(engine_en),
@@ -242,7 +248,7 @@ static void dw_edma_debugfs_regs_rd(struct dentry *dir)
 	nr_entries = ARRAY_SIZE(debugfs_regs);
 	dw_edma_debugfs_create_x32(debugfs_regs, nr_entries, regs_dir);
 
-	if (dw->mf == EDMA_MF_HDMA_COMPAT) {
+	if (chip->mf == EDMA_MF_HDMA_COMPAT) {
 		nr_entries = ARRAY_SIZE(debugfs_unroll_regs);
 		dw_edma_debugfs_create_x32(debugfs_unroll_regs, nr_entries,
 					   regs_dir);
@@ -264,6 +270,7 @@ static void dw_edma_debugfs_regs_rd(struct dentry *dir)
 
 static void dw_edma_debugfs_regs(void)
 {
+	struct dw_edma *dw = chip->dw;
 	const struct debugfs_entries debugfs_regs[] = {
 		REGISTER(ctrl_data_arb_prior),
 		REGISTER(ctrl),
@@ -282,13 +289,15 @@ static void dw_edma_debugfs_regs(void)
 	dw_edma_debugfs_regs_rd(regs_dir);
 }
 
-void dw_edma_v0_debugfs_on(struct dw_edma_chip *chip)
+void dw_edma_v0_debugfs_on(struct dw_edma_chip *p)
 {
-	dw = chip->dw;
-	if (!dw)
+	struct dw_edma *dw;
+	chip = p;
+	if (!chip)
 		return;
 
-	regs = dw->rg_region.vaddr;
+	dw = chip->dw;
+	regs = chip->reg_base;
 	if (!regs)
 		return;
 
@@ -296,19 +305,19 @@ void dw_edma_v0_debugfs_on(struct dw_edma_chip *chip)
 	if (!dw->debugfs)
 		return;
 
-	debugfs_create_u32("mf", 0444, dw->debugfs, &dw->mf);
+	debugfs_create_u32("mf", 0444, dw->debugfs, &chip->mf);
 	debugfs_create_u16("wr_ch_cnt", 0444, dw->debugfs, &dw->wr_ch_cnt);
 	debugfs_create_u16("rd_ch_cnt", 0444, dw->debugfs, &dw->rd_ch_cnt);
 
 	dw_edma_debugfs_regs();
 }
 
-void dw_edma_v0_debugfs_off(struct dw_edma_chip *chip)
+void dw_edma_v0_debugfs_off(struct dw_edma_chip *p)
 {
-	dw = chip->dw;
-	if (!dw)
+	chip = p;
+	if (!chip)
 		return;
 
-	debugfs_remove_recursive(dw->debugfs);
-	dw->debugfs = NULL;
+	debugfs_remove_recursive(chip->dw->debugfs);
+	chip->dw->debugfs = NULL;
 }
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index cab6e18773dad..fcfbc0f47f83d 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -12,19 +12,62 @@
 #include <linux/device.h>
 #include <linux/dmaengine.h>
 
+#define EDMA_MAX_WR_CH                                  8
+#define EDMA_MAX_RD_CH                                  8
+
 struct dw_edma;
 
+struct dw_edma_region {
+	phys_addr_t	paddr;
+	void __iomem	*vaddr;
+	size_t		sz;
+};
+
+struct dw_edma_core_ops {
+	int (*irq_vector)(struct device *dev, unsigned int nr);
+};
+
+enum dw_edma_map_format {
+	EDMA_MF_EDMA_LEGACY = 0x0,
+	EDMA_MF_EDMA_UNROLL = 0x1,
+	EDMA_MF_HDMA_COMPAT = 0x5
+};
+
 /**
  * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
  * @dev:		 struct device of the eDMA controller
  * @id:			 instance ID
- * @irq:		 irq line
+ * @nr_irqs:		 total dma irq number
+ * reg64bit		 if support 64bit write to register
+ * @ops			 DMA channel to IRQ number mapping
+ * @wr_ch_cnt		 DMA write channel number
+ * @rd_ch_cnt		 DMA read channel number
+ * @rg_region		 DMA register region
+ * @ll_region_wr	 DMA descriptor link list memory for write channel
+ * @ll_region_rd	 DMA descriptor link list memory for read channel
+ * @mf			 DMA register map format
  * @dw:			 struct dw_edma that is filed by dw_edma_probe()
  */
 struct dw_edma_chip {
 	struct device		*dev;
 	int			id;
-	int			irq;
+	int			nr_irqs;
+	const struct dw_edma_core_ops   *ops;
+
+	void __iomem		*reg_base;
+
+	u16			ll_wr_cnt;
+	u16			ll_rd_cnt;
+	/* link list address */
+	struct dw_edma_region	ll_region_wr[EDMA_MAX_WR_CH];
+	struct dw_edma_region	ll_region_rd[EDMA_MAX_RD_CH];
+
+	/* data region */
+	struct dw_edma_region	dt_region_wr[EDMA_MAX_WR_CH];
+	struct dw_edma_region	dt_region_rd[EDMA_MAX_RD_CH];
+
+	enum dw_edma_map_format	mf;
+
 	struct dw_edma		*dw;
 };
 
-- 
2.24.0.rc1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 2/6] dmaengine: dw-edma-pcie: don't touch internal struct dw_edma
  2022-03-07 16:24 [PATCH v3 1/6] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Frank Li
@ 2022-03-07 16:24 ` Frank Li
  2022-03-07 16:24 ` [PATCH v3 3/6] dmaengine: dw-edma: Fix programming the source & dest addresses for ep Frank Li
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Frank Li @ 2022-03-07 16:24 UTC (permalink / raw)
  To: gustavo.pimentel, hongxing.zhu, l.stach, linux-imx, linux-pci, lznuaa
  Cc: vkoul, lorenzo.pieralisi, robh, kw, bhelgaas, shawnguo,
	manivannan.sadhasivam

"struct dw_edma" is an internal structure of the eDMA core. This should not be
used by the eDMA controllers like "dw-edma-pcie" for passing the controller
specific information to the core.

Instead, use the fields local to the "struct dw_edma_chip" for passing the
controller specific info to the core.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v2 to v3:
 None

Change from v1 to v2:
 - rework commit message
 - rg_region only use virtual address. using chip->reg_base instead

 drivers/dma/dw-edma/dw-edma-pcie.c | 83 ++++++++++++------------------
 1 file changed, 34 insertions(+), 49 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 44f6e09bdb531..7732537f96086 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -148,7 +148,6 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	struct dw_edma_pcie_data vsec_data;
 	struct device *dev = &pdev->dev;
 	struct dw_edma_chip *chip;
-	struct dw_edma *dw;
 	int err, nr_irqs;
 	int i, mask;
 
@@ -214,10 +213,6 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	if (!chip)
 		return -ENOMEM;
 
-	dw = devm_kzalloc(dev, sizeof(*dw), GFP_KERNEL);
-	if (!dw)
-		return -ENOMEM;
-
 	/* IRQs allocation */
 	nr_irqs = pci_alloc_irq_vectors(pdev, 1, vsec_data.irqs,
 					PCI_IRQ_MSI | PCI_IRQ_MSIX);
@@ -228,29 +223,23 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	}
 
 	/* Data structure initialization */
-	chip->dw = dw;
 	chip->dev = dev;
 	chip->id = pdev->devfn;
-	chip->irq = pdev->irq;
 
-	dw->mf = vsec_data.mf;
-	dw->nr_irqs = nr_irqs;
-	dw->ops = &dw_edma_pcie_core_ops;
-	dw->wr_ch_cnt = vsec_data.wr_ch_cnt;
-	dw->rd_ch_cnt = vsec_data.rd_ch_cnt;
+	chip->mf = vsec_data.mf;
+	chip->nr_irqs = nr_irqs;
+	chip->ops = &dw_edma_pcie_core_ops;
 
-	dw->rg_region.vaddr = pcim_iomap_table(pdev)[vsec_data.rg.bar];
-	if (!dw->rg_region.vaddr)
-		return -ENOMEM;
+	chip->ll_wr_cnt = vsec_data.wr_ch_cnt;
+	chip->ll_rd_cnt = vsec_data.rd_ch_cnt;
 
-	dw->rg_region.vaddr += vsec_data.rg.off;
-	dw->rg_region.paddr = pdev->resource[vsec_data.rg.bar].start;
-	dw->rg_region.paddr += vsec_data.rg.off;
-	dw->rg_region.sz = vsec_data.rg.sz;
+	chip->reg_base = pcim_iomap_table(pdev)[vsec_data.rg.bar];
+	if (!chip->reg_base)
+		return -ENOMEM;
 
-	for (i = 0; i < dw->wr_ch_cnt; i++) {
-		struct dw_edma_region *ll_region = &dw->ll_region_wr[i];
-		struct dw_edma_region *dt_region = &dw->dt_region_wr[i];
+	for (i = 0; i < chip->ll_wr_cnt; 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];
 		struct dw_edma_block *dt_block = &vsec_data.dt_wr[i];
 
@@ -273,9 +262,9 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 		dt_region->sz = dt_block->sz;
 	}
 
-	for (i = 0; i < dw->rd_ch_cnt; i++) {
-		struct dw_edma_region *ll_region = &dw->ll_region_rd[i];
-		struct dw_edma_region *dt_region = &dw->dt_region_rd[i];
+	for (i = 0; i < chip->ll_rd_cnt; 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];
 		struct dw_edma_block *dt_block = &vsec_data.dt_rd[i];
 
@@ -299,45 +288,45 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	}
 
 	/* Debug info */
-	if (dw->mf == EDMA_MF_EDMA_LEGACY)
-		pci_dbg(pdev, "Version:\teDMA Port Logic (0x%x)\n", dw->mf);
-	else if (dw->mf == EDMA_MF_EDMA_UNROLL)
-		pci_dbg(pdev, "Version:\teDMA Unroll (0x%x)\n", dw->mf);
-	else if (dw->mf == EDMA_MF_HDMA_COMPAT)
-		pci_dbg(pdev, "Version:\tHDMA Compatible (0x%x)\n", dw->mf);
+	if (chip->mf == EDMA_MF_EDMA_LEGACY)
+		pci_dbg(pdev, "Version:\teDMA Port Logic (0x%x)\n", chip->mf);
+	else if (chip->mf == EDMA_MF_EDMA_UNROLL)
+		pci_dbg(pdev, "Version:\teDMA Unroll (0x%x)\n", chip->mf);
+	else if (chip->mf == EDMA_MF_HDMA_COMPAT)
+		pci_dbg(pdev, "Version:\tHDMA Compatible (0x%x)\n", chip->mf);
 	else
-		pci_dbg(pdev, "Version:\tUnknown (0x%x)\n", dw->mf);
+		pci_dbg(pdev, "Version:\tUnknown (0x%x)\n", chip->mf);
 
-	pci_dbg(pdev, "Registers:\tBAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p, p=%pa)\n",
+	pci_dbg(pdev, "Registers:\tBAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p)\n",
 		vsec_data.rg.bar, vsec_data.rg.off, vsec_data.rg.sz,
-		dw->rg_region.vaddr, &dw->rg_region.paddr);
+		chip->reg_base);
 
 
-	for (i = 0; i < dw->wr_ch_cnt; i++) {
+	for (i = 0; i < chip->ll_wr_cnt; i++) {
 		pci_dbg(pdev, "L. List:\tWRITE CH%.2u, BAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p, p=%pa)\n",
 			i, vsec_data.ll_wr[i].bar,
-			vsec_data.ll_wr[i].off, dw->ll_region_wr[i].sz,
-			dw->ll_region_wr[i].vaddr, &dw->ll_region_wr[i].paddr);
+			vsec_data.ll_wr[i].off, chip->ll_region_wr[i].sz,
+			chip->ll_region_wr[i].vaddr, &chip->ll_region_wr[i].paddr);
 
 		pci_dbg(pdev, "Data:\tWRITE CH%.2u, BAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p, p=%pa)\n",
 			i, vsec_data.dt_wr[i].bar,
-			vsec_data.dt_wr[i].off, dw->dt_region_wr[i].sz,
-			dw->dt_region_wr[i].vaddr, &dw->dt_region_wr[i].paddr);
+			vsec_data.dt_wr[i].off, chip->dt_region_wr[i].sz,
+			chip->dt_region_wr[i].vaddr, &chip->dt_region_wr[i].paddr);
 	}
 
-	for (i = 0; i < dw->rd_ch_cnt; i++) {
+	for (i = 0; i < chip->ll_rd_cnt; i++) {
 		pci_dbg(pdev, "L. List:\tREAD CH%.2u, BAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p, p=%pa)\n",
 			i, vsec_data.ll_rd[i].bar,
-			vsec_data.ll_rd[i].off, dw->ll_region_rd[i].sz,
-			dw->ll_region_rd[i].vaddr, &dw->ll_region_rd[i].paddr);
+			vsec_data.ll_rd[i].off, chip->ll_region_rd[i].sz,
+			chip->ll_region_rd[i].vaddr, &chip->ll_region_rd[i].paddr);
 
 		pci_dbg(pdev, "Data:\tREAD CH%.2u, BAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p, p=%pa)\n",
 			i, vsec_data.dt_rd[i].bar,
-			vsec_data.dt_rd[i].off, dw->dt_region_rd[i].sz,
-			dw->dt_region_rd[i].vaddr, &dw->dt_region_rd[i].paddr);
+			vsec_data.dt_rd[i].off, chip->dt_region_rd[i].sz,
+			chip->dt_region_rd[i].vaddr, &chip->dt_region_rd[i].paddr);
 	}
 
-	pci_dbg(pdev, "Nr. IRQs:\t%u\n", dw->nr_irqs);
+	pci_dbg(pdev, "Nr. IRQs:\t%u\n", chip->nr_irqs);
 
 	/* Validating if PCI interrupts were enabled */
 	if (!pci_dev_msi_enabled(pdev)) {
@@ -345,10 +334,6 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 		return -EPERM;
 	}
 
-	dw->irq = devm_kcalloc(dev, nr_irqs, sizeof(*dw->irq), GFP_KERNEL);
-	if (!dw->irq)
-		return -ENOMEM;
-
 	/* Starting eDMA driver */
 	err = dw_edma_probe(chip);
 	if (err) {
-- 
2.24.0.rc1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 3/6] dmaengine: dw-edma: Fix programming the source & dest addresses for ep
  2022-03-07 16:24 [PATCH v3 1/6] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Frank Li
  2022-03-07 16:24 ` [PATCH v3 2/6] dmaengine: dw-edma-pcie: don't touch internal struct dw_edma Frank Li
@ 2022-03-07 16:24 ` Frank Li
  2022-03-07 16:24 ` [PATCH v3 4/6] dmaengine: dw-edma: Don't rely on the deprecated "direction" member Frank Li
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Frank Li @ 2022-03-07 16:24 UTC (permalink / raw)
  To: gustavo.pimentel, hongxing.zhu, l.stach, linux-imx, linux-pci, lznuaa
  Cc: vkoul, lorenzo.pieralisi, robh, kw, bhelgaas, shawnguo,
	manivannan.sadhasivam

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

When eDMA is controlled by the Endpoint (EP), the current logic incorrectly
programs the source and destination addresses for read and write. Since the
Root complex and Endpoint uses the opposite channels for read/write, fix the
issue by finding out the read operation first and program the eDMA accordingly.

Cc: stable@vger.kernel.org
Fixes: bd96f1b2f43a ("dmaengine: dw-edma: support local dma device transfer semantics")
Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---

Change from V1-v3
 - Direct pick up from Manivannan

 drivers/dma/dw-edma/dw-edma-core.c | 32 +++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 0cb66434f9e14..3636c48f5df15 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -334,6 +334,7 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
 	struct dw_edma_chunk *chunk;
 	struct dw_edma_burst *burst;
 	struct dw_edma_desc *desc;
+	bool read = false;
 	u32 cnt = 0;
 	int i;
 
@@ -424,7 +425,36 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
 		chunk->ll_region.sz += burst->sz;
 		desc->alloc_sz += burst->sz;
 
-		if (chan->dir == EDMA_DIR_WRITE) {
+		/****************************************************************
+		 *
+		 *        Root Complex                           Endpoint
+		 * +-----------------------+             +----------------------+
+		 * |                       |    TX CH    |                      |
+		 * |                       |             |                      |
+		 * |      DEV_TO_MEM       <-------------+     MEM_TO_DEV       |
+		 * |                       |             |                      |
+		 * |                       |             |                      |
+		 * |      MEM_TO_DEV       +------------->     DEV_TO_MEM       |
+		 * |                       |             |                      |
+		 * |                       |    RX CH    |                      |
+		 * +-----------------------+             +----------------------+
+		 *
+		 * If eDMA is controlled by the Root complex, TX channel
+		 * (EDMA_DIR_WRITE) is used for memory read (DEV_TO_MEM) and RX
+		 * channel (EDMA_DIR_READ) is used for memory write (MEM_TO_DEV).
+		 *
+		 * If eDMA is controlled by the endpoint, RX channel
+		 * (EDMA_DIR_READ) is used for memory read (DEV_TO_MEM) and TX
+		 * channel (EDMA_DIR_WRITE) is used for memory write (MEM_TO_DEV).
+		 *
+		 ****************************************************************/
+
+		if ((dir == DMA_DEV_TO_MEM && chan->dir == EDMA_DIR_READ) ||
+		    (dir == DMA_DEV_TO_MEM && chan->dir == EDMA_DIR_WRITE))
+			read = true;
+
+		/* Program the source and destination addresses for DMA read/write */
+		if (read) {
 			burst->sar = src_addr;
 			if (xfer->type == EDMA_XFER_CYCLIC) {
 				burst->dar = xfer->xfer.cyclic.paddr;
-- 
2.24.0.rc1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 4/6] dmaengine: dw-edma: Don't rely on the deprecated "direction" member
  2022-03-07 16:24 [PATCH v3 1/6] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Frank Li
  2022-03-07 16:24 ` [PATCH v3 2/6] dmaengine: dw-edma-pcie: don't touch internal struct dw_edma Frank Li
  2022-03-07 16:24 ` [PATCH v3 3/6] dmaengine: dw-edma: Fix programming the source & dest addresses for ep Frank Li
@ 2022-03-07 16:24 ` Frank Li
  2022-03-07 16:24 ` [PATCH v3 5/6] dmaengine: dw-edma: add flags at struct dw_edma_chip Frank Li
  2022-03-07 16:24 ` [PATCH v3 6/6] PCI: endpoint: functions/pci-epf-test: Support PCI controller DMA Frank Li
  4 siblings, 0 replies; 12+ messages in thread
From: Frank Li @ 2022-03-07 16:24 UTC (permalink / raw)
  To: gustavo.pimentel, hongxing.zhu, l.stach, linux-imx, linux-pci, lznuaa
  Cc: vkoul, lorenzo.pieralisi, robh, kw, bhelgaas, shawnguo,
	manivannan.sadhasivam

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

The "direction" member of the "dma_slave_config" structure is deprecated.
The clients no longer use this field to specify the direction of the slave
channel. But in the eDMA core, this field is used to differentiate between the
Root complex (remote) and Endpoint (local) DMA accesses.

Nevertheless, we can't differentiate between local and remote accesses without
a dedicated flag. So let's get rid of the old check and add a new check for
verifying the DMA operation between local and remote memory instead.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
Change from v1 to v3
 - direct pick up from Manivannan

 drivers/dma/dw-edma/dw-edma-core.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 3636c48f5df15..0635157d260c1 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -341,22 +341,9 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
 	if (!chan->configured)
 		return NULL;
 
-	switch (chan->config.direction) {
-	case DMA_DEV_TO_MEM: /* local DMA */
-		if (dir == DMA_DEV_TO_MEM && chan->dir == EDMA_DIR_READ)
-			break;
-		return NULL;
-	case DMA_MEM_TO_DEV: /* local DMA */
-		if (dir == DMA_MEM_TO_DEV && chan->dir == EDMA_DIR_WRITE)
-			break;
+	/* eDMA supports only read and write between local and remote memory */
+	if (dir != DMA_DEV_TO_MEM && dir != DMA_MEM_TO_DEV)
 		return NULL;
-	default: /* remote DMA */
-		if (dir == DMA_MEM_TO_DEV && chan->dir == EDMA_DIR_READ)
-			break;
-		if (dir == DMA_DEV_TO_MEM && chan->dir == EDMA_DIR_WRITE)
-			break;
-		return NULL;
-	}
 
 	if (xfer->type == EDMA_XFER_CYCLIC) {
 		if (!xfer->xfer.cyclic.len || !xfer->xfer.cyclic.cnt)
-- 
2.24.0.rc1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 5/6] dmaengine: dw-edma: add flags at struct dw_edma_chip
  2022-03-07 16:24 [PATCH v3 1/6] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Frank Li
                   ` (2 preceding siblings ...)
  2022-03-07 16:24 ` [PATCH v3 4/6] dmaengine: dw-edma: Don't rely on the deprecated "direction" member Frank Li
@ 2022-03-07 16:24 ` Frank Li
  2022-03-07 16:24 ` [PATCH v3 6/6] PCI: endpoint: functions/pci-epf-test: Support PCI controller DMA Frank Li
  4 siblings, 0 replies; 12+ messages in thread
From: Frank Li @ 2022-03-07 16:24 UTC (permalink / raw)
  To: gustavo.pimentel, hongxing.zhu, l.stach, linux-imx, linux-pci, lznuaa
  Cc: vkoul, lorenzo.pieralisi, robh, kw, bhelgaas, shawnguo,
	manivannan.sadhasivam

Allow PCI EP probe DMA locally and prevent use of remote MSI
to remote PCI host.

Add option to force 32bit DBI register access even on
64-bit systems. i.MX8 hardware only allowed 32bit register
access.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v2 to v3
 - rework commit message
 - Change to DW_EDMA_CHIP_32BIT_DBI
 - using DW_EDMA_CHIP_LOCAL control msi
 - Apply Bjorn's comments, 
	if (!j) {
               control |= DW_EDMA_V0_LIE;
               if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
                               control |= DW_EDMA_V0_RIE;
        }

	if ((chan->chip->flags & DW_EDMA_CHIP_REG32BIT) ||
              !IS_ENABLED(CONFIG_64BIT)) {
          SET_CH_32(...);
          SET_CH_32(...);
       } else {
          SET_CH_64(...);
       }


Change from v1 to v2
- none

 drivers/dma/dw-edma/dw-edma-v0-core.c | 20 ++++++++++++--------
 include/linux/dma/edma.h              |  9 +++++++++
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index 6e2f83e31a03a..081cd7997348d 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -307,6 +307,7 @@ u32 dw_edma_v0_core_status_abort_int(struct dw_edma_chip *chip, enum dw_edma_dir
 static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
 {
 	struct dw_edma_burst *child;
+	struct dw_edma_chan *chan = chunk->chan;
 	struct dw_edma_v0_lli __iomem *lli;
 	struct dw_edma_v0_llp __iomem *llp;
 	u32 control = 0, i = 0;
@@ -320,9 +321,11 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
 	j = chunk->bursts_alloc;
 	list_for_each_entry(child, &chunk->burst->list, list) {
 		j--;
-		if (!j)
-			control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE);
-
+		if (!j) {
+			control |= DW_EDMA_V0_LIE;
+			if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
+				control |= DW_EDMA_V0_RIE;
+		}
 		/* Channel control */
 		SET_LL_32(&lli[i].control, control);
 		/* Transfer size */
@@ -420,15 +423,16 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
 		SET_CH_32(chip, chan->dir, chan->id, ch_control1,
 			  (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
 		/* Linked list */
-		#ifdef CONFIG_64BIT
-			SET_CH_64(chip, chan->dir, chan->id, llp.reg,
-				  chunk->ll_region.paddr);
-		#else /* CONFIG_64BIT */
+		if ((chan->chip->flags & DW_EDMA_CHIP_32BIT_DBI) ||
+		    !IS_ENABLED(CONFIG_64BIT)) {
 			SET_CH_32(chip, chan->dir, chan->id, llp.lsb,
 				  lower_32_bits(chunk->ll_region.paddr));
 			SET_CH_32(chip, chan->dir, chan->id, llp.msb,
 				  upper_32_bits(chunk->ll_region.paddr));
-		#endif /* CONFIG_64BIT */
+		} else {
+			SET_CH_64(chip, chan->dir, chan->id, llp.reg,
+				  chunk->ll_region.paddr);
+		}
 	}
 	/* Doorbell */
 	SET_RW_32(chip, chan->dir, doorbell,
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index fcfbc0f47f83d..4321f6378ef66 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -33,6 +33,12 @@ enum dw_edma_map_format {
 	EDMA_MF_HDMA_COMPAT = 0x5
 };
 
+/* Probe EDMA engine locally and prevent generate MSI to host side*/
+#define DW_EDMA_CHIP_LOCAL	BIT(0)
+
+/* Only support 32bit DBI register access */
+#define DW_EDMA_CHIP_32BIT_DBI	BIT(1)
+
 /**
  * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
  * @dev:		 struct device of the eDMA controller
@@ -40,6 +46,8 @@ enum dw_edma_map_format {
  * @nr_irqs:		 total dma irq number
  * reg64bit		 if support 64bit write to register
  * @ops			 DMA channel to IRQ number mapping
+ * @flags		 - DW_EDMA_CHIP_LOCAL
+ *			 - DW_EDMA_CHIP_32BIT_DBI
  * @wr_ch_cnt		 DMA write channel number
  * @rd_ch_cnt		 DMA read channel number
  * @rg_region		 DMA register region
@@ -53,6 +61,7 @@ struct dw_edma_chip {
 	int			id;
 	int			nr_irqs;
 	const struct dw_edma_core_ops   *ops;
+	u32			flags;
 
 	void __iomem		*reg_base;
 
-- 
2.24.0.rc1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 6/6] PCI: endpoint: functions/pci-epf-test: Support PCI controller DMA
  2022-03-07 16:24 [PATCH v3 1/6] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Frank Li
                   ` (3 preceding siblings ...)
  2022-03-07 16:24 ` [PATCH v3 5/6] dmaengine: dw-edma: add flags at struct dw_edma_chip Frank Li
@ 2022-03-07 16:24 ` Frank Li
  4 siblings, 0 replies; 12+ messages in thread
From: Frank Li @ 2022-03-07 16:24 UTC (permalink / raw)
  To: gustavo.pimentel, hongxing.zhu, l.stach, linux-imx, linux-pci, lznuaa
  Cc: vkoul, lorenzo.pieralisi, robh, kw, bhelgaas, shawnguo,
	manivannan.sadhasivam

Designware provided DMA support in controller. This enabled use
this DMA controller to transfer data.

The whole flow align with standard DMA usage module

1. Using dma_request_channel() and filter function to find correct
RX and TX Channel.
2. dmaengine_slave_config() config remote side physcial address.
3. using dmaengine_prep_slave_single() create transfer descriptor
4. tx_submit();
5. dma_async_issue_pending();

Tested at i.MX8DXL platform.

root@imx8qmmek:~# /usr/bin/pcitest -d -w
WRITE ( 102400 bytes):          OKAY
root@imx8qmmek:~# /usr/bin/pcitest -d -r
READ ( 102400 bytes):           OKAY

WRITE => Size: 102400 bytes DMA: YES  Time: 0.000180145 seconds Rate: 555108 KB/s
READ => Size: 102400 bytes  DMA: YES  Time: 0.000194397 seconds Rate: 514411 KB/s

READ => Size: 102400 bytes  DMA: NO   Time: 0.013532597 seconds Rate: 7389 KB/s
WRITE => Size: 102400 bytes DMA: NO   Time: 0.000857090 seconds Rate: 116673 KB/s

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Change from v1 to v3
 - none

 drivers/pci/endpoint/functions/pci-epf-test.c | 106 ++++++++++++++++--
 1 file changed, 96 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 90d84d3bc868f..22ae420c30693 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -52,9 +52,11 @@ struct pci_epf_test {
 	enum pci_barno		test_reg_bar;
 	size_t			msix_table_offset;
 	struct delayed_work	cmd_handler;
-	struct dma_chan		*dma_chan;
+	struct dma_chan		*dma_chan_tx;
+	struct dma_chan		*dma_chan_rx;
 	struct completion	transfer_complete;
 	bool			dma_supported;
+	bool			dma_private;
 	const struct pci_epc_features *epc_features;
 };
 
@@ -105,14 +107,17 @@ static void pci_epf_test_dma_callback(void *param)
  */
 static int pci_epf_test_data_transfer(struct pci_epf_test *epf_test,
 				      dma_addr_t dma_dst, dma_addr_t dma_src,
-				      size_t len)
+				      size_t len, dma_addr_t remote,
+				      enum dma_transfer_direction dir)
 {
 	enum dma_ctrl_flags flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
-	struct dma_chan *chan = epf_test->dma_chan;
+	struct dma_chan *chan = (dir == DMA_DEV_TO_MEM) ? epf_test->dma_chan_tx : epf_test->dma_chan_rx;
 	struct pci_epf *epf = epf_test->epf;
 	struct dma_async_tx_descriptor *tx;
 	struct device *dev = &epf->dev;
 	dma_cookie_t cookie;
+	struct dma_slave_config	sconf;
+	dma_addr_t local = (dir == DMA_MEM_TO_DEV) ? dma_src : dma_dst;
 	int ret;
 
 	if (IS_ERR_OR_NULL(chan)) {
@@ -120,7 +125,20 @@ static int pci_epf_test_data_transfer(struct pci_epf_test *epf_test,
 		return -EINVAL;
 	}
 
-	tx = dmaengine_prep_dma_memcpy(chan, dma_dst, dma_src, len, flags);
+	if (epf_test->dma_private) {
+		memset(&sconf, 0, sizeof(sconf));
+		sconf.direction = dir;
+		if (dir == DMA_MEM_TO_DEV)
+			sconf.dst_addr = remote;
+		else
+			sconf.src_addr = remote;
+
+		dmaengine_slave_config(chan, &sconf);
+		tx = dmaengine_prep_slave_single(chan, local, len, dir, flags);
+	} else {
+		tx = dmaengine_prep_dma_memcpy(chan, dma_dst, dma_src, len, flags);
+	}
+
 	if (!tx) {
 		dev_err(dev, "Failed to prepare DMA memcpy\n");
 		return -EIO;
@@ -148,6 +166,23 @@ static int pci_epf_test_data_transfer(struct pci_epf_test *epf_test,
 	return 0;
 }
 
+struct epf_dma_filter {
+	struct device *dev;
+	u32 dma_mask;
+};
+
+static bool epf_dma_filter_fn(struct dma_chan *chan, void *node)
+{
+	struct epf_dma_filter *filter = node;
+	struct dma_slave_caps caps;
+
+	memset(&caps, 0, sizeof(caps));
+	dma_get_slave_caps(chan, &caps);
+
+	return chan->device->dev == filter->dev
+		&& (filter->dma_mask & caps.directions);
+}
+
 /**
  * pci_epf_test_init_dma_chan() - Function to initialize EPF test DMA channel
  * @epf_test: the EPF test device that performs data transfer operation
@@ -160,8 +195,42 @@ static int pci_epf_test_init_dma_chan(struct pci_epf_test *epf_test)
 	struct device *dev = &epf->dev;
 	struct dma_chan *dma_chan;
 	dma_cap_mask_t mask;
+	struct epf_dma_filter filter;
 	int ret;
 
+	filter.dev = epf->epc->dev.parent;
+	filter.dma_mask = BIT(DMA_DEV_TO_MEM);
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+	dma_chan = dma_request_channel(mask, epf_dma_filter_fn, &filter);
+	if (IS_ERR(dma_chan)) {
+		dev_info(dev, "Failure get built-in DMA channel, fail back to try allocate general DMA channel\n");
+		goto fail_back_tx;
+	}
+
+	epf_test->dma_chan_rx = dma_chan;
+
+	filter.dma_mask = BIT(DMA_MEM_TO_DEV);
+	dma_chan = dma_request_channel(mask, epf_dma_filter_fn, &filter);
+
+	if (IS_ERR(dma_chan)) {
+		dev_info(dev, "Failure get built-in DMA channel, fail back to try allocate general DMA channel\n");
+		goto fail_back_rx;
+	}
+
+	epf_test->dma_chan_tx = dma_chan;
+	epf_test->dma_private = true;
+
+	init_completion(&epf_test->transfer_complete);
+
+	return 0;
+
+fail_back_rx:
+	dma_release_channel(epf_test->dma_chan_rx);
+	epf_test->dma_chan_tx = NULL;
+
+fail_back_tx:
 	dma_cap_zero(mask);
 	dma_cap_set(DMA_MEMCPY, mask);
 
@@ -174,7 +243,7 @@ static int pci_epf_test_init_dma_chan(struct pci_epf_test *epf_test)
 	}
 	init_completion(&epf_test->transfer_complete);
 
-	epf_test->dma_chan = dma_chan;
+	epf_test->dma_chan_tx = epf_test->dma_chan_rx = dma_chan;
 
 	return 0;
 }
@@ -190,8 +259,17 @@ static void pci_epf_test_clean_dma_chan(struct pci_epf_test *epf_test)
 	if (!epf_test->dma_supported)
 		return;
 
-	dma_release_channel(epf_test->dma_chan);
-	epf_test->dma_chan = NULL;
+	dma_release_channel(epf_test->dma_chan_tx);
+	if (epf_test->dma_chan_tx == epf_test->dma_chan_rx) {
+		epf_test->dma_chan_tx = NULL;
+		epf_test->dma_chan_rx = NULL;
+		return;
+	}
+
+	dma_release_channel(epf_test->dma_chan_rx);
+	epf_test->dma_chan_rx = NULL;
+
+	return;
 }
 
 static void pci_epf_test_print_rate(const char *ops, u64 size,
@@ -280,8 +358,14 @@ static int pci_epf_test_copy(struct pci_epf_test *epf_test)
 			goto err_map_addr;
 		}
 
+		if (epf_test->dma_private) {
+			dev_err(dev, "Cannot transfer data using DMA\n");
+			ret = -EINVAL;
+			goto err_map_addr;
+		}
+
 		ret = pci_epf_test_data_transfer(epf_test, dst_phys_addr,
-						 src_phys_addr, reg->size);
+						 src_phys_addr, reg->size, 0, DMA_MEM_TO_MEM);
 		if (ret)
 			dev_err(dev, "Data transfer failed\n");
 	} else {
@@ -363,7 +447,8 @@ static int pci_epf_test_read(struct pci_epf_test *epf_test)
 
 		ktime_get_ts64(&start);
 		ret = pci_epf_test_data_transfer(epf_test, dst_phys_addr,
-						 phys_addr, reg->size);
+						 phys_addr, reg->size,
+						 reg->src_addr, DMA_DEV_TO_MEM);
 		if (ret)
 			dev_err(dev, "Data transfer failed\n");
 		ktime_get_ts64(&end);
@@ -453,8 +538,9 @@ static int pci_epf_test_write(struct pci_epf_test *epf_test)
 		}
 
 		ktime_get_ts64(&start);
+
 		ret = pci_epf_test_data_transfer(epf_test, phys_addr,
-						 src_phys_addr, reg->size);
+						 src_phys_addr, reg->size, reg->dst_addr, DMA_MEM_TO_DEV);
 		if (ret)
 			dev_err(dev, "Data transfer failed\n");
 		ktime_get_ts64(&end);
-- 
2.24.0.rc1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 5/6] dmaengine: dw-edma: add flags at struct dw_edma_chip
  2022-03-18 18:40       ` Zhi Li
@ 2022-03-18 19:28         ` Manivannan Sadhasivam
  0 siblings, 0 replies; 12+ messages in thread
From: Manivannan Sadhasivam @ 2022-03-18 19:28 UTC (permalink / raw)
  To: Zhi Li
  Cc: Frank Li, gustavo.pimentel, hongxing.zhu, Lucas Stach,
	dl-linux-imx, linux-pci, dmaengine, vkoul, lorenzo.pieralisi,
	robh, kw, Bjorn Helgaas, Shawn Guo

On Fri, Mar 18, 2022 at 01:40:44PM -0500, Zhi Li wrote:
> On Thu, Mar 10, 2022 at 11:00 AM Zhi Li <lznuaa@gmail.com> wrote:
> >
> > On Thu, Mar 10, 2022 at 1:44 AM Manivannan Sadhasivam
> > <manivannan.sadhasivam@linaro.org> wrote:
> > >
> > > On Mon, Mar 07, 2022 at 04:47:49PM -0600, Frank Li wrote:
> > > > Allow PCI EP probe DMA locally and prevent use of remote MSI
> > > > to remote PCI host.
> > > >
> > > > Add option to force 32bit DBI register access even on
> > > > 64-bit systems. i.MX8 hardware only allowed 32bit register
> > > > access.
> > > >
> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > >
> > > > Resend added dmaengine@vger.kernel.org
> > > >
> > > > Change from v2 to v3
> > > >  - rework commit message
> > > >  - Change to DW_EDMA_CHIP_32BIT_DBI
> > > >  - using DW_EDMA_CHIP_LOCAL control msi
> > > >  - Apply Bjorn's comments,
> > > >       if (!j) {
> > > >                control |= DW_EDMA_V0_LIE;
> > > >                if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
> > > >                                control |= DW_EDMA_V0_RIE;
> > > >         }
> > > >
> > > >       if ((chan->chip->flags & DW_EDMA_CHIP_REG32BIT) ||
> > > >               !IS_ENABLED(CONFIG_64BIT)) {
> > > >           SET_CH_32(...);
> > > >           SET_CH_32(...);
> > > >        } else {
> > > >           SET_CH_64(...);
> > > >        }
> > > >
> > > >
> > > > Change from v1 to v2
> > > > - none
> > > >
> > > >  drivers/dma/dw-edma/dw-edma-v0-core.c | 20 ++++++++++++--------
> > > >  include/linux/dma/edma.h              |  9 +++++++++
> > > >  2 files changed, 21 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > > index 6e2f83e31a03a..081cd7997348d 100644
> > > > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > > @@ -307,6 +307,7 @@ u32 dw_edma_v0_core_status_abort_int(struct dw_edma_chip *chip, enum dw_edma_dir
> > > >  static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> > > >  {
> > > >       struct dw_edma_burst *child;
> > > > +     struct dw_edma_chan *chan = chunk->chan;
> > > >       struct dw_edma_v0_lli __iomem *lli;
> > > >       struct dw_edma_v0_llp __iomem *llp;
> > > >       u32 control = 0, i = 0;
> > > > @@ -320,9 +321,11 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> > > >       j = chunk->bursts_alloc;
> > > >       list_for_each_entry(child, &chunk->burst->list, list) {
> > > >               j--;
> > > > -             if (!j)
> > > > -                     control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE);
> > > > -
> > > > +             if (!j) {
> > > > +                     control |= DW_EDMA_V0_LIE;
> > > > +                     if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
> > > > +                             control |= DW_EDMA_V0_RIE;
> > > > +             }
> > > >               /* Channel control */
> > > >               SET_LL_32(&lli[i].control, control);
> > > >               /* Transfer size */
> > > > @@ -420,15 +423,16 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > > >               SET_CH_32(chip, chan->dir, chan->id, ch_control1,
> > > >                         (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
> > > >               /* Linked list */
> > > > -             #ifdef CONFIG_64BIT
> > > > -                     SET_CH_64(chip, chan->dir, chan->id, llp.reg,
> > > > -                               chunk->ll_region.paddr);
> > > > -             #else /* CONFIG_64BIT */
> > > > +             if ((chan->chip->flags & DW_EDMA_CHIP_32BIT_DBI) ||
> > > > +                 !IS_ENABLED(CONFIG_64BIT)) {
> > > >                       SET_CH_32(chip, chan->dir, chan->id, llp.lsb,
> > > >                                 lower_32_bits(chunk->ll_region.paddr));
> > > >                       SET_CH_32(chip, chan->dir, chan->id, llp.msb,
> > > >                                 upper_32_bits(chunk->ll_region.paddr));
> > > > -             #endif /* CONFIG_64BIT */
> > > > +             } else {
> > > > +                     SET_CH_64(chip, chan->dir, chan->id, llp.reg,
> > > > +                               chunk->ll_region.paddr);
> > > > +             }
> > > >       }
> > > >       /* Doorbell */
> > > >       SET_RW_32(chip, chan->dir, doorbell,
> > > > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> > > > index fcfbc0f47f83d..4321f6378ef66 100644
> > > > --- a/include/linux/dma/edma.h
> > > > +++ b/include/linux/dma/edma.h
> > > > @@ -33,6 +33,12 @@ enum dw_edma_map_format {
> > > >       EDMA_MF_HDMA_COMPAT = 0x5
> > > >  };
> > > >
> > > > +/* Probe EDMA engine locally and prevent generate MSI to host side*/
> > > > +#define DW_EDMA_CHIP_LOCAL   BIT(0)
> > > > +
> > > > +/* Only support 32bit DBI register access */
> > > > +#define DW_EDMA_CHIP_32BIT_DBI       BIT(1)
> > > > +
> > >
> > > How about using an enum for defining the flags? This would help us organize the
> > > flags in a more coherent way and also will give the benefit of kdoc.
> >
> > Did you see other linux code use enum as bitmask?
> > According to my understanding, enum just chooses values in a list.
>

There are lot of places it is used:

$ grep -r "= BIT(.*)," include/
 
> Do you agree that using define because it will be used as bitmask?
> 

As I said, using enum helps in organizing the flags and it also provides kdoc.
I'd prefer to go with it.

Thanks,
Mani

> best regards
> Frank Li
> 
> >
> > >
> > > /**
> > >  * enum dw_edma_chip_flags - Flags specific to an eDMA chip
> > >  * @DW_EDMA_CHIP_LOCAL:         eDMA is used locally by an endpoint
> > >  * @DW_EDMA_CHIP_32BIT_DBI:     eDMA only supports 32bit DBI access
> > >  */
> > > enum dw_edma_chip_flags {
> > >         DW_EDMA_CHIP_LOCAL = BIT(0),
> > >         DW_EDMA_CHIP_32BIT_DBI = BIT(1),
> > > };
> > >
> > > >  /**
> > > >   * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
> > > >   * @dev:              struct device of the eDMA controller
> > > > @@ -40,6 +46,8 @@ enum dw_edma_map_format {
> > > >   * @nr_irqs:          total dma irq number
> > > >   * reg64bit           if support 64bit write to register
> > > >   * @ops                       DMA channel to IRQ number mapping
> > > > + * @flags             - DW_EDMA_CHIP_LOCAL
> > > > + *                    - DW_EDMA_CHIP_32BIT_DBI
> > >
> > > No need to mention the flags here if you use the enum I suggested above.
> > >
> > > >   * @wr_ch_cnt                 DMA write channel number
> > > >   * @rd_ch_cnt                 DMA read channel number
> > > >   * @rg_region                 DMA register region
> > > > @@ -53,6 +61,7 @@ struct dw_edma_chip {
> > > >       int                     id;
> > > >       int                     nr_irqs;
> > > >       const struct dw_edma_core_ops   *ops;
> > > > +     u32                     flags;
> > >
> > >         enum dw_edma_chip_flags flags;
> > >
> > > Thanks,
> > > Mani
> > >
> > > >
> > > >       void __iomem            *reg_base;
> > > >
> > > > --
> > > > 2.24.0.rc1
> > > >

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 5/6] dmaengine: dw-edma: add flags at struct dw_edma_chip
  2022-03-10 17:00     ` Zhi Li
@ 2022-03-18 18:40       ` Zhi Li
  2022-03-18 19:28         ` Manivannan Sadhasivam
  0 siblings, 1 reply; 12+ messages in thread
From: Zhi Li @ 2022-03-18 18:40 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Frank Li, gustavo.pimentel, hongxing.zhu, Lucas Stach,
	dl-linux-imx, linux-pci, dmaengine, vkoul, lorenzo.pieralisi,
	robh, kw, Bjorn Helgaas, Shawn Guo

On Thu, Mar 10, 2022 at 11:00 AM Zhi Li <lznuaa@gmail.com> wrote:
>
> On Thu, Mar 10, 2022 at 1:44 AM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Mon, Mar 07, 2022 at 04:47:49PM -0600, Frank Li wrote:
> > > Allow PCI EP probe DMA locally and prevent use of remote MSI
> > > to remote PCI host.
> > >
> > > Add option to force 32bit DBI register access even on
> > > 64-bit systems. i.MX8 hardware only allowed 32bit register
> > > access.
> > >
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > >
> > > Resend added dmaengine@vger.kernel.org
> > >
> > > Change from v2 to v3
> > >  - rework commit message
> > >  - Change to DW_EDMA_CHIP_32BIT_DBI
> > >  - using DW_EDMA_CHIP_LOCAL control msi
> > >  - Apply Bjorn's comments,
> > >       if (!j) {
> > >                control |= DW_EDMA_V0_LIE;
> > >                if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
> > >                                control |= DW_EDMA_V0_RIE;
> > >         }
> > >
> > >       if ((chan->chip->flags & DW_EDMA_CHIP_REG32BIT) ||
> > >               !IS_ENABLED(CONFIG_64BIT)) {
> > >           SET_CH_32(...);
> > >           SET_CH_32(...);
> > >        } else {
> > >           SET_CH_64(...);
> > >        }
> > >
> > >
> > > Change from v1 to v2
> > > - none
> > >
> > >  drivers/dma/dw-edma/dw-edma-v0-core.c | 20 ++++++++++++--------
> > >  include/linux/dma/edma.h              |  9 +++++++++
> > >  2 files changed, 21 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > index 6e2f83e31a03a..081cd7997348d 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > @@ -307,6 +307,7 @@ u32 dw_edma_v0_core_status_abort_int(struct dw_edma_chip *chip, enum dw_edma_dir
> > >  static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> > >  {
> > >       struct dw_edma_burst *child;
> > > +     struct dw_edma_chan *chan = chunk->chan;
> > >       struct dw_edma_v0_lli __iomem *lli;
> > >       struct dw_edma_v0_llp __iomem *llp;
> > >       u32 control = 0, i = 0;
> > > @@ -320,9 +321,11 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> > >       j = chunk->bursts_alloc;
> > >       list_for_each_entry(child, &chunk->burst->list, list) {
> > >               j--;
> > > -             if (!j)
> > > -                     control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE);
> > > -
> > > +             if (!j) {
> > > +                     control |= DW_EDMA_V0_LIE;
> > > +                     if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
> > > +                             control |= DW_EDMA_V0_RIE;
> > > +             }
> > >               /* Channel control */
> > >               SET_LL_32(&lli[i].control, control);
> > >               /* Transfer size */
> > > @@ -420,15 +423,16 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > >               SET_CH_32(chip, chan->dir, chan->id, ch_control1,
> > >                         (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
> > >               /* Linked list */
> > > -             #ifdef CONFIG_64BIT
> > > -                     SET_CH_64(chip, chan->dir, chan->id, llp.reg,
> > > -                               chunk->ll_region.paddr);
> > > -             #else /* CONFIG_64BIT */
> > > +             if ((chan->chip->flags & DW_EDMA_CHIP_32BIT_DBI) ||
> > > +                 !IS_ENABLED(CONFIG_64BIT)) {
> > >                       SET_CH_32(chip, chan->dir, chan->id, llp.lsb,
> > >                                 lower_32_bits(chunk->ll_region.paddr));
> > >                       SET_CH_32(chip, chan->dir, chan->id, llp.msb,
> > >                                 upper_32_bits(chunk->ll_region.paddr));
> > > -             #endif /* CONFIG_64BIT */
> > > +             } else {
> > > +                     SET_CH_64(chip, chan->dir, chan->id, llp.reg,
> > > +                               chunk->ll_region.paddr);
> > > +             }
> > >       }
> > >       /* Doorbell */
> > >       SET_RW_32(chip, chan->dir, doorbell,
> > > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> > > index fcfbc0f47f83d..4321f6378ef66 100644
> > > --- a/include/linux/dma/edma.h
> > > +++ b/include/linux/dma/edma.h
> > > @@ -33,6 +33,12 @@ enum dw_edma_map_format {
> > >       EDMA_MF_HDMA_COMPAT = 0x5
> > >  };
> > >
> > > +/* Probe EDMA engine locally and prevent generate MSI to host side*/
> > > +#define DW_EDMA_CHIP_LOCAL   BIT(0)
> > > +
> > > +/* Only support 32bit DBI register access */
> > > +#define DW_EDMA_CHIP_32BIT_DBI       BIT(1)
> > > +
> >
> > How about using an enum for defining the flags? This would help us organize the
> > flags in a more coherent way and also will give the benefit of kdoc.
>
> Did you see other linux code use enum as bitmask?
> According to my understanding, enum just chooses values in a list.

Do you agree that using define because it will be used as bitmask?

best regards
Frank Li

>
> >
> > /**
> >  * enum dw_edma_chip_flags - Flags specific to an eDMA chip
> >  * @DW_EDMA_CHIP_LOCAL:         eDMA is used locally by an endpoint
> >  * @DW_EDMA_CHIP_32BIT_DBI:     eDMA only supports 32bit DBI access
> >  */
> > enum dw_edma_chip_flags {
> >         DW_EDMA_CHIP_LOCAL = BIT(0),
> >         DW_EDMA_CHIP_32BIT_DBI = BIT(1),
> > };
> >
> > >  /**
> > >   * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
> > >   * @dev:              struct device of the eDMA controller
> > > @@ -40,6 +46,8 @@ enum dw_edma_map_format {
> > >   * @nr_irqs:          total dma irq number
> > >   * reg64bit           if support 64bit write to register
> > >   * @ops                       DMA channel to IRQ number mapping
> > > + * @flags             - DW_EDMA_CHIP_LOCAL
> > > + *                    - DW_EDMA_CHIP_32BIT_DBI
> >
> > No need to mention the flags here if you use the enum I suggested above.
> >
> > >   * @wr_ch_cnt                 DMA write channel number
> > >   * @rd_ch_cnt                 DMA read channel number
> > >   * @rg_region                 DMA register region
> > > @@ -53,6 +61,7 @@ struct dw_edma_chip {
> > >       int                     id;
> > >       int                     nr_irqs;
> > >       const struct dw_edma_core_ops   *ops;
> > > +     u32                     flags;
> >
> >         enum dw_edma_chip_flags flags;
> >
> > Thanks,
> > Mani
> >
> > >
> > >       void __iomem            *reg_base;
> > >
> > > --
> > > 2.24.0.rc1
> > >

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 5/6] dmaengine: dw-edma: add flags at struct dw_edma_chip
  2022-03-10  7:44   ` Manivannan Sadhasivam
@ 2022-03-10 17:00     ` Zhi Li
  2022-03-18 18:40       ` Zhi Li
  0 siblings, 1 reply; 12+ messages in thread
From: Zhi Li @ 2022-03-10 17:00 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Frank Li, gustavo.pimentel, hongxing.zhu, Lucas Stach,
	dl-linux-imx, linux-pci, dmaengine, vkoul, lorenzo.pieralisi,
	robh, kw, Bjorn Helgaas, Shawn Guo

On Thu, Mar 10, 2022 at 1:44 AM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Mon, Mar 07, 2022 at 04:47:49PM -0600, Frank Li wrote:
> > Allow PCI EP probe DMA locally and prevent use of remote MSI
> > to remote PCI host.
> >
> > Add option to force 32bit DBI register access even on
> > 64-bit systems. i.MX8 hardware only allowed 32bit register
> > access.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >
> > Resend added dmaengine@vger.kernel.org
> >
> > Change from v2 to v3
> >  - rework commit message
> >  - Change to DW_EDMA_CHIP_32BIT_DBI
> >  - using DW_EDMA_CHIP_LOCAL control msi
> >  - Apply Bjorn's comments,
> >       if (!j) {
> >                control |= DW_EDMA_V0_LIE;
> >                if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
> >                                control |= DW_EDMA_V0_RIE;
> >         }
> >
> >       if ((chan->chip->flags & DW_EDMA_CHIP_REG32BIT) ||
> >               !IS_ENABLED(CONFIG_64BIT)) {
> >           SET_CH_32(...);
> >           SET_CH_32(...);
> >        } else {
> >           SET_CH_64(...);
> >        }
> >
> >
> > Change from v1 to v2
> > - none
> >
> >  drivers/dma/dw-edma/dw-edma-v0-core.c | 20 ++++++++++++--------
> >  include/linux/dma/edma.h              |  9 +++++++++
> >  2 files changed, 21 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > index 6e2f83e31a03a..081cd7997348d 100644
> > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > @@ -307,6 +307,7 @@ u32 dw_edma_v0_core_status_abort_int(struct dw_edma_chip *chip, enum dw_edma_dir
> >  static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> >  {
> >       struct dw_edma_burst *child;
> > +     struct dw_edma_chan *chan = chunk->chan;
> >       struct dw_edma_v0_lli __iomem *lli;
> >       struct dw_edma_v0_llp __iomem *llp;
> >       u32 control = 0, i = 0;
> > @@ -320,9 +321,11 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> >       j = chunk->bursts_alloc;
> >       list_for_each_entry(child, &chunk->burst->list, list) {
> >               j--;
> > -             if (!j)
> > -                     control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE);
> > -
> > +             if (!j) {
> > +                     control |= DW_EDMA_V0_LIE;
> > +                     if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
> > +                             control |= DW_EDMA_V0_RIE;
> > +             }
> >               /* Channel control */
> >               SET_LL_32(&lli[i].control, control);
> >               /* Transfer size */
> > @@ -420,15 +423,16 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> >               SET_CH_32(chip, chan->dir, chan->id, ch_control1,
> >                         (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
> >               /* Linked list */
> > -             #ifdef CONFIG_64BIT
> > -                     SET_CH_64(chip, chan->dir, chan->id, llp.reg,
> > -                               chunk->ll_region.paddr);
> > -             #else /* CONFIG_64BIT */
> > +             if ((chan->chip->flags & DW_EDMA_CHIP_32BIT_DBI) ||
> > +                 !IS_ENABLED(CONFIG_64BIT)) {
> >                       SET_CH_32(chip, chan->dir, chan->id, llp.lsb,
> >                                 lower_32_bits(chunk->ll_region.paddr));
> >                       SET_CH_32(chip, chan->dir, chan->id, llp.msb,
> >                                 upper_32_bits(chunk->ll_region.paddr));
> > -             #endif /* CONFIG_64BIT */
> > +             } else {
> > +                     SET_CH_64(chip, chan->dir, chan->id, llp.reg,
> > +                               chunk->ll_region.paddr);
> > +             }
> >       }
> >       /* Doorbell */
> >       SET_RW_32(chip, chan->dir, doorbell,
> > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> > index fcfbc0f47f83d..4321f6378ef66 100644
> > --- a/include/linux/dma/edma.h
> > +++ b/include/linux/dma/edma.h
> > @@ -33,6 +33,12 @@ enum dw_edma_map_format {
> >       EDMA_MF_HDMA_COMPAT = 0x5
> >  };
> >
> > +/* Probe EDMA engine locally and prevent generate MSI to host side*/
> > +#define DW_EDMA_CHIP_LOCAL   BIT(0)
> > +
> > +/* Only support 32bit DBI register access */
> > +#define DW_EDMA_CHIP_32BIT_DBI       BIT(1)
> > +
>
> How about using an enum for defining the flags? This would help us organize the
> flags in a more coherent way and also will give the benefit of kdoc.

Did you see other linux code use enum as bitmask?
According to my understanding, enum just chooses values in a list.

>
> /**
>  * enum dw_edma_chip_flags - Flags specific to an eDMA chip
>  * @DW_EDMA_CHIP_LOCAL:         eDMA is used locally by an endpoint
>  * @DW_EDMA_CHIP_32BIT_DBI:     eDMA only supports 32bit DBI access
>  */
> enum dw_edma_chip_flags {
>         DW_EDMA_CHIP_LOCAL = BIT(0),
>         DW_EDMA_CHIP_32BIT_DBI = BIT(1),
> };
>
> >  /**
> >   * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
> >   * @dev:              struct device of the eDMA controller
> > @@ -40,6 +46,8 @@ enum dw_edma_map_format {
> >   * @nr_irqs:          total dma irq number
> >   * reg64bit           if support 64bit write to register
> >   * @ops                       DMA channel to IRQ number mapping
> > + * @flags             - DW_EDMA_CHIP_LOCAL
> > + *                    - DW_EDMA_CHIP_32BIT_DBI
>
> No need to mention the flags here if you use the enum I suggested above.
>
> >   * @wr_ch_cnt                 DMA write channel number
> >   * @rd_ch_cnt                 DMA read channel number
> >   * @rg_region                 DMA register region
> > @@ -53,6 +61,7 @@ struct dw_edma_chip {
> >       int                     id;
> >       int                     nr_irqs;
> >       const struct dw_edma_core_ops   *ops;
> > +     u32                     flags;
>
>         enum dw_edma_chip_flags flags;
>
> Thanks,
> Mani
>
> >
> >       void __iomem            *reg_base;
> >
> > --
> > 2.24.0.rc1
> >

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 5/6] dmaengine: dw-edma: add flags at struct dw_edma_chip
  2022-03-07 22:47 ` [PATCH v3 5/6] dmaengine: dw-edma: add flags at struct dw_edma_chip Frank Li
  2022-03-10  7:44   ` Manivannan Sadhasivam
@ 2022-03-10  7:55   ` Manivannan Sadhasivam
  1 sibling, 0 replies; 12+ messages in thread
From: Manivannan Sadhasivam @ 2022-03-10  7:55 UTC (permalink / raw)
  To: Frank Li
  Cc: gustavo.pimentel, hongxing.zhu, l.stach, linux-imx, linux-pci,
	dmaengine, lznuaa, vkoul, lorenzo.pieralisi, robh, kw, bhelgaas,
	shawnguo

Subject could be:

dmaengine: dw-edma: Add support for chip specific flags

On Mon, Mar 07, 2022 at 04:47:49PM -0600, Frank Li wrote:
> Allow PCI EP probe DMA locally and prevent use of remote MSI
> to remote PCI host.
> 
> Add option to force 32bit DBI register access even on
> 64-bit systems. i.MX8 hardware only allowed 32bit register
> access.
> 

Add a "flags" field to the "struct dw_edma_chip" so that the controller drivers
can pass flags that are relevant to the platform. Currently 2 flags are
defined:

1. DW_EDMA_CHIP_LOCAL - Used by the controller drivers accessing eDMA locally.
Local eDMA access doesn't require generating MSIs to the remote.

2. DW_EDMA_CHIP_32BIT_DBI - Used by the controller drivers like i.MX8 that
allows only 32bit access to the DBI region.

Thanks,
Mani

> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> 
> Resend added dmaengine@vger.kernel.org
> 
> Change from v2 to v3
>  - rework commit message
>  - Change to DW_EDMA_CHIP_32BIT_DBI
>  - using DW_EDMA_CHIP_LOCAL control msi
>  - Apply Bjorn's comments, 
> 	if (!j) {
>                control |= DW_EDMA_V0_LIE;
>                if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
>                                control |= DW_EDMA_V0_RIE;
>         }
> 
> 	if ((chan->chip->flags & DW_EDMA_CHIP_REG32BIT) ||
>               !IS_ENABLED(CONFIG_64BIT)) {
>           SET_CH_32(...);
>           SET_CH_32(...);
>        } else {
>           SET_CH_64(...);
>        }
> 
> 
> Change from v1 to v2
> - none
> 
>  drivers/dma/dw-edma/dw-edma-v0-core.c | 20 ++++++++++++--------
>  include/linux/dma/edma.h              |  9 +++++++++
>  2 files changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index 6e2f83e31a03a..081cd7997348d 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -307,6 +307,7 @@ u32 dw_edma_v0_core_status_abort_int(struct dw_edma_chip *chip, enum dw_edma_dir
>  static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
>  {
>  	struct dw_edma_burst *child;
> +	struct dw_edma_chan *chan = chunk->chan;
>  	struct dw_edma_v0_lli __iomem *lli;
>  	struct dw_edma_v0_llp __iomem *llp;
>  	u32 control = 0, i = 0;
> @@ -320,9 +321,11 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
>  	j = chunk->bursts_alloc;
>  	list_for_each_entry(child, &chunk->burst->list, list) {
>  		j--;
> -		if (!j)
> -			control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE);
> -
> +		if (!j) {
> +			control |= DW_EDMA_V0_LIE;
> +			if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
> +				control |= DW_EDMA_V0_RIE;
> +		}
>  		/* Channel control */
>  		SET_LL_32(&lli[i].control, control);
>  		/* Transfer size */
> @@ -420,15 +423,16 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
>  		SET_CH_32(chip, chan->dir, chan->id, ch_control1,
>  			  (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
>  		/* Linked list */
> -		#ifdef CONFIG_64BIT
> -			SET_CH_64(chip, chan->dir, chan->id, llp.reg,
> -				  chunk->ll_region.paddr);
> -		#else /* CONFIG_64BIT */
> +		if ((chan->chip->flags & DW_EDMA_CHIP_32BIT_DBI) ||
> +		    !IS_ENABLED(CONFIG_64BIT)) {
>  			SET_CH_32(chip, chan->dir, chan->id, llp.lsb,
>  				  lower_32_bits(chunk->ll_region.paddr));
>  			SET_CH_32(chip, chan->dir, chan->id, llp.msb,
>  				  upper_32_bits(chunk->ll_region.paddr));
> -		#endif /* CONFIG_64BIT */
> +		} else {
> +			SET_CH_64(chip, chan->dir, chan->id, llp.reg,
> +				  chunk->ll_region.paddr);
> +		}
>  	}
>  	/* Doorbell */
>  	SET_RW_32(chip, chan->dir, doorbell,
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index fcfbc0f47f83d..4321f6378ef66 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -33,6 +33,12 @@ enum dw_edma_map_format {
>  	EDMA_MF_HDMA_COMPAT = 0x5
>  };
>  
> +/* Probe EDMA engine locally and prevent generate MSI to host side*/
> +#define DW_EDMA_CHIP_LOCAL	BIT(0)
> +
> +/* Only support 32bit DBI register access */
> +#define DW_EDMA_CHIP_32BIT_DBI	BIT(1)
> +
>  /**
>   * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
>   * @dev:		 struct device of the eDMA controller
> @@ -40,6 +46,8 @@ enum dw_edma_map_format {
>   * @nr_irqs:		 total dma irq number
>   * reg64bit		 if support 64bit write to register
>   * @ops			 DMA channel to IRQ number mapping
> + * @flags		 - DW_EDMA_CHIP_LOCAL
> + *			 - DW_EDMA_CHIP_32BIT_DBI
>   * @wr_ch_cnt		 DMA write channel number
>   * @rd_ch_cnt		 DMA read channel number
>   * @rg_region		 DMA register region
> @@ -53,6 +61,7 @@ struct dw_edma_chip {
>  	int			id;
>  	int			nr_irqs;
>  	const struct dw_edma_core_ops   *ops;
> +	u32			flags;
>  
>  	void __iomem		*reg_base;
>  
> -- 
> 2.24.0.rc1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 5/6] dmaengine: dw-edma: add flags at struct dw_edma_chip
  2022-03-07 22:47 ` [PATCH v3 5/6] dmaengine: dw-edma: add flags at struct dw_edma_chip Frank Li
@ 2022-03-10  7:44   ` Manivannan Sadhasivam
  2022-03-10 17:00     ` Zhi Li
  2022-03-10  7:55   ` Manivannan Sadhasivam
  1 sibling, 1 reply; 12+ messages in thread
From: Manivannan Sadhasivam @ 2022-03-10  7:44 UTC (permalink / raw)
  To: Frank Li
  Cc: gustavo.pimentel, hongxing.zhu, l.stach, linux-imx, linux-pci,
	dmaengine, lznuaa, vkoul, lorenzo.pieralisi, robh, kw, bhelgaas,
	shawnguo

On Mon, Mar 07, 2022 at 04:47:49PM -0600, Frank Li wrote:
> Allow PCI EP probe DMA locally and prevent use of remote MSI
> to remote PCI host.
> 
> Add option to force 32bit DBI register access even on
> 64-bit systems. i.MX8 hardware only allowed 32bit register
> access.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> 
> Resend added dmaengine@vger.kernel.org
> 
> Change from v2 to v3
>  - rework commit message
>  - Change to DW_EDMA_CHIP_32BIT_DBI
>  - using DW_EDMA_CHIP_LOCAL control msi
>  - Apply Bjorn's comments, 
> 	if (!j) {
>                control |= DW_EDMA_V0_LIE;
>                if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
>                                control |= DW_EDMA_V0_RIE;
>         }
> 
> 	if ((chan->chip->flags & DW_EDMA_CHIP_REG32BIT) ||
>               !IS_ENABLED(CONFIG_64BIT)) {
>           SET_CH_32(...);
>           SET_CH_32(...);
>        } else {
>           SET_CH_64(...);
>        }
> 
> 
> Change from v1 to v2
> - none
> 
>  drivers/dma/dw-edma/dw-edma-v0-core.c | 20 ++++++++++++--------
>  include/linux/dma/edma.h              |  9 +++++++++
>  2 files changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index 6e2f83e31a03a..081cd7997348d 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -307,6 +307,7 @@ u32 dw_edma_v0_core_status_abort_int(struct dw_edma_chip *chip, enum dw_edma_dir
>  static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
>  {
>  	struct dw_edma_burst *child;
> +	struct dw_edma_chan *chan = chunk->chan;
>  	struct dw_edma_v0_lli __iomem *lli;
>  	struct dw_edma_v0_llp __iomem *llp;
>  	u32 control = 0, i = 0;
> @@ -320,9 +321,11 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
>  	j = chunk->bursts_alloc;
>  	list_for_each_entry(child, &chunk->burst->list, list) {
>  		j--;
> -		if (!j)
> -			control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE);
> -
> +		if (!j) {
> +			control |= DW_EDMA_V0_LIE;
> +			if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
> +				control |= DW_EDMA_V0_RIE;
> +		}
>  		/* Channel control */
>  		SET_LL_32(&lli[i].control, control);
>  		/* Transfer size */
> @@ -420,15 +423,16 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
>  		SET_CH_32(chip, chan->dir, chan->id, ch_control1,
>  			  (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
>  		/* Linked list */
> -		#ifdef CONFIG_64BIT
> -			SET_CH_64(chip, chan->dir, chan->id, llp.reg,
> -				  chunk->ll_region.paddr);
> -		#else /* CONFIG_64BIT */
> +		if ((chan->chip->flags & DW_EDMA_CHIP_32BIT_DBI) ||
> +		    !IS_ENABLED(CONFIG_64BIT)) {
>  			SET_CH_32(chip, chan->dir, chan->id, llp.lsb,
>  				  lower_32_bits(chunk->ll_region.paddr));
>  			SET_CH_32(chip, chan->dir, chan->id, llp.msb,
>  				  upper_32_bits(chunk->ll_region.paddr));
> -		#endif /* CONFIG_64BIT */
> +		} else {
> +			SET_CH_64(chip, chan->dir, chan->id, llp.reg,
> +				  chunk->ll_region.paddr);
> +		}
>  	}
>  	/* Doorbell */
>  	SET_RW_32(chip, chan->dir, doorbell,
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index fcfbc0f47f83d..4321f6378ef66 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -33,6 +33,12 @@ enum dw_edma_map_format {
>  	EDMA_MF_HDMA_COMPAT = 0x5
>  };
>  
> +/* Probe EDMA engine locally and prevent generate MSI to host side*/
> +#define DW_EDMA_CHIP_LOCAL	BIT(0)
> +
> +/* Only support 32bit DBI register access */
> +#define DW_EDMA_CHIP_32BIT_DBI	BIT(1)
> +

How about using an enum for defining the flags? This would help us organize the
flags in a more coherent way and also will give the benefit of kdoc.

/**
 * enum dw_edma_chip_flags - Flags specific to an eDMA chip
 * @DW_EDMA_CHIP_LOCAL:		eDMA is used locally by an endpoint
 * @DW_EDMA_CHIP_32BIT_DBI:	eDMA only supports 32bit DBI access
 */
enum dw_edma_chip_flags {
	DW_EDMA_CHIP_LOCAL = BIT(0),
	DW_EDMA_CHIP_32BIT_DBI = BIT(1),
};

>  /**
>   * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
>   * @dev:		 struct device of the eDMA controller
> @@ -40,6 +46,8 @@ enum dw_edma_map_format {
>   * @nr_irqs:		 total dma irq number
>   * reg64bit		 if support 64bit write to register
>   * @ops			 DMA channel to IRQ number mapping
> + * @flags		 - DW_EDMA_CHIP_LOCAL
> + *			 - DW_EDMA_CHIP_32BIT_DBI

No need to mention the flags here if you use the enum I suggested above.

>   * @wr_ch_cnt		 DMA write channel number
>   * @rd_ch_cnt		 DMA read channel number
>   * @rg_region		 DMA register region
> @@ -53,6 +61,7 @@ struct dw_edma_chip {
>  	int			id;
>  	int			nr_irqs;
>  	const struct dw_edma_core_ops   *ops;
> +	u32			flags;

	enum dw_edma_chip_flags	flags;

Thanks,
Mani

>  
>  	void __iomem		*reg_base;
>  
> -- 
> 2.24.0.rc1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 5/6] dmaengine: dw-edma: add flags at struct dw_edma_chip
  2022-03-07 22:47 [PATCH v3 1/6] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Frank Li
@ 2022-03-07 22:47 ` Frank Li
  2022-03-10  7:44   ` Manivannan Sadhasivam
  2022-03-10  7:55   ` Manivannan Sadhasivam
  0 siblings, 2 replies; 12+ messages in thread
From: Frank Li @ 2022-03-07 22:47 UTC (permalink / raw)
  To: gustavo.pimentel, hongxing.zhu, l.stach, linux-imx, linux-pci,
	dmaengine, lznuaa
  Cc: vkoul, lorenzo.pieralisi, robh, kw, bhelgaas, shawnguo,
	manivannan.sadhasivam

Allow PCI EP probe DMA locally and prevent use of remote MSI
to remote PCI host.

Add option to force 32bit DBI register access even on
64-bit systems. i.MX8 hardware only allowed 32bit register
access.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Resend added dmaengine@vger.kernel.org

Change from v2 to v3
 - rework commit message
 - Change to DW_EDMA_CHIP_32BIT_DBI
 - using DW_EDMA_CHIP_LOCAL control msi
 - Apply Bjorn's comments, 
	if (!j) {
               control |= DW_EDMA_V0_LIE;
               if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
                               control |= DW_EDMA_V0_RIE;
        }

	if ((chan->chip->flags & DW_EDMA_CHIP_REG32BIT) ||
              !IS_ENABLED(CONFIG_64BIT)) {
          SET_CH_32(...);
          SET_CH_32(...);
       } else {
          SET_CH_64(...);
       }


Change from v1 to v2
- none

 drivers/dma/dw-edma/dw-edma-v0-core.c | 20 ++++++++++++--------
 include/linux/dma/edma.h              |  9 +++++++++
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index 6e2f83e31a03a..081cd7997348d 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -307,6 +307,7 @@ u32 dw_edma_v0_core_status_abort_int(struct dw_edma_chip *chip, enum dw_edma_dir
 static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
 {
 	struct dw_edma_burst *child;
+	struct dw_edma_chan *chan = chunk->chan;
 	struct dw_edma_v0_lli __iomem *lli;
 	struct dw_edma_v0_llp __iomem *llp;
 	u32 control = 0, i = 0;
@@ -320,9 +321,11 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
 	j = chunk->bursts_alloc;
 	list_for_each_entry(child, &chunk->burst->list, list) {
 		j--;
-		if (!j)
-			control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE);
-
+		if (!j) {
+			control |= DW_EDMA_V0_LIE;
+			if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
+				control |= DW_EDMA_V0_RIE;
+		}
 		/* Channel control */
 		SET_LL_32(&lli[i].control, control);
 		/* Transfer size */
@@ -420,15 +423,16 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
 		SET_CH_32(chip, chan->dir, chan->id, ch_control1,
 			  (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
 		/* Linked list */
-		#ifdef CONFIG_64BIT
-			SET_CH_64(chip, chan->dir, chan->id, llp.reg,
-				  chunk->ll_region.paddr);
-		#else /* CONFIG_64BIT */
+		if ((chan->chip->flags & DW_EDMA_CHIP_32BIT_DBI) ||
+		    !IS_ENABLED(CONFIG_64BIT)) {
 			SET_CH_32(chip, chan->dir, chan->id, llp.lsb,
 				  lower_32_bits(chunk->ll_region.paddr));
 			SET_CH_32(chip, chan->dir, chan->id, llp.msb,
 				  upper_32_bits(chunk->ll_region.paddr));
-		#endif /* CONFIG_64BIT */
+		} else {
+			SET_CH_64(chip, chan->dir, chan->id, llp.reg,
+				  chunk->ll_region.paddr);
+		}
 	}
 	/* Doorbell */
 	SET_RW_32(chip, chan->dir, doorbell,
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index fcfbc0f47f83d..4321f6378ef66 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -33,6 +33,12 @@ enum dw_edma_map_format {
 	EDMA_MF_HDMA_COMPAT = 0x5
 };
 
+/* Probe EDMA engine locally and prevent generate MSI to host side*/
+#define DW_EDMA_CHIP_LOCAL	BIT(0)
+
+/* Only support 32bit DBI register access */
+#define DW_EDMA_CHIP_32BIT_DBI	BIT(1)
+
 /**
  * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
  * @dev:		 struct device of the eDMA controller
@@ -40,6 +46,8 @@ enum dw_edma_map_format {
  * @nr_irqs:		 total dma irq number
  * reg64bit		 if support 64bit write to register
  * @ops			 DMA channel to IRQ number mapping
+ * @flags		 - DW_EDMA_CHIP_LOCAL
+ *			 - DW_EDMA_CHIP_32BIT_DBI
  * @wr_ch_cnt		 DMA write channel number
  * @rd_ch_cnt		 DMA read channel number
  * @rg_region		 DMA register region
@@ -53,6 +61,7 @@ struct dw_edma_chip {
 	int			id;
 	int			nr_irqs;
 	const struct dw_edma_core_ops   *ops;
+	u32			flags;
 
 	void __iomem		*reg_base;
 
-- 
2.24.0.rc1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-03-18 19:29 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-07 16:24 [PATCH v3 1/6] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Frank Li
2022-03-07 16:24 ` [PATCH v3 2/6] dmaengine: dw-edma-pcie: don't touch internal struct dw_edma Frank Li
2022-03-07 16:24 ` [PATCH v3 3/6] dmaengine: dw-edma: Fix programming the source & dest addresses for ep Frank Li
2022-03-07 16:24 ` [PATCH v3 4/6] dmaengine: dw-edma: Don't rely on the deprecated "direction" member Frank Li
2022-03-07 16:24 ` [PATCH v3 5/6] dmaengine: dw-edma: add flags at struct dw_edma_chip Frank Li
2022-03-07 16:24 ` [PATCH v3 6/6] PCI: endpoint: functions/pci-epf-test: Support PCI controller DMA Frank Li
2022-03-07 22:47 [PATCH v3 1/6] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Frank Li
2022-03-07 22:47 ` [PATCH v3 5/6] dmaengine: dw-edma: add flags at struct dw_edma_chip Frank Li
2022-03-10  7:44   ` Manivannan Sadhasivam
2022-03-10 17:00     ` Zhi Li
2022-03-18 18:40       ` Zhi Li
2022-03-18 19:28         ` Manivannan Sadhasivam
2022-03-10  7:55   ` Manivannan Sadhasivam

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.