LinuxLists.cc - [PATCH 0/3] dmaengine: dw-edma: Add support for native HDMA

2022-09-21 06:54:22

Subject: [PATCH 0/3] dmaengine: dw-edma: Add support for native HDMA

2022-09-21 06:54:46

Subject: [PATCH 1/3] dmaengine: dw-edma: Rename dw_edma_core_ops structure to dw_edma_plat_ops

From: caihuoqing <[email protected]>

Rename dw_edma_core_ops structure to dw_edma_plat_ops,
because the 'ops' is related to platform device

Signed-off-by: caihuoqing <[email protected]>
---
drivers/dma/dw-edma/dw-edma-pcie.c | 4 ++--
include/linux/dma/edma.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 3f9dadc73854..5604f3771866 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -109,7 +109,7 @@ static u64 dw_edma_pcie_address(struct device *dev, phys_addr_t cpu_addr)
return region.start;
}

-static const struct dw_edma_core_ops dw_edma_pcie_core_ops = {
+static const struct dw_edma_plat_ops dw_edma_pcie_plat_ops = {
.irq_vector = dw_edma_pcie_irq_vector,
.pci_address = dw_edma_pcie_address,
};
@@ -225,7 +225,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,

chip->mf = vsec_data.mf;
chip->nr_irqs = nr_irqs;
- chip->ops = &dw_edma_pcie_core_ops;
+ chip->ops = &dw_edma_pcie_plat_ops;

chip->ll_wr_cnt = vsec_data.wr_ch_cnt;
chip->ll_rd_cnt = vsec_data.rd_ch_cnt;
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index 9d44da4aa59d..642dd325259b 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -37,7 +37,7 @@ struct dw_edma_region {
* iATU windows. That will be done by the controller
* automatically.
*/
-struct dw_edma_core_ops {
+struct dw_edma_plat_ops {
int (*irq_vector)(struct device *dev, unsigned int nr);
u64 (*pci_address)(struct device *dev, phys_addr_t cpu_addr);
};
@@ -77,7 +77,7 @@ enum dw_edma_chip_flags {
struct dw_edma_chip {
struct device *dev;
int nr_irqs;
- const struct dw_edma_core_ops *ops;
+ const struct dw_edma_plat_ops *ops;
u32 flags;

void __iomem *reg_base;
--
2.25.1

2022-09-21 07:10:35

by Cai Huoqing

[permalink] [raw]

Subject: [PATCH 3/3] dmaengine: dw-edma: Add support for native HDMA

From: caihuoqing <[email protected]>

Add support for HDMA NATIVE, as long the IP design has set
the compatible register map parameter-HDMA_NATIVE,
which allows compatibility for native HDMA register configuration.

The HDMA Hyper-DMA IP is an enhancement of the eDMA embedded-DMA IP.
And the native HDMA registers are different from eDMA,
so this patch add support for HDMA NATIVE mode.

HDMA write and read channels operate independently to maximize
the performance of the HDMA read and write data transfer over
the link When you configure the HDMA with multiple read channels,
then it uses a round robin (RR) arbitration scheme to select
the next read channel to be serviced.
The same applies when you have multiple write channels.

The native HDMA driver also supports a maximum of 16 independent
channels (8 write + 8 read), which can run simultaneously.
Both SAR (Source Address Register) and DAR (Destination Address Register)
are alignmented to byte.

Signed-off-by: caihuoqing <[email protected]>
---
drivers/dma/dw-edma/Makefile | 6 +-
drivers/dma/dw-edma/dw-edma-core.c | 6 +-
drivers/dma/dw-edma/dw-hdma-v0-core.c | 308 +++++++++++++++++++++++
drivers/dma/dw-edma/dw-hdma-v0-core.h | 17 ++
drivers/dma/dw-edma/dw-hdma-v0-debugfs.c | 150 +++++++++++
drivers/dma/dw-edma/dw-hdma-v0-debugfs.h | 22 ++
drivers/dma/dw-edma/dw-hdma-v0-regs.h | 98 ++++++++
include/linux/dma/edma.h | 3 +-
8 files changed, 606 insertions(+), 4 deletions(-)
create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.c
create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.h
create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-regs.h

diff --git a/drivers/dma/dw-edma/Makefile b/drivers/dma/dw-edma/Makefile
index 8d45c0d5689d..958e7f202d11 100644
--- a/drivers/dma/dw-edma/Makefile
+++ b/drivers/dma/dw-edma/Makefile
@@ -1,7 +1,9 @@
# SPDX-License-Identifier: GPL-2.0

obj-$(CONFIG_DW_EDMA) += dw-edma.o
-dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o
+dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o \
+ dw-hdma-v0-debugfs.o
dw-edma-objs := dw-edma-core.o \
- dw-edma-v0-core.o $(dw-edma-y)
+ dw-edma-v0-core.o \
+ dw-hdma-v0-core.o $(dw-edma-y)
obj-$(CONFIG_DW_EDMA_PCIE) += dw-edma-pcie.o
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index a66e4216a5d3..59c71e5cc8f6 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -19,6 +19,7 @@

#include "dw-edma-core.h"
#include "dw-edma-v0-core.h"
+#include "dw-hdma-v0-core.h"
#include "../dmaengine.h"
#include "../virt-dma.h"

@@ -952,7 +953,10 @@ int dw_edma_probe(struct dw_edma_chip *chip)

dw->chip = chip;

- dw_edma_v0_core_register(dw);
+ if (dw->chip->mf == EDMA_MF_HDMA_NATIVE)
+ dw_hdma_v0_core_register(dw);
+ else
+ dw_edma_v0_core_register(dw);

raw_spin_lock_init(&dw->lock);

diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
new file mode 100644
index 000000000000..4ed6881bf3e9
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -0,0 +1,308 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 Cai Huoqing
+ * Synopsys DesignWare HDMA v0 core
+ */
+
+#include <linux/bitfield.h>
+
+#include "dw-edma-core.h"
+#include "dw-edma-v0-core.h"
+#include "dw-edma-v0-regs.h"
+#include "dw-hdma-v0-core.h"
+#include "dw-hdma-v0-regs.h"
+#include "dw-hdma-v0-debugfs.h"
+
+enum dw_hdma_control {
+ DW_HDMA_V0_CB = BIT(0),
+ DW_HDMA_V0_TCB = BIT(1),
+ DW_HDMA_V0_LLP = BIT(2),
+ DW_HDMA_V0_LIE = BIT(3),
+ DW_HDMA_V0_RIE = BIT(4),
+ DW_HDMA_V0_CCS = BIT(8),
+ DW_HDMA_V0_LLE = BIT(9),
+};
+static inline struct dw_hdma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
+{
+ return dw->chip->reg_base;
+}
+
+static inline struct dw_hdma_v0_ch_regs __iomem *
+__dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
+{
+
+ if (dir == EDMA_DIR_WRITE)
+ return &(__dw_regs(dw)->ch[ch].wr);
+ else
+ return &(__dw_regs(dw)->ch[ch].rd);
+}
+
+#define SET_CH_32(dw, dir, ch, name, value) \
+ writel(value, &(__dw_ch_regs(dw, dir, ch)->name))
+
+#define GET_CH_32(dw, dir, ch, name) \
+ readl(&(__dw_ch_regs(dw, dir, ch)->name))
+
+#define SET_BOTH_CH_32(dw, ch, name, value) \
+ do { \
+ writel(value, &(__dw_ch_regs(dw, EDMA_DIR_WRITE, ch)->name)); \
+ writel(value, &(__dw_ch_regs(dw, EDMA_DIR_READ, ch)->name)); \
+ } while (0)
+
+#define SET_LL_32(ll, value) \
+ writel(value, ll)
+
+/* HDMA management callbacks */
+static void dw_hdma_v0_core_off(struct dw_edma *dw)
+{
+ int id;
+
+ for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
+ SET_BOTH_CH_32(dw, id, int_setup,
+ HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
+ SET_BOTH_CH_32(dw, id, int_clear,
+ HDMA_V0_STOP_INT_MASK & HDMA_V0_ABORT_INT_MASK);
+ SET_BOTH_CH_32(dw, id, ch_en, 0);
+ }
+}
+
+static u16 dw_hdma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
+{
+ u32 num_ch = 0;
+ int id;
+
+ for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
+ if (GET_CH_32(dw, id, dir, ch_en) & BIT(0))
+ num_ch++;
+ }
+
+ if (num_ch > HDMA_V0_MAX_NR_CH)
+ num_ch = HDMA_V0_MAX_NR_CH;
+
+ return (u16)num_ch;
+}
+
+static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+ u32 tmp;
+
+ tmp = FIELD_GET(HDMA_V0_CH_STATUS_MASK,
+ GET_CH_32(dw, chan->id, chan->dir, ch_stat));
+
+ if (tmp == 1)
+ return DMA_IN_PROGRESS;
+ else if (tmp == 3)
+ return DMA_COMPLETE;
+ else
+ return DMA_ERROR;
+}
+
+static void dw_hdma_v0_core_clear_done_int(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+
+ SET_CH_32(dw, chan->dir, chan->id, int_clear, HDMA_V0_STOP_INT_MASK);
+}
+
+static void dw_hdma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+
+ SET_CH_32(dw, chan->dir, chan->id, int_clear, HDMA_V0_ABORT_INT_MASK);
+}
+
+static u32 dw_hdma_v0_core_status_int(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+
+ return GET_CH_32(dw, chan->dir, chan->id, int_stat);
+}
+
+static u32 dw_hdma_v0_core_check_done_int(u32 val)
+{
+ return (FIELD_GET(HDMA_V0_STOP_INT_MASK, val) == BIT(0));
+}
+
+static u32 dw_hdma_v0_core_check_abort_int(u32 val)
+{
+ return (FIELD_GET(HDMA_V0_ABORT_INT_MASK, val) == BIT(0));
+}
+
+static
+void dw_hdma_v0_core_done_handle(struct dw_edma_irq *dw_irq,
+ void (*done_cb)(struct dw_edma_chan *chan),
+ enum dw_edma_dir dir)
+{
+ struct dw_edma *dw = dw_irq->dw;
+ unsigned long total, pos, val;
+ unsigned long off, mask;
+
+ if (dir == EDMA_DIR_WRITE) {
+ total = dw->wr_ch_cnt;
+ off = 0;
+ mask = dw_irq->wr_mask;
+ } else {
+ total = dw->rd_ch_cnt;
+ off = dw->wr_ch_cnt;
+ mask = dw_irq->rd_mask;
+ }
+
+ for_each_set_bit(pos, &mask, total) {
+ struct dw_edma_chan *chan = &dw->chan[pos + off];
+
+ val = dw_hdma_v0_core_status_int(chan);
+ if (dw_hdma_v0_core_check_done_int(val))
+ done_cb(chan);
+ }
+}
+
+static
+void dw_hdma_v0_core_abort_handle(struct dw_edma_irq *dw_irq,
+ void (*abort_cb)(struct dw_edma_chan *chan),
+ enum dw_edma_dir dir)
+{
+ struct dw_edma *dw = dw_irq->dw;
+ unsigned long total, pos, val;
+ unsigned long off, mask;
+
+ if (dir == EDMA_DIR_WRITE) {
+ total = dw->wr_ch_cnt;
+ off = 0;
+ mask = dw_irq->wr_mask;
+ } else {
+ total = dw->rd_ch_cnt;
+ off = dw->wr_ch_cnt;
+ mask = dw_irq->rd_mask;
+ }
+
+ for_each_set_bit(pos, &mask, total) {
+ struct dw_edma_chan *chan = &dw->chan[pos + off];
+
+ val = dw_hdma_v0_core_status_int(chan);
+ if (dw_hdma_v0_core_check_abort_int(val))
+ abort_cb(chan);
+ }
+}
+
+static void dw_hdma_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;
+ int j;
+
+ lli = chunk->ll_region.vaddr;
+
+ if (chunk->cb)
+ control = DW_HDMA_V0_CB;
+
+ j = chunk->bursts_alloc;
+ list_for_each_entry(child, &chunk->burst->list, list) {
+ j--;
+ if (!j) {
+ control |= DW_HDMA_V0_LIE;
+ if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
+ control |= DW_HDMA_V0_RIE;
+ }
+ /* Channel control */
+ SET_LL_32(&lli[i].control, control);
+ /* Transfer size */
+ SET_LL_32(&lli[i].transfer_size, child->sz);
+ /* SAR */
+ SET_LL_32(&lli[i].sar.lsb, lower_32_bits(child->sar));
+ SET_LL_32(&lli[i].sar.msb, upper_32_bits(child->sar));
+ /* DAR */
+ SET_LL_32(&lli[i].dar.lsb, lower_32_bits(child->dar));
+ SET_LL_32(&lli[i].dar.msb, upper_32_bits(child->dar));
+ i++;
+ }
+
+ llp = (void __iomem *)&lli[i];
+ control = DW_HDMA_V0_LLP | DW_HDMA_V0_TCB;
+ if (!chunk->cb)
+ control |= DW_HDMA_V0_CB;
+
+ /* Channel control */
+ SET_LL_32(&llp->control, control);
+ /* Linked list */
+ SET_LL_32(&llp->llp.lsb, lower_32_bits(chunk->ll_region.paddr));
+ SET_LL_32(&llp->llp.msb, upper_32_bits(chunk->ll_region.paddr));
+}
+
+static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
+{
+ struct dw_edma_chan *chan = chunk->chan;
+ struct dw_edma *dw = chan->dw;
+ u32 tmp;
+
+ dw_hdma_v0_core_write_chunk(chunk);
+
+ if (first) {
+ /* Enable engine */
+ SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
+ /* Interrupt enable&unmask - done, abort */
+ tmp = GET_CH_32(dw, chan->dir, chan->id, int_setup);
+ tmp |= HDMA_V0_STOP_INT_MASK;
+ tmp |= HDMA_V0_ABORT_INT_MASK;
+ tmp |= HDMA_V0_LOCAL_ABORT_INT_EN;
+ tmp |= HDMA_V0_LOCAL_STOP_INT_EN;
+ SET_CH_32(dw, chan->dir, chan->id, int_setup, tmp);
+ /* Channel control */
+ SET_CH_32(dw, chan->dir, chan->id, control1, HDMA_V0_LINKLIST_EN);
+ /* Linked list */
+ /* llp is not aligned on 64bit -> keep 32bit accesses */
+ SET_CH_32(dw, chan->dir, chan->id, llp.lsb,
+ lower_32_bits(chunk->ll_region.paddr));
+ SET_CH_32(dw, chan->dir, chan->id, llp.msb,
+ upper_32_bits(chunk->ll_region.paddr));
+ }
+ /* set consumer cycle */
+ SET_CH_32(dw, chan->dir, chan->id, cycle_sync,
+ HDMA_V0_CONSUMER_CYCLE_STAT | HDMA_V0_CONSUMER_CYCLE_BIT);
+ /* Doorbell */
+ SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
+}
+
+static int dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+
+ /* MSI done addr - low, high */
+ SET_CH_32(dw, chan->dir, chan->id, msi_stop.lsb, chan->msi.address_lo);
+ SET_CH_32(dw, chan->dir, chan->id, msi_stop.msb, chan->msi.address_hi);
+ /* MSI abort addr - low, high */
+ SET_CH_32(dw, chan->dir, chan->id, msi_abort.lsb, chan->msi.address_lo);
+ SET_CH_32(dw, chan->dir, chan->id, msi_abort.msb, chan->msi.address_hi);
+ SET_CH_32(dw, chan->dir, chan->id, msi_abort.msb, chan->msi.address_hi);
+ /* config MSI data */
+ SET_CH_32(dw, chan->dir, chan->id, msi_msgdata, chan->msi.data);
+
+ return 0;
+}
+
+/* eDMA debugfs callbacks */
+static void dw_hdma_v0_core_debugfs_on(struct dw_edma *dw)
+{
+ dw_hdma_v0_debugfs_on(dw);
+}
+
+static const struct dw_edma_core_ops hdma_core = {
+ .dw_xdma_core_off = dw_hdma_v0_core_off,
+ .dw_xdma_core_ch_count = dw_hdma_v0_core_ch_count,
+ .dw_xdma_core_ch_status = dw_hdma_v0_core_ch_status,
+ .dw_xdma_core_clear_done_int = dw_hdma_v0_core_clear_done_int,
+ .dw_xdma_core_clear_abort_int = dw_hdma_v0_core_clear_abort_int,
+ .dw_xdma_core_done_handle = dw_hdma_v0_core_done_handle,
+ .dw_xdma_core_abort_handle = dw_hdma_v0_core_abort_handle,
+ .dw_xdma_core_start = dw_hdma_v0_core_start,
+ .dw_xdma_core_ch_config = dw_hdma_v0_core_ch_config,
+ .dw_xdma_core_debugfs_on = dw_hdma_v0_core_debugfs_on,
+};
+
+void dw_hdma_v0_core_register(struct dw_edma *dw)
+{
+ dw->core = &hdma_core;
+}
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.h b/drivers/dma/dw-edma/dw-hdma-v0-core.h
new file mode 100644
index 000000000000..dc9b93e469ae
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2022 Cai Huoqing
+ * Synopsys DesignWare HDMA v0 core
+ *
+ * Author: Cai Huoqing <[email protected]>
+ */
+
+#ifndef _DW_HDMA_V0_CORE_H
+#define _DW_HDMA_V0_CORE_H
+
+#include <linux/dma/edma.h>
+
+/* HDMA core operation register*/
+void dw_hdma_v0_core_register(struct dw_edma *dw);
+
+#endif /* _DW_HDMA_V0_CORE_H */
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
new file mode 100644
index 000000000000..1f973b21eef9
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
@@ -0,0 +1,150 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 Cai Huoqing
+ * Synopsys DesignWare HDMA v0 debugfs
+ *
+ * Author: Cai Huoqing <[email protected]>
+ */
+
+#include <linux/debugfs.h>
+#include <linux/bitfield.h>
+
+#include "dw-hdma-v0-debugfs.h"
+#include "dw-hdma-v0-regs.h"
+#include "dw-edma-core.h"
+
+#define REGS_ADDR(name) \
+ ((void __force *)&regs->name)
+
+#define REGISTER(name) \
+ { #name, REGS_ADDR(name) }
+
+#define WRITE_STR "write"
+#define READ_STR "read"
+#define CHANNEL_STR "channel"
+#define REGISTERS_STR "registers"
+
+static struct dw_edma *dw;
+static struct dw_hdma_v0_regs __iomem *regs;
+struct debugfs_entries {
+ const char *name;
+ dma_addr_t *reg;
+};
+
+static int dw_hdma_debugfs_u32_get(void *data, u64 *val)
+{
+ void __iomem *reg = (void __force __iomem *)data;
+ *val = readl(reg);
+
+ return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(fops_x32, dw_hdma_debugfs_u32_get, NULL, "0x%08llx\n");
+
+static void dw_hdma_debugfs_create_x32(const struct debugfs_entries entries[],
+ int nr_entries, struct dentry *dir)
+{
+ int i;
+
+ for (i = 0; i < nr_entries; i++) {
+ if (!debugfs_create_file_unsafe(entries[i].name, 0444, dir,
+ entries[i].reg, &fops_x32))
+ break;
+ }
+}
+
+static void dw_hdma_debugfs_regs_ch(struct dw_hdma_v0_ch_regs __iomem *regs,
+ struct dentry *dir)
+{
+ int nr_entries;
+ const struct debugfs_entries debugfs_regs[] = {
+ REGISTER(ch_en),
+ REGISTER(doorbell),
+ REGISTER(llp.lsb),
+ REGISTER(llp.msb),
+ REGISTER(cycle_sync),
+ REGISTER(transfer_size),
+ REGISTER(sar.lsb),
+ REGISTER(sar.msb),
+ REGISTER(dar.lsb),
+ REGISTER(dar.msb),
+ REGISTER(control1),
+ REGISTER(ch_stat),
+ REGISTER(int_stat),
+ REGISTER(int_setup),
+ REGISTER(int_clear),
+ REGISTER(msi_stop.lsb),
+ REGISTER(msi_stop.msb),
+ REGISTER(msi_abort.lsb),
+ REGISTER(msi_abort.msb),
+ REGISTER(msi_msgdata),
+ };
+
+ nr_entries = ARRAY_SIZE(debugfs_regs);
+ dw_hdma_debugfs_create_x32(debugfs_regs, nr_entries, dir);
+}
+
+static void dw_hdma_debugfs_regs_wr(struct dentry *dir)
+{
+ struct dentry *regs_dir, *ch_dir;
+ int i;
+ char name[16];
+
+ regs_dir = debugfs_create_dir(WRITE_STR, dir);
+ if (!regs_dir)
+ return;
+
+ for (i = 0; i < dw->wr_ch_cnt; i++) {
+ snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i);
+
+ ch_dir = debugfs_create_dir(name, regs_dir);
+ if (!ch_dir)
+ return;
+
+ dw_hdma_debugfs_regs_ch(&regs->ch[i].wr, ch_dir);
+ }
+}
+
+static void dw_hdma_debugfs_regs_rd(struct dentry *dir)
+{
+ struct dentry *regs_dir, *ch_dir;
+ int i;
+ char name[16];
+
+ regs_dir = debugfs_create_dir(READ_STR, dir);
+ if (!regs_dir)
+ return;
+
+ for (i = 0; i < dw->rd_ch_cnt; i++) {
+ snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i);
+
+ ch_dir = debugfs_create_dir(name, regs_dir);
+ if (!ch_dir)
+ return;
+
+ dw_hdma_debugfs_regs_ch(&regs->ch[i].rd, ch_dir);
+ }
+}
+
+static void dw_hdma_debugfs_regs(void)
+{
+ struct dentry *regs_dir;
+
+ regs_dir = debugfs_create_dir(REGISTERS_STR, dw->dma.dbg_dev_root);
+ if (!regs_dir)
+ return;
+
+ dw_hdma_debugfs_regs_wr(regs_dir);
+ dw_hdma_debugfs_regs_rd(regs_dir);
+}
+
+void dw_hdma_v0_debugfs_on(struct dw_edma *dw)
+{
+ if (!debugfs_initialized())
+ return;
+
+ debugfs_create_u32("mf", 0444, dw->dma.dbg_dev_root, &dw->chip->mf);
+ debugfs_create_u16("wr_ch_cnt", 0444, dw->dma.dbg_dev_root, &dw->wr_ch_cnt);
+ debugfs_create_u16("rd_ch_cnt", 0444, dw->dma.dbg_dev_root, &dw->rd_ch_cnt);
+
+ dw_hdma_debugfs_regs();
+}
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
new file mode 100644
index 000000000000..4b70911aedac
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2022 Cai Huoqing
+ * Synopsys DesignWare HDMA v0 debugfs
+ *
+ * Author: Cai Huoqing <[email protected]>
+ */
+
+#ifndef _DW_HDMA_V0_DEBUG_FS_H
+#define _DW_HDMA_V0_DEBUG_FS_H
+
+#include <linux/dma/edma.h>
+
+#ifdef CONFIG_DEBUG_FS
+void dw_hdma_v0_debugfs_on(struct dw_edma *dw);
+#else
+static inline void dw_hdma_v0_debugfs_on(struct dw_edma *dw)
+{
+}
+#endif /* CONFIG_DEBUG_FS */
+
+#endif /* _DW_HDMA_V0_DEBUG_FS_H */
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-regs.h b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
new file mode 100644
index 000000000000..677bf44400b3
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
@@ -0,0 +1,98 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2022 Cai Huoqing
+ * Synopsys DesignWare HDMA v0 reg
+ *
+ * Author: Cai Huoqing <[email protected]>
+ */
+
+#ifndef _DW_HDMA_V0_REGS_H
+#define _DW_HDMA_V0_REGS_H
+
+#include <linux/dmaengine.h>
+
+#define HDMA_V0_MAX_NR_CH 8
+#define HDMA_V0_LOCAL_ABORT_INT_EN BIT(6)
+#define HDMA_V0_REMOTE_ABORT_INT_EN BIT(5)
+#define HDMA_V0_LOCAL_STOP_INT_EN BIT(4)
+#define HDMA_V0_REMOTEL_STOP_INT_EN BIT(3)
+#define HDMA_V0_ABORT_INT_MASK BIT(2)
+#define HDMA_V0_STOP_INT_MASK BIT(0)
+#define HDMA_V0_LINKLIST_EN BIT(0)
+#define HDMA_V0_CONSUMER_CYCLE_STAT BIT(1)
+#define HDMA_V0_CONSUMER_CYCLE_BIT BIT(0)
+#define HDMA_V0_DOORBELL_START BIT(0)
+#define HDMA_V0_CH_STATUS_MASK GENMASK(1, 0)
+
+struct dw_hdma_v0_ch_regs {
+ u32 ch_en; /* 0x0000 */
+ u32 doorbell; /* 0x0004 */
+ u32 prefetch; /* 0x0008 */
+ u32 handshake; /* 0x000c */
+ union {
+ u64 reg; /* 0x0010..0x0014 */
+ struct {
+ u32 lsb; /* 0x0010 */
+ u32 msb; /* 0x0014 */
+ };
+ } llp;
+ u32 cycle_sync; /* 0x0018 */
+ u32 transfer_size; /* 0x001c */
+ union {
+ u64 reg; /* 0x0020..0x0024 */
+ struct {
+ u32 lsb; /* 0x0020 */
+ u32 msb; /* 0x0024 */
+ };
+ } sar;
+ union {
+ u64 reg; /* 0x0028..0x002c */
+ struct {
+ u32 lsb; /* 0x0028 */
+ u32 msb; /* 0x002c */
+ };
+ } dar;
+
+ u32 watermark_en; /* 0x0030 */
+ u32 control1; /* 0x0034 */
+ u32 func_num; /* 0x0038 */
+ u32 qos; /* 0x003c */
+ u32 reserved; /* 0x0040..0x007c */
+ u32 ch_stat; /* 0x0080 */
+ u32 int_stat; /* 0x0084 */
+ u32 int_setup; /* 0x0088 */
+ u32 int_clear; /* 0x008c */
+ union {
+ u64 reg; /* 0x0090..0x0094 */
+ struct {
+ u32 lsb; /* 0x0090 */
+ u32 msb; /* 0x0094 */
+ };
+ } msi_stop;
+ union {
+ u64 reg; /* 0x0098..0x009c */
+ struct {
+ u32 lsb; /* 0x0098 */
+ u32 msb; /* 0x009c */
+ };
+ } msi_watermark;
+ union {
+ u64 reg; /* 0x00a0..0x00a4 */
+ struct {
+ u32 lsb; /* 0x00a0 */
+ u32 msb; /* 0x00a4 */
+ };
+ } msi_abort;
+ u32 msi_msgdata; /* 0x00a8 */
+} __packed;
+
+struct dw_hdma_v0_ch {
+ struct dw_hdma_v0_ch_regs wr; /* 0x0000 */
+ struct dw_hdma_v0_ch_regs rd; /* 0x0100 */
+} __packed;
+
+struct dw_hdma_v0_regs {
+ struct dw_hdma_v0_ch ch[HDMA_V0_MAX_NR_CH]; /* 0x0000..0x0fa8 */
+} __packed;
+
+#endif /* _DW_HDMA_V0_REGS_H */
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index 642dd325259b..b879d107f08b 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -45,7 +45,8 @@ struct dw_edma_plat_ops {
enum dw_edma_map_format {
EDMA_MF_EDMA_LEGACY = 0x0,
EDMA_MF_EDMA_UNROLL = 0x1,
- EDMA_MF_HDMA_COMPAT = 0x5
+ EDMA_MF_HDMA_COMPAT = 0x5,
+ EDMA_MF_HDMA_NATIVE = 0x7
};

/**
--
2.25.1

2022-09-21 07:16:13

by Cai Huoqing

[permalink] [raw]

Subject: [PATCH 2/3] dmaengine: dw-edma: Create a new dw_edma_core_ops structure to abstract controller operation

From: caihuoqing <[email protected]>

The structure dw_edma_core_ops has a set of the pointers
abstracting out the DW eDMA vX and DW HDMA Native controllers.
And use dw_edma_v0_core_register to set up operation.

Signed-off-by: caihuoqing <[email protected]>
---
drivers/dma/dw-edma/dw-edma-core.c | 61 ++++++------------
drivers/dma/dw-edma/dw-edma-core.h | 19 ++++++
drivers/dma/dw-edma/dw-edma-v0-core.c | 90 ++++++++++++++++++++++++---
drivers/dma/dw-edma/dw-edma-v0-core.h | 14 +----
4 files changed, 120 insertions(+), 64 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 5736a537f4c8..a66e4216a5d3 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -187,6 +187,7 @@ static void dw_edma_start_transfer(struct dw_edma_chan *chan)
struct dw_edma_chunk *child;
struct dw_edma_desc *desc;
struct virt_dma_desc *vd;
+ struct dw_edma *dw = chan->dw;

vd = vchan_next_desc(&chan->vc);
if (!vd)
@@ -201,7 +202,7 @@ static void dw_edma_start_transfer(struct dw_edma_chan *chan)
if (!child)
return;

- dw_edma_v0_core_start(child, !desc->xfer_sz);
+ dw->core->dw_xdma_core_start(child, !desc->xfer_sz);
desc->xfer_sz += child->ll_region.sz;
dw_edma_free_burst(child);
list_del(&child->list);
@@ -277,6 +278,7 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
static int dw_edma_device_terminate_all(struct dma_chan *dchan)
{
struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+ struct dw_edma *dw = chan->dw;
int err = 0;

if (!chan->configured) {
@@ -286,7 +288,7 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
chan->configured = false;
} else if (chan->status == EDMA_ST_IDLE) {
chan->configured = false;
- } else if (dw_edma_v0_core_ch_status(chan) == DMA_COMPLETE) {
+ } else if (dw->core->dw_xdma_core_ch_status(chan) == DMA_COMPLETE) {
/*
* The channel is in a false BUSY state, probably didn't
* receive or lost an interrupt
@@ -593,9 +595,10 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
{
struct dw_edma_desc *desc;
struct virt_dma_desc *vd;
+ struct dw_edma *dw = chan->dw;
unsigned long flags;

- dw_edma_v0_core_clear_done_int(chan);
+ dw->core->dw_xdma_core_clear_done_int(chan);

spin_lock_irqsave(&chan->vc.lock, flags);
vd = vchan_next_desc(&chan->vc);
@@ -635,9 +638,10 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
{
struct virt_dma_desc *vd;
+ struct dw_edma *dw = chan->dw;
unsigned long flags;

- dw_edma_v0_core_clear_abort_int(chan);
+ dw->core->dw_xdma_core_clear_abort_int(chan);

spin_lock_irqsave(&chan->vc.lock, flags);
vd = vchan_next_desc(&chan->vc);
@@ -654,39 +658,10 @@ 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;
- unsigned long total, pos, val;
- unsigned long off;
- u32 mask;
-
- if (write) {
- total = dw->wr_ch_cnt;
- off = 0;
- mask = dw_irq->wr_mask;
- } else {
- total = dw->rd_ch_cnt;
- off = dw->wr_ch_cnt;
- mask = dw_irq->rd_mask;
- }
-
- val = dw_edma_v0_core_status_done_int(dw, write ?
- EDMA_DIR_WRITE :
- EDMA_DIR_READ);
- val &= mask;
- for_each_set_bit(pos, &val, total) {
- struct dw_edma_chan *chan = &dw->chan[pos + off];
-
- dw_edma_done_interrupt(chan);
- }
-
- val = dw_edma_v0_core_status_abort_int(dw, write ?
- EDMA_DIR_WRITE :
- EDMA_DIR_READ);
- val &= mask;
- for_each_set_bit(pos, &val, total) {
- struct dw_edma_chan *chan = &dw->chan[pos + off];
+ enum dw_edma_dir dir = write ? EDMA_DIR_WRITE : EDMA_DIR_READ;

- dw_edma_abort_interrupt(chan);
- }
+ dw->core->dw_xdma_core_done_handle(dw_irq, dw_edma_done_interrupt, dir);
+ dw->core->dw_xdma_core_abort_handle(dw_irq, dw_edma_abort_interrupt, dir);

return IRQ_HANDLED;
}
@@ -832,7 +807,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)

vchan_init(&chan->vc, dma);

- dw_edma_v0_core_device_config(chan);
+ dw->core->dw_xdma_core_ch_config(chan);
}

/* Set DMA channel capabilities */
@@ -977,14 +952,16 @@ int dw_edma_probe(struct dw_edma_chip *chip)

dw->chip = chip;

+ dw_edma_v0_core_register(dw);
+
raw_spin_lock_init(&dw->lock);

dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt,
- dw_edma_v0_core_ch_count(dw, EDMA_DIR_WRITE));
+ dw->core->dw_xdma_core_ch_count(dw, 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, chip->ll_rd_cnt,
- dw_edma_v0_core_ch_count(dw, EDMA_DIR_READ));
+ dw->core->dw_xdma_core_ch_count(dw, 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)
@@ -1003,7 +980,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
dev_name(chip->dev));

/* Disable eDMA, only to establish the ideal initial conditions */
- dw_edma_v0_core_off(dw);
+ dw->core->dw_xdma_core_off(dw);

/* Request IRQs */
err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
@@ -1019,7 +996,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
pm_runtime_enable(dev);

/* Turn debugfs on */
- dw_edma_v0_core_debugfs_on(dw);
+ dw->core->dw_xdma_core_debugfs_on(dw);

chip->dw = dw;

@@ -1045,7 +1022,7 @@ int dw_edma_remove(struct dw_edma_chip *chip)
return -ENODEV;

/* Disable eDMA */
- dw_edma_v0_core_off(dw);
+ dw->core->dw_xdma_core_off(dw);

/* Free irqs */
for (i = (dw->nr_irqs - 1); i >= 0; i--)
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 0ab2b6dba880..9b472d3c1596 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -95,6 +95,23 @@ struct dw_edma_irq {
struct dw_edma *dw;
};

+struct dw_edma_core_ops {
+ void (*dw_xdma_core_off)(struct dw_edma *dw);
+ u16 (*dw_xdma_core_ch_count)(struct dw_edma *dw, enum dw_edma_dir dir);
+ enum dma_status (*dw_xdma_core_ch_status)(struct dw_edma_chan *chan);
+ void (*dw_xdma_core_clear_done_int)(struct dw_edma_chan *chan);
+ void (*dw_xdma_core_clear_abort_int)(struct dw_edma_chan *chan);
+ void (*dw_xdma_core_done_handle)(struct dw_edma_irq *dw_irq,
+ void (*done_cb)(struct dw_edma_chan *chan),
+ enum dw_edma_dir dir);
+ void (*dw_xdma_core_abort_handle)(struct dw_edma_irq *dw_irq,
+ void (*abort_cb)(struct dw_edma_chan *chan),
+ enum dw_edma_dir dir);
+ void (*dw_xdma_core_start)(struct dw_edma_chunk *chunk, bool first);
+ int (*dw_xdma_core_ch_config)(struct dw_edma_chan *chan);
+ void (*dw_xdma_core_debugfs_on)(struct dw_edma *dw);
+};
+
struct dw_edma {
char name[32];

@@ -111,6 +128,8 @@ struct dw_edma {
raw_spinlock_t lock; /* Only for legacy */

struct dw_edma_chip *chip;
+
+ const struct dw_edma_core_ops *core;
};

struct dw_edma_sg {
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index 51a34b43434c..0c1a74245ecc 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -223,7 +223,7 @@ static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
writeq(value, ll)

/* eDMA management callbacks */
-void dw_edma_v0_core_off(struct dw_edma *dw)
+static void dw_edma_v0_core_off(struct dw_edma *dw)
{
SET_BOTH_32(dw, int_mask,
EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
@@ -232,7 +232,7 @@ void dw_edma_v0_core_off(struct dw_edma *dw)
SET_BOTH_32(dw, engine_en, 0);
}

-u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
+static u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
{
u32 num_ch;

@@ -249,7 +249,7 @@ u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
return (u16)num_ch;
}

-enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
+static enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
{
struct dw_edma *dw = chan->dw;
u32 tmp;
@@ -265,7 +265,7 @@ enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
return DMA_ERROR;
}

-void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
+static void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
{
struct dw_edma *dw = chan->dw;

@@ -273,7 +273,7 @@ void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id)));
}

-void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
+static void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
{
struct dw_edma *dw = chan->dw;

@@ -281,18 +281,70 @@ void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
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)
+static u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
{
return FIELD_GET(EDMA_V0_DONE_INT_MASK,
GET_RW_32(dw, dir, int_status));
}

-u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
+static u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
{
return FIELD_GET(EDMA_V0_ABORT_INT_MASK,
GET_RW_32(dw, dir, int_status));
}

+static
+void dw_edma_v0_core_done_handle(struct dw_edma_irq *dw_irq,
+ void (*done_cb)(struct dw_edma_chan *chan),
+ enum dw_edma_dir dir)
+{
+ struct dw_edma *dw = dw_irq->dw;
+ unsigned long total, pos, val;
+ unsigned long off;
+ u32 mask;
+
+ if (dir == EDMA_DIR_WRITE) {
+ total = dw->wr_ch_cnt;
+ off = 0;
+ mask = dw_irq->wr_mask;
+ } else {
+ total = dw->rd_ch_cnt;
+ off = dw->wr_ch_cnt;
+ mask = dw_irq->rd_mask;
+ }
+
+ val = dw_edma_v0_core_status_done_int(dw, dir);
+ val &= mask;
+ for_each_set_bit(pos, &val, total)
+ done_cb(&dw->chan[pos + off]);
+}
+
+static
+void dw_edma_v0_core_abort_handle(struct dw_edma_irq *dw_irq,
+ void (*abort_cb)(struct dw_edma_chan *chan),
+ enum dw_edma_dir dir)
+{
+ struct dw_edma *dw = dw_irq->dw;
+ unsigned long total, pos, val;
+ unsigned long off;
+ u32 mask;
+
+ if (dir == EDMA_DIR_WRITE) {
+ total = dw->wr_ch_cnt;
+ off = 0;
+ mask = dw_irq->wr_mask;
+ } else {
+ total = dw->rd_ch_cnt;
+ off = dw->wr_ch_cnt;
+ mask = dw_irq->rd_mask;
+ }
+
+ val = dw_edma_v0_core_status_abort_int(dw, dir);
+ val &= mask;
+ for_each_set_bit(pos, &val, total)
+ abort_cb(&dw->chan[pos + off]);
+}
+
static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
{
struct dw_edma_burst *child;
@@ -338,7 +390,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
SET_LL_64(&llp->llp.reg, chunk->ll_region.paddr);
}

-void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
+static 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->dw;
@@ -409,7 +461,7 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
FIELD_PREP(EDMA_V0_DOORBELL_CH_MASK, chan->id));
}

-int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
+static int dw_edma_v0_core_ch_config(struct dw_edma_chan *chan)
{
struct dw_edma *dw = chan->dw;
u32 tmp = 0;
@@ -481,7 +533,25 @@ int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
}

/* eDMA debugfs callbacks */
-void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
+static void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
{
dw_edma_v0_debugfs_on(dw);
}
+
+static const struct dw_edma_core_ops edma_core = {
+ .dw_xdma_core_off = dw_edma_v0_core_off,
+ .dw_xdma_core_ch_count = dw_edma_v0_core_ch_count,
+ .dw_xdma_core_ch_status = dw_edma_v0_core_ch_status,
+ .dw_xdma_core_clear_done_int = dw_edma_v0_core_clear_done_int,
+ .dw_xdma_core_clear_abort_int = dw_edma_v0_core_clear_abort_int,
+ .dw_xdma_core_done_handle = dw_edma_v0_core_done_handle,
+ .dw_xdma_core_abort_handle = dw_edma_v0_core_abort_handle,
+ .dw_xdma_core_start = dw_edma_v0_core_start,
+ .dw_xdma_core_ch_config = dw_edma_v0_core_ch_config,
+ .dw_xdma_core_debugfs_on = dw_edma_v0_core_debugfs_on,
+};
+
+void dw_edma_v0_core_register(struct dw_edma *dw)
+{
+ dw->core = &edma_core;
+}
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.h b/drivers/dma/dw-edma/dw-edma-v0-core.h
index ab96a1f48080..972994ecc96f 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.h
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.h
@@ -11,17 +11,7 @@

#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);
-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);
-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 */
-void dw_edma_v0_core_debugfs_on(struct dw_edma *dw);
+/* eDMA core operation register*/
+void dw_edma_v0_core_register(struct dw_edma *dw);

#endif /* _DW_EDMA_V0_CORE_H */
--
2.25.1

2022-09-25 17:42:43

by Serge Semin

[permalink] [raw]

Subject: Re: [PATCH 0/3] dmaengine: dw-edma: Add support for native HDMA

On Wed, Sep 21, 2022 at 02:48:49PM +0800, Cai Huoqing wrote:
> From: caihuoqing <[email protected]>
>
> Add support for HDMA NATIVE, as long the IP design has set
> the compatible register map parameter-HDMA_NATIVE,
> which allows compatibility for native HDMA register configuration.
>
> The HDMA Hyper-DMA IP is an enhancement of the eDMA embedded-DMA IP.
> And the native HDMA registers are different from eDMA,
> so this patch add support for HDMA NATIVE mode.
>
> HDMA write and read channels operate independently to maximize
> the performance of the HDMA read and write data transfer over
> the link When you configure the HDMA with multiple read channels,
> then it uses a round robin (RR) arbitration scheme to select
> the next read channel to be serviced.
> The same applies when you have multiple write channels.
>
> The native HDMA driver also supports a maximum of 16 independent
> channels (8 write + 8 read), which can run simultaneously.
> Both SAR (Source Address Register) and DAR (Destination Address Register)
> are alignmented to byte.dmaengine: dw-edma: Add support for native HDMA
>
> These series based on the series
> https://lore.kernel.org/dmaengine/[email protected]/

Great! Thanks for rebasing on top of my series. I'll have a look at
your patchset in several weeks (two or most likely three) since the
next merge window is upon us and neither mine nor your patchset will
get into the Bjorn/Vinod repos before that anyway.

-Sergey

>
> Cai Huoqing (3):
> dmaengine: dw-edma: Rename dw_edma_core_ops structure to
> dw_edma_plat_ops
> dmaengine: dw-edma: Create a new dw_edma_core_ops structure to
> abstract controller operation
> dmaengine: dw-edma: Add support for native HDMA
>
> drivers/dma/dw-edma/Makefile | 6 +-
> drivers/dma/dw-edma/dw-edma-core.c | 65 ++---
> drivers/dma/dw-edma/dw-edma-core.h | 19 ++
> drivers/dma/dw-edma/dw-edma-pcie.c | 4 +-
> drivers/dma/dw-edma/dw-edma-v0-core.c | 90 ++++++-
> drivers/dma/dw-edma/dw-edma-v0-core.h | 14 +-
> drivers/dma/dw-edma/dw-hdma-v0-core.c | 304 +++++++++++++++++++++++
> drivers/dma/dw-edma/dw-hdma-v0-core.h | 17 ++
> drivers/dma/dw-edma/dw-hdma-v0-debugfs.c | 150 +++++++++++
> drivers/dma/dw-edma/dw-hdma-v0-debugfs.h | 22 ++
> drivers/dma/dw-edma/dw-hdma-v0-regs.h | 98 ++++++++
> include/linux/dma/edma.h | 7 +-
> 12 files changed, 725 insertions(+), 71 deletions(-)
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.c
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.h
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-regs.h
>
> --
> 2.25.1
>

2022-11-04 13:55:44

by Vinod Koul

[permalink] [raw]

Subject: Re: [PATCH 2/3] dmaengine: dw-edma: Create a new dw_edma_core_ops structure to abstract controller operation

On 21-09-22, 14:48, Cai Huoqing wrote:
> From: caihuoqing <[email protected]>
>
> The structure dw_edma_core_ops has a set of the pointers
> abstracting out the DW eDMA vX and DW HDMA Native controllers.
> And use dw_edma_v0_core_register to set up operation.

Core can mean different things, why not call it
dw_edma_v0_{controller|cntrl}_regs

> Signed-off-by: caihuoqing <[email protected]>
> ---
> drivers/dma/dw-edma/dw-edma-core.c | 61 ++++++------------
> drivers/dma/dw-edma/dw-edma-core.h | 19 ++++++
> drivers/dma/dw-edma/dw-edma-v0-core.c | 90 ++++++++++++++++++++++++---
> drivers/dma/dw-edma/dw-edma-v0-core.h | 14 +----
> 4 files changed, 120 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 5736a537f4c8..a66e4216a5d3 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -187,6 +187,7 @@ static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> struct dw_edma_chunk *child;
> struct dw_edma_desc *desc;
> struct virt_dma_desc *vd;
> + struct dw_edma *dw = chan->dw;
>
> vd = vchan_next_desc(&chan->vc);
> if (!vd)
> @@ -201,7 +202,7 @@ static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> if (!child)
> return;
>
> - dw_edma_v0_core_start(child, !desc->xfer_sz);
> + dw->core->dw_xdma_core_start(child, !desc->xfer_sz);
> desc->xfer_sz += child->ll_region.sz;
> dw_edma_free_burst(child);
> list_del(&child->list);
> @@ -277,6 +278,7 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
> static int dw_edma_device_terminate_all(struct dma_chan *dchan)
> {
> struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> + struct dw_edma *dw = chan->dw;
> int err = 0;
>
> if (!chan->configured) {
> @@ -286,7 +288,7 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
> chan->configured = false;
> } else if (chan->status == EDMA_ST_IDLE) {
> chan->configured = false;
> - } else if (dw_edma_v0_core_ch_status(chan) == DMA_COMPLETE) {
> + } else if (dw->core->dw_xdma_core_ch_status(chan) == DMA_COMPLETE) {
> /*
> * The channel is in a false BUSY state, probably didn't
> * receive or lost an interrupt
> @@ -593,9 +595,10 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> {
> struct dw_edma_desc *desc;
> struct virt_dma_desc *vd;
> + struct dw_edma *dw = chan->dw;
> unsigned long flags;
>
> - dw_edma_v0_core_clear_done_int(chan);
> + dw->core->dw_xdma_core_clear_done_int(chan);
>
> spin_lock_irqsave(&chan->vc.lock, flags);
> vd = vchan_next_desc(&chan->vc);
> @@ -635,9 +638,10 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> {
> struct virt_dma_desc *vd;
> + struct dw_edma *dw = chan->dw;
> unsigned long flags;
>
> - dw_edma_v0_core_clear_abort_int(chan);
> + dw->core->dw_xdma_core_clear_abort_int(chan);
>
> spin_lock_irqsave(&chan->vc.lock, flags);
> vd = vchan_next_desc(&chan->vc);
> @@ -654,39 +658,10 @@ 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;
> - unsigned long total, pos, val;
> - unsigned long off;
> - u32 mask;
> -
> - if (write) {
> - total = dw->wr_ch_cnt;
> - off = 0;
> - mask = dw_irq->wr_mask;
> - } else {
> - total = dw->rd_ch_cnt;
> - off = dw->wr_ch_cnt;
> - mask = dw_irq->rd_mask;
> - }
> -
> - val = dw_edma_v0_core_status_done_int(dw, write ?
> - EDMA_DIR_WRITE :
> - EDMA_DIR_READ);
> - val &= mask;
> - for_each_set_bit(pos, &val, total) {
> - struct dw_edma_chan *chan = &dw->chan[pos + off];
> -
> - dw_edma_done_interrupt(chan);
> - }
> -
> - val = dw_edma_v0_core_status_abort_int(dw, write ?
> - EDMA_DIR_WRITE :
> - EDMA_DIR_READ);
> - val &= mask;
> - for_each_set_bit(pos, &val, total) {
> - struct dw_edma_chan *chan = &dw->chan[pos + off];
> + enum dw_edma_dir dir = write ? EDMA_DIR_WRITE : EDMA_DIR_READ;
>
> - dw_edma_abort_interrupt(chan);
> - }
> + dw->core->dw_xdma_core_done_handle(dw_irq, dw_edma_done_interrupt, dir);
> + dw->core->dw_xdma_core_abort_handle(dw_irq, dw_edma_abort_interrupt, dir);
>
> return IRQ_HANDLED;
> }
> @@ -832,7 +807,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
>
> vchan_init(&chan->vc, dma);
>
> - dw_edma_v0_core_device_config(chan);
> + dw->core->dw_xdma_core_ch_config(chan);
> }
>
> /* Set DMA channel capabilities */
> @@ -977,14 +952,16 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>
> dw->chip = chip;
>
> + dw_edma_v0_core_register(dw);
> +
> raw_spin_lock_init(&dw->lock);
>
> dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt,
> - dw_edma_v0_core_ch_count(dw, EDMA_DIR_WRITE));
> + dw->core->dw_xdma_core_ch_count(dw, 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, chip->ll_rd_cnt,
> - dw_edma_v0_core_ch_count(dw, EDMA_DIR_READ));
> + dw->core->dw_xdma_core_ch_count(dw, 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)
> @@ -1003,7 +980,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> dev_name(chip->dev));
>
> /* Disable eDMA, only to establish the ideal initial conditions */
> - dw_edma_v0_core_off(dw);
> + dw->core->dw_xdma_core_off(dw);
>
> /* Request IRQs */
> err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
> @@ -1019,7 +996,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> pm_runtime_enable(dev);
>
> /* Turn debugfs on */
> - dw_edma_v0_core_debugfs_on(dw);
> + dw->core->dw_xdma_core_debugfs_on(dw);
>
> chip->dw = dw;
>
> @@ -1045,7 +1022,7 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> return -ENODEV;
>
> /* Disable eDMA */
> - dw_edma_v0_core_off(dw);
> + dw->core->dw_xdma_core_off(dw);
>
> /* Free irqs */
> for (i = (dw->nr_irqs - 1); i >= 0; i--)
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index 0ab2b6dba880..9b472d3c1596 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -95,6 +95,23 @@ struct dw_edma_irq {
> struct dw_edma *dw;
> };
>
> +struct dw_edma_core_ops {
> + void (*dw_xdma_core_off)(struct dw_edma *dw);
> + u16 (*dw_xdma_core_ch_count)(struct dw_edma *dw, enum dw_edma_dir dir);
> + enum dma_status (*dw_xdma_core_ch_status)(struct dw_edma_chan *chan);
> + void (*dw_xdma_core_clear_done_int)(struct dw_edma_chan *chan);
> + void (*dw_xdma_core_clear_abort_int)(struct dw_edma_chan *chan);
> + void (*dw_xdma_core_done_handle)(struct dw_edma_irq *dw_irq,
> + void (*done_cb)(struct dw_edma_chan *chan),
> + enum dw_edma_dir dir);
> + void (*dw_xdma_core_abort_handle)(struct dw_edma_irq *dw_irq,
> + void (*abort_cb)(struct dw_edma_chan *chan),
> + enum dw_edma_dir dir);
> + void (*dw_xdma_core_start)(struct dw_edma_chunk *chunk, bool first);
> + int (*dw_xdma_core_ch_config)(struct dw_edma_chan *chan);
> + void (*dw_xdma_core_debugfs_on)(struct dw_edma *dw);
> +};
> +
> struct dw_edma {
> char name[32];
>
> @@ -111,6 +128,8 @@ struct dw_edma {
> raw_spinlock_t lock; /* Only for legacy */
>
> struct dw_edma_chip *chip;
> +
> + const struct dw_edma_core_ops *core;
> };
>
> struct dw_edma_sg {
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index 51a34b43434c..0c1a74245ecc 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -223,7 +223,7 @@ static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> writeq(value, ll)
>
> /* eDMA management callbacks */
> -void dw_edma_v0_core_off(struct dw_edma *dw)
> +static void dw_edma_v0_core_off(struct dw_edma *dw)
> {
> SET_BOTH_32(dw, int_mask,
> EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
> @@ -232,7 +232,7 @@ void dw_edma_v0_core_off(struct dw_edma *dw)
> SET_BOTH_32(dw, engine_en, 0);
> }
>
> -u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> +static u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> {
> u32 num_ch;
>
> @@ -249,7 +249,7 @@ u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> return (u16)num_ch;
> }
>
> -enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> +static enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> {
> struct dw_edma *dw = chan->dw;
> u32 tmp;
> @@ -265,7 +265,7 @@ enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> return DMA_ERROR;
> }
>
> -void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> +static void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> {
> struct dw_edma *dw = chan->dw;
>
> @@ -273,7 +273,7 @@ void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id)));
> }
>
> -void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> +static void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> {
> struct dw_edma *dw = chan->dw;
>
> @@ -281,18 +281,70 @@ void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> 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)
> +static u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
> {
> return FIELD_GET(EDMA_V0_DONE_INT_MASK,
> GET_RW_32(dw, dir, int_status));
> }
>
> -u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> +static u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> {
> return FIELD_GET(EDMA_V0_ABORT_INT_MASK,
> GET_RW_32(dw, dir, int_status));
> }
>
> +static
> +void dw_edma_v0_core_done_handle(struct dw_edma_irq *dw_irq,
> + void (*done_cb)(struct dw_edma_chan *chan),
> + enum dw_edma_dir dir)
> +{
> + struct dw_edma *dw = dw_irq->dw;
> + unsigned long total, pos, val;
> + unsigned long off;
> + u32 mask;
> +
> + if (dir == EDMA_DIR_WRITE) {
> + total = dw->wr_ch_cnt;
> + off = 0;
> + mask = dw_irq->wr_mask;
> + } else {
> + total = dw->rd_ch_cnt;
> + off = dw->wr_ch_cnt;
> + mask = dw_irq->rd_mask;
> + }
> +
> + val = dw_edma_v0_core_status_done_int(dw, dir);
> + val &= mask;
> + for_each_set_bit(pos, &val, total)
> + done_cb(&dw->chan[pos + off]);
> +}
> +
> +static
> +void dw_edma_v0_core_abort_handle(struct dw_edma_irq *dw_irq,
> + void (*abort_cb)(struct dw_edma_chan *chan),
> + enum dw_edma_dir dir)
> +{
> + struct dw_edma *dw = dw_irq->dw;
> + unsigned long total, pos, val;
> + unsigned long off;
> + u32 mask;
> +
> + if (dir == EDMA_DIR_WRITE) {
> + total = dw->wr_ch_cnt;
> + off = 0;
> + mask = dw_irq->wr_mask;
> + } else {
> + total = dw->rd_ch_cnt;
> + off = dw->wr_ch_cnt;
> + mask = dw_irq->rd_mask;
> + }
> +
> + val = dw_edma_v0_core_status_abort_int(dw, dir);
> + val &= mask;
> + for_each_set_bit(pos, &val, total)
> + abort_cb(&dw->chan[pos + off]);
> +}
> +
> static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> {
> struct dw_edma_burst *child;
> @@ -338,7 +390,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> SET_LL_64(&llp->llp.reg, chunk->ll_region.paddr);
> }
>
> -void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> +static 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->dw;
> @@ -409,7 +461,7 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> FIELD_PREP(EDMA_V0_DOORBELL_CH_MASK, chan->id));
> }
>
> -int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
> +static int dw_edma_v0_core_ch_config(struct dw_edma_chan *chan)
> {
> struct dw_edma *dw = chan->dw;
> u32 tmp = 0;
> @@ -481,7 +533,25 @@ int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
> }
>
> /* eDMA debugfs callbacks */
> -void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> +static void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> {
> dw_edma_v0_debugfs_on(dw);
> }
> +
> +static const struct dw_edma_core_ops edma_core = {
> + .dw_xdma_core_off = dw_edma_v0_core_off,
> + .dw_xdma_core_ch_count = dw_edma_v0_core_ch_count,
> + .dw_xdma_core_ch_status = dw_edma_v0_core_ch_status,
> + .dw_xdma_core_clear_done_int = dw_edma_v0_core_clear_done_int,
> + .dw_xdma_core_clear_abort_int = dw_edma_v0_core_clear_abort_int,
> + .dw_xdma_core_done_handle = dw_edma_v0_core_done_handle,
> + .dw_xdma_core_abort_handle = dw_edma_v0_core_abort_handle,
> + .dw_xdma_core_start = dw_edma_v0_core_start,
> + .dw_xdma_core_ch_config = dw_edma_v0_core_ch_config,
> + .dw_xdma_core_debugfs_on = dw_edma_v0_core_debugfs_on,

dw_xdma_ctrl_xxx sounds more right for these

> +};
> +
> +void dw_edma_v0_core_register(struct dw_edma *dw)
> +{
> + dw->core = &edma_core;
> +}
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.h b/drivers/dma/dw-edma/dw-edma-v0-core.h
> index ab96a1f48080..972994ecc96f 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.h
> @@ -11,17 +11,7 @@
>
> #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);
> -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);
> -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 */
> -void dw_edma_v0_core_debugfs_on(struct dw_edma *dw);
> +/* eDMA core operation register*/
> +void dw_edma_v0_core_register(struct dw_edma *dw);
>
> #endif /* _DW_EDMA_V0_CORE_H */
> --
> 2.25.1

--
~Vinod

2022-11-04 13:56:17

by Vinod Koul

[permalink] [raw]

Subject: Re: [PATCH 1/3] dmaengine: dw-edma: Rename dw_edma_core_ops structure to dw_edma_plat_ops

On 21-09-22, 14:48, Cai Huoqing wrote:
> From: caihuoqing <[email protected]>
>
> Rename dw_edma_core_ops structure to dw_edma_plat_ops,
> because the 'ops' is related to platform device

Is that really a case, the device can be non platform too right?

--
~Vinod

2022-11-04 14:22:02

by Vinod Koul

[permalink] [raw]

Subject: Re: [PATCH 3/3] dmaengine: dw-edma: Add support for native HDMA

On 21-09-22, 14:48, Cai Huoqing wrote:
> From: caihuoqing <[email protected]>
>
> Add support for HDMA NATIVE, as long the IP design has set
> the compatible register map parameter-HDMA_NATIVE,
> which allows compatibility for native HDMA register configuration.
>
> The HDMA Hyper-DMA IP is an enhancement of the eDMA embedded-DMA IP.
> And the native HDMA registers are different from eDMA,
> so this patch add support for HDMA NATIVE mode.
>
> HDMA write and read channels operate independently to maximize
> the performance of the HDMA read and write data transfer over
> the link When you configure the HDMA with multiple read channels,
> then it uses a round robin (RR) arbitration scheme to select
> the next read channel to be serviced.
> The same applies when you have multiple write channels.
>
> The native HDMA driver also supports a maximum of 16 independent
> channels (8 write + 8 read), which can run simultaneously.
> Both SAR (Source Address Register) and DAR (Destination Address Register)
> are alignmented to byte.
>
> Signed-off-by: caihuoqing <[email protected]>
> ---
> drivers/dma/dw-edma/Makefile | 6 +-
> drivers/dma/dw-edma/dw-edma-core.c | 6 +-
> drivers/dma/dw-edma/dw-hdma-v0-core.c | 308 +++++++++++++++++++++++
> drivers/dma/dw-edma/dw-hdma-v0-core.h | 17 ++
> drivers/dma/dw-edma/dw-hdma-v0-debugfs.c | 150 +++++++++++
> drivers/dma/dw-edma/dw-hdma-v0-debugfs.h | 22 ++
> drivers/dma/dw-edma/dw-hdma-v0-regs.h | 98 ++++++++
> include/linux/dma/edma.h | 3 +-
> 8 files changed, 606 insertions(+), 4 deletions(-)

Can you split up, debugfs parts can at least be different patch!

> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.c
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.h
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-regs.h
>
> diff --git a/drivers/dma/dw-edma/Makefile b/drivers/dma/dw-edma/Makefile
> index 8d45c0d5689d..958e7f202d11 100644
> --- a/drivers/dma/dw-edma/Makefile
> +++ b/drivers/dma/dw-edma/Makefile
> @@ -1,7 +1,9 @@
> # SPDX-License-Identifier: GPL-2.0
>
> obj-$(CONFIG_DW_EDMA) += dw-edma.o
> -dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o
> +dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o \
> + dw-hdma-v0-debugfs.o
> dw-edma-objs := dw-edma-core.o \
> - dw-edma-v0-core.o $(dw-edma-y)
> + dw-edma-v0-core.o \
> + dw-hdma-v0-core.o $(dw-edma-y)
> obj-$(CONFIG_DW_EDMA_PCIE) += dw-edma-pcie.o
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index a66e4216a5d3..59c71e5cc8f6 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -19,6 +19,7 @@
>
> #include "dw-edma-core.h"
> #include "dw-edma-v0-core.h"
> +#include "dw-hdma-v0-core.h"
> #include "../dmaengine.h"
> #include "../virt-dma.h"
>
> @@ -952,7 +953,10 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>
> dw->chip = chip;
>
> - dw_edma_v0_core_register(dw);
> + if (dw->chip->mf == EDMA_MF_HDMA_NATIVE)
> + dw_hdma_v0_core_register(dw);
> + else
> + dw_edma_v0_core_register(dw);
>
> raw_spin_lock_init(&dw->lock);
>
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> new file mode 100644
> index 000000000000..4ed6881bf3e9
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -0,0 +1,308 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 core
> + */
> +
> +#include <linux/bitfield.h>
> +
> +#include "dw-edma-core.h"
> +#include "dw-edma-v0-core.h"
> +#include "dw-edma-v0-regs.h"
> +#include "dw-hdma-v0-core.h"
> +#include "dw-hdma-v0-regs.h"
> +#include "dw-hdma-v0-debugfs.h"
> +
> +enum dw_hdma_control {
> + DW_HDMA_V0_CB = BIT(0),
> + DW_HDMA_V0_TCB = BIT(1),
> + DW_HDMA_V0_LLP = BIT(2),
> + DW_HDMA_V0_LIE = BIT(3),
> + DW_HDMA_V0_RIE = BIT(4),
> + DW_HDMA_V0_CCS = BIT(8),
> + DW_HDMA_V0_LLE = BIT(9),
> +};
> +static inline struct dw_hdma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)

why __ in the name here?

> +{
> + return dw->chip->reg_base;
> +}
> +
> +static inline struct dw_hdma_v0_ch_regs __iomem *
> +__dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)

Here as well

> +{
> +
> + if (dir == EDMA_DIR_WRITE)
> + return &(__dw_regs(dw)->ch[ch].wr);
> + else
> + return &(__dw_regs(dw)->ch[ch].rd);
> +}
> +
> +#define SET_CH_32(dw, dir, ch, name, value) \
> + writel(value, &(__dw_ch_regs(dw, dir, ch)->name))
> +
> +#define GET_CH_32(dw, dir, ch, name) \
> + readl(&(__dw_ch_regs(dw, dir, ch)->name))
> +
> +#define SET_BOTH_CH_32(dw, ch, name, value) \
> + do { \
> + writel(value, &(__dw_ch_regs(dw, EDMA_DIR_WRITE, ch)->name)); \
> + writel(value, &(__dw_ch_regs(dw, EDMA_DIR_READ, ch)->name)); \
> + } while (0)
> +
> +#define SET_LL_32(ll, value) \
> + writel(value, ll)
> +
> +/* HDMA management callbacks */
> +static void dw_hdma_v0_core_off(struct dw_edma *dw)
> +{
> + int id;
> +
> + for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
> + SET_BOTH_CH_32(dw, id, int_setup,
> + HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
> + SET_BOTH_CH_32(dw, id, int_clear,
> + HDMA_V0_STOP_INT_MASK & HDMA_V0_ABORT_INT_MASK);
> + SET_BOTH_CH_32(dw, id, ch_en, 0);
> + }
> +}
> +
> +static u16 dw_hdma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> +{
> + u32 num_ch = 0;
> + int id;
> +
> + for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
> + if (GET_CH_32(dw, id, dir, ch_en) & BIT(0))
> + num_ch++;
> + }
> +
> + if (num_ch > HDMA_V0_MAX_NR_CH)
> + num_ch = HDMA_V0_MAX_NR_CH;
> +
> + return (u16)num_ch;
> +}
> +
> +static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> + u32 tmp;
> +
> + tmp = FIELD_GET(HDMA_V0_CH_STATUS_MASK,
> + GET_CH_32(dw, chan->id, chan->dir, ch_stat));
> +
> + if (tmp == 1)
> + return DMA_IN_PROGRESS;
> + else if (tmp == 3)
> + return DMA_COMPLETE;
> + else
> + return DMA_ERROR;
> +}
> +
> +static void dw_hdma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + SET_CH_32(dw, chan->dir, chan->id, int_clear, HDMA_V0_STOP_INT_MASK);
> +}
> +
> +static void dw_hdma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + SET_CH_32(dw, chan->dir, chan->id, int_clear, HDMA_V0_ABORT_INT_MASK);
> +}
> +
> +static u32 dw_hdma_v0_core_status_int(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + return GET_CH_32(dw, chan->dir, chan->id, int_stat);
> +}
> +
> +static u32 dw_hdma_v0_core_check_done_int(u32 val)
> +{
> + return (FIELD_GET(HDMA_V0_STOP_INT_MASK, val) == BIT(0));
> +}
> +
> +static u32 dw_hdma_v0_core_check_abort_int(u32 val)
> +{
> + return (FIELD_GET(HDMA_V0_ABORT_INT_MASK, val) == BIT(0));
> +}
> +
> +static
> +void dw_hdma_v0_core_done_handle(struct dw_edma_irq *dw_irq,
> + void (*done_cb)(struct dw_edma_chan *chan),
> + enum dw_edma_dir dir)

Looks horrible to read, something better like:

static void
dw_hdma_v0_core_done_handle(struct dw_edma_irq *dw_irq,
void (*done_cb)(struct dw_edma_chan *chan),
enum dw_edma_dir dir)

Looks much better to read, use checkpatch --strict to help with these
indents

> +{
> + struct dw_edma *dw = dw_irq->dw;
> + unsigned long total, pos, val;
> + unsigned long off, mask;
> +
> + if (dir == EDMA_DIR_WRITE) {
> + total = dw->wr_ch_cnt;
> + off = 0;
> + mask = dw_irq->wr_mask;
> + } else {
> + total = dw->rd_ch_cnt;
> + off = dw->wr_ch_cnt;
> + mask = dw_irq->rd_mask;
> + }
> +
> + for_each_set_bit(pos, &mask, total) {
> + struct dw_edma_chan *chan = &dw->chan[pos + off];
> +
> + val = dw_hdma_v0_core_status_int(chan);
> + if (dw_hdma_v0_core_check_done_int(val))
> + done_cb(chan);
> + }
> +}
> +
> +static
> +void dw_hdma_v0_core_abort_handle(struct dw_edma_irq *dw_irq,
> + void (*abort_cb)(struct dw_edma_chan *chan),
> + enum dw_edma_dir dir)
> +{
> + struct dw_edma *dw = dw_irq->dw;
> + unsigned long total, pos, val;
> + unsigned long off, mask;
> +
> + if (dir == EDMA_DIR_WRITE) {
> + total = dw->wr_ch_cnt;
> + off = 0;
> + mask = dw_irq->wr_mask;
> + } else {
> + total = dw->rd_ch_cnt;
> + off = dw->wr_ch_cnt;
> + mask = dw_irq->rd_mask;
> + }
> +
> + for_each_set_bit(pos, &mask, total) {
> + struct dw_edma_chan *chan = &dw->chan[pos + off];
> +
> + val = dw_hdma_v0_core_status_int(chan);
> + if (dw_hdma_v0_core_check_abort_int(val))
> + abort_cb(chan);
> + }
> +}
> +
> +static void dw_hdma_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;
> + int j;
> +
> + lli = chunk->ll_region.vaddr;
> +
> + if (chunk->cb)
> + control = DW_HDMA_V0_CB;
> +
> + j = chunk->bursts_alloc;
> + list_for_each_entry(child, &chunk->burst->list, list) {
> + j--;
> + if (!j) {
> + control |= DW_HDMA_V0_LIE;
> + if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
> + control |= DW_HDMA_V0_RIE;
> + }
> + /* Channel control */
> + SET_LL_32(&lli[i].control, control);
> + /* Transfer size */
> + SET_LL_32(&lli[i].transfer_size, child->sz);
> + /* SAR */
> + SET_LL_32(&lli[i].sar.lsb, lower_32_bits(child->sar));
> + SET_LL_32(&lli[i].sar.msb, upper_32_bits(child->sar));
> + /* DAR */
> + SET_LL_32(&lli[i].dar.lsb, lower_32_bits(child->dar));
> + SET_LL_32(&lli[i].dar.msb, upper_32_bits(child->dar));
> + i++;
> + }
> +
> + llp = (void __iomem *)&lli[i];
> + control = DW_HDMA_V0_LLP | DW_HDMA_V0_TCB;
> + if (!chunk->cb)
> + control |= DW_HDMA_V0_CB;
> +
> + /* Channel control */
> + SET_LL_32(&llp->control, control);
> + /* Linked list */
> + SET_LL_32(&llp->llp.lsb, lower_32_bits(chunk->ll_region.paddr));
> + SET_LL_32(&llp->llp.msb, upper_32_bits(chunk->ll_region.paddr));
> +}
> +
> +static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> +{
> + struct dw_edma_chan *chan = chunk->chan;
> + struct dw_edma *dw = chan->dw;
> + u32 tmp;
> +
> + dw_hdma_v0_core_write_chunk(chunk);
> +
> + if (first) {
> + /* Enable engine */
> + SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
> + /* Interrupt enable&unmask - done, abort */
> + tmp = GET_CH_32(dw, chan->dir, chan->id, int_setup);
> + tmp |= HDMA_V0_STOP_INT_MASK;
> + tmp |= HDMA_V0_ABORT_INT_MASK;
> + tmp |= HDMA_V0_LOCAL_ABORT_INT_EN;
> + tmp |= HDMA_V0_LOCAL_STOP_INT_EN;
> + SET_CH_32(dw, chan->dir, chan->id, int_setup, tmp);
> + /* Channel control */
> + SET_CH_32(dw, chan->dir, chan->id, control1, HDMA_V0_LINKLIST_EN);
> + /* Linked list */
> + /* llp is not aligned on 64bit -> keep 32bit accesses */
> + SET_CH_32(dw, chan->dir, chan->id, llp.lsb,
> + lower_32_bits(chunk->ll_region.paddr));
> + SET_CH_32(dw, chan->dir, chan->id, llp.msb,
> + upper_32_bits(chunk->ll_region.paddr));
> + }
> + /* set consumer cycle */
> + SET_CH_32(dw, chan->dir, chan->id, cycle_sync,
> + HDMA_V0_CONSUMER_CYCLE_STAT | HDMA_V0_CONSUMER_CYCLE_BIT);
> + /* Doorbell */
> + SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
> +}
> +
> +static int dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + /* MSI done addr - low, high */
> + SET_CH_32(dw, chan->dir, chan->id, msi_stop.lsb, chan->msi.address_lo);
> + SET_CH_32(dw, chan->dir, chan->id, msi_stop.msb, chan->msi.address_hi);
> + /* MSI abort addr - low, high */
> + SET_CH_32(dw, chan->dir, chan->id, msi_abort.lsb, chan->msi.address_lo);
> + SET_CH_32(dw, chan->dir, chan->id, msi_abort.msb, chan->msi.address_hi);
> + SET_CH_32(dw, chan->dir, chan->id, msi_abort.msb, chan->msi.address_hi);
> + /* config MSI data */
> + SET_CH_32(dw, chan->dir, chan->id, msi_msgdata, chan->msi.data);
> +
> + return 0;
> +}
> +
> +/* eDMA debugfs callbacks */
> +static void dw_hdma_v0_core_debugfs_on(struct dw_edma *dw)
> +{
> + dw_hdma_v0_debugfs_on(dw);
> +}
> +
> +static const struct dw_edma_core_ops hdma_core = {
> + .dw_xdma_core_off = dw_hdma_v0_core_off,
> + .dw_xdma_core_ch_count = dw_hdma_v0_core_ch_count,
> + .dw_xdma_core_ch_status = dw_hdma_v0_core_ch_status,
> + .dw_xdma_core_clear_done_int = dw_hdma_v0_core_clear_done_int,
> + .dw_xdma_core_clear_abort_int = dw_hdma_v0_core_clear_abort_int,
> + .dw_xdma_core_done_handle = dw_hdma_v0_core_done_handle,
> + .dw_xdma_core_abort_handle = dw_hdma_v0_core_abort_handle,
> + .dw_xdma_core_start = dw_hdma_v0_core_start,
> + .dw_xdma_core_ch_config = dw_hdma_v0_core_ch_config,
> + .dw_xdma_core_debugfs_on = dw_hdma_v0_core_debugfs_on,
> +};
> +
> +void dw_hdma_v0_core_register(struct dw_edma *dw)
> +{
> + dw->core = &hdma_core;
> +}
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.h b/drivers/dma/dw-edma/dw-hdma-v0-core.h
> new file mode 100644
> index 000000000000..dc9b93e469ae
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2022 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 core
> + *
> + * Author: Cai Huoqing <[email protected]>
> + */
> +
> +#ifndef _DW_HDMA_V0_CORE_H
> +#define _DW_HDMA_V0_CORE_H
> +
> +#include <linux/dma/edma.h>
> +
> +/* HDMA core operation register*/
> +void dw_hdma_v0_core_register(struct dw_edma *dw);
> +
> +#endif /* _DW_HDMA_V0_CORE_H */
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
> new file mode 100644
> index 000000000000..1f973b21eef9
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
> @@ -0,0 +1,150 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 debugfs
> + *
> + * Author: Cai Huoqing <[email protected]>
> + */
> +
> +#include <linux/debugfs.h>
> +#include <linux/bitfield.h>
> +
> +#include "dw-hdma-v0-debugfs.h"
> +#include "dw-hdma-v0-regs.h"
> +#include "dw-edma-core.h"
> +
> +#define REGS_ADDR(name) \
> + ((void __force *)&regs->name)
> +
> +#define REGISTER(name) \
> + { #name, REGS_ADDR(name) }
> +
> +#define WRITE_STR "write"
> +#define READ_STR "read"
> +#define CHANNEL_STR "channel"
> +#define REGISTERS_STR "registers"
> +
> +static struct dw_edma *dw;
> +static struct dw_hdma_v0_regs __iomem *regs;
> +struct debugfs_entries {
> + const char *name;
> + dma_addr_t *reg;
> +};
> +
> +static int dw_hdma_debugfs_u32_get(void *data, u64 *val)
> +{
> + void __iomem *reg = (void __force __iomem *)data;
> + *val = readl(reg);
> +
> + return 0;
> +}
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_x32, dw_hdma_debugfs_u32_get, NULL, "0x%08llx\n");
> +
> +static void dw_hdma_debugfs_create_x32(const struct debugfs_entries entries[],
> + int nr_entries, struct dentry *dir)
> +{
> + int i;
> +
> + for (i = 0; i < nr_entries; i++) {
> + if (!debugfs_create_file_unsafe(entries[i].name, 0444, dir,
> + entries[i].reg, &fops_x32))
> + break;
> + }
> +}
> +
> +static void dw_hdma_debugfs_regs_ch(struct dw_hdma_v0_ch_regs __iomem *regs,
> + struct dentry *dir)
> +{
> + int nr_entries;
> + const struct debugfs_entries debugfs_regs[] = {
> + REGISTER(ch_en),
> + REGISTER(doorbell),
> + REGISTER(llp.lsb),
> + REGISTER(llp.msb),
> + REGISTER(cycle_sync),
> + REGISTER(transfer_size),
> + REGISTER(sar.lsb),
> + REGISTER(sar.msb),
> + REGISTER(dar.lsb),
> + REGISTER(dar.msb),
> + REGISTER(control1),
> + REGISTER(ch_stat),
> + REGISTER(int_stat),
> + REGISTER(int_setup),
> + REGISTER(int_clear),
> + REGISTER(msi_stop.lsb),
> + REGISTER(msi_stop.msb),
> + REGISTER(msi_abort.lsb),
> + REGISTER(msi_abort.msb),
> + REGISTER(msi_msgdata),
> + };
> +
> + nr_entries = ARRAY_SIZE(debugfs_regs);
> + dw_hdma_debugfs_create_x32(debugfs_regs, nr_entries, dir);
> +}
> +
> +static void dw_hdma_debugfs_regs_wr(struct dentry *dir)
> +{
> + struct dentry *regs_dir, *ch_dir;
> + int i;
> + char name[16];
> +
> + regs_dir = debugfs_create_dir(WRITE_STR, dir);
> + if (!regs_dir)
> + return;
> +
> + for (i = 0; i < dw->wr_ch_cnt; i++) {
> + snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i);
> +
> + ch_dir = debugfs_create_dir(name, regs_dir);
> + if (!ch_dir)
> + return;
> +
> + dw_hdma_debugfs_regs_ch(&regs->ch[i].wr, ch_dir);
> + }
> +}
> +
> +static void dw_hdma_debugfs_regs_rd(struct dentry *dir)
> +{
> + struct dentry *regs_dir, *ch_dir;
> + int i;
> + char name[16];
> +
> + regs_dir = debugfs_create_dir(READ_STR, dir);
> + if (!regs_dir)
> + return;
> +
> + for (i = 0; i < dw->rd_ch_cnt; i++) {
> + snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i);
> +
> + ch_dir = debugfs_create_dir(name, regs_dir);
> + if (!ch_dir)
> + return;
> +
> + dw_hdma_debugfs_regs_ch(&regs->ch[i].rd, ch_dir);
> + }
> +}
> +
> +static void dw_hdma_debugfs_regs(void)
> +{
> + struct dentry *regs_dir;
> +
> + regs_dir = debugfs_create_dir(REGISTERS_STR, dw->dma.dbg_dev_root);
> + if (!regs_dir)
> + return;
> +
> + dw_hdma_debugfs_regs_wr(regs_dir);
> + dw_hdma_debugfs_regs_rd(regs_dir);
> +}
> +
> +void dw_hdma_v0_debugfs_on(struct dw_edma *dw)

not exit routine?

> +{
> + if (!debugfs_initialized())
> + return;
> +
> + debugfs_create_u32("mf", 0444, dw->dma.dbg_dev_root, &dw->chip->mf);
> + debugfs_create_u16("wr_ch_cnt", 0444, dw->dma.dbg_dev_root, &dw->wr_ch_cnt);
> + debugfs_create_u16("rd_ch_cnt", 0444, dw->dma.dbg_dev_root, &dw->rd_ch_cnt);
> +
> + dw_hdma_debugfs_regs();
> +}
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
> new file mode 100644
> index 000000000000..4b70911aedac
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2022 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 debugfs
> + *
> + * Author: Cai Huoqing <[email protected]>
> + */
> +
> +#ifndef _DW_HDMA_V0_DEBUG_FS_H
> +#define _DW_HDMA_V0_DEBUG_FS_H
> +
> +#include <linux/dma/edma.h>
> +
> +#ifdef CONFIG_DEBUG_FS
> +void dw_hdma_v0_debugfs_on(struct dw_edma *dw);
> +#else
> +static inline void dw_hdma_v0_debugfs_on(struct dw_edma *dw)
> +{
> +}
> +#endif /* CONFIG_DEBUG_FS */
> +
> +#endif /* _DW_HDMA_V0_DEBUG_FS_H */
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-regs.h b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
> new file mode 100644
> index 000000000000..677bf44400b3
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
> @@ -0,0 +1,98 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2022 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 reg
> + *
> + * Author: Cai Huoqing <[email protected]>
> + */
> +
> +#ifndef _DW_HDMA_V0_REGS_H
> +#define _DW_HDMA_V0_REGS_H
> +
> +#include <linux/dmaengine.h>
> +
> +#define HDMA_V0_MAX_NR_CH 8
> +#define HDMA_V0_LOCAL_ABORT_INT_EN BIT(6)
> +#define HDMA_V0_REMOTE_ABORT_INT_EN BIT(5)
> +#define HDMA_V0_LOCAL_STOP_INT_EN BIT(4)
> +#define HDMA_V0_REMOTEL_STOP_INT_EN BIT(3)
> +#define HDMA_V0_ABORT_INT_MASK BIT(2)
> +#define HDMA_V0_STOP_INT_MASK BIT(0)
> +#define HDMA_V0_LINKLIST_EN BIT(0)
> +#define HDMA_V0_CONSUMER_CYCLE_STAT BIT(1)
> +#define HDMA_V0_CONSUMER_CYCLE_BIT BIT(0)
> +#define HDMA_V0_DOORBELL_START BIT(0)
> +#define HDMA_V0_CH_STATUS_MASK GENMASK(1, 0)
> +
> +struct dw_hdma_v0_ch_regs {
> + u32 ch_en; /* 0x0000 */
> + u32 doorbell; /* 0x0004 */
> + u32 prefetch; /* 0x0008 */
> + u32 handshake; /* 0x000c */
> + union {
> + u64 reg; /* 0x0010..0x0014 */
> + struct {
> + u32 lsb; /* 0x0010 */
> + u32 msb; /* 0x0014 */
> + };
> + } llp;
> + u32 cycle_sync; /* 0x0018 */
> + u32 transfer_size; /* 0x001c */
> + union {
> + u64 reg; /* 0x0020..0x0024 */
> + struct {
> + u32 lsb; /* 0x0020 */
> + u32 msb; /* 0x0024 */
> + };
> + } sar;
> + union {
> + u64 reg; /* 0x0028..0x002c */
> + struct {
> + u32 lsb; /* 0x0028 */
> + u32 msb; /* 0x002c */
> + };
> + } dar;
> +
> + u32 watermark_en; /* 0x0030 */
> + u32 control1; /* 0x0034 */
> + u32 func_num; /* 0x0038 */
> + u32 qos; /* 0x003c */
> + u32 reserved; /* 0x0040..0x007c */
> + u32 ch_stat; /* 0x0080 */
> + u32 int_stat; /* 0x0084 */
> + u32 int_setup; /* 0x0088 */
> + u32 int_clear; /* 0x008c */
> + union {
> + u64 reg; /* 0x0090..0x0094 */
> + struct {
> + u32 lsb; /* 0x0090 */
> + u32 msb; /* 0x0094 */
> + };
> + } msi_stop;
> + union {
> + u64 reg; /* 0x0098..0x009c */
> + struct {
> + u32 lsb; /* 0x0098 */
> + u32 msb; /* 0x009c */
> + };
> + } msi_watermark;
> + union {
> + u64 reg; /* 0x00a0..0x00a4 */
> + struct {
> + u32 lsb; /* 0x00a0 */
> + u32 msb; /* 0x00a4 */
> + };
> + } msi_abort;
> + u32 msi_msgdata; /* 0x00a8 */
> +} __packed;
> +
> +struct dw_hdma_v0_ch {
> + struct dw_hdma_v0_ch_regs wr; /* 0x0000 */
> + struct dw_hdma_v0_ch_regs rd; /* 0x0100 */
> +} __packed;
> +
> +struct dw_hdma_v0_regs {
> + struct dw_hdma_v0_ch ch[HDMA_V0_MAX_NR_CH]; /* 0x0000..0x0fa8 */
> +} __packed;
> +
> +#endif /* _DW_HDMA_V0_REGS_H */
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index 642dd325259b..b879d107f08b 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -45,7 +45,8 @@ struct dw_edma_plat_ops {
> enum dw_edma_map_format {
> EDMA_MF_EDMA_LEGACY = 0x0,
> EDMA_MF_EDMA_UNROLL = 0x1,
> - EDMA_MF_HDMA_COMPAT = 0x5
> + EDMA_MF_HDMA_COMPAT = 0x5,
> + EDMA_MF_HDMA_NATIVE = 0x7
> };
>
> /**
> --
> 2.25.1

--
~Vinod

2022-11-07 03:57:59

by Cai Huoqing

[permalink] [raw]

Subject: Re: [PATCH 1/3] dmaengine: dw-edma: Rename dw_edma_core_ops structure to dw_edma_plat_ops

On 04 11月 22 19:12:22, Vinod Koul wrote:
> On 21-09-22, 14:48, Cai Huoqing wrote:
> > From: caihuoqing <[email protected]>
> >
> > Rename dw_edma_core_ops structure to dw_edma_plat_ops,
> > because the 'ops' is related to platform device
>
> Is that really a case, the device can be non platform too right?
Hi, Vinod

Just means plat_ops is the common part for different mode or chips.
core_ops is the private part for different spec.

Thanks,
Cai
>
> --
> ~Vinod

2022-11-07 04:00:37

by Cai Huoqing

[permalink] [raw]

Subject: Re: [PATCH 2/3] dmaengine: dw-edma: Create a new dw_edma_core_ops structure to abstract controller operation

On 04 11月 22 19:15:00, Vinod Koul wrote:
> On 21-09-22, 14:48, Cai Huoqing wrote:
> > From: caihuoqing <[email protected]>
> >
> > The structure dw_edma_core_ops has a set of the pointers
> > abstracting out the DW eDMA vX and DW HDMA Native controllers.
> > And use dw_edma_v0_core_register to set up operation.
>
> Core can mean different things, why not call it
> dw_edma_v0_{controller|cntrl}_regs
ok, will use dw_edma_v0_cntrl_resgister/dw_hdma_v0_cntrl_resgister
>
> > Signed-off-by: caihuoqing <[email protected]>
> > ---
> > drivers/dma/dw-edma/dw-edma-core.c | 61 ++++++------------
> > drivers/dma/dw-edma/dw-edma-core.h | 19 ++++++
> > drivers/dma/dw-edma/dw-edma-v0-core.c | 90 ++++++++++++++++++++++++---
> > drivers/dma/dw-edma/dw-edma-v0-core.h | 14 +----
> > 4 files changed, 120 insertions(+), 64 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index 5736a537f4c8..a66e4216a5d3 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -187,6 +187,7 @@ static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> > struct dw_edma_chunk *child;
> > struct dw_edma_desc *desc;
> > struct virt_dma_desc *vd;
> > + struct dw_edma *dw = chan->dw;
> >
> > vd = vchan_next_desc(&chan->vc);
> > if (!vd)
> > @@ -201,7 +202,7 @@ static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> > if (!child)
> > return;
> >
> > - dw_edma_v0_core_start(child, !desc->xfer_sz);
> > + dw->core->dw_xdma_core_start(child, !desc->xfer_sz);
> > desc->xfer_sz += child->ll_region.sz;
> > dw_edma_free_burst(child);
> > list_del(&child->list);
> > @@ -277,6 +278,7 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
> > static int dw_edma_device_terminate_all(struct dma_chan *dchan)
> > {
> > struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > + struct dw_edma *dw = chan->dw;
> > int err = 0;
> >
> > if (!chan->configured) {
> > @@ -286,7 +288,7 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
> > chan->configured = false;
> > } else if (chan->status == EDMA_ST_IDLE) {
> > chan->configured = false;
> > - } else if (dw_edma_v0_core_ch_status(chan) == DMA_COMPLETE) {
> > + } else if (dw->core->dw_xdma_core_ch_status(chan) == DMA_COMPLETE) {
> > /*
> > * The channel is in a false BUSY state, probably didn't
> > * receive or lost an interrupt
> > @@ -593,9 +595,10 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> > {
> > struct dw_edma_desc *desc;
> > struct virt_dma_desc *vd;
> > + struct dw_edma *dw = chan->dw;
> > unsigned long flags;
> >
> > - dw_edma_v0_core_clear_done_int(chan);
> > + dw->core->dw_xdma_core_clear_done_int(chan);
> >
> > spin_lock_irqsave(&chan->vc.lock, flags);
> > vd = vchan_next_desc(&chan->vc);
> > @@ -635,9 +638,10 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> > static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> > {
> > struct virt_dma_desc *vd;
> > + struct dw_edma *dw = chan->dw;
> > unsigned long flags;
> >
> > - dw_edma_v0_core_clear_abort_int(chan);
> > + dw->core->dw_xdma_core_clear_abort_int(chan);
> >
> > spin_lock_irqsave(&chan->vc.lock, flags);
> > vd = vchan_next_desc(&chan->vc);
> > @@ -654,39 +658,10 @@ 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;
> > - unsigned long total, pos, val;
> > - unsigned long off;
> > - u32 mask;
> > -
> > - if (write) {
> > - total = dw->wr_ch_cnt;
> > - off = 0;
> > - mask = dw_irq->wr_mask;
> > - } else {
> > - total = dw->rd_ch_cnt;
> > - off = dw->wr_ch_cnt;
> > - mask = dw_irq->rd_mask;
> > - }
> > -
> > - val = dw_edma_v0_core_status_done_int(dw, write ?
> > - EDMA_DIR_WRITE :
> > - EDMA_DIR_READ);
> > - val &= mask;
> > - for_each_set_bit(pos, &val, total) {
> > - struct dw_edma_chan *chan = &dw->chan[pos + off];
> > -
> > - dw_edma_done_interrupt(chan);
> > - }
> > -
> > - val = dw_edma_v0_core_status_abort_int(dw, write ?
> > - EDMA_DIR_WRITE :
> > - EDMA_DIR_READ);
> > - val &= mask;
> > - for_each_set_bit(pos, &val, total) {
> > - struct dw_edma_chan *chan = &dw->chan[pos + off];
> > + enum dw_edma_dir dir = write ? EDMA_DIR_WRITE : EDMA_DIR_READ;
> >
> > - dw_edma_abort_interrupt(chan);
> > - }
> > + dw->core->dw_xdma_core_done_handle(dw_irq, dw_edma_done_interrupt, dir);
> > + dw->core->dw_xdma_core_abort_handle(dw_irq, dw_edma_abort_interrupt, dir);
> >
> > return IRQ_HANDLED;
> > }
> > @@ -832,7 +807,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
> >
> > vchan_init(&chan->vc, dma);
> >
> > - dw_edma_v0_core_device_config(chan);
> > + dw->core->dw_xdma_core_ch_config(chan);
> > }
> >
> > /* Set DMA channel capabilities */
> > @@ -977,14 +952,16 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> >
> > dw->chip = chip;
> >
> > + dw_edma_v0_core_register(dw);
> > +
> > raw_spin_lock_init(&dw->lock);
> >
> > dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt,
> > - dw_edma_v0_core_ch_count(dw, EDMA_DIR_WRITE));
> > + dw->core->dw_xdma_core_ch_count(dw, 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, chip->ll_rd_cnt,
> > - dw_edma_v0_core_ch_count(dw, EDMA_DIR_READ));
> > + dw->core->dw_xdma_core_ch_count(dw, 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)
> > @@ -1003,7 +980,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> > dev_name(chip->dev));
> >
> > /* Disable eDMA, only to establish the ideal initial conditions */
> > - dw_edma_v0_core_off(dw);
> > + dw->core->dw_xdma_core_off(dw);
> >
> > /* Request IRQs */
> > err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
> > @@ -1019,7 +996,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> > pm_runtime_enable(dev);
> >
> > /* Turn debugfs on */
> > - dw_edma_v0_core_debugfs_on(dw);
> > + dw->core->dw_xdma_core_debugfs_on(dw);
> >
> > chip->dw = dw;
> >
> > @@ -1045,7 +1022,7 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> > return -ENODEV;
> >
> > /* Disable eDMA */
> > - dw_edma_v0_core_off(dw);
> > + dw->core->dw_xdma_core_off(dw);
> >
> > /* Free irqs */
> > for (i = (dw->nr_irqs - 1); i >= 0; i--)
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> > index 0ab2b6dba880..9b472d3c1596 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.h
> > +++ b/drivers/dma/dw-edma/dw-edma-core.h
> > @@ -95,6 +95,23 @@ struct dw_edma_irq {
> > struct dw_edma *dw;
> > };
> >
> > +struct dw_edma_core_ops {
> > + void (*dw_xdma_core_off)(struct dw_edma *dw);
> > + u16 (*dw_xdma_core_ch_count)(struct dw_edma *dw, enum dw_edma_dir dir);
> > + enum dma_status (*dw_xdma_core_ch_status)(struct dw_edma_chan *chan);
> > + void (*dw_xdma_core_clear_done_int)(struct dw_edma_chan *chan);
> > + void (*dw_xdma_core_clear_abort_int)(struct dw_edma_chan *chan);
> > + void (*dw_xdma_core_done_handle)(struct dw_edma_irq *dw_irq,
> > + void (*done_cb)(struct dw_edma_chan *chan),
> > + enum dw_edma_dir dir);
> > + void (*dw_xdma_core_abort_handle)(struct dw_edma_irq *dw_irq,
> > + void (*abort_cb)(struct dw_edma_chan *chan),
> > + enum dw_edma_dir dir);
> > + void (*dw_xdma_core_start)(struct dw_edma_chunk *chunk, bool first);
> > + int (*dw_xdma_core_ch_config)(struct dw_edma_chan *chan);
> > + void (*dw_xdma_core_debugfs_on)(struct dw_edma *dw);
> > +};
> > +
> > struct dw_edma {
> > char name[32];
> >
> > @@ -111,6 +128,8 @@ struct dw_edma {
> > raw_spinlock_t lock; /* Only for legacy */
> >
> > struct dw_edma_chip *chip;
> > +
> > + const struct dw_edma_core_ops *core;
> > };
> >
> > struct dw_edma_sg {
> > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > index 51a34b43434c..0c1a74245ecc 100644
> > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > @@ -223,7 +223,7 @@ static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> > writeq(value, ll)
> >
> > /* eDMA management callbacks */
> > -void dw_edma_v0_core_off(struct dw_edma *dw)
> > +static void dw_edma_v0_core_off(struct dw_edma *dw)
> > {
> > SET_BOTH_32(dw, int_mask,
> > EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
> > @@ -232,7 +232,7 @@ void dw_edma_v0_core_off(struct dw_edma *dw)
> > SET_BOTH_32(dw, engine_en, 0);
> > }
> >
> > -u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > +static u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > {
> > u32 num_ch;
> >
> > @@ -249,7 +249,7 @@ u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > return (u16)num_ch;
> > }
> >
> > -enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> > +static enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> > {
> > struct dw_edma *dw = chan->dw;
> > u32 tmp;
> > @@ -265,7 +265,7 @@ enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> > return DMA_ERROR;
> > }
> >
> > -void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> > +static void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> > {
> > struct dw_edma *dw = chan->dw;
> >
> > @@ -273,7 +273,7 @@ void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> > FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id)));
> > }
> >
> > -void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> > +static void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> > {
> > struct dw_edma *dw = chan->dw;
> >
> > @@ -281,18 +281,70 @@ void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> > 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)
> > +static u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > {
> > return FIELD_GET(EDMA_V0_DONE_INT_MASK,
> > GET_RW_32(dw, dir, int_status));
> > }
> >
> > -u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > +static u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > {
> > return FIELD_GET(EDMA_V0_ABORT_INT_MASK,
> > GET_RW_32(dw, dir, int_status));
> > }
> >
> > +static
> > +void dw_edma_v0_core_done_handle(struct dw_edma_irq *dw_irq,
> > + void (*done_cb)(struct dw_edma_chan *chan),
> > + enum dw_edma_dir dir)
> > +{
> > + struct dw_edma *dw = dw_irq->dw;
> > + unsigned long total, pos, val;
> > + unsigned long off;
> > + u32 mask;
> > +
> > + if (dir == EDMA_DIR_WRITE) {
> > + total = dw->wr_ch_cnt;
> > + off = 0;
> > + mask = dw_irq->wr_mask;
> > + } else {
> > + total = dw->rd_ch_cnt;
> > + off = dw->wr_ch_cnt;
> > + mask = dw_irq->rd_mask;
> > + }
> > +
> > + val = dw_edma_v0_core_status_done_int(dw, dir);
> > + val &= mask;
> > + for_each_set_bit(pos, &val, total)
> > + done_cb(&dw->chan[pos + off]);
> > +}
> > +
> > +static
> > +void dw_edma_v0_core_abort_handle(struct dw_edma_irq *dw_irq,
> > + void (*abort_cb)(struct dw_edma_chan *chan),
> > + enum dw_edma_dir dir)
> > +{
> > + struct dw_edma *dw = dw_irq->dw;
> > + unsigned long total, pos, val;
> > + unsigned long off;
> > + u32 mask;
> > +
> > + if (dir == EDMA_DIR_WRITE) {
> > + total = dw->wr_ch_cnt;
> > + off = 0;
> > + mask = dw_irq->wr_mask;
> > + } else {
> > + total = dw->rd_ch_cnt;
> > + off = dw->wr_ch_cnt;
> > + mask = dw_irq->rd_mask;
> > + }
> > +
> > + val = dw_edma_v0_core_status_abort_int(dw, dir);
> > + val &= mask;
> > + for_each_set_bit(pos, &val, total)
> > + abort_cb(&dw->chan[pos + off]);
> > +}
> > +
> > static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> > {
> > struct dw_edma_burst *child;
> > @@ -338,7 +390,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> > SET_LL_64(&llp->llp.reg, chunk->ll_region.paddr);
> > }
> >
> > -void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > +static 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->dw;
> > @@ -409,7 +461,7 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > FIELD_PREP(EDMA_V0_DOORBELL_CH_MASK, chan->id));
> > }
> >
> > -int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
> > +static int dw_edma_v0_core_ch_config(struct dw_edma_chan *chan)
> > {
> > struct dw_edma *dw = chan->dw;
> > u32 tmp = 0;
> > @@ -481,7 +533,25 @@ int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
> > }
> >
> > /* eDMA debugfs callbacks */
> > -void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> > +static void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> > {
> > dw_edma_v0_debugfs_on(dw);
> > }
> > +
> > +static const struct dw_edma_core_ops edma_core = {
> > + .dw_xdma_core_off = dw_edma_v0_core_off,
> > + .dw_xdma_core_ch_count = dw_edma_v0_core_ch_count,
> > + .dw_xdma_core_ch_status = dw_edma_v0_core_ch_status,
> > + .dw_xdma_core_clear_done_int = dw_edma_v0_core_clear_done_int,
> > + .dw_xdma_core_clear_abort_int = dw_edma_v0_core_clear_abort_int,
> > + .dw_xdma_core_done_handle = dw_edma_v0_core_done_handle,
> > + .dw_xdma_core_abort_handle = dw_edma_v0_core_abort_handle,
> > + .dw_xdma_core_start = dw_edma_v0_core_start,
> > + .dw_xdma_core_ch_config = dw_edma_v0_core_ch_config,
> > + .dw_xdma_core_debugfs_on = dw_edma_v0_core_debugfs_on,
>
> dw_xdma_ctrl_xxx sounds more right for these
>
> > +};
> > +
> > +void dw_edma_v0_core_register(struct dw_edma *dw)
> > +{
> > + dw->core = &edma_core;
> > +}
> > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.h b/drivers/dma/dw-edma/dw-edma-v0-core.h
> > index ab96a1f48080..972994ecc96f 100644
> > --- a/drivers/dma/dw-edma/dw-edma-v0-core.h
> > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.h
> > @@ -11,17 +11,7 @@
> >
> > #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);
> > -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);
> > -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 */
> > -void dw_edma_v0_core_debugfs_on(struct dw_edma *dw);
> > +/* eDMA core operation register*/
> > +void dw_edma_v0_core_register(struct dw_edma *dw);
> >
> > #endif /* _DW_EDMA_V0_CORE_H */
> > --
> > 2.25.1
>
> --
> ~Vinod

2023-02-09 22:41:23

by Serge Semin

[permalink] [raw]

Subject: Re: [PATCH 0/3] dmaengine: dw-edma: Add support for native HDMA

Hello Cai

On Wed, Sep 21, 2022 at 02:48:49PM +0800, Cai Huoqing wrote:
> From: caihuoqing <[email protected]>
>
> Add support for HDMA NATIVE, as long the IP design has set
> the compatible register map parameter-HDMA_NATIVE,
> which allows compatibility for native HDMA register configuration.
>
> The HDMA Hyper-DMA IP is an enhancement of the eDMA embedded-DMA IP.
> And the native HDMA registers are different from eDMA,
> so this patch add support for HDMA NATIVE mode.
>
> HDMA write and read channels operate independently to maximize
> the performance of the HDMA read and write data transfer over
> the link When you configure the HDMA with multiple read channels,
> then it uses a round robin (RR) arbitration scheme to select
> the next read channel to be serviced.
> The same applies when you have multiple write channels.
>
> The native HDMA driver also supports a maximum of 16 independent
> channels (8 write + 8 read), which can run simultaneously.
> Both SAR (Source Address Register) and DAR (Destination Address Register)
> are alignmented to byte.dmaengine: dw-edma: Add support for native HDMA
>
> These series based on the series
> https://lore.kernel.org/dmaengine/[email protected]/

My DW PCIe/eDMA series has been finally accepted by @Bjorn:
https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/log/?h=pci/ctrl/dwc
(@Lorenzo, please note my changes in your pci/dwc are a bit outdated.)
The patchset will get into the mainline kernel on v6.3. So if you want
your series to go further you'll need either rebase your patches on the
@Bjorn's branch or wait for the kernel v6.3 release and rebase your work
on top of the latest kernel then. I'll provide some comments regarding
this patchset shortly.

-Serge(y)

>
> Cai Huoqing (3):
> dmaengine: dw-edma: Rename dw_edma_core_ops structure to
> dw_edma_plat_ops
> dmaengine: dw-edma: Create a new dw_edma_core_ops structure to
> abstract controller operation
> dmaengine: dw-edma: Add support for native HDMA
>
> drivers/dma/dw-edma/Makefile | 6 +-
> drivers/dma/dw-edma/dw-edma-core.c | 65 ++---
> drivers/dma/dw-edma/dw-edma-core.h | 19 ++
> drivers/dma/dw-edma/dw-edma-pcie.c | 4 +-
> drivers/dma/dw-edma/dw-edma-v0-core.c | 90 ++++++-
> drivers/dma/dw-edma/dw-edma-v0-core.h | 14 +-
> drivers/dma/dw-edma/dw-hdma-v0-core.c | 304 +++++++++++++++++++++++
> drivers/dma/dw-edma/dw-hdma-v0-core.h | 17 ++
> drivers/dma/dw-edma/dw-hdma-v0-debugfs.c | 150 +++++++++++
> drivers/dma/dw-edma/dw-hdma-v0-debugfs.h | 22 ++
> drivers/dma/dw-edma/dw-hdma-v0-regs.h | 98 ++++++++
> include/linux/dma/edma.h | 7 +-
> 12 files changed, 725 insertions(+), 71 deletions(-)
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.c
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.h
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-regs.h
>
> --
> 2.25.1
>

2023-02-09 23:26:46

by Serge Semin

[permalink] [raw]

Subject: Re: [PATCH 1/3] dmaengine: dw-edma: Rename dw_edma_core_ops structure to dw_edma_plat_ops

Hi Vinod

On Fri, Nov 04, 2022 at 07:12:22PM +0530, Vinod Koul wrote:
> On 21-09-22, 14:48, Cai Huoqing wrote:
> > From: caihuoqing <[email protected]>
> >
> > Rename dw_edma_core_ops structure to dw_edma_plat_ops,
> > because the 'ops' is related to platform device
>
> Is that really a case, the device can be non platform too right?

The patchlog is misleading in the suggested naming convention. The
dw_edma_pcie_plat_ops name was supposed to refer to the platform which
the DW eDMA engine is embedded to, like PCIe end-point (accessible via
the PCIe bus) or a PCIe root port (directly accessible by CPU).
Needless to say that for them the IRQ-vector and PCI-addresses are
differently determined. The suggested name has a connection with the
kernel platform device only as a private case of the eDMA/hDMA embedded
into the DW PCI Root ports, though basically it was supposed to refer to
any platform in which the DMA hardware lives.

Anyway the renaming was necessary to distinguish two types of
the implementation callbacks:
1. DW eDMA/hDMA IP-core specific operations: device-specific CSR
setups in one or another aspect of the DMA-engine initialization.
2. DW eDMA/hDMA platform specific operations: the DMA device
environment configs like IRQs, address translation, etc.

dw_edma_pcie_core_ops is supposed to be used for the case 1, and
dw_edma_pcie_plat_ops - for the case 2.

-Serge(y)

>
> --
> ~Vinod

2023-02-09 23:34:43

by Serge Semin

[permalink] [raw]

Subject: Re: [PATCH 1/3] dmaengine: dw-edma: Rename dw_edma_core_ops structure to dw_edma_plat_ops

On Wed, Sep 21, 2022 at 02:48:50PM +0800, Cai Huoqing wrote:
> From: caihuoqing <[email protected]>
>
> Rename dw_edma_core_ops structure to dw_edma_plat_ops,
> because the 'ops' is related to platform device

@Vinod is right. The commit log is misleading. Please elaborate.

>
> Signed-off-by: caihuoqing <[email protected]>
> ---
> drivers/dma/dw-edma/dw-edma-pcie.c | 4 ++--
> include/linux/dma/edma.h | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index 3f9dadc73854..5604f3771866 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -109,7 +109,7 @@ static u64 dw_edma_pcie_address(struct device *dev, phys_addr_t cpu_addr)
> return region.start;
> }
>
> -static const struct dw_edma_core_ops dw_edma_pcie_core_ops = {
> +static const struct dw_edma_plat_ops dw_edma_pcie_plat_ops = {
> .irq_vector = dw_edma_pcie_irq_vector,
> .pci_address = dw_edma_pcie_address,
> };
> @@ -225,7 +225,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>
> chip->mf = vsec_data.mf;
> chip->nr_irqs = nr_irqs;
> - chip->ops = &dw_edma_pcie_core_ops;
> + chip->ops = &dw_edma_pcie_plat_ops;
>
> chip->ll_wr_cnt = vsec_data.wr_ch_cnt;
> chip->ll_rd_cnt = vsec_data.rd_ch_cnt;
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index 9d44da4aa59d..642dd325259b 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -37,7 +37,7 @@ struct dw_edma_region {
> * iATU windows. That will be done by the controller
> * automatically.
> */
> -struct dw_edma_core_ops {
> +struct dw_edma_plat_ops {
> int (*irq_vector)(struct device *dev, unsigned int nr);
> u64 (*pci_address)(struct device *dev, phys_addr_t cpu_addr);
> };
> @@ -77,7 +77,7 @@ enum dw_edma_chip_flags {
> struct dw_edma_chip {
> struct device *dev;
> int nr_irqs;
> - const struct dw_edma_core_ops *ops;
> + const struct dw_edma_plat_ops *ops;
> u32 flags;
>
> void __iomem *reg_base;

This patch should also update the structure name in the DW PCIe driver:
https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/tree/drivers/pci/controller/dwc/pcie-designware.c?h=pci/ctrl/dwc#n831

-Serge(y)

> --
> 2.25.1
>

2023-02-10 00:59:10

by Serge Semin

[permalink] [raw]

Subject: Re: [PATCH 2/3] dmaengine: dw-edma: Create a new dw_edma_core_ops structure to abstract controller operation

On Fri, Nov 04, 2022 at 07:15:00PM +0530, Vinod Koul wrote:
> On 21-09-22, 14:48, Cai Huoqing wrote:
> > From: caihuoqing <[email protected]>
> >
> > The structure dw_edma_core_ops has a set of the pointers
> > abstracting out the DW eDMA vX and DW HDMA Native controllers.
> > And use dw_edma_v0_core_register to set up operation.
>

> Core can mean different things, why not call it
> dw_edma_v0_{controller|cntrl}_regs

In this case it refers to the DW eDMA and HDMA IP-cores and to the
driver core where their specifics are implemented. In the later
aspects it just reflects the already defined naming conventions:
dw-edma-core.c
dw-edma-v0-core.c
dw_edma_v0_core_off()
dw_edma_v0_core_ch_count()
dw_edma_v0_core_ch_status()
dw_edma_v0_core_clear_done_int()
dw_edma_v0_core_clear_abort_int()
dw_edma_v0_core_status_done_int()
dw_edma_v0_core_status_abort_int()
dw_edma_v0_core_write_chunk()
dw_edma_v0_core_start()
dw_edma_v0_core_device_config()
dw_edma_v0_core_debugfs_on()
dw_edma_v0_core_debugfs_off()
introducing a new name may cause a redundant confusion meanwhile using
the same suffix for all driver core, device implementation specific
callbacks and operations descriptor would give a better readability.
Don't you think?

I also don't think that the "_regs" suffix looks better here.
"_ops" seems more descriptive. Do you think otherwise?

-Serge(y)

>
> > Signed-off-by: caihuoqing <[email protected]>
> > ---
> > drivers/dma/dw-edma/dw-edma-core.c | 61 ++++++------------
> > drivers/dma/dw-edma/dw-edma-core.h | 19 ++++++
> > drivers/dma/dw-edma/dw-edma-v0-core.c | 90 ++++++++++++++++++++++++---
> > drivers/dma/dw-edma/dw-edma-v0-core.h | 14 +----
> > 4 files changed, 120 insertions(+), 64 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index 5736a537f4c8..a66e4216a5d3 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -187,6 +187,7 @@ static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> > struct dw_edma_chunk *child;
> > struct dw_edma_desc *desc;
> > struct virt_dma_desc *vd;
> > + struct dw_edma *dw = chan->dw;
> >
> > vd = vchan_next_desc(&chan->vc);
> > if (!vd)
> > @@ -201,7 +202,7 @@ static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> > if (!child)
> > return;
> >
> > - dw_edma_v0_core_start(child, !desc->xfer_sz);
> > + dw->core->dw_xdma_core_start(child, !desc->xfer_sz);
> > desc->xfer_sz += child->ll_region.sz;
> > dw_edma_free_burst(child);
> > list_del(&child->list);
> > @@ -277,6 +278,7 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
> > static int dw_edma_device_terminate_all(struct dma_chan *dchan)
> > {
> > struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > + struct dw_edma *dw = chan->dw;
> > int err = 0;
> >
> > if (!chan->configured) {
> > @@ -286,7 +288,7 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
> > chan->configured = false;
> > } else if (chan->status == EDMA_ST_IDLE) {
> > chan->configured = false;
> > - } else if (dw_edma_v0_core_ch_status(chan) == DMA_COMPLETE) {
> > + } else if (dw->core->dw_xdma_core_ch_status(chan) == DMA_COMPLETE) {
> > /*
> > * The channel is in a false BUSY state, probably didn't
> > * receive or lost an interrupt
> > @@ -593,9 +595,10 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> > {
> > struct dw_edma_desc *desc;
> > struct virt_dma_desc *vd;
> > + struct dw_edma *dw = chan->dw;
> > unsigned long flags;
> >
> > - dw_edma_v0_core_clear_done_int(chan);
> > + dw->core->dw_xdma_core_clear_done_int(chan);
> >
> > spin_lock_irqsave(&chan->vc.lock, flags);
> > vd = vchan_next_desc(&chan->vc);
> > @@ -635,9 +638,10 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> > static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> > {
> > struct virt_dma_desc *vd;
> > + struct dw_edma *dw = chan->dw;
> > unsigned long flags;
> >
> > - dw_edma_v0_core_clear_abort_int(chan);
> > + dw->core->dw_xdma_core_clear_abort_int(chan);
> >
> > spin_lock_irqsave(&chan->vc.lock, flags);
> > vd = vchan_next_desc(&chan->vc);
> > @@ -654,39 +658,10 @@ 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;
> > - unsigned long total, pos, val;
> > - unsigned long off;
> > - u32 mask;
> > -
> > - if (write) {
> > - total = dw->wr_ch_cnt;
> > - off = 0;
> > - mask = dw_irq->wr_mask;
> > - } else {
> > - total = dw->rd_ch_cnt;
> > - off = dw->wr_ch_cnt;
> > - mask = dw_irq->rd_mask;
> > - }
> > -
> > - val = dw_edma_v0_core_status_done_int(dw, write ?
> > - EDMA_DIR_WRITE :
> > - EDMA_DIR_READ);
> > - val &= mask;
> > - for_each_set_bit(pos, &val, total) {
> > - struct dw_edma_chan *chan = &dw->chan[pos + off];
> > -
> > - dw_edma_done_interrupt(chan);
> > - }
> > -
> > - val = dw_edma_v0_core_status_abort_int(dw, write ?
> > - EDMA_DIR_WRITE :
> > - EDMA_DIR_READ);
> > - val &= mask;
> > - for_each_set_bit(pos, &val, total) {
> > - struct dw_edma_chan *chan = &dw->chan[pos + off];
> > + enum dw_edma_dir dir = write ? EDMA_DIR_WRITE : EDMA_DIR_READ;
> >
> > - dw_edma_abort_interrupt(chan);
> > - }
> > + dw->core->dw_xdma_core_done_handle(dw_irq, dw_edma_done_interrupt, dir);
> > + dw->core->dw_xdma_core_abort_handle(dw_irq, dw_edma_abort_interrupt, dir);
> >
> > return IRQ_HANDLED;
> > }
> > @@ -832,7 +807,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
> >
> > vchan_init(&chan->vc, dma);
> >
> > - dw_edma_v0_core_device_config(chan);
> > + dw->core->dw_xdma_core_ch_config(chan);
> > }
> >
> > /* Set DMA channel capabilities */
> > @@ -977,14 +952,16 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> >
> > dw->chip = chip;
> >
> > + dw_edma_v0_core_register(dw);
> > +
> > raw_spin_lock_init(&dw->lock);
> >
> > dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt,
> > - dw_edma_v0_core_ch_count(dw, EDMA_DIR_WRITE));
> > + dw->core->dw_xdma_core_ch_count(dw, 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, chip->ll_rd_cnt,
> > - dw_edma_v0_core_ch_count(dw, EDMA_DIR_READ));
> > + dw->core->dw_xdma_core_ch_count(dw, 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)
> > @@ -1003,7 +980,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> > dev_name(chip->dev));
> >
> > /* Disable eDMA, only to establish the ideal initial conditions */
> > - dw_edma_v0_core_off(dw);
> > + dw->core->dw_xdma_core_off(dw);
> >
> > /* Request IRQs */
> > err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
> > @@ -1019,7 +996,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> > pm_runtime_enable(dev);
> >
> > /* Turn debugfs on */
> > - dw_edma_v0_core_debugfs_on(dw);
> > + dw->core->dw_xdma_core_debugfs_on(dw);
> >
> > chip->dw = dw;
> >
> > @@ -1045,7 +1022,7 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> > return -ENODEV;
> >
> > /* Disable eDMA */
> > - dw_edma_v0_core_off(dw);
> > + dw->core->dw_xdma_core_off(dw);
> >
> > /* Free irqs */
> > for (i = (dw->nr_irqs - 1); i >= 0; i--)
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> > index 0ab2b6dba880..9b472d3c1596 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.h
> > +++ b/drivers/dma/dw-edma/dw-edma-core.h
> > @@ -95,6 +95,23 @@ struct dw_edma_irq {
> > struct dw_edma *dw;
> > };
> >
> > +struct dw_edma_core_ops {
> > + void (*dw_xdma_core_off)(struct dw_edma *dw);
> > + u16 (*dw_xdma_core_ch_count)(struct dw_edma *dw, enum dw_edma_dir dir);
> > + enum dma_status (*dw_xdma_core_ch_status)(struct dw_edma_chan *chan);
> > + void (*dw_xdma_core_clear_done_int)(struct dw_edma_chan *chan);
> > + void (*dw_xdma_core_clear_abort_int)(struct dw_edma_chan *chan);
> > + void (*dw_xdma_core_done_handle)(struct dw_edma_irq *dw_irq,
> > + void (*done_cb)(struct dw_edma_chan *chan),
> > + enum dw_edma_dir dir);
> > + void (*dw_xdma_core_abort_handle)(struct dw_edma_irq *dw_irq,
> > + void (*abort_cb)(struct dw_edma_chan *chan),
> > + enum dw_edma_dir dir);
> > + void (*dw_xdma_core_start)(struct dw_edma_chunk *chunk, bool first);
> > + int (*dw_xdma_core_ch_config)(struct dw_edma_chan *chan);
> > + void (*dw_xdma_core_debugfs_on)(struct dw_edma *dw);
> > +};
> > +
> > struct dw_edma {
> > char name[32];
> >
> > @@ -111,6 +128,8 @@ struct dw_edma {
> > raw_spinlock_t lock; /* Only for legacy */
> >
> > struct dw_edma_chip *chip;
> > +
> > + const struct dw_edma_core_ops *core;
> > };
> >
> > struct dw_edma_sg {
> > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > index 51a34b43434c..0c1a74245ecc 100644
> > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > @@ -223,7 +223,7 @@ static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> > writeq(value, ll)
> >
> > /* eDMA management callbacks */
> > -void dw_edma_v0_core_off(struct dw_edma *dw)
> > +static void dw_edma_v0_core_off(struct dw_edma *dw)
> > {
> > SET_BOTH_32(dw, int_mask,
> > EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
> > @@ -232,7 +232,7 @@ void dw_edma_v0_core_off(struct dw_edma *dw)
> > SET_BOTH_32(dw, engine_en, 0);
> > }
> >
> > -u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > +static u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > {
> > u32 num_ch;
> >
> > @@ -249,7 +249,7 @@ u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > return (u16)num_ch;
> > }
> >
> > -enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> > +static enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> > {
> > struct dw_edma *dw = chan->dw;
> > u32 tmp;
> > @@ -265,7 +265,7 @@ enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> > return DMA_ERROR;
> > }
> >
> > -void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> > +static void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> > {
> > struct dw_edma *dw = chan->dw;
> >
> > @@ -273,7 +273,7 @@ void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> > FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id)));
> > }
> >
> > -void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> > +static void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> > {
> > struct dw_edma *dw = chan->dw;
> >
> > @@ -281,18 +281,70 @@ void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> > 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)
> > +static u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > {
> > return FIELD_GET(EDMA_V0_DONE_INT_MASK,
> > GET_RW_32(dw, dir, int_status));
> > }
> >
> > -u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > +static u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > {
> > return FIELD_GET(EDMA_V0_ABORT_INT_MASK,
> > GET_RW_32(dw, dir, int_status));
> > }
> >
> > +static
> > +void dw_edma_v0_core_done_handle(struct dw_edma_irq *dw_irq,
> > + void (*done_cb)(struct dw_edma_chan *chan),
> > + enum dw_edma_dir dir)
> > +{
> > + struct dw_edma *dw = dw_irq->dw;
> > + unsigned long total, pos, val;
> > + unsigned long off;
> > + u32 mask;
> > +
> > + if (dir == EDMA_DIR_WRITE) {
> > + total = dw->wr_ch_cnt;
> > + off = 0;
> > + mask = dw_irq->wr_mask;
> > + } else {
> > + total = dw->rd_ch_cnt;
> > + off = dw->wr_ch_cnt;
> > + mask = dw_irq->rd_mask;
> > + }
> > +
> > + val = dw_edma_v0_core_status_done_int(dw, dir);
> > + val &= mask;
> > + for_each_set_bit(pos, &val, total)
> > + done_cb(&dw->chan[pos + off]);
> > +}
> > +
> > +static
> > +void dw_edma_v0_core_abort_handle(struct dw_edma_irq *dw_irq,
> > + void (*abort_cb)(struct dw_edma_chan *chan),
> > + enum dw_edma_dir dir)
> > +{
> > + struct dw_edma *dw = dw_irq->dw;
> > + unsigned long total, pos, val;
> > + unsigned long off;
> > + u32 mask;
> > +
> > + if (dir == EDMA_DIR_WRITE) {
> > + total = dw->wr_ch_cnt;
> > + off = 0;
> > + mask = dw_irq->wr_mask;
> > + } else {
> > + total = dw->rd_ch_cnt;
> > + off = dw->wr_ch_cnt;
> > + mask = dw_irq->rd_mask;
> > + }
> > +
> > + val = dw_edma_v0_core_status_abort_int(dw, dir);
> > + val &= mask;
> > + for_each_set_bit(pos, &val, total)
> > + abort_cb(&dw->chan[pos + off]);
> > +}
> > +
> > static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> > {
> > struct dw_edma_burst *child;
> > @@ -338,7 +390,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> > SET_LL_64(&llp->llp.reg, chunk->ll_region.paddr);
> > }
> >
> > -void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > +static 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->dw;
> > @@ -409,7 +461,7 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > FIELD_PREP(EDMA_V0_DOORBELL_CH_MASK, chan->id));
> > }
> >
> > -int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
> > +static int dw_edma_v0_core_ch_config(struct dw_edma_chan *chan)
> > {
> > struct dw_edma *dw = chan->dw;
> > u32 tmp = 0;
> > @@ -481,7 +533,25 @@ int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
> > }
> >
> > /* eDMA debugfs callbacks */
> > -void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> > +static void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> > {
> > dw_edma_v0_debugfs_on(dw);
> > }
> > +
> > +static const struct dw_edma_core_ops edma_core = {
> > + .dw_xdma_core_off = dw_edma_v0_core_off,
> > + .dw_xdma_core_ch_count = dw_edma_v0_core_ch_count,
> > + .dw_xdma_core_ch_status = dw_edma_v0_core_ch_status,
> > + .dw_xdma_core_clear_done_int = dw_edma_v0_core_clear_done_int,
> > + .dw_xdma_core_clear_abort_int = dw_edma_v0_core_clear_abort_int,
> > + .dw_xdma_core_done_handle = dw_edma_v0_core_done_handle,
> > + .dw_xdma_core_abort_handle = dw_edma_v0_core_abort_handle,
> > + .dw_xdma_core_start = dw_edma_v0_core_start,
> > + .dw_xdma_core_ch_config = dw_edma_v0_core_ch_config,
> > + .dw_xdma_core_debugfs_on = dw_edma_v0_core_debugfs_on,
>
> dw_xdma_ctrl_xxx sounds more right for these
>
> > +};
> > +
> > +void dw_edma_v0_core_register(struct dw_edma *dw)
> > +{
> > + dw->core = &edma_core;
> > +}
> > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.h b/drivers/dma/dw-edma/dw-edma-v0-core.h
> > index ab96a1f48080..972994ecc96f 100644
> > --- a/drivers/dma/dw-edma/dw-edma-v0-core.h
> > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.h
> > @@ -11,17 +11,7 @@
> >
> > #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);
> > -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);
> > -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 */
> > -void dw_edma_v0_core_debugfs_on(struct dw_edma *dw);
> > +/* eDMA core operation register*/
> > +void dw_edma_v0_core_register(struct dw_edma *dw);
> >
> > #endif /* _DW_EDMA_V0_CORE_H */
> > --
> > 2.25.1
>
> --
> ~Vinod

2023-02-10 01:00:22

by Serge Semin

[permalink] [raw]

Subject: Re: [PATCH 0/3] dmaengine: dw-edma: Add support for native HDMA

Hi Vinod

On Wed, Sep 21, 2022 at 02:48:49PM +0800, Cai Huoqing wrote:
> From: caihuoqing <[email protected]>
>
> Add support for HDMA NATIVE, as long the IP design has set
> the compatible register map parameter-HDMA_NATIVE,
> which allows compatibility for native HDMA register configuration.
>
> The HDMA Hyper-DMA IP is an enhancement of the eDMA embedded-DMA IP.
> And the native HDMA registers are different from eDMA,
> so this patch add support for HDMA NATIVE mode.
>
> HDMA write and read channels operate independently to maximize
> the performance of the HDMA read and write data transfer over
> the link When you configure the HDMA with multiple read channels,
> then it uses a round robin (RR) arbitration scheme to select
> the next read channel to be serviced.
> The same applies when you have multiple write channels.
>
> The native HDMA driver also supports a maximum of 16 independent
> channels (8 write + 8 read), which can run simultaneously.
> Both SAR (Source Address Register) and DAR (Destination Address Register)
> are alignmented to byte.dmaengine: dw-edma: Add support for native HDMA
>
> These series based on the series
> https://lore.kernel.org/dmaengine/[email protected]/

Please note this patchset is a refactored version of the patch
submitted by Cai a while ago
https://lore.kernel.org/dmaengine/[email protected]/
The main design aspects implemented here were discussed in the
framework of that thread.

-Serge(y)

>
> Cai Huoqing (3):
> dmaengine: dw-edma: Rename dw_edma_core_ops structure to
> dw_edma_plat_ops
> dmaengine: dw-edma: Create a new dw_edma_core_ops structure to
> abstract controller operation
> dmaengine: dw-edma: Add support for native HDMA
>
> drivers/dma/dw-edma/Makefile | 6 +-
> drivers/dma/dw-edma/dw-edma-core.c | 65 ++---
> drivers/dma/dw-edma/dw-edma-core.h | 19 ++
> drivers/dma/dw-edma/dw-edma-pcie.c | 4 +-
> drivers/dma/dw-edma/dw-edma-v0-core.c | 90 ++++++-
> drivers/dma/dw-edma/dw-edma-v0-core.h | 14 +-
> drivers/dma/dw-edma/dw-hdma-v0-core.c | 304 +++++++++++++++++++++++
> drivers/dma/dw-edma/dw-hdma-v0-core.h | 17 ++
> drivers/dma/dw-edma/dw-hdma-v0-debugfs.c | 150 +++++++++++
> drivers/dma/dw-edma/dw-hdma-v0-debugfs.h | 22 ++
> drivers/dma/dw-edma/dw-hdma-v0-regs.h | 98 ++++++++
> include/linux/dma/edma.h | 7 +-
> 12 files changed, 725 insertions(+), 71 deletions(-)
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.c
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.h
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-regs.h
>
> --
> 2.25.1
>

2023-02-10 17:06:32

by Serge Semin

[permalink] [raw]

Subject: Re: [PATCH 2/3] dmaengine: dw-edma: Create a new dw_edma_core_ops structure to abstract controller operation

On Wed, Sep 21, 2022 at 02:48:51PM +0800, Cai Huoqing wrote:
> From: caihuoqing <[email protected]>
>
> The structure dw_edma_core_ops has a set of the pointers
> abstracting out the DW eDMA vX and DW HDMA Native controllers.
> And use dw_edma_v0_core_register to set up operation.
>
> Signed-off-by: caihuoqing <[email protected]>
> ---
> drivers/dma/dw-edma/dw-edma-core.c | 61 ++++++------------
> drivers/dma/dw-edma/dw-edma-core.h | 19 ++++++
> drivers/dma/dw-edma/dw-edma-v0-core.c | 90 ++++++++++++++++++++++++---
> drivers/dma/dw-edma/dw-edma-v0-core.h | 14 +----
> 4 files changed, 120 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 5736a537f4c8..a66e4216a5d3 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -187,6 +187,7 @@ static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> struct dw_edma_chunk *child;
> struct dw_edma_desc *desc;
> struct virt_dma_desc *vd;

> + struct dw_edma *dw = chan->dw;

Either use the reverse xmas tree order or alternatively you could
directly pass "chan->dw" to the corresponding function (see my
suggestion below).

>
> vd = vchan_next_desc(&chan->vc);
> if (!vd)
> @@ -201,7 +202,7 @@ static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> if (!child)
> return;
>

> - dw_edma_v0_core_start(child, !desc->xfer_sz);
> + dw->core->dw_xdma_core_start(child, !desc->xfer_sz);

Using the naked function pointers isn't that much readable. What about
introducing a set of static inline methods defined in the
dw-edma-core.h file for each op-pointer? For instance in this case the
wrapper would look like this:

static inline
dw_edma_v0_core_start(struct dw_edma *dw, struct dw_edma_chunk *chunk, bool first)
{
dw->core->start(child, first);
}

> desc->xfer_sz += child->ll_region.sz;
> dw_edma_free_burst(child);
> list_del(&child->list);
> @@ -277,6 +278,7 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
> static int dw_edma_device_terminate_all(struct dma_chan *dchan)
> {
> struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> + struct dw_edma *dw = chan->dw;
> int err = 0;
>
> if (!chan->configured) {
> @@ -286,7 +288,7 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
> chan->configured = false;
> } else if (chan->status == EDMA_ST_IDLE) {
> chan->configured = false;
> - } else if (dw_edma_v0_core_ch_status(chan) == DMA_COMPLETE) {
> + } else if (dw->core->dw_xdma_core_ch_status(chan) == DMA_COMPLETE) {
> /*
> * The channel is in a false BUSY state, probably didn't
> * receive or lost an interrupt
> @@ -593,9 +595,10 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> {

> struct dw_edma_desc *desc;
> struct virt_dma_desc *vd;
> + struct dw_edma *dw = chan->dw;

ditto

> unsigned long flags;
>
> - dw_edma_v0_core_clear_done_int(chan);
> + dw->core->dw_xdma_core_clear_done_int(chan);
>
> spin_lock_irqsave(&chan->vc.lock, flags);
> vd = vchan_next_desc(&chan->vc);
> @@ -635,9 +638,10 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> {

> struct virt_dma_desc *vd;
> + struct dw_edma *dw = chan->dw;

ditto

> unsigned long flags;
>
> - dw_edma_v0_core_clear_abort_int(chan);
> + dw->core->dw_xdma_core_clear_abort_int(chan);
>
> spin_lock_irqsave(&chan->vc.lock, flags);
> vd = vchan_next_desc(&chan->vc);
> @@ -654,39 +658,10 @@ 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;
> - unsigned long total, pos, val;
> - unsigned long off;
> - u32 mask;
> -
> - if (write) {
> - total = dw->wr_ch_cnt;
> - off = 0;
> - mask = dw_irq->wr_mask;
> - } else {
> - total = dw->rd_ch_cnt;
> - off = dw->wr_ch_cnt;
> - mask = dw_irq->rd_mask;
> - }
> -
> - val = dw_edma_v0_core_status_done_int(dw, write ?
> - EDMA_DIR_WRITE :
> - EDMA_DIR_READ);
> - val &= mask;
> - for_each_set_bit(pos, &val, total) {
> - struct dw_edma_chan *chan = &dw->chan[pos + off];
> -
> - dw_edma_done_interrupt(chan);
> - }
> -
> - val = dw_edma_v0_core_status_abort_int(dw, write ?
> - EDMA_DIR_WRITE :
> - EDMA_DIR_READ);
> - val &= mask;
> - for_each_set_bit(pos, &val, total) {
> - struct dw_edma_chan *chan = &dw->chan[pos + off];

> + enum dw_edma_dir dir = write ? EDMA_DIR_WRITE : EDMA_DIR_READ;

ditto. Mind using reverse xmas tree vars definition order.

>
> - dw_edma_abort_interrupt(chan);
> - }
> + dw->core->dw_xdma_core_done_handle(dw_irq, dw_edma_done_interrupt, dir);

> + dw->core->dw_xdma_core_abort_handle(dw_irq, dw_edma_abort_interrupt, dir);

In addition to the status_done_int() and status_abort_int() global
methods you can get rid from two callbacks in this case:
dw_xdma_core_clear_done_int()
dw_xdma_core_clear_abort_int()
Just call the corresponding functions within the core-specific
core_done_handle() and core_abort_handle() methods before calling the
callbacks: dw_edma_done_interrupt() and dw_edma_abort_interrupt().

That way the IRQs handling procedure will be simplified a bit.

>
> return IRQ_HANDLED;
> }
> @@ -832,7 +807,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
>
> vchan_init(&chan->vc, dma);
>
> - dw_edma_v0_core_device_config(chan);
> + dw->core->dw_xdma_core_ch_config(chan);
> }
>
> /* Set DMA channel capabilities */
> @@ -977,14 +952,16 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>
> dw->chip = chip;
>
> + dw_edma_v0_core_register(dw);
> +
> raw_spin_lock_init(&dw->lock);
>
> dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt,
> - dw_edma_v0_core_ch_count(dw, EDMA_DIR_WRITE));
> + dw->core->dw_xdma_core_ch_count(dw, 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, chip->ll_rd_cnt,
> - dw_edma_v0_core_ch_count(dw, EDMA_DIR_READ));
> + dw->core->dw_xdma_core_ch_count(dw, 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)
> @@ -1003,7 +980,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> dev_name(chip->dev));
>
> /* Disable eDMA, only to establish the ideal initial conditions */
> - dw_edma_v0_core_off(dw);
> + dw->core->dw_xdma_core_off(dw);
>
> /* Request IRQs */
> err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
> @@ -1019,7 +996,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> pm_runtime_enable(dev);
>
> /* Turn debugfs on */
> - dw_edma_v0_core_debugfs_on(dw);
> + dw->core->dw_xdma_core_debugfs_on(dw);
>
> chip->dw = dw;
>
> @@ -1045,7 +1022,7 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> return -ENODEV;
>
> /* Disable eDMA */
> - dw_edma_v0_core_off(dw);
> + dw->core->dw_xdma_core_off(dw);
>
> /* Free irqs */
> for (i = (dw->nr_irqs - 1); i >= 0; i--)
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index 0ab2b6dba880..9b472d3c1596 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -95,6 +95,23 @@ struct dw_edma_irq {
> struct dw_edma *dw;
> };
>

> +struct dw_edma_core_ops {
> + void (*dw_xdma_core_off)(struct dw_edma *dw);
> + u16 (*dw_xdma_core_ch_count)(struct dw_edma *dw, enum dw_edma_dir dir);
> + enum dma_status (*dw_xdma_core_ch_status)(struct dw_edma_chan *chan);

> + void (*dw_xdma_core_clear_done_int)(struct dw_edma_chan *chan);
> + void (*dw_xdma_core_clear_abort_int)(struct dw_edma_chan *chan);

As I said above just call these methods in the IRQ-handle loops within
the core-specific module. Thus you'll be able to drop these pointers.

> + void (*dw_xdma_core_done_handle)(struct dw_edma_irq *dw_irq,
> + void (*done_cb)(struct dw_edma_chan *chan),
> + enum dw_edma_dir dir);
> + void (*dw_xdma_core_abort_handle)(struct dw_edma_irq *dw_irq,
> + void (*abort_cb)(struct dw_edma_chan *chan),
> + enum dw_edma_dir dir);

1. Please fix the indentation so the arguments would be vertically
aligned.
2. "_cb" suffix seems redundant. It's obvious that the function
pointer is a callback.

> + void (*dw_xdma_core_start)(struct dw_edma_chunk *chunk, bool first);
> + int (*dw_xdma_core_ch_config)(struct dw_edma_chan *chan);
> + void (*dw_xdma_core_debugfs_on)(struct dw_edma *dw);
> +};

You can completely drop the "dw_xdma_core_" prefix from the field names.
It's obvious that they refer to the IP-core specific operation.

> +
> struct dw_edma {
> char name[32];
>
> @@ -111,6 +128,8 @@ struct dw_edma {
> raw_spinlock_t lock; /* Only for legacy */
>
> struct dw_edma_chip *chip;

> +
> + const struct dw_edma_core_ops *core;
^
+-------------------------------+
Use tab in here?

> };
>
> struct dw_edma_sg {
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index 51a34b43434c..0c1a74245ecc 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -223,7 +223,7 @@ static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> writeq(value, ll)
>
> /* eDMA management callbacks */
> -void dw_edma_v0_core_off(struct dw_edma *dw)
> +static void dw_edma_v0_core_off(struct dw_edma *dw)
> {
> SET_BOTH_32(dw, int_mask,
> EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
> @@ -232,7 +232,7 @@ void dw_edma_v0_core_off(struct dw_edma *dw)
> SET_BOTH_32(dw, engine_en, 0);
> }
>
> -u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> +static u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> {
> u32 num_ch;
>
> @@ -249,7 +249,7 @@ u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> return (u16)num_ch;
> }
>
> -enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> +static enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> {
> struct dw_edma *dw = chan->dw;
> u32 tmp;
> @@ -265,7 +265,7 @@ enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> return DMA_ERROR;
> }
>
> -void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> +static void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> {
> struct dw_edma *dw = chan->dw;
>
> @@ -273,7 +273,7 @@ void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id)));
> }
>
> -void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> +static void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> {
> struct dw_edma *dw = chan->dw;
>
> @@ -281,18 +281,70 @@ void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> 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)
> +static u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
> {
> return FIELD_GET(EDMA_V0_DONE_INT_MASK,
> GET_RW_32(dw, dir, int_status));
> }
>
> -u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> +static u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> {
> return FIELD_GET(EDMA_V0_ABORT_INT_MASK,
> GET_RW_32(dw, dir, int_status));
> }
>
> +static
> +void dw_edma_v0_core_done_handle(struct dw_edma_irq *dw_irq,
> + void (*done_cb)(struct dw_edma_chan *chan),
> + enum dw_edma_dir dir)
> +{
> + struct dw_edma *dw = dw_irq->dw;
> + unsigned long total, pos, val;
> + unsigned long off;
> + u32 mask;
> +
> + if (dir == EDMA_DIR_WRITE) {
> + total = dw->wr_ch_cnt;
> + off = 0;
> + mask = dw_irq->wr_mask;
> + } else {
> + total = dw->rd_ch_cnt;
> + off = dw->wr_ch_cnt;
> + mask = dw_irq->rd_mask;
> + }
> +
> + val = dw_edma_v0_core_status_done_int(dw, dir);
> + val &= mask;
> + for_each_set_bit(pos, &val, total)
<-+
> + done_cb(&dw->chan[pos + off]); |
|
dw_edma_v0_core_clear_done_int()----------------+
?

> +}
> +
> +static
> +void dw_edma_v0_core_abort_handle(struct dw_edma_irq *dw_irq,
> + void (*abort_cb)(struct dw_edma_chan *chan),
> + enum dw_edma_dir dir)
> +{
> + struct dw_edma *dw = dw_irq->dw;
> + unsigned long total, pos, val;
> + unsigned long off;
> + u32 mask;
> +
> + if (dir == EDMA_DIR_WRITE) {
> + total = dw->wr_ch_cnt;
> + off = 0;
> + mask = dw_irq->wr_mask;
> + } else {
> + total = dw->rd_ch_cnt;
> + off = dw->wr_ch_cnt;
> + mask = dw_irq->rd_mask;
> + }
> +
> + val = dw_edma_v0_core_status_abort_int(dw, dir);
> + val &= mask;
> + for_each_set_bit(pos, &val, total)
<-+
> + abort_cb(&dw->chan[pos + off]); |
|
dw_edma_v0_core_clear_abort_int()---------------+
?

> +}
> +
> static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> {
> struct dw_edma_burst *child;
> @@ -338,7 +390,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> SET_LL_64(&llp->llp.reg, chunk->ll_region.paddr);
> }
>
> -void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> +static 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->dw;
> @@ -409,7 +461,7 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> FIELD_PREP(EDMA_V0_DOORBELL_CH_MASK, chan->id));
> }
>
> -int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
> +static int dw_edma_v0_core_ch_config(struct dw_edma_chan *chan)
> {
> struct dw_edma *dw = chan->dw;
> u32 tmp = 0;
> @@ -481,7 +533,25 @@ int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
> }
>
> /* eDMA debugfs callbacks */
> -void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> +static void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> {
> dw_edma_v0_debugfs_on(dw);
> }
> +

> +static const struct dw_edma_core_ops edma_core = {

Please use dw_edma_core name for the structure instance in order to
maintain the driver naming convention.

> + .dw_xdma_core_off = dw_edma_v0_core_off,
> + .dw_xdma_core_ch_count = dw_edma_v0_core_ch_count,
> + .dw_xdma_core_ch_status = dw_edma_v0_core_ch_status,
> + .dw_xdma_core_clear_done_int = dw_edma_v0_core_clear_done_int,
> + .dw_xdma_core_clear_abort_int = dw_edma_v0_core_clear_abort_int,
> + .dw_xdma_core_done_handle = dw_edma_v0_core_done_handle,
> + .dw_xdma_core_abort_handle = dw_edma_v0_core_abort_handle,
> + .dw_xdma_core_start = dw_edma_v0_core_start,
> + .dw_xdma_core_ch_config = dw_edma_v0_core_ch_config,
> + .dw_xdma_core_debugfs_on = dw_edma_v0_core_debugfs_on,
> +};
> +
> +void dw_edma_v0_core_register(struct dw_edma *dw)
> +{
> + dw->core = &edma_core;
> +}
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.h b/drivers/dma/dw-edma/dw-edma-v0-core.h
> index ab96a1f48080..972994ecc96f 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.h
> @@ -11,17 +11,7 @@
>
> #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);
> -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);
> -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 */
> -void dw_edma_v0_core_debugfs_on(struct dw_edma *dw);

> +/* eDMA core operation register*/

"eDMA core register"?

-Serge(y)

> +void dw_edma_v0_core_register(struct dw_edma *dw);
>
> #endif /* _DW_EDMA_V0_CORE_H */
> --
> 2.25.1
>

2023-02-10 18:14:07

by Serge Semin

[permalink] [raw]

Subject: Re: [PATCH 3/3] dmaengine: dw-edma: Add support for native HDMA

On Wed, Sep 21, 2022 at 02:48:52PM +0800, Cai Huoqing wrote:
> From: caihuoqing <[email protected]>
>
> Add support for HDMA NATIVE, as long the IP design has set
> the compatible register map parameter-HDMA_NATIVE,
> which allows compatibility for native HDMA register configuration.
>
> The HDMA Hyper-DMA IP is an enhancement of the eDMA embedded-DMA IP.
> And the native HDMA registers are different from eDMA,
> so this patch add support for HDMA NATIVE mode.
>
> HDMA write and read channels operate independently to maximize
> the performance of the HDMA read and write data transfer over
> the link When you configure the HDMA with multiple read channels,
> then it uses a round robin (RR) arbitration scheme to select
> the next read channel to be serviced.
> The same applies when you have multiple write channels.
>
> The native HDMA driver also supports a maximum of 16 independent
> channels (8 write + 8 read), which can run simultaneously.
> Both SAR (Source Address Register) and DAR (Destination Address Register)
> are alignmented to byte.
>
> Signed-off-by: caihuoqing <[email protected]>
> ---
> drivers/dma/dw-edma/Makefile | 6 +-
> drivers/dma/dw-edma/dw-edma-core.c | 6 +-
> drivers/dma/dw-edma/dw-hdma-v0-core.c | 308 +++++++++++++++++++++++
> drivers/dma/dw-edma/dw-hdma-v0-core.h | 17 ++
> drivers/dma/dw-edma/dw-hdma-v0-debugfs.c | 150 +++++++++++
> drivers/dma/dw-edma/dw-hdma-v0-debugfs.h | 22 ++
> drivers/dma/dw-edma/dw-hdma-v0-regs.h | 98 ++++++++
> include/linux/dma/edma.h | 3 +-
> 8 files changed, 606 insertions(+), 4 deletions(-)
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.c
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.h
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-regs.h
>
> diff --git a/drivers/dma/dw-edma/Makefile b/drivers/dma/dw-edma/Makefile
> index 8d45c0d5689d..958e7f202d11 100644
> --- a/drivers/dma/dw-edma/Makefile
> +++ b/drivers/dma/dw-edma/Makefile
> @@ -1,7 +1,9 @@
> # SPDX-License-Identifier: GPL-2.0
>
> obj-$(CONFIG_DW_EDMA) += dw-edma.o
> -dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o
> +dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o \
> + dw-hdma-v0-debugfs.o
> dw-edma-objs := dw-edma-core.o \
> - dw-edma-v0-core.o $(dw-edma-y)
> + dw-edma-v0-core.o \
> + dw-hdma-v0-core.o $(dw-edma-y)
> obj-$(CONFIG_DW_EDMA_PCIE) += dw-edma-pcie.o
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index a66e4216a5d3..59c71e5cc8f6 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -19,6 +19,7 @@
>
> #include "dw-edma-core.h"
> #include "dw-edma-v0-core.h"
> +#include "dw-hdma-v0-core.h"
> #include "../dmaengine.h"
> #include "../virt-dma.h"
>
> @@ -952,7 +953,10 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>
> dw->chip = chip;
>
> - dw_edma_v0_core_register(dw);
> + if (dw->chip->mf == EDMA_MF_HDMA_NATIVE)
> + dw_hdma_v0_core_register(dw);
> + else
> + dw_edma_v0_core_register(dw);
>
> raw_spin_lock_init(&dw->lock);
>
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> new file mode 100644
> index 000000000000..4ed6881bf3e9
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -0,0 +1,308 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 core
> + */
> +
> +#include <linux/bitfield.h>
> +
> +#include "dw-edma-core.h"

> +#include "dw-edma-v0-core.h"
> +#include "dw-edma-v0-regs.h"

Do you need these headers here?

> +#include "dw-hdma-v0-core.h"
> +#include "dw-hdma-v0-regs.h"
> +#include "dw-hdma-v0-debugfs.h"
> +
> +enum dw_hdma_control {
> + DW_HDMA_V0_CB = BIT(0),
> + DW_HDMA_V0_TCB = BIT(1),
> + DW_HDMA_V0_LLP = BIT(2),
> + DW_HDMA_V0_LIE = BIT(3),
> + DW_HDMA_V0_RIE = BIT(4),
> + DW_HDMA_V0_CCS = BIT(8),
> + DW_HDMA_V0_LLE = BIT(9),
> +};
> +static inline struct dw_hdma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
> +{
> + return dw->chip->reg_base;
> +}
> +
> +static inline struct dw_hdma_v0_ch_regs __iomem *
> +__dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
> +{
> +
> + if (dir == EDMA_DIR_WRITE)
> + return &(__dw_regs(dw)->ch[ch].wr);
> + else
> + return &(__dw_regs(dw)->ch[ch].rd);
> +}
> +
> +#define SET_CH_32(dw, dir, ch, name, value) \
> + writel(value, &(__dw_ch_regs(dw, dir, ch)->name))
> +
> +#define GET_CH_32(dw, dir, ch, name) \
> + readl(&(__dw_ch_regs(dw, dir, ch)->name))
> +
> +#define SET_BOTH_CH_32(dw, ch, name, value) \
> + do { \
> + writel(value, &(__dw_ch_regs(dw, EDMA_DIR_WRITE, ch)->name)); \
> + writel(value, &(__dw_ch_regs(dw, EDMA_DIR_READ, ch)->name)); \
> + } while (0)
> +

> +#define SET_LL_32(ll, value) \
> + writel(value, ll)

This macro is no longer value since the linked-list descriptors are
now located on the CPU memory. Please see the patch
https://lore.kernel.org/linux-pci/[email protected]/
which is already in the @Bjron tree for being merged on the next
m-window.

> +
> +/* HDMA management callbacks */
> +static void dw_hdma_v0_core_off(struct dw_edma *dw)
> +{
> + int id;
> +
> + for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
> + SET_BOTH_CH_32(dw, id, int_setup,
> + HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
> + SET_BOTH_CH_32(dw, id, int_clear,

> + HDMA_V0_STOP_INT_MASK & HDMA_V0_ABORT_INT_MASK);
^
Shouldn't '|' be here instead? -----------------------+

> + SET_BOTH_CH_32(dw, id, ch_en, 0);
> + }
> +}
> +
> +static u16 dw_hdma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> +{
> + u32 num_ch = 0;
> + int id;
> +
> + for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
> + if (GET_CH_32(dw, id, dir, ch_en) & BIT(0))
> + num_ch++;
> + }
> +
> + if (num_ch > HDMA_V0_MAX_NR_CH)
> + num_ch = HDMA_V0_MAX_NR_CH;
> +
> + return (u16)num_ch;
> +}
> +
> +static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> + u32 tmp;
> +
> + tmp = FIELD_GET(HDMA_V0_CH_STATUS_MASK,
> + GET_CH_32(dw, chan->id, chan->dir, ch_stat));
> +
> + if (tmp == 1)
> + return DMA_IN_PROGRESS;
> + else if (tmp == 3)
> + return DMA_COMPLETE;
> + else
> + return DMA_ERROR;
> +}
> +
> +static void dw_hdma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + SET_CH_32(dw, chan->dir, chan->id, int_clear, HDMA_V0_STOP_INT_MASK);
> +}
> +
> +static void dw_hdma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + SET_CH_32(dw, chan->dir, chan->id, int_clear, HDMA_V0_ABORT_INT_MASK);
> +}
> +
> +static u32 dw_hdma_v0_core_status_int(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + return GET_CH_32(dw, chan->dir, chan->id, int_stat);
> +}
> +
> +static u32 dw_hdma_v0_core_check_done_int(u32 val)
> +{

> + return (FIELD_GET(HDMA_V0_STOP_INT_MASK, val) == BIT(0));
> +}
> +
> +static u32 dw_hdma_v0_core_check_abort_int(u32 val)
> +{
> + return (FIELD_GET(HDMA_V0_ABORT_INT_MASK, val) == BIT(0));

Em, what about
return FIELD_GET(HDMA_V0_*_INT_MASK, val);
?

> +}
> +
> +static
> +void dw_hdma_v0_core_done_handle(struct dw_edma_irq *dw_irq,
> + void (*done_cb)(struct dw_edma_chan *chan),
> + enum dw_edma_dir dir)
> +{
> + struct dw_edma *dw = dw_irq->dw;
> + unsigned long total, pos, val;
> + unsigned long off, mask;
> +
> + if (dir == EDMA_DIR_WRITE) {
> + total = dw->wr_ch_cnt;
> + off = 0;
> + mask = dw_irq->wr_mask;
> + } else {
> + total = dw->rd_ch_cnt;
> + off = dw->wr_ch_cnt;
> + mask = dw_irq->rd_mask;
> + }
> +
> + for_each_set_bit(pos, &mask, total) {
> + struct dw_edma_chan *chan = &dw->chan[pos + off];
> +
> + val = dw_hdma_v0_core_status_int(chan);
> + if (dw_hdma_v0_core_check_done_int(val))
> + done_cb(chan);
> + }
> +}
> +
> +static
> +void dw_hdma_v0_core_abort_handle(struct dw_edma_irq *dw_irq,
> + void (*abort_cb)(struct dw_edma_chan *chan),
> + enum dw_edma_dir dir)
> +{
> + struct dw_edma *dw = dw_irq->dw;
> + unsigned long total, pos, val;
> + unsigned long off, mask;
> +
> + if (dir == EDMA_DIR_WRITE) {
> + total = dw->wr_ch_cnt;
> + off = 0;
> + mask = dw_irq->wr_mask;
> + } else {
> + total = dw->rd_ch_cnt;
> + off = dw->wr_ch_cnt;
> + mask = dw_irq->rd_mask;
> + }
> +
> + for_each_set_bit(pos, &mask, total) {
> + struct dw_edma_chan *chan = &dw->chan[pos + off];
> +
> + val = dw_hdma_v0_core_status_int(chan);
> + if (dw_hdma_v0_core_check_abort_int(val))
> + abort_cb(chan);
> + }
> +}
> +
> +static void dw_hdma_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;
> + int j;
> +
> + lli = chunk->ll_region.vaddr;
> +
> + if (chunk->cb)
> + control = DW_HDMA_V0_CB;
> +
> + j = chunk->bursts_alloc;
> + list_for_each_entry(child, &chunk->burst->list, list) {
> + j--;
> + if (!j) {
> + control |= DW_HDMA_V0_LIE;
> + if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
> + control |= DW_HDMA_V0_RIE;
> + }

> + /* Channel control */
> + SET_LL_32(&lli[i].control, control);
> + /* Transfer size */
> + SET_LL_32(&lli[i].transfer_size, child->sz);
> + /* SAR */
> + SET_LL_32(&lli[i].sar.lsb, lower_32_bits(child->sar));
> + SET_LL_32(&lli[i].sar.msb, upper_32_bits(child->sar));
> + /* DAR */
> + SET_LL_32(&lli[i].dar.lsb, lower_32_bits(child->dar));
> + SET_LL_32(&lli[i].dar.msb, upper_32_bits(child->dar));
> + i++;
> + }
> +
> + llp = (void __iomem *)&lli[i];
> + control = DW_HDMA_V0_LLP | DW_HDMA_V0_TCB;
> + if (!chunk->cb)
> + control |= DW_HDMA_V0_CB;
> +
> + /* Channel control */
> + SET_LL_32(&llp->control, control);
> + /* Linked list */
> + SET_LL_32(&llp->llp.lsb, lower_32_bits(chunk->ll_region.paddr));
> + SET_LL_32(&llp->llp.msb, upper_32_bits(chunk->ll_region.paddr));

Please see the way the LL access is now implemented for eDMA:
https://lore.kernel.org/linux-pci/[email protected]/
The eDMA v0 core now supports the LL descriptors placed on the CPU memory.
This driver should be properly fixed too.

> +}
> +
> +static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> +{
> + struct dw_edma_chan *chan = chunk->chan;
> + struct dw_edma *dw = chan->dw;
> + u32 tmp;
> +
> + dw_hdma_v0_core_write_chunk(chunk);
> +
> + if (first) {
> + /* Enable engine */
> + SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
> + /* Interrupt enable&unmask - done, abort */
> + tmp = GET_CH_32(dw, chan->dir, chan->id, int_setup);

> + tmp |= HDMA_V0_STOP_INT_MASK;
> + tmp |= HDMA_V0_ABORT_INT_MASK;
> + tmp |= HDMA_V0_LOCAL_ABORT_INT_EN;
> + tmp |= HDMA_V0_LOCAL_STOP_INT_EN;

Using multiple bitwise-OR ops seems a bit more robust then multiple
'|=' ops since the constants will be combined on compile-time.

> + SET_CH_32(dw, chan->dir, chan->id, int_setup, tmp);
> + /* Channel control */
> + SET_CH_32(dw, chan->dir, chan->id, control1, HDMA_V0_LINKLIST_EN);
> + /* Linked list */
> + /* llp is not aligned on 64bit -> keep 32bit accesses */
> + SET_CH_32(dw, chan->dir, chan->id, llp.lsb,
> + lower_32_bits(chunk->ll_region.paddr));
> + SET_CH_32(dw, chan->dir, chan->id, llp.msb,
> + upper_32_bits(chunk->ll_region.paddr));
> + }
> + /* set consumer cycle */
> + SET_CH_32(dw, chan->dir, chan->id, cycle_sync,
> + HDMA_V0_CONSUMER_CYCLE_STAT | HDMA_V0_CONSUMER_CYCLE_BIT);
> + /* Doorbell */
> + SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
> +}
> +
> +static int dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + /* MSI done addr - low, high */
> + SET_CH_32(dw, chan->dir, chan->id, msi_stop.lsb, chan->msi.address_lo);
> + SET_CH_32(dw, chan->dir, chan->id, msi_stop.msb, chan->msi.address_hi);
> + /* MSI abort addr - low, high */
> + SET_CH_32(dw, chan->dir, chan->id, msi_abort.lsb, chan->msi.address_lo);

> + SET_CH_32(dw, chan->dir, chan->id, msi_abort.msb, chan->msi.address_hi);
> + SET_CH_32(dw, chan->dir, chan->id, msi_abort.msb, chan->msi.address_hi);

Two identical lines. Typo?

> + /* config MSI data */
> + SET_CH_32(dw, chan->dir, chan->id, msi_msgdata, chan->msi.data);
> +
> + return 0;
> +}
> +
> +/* eDMA debugfs callbacks */
> +static void dw_hdma_v0_core_debugfs_on(struct dw_edma *dw)
> +{
> + dw_hdma_v0_debugfs_on(dw);
> +}
> +
> +static const struct dw_edma_core_ops hdma_core = {
> + .dw_xdma_core_off = dw_hdma_v0_core_off,
> + .dw_xdma_core_ch_count = dw_hdma_v0_core_ch_count,
> + .dw_xdma_core_ch_status = dw_hdma_v0_core_ch_status,
> + .dw_xdma_core_clear_done_int = dw_hdma_v0_core_clear_done_int,
> + .dw_xdma_core_clear_abort_int = dw_hdma_v0_core_clear_abort_int,
> + .dw_xdma_core_done_handle = dw_hdma_v0_core_done_handle,
> + .dw_xdma_core_abort_handle = dw_hdma_v0_core_abort_handle,
> + .dw_xdma_core_start = dw_hdma_v0_core_start,
> + .dw_xdma_core_ch_config = dw_hdma_v0_core_ch_config,
> + .dw_xdma_core_debugfs_on = dw_hdma_v0_core_debugfs_on,
> +};
> +
> +void dw_hdma_v0_core_register(struct dw_edma *dw)
> +{
> + dw->core = &hdma_core;
> +}
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.h b/drivers/dma/dw-edma/dw-hdma-v0-core.h
> new file mode 100644
> index 000000000000..dc9b93e469ae
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2022 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 core
> + *
> + * Author: Cai Huoqing <[email protected]>
> + */
> +
> +#ifndef _DW_HDMA_V0_CORE_H
> +#define _DW_HDMA_V0_CORE_H
> +
> +#include <linux/dma/edma.h>
> +
> +/* HDMA core operation register*/
> +void dw_hdma_v0_core_register(struct dw_edma *dw);
> +
> +#endif /* _DW_HDMA_V0_CORE_H */
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
> new file mode 100644
> index 000000000000..1f973b21eef9

> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c

Please see the way the module now looks in the @Bjorn tree and
refactor your DebugFS part being similar. Currently due to the way
your driver is designed it's impossible to register more than one HDMA
device in the system.

> @@ -0,0 +1,150 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 debugfs
> + *
> + * Author: Cai Huoqing <[email protected]>
> + */
> +
> +#include <linux/debugfs.h>
> +#include <linux/bitfield.h>
> +
> +#include "dw-hdma-v0-debugfs.h"
> +#include "dw-hdma-v0-regs.h"
> +#include "dw-edma-core.h"
> +
> +#define REGS_ADDR(name) \
> + ((void __force *)&regs->name)
> +
> +#define REGISTER(name) \
> + { #name, REGS_ADDR(name) }
> +
> +#define WRITE_STR "write"
> +#define READ_STR "read"
> +#define CHANNEL_STR "channel"
> +#define REGISTERS_STR "registers"
> +
> +static struct dw_edma *dw;
> +static struct dw_hdma_v0_regs __iomem *regs;

Empty line please.

> +struct debugfs_entries {
> + const char *name;
> + dma_addr_t *reg;
> +};
> +
> +static int dw_hdma_debugfs_u32_get(void *data, u64 *val)
> +{
> + void __iomem *reg = (void __force __iomem *)data;
> + *val = readl(reg);
> +
> + return 0;
> +}
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_x32, dw_hdma_debugfs_u32_get, NULL, "0x%08llx\n");
> +
> +static void dw_hdma_debugfs_create_x32(const struct debugfs_entries entries[],
> + int nr_entries, struct dentry *dir)
> +{
> + int i;
> +
> + for (i = 0; i < nr_entries; i++) {
> + if (!debugfs_create_file_unsafe(entries[i].name, 0444, dir,
> + entries[i].reg, &fops_x32))
> + break;
> + }
> +}
> +
> +static void dw_hdma_debugfs_regs_ch(struct dw_hdma_v0_ch_regs __iomem *regs,
> + struct dentry *dir)
> +{
> + int nr_entries;
> + const struct debugfs_entries debugfs_regs[] = {
> + REGISTER(ch_en),
> + REGISTER(doorbell),
> + REGISTER(llp.lsb),
> + REGISTER(llp.msb),
> + REGISTER(cycle_sync),
> + REGISTER(transfer_size),
> + REGISTER(sar.lsb),
> + REGISTER(sar.msb),
> + REGISTER(dar.lsb),
> + REGISTER(dar.msb),
> + REGISTER(control1),
> + REGISTER(ch_stat),
> + REGISTER(int_stat),
> + REGISTER(int_setup),
> + REGISTER(int_clear),
> + REGISTER(msi_stop.lsb),
> + REGISTER(msi_stop.msb),
> + REGISTER(msi_abort.lsb),
> + REGISTER(msi_abort.msb),
> + REGISTER(msi_msgdata),
> + };
> +
> + nr_entries = ARRAY_SIZE(debugfs_regs);
> + dw_hdma_debugfs_create_x32(debugfs_regs, nr_entries, dir);
> +}
> +
> +static void dw_hdma_debugfs_regs_wr(struct dentry *dir)
> +{
> + struct dentry *regs_dir, *ch_dir;
> + int i;
> + char name[16];
> +
> + regs_dir = debugfs_create_dir(WRITE_STR, dir);
> + if (!regs_dir)
> + return;
> +
> + for (i = 0; i < dw->wr_ch_cnt; i++) {
> + snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i);
> +
> + ch_dir = debugfs_create_dir(name, regs_dir);
> + if (!ch_dir)
> + return;
> +
> + dw_hdma_debugfs_regs_ch(&regs->ch[i].wr, ch_dir);
> + }
> +}
> +
> +static void dw_hdma_debugfs_regs_rd(struct dentry *dir)
> +{
> + struct dentry *regs_dir, *ch_dir;
> + int i;
> + char name[16];
> +
> + regs_dir = debugfs_create_dir(READ_STR, dir);
> + if (!regs_dir)
> + return;
> +
> + for (i = 0; i < dw->rd_ch_cnt; i++) {
> + snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i);
> +
> + ch_dir = debugfs_create_dir(name, regs_dir);
> + if (!ch_dir)
> + return;
> +
> + dw_hdma_debugfs_regs_ch(&regs->ch[i].rd, ch_dir);
> + }
> +}
> +
> +static void dw_hdma_debugfs_regs(void)
> +{
> + struct dentry *regs_dir;
> +
> + regs_dir = debugfs_create_dir(REGISTERS_STR, dw->dma.dbg_dev_root);
> + if (!regs_dir)
> + return;
> +
> + dw_hdma_debugfs_regs_wr(regs_dir);
> + dw_hdma_debugfs_regs_rd(regs_dir);
> +}
> +
> +void dw_hdma_v0_debugfs_on(struct dw_edma *dw)
> +{
> + if (!debugfs_initialized())
> + return;
> +
> + debugfs_create_u32("mf", 0444, dw->dma.dbg_dev_root, &dw->chip->mf);
> + debugfs_create_u16("wr_ch_cnt", 0444, dw->dma.dbg_dev_root, &dw->wr_ch_cnt);
> + debugfs_create_u16("rd_ch_cnt", 0444, dw->dma.dbg_dev_root, &dw->rd_ch_cnt);
> +
> + dw_hdma_debugfs_regs();
> +}
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
> new file mode 100644
> index 000000000000..4b70911aedac
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2022 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 debugfs
> + *
> + * Author: Cai Huoqing <[email protected]>
> + */
> +
> +#ifndef _DW_HDMA_V0_DEBUG_FS_H
> +#define _DW_HDMA_V0_DEBUG_FS_H
> +
> +#include <linux/dma/edma.h>
> +
> +#ifdef CONFIG_DEBUG_FS
> +void dw_hdma_v0_debugfs_on(struct dw_edma *dw);
> +#else
> +static inline void dw_hdma_v0_debugfs_on(struct dw_edma *dw)
> +{
> +}
> +#endif /* CONFIG_DEBUG_FS */
> +
> +#endif /* _DW_HDMA_V0_DEBUG_FS_H */
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-regs.h b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
> new file mode 100644
> index 000000000000..677bf44400b3
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
> @@ -0,0 +1,98 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2022 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 reg
> + *
> + * Author: Cai Huoqing <[email protected]>
> + */
> +
> +#ifndef _DW_HDMA_V0_REGS_H
> +#define _DW_HDMA_V0_REGS_H
> +
> +#include <linux/dmaengine.h>
> +

> +#define HDMA_V0_MAX_NR_CH 8

Remove one redundant tab

> +#define HDMA_V0_LOCAL_ABORT_INT_EN BIT(6)
> +#define HDMA_V0_REMOTE_ABORT_INT_EN BIT(5)
> +#define HDMA_V0_LOCAL_STOP_INT_EN BIT(4)
> +#define HDMA_V0_REMOTEL_STOP_INT_EN BIT(3)
> +#define HDMA_V0_ABORT_INT_MASK BIT(2)
> +#define HDMA_V0_STOP_INT_MASK BIT(0)

> +#define HDMA_V0_LINKLIST_EN BIT(0)

ditto

> +#define HDMA_V0_CONSUMER_CYCLE_STAT BIT(1)
> +#define HDMA_V0_CONSUMER_CYCLE_BIT BIT(0)
> +#define HDMA_V0_DOORBELL_START BIT(0)
> +#define HDMA_V0_CH_STATUS_MASK GENMASK(1, 0)
> +

> +struct dw_hdma_v0_ch_regs {
> + u32 ch_en; /* 0x0000 */

Please make sure all the inline comments are aligned along a single
vertical line. The comments jump out of line here and there.

> + u32 doorbell; /* 0x0004 */
> + u32 prefetch; /* 0x0008 */
> + u32 handshake; /* 0x000c */
> + union {
> + u64 reg; /* 0x0010..0x0014 */
> + struct {
> + u32 lsb; /* 0x0010 */
> + u32 msb; /* 0x0014 */
> + };
> + } llp;
> + u32 cycle_sync; /* 0x0018 */
> + u32 transfer_size; /* 0x001c */
> + union {
> + u64 reg; /* 0x0020..0x0024 */
> + struct {
> + u32 lsb; /* 0x0020 */
> + u32 msb; /* 0x0024 */
> + };
> + } sar;
> + union {
> + u64 reg; /* 0x0028..0x002c */
> + struct {
> + u32 lsb; /* 0x0028 */
> + u32 msb; /* 0x002c */
> + };
> + } dar;
> +
> + u32 watermark_en; /* 0x0030 */

> + u32 control1; /* 0x0034 */
> + u32 func_num; /* 0x0038 */
> + u32 qos; /* 0x003c */
> + u32 reserved; /* 0x0040..0x007c */

Use spaces instead of tabs in the field declaration in the same way
as it's done for the rest of the fields.

> + u32 ch_stat; /* 0x0080 */
> + u32 int_stat; /* 0x0084 */
> + u32 int_setup; /* 0x0088 */
> + u32 int_clear; /* 0x008c */
> + union {
> + u64 reg; /* 0x0090..0x0094 */
> + struct {
> + u32 lsb; /* 0x0090 */
> + u32 msb; /* 0x0094 */
> + };
> + } msi_stop;
> + union {
> + u64 reg; /* 0x0098..0x009c */
> + struct {
> + u32 lsb; /* 0x0098 */
> + u32 msb; /* 0x009c */
> + };
> + } msi_watermark;
> + union {
> + u64 reg; /* 0x00a0..0x00a4 */
> + struct {
> + u32 lsb; /* 0x00a0 */
> + u32 msb; /* 0x00a4 */
> + };
> + } msi_abort;

> + u32 msi_msgdata; /* 0x00a8 */

ditto

> +} __packed;
> +
> +struct dw_hdma_v0_ch {
> + struct dw_hdma_v0_ch_regs wr; /* 0x0000 */
> + struct dw_hdma_v0_ch_regs rd; /* 0x0100 */
> +} __packed;
> +
> +struct dw_hdma_v0_regs {
> + struct dw_hdma_v0_ch ch[HDMA_V0_MAX_NR_CH]; /* 0x0000..0x0fa8 */
> +} __packed;
> +
> +#endif /* _DW_HDMA_V0_REGS_H */
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index 642dd325259b..b879d107f08b 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -45,7 +45,8 @@ struct dw_edma_plat_ops {
> enum dw_edma_map_format {
> EDMA_MF_EDMA_LEGACY = 0x0,
> EDMA_MF_EDMA_UNROLL = 0x1,
> - EDMA_MF_HDMA_COMPAT = 0x5
> + EDMA_MF_HDMA_COMPAT = 0x5,

> + EDMA_MF_HDMA_NATIVE = 0x7

Please add terminating comma here.

-Serge(y)

> };
>
> /**
> --
> 2.25.1
>