* [PATCH v2 1/5] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally
@ 2022-03-03 18:46 Frank Li
2022-03-03 18:46 ` [PATCH v2 2/5] dmaengine: dw-edma-pcie: don't touch internal struct dw_edma Frank Li
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Frank Li @ 2022-03-03 18:46 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 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 *)®s->type.legacy.ch) {
void __iomem *ptr = ®s->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] 18+ messages in thread
* [PATCH v2 2/5] dmaengine: dw-edma-pcie: don't touch internal struct dw_edma
2022-03-03 18:46 [PATCH v2 1/5] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Frank Li
@ 2022-03-03 18:46 ` Frank Li
2022-03-03 18:46 ` [PATCH v2 3/5] dmaengine: dw-edma: add flags at struct dw_edma_chip Frank Li
` (3 subsequent siblings)
4 siblings, 0 replies; 18+ messages in thread
From: Frank Li @ 2022-03-03 18:46 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 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] 18+ messages in thread
* [PATCH v2 3/5] dmaengine: dw-edma: add flags at struct dw_edma_chip
2022-03-03 18:46 [PATCH v2 1/5] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Frank Li
2022-03-03 18:46 ` [PATCH v2 2/5] dmaengine: dw-edma-pcie: don't touch internal struct dw_edma Frank Li
@ 2022-03-03 18:46 ` Frank Li
2022-03-04 16:19 ` Manivannan Sadhasivam
2022-03-04 21:31 ` Bjorn Helgaas
2022-03-03 18:46 ` [PATCH v2 4/5] PCI: imx6: add PCIe embedded DMA support Frank Li
` (2 subsequent siblings)
4 siblings, 2 replies; 18+ messages in thread
From: Frank Li @ 2022-03-03 18:46 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 user don't enable remote MSI irq.
PCI ep probe dma locally, don't want to raise irq
to remote PCI host.
Add option allow force 32bit register access even at
64bit system. i.MX8 hardware only allowed 32bit register
access.
Add option allow EP side probe dma. remote side dma is
continue physical memory, local memory is scatter list.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v1 to v2
- none
drivers/dma/dw-edma/dw-edma-core.c | 7 ++++++-
drivers/dma/dw-edma/dw-edma-v0-core.c | 14 ++++++++++----
include/linux/dma/edma.h | 7 +++++++
3 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 0cb66434f9e14..a43bb26c8bf96 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -336,6 +336,7 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
struct dw_edma_desc *desc;
u32 cnt = 0;
int i;
+ bool b;
if (!chan->configured)
return NULL;
@@ -424,7 +425,11 @@ 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) {
+ b = (chan->dir == EDMA_DIR_WRITE);
+ if (chan->chip->flags & DW_EDMA_CHIP_LOCAL_EP)
+ b = !b;
+
+ if (b) {
burst->sar = src_addr;
if (xfer->type == EDMA_XFER_CYCLIC) {
burst->dar = xfer->xfer.cyclic.paddr;
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index 6e2f83e31a03a..d5c2415e2c616 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -307,13 +307,18 @@ 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;
+ u32 rie = 0;
int j;
lli = chunk->ll_region.vaddr;
+ if (!(chan->chip->flags & DW_EDMA_CHIP_NO_MSI))
+ rie = DW_EDMA_V0_RIE;
+
if (chunk->cb)
control = DW_EDMA_V0_CB;
@@ -321,7 +326,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
list_for_each_entry(child, &chunk->burst->list, list) {
j--;
if (!j)
- control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE);
+ control |= (DW_EDMA_V0_LIE | rie);
/* Channel control */
SET_LL_32(&lli[i].control, control);
@@ -420,15 +425,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
+ if (!(chan->chip->flags & DW_EDMA_CHIP_REG32BIT) &&
+ IS_ENABLED(CONFIG_64BIT)) {
SET_CH_64(chip, chan->dir, chan->id, llp.reg,
chunk->ll_region.paddr);
- #else /* CONFIG_64BIT */
+ } else {
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 */
+ }
}
/* Doorbell */
SET_RW_32(chip, chan->dir, doorbell,
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index fcfbc0f47f83d..e74ee15d9832a 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -33,6 +33,10 @@ enum dw_edma_map_format {
EDMA_MF_HDMA_COMPAT = 0x5
};
+#define DW_EDMA_CHIP_NO_MSI BIT(0)
+#define DW_EDMA_CHIP_REG32BIT BIT(1)
+#define DW_EDMA_CHIP_LOCAL_EP BIT(2)
+
/**
* struct dw_edma_chip - representation of DesignWare eDMA controller hardware
* @dev: struct device of the eDMA controller
@@ -40,6 +44,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_NO_MSI can't generate remote MSI irq
+ * - DW_EDMA_CHIP_REG32BIT only support 32bit register write
* @wr_ch_cnt DMA write channel number
* @rd_ch_cnt DMA read channel number
* @rg_region DMA register region
@@ -53,6 +59,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] 18+ messages in thread
* [PATCH v2 4/5] PCI: imx6: add PCIe embedded DMA support
2022-03-03 18:46 [PATCH v2 1/5] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Frank Li
2022-03-03 18:46 ` [PATCH v2 2/5] dmaengine: dw-edma-pcie: don't touch internal struct dw_edma Frank Li
2022-03-03 18:46 ` [PATCH v2 3/5] dmaengine: dw-edma: add flags at struct dw_edma_chip Frank Li
@ 2022-03-03 18:46 ` Frank Li
2022-03-04 21:33 ` Bjorn Helgaas
2022-03-09 12:01 ` Manivannan Sadhasivam
2022-03-03 18:46 ` [PATCH v2 5/5] PCI: endpoint: functions/pci-epf-test: Support PCI controller DMA Frank Li
2022-03-10 22:07 ` [PATCH v2 1/5] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Zhi Li
4 siblings, 2 replies; 18+ messages in thread
From: Frank Li @ 2022-03-03 18:46 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
Add support for the DMA controller in the DesignWare PCIe core
The DMA can transfer data to any remote address location
regardless PCI address space size.
Prepare struct dw_edma_chip() and call dw_edma_probe()
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
This patch depended on some ep enable patch for imx.
Change from v1 to v2
- rework commit message
- align dw_edma_chip change
drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index efa8b81711090..7dc55986c947d 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -38,6 +38,7 @@
#include "../../pci.h"
#include "pcie-designware.h"
+#include "linux/dma/edma.h"
#define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c
#define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17)
@@ -164,6 +165,8 @@ struct imx6_pcie {
const struct imx6_pcie_drvdata *drvdata;
struct regulator *epdev_on;
struct phy *phy;
+
+ struct dw_edma_chip dma_chip;
};
/* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
@@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
.get_features = imx_pcie_ep_get_features,
};
+static int imx_dma_irq_vector(struct device *dev, unsigned int nr)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+
+ return platform_get_irq_byname(pdev, "dma");
+}
+
+static struct dw_edma_core_ops dma_ops = {
+ .irq_vector = imx_dma_irq_vector,
+};
+
+static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie)
+{
+ unsigned int pcie_dma_offset;
+ struct dw_pcie *pci = imx6_pcie->pci;
+ struct device *dev = pci->dev;
+ struct dw_edma_chip *dma = &imx6_pcie->dma_chip;
+ int i = 0;
+ int sz = PAGE_SIZE;
+
+ pcie_dma_offset = 0x970;
+
+ dma->dev = dev;
+
+ dma->reg_base = pci->dbi_base + pcie_dma_offset;
+
+ dma->ops = &dma_ops;
+ dma->nr_irqs = 1;
+
+ dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP;
+
+ dma->ll_wr_cnt = dma->ll_rd_cnt=1;
+ dma->ll_region_wr[0].sz = sz;
+ dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz,
+ &dma->ll_region_wr[i].paddr,
+ GFP_KERNEL);
+
+ dma->ll_region_rd[0].sz = sz;
+ dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz,
+ &dma->ll_region_rd[i].paddr,
+ GFP_KERNEL);
+
+ return dw_edma_probe(dma);
+}
+
static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie,
struct platform_device *pdev)
{
@@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev)
goto err_ret;
}
+ if (imx_add_pcie_dma(imx6_pcie))
+ dev_info(dev, "pci edma probe failure\n");
+
return 0;
err_ret:
--
2.24.0.rc1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 5/5] PCI: endpoint: functions/pci-epf-test: Support PCI controller DMA
2022-03-03 18:46 [PATCH v2 1/5] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Frank Li
` (2 preceding siblings ...)
2022-03-03 18:46 ` [PATCH v2 4/5] PCI: imx6: add PCIe embedded DMA support Frank Li
@ 2022-03-03 18:46 ` Frank Li
2022-03-10 22:07 ` [PATCH v2 1/5] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Zhi Li
4 siblings, 0 replies; 18+ messages in thread
From: Frank Li @ 2022-03-03 18:46 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 v2
- 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] 18+ messages in thread
* Re: [PATCH v2 3/5] dmaengine: dw-edma: add flags at struct dw_edma_chip
2022-03-03 18:46 ` [PATCH v2 3/5] dmaengine: dw-edma: add flags at struct dw_edma_chip Frank Li
@ 2022-03-04 16:19 ` Manivannan Sadhasivam
2022-03-04 17:16 ` Zhi Li
2022-03-04 21:31 ` Bjorn Helgaas
1 sibling, 1 reply; 18+ messages in thread
From: Manivannan Sadhasivam @ 2022-03-04 16:19 UTC (permalink / raw)
To: Frank Li
Cc: gustavo.pimentel, hongxing.zhu, l.stach, linux-imx, linux-pci,
lznuaa, vkoul, lorenzo.pieralisi, robh, kw, bhelgaas, shawnguo
On Thu, Mar 03, 2022 at 12:46:33PM -0600, Frank Li wrote:
> Allow user don't enable remote MSI irq.
> PCI ep probe dma locally, don't want to raise irq
> to remote PCI host.
>
> Add option allow force 32bit register access even at
> 64bit system. i.MX8 hardware only allowed 32bit register
> access.
>
> Add option allow EP side probe dma. remote side dma is
> continue physical memory, local memory is scatter list.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Change from v1 to v2
> - none
>
> drivers/dma/dw-edma/dw-edma-core.c | 7 ++++++-
> drivers/dma/dw-edma/dw-edma-v0-core.c | 14 ++++++++++----
> include/linux/dma/edma.h | 7 +++++++
> 3 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 0cb66434f9e14..a43bb26c8bf96 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -336,6 +336,7 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
> struct dw_edma_desc *desc;
> u32 cnt = 0;
> int i;
> + bool b;
>
> if (!chan->configured)
> return NULL;
> @@ -424,7 +425,11 @@ 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) {
> + b = (chan->dir == EDMA_DIR_WRITE);
> + if (chan->chip->flags & DW_EDMA_CHIP_LOCAL_EP)
> + b = !b;
> +
> + if (b) {
I've added a patch that uses the xfer direction and channel direction to
find out whether it is a read operation or not. Please take a look:
https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=tracking-qcomlt-sdx55-drivers&id=d6a3f432204614ad8531321949a8a9e2c3e94c3b
The patch could also make use of the "DW_EDMA_CHIP_LOCAL" flag if you
prefer.
> burst->sar = src_addr;
> if (xfer->type == EDMA_XFER_CYCLIC) {
> burst->dar = xfer->xfer.cyclic.paddr;
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index 6e2f83e31a03a..d5c2415e2c616 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -307,13 +307,18 @@ 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;
> + u32 rie = 0;
> int j;
>
> lli = chunk->ll_region.vaddr;
>
> + if (!(chan->chip->flags & DW_EDMA_CHIP_NO_MSI))
> + rie = DW_EDMA_V0_RIE;
> +
> if (chunk->cb)
> control = DW_EDMA_V0_CB;
>
> @@ -321,7 +326,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> list_for_each_entry(child, &chunk->burst->list, list) {
> j--;
> if (!j)
> - control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE);
> + control |= (DW_EDMA_V0_LIE | rie);
I think the MSI makes sense only for eDMA access from remote. So how
about reusing the same flag you used above?
control |= DW_EDMA_V0_LIE;
/* Raise MSI only if the eDMA is accessed by remote */
if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
control |= DW_EDMA_V0_RIE;
>
> /* Channel control */
> SET_LL_32(&lli[i].control, control);
> @@ -420,15 +425,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
> + if (!(chan->chip->flags & DW_EDMA_CHIP_REG32BIT) &&
> + IS_ENABLED(CONFIG_64BIT)) {
> SET_CH_64(chip, chan->dir, chan->id, llp.reg,
> chunk->ll_region.paddr);
Why you are doing 32bit access here only and not in other places?
Thanks,
Mani
> - #else /* CONFIG_64BIT */
> + } else {
> 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 */
> + }
> }
> /* Doorbell */
> SET_RW_32(chip, chan->dir, doorbell,
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index fcfbc0f47f83d..e74ee15d9832a 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -33,6 +33,10 @@ enum dw_edma_map_format {
> EDMA_MF_HDMA_COMPAT = 0x5
> };
>
> +#define DW_EDMA_CHIP_NO_MSI BIT(0)
> +#define DW_EDMA_CHIP_REG32BIT BIT(1)
> +#define DW_EDMA_CHIP_LOCAL_EP BIT(2)
As per my understanding the
> +
> /**
> * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
> * @dev: struct device of the eDMA controller
> @@ -40,6 +44,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_NO_MSI can't generate remote MSI irq
> + * - DW_EDMA_CHIP_REG32BIT only support 32bit register write
> * @wr_ch_cnt DMA write channel number
> * @rd_ch_cnt DMA read channel number
> * @rg_region DMA register region
> @@ -53,6 +59,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] 18+ messages in thread
* Re: [PATCH v2 3/5] dmaengine: dw-edma: add flags at struct dw_edma_chip
2022-03-04 16:19 ` Manivannan Sadhasivam
@ 2022-03-04 17:16 ` Zhi Li
2022-03-07 13:59 ` Manivannan Sadhasivam
0 siblings, 1 reply; 18+ messages in thread
From: Zhi Li @ 2022-03-04 17:16 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Frank Li, gustavo.pimentel, hongxing.zhu, Lucas Stach,
dl-linux-imx, linux-pci, vkoul, lorenzo.pieralisi, robh, kw,
Bjorn Helgaas, Shawn Guo
i
On Fri, Mar 4, 2022 at 10:19 AM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Thu, Mar 03, 2022 at 12:46:33PM -0600, Frank Li wrote:
> > Allow user don't enable remote MSI irq.
> > PCI ep probe dma locally, don't want to raise irq
> > to remote PCI host.
> >
> > Add option allow force 32bit register access even at
> > 64bit system. i.MX8 hardware only allowed 32bit register
> > access.
> >
> > Add option allow EP side probe dma. remote side dma is
> > continue physical memory, local memory is scatter list.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > Change from v1 to v2
> > - none
> >
> > drivers/dma/dw-edma/dw-edma-core.c | 7 ++++++-
> > drivers/dma/dw-edma/dw-edma-v0-core.c | 14 ++++++++++----
> > include/linux/dma/edma.h | 7 +++++++
> > 3 files changed, 23 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index 0cb66434f9e14..a43bb26c8bf96 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -336,6 +336,7 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
> > struct dw_edma_desc *desc;
> > u32 cnt = 0;
> > int i;
> > + bool b;
> >
> > if (!chan->configured)
> > return NULL;
> > @@ -424,7 +425,11 @@ 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) {
> > + b = (chan->dir == EDMA_DIR_WRITE);
> > + if (chan->chip->flags & DW_EDMA_CHIP_LOCAL_EP)
> > + b = !b;
> > +
> > + if (b) {
>
> I've added a patch that uses the xfer direction and channel direction to
> find out whether it is a read operation or not. Please take a look:
>
> https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=tracking-qcomlt-sdx55-drivers&id=d6a3f432204614ad8531321949a8a9e2c3e94c3b
>
I think your patch is better. Can I include your patch into this sequence ?
> The patch could also make use of the "DW_EDMA_CHIP_LOCAL" flag if you
> prefer.
>
> > burst->sar = src_addr;
> > if (xfer->type == EDMA_XFER_CYCLIC) {
> > burst->dar = xfer->xfer.cyclic.paddr;
> > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > index 6e2f83e31a03a..d5c2415e2c616 100644
> > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > @@ -307,13 +307,18 @@ 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;
> > + u32 rie = 0;
> > int j;
> >
> > lli = chunk->ll_region.vaddr;
> >
> > + if (!(chan->chip->flags & DW_EDMA_CHIP_NO_MSI))
> > + rie = DW_EDMA_V0_RIE;
> > +
> > if (chunk->cb)
> > control = DW_EDMA_V0_CB;
> >
> > @@ -321,7 +326,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> > list_for_each_entry(child, &chunk->burst->list, list) {
> > j--;
> > if (!j)
> > - control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE);
> > + control |= (DW_EDMA_V0_LIE | rie);
>
> I think the MSI makes sense only for eDMA access from remote. So how
> about reusing the same flag you used above?
Understand, DW_EDMA_CHIP_NO_MSI is more direct. User can know what
exactly control.
DW_EDMA_CHIP_LOCAL is quite general. User don't know what to do from naming.
I am okay for both naming.
If I pick up your patch for dma dir, Only below two flags need,
#define DW_EDMA_CHIP_NO_MSI BIT(0)
#define DW_EDMA_CHIP_REG32BIT BIT(1)
vs
#define DW_EDMA_CHIP_LOCAL BIT(0)
#define DW_EDMA_CHIP_REG32BIT BIT(1)
which one is better?
>
> control |= DW_EDMA_V0_LIE;
>
> /* Raise MSI only if the eDMA is accessed by remote */
> if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
> control |= DW_EDMA_V0_RIE;
>
> >
> > /* Channel control */
> > SET_LL_32(&lli[i].control, control);
> > @@ -420,15 +425,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
> > + if (!(chan->chip->flags & DW_EDMA_CHIP_REG32BIT) &&
> > + IS_ENABLED(CONFIG_64BIT)) {
> > SET_CH_64(chip, chan->dir, chan->id, llp.reg,
> > chunk->ll_region.paddr);
>
> Why you are doing 32bit access here only and not in other places?
>
Only here access DBI register, other place is access dma link list,
which is memory.
> Thanks,
> Mani
>
> > - #else /* CONFIG_64BIT */
> > + } else {
> > 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 */
> > + }
> > }
> > /* Doorbell */
> > SET_RW_32(chip, chan->dir, doorbell,
> > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> > index fcfbc0f47f83d..e74ee15d9832a 100644
> > --- a/include/linux/dma/edma.h
> > +++ b/include/linux/dma/edma.h
> > @@ -33,6 +33,10 @@ enum dw_edma_map_format {
> > EDMA_MF_HDMA_COMPAT = 0x5
> > };
> >
> > +#define DW_EDMA_CHIP_NO_MSI BIT(0)
> > +#define DW_EDMA_CHIP_REG32BIT BIT(1)
> > +#define DW_EDMA_CHIP_LOCAL_EP BIT(2)
>
> As per my understanding the
what's you mean?
>
> > +
> > /**
> > * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
> > * @dev: struct device of the eDMA controller
> > @@ -40,6 +44,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_NO_MSI can't generate remote MSI irq
> > + * - DW_EDMA_CHIP_REG32BIT only support 32bit register write
> > * @wr_ch_cnt DMA write channel number
> > * @rd_ch_cnt DMA read channel number
> > * @rg_region DMA register region
> > @@ -53,6 +59,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] 18+ messages in thread
* Re: [PATCH v2 3/5] dmaengine: dw-edma: add flags at struct dw_edma_chip
2022-03-03 18:46 ` [PATCH v2 3/5] dmaengine: dw-edma: add flags at struct dw_edma_chip Frank Li
2022-03-04 16:19 ` Manivannan Sadhasivam
@ 2022-03-04 21:31 ` Bjorn Helgaas
1 sibling, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2022-03-04 21:31 UTC (permalink / raw)
To: Frank Li
Cc: gustavo.pimentel, hongxing.zhu, l.stach, linux-imx, linux-pci,
lznuaa, vkoul, lorenzo.pieralisi, robh, kw, bhelgaas, shawnguo,
manivannan.sadhasivam
On Thu, Mar 03, 2022 at 12:46:33PM -0600, Frank Li wrote:
> Allow user don't enable remote MSI irq.
> PCI ep probe dma locally, don't want to raise irq
> to remote PCI host.
>
> Add option allow force 32bit register access even at
> 64bit system. i.MX8 hardware only allowed 32bit register
> access.
>
> Add option allow EP side probe dma. remote side dma is
> continue physical memory, local memory is scatter list.
s/Allow user don't enable remote MSI/Allow user to prevent use of remote MSI/ ?
s/irq/IRQ/ (several, and in comment below)
s/ep/EP/ (capitalize consistently)
s/dma/DMA/ (several)
s/Add option allow force 32bit/Add option to force 32-bit/
s/even at 64bit system/even on 64-bit systems/
s/remote side/Remote side/ (start sentence with capital letter)
s/Add option allow EP side probe dma/Add option to allow EP to probe DMA/
(I'm not quite sure what this means).
s/continue physical memory/contiguous physical memory/ ?
Rewrap above to fill 75 columns.
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Change from v1 to v2
> - none
>
> drivers/dma/dw-edma/dw-edma-core.c | 7 ++++++-
> drivers/dma/dw-edma/dw-edma-v0-core.c | 14 ++++++++++----
> include/linux/dma/edma.h | 7 +++++++
> 3 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 0cb66434f9e14..a43bb26c8bf96 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -336,6 +336,7 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
> struct dw_edma_desc *desc;
> u32 cnt = 0;
> int i;
> + bool b;
>
> if (!chan->configured)
> return NULL;
> @@ -424,7 +425,11 @@ 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) {
> + b = (chan->dir == EDMA_DIR_WRITE);
> + if (chan->chip->flags & DW_EDMA_CHIP_LOCAL_EP)
> + b = !b;
> +
> + if (b) {
> burst->sar = src_addr;
> if (xfer->type == EDMA_XFER_CYCLIC) {
> burst->dar = xfer->xfer.cyclic.paddr;
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index 6e2f83e31a03a..d5c2415e2c616 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -307,13 +307,18 @@ 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;
> + u32 rie = 0;
> int j;
>
> lli = chunk->ll_region.vaddr;
>
> + if (!(chan->chip->flags & DW_EDMA_CHIP_NO_MSI))
> + rie = DW_EDMA_V0_RIE;
> +
> if (chunk->cb)
> control = DW_EDMA_V0_CB;
>
> @@ -321,7 +326,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> list_for_each_entry(child, &chunk->burst->list, list) {
> j--;
> if (!j)
> - control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE);
> + control |= (DW_EDMA_V0_LIE | rie);
Could drop "rie" and do this, which would keep DW_EDMA_CHIP_NO_MSI
closer to where it's used:
if (!j) {
control |= DW_EDMA_V0_LIE;
if (!(chan->chip->flags & DW_EDMA_CHIP_NO_MSI))
control |= DW_EDMA_V0_RIE;
}
> /* Channel control */
> SET_LL_32(&lli[i].control, control);
> @@ -420,15 +425,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
> + if (!(chan->chip->flags & DW_EDMA_CHIP_REG32BIT) &&
> + IS_ENABLED(CONFIG_64BIT)) {
> SET_CH_64(chip, chan->dir, chan->id, llp.reg,
> chunk->ll_region.paddr);
> - #else /* CONFIG_64BIT */
> + } else {
> 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 */
I think this would read better if you reversed the sense:
if ((chan->chip->flags & DW_EDMA_CHIP_REG32BIT) ||
!IS_ENABLED(CONFIG_64BIT)) {
SET_CH_32(...);
SET_CH_32(...);
} else {
SET_CH_64(...);
}
> + }
> }
> /* Doorbell */
> SET_RW_32(chip, chan->dir, doorbell,
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index fcfbc0f47f83d..e74ee15d9832a 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -33,6 +33,10 @@ enum dw_edma_map_format {
> EDMA_MF_HDMA_COMPAT = 0x5
> };
>
> +#define DW_EDMA_CHIP_NO_MSI BIT(0)
> +#define DW_EDMA_CHIP_REG32BIT BIT(1)
> +#define DW_EDMA_CHIP_LOCAL_EP BIT(2)
> +
> /**
> * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
> * @dev: struct device of the eDMA controller
> @@ -40,6 +44,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_NO_MSI can't generate remote MSI irq
> + * - DW_EDMA_CHIP_REG32BIT only support 32bit register write
Looks like DW_EDMA_CHIP_LOCAL_EP should be documented here, too?
> * @wr_ch_cnt DMA write channel number
> * @rd_ch_cnt DMA read channel number
> * @rg_region DMA register region
> @@ -53,6 +59,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] 18+ messages in thread
* Re: [PATCH v2 4/5] PCI: imx6: add PCIe embedded DMA support
2022-03-03 18:46 ` [PATCH v2 4/5] PCI: imx6: add PCIe embedded DMA support Frank Li
@ 2022-03-04 21:33 ` Bjorn Helgaas
2022-03-05 1:31 ` Zhi Li
2022-03-09 12:01 ` Manivannan Sadhasivam
1 sibling, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2022-03-04 21:33 UTC (permalink / raw)
To: Frank Li
Cc: gustavo.pimentel, hongxing.zhu, l.stach, linux-imx, linux-pci,
lznuaa, vkoul, lorenzo.pieralisi, robh, kw, bhelgaas, shawnguo,
manivannan.sadhasivam
On Thu, Mar 03, 2022 at 12:46:34PM -0600, Frank Li wrote:
> Add support for the DMA controller in the DesignWare PCIe core
>
> The DMA can transfer data to any remote address location
> regardless PCI address space size.
>
> Prepare struct dw_edma_chip() and call dw_edma_probe()
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>
> This patch depended on some ep enable patch for imx.
This makes it really hard to apply because we don't know *which* patch
this depends on. Patches that depend on each other should be posted
in the same series, or at the very least, include lore URL to what
they depend on.
> Change from v1 to v2
> - rework commit message
> - align dw_edma_chip change
>
> drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index efa8b81711090..7dc55986c947d 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -38,6 +38,7 @@
> #include "../../pci.h"
>
> #include "pcie-designware.h"
> +#include "linux/dma/edma.h"
>
> #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c
> #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17)
> @@ -164,6 +165,8 @@ struct imx6_pcie {
> const struct imx6_pcie_drvdata *drvdata;
> struct regulator *epdev_on;
> struct phy *phy;
> +
> + struct dw_edma_chip dma_chip;
> };
>
> /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
> @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
> .get_features = imx_pcie_ep_get_features,
> };
>
> +static int imx_dma_irq_vector(struct device *dev, unsigned int nr)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> +
> + return platform_get_irq_byname(pdev, "dma");
> +}
> +
> +static struct dw_edma_core_ops dma_ops = {
> + .irq_vector = imx_dma_irq_vector,
> +};
> +
> +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie)
Rename these functions to use "imx6" prefix, as the other functions in
the file do. Mentioned this last time, too. Also applies to
imx_add_pcie_ep() below; I guess that must be in the other
prerequisite patch?
> +{
> + unsigned int pcie_dma_offset;
> + struct dw_pcie *pci = imx6_pcie->pci;
> + struct device *dev = pci->dev;
> + struct dw_edma_chip *dma = &imx6_pcie->dma_chip;
> + int i = 0;
No need to make a variable for this. Just use 0 below, which will be
easier to read.
> + int sz = PAGE_SIZE;
> +
> + pcie_dma_offset = 0x970;
This would be better as a #define. No point in making a local
variable on the stack.
> + dma->dev = dev;
> +
> + dma->reg_base = pci->dbi_base + pcie_dma_offset;
> +
> + dma->ops = &dma_ops;
> + dma->nr_irqs = 1;
> +
> + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP;
> +
> + dma->ll_wr_cnt = dma->ll_rd_cnt=1;
> + dma->ll_region_wr[0].sz = sz;
> + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz,
> + &dma->ll_region_wr[i].paddr,
> + GFP_KERNEL);
> +
> + dma->ll_region_rd[0].sz = sz;
> + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz,
> + &dma->ll_region_rd[i].paddr,
> + GFP_KERNEL);
Wrap all the above to fit in 80 columns.
I would consider something like this to reduce some of the repetition:
struct dw_edma_region *region;
dma->ll_wr_cnt = 1;
region = &dma->ll_region_wr[0];
region->sz = sz;
region->vaddr = dmam_alloc_coherent(dev, sz, ®ion->paddr, GFP_KERNEL);
dma->ll_rd_cnt = 1;
region = &dma->ll_region_rd[0];
region->sz = sz;
region->vaddr = dmam_alloc_coherent(dev, sz, ®ion->paddr, GFP_KERNEL);
> +
> + return dw_edma_probe(dma);
> +}
> +
> static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie,
> struct platform_device *pdev)
> {
> @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> goto err_ret;
> }
>
> + if (imx_add_pcie_dma(imx6_pcie))
> + dev_info(dev, "pci edma probe failure\n");
> +
> return 0;
>
> err_ret:
> --
> 2.24.0.rc1
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/5] PCI: imx6: add PCIe embedded DMA support
2022-03-04 21:33 ` Bjorn Helgaas
@ 2022-03-05 1:31 ` Zhi Li
0 siblings, 0 replies; 18+ messages in thread
From: Zhi Li @ 2022-03-05 1:31 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Frank Li, gustavo.pimentel, hongxing.zhu, Lucas Stach,
dl-linux-imx, linux-pci, vkoul, lorenzo.pieralisi, robh, kw,
Bjorn Helgaas, Shawn Guo, Manivannan Sadhasivam
On Fri, Mar 4, 2022 at 3:33 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Thu, Mar 03, 2022 at 12:46:34PM -0600, Frank Li wrote:
> > Add support for the DMA controller in the DesignWare PCIe core
> >
> > The DMA can transfer data to any remote address location
> > regardless PCI address space size.
> >
> > Prepare struct dw_edma_chip() and call dw_edma_probe()
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >
> > This patch depended on some ep enable patch for imx.
>
> This makes it really hard to apply because we don't know *which* patch
> this depends on. Patches that depend on each other should be posted
> in the same series, or at the very least, include lore URL to what
> they depend on.
I did not realize It was missing some EP support patches in pci-imx6.c
upstream at v1.
Richard is working on this upstream work.
This patch don't impact previous patch 1-3 and 5
I will skip this patch next time and wait for richard finish imx6 EP
function upstream.
Other patches are independent and can be merged separately.
With patch 1-3 and 5, other platforms rather than imx that using
designware pci ip can be benefited.
>
> > Change from v1 to v2
> > - rework commit message
> > - align dw_edma_chip change
> >
> > drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++
> > 1 file changed, 51 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index efa8b81711090..7dc55986c947d 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -38,6 +38,7 @@
> > #include "../../pci.h"
> >
> > #include "pcie-designware.h"
> > +#include "linux/dma/edma.h"
> >
> > #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c
> > #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17)
> > @@ -164,6 +165,8 @@ struct imx6_pcie {
> > const struct imx6_pcie_drvdata *drvdata;
> > struct regulator *epdev_on;
> > struct phy *phy;
> > +
> > + struct dw_edma_chip dma_chip;
> > };
> >
> > /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
> > @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
> > .get_features = imx_pcie_ep_get_features,
> > };
> >
> > +static int imx_dma_irq_vector(struct device *dev, unsigned int nr)
> > +{
> > + struct platform_device *pdev = to_platform_device(dev);
> > +
> > + return platform_get_irq_byname(pdev, "dma");
> > +}
> > +
> > +static struct dw_edma_core_ops dma_ops = {
> > + .irq_vector = imx_dma_irq_vector,
> > +};
> > +
> > +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie)
>
> Rename these functions to use "imx6" prefix, as the other functions in
> the file do. Mentioned this last time, too. Also applies to
> imx_add_pcie_ep() below; I guess that must be in the other
> prerequisite patch?
>
> > +{
> > + unsigned int pcie_dma_offset;
> > + struct dw_pcie *pci = imx6_pcie->pci;
> > + struct device *dev = pci->dev;
> > + struct dw_edma_chip *dma = &imx6_pcie->dma_chip;
> > + int i = 0;
>
> No need to make a variable for this. Just use 0 below, which will be
> easier to read.
>
> > + int sz = PAGE_SIZE;
> > +
> > + pcie_dma_offset = 0x970;
>
> This would be better as a #define. No point in making a local
> variable on the stack.
>
> > + dma->dev = dev;
> > +
> > + dma->reg_base = pci->dbi_base + pcie_dma_offset;
> > +
> > + dma->ops = &dma_ops;
> > + dma->nr_irqs = 1;
> > +
> > + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP;
> > +
> > + dma->ll_wr_cnt = dma->ll_rd_cnt=1;
> > + dma->ll_region_wr[0].sz = sz;
> > + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz,
> > + &dma->ll_region_wr[i].paddr,
> > + GFP_KERNEL);
> > +
> > + dma->ll_region_rd[0].sz = sz;
> > + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz,
> > + &dma->ll_region_rd[i].paddr,
> > + GFP_KERNEL);
>
> Wrap all the above to fit in 80 columns.
>
> I would consider something like this to reduce some of the repetition:
>
> struct dw_edma_region *region;
>
> dma->ll_wr_cnt = 1;
> region = &dma->ll_region_wr[0];
> region->sz = sz;
> region->vaddr = dmam_alloc_coherent(dev, sz, ®ion->paddr, GFP_KERNEL);
>
> dma->ll_rd_cnt = 1;
> region = &dma->ll_region_rd[0];
> region->sz = sz;
> region->vaddr = dmam_alloc_coherent(dev, sz, ®ion->paddr, GFP_KERNEL);
>
> > +
> > + return dw_edma_probe(dma);
> > +}
> > +
> > static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie,
> > struct platform_device *pdev)
> > {
> > @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > goto err_ret;
> > }
> >
> > + if (imx_add_pcie_dma(imx6_pcie))
> > + dev_info(dev, "pci edma probe failure\n");
> > +
> > return 0;
> >
> > err_ret:
> > --
> > 2.24.0.rc1
> >
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/5] dmaengine: dw-edma: add flags at struct dw_edma_chip
2022-03-04 17:16 ` Zhi Li
@ 2022-03-07 13:59 ` Manivannan Sadhasivam
0 siblings, 0 replies; 18+ messages in thread
From: Manivannan Sadhasivam @ 2022-03-07 13:59 UTC (permalink / raw)
To: Zhi Li
Cc: Frank Li, gustavo.pimentel, hongxing.zhu, Lucas Stach,
dl-linux-imx, linux-pci, vkoul, lorenzo.pieralisi, robh, kw,
Bjorn Helgaas, Shawn Guo
On Fri, Mar 04, 2022 at 11:16:11AM -0600, Zhi Li wrote:
> i
>
> On Fri, Mar 4, 2022 at 10:19 AM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Thu, Mar 03, 2022 at 12:46:33PM -0600, Frank Li wrote:
> > > Allow user don't enable remote MSI irq.
> > > PCI ep probe dma locally, don't want to raise irq
> > > to remote PCI host.
> > >
> > > Add option allow force 32bit register access even at
> > > 64bit system. i.MX8 hardware only allowed 32bit register
> > > access.
> > >
> > > Add option allow EP side probe dma. remote side dma is
> > > continue physical memory, local memory is scatter list.
> > >
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > > Change from v1 to v2
> > > - none
> > >
> > > drivers/dma/dw-edma/dw-edma-core.c | 7 ++++++-
> > > drivers/dma/dw-edma/dw-edma-v0-core.c | 14 ++++++++++----
> > > include/linux/dma/edma.h | 7 +++++++
> > > 3 files changed, 23 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > > index 0cb66434f9e14..a43bb26c8bf96 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > > @@ -336,6 +336,7 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
> > > struct dw_edma_desc *desc;
> > > u32 cnt = 0;
> > > int i;
> > > + bool b;
> > >
> > > if (!chan->configured)
> > > return NULL;
> > > @@ -424,7 +425,11 @@ 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) {
> > > + b = (chan->dir == EDMA_DIR_WRITE);
> > > + if (chan->chip->flags & DW_EDMA_CHIP_LOCAL_EP)
> > > + b = !b;
> > > +
> > > + if (b) {
> >
> > I've added a patch that uses the xfer direction and channel direction to
> > find out whether it is a read operation or not. Please take a look:
> >
> > https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=tracking-qcomlt-sdx55-drivers&id=d6a3f432204614ad8531321949a8a9e2c3e94c3b
> >
>
> I think your patch is better. Can I include your patch into this sequence ?
>
Yes, feel free to. Also, there is one more cleanup patch I've added:
https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=tracking-qcomlt-sdx55-drivers&id=85eb58bd078000dd938487f560cee5d259f06577
> > The patch could also make use of the "DW_EDMA_CHIP_LOCAL" flag if you
> > prefer.
> >
> > > burst->sar = src_addr;
> > > if (xfer->type == EDMA_XFER_CYCLIC) {
> > > burst->dar = xfer->xfer.cyclic.paddr;
> > > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > index 6e2f83e31a03a..d5c2415e2c616 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > @@ -307,13 +307,18 @@ 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;
> > > + u32 rie = 0;
> > > int j;
> > >
> > > lli = chunk->ll_region.vaddr;
> > >
> > > + if (!(chan->chip->flags & DW_EDMA_CHIP_NO_MSI))
> > > + rie = DW_EDMA_V0_RIE;
> > > +
> > > if (chunk->cb)
> > > control = DW_EDMA_V0_CB;
> > >
> > > @@ -321,7 +326,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> > > list_for_each_entry(child, &chunk->burst->list, list) {
> > > j--;
> > > if (!j)
> > > - control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE);
> > > + control |= (DW_EDMA_V0_LIE | rie);
> >
> > I think the MSI makes sense only for eDMA access from remote. So how
> > about reusing the same flag you used above?
>
> Understand, DW_EDMA_CHIP_NO_MSI is more direct. User can know what
> exactly control.
>
Right but isn't using "DW_EDMA_CHIP_LOCAL" implies NO_MSI? Or is there
any endpoint implementation that makes use of MSIs also?
> DW_EDMA_CHIP_LOCAL is quite general. User don't know what to do from naming.
>
> I am okay for both naming.
>
> If I pick up your patch for dma dir, Only below two flags need,
> #define DW_EDMA_CHIP_NO_MSI BIT(0)
> #define DW_EDMA_CHIP_REG32BIT BIT(1)
>
> vs
> #define DW_EDMA_CHIP_LOCAL BIT(0)
> #define DW_EDMA_CHIP_REG32BIT BIT(1)
How about "DW_EDMA_CHIP_32BIT_DBI"?
Also add comments for both flags.
>
> which one is better?
>
> >
> > control |= DW_EDMA_V0_LIE;
> >
> > /* Raise MSI only if the eDMA is accessed by remote */
> > if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL))
> > control |= DW_EDMA_V0_RIE;
> >
> > >
> > > /* Channel control */
> > > SET_LL_32(&lli[i].control, control);
> > > @@ -420,15 +425,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
> > > + if (!(chan->chip->flags & DW_EDMA_CHIP_REG32BIT) &&
> > > + IS_ENABLED(CONFIG_64BIT)) {
> > > SET_CH_64(chip, chan->dir, chan->id, llp.reg,
> > > chunk->ll_region.paddr);
> >
> > Why you are doing 32bit access here only and not in other places?
> >
>
> Only here access DBI register, other place is access dma link list,
> which is memory.
>
Okay. Then you need to specify it clearly in commit message as well.
Thanks,
Mani
> > Thanks,
> > Mani
> >
> > > - #else /* CONFIG_64BIT */
> > > + } else {
> > > 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 */
> > > + }
> > > }
> > > /* Doorbell */
> > > SET_RW_32(chip, chan->dir, doorbell,
> > > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> > > index fcfbc0f47f83d..e74ee15d9832a 100644
> > > --- a/include/linux/dma/edma.h
> > > +++ b/include/linux/dma/edma.h
> > > @@ -33,6 +33,10 @@ enum dw_edma_map_format {
> > > EDMA_MF_HDMA_COMPAT = 0x5
> > > };
> > >
> > > +#define DW_EDMA_CHIP_NO_MSI BIT(0)
> > > +#define DW_EDMA_CHIP_REG32BIT BIT(1)
> > > +#define DW_EDMA_CHIP_LOCAL_EP BIT(2)
> >
> > As per my understanding the
>
> what's you mean?
>
> >
> > > +
> > > /**
> > > * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
> > > * @dev: struct device of the eDMA controller
> > > @@ -40,6 +44,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_NO_MSI can't generate remote MSI irq
> > > + * - DW_EDMA_CHIP_REG32BIT only support 32bit register write
> > > * @wr_ch_cnt DMA write channel number
> > > * @rd_ch_cnt DMA read channel number
> > > * @rg_region DMA register region
> > > @@ -53,6 +59,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] 18+ messages in thread
* Re: [PATCH v2 4/5] PCI: imx6: add PCIe embedded DMA support
2022-03-03 18:46 ` [PATCH v2 4/5] PCI: imx6: add PCIe embedded DMA support Frank Li
2022-03-04 21:33 ` Bjorn Helgaas
@ 2022-03-09 12:01 ` Manivannan Sadhasivam
2022-03-09 18:31 ` Zhi Li
1 sibling, 1 reply; 18+ messages in thread
From: Manivannan Sadhasivam @ 2022-03-09 12:01 UTC (permalink / raw)
To: Frank Li
Cc: gustavo.pimentel, hongxing.zhu, l.stach, linux-imx, linux-pci,
lznuaa, vkoul, lorenzo.pieralisi, robh, kw, bhelgaas, shawnguo
On Thu, Mar 03, 2022 at 12:46:34PM -0600, Frank Li wrote:
> Add support for the DMA controller in the DesignWare PCIe core
>
> The DMA can transfer data to any remote address location
> regardless PCI address space size.
>
> Prepare struct dw_edma_chip() and call dw_edma_probe()
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>
> This patch depended on some ep enable patch for imx.
>
> Change from v1 to v2
> - rework commit message
> - align dw_edma_chip change
>
> drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index efa8b81711090..7dc55986c947d 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -38,6 +38,7 @@
> #include "../../pci.h"
>
> #include "pcie-designware.h"
> +#include "linux/dma/edma.h"
>
> #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c
> #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17)
> @@ -164,6 +165,8 @@ struct imx6_pcie {
> const struct imx6_pcie_drvdata *drvdata;
> struct regulator *epdev_on;
> struct phy *phy;
> +
> + struct dw_edma_chip dma_chip;
> };
>
> /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
> @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
> .get_features = imx_pcie_ep_get_features,
> };
>
> +static int imx_dma_irq_vector(struct device *dev, unsigned int nr)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> +
> + return platform_get_irq_byname(pdev, "dma");
> +}
> +
> +static struct dw_edma_core_ops dma_ops = {
> + .irq_vector = imx_dma_irq_vector,
> +};
> +
> +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie)
> +{
> + unsigned int pcie_dma_offset;
> + struct dw_pcie *pci = imx6_pcie->pci;
> + struct device *dev = pci->dev;
> + struct dw_edma_chip *dma = &imx6_pcie->dma_chip;
> + int i = 0;
Unused?
> + int sz = PAGE_SIZE;
> +
> + pcie_dma_offset = 0x970;
Can you get this offset from the devicetree node of ep?
> +
> + dma->dev = dev;
> +
> + dma->reg_base = pci->dbi_base + pcie_dma_offset;
> +
> + dma->ops = &dma_ops;
> + dma->nr_irqs = 1;
> +
> + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP;
> +
> + dma->ll_wr_cnt = dma->ll_rd_cnt=1;
Is this a hard limitation of the eDMA implementation or because of difficulties
in requesting the correct channel from client driver?
If it's the latter, you could use my patch:
https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=tracking-qcomlt-sdx55-drivers&id=c77ad9d929372b1ff495709714b24486d266a810
> + dma->ll_region_wr[0].sz = sz;
> + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz,
> + &dma->ll_region_wr[i].paddr,
> + GFP_KERNEL);
Allocation could fail. Please add error checking here and below.
Thanks,
Mani
> +
> + dma->ll_region_rd[0].sz = sz;
> + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz,
> + &dma->ll_region_rd[i].paddr,
> + GFP_KERNEL);
> +
> + return dw_edma_probe(dma);
> +}
> +
> static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie,
> struct platform_device *pdev)
> {
> @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> goto err_ret;
> }
>
> + if (imx_add_pcie_dma(imx6_pcie))
> + dev_info(dev, "pci edma probe failure\n");
> +
> return 0;
>
> err_ret:
> --
> 2.24.0.rc1
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/5] PCI: imx6: add PCIe embedded DMA support
2022-03-09 12:01 ` Manivannan Sadhasivam
@ 2022-03-09 18:31 ` Zhi Li
2022-03-09 18:42 ` Manivannan Sadhasivam
0 siblings, 1 reply; 18+ messages in thread
From: Zhi Li @ 2022-03-09 18:31 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Frank Li, gustavo.pimentel, hongxing.zhu, Lucas Stach,
dl-linux-imx, linux-pci, vkoul, lorenzo.pieralisi, robh, kw,
Bjorn Helgaas, Shawn Guo
On Wed, Mar 9, 2022 at 6:02 AM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Thu, Mar 03, 2022 at 12:46:34PM -0600, Frank Li wrote:
> > Add support for the DMA controller in the DesignWare PCIe core
> >
> > The DMA can transfer data to any remote address location
> > regardless PCI address space size.
> >
> > Prepare struct dw_edma_chip() and call dw_edma_probe()
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >
> > This patch depended on some ep enable patch for imx.
> >
> > Change from v1 to v2
> > - rework commit message
> > - align dw_edma_chip change
> >
> > drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++
> > 1 file changed, 51 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index efa8b81711090..7dc55986c947d 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -38,6 +38,7 @@
> > #include "../../pci.h"
> >
> > #include "pcie-designware.h"
> > +#include "linux/dma/edma.h"
> >
> > #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c
> > #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17)
> > @@ -164,6 +165,8 @@ struct imx6_pcie {
> > const struct imx6_pcie_drvdata *drvdata;
> > struct regulator *epdev_on;
> > struct phy *phy;
> > +
> > + struct dw_edma_chip dma_chip;
> > };
> >
> > /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
> > @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
> > .get_features = imx_pcie_ep_get_features,
> > };
> >
> > +static int imx_dma_irq_vector(struct device *dev, unsigned int nr)
> > +{
> > + struct platform_device *pdev = to_platform_device(dev);
> > +
> > + return platform_get_irq_byname(pdev, "dma");
> > +}
> > +
> > +static struct dw_edma_core_ops dma_ops = {
> > + .irq_vector = imx_dma_irq_vector,
> > +};
> > +
> > +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie)
> > +{
> > + unsigned int pcie_dma_offset;
> > + struct dw_pcie *pci = imx6_pcie->pci;
> > + struct device *dev = pci->dev;
> > + struct dw_edma_chip *dma = &imx6_pcie->dma_chip;
> > + int i = 0;
>
> Unused?
>
> > + int sz = PAGE_SIZE;
> > +
> > + pcie_dma_offset = 0x970;
>
> Can you get this offset from the devicetree node of ep?
>
> > +
> > + dma->dev = dev;
> > +
> > + dma->reg_base = pci->dbi_base + pcie_dma_offset;
> > +
> > + dma->ops = &dma_ops;
> > + dma->nr_irqs = 1;
> > +
> > + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP;
> > +
> > + dma->ll_wr_cnt = dma->ll_rd_cnt=1;
>
> Is this a hard limitation of the eDMA implementation or because of difficulties
> in requesting the correct channel from client driver?
>
> If it's the latter, you could use my patch:
It is because our hardware only has 1 channel.
>
> https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=tracking-qcomlt-sdx55-drivers&id=c77ad9d929372b1ff495709714b24486d266a810
My problem is
in dw_edma_channel_setup()
dma->directions = BIT(write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV);
Already set direction. why need overwrite default device_caps?
>
> > + dma->ll_region_wr[0].sz = sz;
> > + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz,
> > + &dma->ll_region_wr[i].paddr,
> > + GFP_KERNEL);
>
> Allocation could fail. Please add error checking here and below.
>
> Thanks,
> Mani
>
> > +
> > + dma->ll_region_rd[0].sz = sz;
> > + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz,
> > + &dma->ll_region_rd[i].paddr,
> > + GFP_KERNEL);
> > +
> > + return dw_edma_probe(dma);
> > +}
> > +
> > static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie,
> > struct platform_device *pdev)
> > {
> > @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > goto err_ret;
> > }
> >
> > + if (imx_add_pcie_dma(imx6_pcie))
> > + dev_info(dev, "pci edma probe failure\n");
> > +
> > return 0;
> >
> > err_ret:
> > --
> > 2.24.0.rc1
> >
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/5] PCI: imx6: add PCIe embedded DMA support
2022-03-09 18:31 ` Zhi Li
@ 2022-03-09 18:42 ` Manivannan Sadhasivam
2022-03-09 19:14 ` Zhi Li
0 siblings, 1 reply; 18+ messages in thread
From: Manivannan Sadhasivam @ 2022-03-09 18:42 UTC (permalink / raw)
To: Zhi Li
Cc: Frank Li, gustavo.pimentel, hongxing.zhu, Lucas Stach,
dl-linux-imx, linux-pci, vkoul, lorenzo.pieralisi, robh, kw,
Bjorn Helgaas, Shawn Guo
On Wed, Mar 09, 2022 at 12:31:57PM -0600, Zhi Li wrote:
> On Wed, Mar 9, 2022 at 6:02 AM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Thu, Mar 03, 2022 at 12:46:34PM -0600, Frank Li wrote:
> > > Add support for the DMA controller in the DesignWare PCIe core
> > >
> > > The DMA can transfer data to any remote address location
> > > regardless PCI address space size.
> > >
> > > Prepare struct dw_edma_chip() and call dw_edma_probe()
> > >
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > >
> > > This patch depended on some ep enable patch for imx.
> > >
> > > Change from v1 to v2
> > > - rework commit message
> > > - align dw_edma_chip change
> > >
> > > drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++
> > > 1 file changed, 51 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > > index efa8b81711090..7dc55986c947d 100644
> > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > @@ -38,6 +38,7 @@
> > > #include "../../pci.h"
> > >
> > > #include "pcie-designware.h"
> > > +#include "linux/dma/edma.h"
> > >
> > > #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c
> > > #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17)
> > > @@ -164,6 +165,8 @@ struct imx6_pcie {
> > > const struct imx6_pcie_drvdata *drvdata;
> > > struct regulator *epdev_on;
> > > struct phy *phy;
> > > +
> > > + struct dw_edma_chip dma_chip;
> > > };
> > >
> > > /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
> > > @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
> > > .get_features = imx_pcie_ep_get_features,
> > > };
> > >
> > > +static int imx_dma_irq_vector(struct device *dev, unsigned int nr)
> > > +{
> > > + struct platform_device *pdev = to_platform_device(dev);
> > > +
> > > + return platform_get_irq_byname(pdev, "dma");
> > > +}
> > > +
> > > +static struct dw_edma_core_ops dma_ops = {
> > > + .irq_vector = imx_dma_irq_vector,
> > > +};
> > > +
> > > +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie)
> > > +{
> > > + unsigned int pcie_dma_offset;
> > > + struct dw_pcie *pci = imx6_pcie->pci;
> > > + struct device *dev = pci->dev;
> > > + struct dw_edma_chip *dma = &imx6_pcie->dma_chip;
> > > + int i = 0;
> >
> > Unused?
> >
> > > + int sz = PAGE_SIZE;
> > > +
> > > + pcie_dma_offset = 0x970;
> >
> > Can you get this offset from the devicetree node of ep?
> >
> > > +
> > > + dma->dev = dev;
> > > +
> > > + dma->reg_base = pci->dbi_base + pcie_dma_offset;
> > > +
> > > + dma->ops = &dma_ops;
> > > + dma->nr_irqs = 1;
> > > +
> > > + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP;
> > > +
> > > + dma->ll_wr_cnt = dma->ll_rd_cnt=1;
> >
> > Is this a hard limitation of the eDMA implementation or because of difficulties
> > in requesting the correct channel from client driver?
> >
> > If it's the latter, you could use my patch:
>
> It is because our hardware only has 1 channel.
Ah okay. It is fine if that's the case.
>
> >
> > https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=tracking-qcomlt-sdx55-drivers&id=c77ad9d929372b1ff495709714b24486d266a810
>
> My problem is
> in dw_edma_channel_setup()
> dma->directions = BIT(write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV);
>
> Already set direction. why need overwrite default device_caps?
>
The above sets both direction in the caps. That's fine if you want to verify
whether the channel is valid or not. But that won't help you want to choose the
correct channel.
In your case it will work because, read and write channel count is 1. But what
if the channel count is 8?
write channel - 0 to 7
read channel - 8 to 15
Now without identifying the channel direction, the client would be getting the
wrong one. For instance, if client requests read channel after write, the
dmaengine would pass 1 because the requested direction matches the caps and
that's wrong.
Thanks,
Mani
> >
> > > + dma->ll_region_wr[0].sz = sz;
> > > + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz,
> > > + &dma->ll_region_wr[i].paddr,
> > > + GFP_KERNEL);
> >
> > Allocation could fail. Please add error checking here and below.
> >
> > Thanks,
> > Mani
> >
> > > +
> > > + dma->ll_region_rd[0].sz = sz;
> > > + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz,
> > > + &dma->ll_region_rd[i].paddr,
> > > + GFP_KERNEL);
> > > +
> > > + return dw_edma_probe(dma);
> > > +}
> > > +
> > > static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie,
> > > struct platform_device *pdev)
> > > {
> > > @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > > goto err_ret;
> > > }
> > >
> > > + if (imx_add_pcie_dma(imx6_pcie))
> > > + dev_info(dev, "pci edma probe failure\n");
> > > +
> > > return 0;
> > >
> > > err_ret:
> > > --
> > > 2.24.0.rc1
> > >
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/5] PCI: imx6: add PCIe embedded DMA support
2022-03-09 18:42 ` Manivannan Sadhasivam
@ 2022-03-09 19:14 ` Zhi Li
2022-03-09 20:48 ` Zhi Li
2022-03-10 4:45 ` Manivannan Sadhasivam
0 siblings, 2 replies; 18+ messages in thread
From: Zhi Li @ 2022-03-09 19:14 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Frank Li, gustavo.pimentel, hongxing.zhu, Lucas Stach,
dl-linux-imx, linux-pci, vkoul, lorenzo.pieralisi, robh, kw,
Bjorn Helgaas, Shawn Guo
On Wed, Mar 9, 2022 at 12:42 PM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Wed, Mar 09, 2022 at 12:31:57PM -0600, Zhi Li wrote:
> > On Wed, Mar 9, 2022 at 6:02 AM Manivannan Sadhasivam
> > <manivannan.sadhasivam@linaro.org> wrote:
> > >
> > > On Thu, Mar 03, 2022 at 12:46:34PM -0600, Frank Li wrote:
> > > > Add support for the DMA controller in the DesignWare PCIe core
> > > >
> > > > The DMA can transfer data to any remote address location
> > > > regardless PCI address space size.
> > > >
> > > > Prepare struct dw_edma_chip() and call dw_edma_probe()
> > > >
> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > >
> > > > This patch depended on some ep enable patch for imx.
> > > >
> > > > Change from v1 to v2
> > > > - rework commit message
> > > > - align dw_edma_chip change
> > > >
> > > > drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++
> > > > 1 file changed, 51 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > > > index efa8b81711090..7dc55986c947d 100644
> > > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > > @@ -38,6 +38,7 @@
> > > > #include "../../pci.h"
> > > >
> > > > #include "pcie-designware.h"
> > > > +#include "linux/dma/edma.h"
> > > >
> > > > #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c
> > > > #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17)
> > > > @@ -164,6 +165,8 @@ struct imx6_pcie {
> > > > const struct imx6_pcie_drvdata *drvdata;
> > > > struct regulator *epdev_on;
> > > > struct phy *phy;
> > > > +
> > > > + struct dw_edma_chip dma_chip;
> > > > };
> > > >
> > > > /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
> > > > @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
> > > > .get_features = imx_pcie_ep_get_features,
> > > > };
> > > >
> > > > +static int imx_dma_irq_vector(struct device *dev, unsigned int nr)
> > > > +{
> > > > + struct platform_device *pdev = to_platform_device(dev);
> > > > +
> > > > + return platform_get_irq_byname(pdev, "dma");
> > > > +}
> > > > +
> > > > +static struct dw_edma_core_ops dma_ops = {
> > > > + .irq_vector = imx_dma_irq_vector,
> > > > +};
> > > > +
> > > > +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie)
> > > > +{
> > > > + unsigned int pcie_dma_offset;
> > > > + struct dw_pcie *pci = imx6_pcie->pci;
> > > > + struct device *dev = pci->dev;
> > > > + struct dw_edma_chip *dma = &imx6_pcie->dma_chip;
> > > > + int i = 0;
> > >
> > > Unused?
> > >
> > > > + int sz = PAGE_SIZE;
> > > > +
> > > > + pcie_dma_offset = 0x970;
> > >
> > > Can you get this offset from the devicetree node of ep?
> > >
> > > > +
> > > > + dma->dev = dev;
> > > > +
> > > > + dma->reg_base = pci->dbi_base + pcie_dma_offset;
> > > > +
> > > > + dma->ops = &dma_ops;
> > > > + dma->nr_irqs = 1;
> > > > +
> > > > + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP;
> > > > +
> > > > + dma->ll_wr_cnt = dma->ll_rd_cnt=1;
> > >
> > > Is this a hard limitation of the eDMA implementation or because of difficulties
> > > in requesting the correct channel from client driver?
> > >
> > > If it's the latter, you could use my patch:
> >
> > It is because our hardware only has 1 channel.
>
> Ah okay. It is fine if that's the case.
>
> >
> > >
> > > https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=tracking-qcomlt-sdx55-drivers&id=c77ad9d929372b1ff495709714b24486d266a810
> >
> > My problem is
> > in dw_edma_channel_setup()
> > dma->directions = BIT(write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV);
> >
> > Already set direction. why need overwrite default device_caps?
> >
>
> The above sets both direction in the caps.
Why set both direction?
write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV
Only one bit set
> That's fine if you want to verify
> whether the channel is valid or not. But that won't help you want to choose the
> correct channel.
>
> In your case it will work because, read and write channel count is 1. But what
> if the channel count is 8?
I know my case is special one. I just feel strange.
dma_async_device_register(dma); register two devices, one read, one write.
The default int dma_get_slave_caps(struct dma_chan *chan, struct
dma_slave_caps *caps)
{
device = chan->device;
caps->directions = device->directions;
}
It should return read/write devices's directions.
>
> write channel - 0 to 7
> read channel - 8 to 15
>
> Now without identifying the channel direction, the client would be getting the
> wrong one. For instance, if client requests read channel after write, the
> dmaengine would pass 1 because the requested direction matches the caps and
> that's wrong.
Do you mean if I request a read after write can reproduce the problem?
>
> Thanks,
> Mani
>
> > >
> > > > + dma->ll_region_wr[0].sz = sz;
> > > > + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz,
> > > > + &dma->ll_region_wr[i].paddr,
> > > > + GFP_KERNEL);
> > >
> > > Allocation could fail. Please add error checking here and below.
> > >
> > > Thanks,
> > > Mani
> > >
> > > > +
> > > > + dma->ll_region_rd[0].sz = sz;
> > > > + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz,
> > > > + &dma->ll_region_rd[i].paddr,
> > > > + GFP_KERNEL);
> > > > +
> > > > + return dw_edma_probe(dma);
> > > > +}
> > > > +
> > > > static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie,
> > > > struct platform_device *pdev)
> > > > {
> > > > @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > > > goto err_ret;
> > > > }
> > > >
> > > > + if (imx_add_pcie_dma(imx6_pcie))
> > > > + dev_info(dev, "pci edma probe failure\n");
> > > > +
> > > > return 0;
> > > >
> > > > err_ret:
> > > > --
> > > > 2.24.0.rc1
> > > >
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/5] PCI: imx6: add PCIe embedded DMA support
2022-03-09 19:14 ` Zhi Li
@ 2022-03-09 20:48 ` Zhi Li
2022-03-10 4:45 ` Manivannan Sadhasivam
1 sibling, 0 replies; 18+ messages in thread
From: Zhi Li @ 2022-03-09 20:48 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Frank Li, gustavo.pimentel, hongxing.zhu, Lucas Stach,
dl-linux-imx, linux-pci, vkoul, lorenzo.pieralisi, robh, kw,
Bjorn Helgaas, Shawn Guo
On Wed, Mar 9, 2022 at 1:14 PM Zhi Li <lznuaa@gmail.com> wrote:
>
> On Wed, Mar 9, 2022 at 12:42 PM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Wed, Mar 09, 2022 at 12:31:57PM -0600, Zhi Li wrote:
> > > On Wed, Mar 9, 2022 at 6:02 AM Manivannan Sadhasivam
> > > <manivannan.sadhasivam@linaro.org> wrote:
> > > >
> > > > On Thu, Mar 03, 2022 at 12:46:34PM -0600, Frank Li wrote:
> > > > > Add support for the DMA controller in the DesignWare PCIe core
> > > > >
> > > > > The DMA can transfer data to any remote address location
> > > > > regardless PCI address space size.
> > > > >
> > > > > Prepare struct dw_edma_chip() and call dw_edma_probe()
> > > > >
> > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > ---
> > > > >
> > > > > This patch depended on some ep enable patch for imx.
> > > > >
> > > > > Change from v1 to v2
> > > > > - rework commit message
> > > > > - align dw_edma_chip change
> > > > >
> > > > > drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++
> > > > > 1 file changed, 51 insertions(+)
> > > > >
> > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > > > > index efa8b81711090..7dc55986c947d 100644
> > > > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > > > @@ -38,6 +38,7 @@
> > > > > #include "../../pci.h"
> > > > >
> > > > > #include "pcie-designware.h"
> > > > > +#include "linux/dma/edma.h"
> > > > >
> > > > > #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c
> > > > > #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17)
> > > > > @@ -164,6 +165,8 @@ struct imx6_pcie {
> > > > > const struct imx6_pcie_drvdata *drvdata;
> > > > > struct regulator *epdev_on;
> > > > > struct phy *phy;
> > > > > +
> > > > > + struct dw_edma_chip dma_chip;
> > > > > };
> > > > >
> > > > > /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
> > > > > @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
> > > > > .get_features = imx_pcie_ep_get_features,
> > > > > };
> > > > >
> > > > > +static int imx_dma_irq_vector(struct device *dev, unsigned int nr)
> > > > > +{
> > > > > + struct platform_device *pdev = to_platform_device(dev);
> > > > > +
> > > > > + return platform_get_irq_byname(pdev, "dma");
> > > > > +}
> > > > > +
> > > > > +static struct dw_edma_core_ops dma_ops = {
> > > > > + .irq_vector = imx_dma_irq_vector,
> > > > > +};
> > > > > +
> > > > > +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie)
> > > > > +{
> > > > > + unsigned int pcie_dma_offset;
> > > > > + struct dw_pcie *pci = imx6_pcie->pci;
> > > > > + struct device *dev = pci->dev;
> > > > > + struct dw_edma_chip *dma = &imx6_pcie->dma_chip;
> > > > > + int i = 0;
> > > >
> > > > Unused?
> > > >
> > > > > + int sz = PAGE_SIZE;
> > > > > +
> > > > > + pcie_dma_offset = 0x970;
> > > >
> > > > Can you get this offset from the devicetree node of ep?
> > > >
> > > > > +
> > > > > + dma->dev = dev;
> > > > > +
> > > > > + dma->reg_base = pci->dbi_base + pcie_dma_offset;
> > > > > +
> > > > > + dma->ops = &dma_ops;
> > > > > + dma->nr_irqs = 1;
> > > > > +
> > > > > + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP;
> > > > > +
> > > > > + dma->ll_wr_cnt = dma->ll_rd_cnt=1;
> > > >
> > > > Is this a hard limitation of the eDMA implementation or because of difficulties
> > > > in requesting the correct channel from client driver?
> > > >
> > > > If it's the latter, you could use my patch:
> > >
> > > It is because our hardware only has 1 channel.
> >
> > Ah okay. It is fine if that's the case.
> >
> > >
> > > >
> > > > https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=tracking-qcomlt-sdx55-drivers&id=c77ad9d929372b1ff495709714b24486d266a810
> > >
> > > My problem is
> > > in dw_edma_channel_setup()
> > > dma->directions = BIT(write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV);
> > >
> > > Already set direction. why need overwrite default device_caps?
> > >
> >
> > The above sets both direction in the caps.
>
> Why set both direction?
> write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV
> Only one bit set
>
> > That's fine if you want to verify
> > whether the channel is valid or not. But that won't help you want to choose the
> > correct channel.
> >
> > In your case it will work because, read and write channel count is 1. But what
> > if the channel count is 8?
>
> I know my case is special one. I just feel strange.
>
> dma_async_device_register(dma); register two devices, one read, one write.
>
> The default int dma_get_slave_caps(struct dma_chan *chan, struct
> dma_slave_caps *caps)
> {
> device = chan->device;
> caps->directions = device->directions;
> }
>
> It should return read/write devices's directions.
>
> >
> > write channel - 0 to 7
> > read channel - 8 to 15
> >
> > Now without identifying the channel direction, the client would be getting the
> > wrong one. For instance, if client requests read channel after write, the
> > dmaengine would pass 1 because the requested direction matches the caps and
> > that's wrong.
>
> Do you mean if I request a read after write can reproduce the problem?
After I force channel number to 8,
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c
b/drivers/pci/endpoint/functions/pci-epf-test.c
index f26afd02f3a86..04ec644ecde5a 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -180,7 +180,7 @@ static bool epf_dma_filter_fn(struct dma_chan
*chan, void *node)
memset(&caps, 0, sizeof(caps));
dma_get_slave_caps(chan, &caps);
-
+pr_err("dma cap: 0x%llx\n", caps.directions);
return chan->device->dev == filter->dev
&& (filter->dma_mask & caps.directions);
}
[ 17.715167] dma cap: 0x6 ( other DMA engine)
[ 17.717707] dma cap: 0x6 ( other DMA engine)
[ 17.720299] dma cap: 0x6 ( other DMA engine)
[ 17.722893] dma cap: 0x6 ( other DMA engine)
[ 17.725437] dma cap: 0x6 ( other DMA engine)
[ 17.728040] dma cap: 0x6 ( other DMA engine)
[ 17.730652] dma cap: 0x6 ( other DMA engine)
[ 17.733197] dma cap: 0x4 (EDMA W)
[ 17.735762] dma cap: 0x4 (EDMA W)
[ 17.738390] dma cap: 0x4 .....
[ 17.740937] dma cap: 0x4
[ 17.743518] dma cap: 0x4
[ 17.746108] dma cap: 0x4
[ 17.748651] dma cap: 0x4
[ 17.751219] dma cap: 0x2 (EDMA WR)
I get the correct DMA channel without your patch.
> > Mani
> >
> > > >
> > > > > + dma->ll_region_wr[0].sz = sz;
> > > > > + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz,
> > > > > + &dma->ll_region_wr[i].paddr,
> > > > > + GFP_KERNEL);
> > > >
> > > > Allocation could fail. Please add error checking here and below.
> > > >
> > > > Thanks,
> > > > Mani
> > > >
> > > > > +
> > > > > + dma->ll_region_rd[0].sz = sz;
> > > > > + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz,
> > > > > + &dma->ll_region_rd[i].paddr,
> > > > > + GFP_KERNEL);
> > > > > +
> > > > > + return dw_edma_probe(dma);
> > > > > +}
> > > > > +
> > > > > static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie,
> > > > > struct platform_device *pdev)
> > > > > {
> > > > > @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > > > > goto err_ret;
> > > > > }
> > > > >
> > > > > + if (imx_add_pcie_dma(imx6_pcie))
> > > > > + dev_info(dev, "pci edma probe failure\n");
> > > > > +
> > > > > return 0;
> > > > >
> > > > > err_ret:
> > > > > --
> > > > > 2.24.0.rc1
> > > > >
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/5] PCI: imx6: add PCIe embedded DMA support
2022-03-09 19:14 ` Zhi Li
2022-03-09 20:48 ` Zhi Li
@ 2022-03-10 4:45 ` Manivannan Sadhasivam
1 sibling, 0 replies; 18+ messages in thread
From: Manivannan Sadhasivam @ 2022-03-10 4:45 UTC (permalink / raw)
To: Zhi Li
Cc: Frank Li, gustavo.pimentel, hongxing.zhu, Lucas Stach,
dl-linux-imx, linux-pci, vkoul, lorenzo.pieralisi, robh, kw,
Bjorn Helgaas, Shawn Guo
On Wed, Mar 09, 2022 at 01:14:33PM -0600, Zhi Li wrote:
> On Wed, Mar 9, 2022 at 12:42 PM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Wed, Mar 09, 2022 at 12:31:57PM -0600, Zhi Li wrote:
> > > On Wed, Mar 9, 2022 at 6:02 AM Manivannan Sadhasivam
> > > <manivannan.sadhasivam@linaro.org> wrote:
> > > >
> > > > On Thu, Mar 03, 2022 at 12:46:34PM -0600, Frank Li wrote:
> > > > > Add support for the DMA controller in the DesignWare PCIe core
> > > > >
> > > > > The DMA can transfer data to any remote address location
> > > > > regardless PCI address space size.
> > > > >
> > > > > Prepare struct dw_edma_chip() and call dw_edma_probe()
> > > > >
> > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > ---
> > > > >
> > > > > This patch depended on some ep enable patch for imx.
> > > > >
> > > > > Change from v1 to v2
> > > > > - rework commit message
> > > > > - align dw_edma_chip change
> > > > >
> > > > > drivers/pci/controller/dwc/pci-imx6.c | 51 +++++++++++++++++++++++++++
> > > > > 1 file changed, 51 insertions(+)
> > > > >
> > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > > > > index efa8b81711090..7dc55986c947d 100644
> > > > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > > > @@ -38,6 +38,7 @@
> > > > > #include "../../pci.h"
> > > > >
> > > > > #include "pcie-designware.h"
> > > > > +#include "linux/dma/edma.h"
> > > > >
> > > > > #define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET 0x7c
> > > > > #define IMX8MQ_PCIE_LINK_CAP_L1EL_64US GENMASK(18, 17)
> > > > > @@ -164,6 +165,8 @@ struct imx6_pcie {
> > > > > const struct imx6_pcie_drvdata *drvdata;
> > > > > struct regulator *epdev_on;
> > > > > struct phy *phy;
> > > > > +
> > > > > + struct dw_edma_chip dma_chip;
> > > > > };
> > > > >
> > > > > /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
> > > > > @@ -2031,6 +2034,51 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
> > > > > .get_features = imx_pcie_ep_get_features,
> > > > > };
> > > > >
> > > > > +static int imx_dma_irq_vector(struct device *dev, unsigned int nr)
> > > > > +{
> > > > > + struct platform_device *pdev = to_platform_device(dev);
> > > > > +
> > > > > + return platform_get_irq_byname(pdev, "dma");
> > > > > +}
> > > > > +
> > > > > +static struct dw_edma_core_ops dma_ops = {
> > > > > + .irq_vector = imx_dma_irq_vector,
> > > > > +};
> > > > > +
> > > > > +static int imx_add_pcie_dma(struct imx6_pcie *imx6_pcie)
> > > > > +{
> > > > > + unsigned int pcie_dma_offset;
> > > > > + struct dw_pcie *pci = imx6_pcie->pci;
> > > > > + struct device *dev = pci->dev;
> > > > > + struct dw_edma_chip *dma = &imx6_pcie->dma_chip;
> > > > > + int i = 0;
> > > >
> > > > Unused?
> > > >
> > > > > + int sz = PAGE_SIZE;
> > > > > +
> > > > > + pcie_dma_offset = 0x970;
> > > >
> > > > Can you get this offset from the devicetree node of ep?
> > > >
> > > > > +
> > > > > + dma->dev = dev;
> > > > > +
> > > > > + dma->reg_base = pci->dbi_base + pcie_dma_offset;
> > > > > +
> > > > > + dma->ops = &dma_ops;
> > > > > + dma->nr_irqs = 1;
> > > > > +
> > > > > + dma->flags = DW_EDMA_CHIP_NO_MSI | DW_EDMA_CHIP_REG32BIT | DW_EDMA_CHIP_LOCAL_EP;
> > > > > +
> > > > > + dma->ll_wr_cnt = dma->ll_rd_cnt=1;
> > > >
> > > > Is this a hard limitation of the eDMA implementation or because of difficulties
> > > > in requesting the correct channel from client driver?
> > > >
> > > > If it's the latter, you could use my patch:
> > >
> > > It is because our hardware only has 1 channel.
> >
> > Ah okay. It is fine if that's the case.
> >
> > >
> > > >
> > > > https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=tracking-qcomlt-sdx55-drivers&id=c77ad9d929372b1ff495709714b24486d266a810
> > >
> > > My problem is
> > > in dw_edma_channel_setup()
> > > dma->directions = BIT(write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV);
> > >
> > > Already set direction. why need overwrite default device_caps?
> > >
> >
> > The above sets both direction in the caps.
>
> Why set both direction?
> write ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV
> Only one bit set
>
> > That's fine if you want to verify
> > whether the channel is valid or not. But that won't help you want to choose the
> > correct channel.
> >
> > In your case it will work because, read and write channel count is 1. But what
> > if the channel count is 8?
>
> I know my case is special one. I just feel strange.
>
> dma_async_device_register(dma); register two devices, one read, one write.
>
> The default int dma_get_slave_caps(struct dma_chan *chan, struct
> dma_slave_caps *caps)
> {
> device = chan->device;
> caps->directions = device->directions;
> }
>
> It should return read/write devices's directions.
>
Urgh... In our kernel tree, my colleague merged two dma devices into one and I
failed to notice that :/
So yeah, it should work by default. Sorry for the noise.
Thanks,
Mani
> >
> > write channel - 0 to 7
> > read channel - 8 to 15
> >
> > Now without identifying the channel direction, the client would be getting the
> > wrong one. For instance, if client requests read channel after write, the
> > dmaengine would pass 1 because the requested direction matches the caps and
> > that's wrong.
>
> Do you mean if I request a read after write can reproduce the problem?
>
>
> >
> > Thanks,
> > Mani
> >
> > > >
> > > > > + dma->ll_region_wr[0].sz = sz;
> > > > > + dma->ll_region_wr[0].vaddr = dmam_alloc_coherent(dev, sz,
> > > > > + &dma->ll_region_wr[i].paddr,
> > > > > + GFP_KERNEL);
> > > >
> > > > Allocation could fail. Please add error checking here and below.
> > > >
> > > > Thanks,
> > > > Mani
> > > >
> > > > > +
> > > > > + dma->ll_region_rd[0].sz = sz;
> > > > > + dma->ll_region_rd[0].vaddr = dmam_alloc_coherent(dev, sz,
> > > > > + &dma->ll_region_rd[i].paddr,
> > > > > + GFP_KERNEL);
> > > > > +
> > > > > + return dw_edma_probe(dma);
> > > > > +}
> > > > > +
> > > > > static int imx_add_pcie_ep(struct imx6_pcie *imx6_pcie,
> > > > > struct platform_device *pdev)
> > > > > {
> > > > > @@ -2694,6 +2742,9 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > > > > goto err_ret;
> > > > > }
> > > > >
> > > > > + if (imx_add_pcie_dma(imx6_pcie))
> > > > > + dev_info(dev, "pci edma probe failure\n");
> > > > > +
> > > > > return 0;
> > > > >
> > > > > err_ret:
> > > > > --
> > > > > 2.24.0.rc1
> > > > >
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/5] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally
2022-03-03 18:46 [PATCH v2 1/5] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Frank Li
` (3 preceding siblings ...)
2022-03-03 18:46 ` [PATCH v2 5/5] PCI: endpoint: functions/pci-epf-test: Support PCI controller DMA Frank Li
@ 2022-03-10 22:07 ` Zhi Li
4 siblings, 0 replies; 18+ messages in thread
From: Zhi Li @ 2022-03-10 22:07 UTC (permalink / raw)
To: Frank Li
Cc: gustavo.pimentel, hongxing.zhu, Lucas Stach, dl-linux-imx,
linux-pci, vkoul, lorenzo.pieralisi, robh, kw, Bjorn Helgaas,
Shawn Guo, Manivannan Sadhasivam, ntb
On Thu, Mar 3, 2022 at 12:46 PM Frank Li <Frank.Li@nxp.com> wrote:
>
> "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>
Update ntb mail list.
> ---
> 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 *)®s->type.legacy.ch) {
> void __iomem *ptr = ®s->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 [flat|nested] 18+ messages in thread
end of thread, other threads:[~2022-03-10 22:08 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-03 18:46 [PATCH v2 1/5] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Frank Li
2022-03-03 18:46 ` [PATCH v2 2/5] dmaengine: dw-edma-pcie: don't touch internal struct dw_edma Frank Li
2022-03-03 18:46 ` [PATCH v2 3/5] dmaengine: dw-edma: add flags at struct dw_edma_chip Frank Li
2022-03-04 16:19 ` Manivannan Sadhasivam
2022-03-04 17:16 ` Zhi Li
2022-03-07 13:59 ` Manivannan Sadhasivam
2022-03-04 21:31 ` Bjorn Helgaas
2022-03-03 18:46 ` [PATCH v2 4/5] PCI: imx6: add PCIe embedded DMA support Frank Li
2022-03-04 21:33 ` Bjorn Helgaas
2022-03-05 1:31 ` Zhi Li
2022-03-09 12:01 ` Manivannan Sadhasivam
2022-03-09 18:31 ` Zhi Li
2022-03-09 18:42 ` Manivannan Sadhasivam
2022-03-09 19:14 ` Zhi Li
2022-03-09 20:48 ` Zhi Li
2022-03-10 4:45 ` Manivannan Sadhasivam
2022-03-03 18:46 ` [PATCH v2 5/5] PCI: endpoint: functions/pci-epf-test: Support PCI controller DMA Frank Li
2022-03-10 22:07 ` [PATCH v2 1/5] dmaengine: dw-edma: fix dw_edma_probe() can't be call globally Zhi Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).