* [U-Boot] [PATCH 0/5] Add DMA driver for DMA330 controller
@ 2018-05-31 8:08 tien.fong.chee at intel.com
2018-05-31 8:08 ` [U-Boot] [PATCH 1/5] drivers: dma: Enable DMA-330 driver support tien.fong.chee at intel.com
` (5 more replies)
0 siblings, 6 replies; 27+ messages in thread
From: tien.fong.chee at intel.com @ 2018-05-31 8:08 UTC (permalink / raw)
To: u-boot
From: Tien Fong Chee <tien.fong.chee@intel.com>
This patchset contains dm driver for DMA330 controller.
This series is working on top of u-boot-socfpga.git -
http://git.denx.de/u-boot.git .
Tien Fong Chee (5):
drivers: dma: Enable DMA-330 driver support
drivers: dma: Add function to zeroes a range of destination such as
memory
drivers: dma: Factor out dma_get_device from DMA class function
include: dma: Update the function description for dma_memcpy
arm: dts: socfpga: stratix10: update pdma
arch/arm/dts/socfpga_stratix10.dtsi | 20 +
drivers/dma/Kconfig | 9 +-
drivers/dma/Makefile | 1 +
drivers/dma/dma-uclass.c | 23 +-
drivers/dma/dma330.c | 1514 +++++++++++++++++++++++++++++++++++
drivers/mtd/spi/spi_flash.c | 9 +-
include/dma.h | 19 +-
include/dma330.h | 136 ++++
8 files changed, 1719 insertions(+), 12 deletions(-)
create mode 100644 drivers/dma/dma330.c
create mode 100644 include/dma330.h
--
2.2.0
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 1/5] drivers: dma: Enable DMA-330 driver support
2018-05-31 8:08 [U-Boot] [PATCH 0/5] Add DMA driver for DMA330 controller tien.fong.chee at intel.com
@ 2018-05-31 8:08 ` tien.fong.chee at intel.com
2018-06-01 14:24 ` Simon Glass
2018-05-31 8:08 ` [U-Boot] [PATCH 2/5] drivers: dma: Add function to zeroes a range of destination such as memory tien.fong.chee at intel.com
` (4 subsequent siblings)
5 siblings, 1 reply; 27+ messages in thread
From: tien.fong.chee at intel.com @ 2018-05-31 8:08 UTC (permalink / raw)
To: u-boot
From: Tien Fong Chee <tien.fong.chee@intel.com>
Enable DMAC driver support for DMA-330 controller.
The driver is also compatible to PL330 product.
Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
---
drivers/dma/Kconfig | 9 +-
drivers/dma/Makefile | 1 +
drivers/dma/dma330.c | 1514 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/dma330.h | 136 +++++
4 files changed, 1659 insertions(+), 1 deletion(-)
create mode 100644 drivers/dma/dma330.c
create mode 100644 include/dma330.h
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 4ee6afa..6e77e07 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -2,7 +2,7 @@ menu "DMA Support"
config DMA
bool "Enable Driver Model for DMA drivers"
- depends on DM
+ depends on DM || SPL_DM
help
Enable driver model for DMA. DMA engines can do
asynchronous data transfers without involving the host
@@ -34,4 +34,11 @@ config APBH_DMA_BURST8
endif
+config DMA330_DMA
+ bool "PL330/DMA-330 DMA Controller(DMAC) driver"
+ depends on DMA
+ help
+ Enable the DMA controller driver for both PL330 and
+ DMA-330 products.
+
endmenu # menu "DMA Support"
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index 4eaef8a..bfad0dd 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_FSL_DMA) += fsl_dma.o
obj-$(CONFIG_TI_KSNAV) += keystone_nav.o keystone_nav_cfg.o
obj-$(CONFIG_TI_EDMA3) += ti-edma3.o
obj-$(CONFIG_DMA_LPC32XX) += lpc32xx_dma.o
+obj-$(CONFIG_DMA330_DMA) += dma330.o
diff --git a/drivers/dma/dma330.c b/drivers/dma/dma330.c
new file mode 100644
index 0000000..66575d8
--- /dev/null
+++ b/drivers/dma/dma330.c
@@ -0,0 +1,1514 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Intel Corporation <www.intel.com>
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <dma330.h>
+#include <dm.h>
+#include <fdtdec.h>
+#include <wait_bit.h>
+#include <linux/unaligned/access_ok.h>
+
+/* Register and Bit field Definitions */
+
+/* DMA Status */
+#define DS 0x0
+#define DS_ST_STOP 0x0
+#define DS_ST_EXEC 0x1
+#define DS_ST_CMISS 0x2
+#define DS_ST_UPDTPC 0x3
+#define DS_ST_WFE 0x4
+#define DS_ST_ATBRR 0x5
+#define DS_ST_QBUSY 0x6
+#define DS_ST_WFP 0x7
+#define DS_ST_KILL 0x8
+#define DS_ST_CMPLT 0x9
+#define DS_ST_FLTCMP 0xe
+#define DS_ST_FAULT 0xf
+
+/* DMA Program Count register */
+#define DPC 0x4
+/* Interrupt Enable register */
+#define INTEN 0x20
+/* event-Interrupt Raw Status register */
+#define ES 0x24
+/* Interrupt Status register */
+#define INTSTATUS 0x28
+/* Interrupt Clear register */
+#define INTCLR 0x2c
+/* Fault Status DMA Manager register */
+#define FSM 0x30
+/* Fault Status DMA Channel register */
+#define FSC 0x34
+/* Fault Type DMA Manager register */
+#define FTM 0x38
+
+/* Fault Type DMA Channel register */
+#define _FTC 0x40
+#define FTC(n) (_FTC + (n) * 0x4)
+
+/* Channel Status register */
+#define _CS 0x100
+#define CS(n) (_CS + (n) * 0x8)
+#define CS_CNS BIT(21)
+
+/* Channel Program Counter register */
+#define _CPC 0x104
+#define CPC(n) (_CPC + (n) * 0x8)
+
+/* Source Address register */
+#define _SA 0x400
+#define SA(n) (_SA + (n) * 0x20)
+
+/* Destination Address register */
+#define _DA 0x404
+#define DA(n) (_DA + (n) * 0x20)
+
+/* Channel Control register */
+#define _CC 0x408
+#define CC(n) (_CC + (n) * 0x20)
+
+/* Channel Control register (CCR) Setting */
+#define CC_SRCINC BIT(0)
+#define CC_DSTINC BIT(14)
+#define CC_SRCPRI BIT(8)
+#define CC_DSTPRI BIT(22)
+#define CC_SRCNS BIT(9)
+#define CC_DSTNS BIT(23)
+#define CC_SRCIA BIT(10)
+#define CC_DSTIA BIT(24)
+#define CC_SRCBRSTLEN_SHFT 4
+#define CC_DSTBRSTLEN_SHFT 18
+#define CC_SRCBRSTSIZE_SHFT 1
+#define CC_DSTBRSTSIZE_SHFT 15
+#define CC_SRCCCTRL_SHFT 11
+#define CC_SRCCCTRL_MASK 0x7
+#define CC_DSTCCTRL_SHFT 25
+#define CC_DRCCCTRL_MASK 0x7
+#define CC_SWAP_SHFT 28
+
+/* Loop Counter 0 register */
+#define _LC0 0x40c
+#define LC0(n) (_LC0 + (n) * 0x20)
+
+/* Loop Counter 1 register */
+#define _LC1 0x410
+#define LC1(n) (_LC1 + (n) * 0x20)
+
+/* Debug Status register */
+#define DBGSTATUS 0xd00
+#define DBG_BUSY BIT(0)
+
+/* Debug Command register */
+#define DBGCMD 0xd04
+/* Debug Instruction 0 register */
+#define DBGINST0 0xd08
+/* Debug Instruction 1 register */
+#define DBGINST1 0xd0c
+
+/* Configuration register */
+#define CR0 0xe00
+#define CR1 0xe04
+#define CR2 0xe08
+#define CR3 0xe0c
+#define CR4 0xe10
+#define CRD 0xe14
+
+/* Peripheral Identification register */
+#define PERIPH_ID 0xfe0
+/* Component Identification register */
+#define PCELL_ID 0xff0
+
+/* Configuration register value */
+#define CR0_PERIPH_REQ_SET BIT(0)
+#define CR0_BOOT_EN_SET BIT(1)
+#define CR0_BOOT_MAN_NS BIT(2)
+#define CR0_NUM_CHANS_SHIFT 4
+#define CR0_NUM_CHANS_MASK 0x7
+#define CR0_NUM_PERIPH_SHIFT 12
+#define CR0_NUM_PERIPH_MASK 0x1f
+#define CR0_NUM_EVENTS_SHIFT 17
+#define CR0_NUM_EVENTS_MASK 0x1f
+
+/* Configuration register value */
+#define CR1_ICACHE_LEN_SHIFT 0
+#define CR1_ICACHE_LEN_MASK 0x7
+#define CR1_NUM_ICACHELINES_SHIFT 4
+#define CR1_NUM_ICACHELINES_MASK 0xf
+
+/* Configuration register value */
+#define CRD_DATA_WIDTH_SHIFT 0
+#define CRD_DATA_WIDTH_MASK 0x7
+#define CRD_WR_CAP_SHIFT 4
+#define CRD_WR_CAP_MASK 0x7
+#define CRD_WR_Q_DEP_SHIFT 8
+#define CRD_WR_Q_DEP_MASK 0xf
+#define CRD_RD_CAP_SHIFT 12
+#define CRD_RD_CAP_MASK 0x7
+#define CRD_RD_Q_DEP_SHIFT 16
+#define CRD_RD_Q_DEP_MASK 0xf
+#define CRD_DATA_BUFF_SHIFT 20
+#define CRD_DATA_BUFF_MASK 0x3ff
+
+/* Microcode opcode value */
+#define CMD_DMAADDH 0x54
+#define CMD_DMAEND 0x00
+#define CMD_DMAFLUSHP 0x35
+#define CMD_DMAGO 0xa0
+#define CMD_DMALD 0x04
+#define CMD_DMALDP 0x25
+#define CMD_DMALP 0x20
+#define CMD_DMALPEND 0x28
+#define CMD_DMAKILL 0x01
+#define CMD_DMAMOV 0xbc
+#define CMD_DMANOP 0x18
+#define CMD_DMARMB 0x12
+#define CMD_DMASEV 0x34
+#define CMD_DMAST 0x08
+#define CMD_DMASTP 0x29
+#define CMD_DMASTZ 0x0c
+#define CMD_DMAWFE 0x36
+#define CMD_DMAWFP 0x30
+#define CMD_DMAWMB 0x13
+
+/* the size of opcode plus opcode required settings */
+#define SZ_DMAADDH 3
+#define SZ_DMAEND 1
+#define SZ_DMAFLUSHP 2
+#define SZ_DMALD 1
+#define SZ_DMALDP 2
+#define SZ_DMALP 2
+#define SZ_DMALPEND 2
+#define SZ_DMAKILL 1
+#define SZ_DMAMOV 6
+#define SZ_DMANOP 1
+#define SZ_DMARMB 1
+#define SZ_DMASEV 2
+#define SZ_DMAST 1
+#define SZ_DMASTP 2
+#define SZ_DMASTZ 1
+#define SZ_DMAWFE 2
+#define SZ_DMAWFP 2
+#define SZ_DMAWMB 1
+#define SZ_DMAGO 6
+
+/* Use this _only_ to wait on transient states */
+#define UNTIL(t, s) do { \
+ WATCHDOG_RESET(); \
+ } while (!(dma330_getstate(t) & (s)))
+
+/* debug message printout */
+#ifdef DEBUG
+#define DMA330_DBGCMD_DUMP(off, x...) do { \
+ printf("%x bytes:", off); \
+ printf(x); \
+ WATCHDOG_RESET(); \
+ } while (0)
+#else
+#define DMA330_DBGCMD_DUMP(off, x...) do {} while (0)
+#endif
+
+/* Enum declaration */
+enum dmamov_dst {
+ SAR = 0,
+ CCR,
+ DAR,
+};
+
+enum dma330_dst {
+ SRC = 0,
+ DST,
+};
+
+enum dma330_cond {
+ SINGLE,
+ BURST,
+ ALWAYS,
+};
+
+/* Structure will be used by _emit_lpend function */
+struct _arg_lpend {
+ enum dma330_cond cond;
+ int forever;
+ u32 loop;
+ u8 bjump;
+};
+
+/* Structure will be used by _emit_go function */
+struct _arg_go {
+ u8 chan;
+ u32 addr;
+ u32 ns;
+};
+
+/*
+ * _emit_end - Add opcode DMAEND into microcode (end).
+ *
+ * @buf: The buffer which stored the microcode program.
+ *
+ * Return: Size of opcode.
+ */
+static inline u32 _emit_end(u8 buf[])
+{
+ buf[0] = CMD_DMAEND;
+ DMA330_DBGCMD_DUMP(SZ_DMAEND, "\tDMAEND\n");
+ return SZ_DMAEND;
+}
+
+/*
+ * _emit_flushp - Add opcode DMAFLUSHP into microcode (flush peripheral).
+ *
+ * @buf -> The buffer which stored the microcode program.
+ * @peri -> Peripheral ID as listed in DMA NPP.
+ *
+ * Return: Size of opcode.
+ */
+static inline u32 _emit_flushp(u8 buf[], u8 peri)
+{
+ u8 *buffer = buf;
+
+ buffer[0] = CMD_DMAFLUSHP;
+ peri &= 0x1f;
+ peri <<= 3;
+ buffer[1] = peri;
+ DMA330_DBGCMD_DUMP(SZ_DMAFLUSHP, "\tDMAFLUSHP %u\n", peri >> 3);
+ return SZ_DMAFLUSHP;
+}
+
+/**
+ * _emit_ld - Add opcode DMALD into microcode (load).
+ *
+ * @buf: The buffer which stored the microcode program.
+ * @cond: Execution criteria such as single, burst or always.
+ *
+ * Return: Size of opcode.
+ */
+static inline u32 _emit_ld(u8 buf[], enum dma330_cond cond)
+{
+ buf[0] = CMD_DMALD;
+ if (cond == SINGLE)
+ buf[0] |= (0 << 1) | (1 << 0);
+ else if (cond == BURST)
+ buf[0] |= (1 << 1) | (1 << 0);
+ DMA330_DBGCMD_DUMP(SZ_DMALD, "\tDMALD%c\n",
+ cond == SINGLE ? 'S' : (cond == BURST ? 'B' : 'A'));
+ return SZ_DMALD;
+}
+
+/**
+ * _emit_lp - Add opcode DMALP into microcode (loop).
+ *
+ * @buf: The buffer which stored the microcode program.
+ * @loop: Selection of using counter 0 or 1 (valid value 0 or 1).
+ * @cnt: number of iteration (valid value 1-256).
+ *
+ * Return: Size of opcode.
+ */
+static inline u32 _emit_lp(u8 buf[], u32 loop, u32 cnt)
+{
+ u8 *buffer = buf;
+
+ buffer[0] = CMD_DMALP;
+ if (loop)
+ buffer[0] |= (1 << 1);
+ /* DMAC increments by 1 internally */
+ cnt--;
+ buffer[1] = cnt;
+ DMA330_DBGCMD_DUMP(SZ_DMALP, "\tDMALP_%c %u\n", loop ? '1' : '0', cnt);
+ return SZ_DMALP;
+}
+
+/**
+ * _emit_lpend - Add opcode DMALPEND into microcode (loop end).
+ *
+ * @buf: The buffer which stored the microcode program.
+ * @arg: Structure _arg_lpend which contain all needed info.
+ * arg->cond -> Execution criteria such as single, burst or always
+ * arg->forever -> Forever loop? used if DMALPFE started the loop
+ * arg->bjump -> Backwards jump (relative location of
+ * 1st instruction in the loop.
+ *
+ * Return: Size of opcode.
+ */
+static inline u32 _emit_lpend(u8 buf[], const struct _arg_lpend *arg)
+{
+ u8 *buffer = buf;
+ enum dma330_cond cond = arg->cond;
+ int forever = arg->forever;
+ u32 loop = arg->loop;
+ u8 bjump = arg->bjump;
+
+ buffer[0] = CMD_DMALPEND;
+ if (loop)
+ buffer[0] |= (1 << 2);
+ if (!forever)
+ buffer[0] |= (1 << 4);
+ if (cond == SINGLE)
+ buffer[0] |= (0 << 1) | (1 << 0);
+ else if (cond == BURST)
+ buffer[0] |= (1 << 1) | (1 << 0);
+
+ buffer[1] = bjump;
+ DMA330_DBGCMD_DUMP(SZ_DMALPEND, "\tDMALP%s%c_%c bjmpto_%x\n",
+ forever ? "FE" : "END",
+ cond == SINGLE ? 'S' : (cond == BURST ? 'B' : 'A'),
+ loop ? '1' : '0',
+ bjump);
+ return SZ_DMALPEND;
+}
+
+/**
+ * _emit_kill - Add opcode DMAKILL into microcode (kill).
+ *
+ * @buf: The buffer which stored the microcode program.
+ *
+ * Return: Size of opcode.
+ */
+static inline u32 _emit_kill(u8 buf[])
+{
+ buf[0] = CMD_DMAKILL;
+ return SZ_DMAKILL;
+}
+
+/**
+ * _emit_mov - Add opcode DMAMOV into microcode (move).
+ *
+ * @buf: The buffer which stored the microcode program.
+ * @dst: Destination register (valid value SAR[0b000], DAR[0b010],
+ * or CCR[0b001]).
+ * @val: 32bit value that to be written into destination register.
+ *
+ * Return: Size of opcode.
+ */
+static inline u32 _emit_mov(u8 buf[], enum dmamov_dst dst, u32 val)
+{
+ u8 *buffer = buf;
+
+ buffer[0] = CMD_DMAMOV;
+ buffer[1] = dst;
+ buffer[2] = val & 0xFF;
+ buffer[3] = (val >> 8) & 0xFF;
+ buffer[4] = (val >> 16) & 0xFF;
+ buffer[5] = (val >> 24) & 0xFF;
+ DMA330_DBGCMD_DUMP(SZ_DMAMOV, "\tDMAMOV %s 0x%x\n",
+ dst == SAR ? "SAR" : (dst == DAR ? "DAR" : "CCR"),
+ val);
+ return SZ_DMAMOV;
+}
+
+/**
+ * _emit_nop - Add opcode DMANOP into microcode (no operation).
+ *
+ * @buf: The buffer which stored the microcode program.
+ *
+ * Return: Size of opcode.
+ */
+static inline u32 _emit_nop(u8 buf[])
+{
+ buf[0] = CMD_DMANOP;
+ DMA330_DBGCMD_DUMP(SZ_DMANOP, "\tDMANOP\n");
+ return SZ_DMANOP;
+}
+
+/**
+ * _emit_rmb - Add opcode DMARMB into microcode (read memory barrier).
+ *
+ * @buf: The buffer which stored the microcode program.
+ *
+ * Return: Size of opcode.
+ */
+static inline u32 _emit_rmb(u8 buf[])
+{
+ buf[0] = CMD_DMARMB;
+ DMA330_DBGCMD_DUMP(SZ_DMARMB, "\tDMARMB\n");
+ return SZ_DMARMB;
+}
+
+/**
+ * _emit_sev - Add opcode DMASEV into microcode (send event).
+ *
+ * @buf: The buffer which stored the microcode program.
+ * @ev: The event number (valid 0 - 31).
+ *
+ * Return: Size of opcode.
+ */
+static inline u32 _emit_sev(u8 buf[], u8 ev)
+{
+ u8 *buffer = buf;
+
+ buffer[0] = CMD_DMASEV;
+ ev &= 0x1f;
+ ev <<= 3;
+ buffer[1] = ev;
+ DMA330_DBGCMD_DUMP(SZ_DMASEV, "\tDMASEV %u\n", ev >> 3);
+ return SZ_DMASEV;
+}
+
+/**
+ * _emit_st - Add opcode DMAST into microcode (store).
+ *
+ * @buf: The buffer which stored the microcode program.
+ * @cond: Execution criteria such as single, burst or always.
+ *
+ * Return: Size of opcode.
+ */
+static inline u32 _emit_st(u8 buf[], enum dma330_cond cond)
+{
+ buf[0] = CMD_DMAST;
+ if (cond == SINGLE)
+ buf[0] |= (0 << 1) | (1 << 0);
+ else if (cond == BURST)
+ buf[0] |= (1 << 1) | (1 << 0);
+
+ DMA330_DBGCMD_DUMP(SZ_DMAST, "\tDMAST%c\n",
+ cond == SINGLE ? 'S' : (cond == BURST ? 'B' : 'A'));
+ return SZ_DMAST;
+}
+
+/**
+ * _emit_stp - Add opcode DMASTP into microcode (store and notify peripheral).
+ *
+ * @buf: The buffer which stored the microcode program.
+ * @cond: Execution criteria such as single, burst or always.
+ * @peri: Peripheral ID as listed in DMA NPP.
+ *
+ * Return: Size of opcode.
+ */
+static inline u32 _emit_stp(u8 buf[], enum dma330_cond cond, u8 peri)
+{
+ u8 *buffer = buf;
+
+ buffer[0] = CMD_DMASTP;
+ if (cond == BURST)
+ buf[0] |= (1 << 1);
+ peri &= 0x1f;
+ peri <<= 3;
+ buffer[1] = peri;
+ DMA330_DBGCMD_DUMP(SZ_DMASTP, "\tDMASTP%c %u\n",
+ cond == SINGLE ? 'S' : 'B', peri >> 3);
+ return SZ_DMASTP;
+}
+
+/**
+ * _emit_stz - Add opcode DMASTZ into microcode (store zeros).
+ *
+ * @buf -> The buffer which stored the microcode program.
+ *
+ * Return: Size of opcode.
+ */
+static inline u32 _emit_stz(u8 buf[])
+{
+ buf[0] = CMD_DMASTZ;
+ DMA330_DBGCMD_DUMP(SZ_DMASTZ, "\tDMASTZ\n");
+ return SZ_DMASTZ;
+}
+
+/**
+ * _emit_wfp - Add opcode DMAWFP into microcode (wait for peripheral).
+ *
+ * @buf: The buffer which stored the microcode program.
+ * @cond: Execution criteria such as single, burst or always.
+ * @peri: Peripheral ID as listed in DMA NPP.
+ *
+ * Return: Size of opcode.
+ */
+static inline u32 _emit_wfp(u8 buf[], enum dma330_cond cond, u8 peri)
+{
+ u8 *buffer = buf;
+
+ buffer[0] = CMD_DMAWFP;
+ if (cond == SINGLE)
+ buffer[0] |= (0 << 1) | (0 << 0);
+ else if (cond == BURST)
+ buffer[0] |= (1 << 1) | (0 << 0);
+ else
+ buffer[0] |= (0 << 1) | (1 << 0);
+
+ peri &= 0x1f;
+ peri <<= 3;
+ buffer[1] = peri;
+ DMA330_DBGCMD_DUMP(SZ_DMAWFP, "\tDMAWFP%c %u\n",
+ cond == SINGLE ? 'S' : (cond == BURST ? 'B' : 'P'), peri >> 3);
+ return SZ_DMAWFP;
+}
+
+/**
+ * _emit_wmb - Add opcode DMAWMB into microcode (write memory barrier).
+ *
+ * @buf: The buffer which stored the microcode program.
+ *
+ * Return: Size of opcode.
+ */
+static inline u32 _emit_wmb(u8 buf[])
+{
+ buf[0] = CMD_DMAWMB;
+ DMA330_DBGCMD_DUMP(SZ_DMAWMB, "\tDMAWMB\n");
+ return SZ_DMAWMB;
+}
+
+/**
+ * _emit_go - Add opcode DMALGO into microcode (go).
+ *
+ * @buf: The buffer which stored the microcode program.
+ * @arg: structure _arg_go which contain all needed info
+ * arg->chan -> channel number
+ * arg->addr -> start address of the microcode program
+ * which will be wrote into CPC register
+ * arg->ns -> 1 for non secure, 0 for secure
+ * (if only DMA Manager is in secure).
+ *
+ * Return: Size of opcode.
+ */
+static inline u32 _emit_go(u8 buf[], const struct _arg_go *arg)
+{
+ u8 *buffer = buf;
+ u8 chan = arg->chan;
+ u32 addr = arg->addr;
+ u32 ns = arg->ns;
+
+ buffer[0] = CMD_DMAGO;
+ buffer[0] |= (ns << 1);
+ buffer[1] = chan & 0x7;
+ buf[2] = addr & 0xFF;
+ buf[3] = (addr >> 8) & 0xFF;
+ buf[4] = (addr >> 16) & 0xFF;
+ buf[5] = (addr >> 24) & 0xFF;
+ return SZ_DMAGO;
+}
+
+/**
+ * _prepare_ccr - Populate the CCR register.
+ * @rqc: Request Configuration.
+ *
+ * Return: Channel Control register (CCR) Setting.
+ */
+static inline u32 _prepare_ccr(const struct dma330_reqcfg *rqc)
+{
+ u32 ccr = 0;
+
+ if (rqc->src_inc)
+ ccr |= CC_SRCINC;
+ if (rqc->dst_inc)
+ ccr |= CC_DSTINC;
+
+ /* We set same protection levels for Src and DST for now */
+ if (rqc->privileged)
+ ccr |= CC_SRCPRI | CC_DSTPRI;
+ if (rqc->nonsecure)
+ ccr |= CC_SRCNS | CC_DSTNS;
+ if (rqc->insnaccess)
+ ccr |= CC_SRCIA | CC_DSTIA;
+
+ ccr |= (((rqc->brst_len - 1) & 0xf) << CC_SRCBRSTLEN_SHFT);
+ ccr |= (((rqc->brst_len - 1) & 0xf) << CC_DSTBRSTLEN_SHFT);
+
+ ccr |= (rqc->brst_size << CC_SRCBRSTSIZE_SHFT);
+ ccr |= (rqc->brst_size << CC_DSTBRSTSIZE_SHFT);
+
+ ccr |= (rqc->scctl << CC_SRCCCTRL_SHFT);
+ ccr |= (rqc->dcctl << CC_DSTCCTRL_SHFT);
+
+ ccr |= (rqc->swap << CC_SWAP_SHFT);
+ return ccr;
+}
+
+/**
+ * dma330_until_dmac_idle - Wait until DMA Manager is idle.
+ *
+ * @plat: Pointer to struct dma_dma330_platdata.
+ * @timeout_ms: Timeout (in miliseconds).
+ *
+ * Return: Negative value for error / timeout ocurred before idle,
+ * 0 for successful.
+ */
+static int dma330_until_dmac_idle(struct dma_dma330_platdata *plat,
+ const u32 timeout_ms)
+{
+ void __iomem *regs = plat->base;
+
+ return wait_for_bit(__func__,
+ (const u32 *)(uintptr_t)(regs + DBGSTATUS),
+ DBG_BUSY, 0, timeout_ms, false);
+}
+
+/**
+ * dma330_execute_dbginsn - Execute debug instruction such as DMAGO and DMAKILL.
+ *
+ * @insn: The buffer which stored the debug instruction.
+ * @plat: Pointer to struct dma_dma330_platdata.
+ * @timeout_ms: Timeout (in miliseconds).
+ *
+ * Return: void.
+ */
+static inline void dma330_execute_dbginsn(u8 insn[],
+ struct dma_dma330_platdata *plat,
+ const u32 timeout_ms)
+{
+ void __iomem *regs = plat->base;
+ u32 val;
+
+ val = (insn[0] << 16) | (insn[1] << 24);
+ if (!plat->dma330.channel0_manager1)
+ val |= (1 << 0);
+ val |= (plat->dma330.channel_num << 8); /* Channel Number */
+ writel(val, regs + DBGINST0);
+ val = insn[2];
+ val = val | (insn[3] << 8);
+ val = val | (insn[4] << 16);
+ val = val | (insn[5] << 24);
+ writel(val, regs + DBGINST1);
+
+ /* If timed out due to halted state-machine */
+ if (dma330_until_dmac_idle(plat, timeout_ms)) {
+ debug("DMAC halted!\n");
+ return;
+ }
+ /* Get going */
+ writel(0, regs + DBGCMD);
+}
+
+/**
+ * dma330_getstate - Get the status of current channel or manager.
+ *
+ * @plat: Pointer to struct dma_dma330_platdata.
+ *
+ * Return: Status state of current channel or current manager.
+ */
+static inline u32 dma330_getstate(struct dma_dma330_platdata *plat)
+{
+ void __iomem *regs = plat->base;
+ u32 val;
+
+ if (plat->dma330.channel0_manager1)
+ val = readl((uintptr_t)(regs + DS)) & 0xf;
+ else
+ val = readl((uintptr_t)(regs + CS(plat->dma330.channel_num))) &
+ 0xf;
+
+ switch (val) {
+ case DS_ST_STOP:
+ return DMA330_STATE_STOPPED;
+ case DS_ST_EXEC:
+ return DMA330_STATE_EXECUTING;
+ case DS_ST_CMISS:
+ return DMA330_STATE_CACHEMISS;
+ case DS_ST_UPDTPC:
+ return DMA330_STATE_UPDTPC;
+ case DS_ST_WFE:
+ return DMA330_STATE_WFE;
+ case DS_ST_FAULT:
+ return DMA330_STATE_FAULTING;
+ case DS_ST_ATBRR:
+ if (plat->dma330.channel0_manager1)
+ return DMA330_STATE_INVALID;
+ else
+ return DMA330_STATE_ATBARRIER;
+ case DS_ST_QBUSY:
+ if (plat->dma330.channel0_manager1)
+ return DMA330_STATE_INVALID;
+ else
+ return DMA330_STATE_QUEUEBUSY;
+ case DS_ST_WFP:
+ if (plat->dma330.channel0_manager1)
+ return DMA330_STATE_INVALID;
+ else
+ return DMA330_STATE_WFP;
+ case DS_ST_KILL:
+ if (plat->dma330.channel0_manager1)
+ return DMA330_STATE_INVALID;
+ else
+ return DMA330_STATE_KILLING;
+ case DS_ST_CMPLT:
+ if (plat->dma330.channel0_manager1)
+ return DMA330_STATE_INVALID;
+ else
+ return DMA330_STATE_COMPLETING;
+ case DS_ST_FLTCMP:
+ if (plat->dma330.channel0_manager1)
+ return DMA330_STATE_INVALID;
+ else
+ return DMA330_STATE_FAULT_COMPLETING;
+ default:
+ return DMA330_STATE_INVALID;
+ }
+}
+
+/**
+ * dma330_trigger - Execute the DMAGO through debug instruction.
+ *
+ * @plat: Pointer to struct dma_dma330_platdata.
+ * @timeout_ms: Timeout (in miliseconds).
+ *
+ * When the DMA manager executes Go for a DMA channel that is in the Stopped
+ * state, it moves a 32-bit immediate into the program counter, then setting
+ * its security state and updating DMA channel to the Executing state.
+ *
+ * Return: Negative value for error as DMA is not ready. 0 for successful.
+ */
+static int dma330_trigger(struct dma_dma330_platdata *plat,
+ const u32 timeout_ms)
+{
+ u8 insn[6] = {0, 0, 0, 0, 0, 0};
+ struct _arg_go go;
+
+ /*
+ * Return if already ACTIVE. It will ensure DMAGO is executed at
+ * STOPPED state too
+ */
+ plat->dma330.channel0_manager1 = 0;
+ if (dma330_getstate(plat) !=
+ DMA330_STATE_STOPPED)
+ return -EPERM;
+
+ go.chan = plat->dma330.channel_num;
+ go.addr = (uintptr_t)plat->dma330.buf;
+
+ if (!plat->dma330.secure)
+ go.ns = 1; /* non-secure condition */
+ else
+ go.ns = 0; /* secure condition */
+
+ _emit_go(insn, &go);
+
+ /* Only manager can execute GO */
+ plat->dma330.channel0_manager1 = 1;
+ dma330_execute_dbginsn(insn, plat, timeout_ms);
+ return 0;
+}
+
+/**
+ * dma330_stop - Stop the manager or channel.
+ *
+ * @plat: Pointer to struct dma_dma330_platdata.
+ * @timeout_ms: Timeout (in miliseconds).
+ *
+ * Stop the manager/channel or killing them and ensure they reach to stop
+ * state.
+ *
+ * Return: void.
+ */
+static void dma330_stop(struct dma_dma330_platdata *plat, const u32 timeout_ms)
+{
+ u8 insn[6] = {0, 0, 0, 0, 0, 0};
+
+ /* If fault completing, wait until reach faulting and killing state */
+ if (dma330_getstate(plat) == DMA330_STATE_FAULT_COMPLETING)
+ UNTIL(plat, DMA330_STATE_FAULTING | DMA330_STATE_KILLING);
+
+ /* Return if nothing needs to be done */
+ if (dma330_getstate(plat) == DMA330_STATE_COMPLETING ||
+ dma330_getstate(plat) == DMA330_STATE_KILLING ||
+ dma330_getstate(plat) == DMA330_STATE_STOPPED)
+ return;
+
+ /* Kill it to ensure it reach to stop state */
+ _emit_kill(insn);
+
+ /* Execute the KILL instruction through debug registers */
+ dma330_execute_dbginsn(insn, plat, timeout_ms);
+}
+
+/**
+ * dma330_start - Execute the command list through DMAGO and debug instruction.
+ *
+ * @plat: Pointer to struct dma_dma330_platdata.
+ * @timeout_ms: Timeout (in miliseconds).
+ *
+ * Return: Negative value for error where DMA is not ready. 0 for successful.
+ */
+static int dma330_start(struct dma_dma330_platdata *plat, const u32 timeout_ms)
+{
+ debug("INFO: DMA BASE Address = 0x%08x\n",
+ (u32)(uintptr_t)plat->base);
+
+ switch (dma330_getstate(plat)) {
+ case DMA330_STATE_FAULT_COMPLETING:
+ UNTIL(plat, DMA330_STATE_FAULTING | DMA330_STATE_KILLING);
+
+ if (dma330_getstate(plat) == DMA330_STATE_KILLING)
+ UNTIL(plat, DMA330_STATE_STOPPED);
+
+ case DMA330_STATE_FAULTING:
+ dma330_stop(plat, timeout_ms);
+
+ case DMA330_STATE_KILLING:
+ case DMA330_STATE_COMPLETING:
+ UNTIL(plat, DMA330_STATE_STOPPED);
+
+ case DMA330_STATE_STOPPED:
+ return dma330_trigger(plat, timeout_ms);
+
+ case DMA330_STATE_WFP:
+ case DMA330_STATE_QUEUEBUSY:
+ case DMA330_STATE_ATBARRIER:
+ case DMA330_STATE_UPDTPC:
+ case DMA330_STATE_CACHEMISS:
+ case DMA330_STATE_EXECUTING:
+ return -ESRCH;
+ /* For RESUME, nothing yet */
+ case DMA330_STATE_WFE:
+ default:
+ return -ESRCH;
+ }
+}
+
+/**
+ * dma330_transfer_start - DMA start to run.
+ *
+ * @plat: Pointer to struct dma_dma330_platdata.
+ *
+ * DMA start to excecute microcode command list.
+ *
+ * Return: Negative value for error or not successful. 0 for successful.
+ */
+static int dma330_transfer_start(struct dma_dma330_platdata *plat)
+{
+ const u32 timeout_ms = 1000;
+
+ /* Execute the command list */
+ return dma330_start(plat, timeout_ms);
+}
+
+/**
+ * dma330_transfer_finish - DMA polling until execution finish or error.
+ *
+ * @plat: Pointer to struct dma_dma330_platdata.
+ *
+ * DMA polling until excution finish and checking the state status.
+ *
+ * Return: Negative value for state error or not successful. 0 for successful.
+ */
+static int dma330_transfer_finish(struct dma_dma330_platdata *plat)
+{
+ void __iomem *regs = plat->base;
+
+ if (!plat->dma330.buf) {
+ debug("ERROR DMA330 : DMA Microcode buffer pointer is NULL\n");
+ return -EINVAL;
+ }
+
+ plat->dma330.channel0_manager1 = 0;
+
+ /* Wait until finish execution to ensure we compared correct result*/
+ UNTIL(plat, DMA330_STATE_STOPPED | DMA330_STATE_FAULTING);
+
+ /* check the state */
+ if (dma330_getstate(plat) == DMA330_STATE_FAULTING) {
+ debug("FAULT Mode: Channel %d Faulting, FTR = 0x%08x,",
+ plat->dma330.channel_num,
+ readl(regs + FTC(plat->dma330.channel_num)));
+ debug("CPC = 0x%08lx\n",
+ (readl(regs + CPC(plat->dma330.channel_num))
+ - (uintptr_t)plat->dma330.buf));
+ return -EPROTO;
+ }
+ return 0;
+}
+
+/**
+ * dma330_transfer_setup - DMA transfer setup (DMA_MEM_TO_MEM, DMA_MEM_TO_DEV
+ * or DMA_DEV_TO_MEM).
+ *
+ * @plat: Pointer to struct dma_dma330_platdata.
+ *
+ * For Peripheral transfer, the FIFO threshold value is expected at
+ * 2 ^ plat->brst_size * plat->brst_len.
+ *
+ * Return: Negative value for error or not successful. 0 for successful.
+ */
+static int dma330_transfer_setup(struct dma_dma330_platdata *plat)
+{
+ /* Variable declaration */
+ /* Buffer offset clear to 0 */
+ int off = 0;
+ int ret = 0;
+ /* For DMALPEND */
+ u32 loopjmp0, loopjmp1;
+ /* Loop count 0 */
+ u32 lcnt0 = 0;
+ /* Loop count 1 */
+ u32 lcnt1 = 0;
+ u32 brst_size = 0;
+ u32 brst_len = 0;
+ u32 data_size_byte = plat->dma330.size_byte;
+ /* Strong order memory is required to store microcode command list */
+ u8 *buf = (u8 *)plat->dma330.buf;
+ /* Channel Control Register */
+ u32 ccr = 0;
+ struct dma330_reqcfg reqcfg;
+
+ if (!buf) {
+ debug("ERROR DMA330 : DMA Microcode buffer pointer is NULL\n");
+ return -EINVAL;
+ }
+
+ if (plat->dma330.transfer_type == DMA_MEM_TO_DEV)
+ debug("INFO: mem2perip");
+ else if (plat->dma330.transfer_type == DMA_DEV_TO_MEM)
+ debug("INFO: perip2mem");
+ else
+ debug("INFO: DMA_MEM_TO_MEM");
+
+ debug(" - 0x%x%x -> 0x%x%x\nsize=%08x brst_size=2^%d ",
+ (u32)(plat->dma330.src_addr >> 32),
+ (u32)plat->dma330.src_addr,
+ (u32)(plat->dma330.dst_addr >> 32),
+ (u32)plat->dma330.dst_addr,
+ data_size_byte,
+ plat->brst_size);
+
+ debug("brst_len=%d singles_brst_size=2^%d\n", plat->brst_len,
+ plat->dma330.single_brst_size);
+
+ /* brst_size = 2 ^ plat->brst_size */
+ brst_size = 1 << plat->brst_size;
+
+ /* Fool proof checking */
+ if (plat->brst_size < 0 || plat->brst_size >
+ plat->max_brst_size) {
+ debug("ERROR DMA330: brst_size must 0-%d (not %d)\n",
+ plat->max_brst_size, plat->brst_size);
+ return -EINVAL;
+ }
+ if (plat->dma330.single_brst_size < 0 ||
+ plat->dma330.single_brst_size > plat->max_brst_size) {
+ debug("ERROR DMA330 : single_brst_size must 0-%d (not %d)\n",
+ plat->max_brst_size, plat->dma330.single_brst_size);
+ return -EINVAL;
+ }
+ if (plat->brst_len < 1 || plat->brst_len > 16) {
+ debug("ERROR DMA330 : brst_len must 1-16 (not %d)\n",
+ plat->brst_len);
+ return -EINVAL;
+ }
+ if (plat->dma330.src_addr & (brst_size - 1)) {
+ debug("ERROR DMA330 : Source address unaligned\n");
+ return -EINVAL;
+ }
+ if (plat->dma330.dst_addr & (brst_size - 1)) {
+ debug("ERROR DMA330 : Destination address unaligned\n");
+ return -EINVAL;
+ }
+
+ /* Setup the command list */
+ /* DMAMOV SAR, x->src_addr */
+ off += _emit_mov(&buf[off], SAR, plat->dma330.src_addr);
+ /* DMAMOV DAR, x->dst_addr */
+ off += _emit_mov(&buf[off], DAR, plat->dma330.dst_addr);
+ /* DMAFLUSHP P(periheral_id) */
+ if (plat->dma330.transfer_type != DMA_MEM_TO_MEM)
+ off += _emit_flushp(&buf[off], plat->dma330.peripheral_id);
+
+ /* Preparing the CCR value */
+ if (plat->dma330.transfer_type == DMA_MEM_TO_DEV) {
+ /* Disable auto increment */
+ reqcfg.dst_inc = 0;
+ /* Enable auto increment */
+ reqcfg.src_inc = 1;
+ } else if (plat->dma330.transfer_type == DMA_DEV_TO_MEM) {
+ reqcfg.dst_inc = 1;
+ reqcfg.src_inc = 0;
+ } else {
+ /* DMA_MEM_TO_MEM */
+ reqcfg.dst_inc = 1;
+ reqcfg.src_inc = 1;
+ }
+
+ if (!plat->dma330.secure)
+ reqcfg.nonsecure = 1; /* Non Secure mode */
+ else
+ reqcfg.nonsecure = 0; /* Secure mode */
+
+ if (plat->dma330.enable_cache1 == 0) {
+ /* Noncacheable but bufferable */
+ reqcfg.dcctl = 0x1;
+ reqcfg.scctl = 0x1;
+ } else {
+ if (plat->dma330.transfer_type == DMA_MEM_TO_DEV) {
+ reqcfg.dcctl = 0x1;
+ /* Cacheable write back */
+ reqcfg.scctl = 0x7;
+ } else if (plat->dma330.transfer_type == DMA_DEV_TO_MEM) {
+ reqcfg.dcctl = 0x7;
+ reqcfg.scctl = 0x1;
+ } else {
+ reqcfg.dcctl = 0x7;
+ reqcfg.scctl = 0x7;
+ }
+ }
+ /* 1 - Privileged */
+ reqcfg.privileged = 1;
+ /* 0 - Data access */
+ reqcfg.insnaccess = 0;
+ /* 0 - No endian swap */
+ reqcfg.swap = 0;
+ /* DMA burst length */
+ reqcfg.brst_len = plat->brst_len;
+ /* DMA burst size */
+ reqcfg.brst_size = plat->brst_size;
+ /* Preparing the CCR value */
+ ccr = _prepare_ccr(&reqcfg);
+ /* DMAMOV CCR, ccr */
+ off += _emit_mov(&buf[off], CCR, ccr);
+
+ /* BURST */
+ /* Can initiate a burst? */
+ while (data_size_byte >= brst_size * plat->brst_len) {
+ lcnt0 = data_size_byte / (brst_size * plat->brst_len);
+ lcnt1 = 0;
+ if (lcnt0 >= 256 * 256) {
+ lcnt0 = 256;
+ lcnt1 = 256;
+ } else if (lcnt0 >= 256) {
+ lcnt1 = lcnt0 / 256;
+ lcnt0 = 256;
+ }
+ data_size_byte = data_size_byte -
+ (brst_size * plat->brst_len * lcnt0 * lcnt1);
+
+ debug("Transferring 0x%08x Remain 0x%08x\n", (brst_size *
+ plat->brst_len * lcnt0 * lcnt1), data_size_byte);
+ debug("Running burst - brst_size=2^%d, brst_len=%d, ",
+ plat->brst_size, plat->brst_len);
+ debug("lcnt0=%d, lcnt1=%d\n", lcnt0, lcnt1);
+
+ if (lcnt1) {
+ /* DMALP1 */
+ off += _emit_lp(&buf[off], 1, lcnt1);
+ loopjmp1 = off;
+ }
+ /* DMALP0 */
+ off += _emit_lp(&buf[off], 0, lcnt0);
+ loopjmp0 = off;
+ /* DMAWFP periheral_id, burst */
+ if (plat->dma330.transfer_type != DMA_MEM_TO_MEM)
+ off += _emit_wfp(&buf[off], BURST,
+ plat->dma330.peripheral_id);
+ /* DMALD */
+ off += _emit_ld(&buf[off], ALWAYS);
+
+ WATCHDOG_RESET();
+
+ /* DMARMB */
+ off += _emit_rmb(&buf[off]);
+ /* DMASTPB peripheral_id */
+ if (plat->dma330.transfer_type != DMA_MEM_TO_MEM)
+ off += _emit_stp(&buf[off], BURST,
+ plat->dma330.peripheral_id);
+ else
+ off += _emit_st(&buf[off], ALWAYS);
+ /* DMAWMB */
+ off += _emit_wmb(&buf[off]);
+ /* DMALP0END */
+ struct _arg_lpend lpend;
+
+ lpend.cond = ALWAYS;
+ lpend.forever = 0;
+ /* Loop cnt 0 */
+ lpend.loop = 0;
+ lpend.bjump = off - loopjmp0;
+ off += _emit_lpend(&buf[off], &lpend);
+ /* DMALP1END */
+ if (lcnt1) {
+ struct _arg_lpend lpend;
+
+ lpend.cond = ALWAYS;
+ lpend.forever = 0;
+ lpend.loop = 1; /* loop cnt 1*/
+ lpend.bjump = off - loopjmp1;
+ off += _emit_lpend(&buf[off], &lpend);
+ }
+ /* Ensure the microcode don't exceed buffer size */
+ if (off > plat->buf_size) {
+ debug("ERROR DMA330 : Exceed buffer size\n");
+ return -ENOMEM;
+ }
+ }
+
+ /* SINGLE */
+ brst_len = 1;
+ /* brst_size = 2 ^ plat->dma330.single_brst_size; */
+ brst_size = (1 << plat->dma330.single_brst_size);
+ lcnt0 = data_size_byte / (brst_size * brst_len);
+
+ /* Ensure all data will be transferred */
+ data_size_byte = data_size_byte -
+ (brst_size * brst_len * lcnt0);
+ if (data_size_byte) {
+ debug("ERROR DMA330 : Detected the possibility of ");
+ debug("untransfered data. Please ensure correct single ");
+ debug("burst size\n");
+ }
+
+ if (lcnt0) {
+ debug("Transferring 0x%08x Remain 0x%08x\n", (brst_size *
+ brst_len * lcnt0 * lcnt1), data_size_byte);
+ debug("Running single - brst_size=2^%d, brst_len=%d, ",
+ brst_size, brst_len);
+ debug("lcnt0=%d\n", lcnt0);
+
+ /* Preparing the CCR value */
+ /* DMA burst length */
+ reqcfg.brst_len = brst_len;
+ /* DMA burst size */
+ reqcfg.brst_size = brst_size;
+ ccr = _prepare_ccr(&reqcfg);
+ /* DMAMOV CCR, ccr */
+ off += _emit_mov(&buf[off], CCR, ccr);
+
+ /* DMALP0 */
+ off += _emit_lp(&buf[off], 0, lcnt0);
+ loopjmp0 = off;
+ /* DMAWFP peripheral_id, single */
+ if (plat->dma330.transfer_type != DMA_MEM_TO_MEM)
+ off += _emit_wfp(&buf[off], SINGLE,
+ plat->dma330.peripheral_id);
+
+ /* DMALD */
+ off += _emit_ld(&buf[off], ALWAYS);
+ /* DMARMB */
+ off += _emit_rmb(&buf[off]);
+ /* DMASTPS peripheral_id */
+ if (plat->dma330.transfer_type != DMA_MEM_TO_MEM)
+ off += _emit_stp(&buf[off], SINGLE,
+ plat->dma330.peripheral_id);
+ else
+ off += _emit_st(&buf[off], ALWAYS);
+
+ /* DMAWMB */
+ off += _emit_wmb(&buf[off]);
+ /* DMALPEND */
+ struct _arg_lpend lpend1;
+
+ lpend1.cond = ALWAYS;
+ lpend1.forever = 0;
+ /* loop cnt 0 */
+ lpend1.loop = 0;
+ lpend1.bjump = off - loopjmp0;
+ off += _emit_lpend(&buf[off], &lpend1);
+ /* Ensure the microcode don't exceed buffer size */
+ if (off > plat->buf_size) {
+ puts("ERROR DMA330 : Exceed buffer size\n");
+ return -ENOMEM;
+ }
+ }
+
+ /* DMAEND */
+ off += _emit_end(&buf[off]);
+
+ ret = dma330_transfer_start(plat);
+ if (ret)
+ return ret;
+
+ return dma330_transfer_finish(plat);
+}
+
+/**
+ * dma330_transfer_zeroes - DMA transfer zeros.
+ *
+ * @plat: Pointer to struct dma_dma330_platdata.
+ *
+ * Used to write zeros to a memory chunk for memory scrubbing purpose.
+ *
+ * Return: Negative value for error or not successful. 0 for successful.
+ */
+static int dma330_transfer_zeroes_setup(struct dma_dma330_platdata *plat)
+{
+ /* Variable declaration */
+ /* Buffer offset clear to 0 */
+ int off = 0;
+ int ret = 0;
+ /* For DMALPEND */
+ u32 loopjmp0, loopjmp1;
+ /* Loop count 0 */
+ u32 lcnt0 = 0;
+ /* Loop count 1 */
+ u32 lcnt1 = 0;
+ u32 brst_size = 0;
+ u32 brst_len = 0;
+ u32 data_size_byte = plat->dma330.size_byte;
+ /* Strong order memory is required to store microcode command list */
+ u8 *buf = (u8 *)plat->dma330.buf;
+ /* Channel Control Register */
+ u32 ccr = 0;
+ struct dma330_reqcfg reqcfg;
+
+ if (!buf) {
+ debug("ERROR DMA330 : DMA Microcode buffer pointer is NULL\n");
+ return -EINVAL;
+ }
+
+ debug("INFO: Write zeros -> ");
+ debug("0x%x%x size=0x%08x\n",
+ (u32)(plat->dma330.dst_addr >> 32),
+ (u32)plat->dma330.dst_addr,
+ data_size_byte);
+
+ plat->dma330.single_brst_size = 1;
+
+ /* burst_size = 2 ^ plat->brst_size */
+ brst_size = 1 << plat->brst_size;
+
+ /* Setup the command list */
+ /* DMAMOV DAR, x->dst_addr */
+ off += _emit_mov(&buf[off], DAR, plat->dma330.dst_addr);
+
+ /* Preparing the CCR value */
+ /* Enable auto increment */
+ reqcfg.dst_inc = 1;
+ /* Disable auto increment (not applicable) */
+ reqcfg.src_inc = 0;
+ /* Noncacheable but bufferable */
+ reqcfg.dcctl = 0x1;
+ /* Noncacheable and bufferable */
+ reqcfg.scctl = 0x1;
+ /* 1 - Privileged */
+ reqcfg.privileged = 1;
+ /* 0 - Data access */
+ reqcfg.insnaccess = 0;
+ /* 0 - No endian swap */
+ reqcfg.swap = 0;
+
+ if (!plat->dma330.secure)
+ reqcfg.nonsecure = 1; /* Non Secure mode */
+ else
+ reqcfg.nonsecure = 0; /* Secure mode */
+
+ /* DMA burst length */
+ reqcfg.brst_len = plat->brst_len;
+ /* DMA burst size */
+ reqcfg.brst_size = plat->brst_size;
+ /* Preparing the CCR value */
+ ccr = _prepare_ccr(&reqcfg);
+ /* DMAMOV CCR, ccr */
+ off += _emit_mov(&buf[off], CCR, ccr);
+
+ /* BURST */
+ /* Can initiate a burst? */
+ while (data_size_byte >= brst_size * plat->brst_len) {
+ lcnt0 = data_size_byte / (brst_size * plat->brst_len);
+ lcnt1 = 0;
+ if (lcnt0 >= 256 * 256) {
+ lcnt0 = 256;
+ lcnt1 = 256;
+ } else if (lcnt0 >= 256) {
+ lcnt1 = lcnt0 / 256;
+ lcnt0 = 256;
+ }
+
+ data_size_byte = data_size_byte -
+ (brst_size * plat->brst_len * lcnt0 * lcnt1);
+
+ debug("Transferring 0x%08x Remain 0x%08x\n", (brst_size *
+ plat->brst_len * lcnt0 * lcnt1), data_size_byte);
+ debug("Running burst - brst_size=2^%d, brst_len=%d,",
+ plat->brst_size, plat->brst_len);
+ debug("lcnt0=%d, lcnt1=%d\n", lcnt0, lcnt1);
+
+ if (lcnt1) {
+ /* DMALP1 */
+ off += _emit_lp(&buf[off], 1, lcnt1);
+ loopjmp1 = off;
+ }
+ /* DMALP0 */
+ off += _emit_lp(&buf[off], 0, lcnt0);
+ loopjmp0 = off;
+ /* DMALSTZ */
+ off += _emit_stz(&buf[off]);
+
+ WATCHDOG_RESET();
+
+ /* DMALP0END */
+ struct _arg_lpend lpend;
+
+ lpend.cond = ALWAYS;
+ lpend.forever = 0;
+ /* Loop cnt 0 */
+ lpend.loop = 0;
+ lpend.bjump = off - loopjmp0;
+ off += _emit_lpend(&buf[off], &lpend);
+ /* DMALP1END */
+ if (lcnt1) {
+ struct _arg_lpend lpend;
+
+ lpend.cond = ALWAYS;
+ lpend.forever = 0;
+ /* Loop cnt 1*/
+ lpend.loop = 1;
+ lpend.bjump = off - loopjmp1;
+ off += _emit_lpend(&buf[off], &lpend);
+ }
+ /* Ensure the microcode don't exceed buffer size */
+ if (off > plat->buf_size) {
+ printf("off = %d\n", off);
+ debug("ERROR DMA330 : Exceed buffer size off %d\n",
+ off);
+ return -ENOMEM;
+ }
+ }
+
+ /* SINGLE */
+ brst_len = 1;
+ /* brst_size = 2 ^ plat->dma330.single_brst_size */
+ brst_size = (1 << plat->dma330.single_brst_size);
+ lcnt0 = data_size_byte / (brst_size * brst_len);
+
+ /* ensure all data will be transferred */
+ data_size_byte = data_size_byte -
+ (brst_size * brst_len * lcnt0);
+ if (data_size_byte) {
+ debug("ERROR DMA330 : Detected the possibility of ");
+ debug("untransfered data. Please ensure correct single ");
+ debug("burst size\n");
+ }
+
+ if (lcnt0) {
+ debug("Transferring 0x%08x Remain 0x%08x\n", (brst_size *
+ brst_len * lcnt0), data_size_byte);
+ debug("Running single - brst_size=2^%d, brst_len=%d, ",
+ brst_size, brst_len);
+ debug("lcnt0=%d\n", lcnt0);
+
+ /* Preparing the CCR value */
+ /* DMA burst length */
+ reqcfg.brst_len = brst_len;
+ /* DMA burst size */
+ reqcfg.brst_size = brst_size;
+ ccr = _prepare_ccr(&reqcfg);
+ /* DMAMOV CCR, ccr */
+ off += _emit_mov(&buf[off], CCR, ccr);
+
+ /* DMALP0 */
+ off += _emit_lp(&buf[off], 0, lcnt0);
+ loopjmp0 = off;
+ /* DMALSTZ */
+ off += _emit_stz(&buf[off]);
+ /* DMALPEND */
+ struct _arg_lpend lpend1;
+
+ lpend1.cond = ALWAYS;
+ lpend1.forever = 0;
+ /* Loop cnt 0 */
+ lpend1.loop = 0;
+ lpend1.bjump = off - loopjmp0;
+ off += _emit_lpend(&buf[off], &lpend1);
+ /* Ensure the microcode don't exceed buffer size */
+ if (off > plat->buf_size) {
+ debug("ERROR DMA330 : Exceed buffer size\n");
+ return -ENOMEM;
+ }
+ }
+
+ /* DMAEND */
+ off += _emit_end(&buf[off]);
+
+ ret = dma330_transfer_start(plat);
+ if (ret)
+ return ret;
+
+ return dma330_transfer_finish(plat);
+}
+
+static int dma330_transfer_zeroes(struct udevice *dev, void *dst, size_t len)
+{
+ struct dma_dma330_platdata *plat = dev->platdata;
+ int ret = 0;
+
+ /* If DMA access is set to secure, base change to DMA secure base */
+ if (plat->dma330.secure)
+ plat->base += 0x1000;
+
+ plat->dma330.dst_addr = (phys_size_t)(uintptr_t)dst;
+ plat->dma330.size_byte = len;
+
+ ret = dma330_transfer_zeroes_setup(plat);
+
+ /* Revert back to non secure DMA base */
+ if (plat->dma330.secure)
+ plat->base -= 0x1000;
+
+ return ret;
+}
+
+static int dma330_transfer(struct udevice *dev, int direction, void *dst,
+ void *src, size_t len)
+{
+ struct dma_dma330_platdata *plat = dev->platdata;
+ int ret = 0;
+
+ /* If DMA access is set to secure, base change to DMA secure base */
+ if (plat->dma330.secure)
+ plat->base += 0x1000;
+
+ plat->dma330.dst_addr = (phys_size_t)(uintptr_t)dst;
+ plat->dma330.src_addr = (phys_size_t)(uintptr_t)src;
+ plat->dma330.size_byte = len;
+
+ switch (direction) {
+ case DMA_MEM_TO_MEM:
+ plat->dma330.transfer_type = DMA_SUPPORTS_MEM_TO_MEM;
+ break;
+ case DMA_MEM_TO_DEV:
+ plat->dma330.transfer_type = DMA_SUPPORTS_MEM_TO_DEV;
+ break;
+ case DMA_DEV_TO_MEM:
+ plat->dma330.transfer_type = DMA_SUPPORTS_DEV_TO_MEM;
+ break;
+ }
+
+ ret = dma330_transfer_setup(plat);
+
+ /* Revert back to non secure DMA base */
+ if (plat->dma330.secure)
+ plat->base -= 0x1000;
+
+ return ret;
+}
+
+static int dma330_ofdata_to_platdata(struct udevice *dev)
+{
+ struct dma_dma330_platdata *plat = dev->platdata;
+ const void *blob = gd->fdt_blob;
+ int node = dev_of_offset(dev);
+
+ plat->base = (void __iomem *)devfdt_get_addr(dev);
+ plat->max_brst_size = fdtdec_get_uint(blob, node, "dma-max-burst-size",
+ 2);
+ plat->brst_size = fdtdec_get_uint(blob, node, "dma-burst-size", 2);
+ plat->brst_len = fdtdec_get_uint(blob, node, "dma-burst-length", 2);
+
+ return 0;
+}
+
+static int dma330_probe(struct udevice *adev)
+{
+ struct dma_dev_priv *uc_priv = dev_get_uclass_priv(adev);
+
+ uc_priv->supported = (DMA_SUPPORTS_MEM_TO_MEM |
+ DMA_SUPPORTS_MEM_TO_DEV |
+ DMA_SUPPORTS_DEV_TO_MEM);
+ return 0;
+}
+
+static const struct dma_ops dma330_ops = {
+ .transfer = dma330_transfer,
+ .transfer_zeroes = dma330_transfer_zeroes,
+};
+
+static const struct udevice_id dma330_ids[] = {
+ { .compatible = "arm,pl330" },
+ { .compatible = "arm,dma330" },
+ { /* sentinel */ }
+};
+
+U_BOOT_DRIVER(dma_dma330) = {
+ .name = "dma_dma330",
+ .id = UCLASS_DMA,
+ .of_match = dma330_ids,
+ .ops = &dma330_ops,
+ .ofdata_to_platdata = dma330_ofdata_to_platdata,
+ .probe = dma330_probe,
+ .platdata_auto_alloc_size = sizeof(struct dma_dma330_platdata),
+};
diff --git a/include/dma330.h b/include/dma330.h
new file mode 100644
index 0000000..c054bf2
--- /dev/null
+++ b/include/dma330.h
@@ -0,0 +1,136 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018 Intel Corporation <www.intel.com>
+ */
+
+#include <dma.h>
+#include <linux/sizes.h>
+
+#ifndef __DMA330_CORE_H
+#define __DMA330_CORE_H
+
+#define DMA330_MAX_CHAN 8
+#define DMA330_MAX_IRQS 32
+#define DMA330_MAX_PERI 32
+
+#define DMA330_STATE_STOPPED BIT(0)
+#define DMA330_STATE_EXECUTING BIT(1)
+#define DMA330_STATE_WFE BIT(2)
+#define DMA330_STATE_FAULTING BIT(3)
+#define DMA330_STATE_COMPLETING BIT(4)
+#define DMA330_STATE_WFP BIT(5)
+#define DMA330_STATE_KILLING BIT(6)
+#define DMA330_STATE_FAULT_COMPLETING BIT(7)
+#define DMA330_STATE_CACHEMISS BIT(8)
+#define DMA330_STATE_UPDTPC BIT(9)
+#define DMA330_STATE_ATBARRIER BIT(10)
+#define DMA330_STATE_QUEUEBUSY BIT(11))
+#define DMA330_STATE_INVALID BIT(15)
+
+enum dma330_srccachectrl {
+ SCCTRL0 = 0, /* Noncacheable and nonbufferable */
+ SCCTRL1, /* Bufferable only */
+ SCCTRL2, /* Cacheable, but do not allocate */
+ SCCTRL3, /* Cacheable and bufferable, but do not allocate */
+ SINVALID1,
+ SINVALID2,
+ SCCTRL6, /* Cacheable write-through, allocate on reads only */
+ SCCTRL7, /* Cacheable write-back, allocate on reads only */
+};
+
+enum dma330_dstcachectrl {
+ DCCTRL0 = 0, /* Noncacheable and nonbufferable */
+ DCCTRL1, /* Bufferable only */
+ DCCTRL2, /* Cacheable, but do not allocate */
+ DCCTRL3, /* Cacheable and bufferable, but do not allocate */
+ DINVALID1 = 8,
+ DINVALID2,
+ DCCTRL6, /* Cacheable write-through, allocate on writes only */
+ DCCTRL7, /* Cacheable write-back, allocate on writes only */
+};
+
+enum dma330_byteswap {
+ SWAP_NO = 0,
+ SWAP_2,
+ SWAP_4,
+ SWAP_8,
+ SWAP_16,
+};
+
+/**
+ * dma330_reqcfg - Request Configuration.
+ *
+ * The DMA330 core does not modify this and uses the last
+ * working configuration if the request doesn't provide any.
+ *
+ * The Client may want to provide this info only for the
+ * first request and a request with new settings.
+ */
+struct dma330_reqcfg {
+ /* Address Incrementing */
+ u8 dst_inc;
+ u8 src_inc;
+
+ /*
+ * For now, the SRC & DST protection levels
+ * and burst size/length are assumed same.
+ */
+ u8 nonsecure;
+ u8 privileged;
+ u8 insnaccess;
+ u8 brst_len;
+ u8 brst_size; /* in power of 2 */
+
+ enum dma330_dstcachectrl dcctl;
+ enum dma330_srccachectrl scctl;
+ enum dma330_byteswap swap;
+};
+
+/**
+ * dma330_transfer_struct - Structure to be passed in for dma330_transfer_x.
+ *
+ * @channel0_manager1: Switching configuration on DMA channel or DMA manager.
+ * @channel_num: Channel number assigned, valid from 0 to 7.
+ * @src_addr: Address to transfer from / source.
+ * @dst_addr: Address to transfer to / destination.
+ * @size_byte: Number of bytes to be transferred.
+ * @brst_size: Valid from 0 - 4,
+ * where 0 = 1 (2 ^ 0) bytes and 3 = 16 bytes (2 ^ 4).
+ * @max_brst_size: Max transfer size (from 0 - 4).
+ * @single_brst_size: Single transfer size (from 0 - 4).
+ * @brst_len: Valid from 1 - 16 where each burst can transfer 1 - 16
+ * Data chunk (each chunk size equivalent to brst_size).
+ * @peripheral_id: Assigned peripheral_id, valid from 0 to 31.
+ * @transfer_type: MEM2MEM, MEM2PERIPH or PERIPH2MEM.
+ * @enable_cache1: 1 for cache enabled for memory
+ * (cacheable and bufferable, but do not allocate).
+ * @buf: Buffer handler which will point to the memory
+ * allocated for dma microcode
+ * @secure: Set DMA channel in secure mode.
+ *
+ * Description of the structure.
+ */
+struct dma330_transfer_struct {
+ u32 channel0_manager1;
+ u32 channel_num;
+ phys_size_t src_addr;
+ phys_size_t dst_addr;
+ u32 size_byte;
+ u32 single_brst_size;
+ u32 peripheral_id;
+ u32 transfer_type;
+ u32 enable_cache1;
+ u32 *buf;
+ u8 secure;
+};
+
+struct dma_dma330_platdata {
+ void __iomem *base;
+ u32 buf_size;
+ u32 max_brst_size;
+ u32 brst_size;
+ u32 brst_len;
+ struct dma330_transfer_struct dma330;
+};
+
+#endif /* __DMA330_CORE_H */
--
2.2.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/5] drivers: dma: Add function to zeroes a range of destination such as memory
2018-05-31 8:08 [U-Boot] [PATCH 0/5] Add DMA driver for DMA330 controller tien.fong.chee at intel.com
2018-05-31 8:08 ` [U-Boot] [PATCH 1/5] drivers: dma: Enable DMA-330 driver support tien.fong.chee at intel.com
@ 2018-05-31 8:08 ` tien.fong.chee at intel.com
2018-06-01 14:24 ` Simon Glass
2018-05-31 8:08 ` [U-Boot] [PATCH 3/5] drivers: dma: Factor out dma_get_device from DMA class function tien.fong.chee at intel.com
` (3 subsequent siblings)
5 siblings, 1 reply; 27+ messages in thread
From: tien.fong.chee at intel.com @ 2018-05-31 8:08 UTC (permalink / raw)
To: u-boot
From: Tien Fong Chee <tien.fong.chee@intel.com>
This new DMA class function enables DMA being used for initializing
a range of destination such as memory to zeros. This is quite useful to
help accelerating the performance in scrubbing memory when ECC is enabled.
Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
---
drivers/dma/dma-uclass.c | 15 +++++++++++++++
include/dma.h | 12 ++++++++++++
2 files changed, 27 insertions(+)
diff --git a/drivers/dma/dma-uclass.c b/drivers/dma/dma-uclass.c
index a33f7d5..cb83c24 100644
--- a/drivers/dma/dma-uclass.c
+++ b/drivers/dma/dma-uclass.c
@@ -61,6 +61,21 @@ int dma_memcpy(void *dst, void *src, size_t len)
return ops->transfer(dev, DMA_MEM_TO_MEM, dst, src, len);
}
+int dma_memcpy_zeroes(struct udevice *dev, void *dst, size_t len)
+{
+ const struct dma_ops *ops;
+
+ ops = device_get_ops(dev);
+ if (!ops->transfer_zeroes)
+ return -ENOSYS;
+
+ /* Invalidate the area, so no writeback into the RAM races with DMA */
+ invalidate_dcache_range((unsigned long)dst, (unsigned long)dst +
+ roundup(len, ARCH_DMA_MINALIGN));
+
+ return ops->transfer_zeroes(dev, dst, len);
+}
+
UCLASS_DRIVER(dma) = {
.id = UCLASS_DMA,
.name = "dma",
diff --git a/include/dma.h b/include/dma.h
index 50e9652..6bad2264 100644
--- a/include/dma.h
+++ b/include/dma.h
@@ -46,6 +46,7 @@ struct dma_ops {
*/
int (*transfer)(struct udevice *dev, int direction, void *dst,
void *src, size_t len);
+ int (*transfer_zeroes)(struct udevice *dev, void *dst, size_t len);
};
/*
@@ -82,4 +83,15 @@ int dma_get_device(u32 transfer_type, struct udevice **devp);
*/
int dma_memcpy(void *dst, void *src, size_t len);
+/*
+ * dma_memcpy_zeroes - Fill up destination with zeros through DMA.
+ *
+ * @dev: The DMA device
+ * @dst: destination pointer
+ * @len: length to be copied with zero
+ * @return: on successful transfer returns zero.
+ * on failure returns error code.
+ */
+int dma_memcpy_zeroes(struct udevice *dev, void *dst, size_t len);
+
#endif /* _DMA_H_ */
--
2.2.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/5] drivers: dma: Factor out dma_get_device from DMA class function
2018-05-31 8:08 [U-Boot] [PATCH 0/5] Add DMA driver for DMA330 controller tien.fong.chee at intel.com
2018-05-31 8:08 ` [U-Boot] [PATCH 1/5] drivers: dma: Enable DMA-330 driver support tien.fong.chee at intel.com
2018-05-31 8:08 ` [U-Boot] [PATCH 2/5] drivers: dma: Add function to zeroes a range of destination such as memory tien.fong.chee at intel.com
@ 2018-05-31 8:08 ` tien.fong.chee at intel.com
2018-06-01 14:25 ` Simon Glass
2018-05-31 8:08 ` [U-Boot] [PATCH 4/5] include: dma: Update the function description for dma_memcpy tien.fong.chee at intel.com
` (2 subsequent siblings)
5 siblings, 1 reply; 27+ messages in thread
From: tien.fong.chee at intel.com @ 2018-05-31 8:08 UTC (permalink / raw)
To: u-boot
From: Tien Fong Chee <tien.fong.chee@intel.com>
Factor out the dma_get_device from DMA class function so caller can
set some configuration and changes on the DMA device structure which
is return by calling dma_get_device before device instance is processed by
DMA class functions.
Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
---
drivers/dma/dma-uclass.c | 8 +-------
drivers/mtd/spi/spi_flash.c | 9 ++++++++-
include/dma.h | 3 ++-
3 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/dma/dma-uclass.c b/drivers/dma/dma-uclass.c
index cb83c24..5663be8 100644
--- a/drivers/dma/dma-uclass.c
+++ b/drivers/dma/dma-uclass.c
@@ -40,15 +40,9 @@ int dma_get_device(u32 transfer_type, struct udevice **devp)
return ret;
}
-int dma_memcpy(void *dst, void *src, size_t len)
+int dma_memcpy(struct udevice *dev, void *dst, void *src, size_t len)
{
- struct udevice *dev;
const struct dma_ops *ops;
- int ret;
-
- ret = dma_get_device(DMA_SUPPORTS_MEM_TO_MEM, &dev);
- if (ret < 0)
- return ret;
ops = device_get_ops(dev);
if (!ops->transfer)
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 2911729..7aa66b6 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -457,7 +457,14 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
void __weak spi_flash_copy_mmap(void *data, void *offset, size_t len)
{
#ifdef CONFIG_DMA
- if (!dma_memcpy(data, offset, len))
+ struct udevice *dev;
+ int ret;
+
+ ret = dma_get_device(DMA_SUPPORTS_MEM_TO_MEM, &dev);
+ if (ret < 0)
+ return;
+
+ if (!dma_memcpy(dev, data, offset, len))
return;
#endif
memcpy(data, offset, len);
diff --git a/include/dma.h b/include/dma.h
index 6bad2264..0a0c9dd 100644
--- a/include/dma.h
+++ b/include/dma.h
@@ -75,13 +75,14 @@ int dma_get_device(u32 transfer_type, struct udevice **devp);
* dma_memcpy - try to use DMA to do a mem copy which will be
* much faster than CPU mem copy
*
+ * @dev - The DMA device
* @dst - destination pointer
* @src - souce pointer
* @len - data length to be copied
* @return - on successful transfer returns no of bytes
transferred and on failure return error code.
*/
-int dma_memcpy(void *dst, void *src, size_t len);
+int dma_memcpy(struct udevice *dev, void *dst, void *src, size_t len);
/*
* dma_memcpy_zeroes - Fill up destination with zeros through DMA.
--
2.2.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 4/5] include: dma: Update the function description for dma_memcpy
2018-05-31 8:08 [U-Boot] [PATCH 0/5] Add DMA driver for DMA330 controller tien.fong.chee at intel.com
` (2 preceding siblings ...)
2018-05-31 8:08 ` [U-Boot] [PATCH 3/5] drivers: dma: Factor out dma_get_device from DMA class function tien.fong.chee at intel.com
@ 2018-05-31 8:08 ` tien.fong.chee at intel.com
2018-06-01 14:25 ` Simon Glass
2018-05-31 8:08 ` [U-Boot] [PATCH 5/5] arm: dts: socfpga: stratix10: update pdma tien.fong.chee at intel.com
2018-05-31 9:24 ` [U-Boot] [PATCH 0/5] Add DMA driver for DMA330 controller Marek Vasut
5 siblings, 1 reply; 27+ messages in thread
From: tien.fong.chee at intel.com @ 2018-05-31 8:08 UTC (permalink / raw)
To: u-boot
From: Tien Fong Chee <tien.fong.chee@intel.com>
Update the dma_memcpy description on return argument for DMA330 driver.
Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
---
include/dma.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/dma.h b/include/dma.h
index 0a0c9dd..b825c1e 100644
--- a/include/dma.h
+++ b/include/dma.h
@@ -79,8 +79,8 @@ int dma_get_device(u32 transfer_type, struct udevice **devp);
* @dst - destination pointer
* @src - souce pointer
* @len - data length to be copied
- * @return - on successful transfer returns no of bytes
- transferred and on failure return error code.
+ * @return - on successful transfer returns no of bytes or zero(for DMA330)
+ * transferred and on failure return error code.
*/
int dma_memcpy(struct udevice *dev, void *dst, void *src, size_t len);
--
2.2.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 5/5] arm: dts: socfpga: stratix10: update pdma
2018-05-31 8:08 [U-Boot] [PATCH 0/5] Add DMA driver for DMA330 controller tien.fong.chee at intel.com
` (3 preceding siblings ...)
2018-05-31 8:08 ` [U-Boot] [PATCH 4/5] include: dma: Update the function description for dma_memcpy tien.fong.chee at intel.com
@ 2018-05-31 8:08 ` tien.fong.chee at intel.com
2018-06-01 14:25 ` Simon Glass
2018-07-09 18:03 ` Dinh Nguyen
2018-05-31 9:24 ` [U-Boot] [PATCH 0/5] Add DMA driver for DMA330 controller Marek Vasut
5 siblings, 2 replies; 27+ messages in thread
From: tien.fong.chee at intel.com @ 2018-05-31 8:08 UTC (permalink / raw)
To: u-boot
From: Tien Fong Chee <tien.fong.chee@intel.com>
Update pdma properties for Stratix 10
Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
---
arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/arch/arm/dts/socfpga_stratix10.dtsi b/arch/arm/dts/socfpga_stratix10.dtsi
index ccd3f32..311ba09 100644
--- a/arch/arm/dts/socfpga_stratix10.dtsi
+++ b/arch/arm/dts/socfpga_stratix10.dtsi
@@ -82,6 +82,26 @@
ranges = <0 0 0 0xffffffff>;
u-boot,dm-pre-reloc;
+ amba {
+ u-boot,dm-pre-reloc;
+ compatible = "arm,amba-bus";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ pdma: pdma at ffda0000 {
+ u-boot,dm-pre-reloc;
+ compatible = "arm,pl330", "arm,dma330";
+ reg = <0xffda0000 0x2000>;
+ #dma-cells = <1>;
+ #dma-channels = <8>;
+ #dma-requests = <32>;
+ dma-max-burst-size = <2>;
+ dma-burst-size = <2>;
+ dma-burst-length = <16>;
+ };
+ };
+
clkmgr at ffd1000 {
compatible = "altr,clk-mgr";
reg = <0xffd10000 0x1000>;
--
2.2.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 0/5] Add DMA driver for DMA330 controller
2018-05-31 8:08 [U-Boot] [PATCH 0/5] Add DMA driver for DMA330 controller tien.fong.chee at intel.com
` (4 preceding siblings ...)
2018-05-31 8:08 ` [U-Boot] [PATCH 5/5] arm: dts: socfpga: stratix10: update pdma tien.fong.chee at intel.com
@ 2018-05-31 9:24 ` Marek Vasut
2018-05-31 9:50 ` See, Chin Liang
5 siblings, 1 reply; 27+ messages in thread
From: Marek Vasut @ 2018-05-31 9:24 UTC (permalink / raw)
To: u-boot
On 05/31/2018 10:08 AM, tien.fong.chee at intel.com wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
>
> This patchset contains dm driver for DMA330 controller.
>
> This series is working on top of u-boot-socfpga.git -
> http://git.denx.de/u-boot.git .
>
> Tien Fong Chee (5):
> drivers: dma: Enable DMA-330 driver support
> drivers: dma: Add function to zeroes a range of destination such as
> memory
> drivers: dma: Factor out dma_get_device from DMA class function
> include: dma: Update the function description for dma_memcpy
> arm: dts: socfpga: stratix10: update pdma
>
> arch/arm/dts/socfpga_stratix10.dtsi | 20 +
> drivers/dma/Kconfig | 9 +-
> drivers/dma/Makefile | 1 +
> drivers/dma/dma-uclass.c | 23 +-
> drivers/dma/dma330.c | 1514 +++++++++++++++++++++++++++++++++++
> drivers/mtd/spi/spi_flash.c | 9 +-
> include/dma.h | 19 +-
> include/dma330.h | 136 ++++
> 8 files changed, 1719 insertions(+), 12 deletions(-)
> create mode 100644 drivers/dma/dma330.c
> create mode 100644 include/dma330.h
>
I presume this is to zero-out the ECC RAM ? Just enable caches and use
memset, it is much faster than this DMA witchcraft, at least on the A10.
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 0/5] Add DMA driver for DMA330 controller
2018-05-31 9:24 ` [U-Boot] [PATCH 0/5] Add DMA driver for DMA330 controller Marek Vasut
@ 2018-05-31 9:50 ` See, Chin Liang
2018-05-31 9:58 ` Marek Vasut
0 siblings, 1 reply; 27+ messages in thread
From: See, Chin Liang @ 2018-05-31 9:50 UTC (permalink / raw)
To: u-boot
On Thu, 2018-05-31 at 11:24 +0200, Marek Vasut wrote:
> On 05/31/2018 10:08 AM, tien.fong.chee at intel.com wrote:
> >
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> >
> > This patchset contains dm driver for DMA330 controller.
> >
> > This series is working on top of u-boot-socfpga.git -
> > http://git.denx.de/u-boot.git .
> >
> > Tien Fong Chee (5):
> > drivers: dma: Enable DMA-330 driver support
> > drivers: dma: Add function to zeroes a range of destination such
> > as
> > memory
> > drivers: dma: Factor out dma_get_device from DMA class function
> > include: dma: Update the function description for dma_memcpy
> > arm: dts: socfpga: stratix10: update pdma
> >
> > arch/arm/dts/socfpga_stratix10.dtsi | 20 +
> > drivers/dma/Kconfig | 9 +-
> > drivers/dma/Makefile | 1 +
> > drivers/dma/dma-uclass.c | 23 +-
> > drivers/dma/dma330.c | 1514
> > +++++++++++++++++++++++++++++++++++
> > drivers/mtd/spi/spi_flash.c | 9 +-
> > include/dma.h | 19 +-
> > include/dma330.h | 136 ++++
> > 8 files changed, 1719 insertions(+), 12 deletions(-)
> > create mode 100644 drivers/dma/dma330.c
> > create mode 100644 include/dma330.h
> >
> I presume this is to zero-out the ECC RAM ? Just enable caches and
> use
> memset, it is much faster than this DMA witchcraft, at least on the
> A10.
Yes and no :)
The patch enable DMA-330 support and also zero-out ECC RAM.
While for the speed, DMA still faster as it has larger burst size. We
can also parallel the zero out task with other tasks such as flash
controller init.
Thanks
Chin Liang
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 0/5] Add DMA driver for DMA330 controller
2018-05-31 9:50 ` See, Chin Liang
@ 2018-05-31 9:58 ` Marek Vasut
2018-06-04 6:47 ` Chee, Tien Fong
0 siblings, 1 reply; 27+ messages in thread
From: Marek Vasut @ 2018-05-31 9:58 UTC (permalink / raw)
To: u-boot
On 05/31/2018 11:50 AM, See, Chin Liang wrote:
> On Thu, 2018-05-31 at 11:24 +0200, Marek Vasut wrote:
>> On 05/31/2018 10:08 AM, tien.fong.chee at intel.com wrote:
>>>
>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>
>>> This patchset contains dm driver for DMA330 controller.
>>>
>>> This series is working on top of u-boot-socfpga.git -
>>> http://git.denx.de/u-boot.git .
>>>
>>> Tien Fong Chee (5):
>>> drivers: dma: Enable DMA-330 driver support
>>> drivers: dma: Add function to zeroes a range of destination such
>>> as
>>> memory
>>> drivers: dma: Factor out dma_get_device from DMA class function
>>> include: dma: Update the function description for dma_memcpy
>>> arm: dts: socfpga: stratix10: update pdma
>>>
>>> arch/arm/dts/socfpga_stratix10.dtsi | 20 +
>>> drivers/dma/Kconfig | 9 +-
>>> drivers/dma/Makefile | 1 +
>>> drivers/dma/dma-uclass.c | 23 +-
>>> drivers/dma/dma330.c | 1514
>>> +++++++++++++++++++++++++++++++++++
>>> drivers/mtd/spi/spi_flash.c | 9 +-
>>> include/dma.h | 19 +-
>>> include/dma330.h | 136 ++++
>>> 8 files changed, 1719 insertions(+), 12 deletions(-)
>>> create mode 100644 drivers/dma/dma330.c
>>> create mode 100644 include/dma330.h
>>>
>> I presume this is to zero-out the ECC RAM ? Just enable caches and
>> use
>> memset, it is much faster than this DMA witchcraft, at least on the
>> A10.
>
> Yes and no :)
>
> The patch enable DMA-330 support and also zero-out ECC RAM.
Right, that's what I said ?
> While for the speed, DMA still faster as it has larger burst size.
It takes ~0.9s to erase 2 GiB of RAM on the A10. How fast is the DMA?
> We
> can also parallel the zero out task with other tasks such as flash
> controller init.
The flash controller also needs some zeroing out ?
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 1/5] drivers: dma: Enable DMA-330 driver support
2018-05-31 8:08 ` [U-Boot] [PATCH 1/5] drivers: dma: Enable DMA-330 driver support tien.fong.chee at intel.com
@ 2018-06-01 14:24 ` Simon Glass
2018-06-06 5:22 ` Chee, Tien Fong
0 siblings, 1 reply; 27+ messages in thread
From: Simon Glass @ 2018-06-01 14:24 UTC (permalink / raw)
To: u-boot
Hi Tien,
On 31 May 2018 at 02:08, <tien.fong.chee@intel.com> wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
>
> Enable DMAC driver support for DMA-330 controller.
> The driver is also compatible to PL330 product.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> ---
> drivers/dma/Kconfig | 9 +-
> drivers/dma/Makefile | 1 +
> drivers/dma/dma330.c | 1514 ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/dma330.h | 136 +++++
> 4 files changed, 1659 insertions(+), 1 deletion(-)
> create mode 100644 drivers/dma/dma330.c
> create mode 100644 include/dma330.h
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 4ee6afa..6e77e07 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -2,7 +2,7 @@ menu "DMA Support"
>
> config DMA
> bool "Enable Driver Model for DMA drivers"
> - depends on DM
> + depends on DM || SPL_DM
> help
> Enable driver model for DMA. DMA engines can do
> asynchronous data transfers without involving the host
> @@ -34,4 +34,11 @@ config APBH_DMA_BURST8
>
> endif
>
> +config DMA330_DMA
> + bool "PL330/DMA-330 DMA Controller(DMAC) driver"
> + depends on DMA
> + help
> + Enable the DMA controller driver for both PL330 and
> + DMA-330 products.
> +
> endmenu # menu "DMA Support"
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index 4eaef8a..bfad0dd 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_FSL_DMA) += fsl_dma.o
> obj-$(CONFIG_TI_KSNAV) += keystone_nav.o keystone_nav_cfg.o
> obj-$(CONFIG_TI_EDMA3) += ti-edma3.o
> obj-$(CONFIG_DMA_LPC32XX) += lpc32xx_dma.o
> +obj-$(CONFIG_DMA330_DMA) += dma330.o
> diff --git a/drivers/dma/dma330.c b/drivers/dma/dma330.c
> new file mode 100644
> index 0000000..66575d8
> --- /dev/null
> +++ b/drivers/dma/dma330.c
> @@ -0,0 +1,1514 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Intel Corporation <www.intel.com>
> + */
> +
> +#include <common.h>
> +#include <asm/io.h>
> +#include <dma330.h>
> +#include <dm.h>
> +#include <fdtdec.h>
> +#include <wait_bit.h>
> +#include <linux/unaligned/access_ok.h>
> +
> +/* Register and Bit field Definitions */
> +
> +/* DMA Status */
> +#define DS 0x0
> +#define DS_ST_STOP 0x0
> +#define DS_ST_EXEC 0x1
> +#define DS_ST_CMISS 0x2
> +#define DS_ST_UPDTPC 0x3
> +#define DS_ST_WFE 0x4
> +#define DS_ST_ATBRR 0x5
> +#define DS_ST_QBUSY 0x6
> +#define DS_ST_WFP 0x7
> +#define DS_ST_KILL 0x8
> +#define DS_ST_CMPLT 0x9
> +#define DS_ST_FLTCMP 0xe
> +#define DS_ST_FAULT 0xf
It is possible to use enum for some of these?
enum {
DS = 0,
DS_ST_STOP,
...
}
> +
> +/* DMA Program Count register */
> +#define DPC 0x4
> +/* Interrupt Enable register */
> +#define INTEN 0x20
> +/* event-Interrupt Raw Status register */
> +#define ES 0x24
> +/* Interrupt Status register */
> +#define INTSTATUS 0x28
> +/* Interrupt Clear register */
> +#define INTCLR 0x2c
> +/* Fault Status DMA Manager register */
> +#define FSM 0x30
> +/* Fault Status DMA Channel register */
> +#define FSC 0x34
> +/* Fault Type DMA Manager register */
> +#define FTM 0x38
> +
> +/* Fault Type DMA Channel register */
> +#define _FTC 0x40
> +#define FTC(n) (_FTC + (n) * 0x4)
> +
> +/* Channel Status register */
> +#define _CS 0x100
> +#define CS(n) (_CS + (n) * 0x8)
> +#define CS_CNS BIT(21)
> +
> +/* Channel Program Counter register */
> +#define _CPC 0x104
> +#define CPC(n) (_CPC + (n) * 0x8)
> +
> +/* Source Address register */
> +#define _SA 0x400
> +#define SA(n) (_SA + (n) * 0x20)
> +
> +/* Destination Address register */
> +#define _DA 0x404
> +#define DA(n) (_DA + (n) * 0x20)
> +
> +/* Channel Control register */
> +#define _CC 0x408
> +#define CC(n) (_CC + (n) * 0x20)
> +
> +/* Channel Control register (CCR) Setting */
> +#define CC_SRCINC BIT(0)
> +#define CC_DSTINC BIT(14)
> +#define CC_SRCPRI BIT(8)
> +#define CC_DSTPRI BIT(22)
> +#define CC_SRCNS BIT(9)
> +#define CC_DSTNS BIT(23)
> +#define CC_SRCIA BIT(10)
> +#define CC_DSTIA BIT(24)
> +#define CC_SRCBRSTLEN_SHFT 4
> +#define CC_DSTBRSTLEN_SHFT 18
> +#define CC_SRCBRSTSIZE_SHFT 1
> +#define CC_DSTBRSTSIZE_SHFT 15
> +#define CC_SRCCCTRL_SHFT 11
> +#define CC_SRCCCTRL_MASK 0x7
Can you make the mask a shifted mask? Then it is easier to use in
clrsetbits_le32(), for example.
e.g.
#define CC_SRCCCTRL_MASK (7 << CC_SRCCCTRL_SHFT)
> +#define CC_DSTCCTRL_SHFT 25
> +#define CC_DRCCCTRL_MASK 0x7
> +#define CC_SWAP_SHFT 28
> +
> +/* Loop Counter 0 register */
> +#define _LC0 0x40c
> +#define LC0(n) (_LC0 + (n) * 0x20)
> +
> +/* Loop Counter 1 register */
> +#define _LC1 0x410
> +#define LC1(n) (_LC1 + (n) * 0x20)
> +
> +/* Debug Status register */
> +#define DBGSTATUS 0xd00
> +#define DBG_BUSY BIT(0)
> +
> +/* Debug Command register */
> +#define DBGCMD 0xd04
> +/* Debug Instruction 0 register */
> +#define DBGINST0 0xd08
> +/* Debug Instruction 1 register */
> +#define DBGINST1 0xd0c
> +
> +/* Configuration register */
> +#define CR0 0xe00
> +#define CR1 0xe04
> +#define CR2 0xe08
> +#define CR3 0xe0c
> +#define CR4 0xe10
> +#define CRD 0xe14
> +
> +/* Peripheral Identification register */
> +#define PERIPH_ID 0xfe0
> +/* Component Identification register */
> +#define PCELL_ID 0xff0
> +
> +/* Configuration register value */
> +#define CR0_PERIPH_REQ_SET BIT(0)
> +#define CR0_BOOT_EN_SET BIT(1)
> +#define CR0_BOOT_MAN_NS BIT(2)
> +#define CR0_NUM_CHANS_SHIFT 4
> +#define CR0_NUM_CHANS_MASK 0x7
> +#define CR0_NUM_PERIPH_SHIFT 12
> +#define CR0_NUM_PERIPH_MASK 0x1f
> +#define CR0_NUM_EVENTS_SHIFT 17
> +#define CR0_NUM_EVENTS_MASK 0x1f
> +
> +/* Configuration register value */
> +#define CR1_ICACHE_LEN_SHIFT 0
> +#define CR1_ICACHE_LEN_MASK 0x7
> +#define CR1_NUM_ICACHELINES_SHIFT 4
> +#define CR1_NUM_ICACHELINES_MASK 0xf
> +
> +/* Configuration register value */
> +#define CRD_DATA_WIDTH_SHIFT 0
> +#define CRD_DATA_WIDTH_MASK 0x7
> +#define CRD_WR_CAP_SHIFT 4
> +#define CRD_WR_CAP_MASK 0x7
> +#define CRD_WR_Q_DEP_SHIFT 8
> +#define CRD_WR_Q_DEP_MASK 0xf
> +#define CRD_RD_CAP_SHIFT 12
> +#define CRD_RD_CAP_MASK 0x7
> +#define CRD_RD_Q_DEP_SHIFT 16
> +#define CRD_RD_Q_DEP_MASK 0xf
> +#define CRD_DATA_BUFF_SHIFT 20
> +#define CRD_DATA_BUFF_MASK 0x3ff
> +
> +/* Microcode opcode value */
> +#define CMD_DMAADDH 0x54
> +#define CMD_DMAEND 0x00
> +#define CMD_DMAFLUSHP 0x35
> +#define CMD_DMAGO 0xa0
> +#define CMD_DMALD 0x04
> +#define CMD_DMALDP 0x25
> +#define CMD_DMALP 0x20
> +#define CMD_DMALPEND 0x28
> +#define CMD_DMAKILL 0x01
> +#define CMD_DMAMOV 0xbc
> +#define CMD_DMANOP 0x18
> +#define CMD_DMARMB 0x12
> +#define CMD_DMASEV 0x34
> +#define CMD_DMAST 0x08
> +#define CMD_DMASTP 0x29
> +#define CMD_DMASTZ 0x0c
> +#define CMD_DMAWFE 0x36
> +#define CMD_DMAWFP 0x30
> +#define CMD_DMAWMB 0x13
> +
> +/* the size of opcode plus opcode required settings */
> +#define SZ_DMAADDH 3
> +#define SZ_DMAEND 1
> +#define SZ_DMAFLUSHP 2
> +#define SZ_DMALD 1
> +#define SZ_DMALDP 2
> +#define SZ_DMALP 2
> +#define SZ_DMALPEND 2
> +#define SZ_DMAKILL 1
> +#define SZ_DMAMOV 6
> +#define SZ_DMANOP 1
> +#define SZ_DMARMB 1
> +#define SZ_DMASEV 2
> +#define SZ_DMAST 1
> +#define SZ_DMASTP 2
> +#define SZ_DMASTZ 1
> +#define SZ_DMAWFE 2
> +#define SZ_DMAWFP 2
> +#define SZ_DMAWMB 1
> +#define SZ_DMAGO 6
> +
> +/* Use this _only_ to wait on transient states */
> +#define UNTIL(t, s) do { \
> + WATCHDOG_RESET(); \
> + } while (!(dma330_getstate(t) & (s)))
> +
> +/* debug message printout */
> +#ifdef DEBUG
> +#define DMA330_DBGCMD_DUMP(off, x...) do { \
> + printf("%x bytes:", off); \
> + printf(x); \
> + WATCHDOG_RESET(); \
> + } while (0)
> +#else
> +#define DMA330_DBGCMD_DUMP(off, x...) do {} while (0)
> +#endif
Can DMA330_DBGCMD_DUMP be a static function, and lower case? It's only
used in one file, right?
> +
> +/* Enum declaration */
We know that :-)
Can you please add comments explaining what these enums are for, and
what the values mean?
> +enum dmamov_dst {
> + SAR = 0,
> + CCR,
> + DAR,
> +};
> +
> +enum dma330_dst {
> + SRC = 0,
> + DST,
> +};
> +
> +enum dma330_cond {
> + SINGLE,
> + BURST,
> + ALWAYS,
> +};
> +
> +/* Structure will be used by _emit_lpend function */
> +struct _arg_lpend {
> + enum dma330_cond cond;
> + int forever;
> + u32 loop;
> + u8 bjump;
> +};
> +
> +/* Structure will be used by _emit_go function */
> +struct _arg_go {
Please drop the _ on these struct names, and document the members here.
> + u8 chan;
> + u32 addr;
> + u32 ns;
> +};
> +
> +/*
> + * _emit_end - Add opcode DMAEND into microcode (end).
> + *
> + * @buf: The buffer which stored the microcode program.
> + *
> + * Return: Size of opcode.
> + */
> +static inline u32 _emit_end(u8 buf[])
Why is everything inline? The compiler will do it automatically when
there is only one caller. Other than that, it just increases code size
I think.
> +{
> + buf[0] = CMD_DMAEND;
> + DMA330_DBGCMD_DUMP(SZ_DMAEND, "\tDMAEND\n");
> + return SZ_DMAEND;
> +}
> +
> +/*
> + * _emit_flushp - Add opcode DMAFLUSHP into microcode (flush peripheral).
> + *
> + * @buf -> The buffer which stored the microcode program.
> + * @peri -> Peripheral ID as listed in DMA NPP.
> + *
> + * Return: Size of opcode.
> + */
> +static inline u32 _emit_flushp(u8 buf[], u8 peri)
> +{
> + u8 *buffer = buf;
> +
> + buffer[0] = CMD_DMAFLUSHP;
> + peri &= 0x1f;
> + peri <<= 3;
These should be enums too so we don't have open-coded values in the
code, Please check throughout.
> + buffer[1] = peri;
> + DMA330_DBGCMD_DUMP(SZ_DMAFLUSHP, "\tDMAFLUSHP %u\n", peri >> 3);
> + return SZ_DMAFLUSHP;
> +}
> +
> +/**
> + * _emit_ld - Add opcode DMALD into microcode (load).
> + *
> + * @buf: The buffer which stored the microcode program.
> + * @cond: Execution criteria such as single, burst or always.
> + *
> + * Return: Size of opcode.
> + */
> +static inline u32 _emit_ld(u8 buf[], enum dma330_cond cond)
> +{
> + buf[0] = CMD_DMALD;
> + if (cond == SINGLE)
> + buf[0] |= (0 << 1) | (1 << 0);
More open-coded things
> + else if (cond == BURST)
> + buf[0] |= (1 << 1) | (1 << 0);
> + DMA330_DBGCMD_DUMP(SZ_DMALD, "\tDMALD%c\n",
> + cond == SINGLE ? 'S' : (cond == BURST ? 'B' : 'A'));
> + return SZ_DMALD;
> +}
> +
> +/**
> + * _emit_lp - Add opcode DMALP into microcode (loop).
> + *
> + * @buf: The buffer which stored the microcode program.
> + * @loop: Selection of using counter 0 or 1 (valid value 0 or 1).
> + * @cnt: number of iteration (valid value 1-256).
> + *
> + * Return: Size of opcode.
> + */
> +static inline u32 _emit_lp(u8 buf[], u32 loop, u32 cnt)
> +{
> + u8 *buffer = buf;
> +
> + buffer[0] = CMD_DMALP;
> + if (loop)
> + buffer[0] |= (1 << 1);
> + /* DMAC increments by 1 internally */
> + cnt--;
> + buffer[1] = cnt;
> + DMA330_DBGCMD_DUMP(SZ_DMALP, "\tDMALP_%c %u\n", loop ? '1' : '0', cnt);
> + return SZ_DMALP;
> +}
> +
> +/**
> + * _emit_lpend - Add opcode DMALPEND into microcode (loop end).
> + *
> + * @buf: The buffer which stored the microcode program.
> + * @arg: Structure _arg_lpend which contain all needed info.
> + * arg->cond -> Execution criteria such as single, burst or always
> + * arg->forever -> Forever loop? used if DMALPFE started the loop
> + * arg->bjump -> Backwards jump (relative location of
> + * 1st instruction in the loop.
The members of the struct should be documented in the struct above, not here.
> + *
> + * Return: Size of opcode.
> + */
> +static inline u32 _emit_lpend(u8 buf[], const struct _arg_lpend *arg)
> +{
> + u8 *buffer = buf;
> + enum dma330_cond cond = arg->cond;
> + int forever = arg->forever;
> + u32 loop = arg->loop;
> + u8 bjump = arg->bjump;
Why have these? Can you not use arg-bjump, etc., directly?
> +
> + buffer[0] = CMD_DMALPEND;
> + if (loop)
> + buffer[0] |= (1 << 2);
More open-coded things
> + if (!forever)
> + buffer[0] |= (1 << 4);
> + if (cond == SINGLE)
> + buffer[0] |= (0 << 1) | (1 << 0);
> + else if (cond == BURST)
> + buffer[0] |= (1 << 1) | (1 << 0);
> +
> + buffer[1] = bjump;
> + DMA330_DBGCMD_DUMP(SZ_DMALPEND, "\tDMALP%s%c_%c bjmpto_%x\n",
> + forever ? "FE" : "END",
> + cond == SINGLE ? 'S' : (cond == BURST ? 'B' : 'A'),
> + loop ? '1' : '0',
> + bjump);
> + return SZ_DMALPEND;
> +}
> +
> +/**
> + * _emit_kill - Add opcode DMAKILL into microcode (kill).
> + *
> + * @buf: The buffer which stored the microcode program.
> + *
> + * Return: Size of opcode.
> + */
> +static inline u32 _emit_kill(u8 buf[])
> +{
> + buf[0] = CMD_DMAKILL;
> + return SZ_DMAKILL;
> +}
> +
> +/**
> + * _emit_mov - Add opcode DMAMOV into microcode (move).
> + *
> + * @buf: The buffer which stored the microcode program.
> + * @dst: Destination register (valid value SAR[0b000], DAR[0b010],
> + * or CCR[0b001]).
> + * @val: 32bit value that to be written into destination register.
> + *
> + * Return: Size of opcode.
> + */
> +static inline u32 _emit_mov(u8 buf[], enum dmamov_dst dst, u32 val)
> +{
> + u8 *buffer = buf;
> +
> + buffer[0] = CMD_DMAMOV;
> + buffer[1] = dst;
> + buffer[2] = val & 0xFF;
> + buffer[3] = (val >> 8) & 0xFF;
> + buffer[4] = (val >> 16) & 0xFF;
> + buffer[5] = (val >> 24) & 0xFF;
Can you use put_unaligned_le32() ? Also check the same thing below.
> + DMA330_DBGCMD_DUMP(SZ_DMAMOV, "\tDMAMOV %s 0x%x\n",
> + dst == SAR ? "SAR" : (dst == DAR ? "DAR" : "CCR"),
> + val);
> + return SZ_DMAMOV;
> +}
> +
> +/**
> + * _emit_nop - Add opcode DMANOP into microcode (no operation).
> + *
> + * @buf: The buffer which stored the microcode program.
> + *
> + * Return: Size of opcode.
> + */
> +static inline u32 _emit_nop(u8 buf[])
> +{
> + buf[0] = CMD_DMANOP;
> + DMA330_DBGCMD_DUMP(SZ_DMANOP, "\tDMANOP\n");
please add a blank line before 'return'
> + return SZ_DMANOP;
> +}
> +
> +/**
> + * _emit_rmb - Add opcode DMARMB into microcode (read memory barrier).
> + *
> + * @buf: The buffer which stored the microcode program.
> + *
> + * Return: Size of opcode.
> + */
> +static inline u32 _emit_rmb(u8 buf[])
> +{
> + buf[0] = CMD_DMARMB;
> + DMA330_DBGCMD_DUMP(SZ_DMARMB, "\tDMARMB\n");
> + return SZ_DMARMB;
> +}
> +
> +/**
> + * _emit_sev - Add opcode DMASEV into microcode (send event).
> + *
> + * @buf: The buffer which stored the microcode program.
> + * @ev: The event number (valid 0 - 31).
> + *
> + * Return: Size of opcode.
> + */
> +static inline u32 _emit_sev(u8 buf[], u8 ev)
> +{
> + u8 *buffer = buf;
> +
> + buffer[0] = CMD_DMASEV;
> + ev &= 0x1f;
> + ev <<= 3;
> + buffer[1] = ev;
> + DMA330_DBGCMD_DUMP(SZ_DMASEV, "\tDMASEV %u\n", ev >> 3);
> + return SZ_DMASEV;
> +}
> +
> +/**
> + * _emit_st - Add opcode DMAST into microcode (store).
> + *
> + * @buf: The buffer which stored the microcode program.
> + * @cond: Execution criteria such as single, burst or always.
> + *
> + * Return: Size of opcode.
> + */
> +static inline u32 _emit_st(u8 buf[], enum dma330_cond cond)
> +{
> + buf[0] = CMD_DMAST;
> + if (cond == SINGLE)
> + buf[0] |= (0 << 1) | (1 << 0);
> + else if (cond == BURST)
> + buf[0] |= (1 << 1) | (1 << 0);
> +
> + DMA330_DBGCMD_DUMP(SZ_DMAST, "\tDMAST%c\n",
> + cond == SINGLE ? 'S' : (cond == BURST ? 'B' : 'A'));
> + return SZ_DMAST;
> +}
> +
> +/**
> + * _emit_stp - Add opcode DMASTP into microcode (store and notify peripheral).
> + *
> + * @buf: The buffer which stored the microcode program.
> + * @cond: Execution criteria such as single, burst or always.
> + * @peri: Peripheral ID as listed in DMA NPP.
> + *
> + * Return: Size of opcode.
> + */
> +static inline u32 _emit_stp(u8 buf[], enum dma330_cond cond, u8 peri)
> +{
> + u8 *buffer = buf;
> +
> + buffer[0] = CMD_DMASTP;
> + if (cond == BURST)
> + buf[0] |= (1 << 1);
> + peri &= 0x1f;
> + peri <<= 3;
> + buffer[1] = peri;
> + DMA330_DBGCMD_DUMP(SZ_DMASTP, "\tDMASTP%c %u\n",
> + cond == SINGLE ? 'S' : 'B', peri >> 3);
> + return SZ_DMASTP;
> +}
> +
> +/**
> + * _emit_stz - Add opcode DMASTZ into microcode (store zeros).
> + *
> + * @buf -> The buffer which stored the microcode program.
> + *
> + * Return: Size of opcode.
> + */
> +static inline u32 _emit_stz(u8 buf[])
> +{
> + buf[0] = CMD_DMASTZ;
> + DMA330_DBGCMD_DUMP(SZ_DMASTZ, "\tDMASTZ\n");
> + return SZ_DMASTZ;
> +}
> +
> +/**
> + * _emit_wfp - Add opcode DMAWFP into microcode (wait for peripheral).
> + *
> + * @buf: The buffer which stored the microcode program.
> + * @cond: Execution criteria such as single, burst or always.
> + * @peri: Peripheral ID as listed in DMA NPP.
> + *
> + * Return: Size of opcode.
> + */
> +static inline u32 _emit_wfp(u8 buf[], enum dma330_cond cond, u8 peri)
> +{
> + u8 *buffer = buf;
> +
> + buffer[0] = CMD_DMAWFP;
> + if (cond == SINGLE)
> + buffer[0] |= (0 << 1) | (0 << 0);
> + else if (cond == BURST)
> + buffer[0] |= (1 << 1) | (0 << 0);
> + else
> + buffer[0] |= (0 << 1) | (1 << 0);
Perhaps these three values are the same for each command? If so, the
three expressions (0, 1, 2) could go in an enum.
> +
> + peri &= 0x1f;
> + peri <<= 3;
> + buffer[1] = peri;
> + DMA330_DBGCMD_DUMP(SZ_DMAWFP, "\tDMAWFP%c %u\n",
> + cond == SINGLE ? 'S' : (cond == BURST ? 'B' : 'P'), peri >> 3);
> + return SZ_DMAWFP;
> +}
> +
> +/**
> + * _emit_wmb - Add opcode DMAWMB into microcode (write memory barrier).
> + *
> + * @buf: The buffer which stored the microcode program.
> + *
> + * Return: Size of opcode.
> + */
> +static inline u32 _emit_wmb(u8 buf[])
> +{
> + buf[0] = CMD_DMAWMB;
> + DMA330_DBGCMD_DUMP(SZ_DMAWMB, "\tDMAWMB\n");
> + return SZ_DMAWMB;
> +}
> +
> +/**
> + * _emit_go - Add opcode DMALGO into microcode (go).
> + *
> + * @buf: The buffer which stored the microcode program.
> + * @arg: structure _arg_go which contain all needed info
> + * arg->chan -> channel number
> + * arg->addr -> start address of the microcode program
> + * which will be wrote into CPC register
> + * arg->ns -> 1 for non secure, 0 for secure
> + * (if only DMA Manager is in secure).
> + *
> + * Return: Size of opcode.
> + */
> +static inline u32 _emit_go(u8 buf[], const struct _arg_go *arg)
> +{
> + u8 *buffer = buf;
> + u8 chan = arg->chan;
> + u32 addr = arg->addr;
> + u32 ns = arg->ns;
These local vars seem pointless as the are only used once. Maybe it's
worth it for 'addr', but even there you should use the put_unaligned
thing.
> +
> + buffer[0] = CMD_DMAGO;
> + buffer[0] |= (ns << 1);
> + buffer[1] = chan & 0x7;
> + buf[2] = addr & 0xFF;
> + buf[3] = (addr >> 8) & 0xFF;
> + buf[4] = (addr >> 16) & 0xFF;
> + buf[5] = (addr >> 24) & 0xFF;
> + return SZ_DMAGO;
> +}
> +
> +/**
> + * _prepare_ccr - Populate the CCR register.
> + * @rqc: Request Configuration.
> + *
> + * Return: Channel Control register (CCR) Setting.
> + */
> +static inline u32 _prepare_ccr(const struct dma330_reqcfg *rqc)
> +{
> + u32 ccr = 0;
> +
> + if (rqc->src_inc)
> + ccr |= CC_SRCINC;
> + if (rqc->dst_inc)
> + ccr |= CC_DSTINC;
> +
> + /* We set same protection levels for Src and DST for now */
> + if (rqc->privileged)
> + ccr |= CC_SRCPRI | CC_DSTPRI;
> + if (rqc->nonsecure)
> + ccr |= CC_SRCNS | CC_DSTNS;
> + if (rqc->insnaccess)
> + ccr |= CC_SRCIA | CC_DSTIA;
> +
> + ccr |= (((rqc->brst_len - 1) & 0xf) << CC_SRCBRSTLEN_SHFT);
> + ccr |= (((rqc->brst_len - 1) & 0xf) << CC_DSTBRSTLEN_SHFT);
> +
> + ccr |= (rqc->brst_size << CC_SRCBRSTSIZE_SHFT);
> + ccr |= (rqc->brst_size << CC_DSTBRSTSIZE_SHFT);
> +
> + ccr |= (rqc->scctl << CC_SRCCCTRL_SHFT);
> + ccr |= (rqc->dcctl << CC_DSTCCTRL_SHFT);
You don't need the outer brackets on these.
> +
> + ccr |= (rqc->swap << CC_SWAP_SHFT);
> + return ccr;
> +}
> +
> +/**
> + * dma330_until_dmac_idle - Wait until DMA Manager is idle.
> + *
> + * @plat: Pointer to struct dma_dma330_platdata.
> + * @timeout_ms: Timeout (in miliseconds).
> + *
> + * Return: Negative value for error / timeout ocurred before idle,
> + * 0 for successful.
> + */
> +static int dma330_until_dmac_idle(struct dma_dma330_platdata *plat,
> + const u32 timeout_ms)
> +{
> + void __iomem *regs = plat->base;
> +
> + return wait_for_bit(__func__,
> + (const u32 *)(uintptr_t)(regs + DBGSTATUS),
> + DBG_BUSY, 0, timeout_ms, false);
> +}
> +
> +/**
> + * dma330_execute_dbginsn - Execute debug instruction such as DMAGO and DMAKILL.
> + *
> + * @insn: The buffer which stored the debug instruction.
> + * @plat: Pointer to struct dma_dma330_platdata.
> + * @timeout_ms: Timeout (in miliseconds).
> + *
> + * Return: void.
> + */
> +static inline void dma330_execute_dbginsn(u8 insn[],
> + struct dma_dma330_platdata *plat,
> + const u32 timeout_ms)
> +{
> + void __iomem *regs = plat->base;
> + u32 val;
> +
> + val = (insn[0] << 16) | (insn[1] << 24);
> + if (!plat->dma330.channel0_manager1)
> + val |= (1 << 0);
> + val |= (plat->dma330.channel_num << 8); /* Channel Number */
> + writel(val, regs + DBGINST0);
> + val = insn[2];
> + val = val | (insn[3] << 8);
> + val = val | (insn[4] << 16);
> + val = val | (insn[5] << 24);
> + writel(val, regs + DBGINST1);
> +
> + /* If timed out due to halted state-machine */
> + if (dma330_until_dmac_idle(plat, timeout_ms)) {
> + debug("DMAC halted!\n");
> + return;
> + }
> + /* Get going */
> + writel(0, regs + DBGCMD);
> +}
> +
> +/**
> + * dma330_getstate - Get the status of current channel or manager.
> + *
> + * @plat: Pointer to struct dma_dma330_platdata.
> + *
> + * Return: Status state of current channel or current manager.
> + */
> +static inline u32 dma330_getstate(struct dma_dma330_platdata *plat)
> +{
> + void __iomem *regs = plat->base;
> + u32 val;
> +
> + if (plat->dma330.channel0_manager1)
> + val = readl((uintptr_t)(regs + DS)) & 0xf;
> + else
> + val = readl((uintptr_t)(regs + CS(plat->dma330.channel_num))) &
> + 0xf;
> +
> + switch (val) {
> + case DS_ST_STOP:
> + return DMA330_STATE_STOPPED;
> + case DS_ST_EXEC:
> + return DMA330_STATE_EXECUTING;
> + case DS_ST_CMISS:
> + return DMA330_STATE_CACHEMISS;
> + case DS_ST_UPDTPC:
> + return DMA330_STATE_UPDTPC;
> + case DS_ST_WFE:
> + return DMA330_STATE_WFE;
> + case DS_ST_FAULT:
> + return DMA330_STATE_FAULTING;
> + case DS_ST_ATBRR:
> + if (plat->dma330.channel0_manager1)
> + return DMA330_STATE_INVALID;
> + else
> + return DMA330_STATE_ATBARRIER;
> + case DS_ST_QBUSY:
> + if (plat->dma330.channel0_manager1)
> + return DMA330_STATE_INVALID;
> + else
> + return DMA330_STATE_QUEUEBUSY;
> + case DS_ST_WFP:
> + if (plat->dma330.channel0_manager1)
> + return DMA330_STATE_INVALID;
> + else
> + return DMA330_STATE_WFP;
> + case DS_ST_KILL:
> + if (plat->dma330.channel0_manager1)
> + return DMA330_STATE_INVALID;
> + else
> + return DMA330_STATE_KILLING;
> + case DS_ST_CMPLT:
> + if (plat->dma330.channel0_manager1)
> + return DMA330_STATE_INVALID;
> + else
> + return DMA330_STATE_COMPLETING;
> + case DS_ST_FLTCMP:
> + if (plat->dma330.channel0_manager1)
> + return DMA330_STATE_INVALID;
> + else
> + return DMA330_STATE_FAULT_COMPLETING;
> + default:
> + return DMA330_STATE_INVALID;
> + }
> +}
> +
> +/**
> + * dma330_trigger - Execute the DMAGO through debug instruction.
> + *
> + * @plat: Pointer to struct dma_dma330_platdata.
> + * @timeout_ms: Timeout (in miliseconds).
> + *
> + * When the DMA manager executes Go for a DMA channel that is in the Stopped
> + * state, it moves a 32-bit immediate into the program counter, then setting
> + * its security state and updating DMA channel to the Executing state.
> + *
> + * Return: Negative value for error as DMA is not ready. 0 for successful.
> + */
> +static int dma330_trigger(struct dma_dma330_platdata *plat,
> + const u32 timeout_ms)
> +{
> + u8 insn[6] = {0, 0, 0, 0, 0, 0};
> + struct _arg_go go;
> +
> + /*
> + * Return if already ACTIVE. It will ensure DMAGO is executed at
> + * STOPPED state too
> + */
> + plat->dma330.channel0_manager1 = 0;
> + if (dma330_getstate(plat) !=
> + DMA330_STATE_STOPPED)
> + return -EPERM;
> +
> + go.chan = plat->dma330.channel_num;
> + go.addr = (uintptr_t)plat->dma330.buf;
> +
> + if (!plat->dma330.secure)
> + go.ns = 1; /* non-secure condition */
> + else
> + go.ns = 0; /* secure condition */
> +
> + _emit_go(insn, &go);
> +
> + /* Only manager can execute GO */
> + plat->dma330.channel0_manager1 = 1;
> + dma330_execute_dbginsn(insn, plat, timeout_ms);
> + return 0;
> +}
> +
> +/**
> + * dma330_stop - Stop the manager or channel.
> + *
> + * @plat: Pointer to struct dma_dma330_platdata.
> + * @timeout_ms: Timeout (in miliseconds).
> + *
> + * Stop the manager/channel or killing them and ensure they reach to stop
> + * state.
> + *
> + * Return: void.
> + */
> +static void dma330_stop(struct dma_dma330_platdata *plat, const u32 timeout_ms)
> +{
> + u8 insn[6] = {0, 0, 0, 0, 0, 0};
> +
> + /* If fault completing, wait until reach faulting and killing state */
> + if (dma330_getstate(plat) == DMA330_STATE_FAULT_COMPLETING)
> + UNTIL(plat, DMA330_STATE_FAULTING | DMA330_STATE_KILLING);
> +
> + /* Return if nothing needs to be done */
> + if (dma330_getstate(plat) == DMA330_STATE_COMPLETING ||
> + dma330_getstate(plat) == DMA330_STATE_KILLING ||
> + dma330_getstate(plat) == DMA330_STATE_STOPPED)
> + return;
> +
> + /* Kill it to ensure it reach to stop state */
> + _emit_kill(insn);
> +
> + /* Execute the KILL instruction through debug registers */
> + dma330_execute_dbginsn(insn, plat, timeout_ms);
> +}
> +
> +/**
> + * dma330_start - Execute the command list through DMAGO and debug instruction.
> + *
> + * @plat: Pointer to struct dma_dma330_platdata.
> + * @timeout_ms: Timeout (in miliseconds).
> + *
> + * Return: Negative value for error where DMA is not ready. 0 for successful.
> + */
> +static int dma330_start(struct dma_dma330_platdata *plat, const u32 timeout_ms)
> +{
> + debug("INFO: DMA BASE Address = 0x%08x\n",
> + (u32)(uintptr_t)plat->base);
> +
> + switch (dma330_getstate(plat)) {
> + case DMA330_STATE_FAULT_COMPLETING:
> + UNTIL(plat, DMA330_STATE_FAULTING | DMA330_STATE_KILLING);
> +
> + if (dma330_getstate(plat) == DMA330_STATE_KILLING)
> + UNTIL(plat, DMA330_STATE_STOPPED);
> +
> + case DMA330_STATE_FAULTING:
> + dma330_stop(plat, timeout_ms);
> +
> + case DMA330_STATE_KILLING:
> + case DMA330_STATE_COMPLETING:
> + UNTIL(plat, DMA330_STATE_STOPPED);
> +
> + case DMA330_STATE_STOPPED:
> + return dma330_trigger(plat, timeout_ms);
> +
> + case DMA330_STATE_WFP:
> + case DMA330_STATE_QUEUEBUSY:
> + case DMA330_STATE_ATBARRIER:
> + case DMA330_STATE_UPDTPC:
> + case DMA330_STATE_CACHEMISS:
> + case DMA330_STATE_EXECUTING:
> + return -ESRCH;
> + /* For RESUME, nothing yet */
> + case DMA330_STATE_WFE:
> + default:
> + return -ESRCH;
> + }
> +}
> +
> +/**
> + * dma330_transfer_start - DMA start to run.
> + *
> + * @plat: Pointer to struct dma_dma330_platdata.
> + *
> + * DMA start to excecute microcode command list.
> + *
> + * Return: Negative value for error or not successful. 0 for successful.
> + */
> +static int dma330_transfer_start(struct dma_dma330_platdata *plat)
> +{
> + const u32 timeout_ms = 1000;
> +
> + /* Execute the command list */
> + return dma330_start(plat, timeout_ms);
> +}
> +
> +/**
> + * dma330_transfer_finish - DMA polling until execution finish or error.
> + *
> + * @plat: Pointer to struct dma_dma330_platdata.
> + *
> + * DMA polling until excution finish and checking the state status.
> + *
> + * Return: Negative value for state error or not successful. 0 for successful.
> + */
> +static int dma330_transfer_finish(struct dma_dma330_platdata *plat)
> +{
> + void __iomem *regs = plat->base;
> +
> + if (!plat->dma330.buf) {
> + debug("ERROR DMA330 : DMA Microcode buffer pointer is NULL\n");
> + return -EINVAL;
> + }
> +
> + plat->dma330.channel0_manager1 = 0;
> +
> + /* Wait until finish execution to ensure we compared correct result*/
> + UNTIL(plat, DMA330_STATE_STOPPED | DMA330_STATE_FAULTING);
> +
> + /* check the state */
> + if (dma330_getstate(plat) == DMA330_STATE_FAULTING) {
> + debug("FAULT Mode: Channel %d Faulting, FTR = 0x%08x,",
> + plat->dma330.channel_num,
> + readl(regs + FTC(plat->dma330.channel_num)));
> + debug("CPC = 0x%08lx\n",
> + (readl(regs + CPC(plat->dma330.channel_num))
> + - (uintptr_t)plat->dma330.buf));
> + return -EPROTO;
> + }
> + return 0;
> +}
> +
> +/**
> + * dma330_transfer_setup - DMA transfer setup (DMA_MEM_TO_MEM, DMA_MEM_TO_DEV
> + * or DMA_DEV_TO_MEM).
> + *
> + * @plat: Pointer to struct dma_dma330_platdata.
> + *
> + * For Peripheral transfer, the FIFO threshold value is expected at
> + * 2 ^ plat->brst_size * plat->brst_len.
> + *
> + * Return: Negative value for error or not successful. 0 for successful.
> + */
> +static int dma330_transfer_setup(struct dma_dma330_platdata *plat)
This function is extremely long, Can it be split into separete
sub-functions which perform the work, if the work can be split
sensibly into different parts?
> +{
> + /* Variable declaration */
> + /* Buffer offset clear to 0 */
> + int off = 0;
> + int ret = 0;
> + /* For DMALPEND */
> + u32 loopjmp0, loopjmp1;
> + /* Loop count 0 */
> + u32 lcnt0 = 0;
> + /* Loop count 1 */
> + u32 lcnt1 = 0;
> + u32 brst_size = 0;
> + u32 brst_len = 0;
> + u32 data_size_byte = plat->dma330.size_byte;
> + /* Strong order memory is required to store microcode command list */
> + u8 *buf = (u8 *)plat->dma330.buf;
> + /* Channel Control Register */
> + u32 ccr = 0;
> + struct dma330_reqcfg reqcfg;
> +
> + if (!buf) {
> + debug("ERROR DMA330 : DMA Microcode buffer pointer is NULL\n");
> + return -EINVAL;
> + }
> +
> + if (plat->dma330.transfer_type == DMA_MEM_TO_DEV)
> + debug("INFO: mem2perip");
> + else if (plat->dma330.transfer_type == DMA_DEV_TO_MEM)
> + debug("INFO: perip2mem");
> + else
> + debug("INFO: DMA_MEM_TO_MEM");
> +
> + debug(" - 0x%x%x -> 0x%x%x\nsize=%08x brst_size=2^%d ",
> + (u32)(plat->dma330.src_addr >> 32),
> + (u32)plat->dma330.src_addr,
> + (u32)(plat->dma330.dst_addr >> 32),
> + (u32)plat->dma330.dst_addr,
> + data_size_byte,
> + plat->brst_size);
> +
> + debug("brst_len=%d singles_brst_size=2^%d\n", plat->brst_len,
> + plat->dma330.single_brst_size);
> +
> + /* brst_size = 2 ^ plat->brst_size */
> + brst_size = 1 << plat->brst_size;
> +
> + /* Fool proof checking */
> + if (plat->brst_size < 0 || plat->brst_size >
> + plat->max_brst_size) {
> + debug("ERROR DMA330: brst_size must 0-%d (not %d)\n",
> + plat->max_brst_size, plat->brst_size);
> + return -EINVAL;
> + }
> + if (plat->dma330.single_brst_size < 0 ||
> + plat->dma330.single_brst_size > plat->max_brst_size) {
> + debug("ERROR DMA330 : single_brst_size must 0-%d (not %d)\n",
> + plat->max_brst_size, plat->dma330.single_brst_size);
> + return -EINVAL;
> + }
> + if (plat->brst_len < 1 || plat->brst_len > 16) {
> + debug("ERROR DMA330 : brst_len must 1-16 (not %d)\n",
> + plat->brst_len);
> + return -EINVAL;
> + }
> + if (plat->dma330.src_addr & (brst_size - 1)) {
> + debug("ERROR DMA330 : Source address unaligned\n");
> + return -EINVAL;
> + }
> + if (plat->dma330.dst_addr & (brst_size - 1)) {
> + debug("ERROR DMA330 : Destination address unaligned\n");
> + return -EINVAL;
> + }
> +
> + /* Setup the command list */
> + /* DMAMOV SAR, x->src_addr */
> + off += _emit_mov(&buf[off], SAR, plat->dma330.src_addr);
> + /* DMAMOV DAR, x->dst_addr */
> + off += _emit_mov(&buf[off], DAR, plat->dma330.dst_addr);
> + /* DMAFLUSHP P(periheral_id) */
> + if (plat->dma330.transfer_type != DMA_MEM_TO_MEM)
> + off += _emit_flushp(&buf[off], plat->dma330.peripheral_id);
> +
> + /* Preparing the CCR value */
> + if (plat->dma330.transfer_type == DMA_MEM_TO_DEV) {
> + /* Disable auto increment */
> + reqcfg.dst_inc = 0;
> + /* Enable auto increment */
> + reqcfg.src_inc = 1;
> + } else if (plat->dma330.transfer_type == DMA_DEV_TO_MEM) {
> + reqcfg.dst_inc = 1;
> + reqcfg.src_inc = 0;
> + } else {
> + /* DMA_MEM_TO_MEM */
> + reqcfg.dst_inc = 1;
> + reqcfg.src_inc = 1;
> + }
> +
> + if (!plat->dma330.secure)
> + reqcfg.nonsecure = 1; /* Non Secure mode */
> + else
> + reqcfg.nonsecure = 0; /* Secure mode */
> +
> + if (plat->dma330.enable_cache1 == 0) {
> + /* Noncacheable but bufferable */
> + reqcfg.dcctl = 0x1;
> + reqcfg.scctl = 0x1;
> + } else {
> + if (plat->dma330.transfer_type == DMA_MEM_TO_DEV) {
> + reqcfg.dcctl = 0x1;
> + /* Cacheable write back */
> + reqcfg.scctl = 0x7;
> + } else if (plat->dma330.transfer_type == DMA_DEV_TO_MEM) {
> + reqcfg.dcctl = 0x7;
> + reqcfg.scctl = 0x1;
> + } else {
> + reqcfg.dcctl = 0x7;
> + reqcfg.scctl = 0x7;
> + }
> + }
> + /* 1 - Privileged */
> + reqcfg.privileged = 1;
> + /* 0 - Data access */
> + reqcfg.insnaccess = 0;
> + /* 0 - No endian swap */
I am not keen on these comments. The struct members should be
documented when the struct is declared, not here.
> + reqcfg.swap = 0;
> + /* DMA burst length */
> + reqcfg.brst_len = plat->brst_len;
> + /* DMA burst size */
> + reqcfg.brst_size = plat->brst_size;
> + /* Preparing the CCR value */
> + ccr = _prepare_ccr(&reqcfg);
> + /* DMAMOV CCR, ccr */
> + off += _emit_mov(&buf[off], CCR, ccr);
> +
> + /* BURST */
> + /* Can initiate a burst? */
These two comments should be joined
> + while (data_size_byte >= brst_size * plat->brst_len) {
> + lcnt0 = data_size_byte / (brst_size * plat->brst_len);
> + lcnt1 = 0;
> + if (lcnt0 >= 256 * 256) {
> + lcnt0 = 256;
> + lcnt1 = 256;
> + } else if (lcnt0 >= 256) {
> + lcnt1 = lcnt0 / 256;
> + lcnt0 = 256;
> + }
> + data_size_byte = data_size_byte -
> + (brst_size * plat->brst_len * lcnt0 * lcnt1);
> +
> + debug("Transferring 0x%08x Remain 0x%08x\n", (brst_size *
> + plat->brst_len * lcnt0 * lcnt1), data_size_byte);
> + debug("Running burst - brst_size=2^%d, brst_len=%d, ",
> + plat->brst_size, plat->brst_len);
> + debug("lcnt0=%d, lcnt1=%d\n", lcnt0, lcnt1);
> +
> + if (lcnt1) {
> + /* DMALP1 */
> + off += _emit_lp(&buf[off], 1, lcnt1);
> + loopjmp1 = off;
> + }
> + /* DMALP0 */
What does this comment mean?
> + off += _emit_lp(&buf[off], 0, lcnt0);
> + loopjmp0 = off;
> + /* DMAWFP periheral_id, burst */
> + if (plat->dma330.transfer_type != DMA_MEM_TO_MEM)
> + off += _emit_wfp(&buf[off], BURST,
> + plat->dma330.peripheral_id);
> + /* DMALD */
> + off += _emit_ld(&buf[off], ALWAYS);
> +
> + WATCHDOG_RESET();
> +
> + /* DMARMB */
> + off += _emit_rmb(&buf[off]);
> + /* DMASTPB peripheral_id */
> + if (plat->dma330.transfer_type != DMA_MEM_TO_MEM)
> + off += _emit_stp(&buf[off], BURST,
> + plat->dma330.peripheral_id);
> + else
> + off += _emit_st(&buf[off], ALWAYS);
> + /* DMAWMB */
> + off += _emit_wmb(&buf[off]);
> + /* DMALP0END */
> + struct _arg_lpend lpend;
> +
> + lpend.cond = ALWAYS;
> + lpend.forever = 0;
> + /* Loop cnt 0 */
> + lpend.loop = 0;
> + lpend.bjump = off - loopjmp0;
> + off += _emit_lpend(&buf[off], &lpend);
> + /* DMALP1END */
> + if (lcnt1) {
> + struct _arg_lpend lpend;
> +
> + lpend.cond = ALWAYS;
> + lpend.forever = 0;
> + lpend.loop = 1; /* loop cnt 1*/
> + lpend.bjump = off - loopjmp1;
> + off += _emit_lpend(&buf[off], &lpend);
> + }
> + /* Ensure the microcode don't exceed buffer size */
> + if (off > plat->buf_size) {
> + debug("ERROR DMA330 : Exceed buffer size\n");
> + return -ENOMEM;
> + }
> + }
> +
> + /* SINGLE */
> + brst_len = 1;
> + /* brst_size = 2 ^ plat->dma330.single_brst_size; */
> + brst_size = (1 << plat->dma330.single_brst_size);
> + lcnt0 = data_size_byte / (brst_size * brst_len);
> +
> + /* Ensure all data will be transferred */
> + data_size_byte = data_size_byte -
> + (brst_size * brst_len * lcnt0);
> + if (data_size_byte) {
> + debug("ERROR DMA330 : Detected the possibility of ");
> + debug("untransfered data. Please ensure correct single ");
> + debug("burst size\n");
> + }
> +
> + if (lcnt0) {
> + debug("Transferring 0x%08x Remain 0x%08x\n", (brst_size *
> + brst_len * lcnt0 * lcnt1), data_size_byte);
> + debug("Running single - brst_size=2^%d, brst_len=%d, ",
> + brst_size, brst_len);
> + debug("lcnt0=%d\n", lcnt0);
> +
> + /* Preparing the CCR value */
> + /* DMA burst length */
> + reqcfg.brst_len = brst_len;
> + /* DMA burst size */
> + reqcfg.brst_size = brst_size;
> + ccr = _prepare_ccr(&reqcfg);
> + /* DMAMOV CCR, ccr */
> + off += _emit_mov(&buf[off], CCR, ccr);
> +
> + /* DMALP0 */
> + off += _emit_lp(&buf[off], 0, lcnt0);
> + loopjmp0 = off;
> + /* DMAWFP peripheral_id, single */
> + if (plat->dma330.transfer_type != DMA_MEM_TO_MEM)
> + off += _emit_wfp(&buf[off], SINGLE,
> + plat->dma330.peripheral_id);
> +
> + /* DMALD */
> + off += _emit_ld(&buf[off], ALWAYS);
> + /* DMARMB */
> + off += _emit_rmb(&buf[off]);
> + /* DMASTPS peripheral_id */
> + if (plat->dma330.transfer_type != DMA_MEM_TO_MEM)
> + off += _emit_stp(&buf[off], SINGLE,
> + plat->dma330.peripheral_id);
> + else
> + off += _emit_st(&buf[off], ALWAYS);
> +
> + /* DMAWMB */
> + off += _emit_wmb(&buf[off]);
> + /* DMALPEND */
> + struct _arg_lpend lpend1;
> +
> + lpend1.cond = ALWAYS;
> + lpend1.forever = 0;
> + /* loop cnt 0 */
> + lpend1.loop = 0;
> + lpend1.bjump = off - loopjmp0;
> + off += _emit_lpend(&buf[off], &lpend1);
> + /* Ensure the microcode don't exceed buffer size */
> + if (off > plat->buf_size) {
> + puts("ERROR DMA330 : Exceed buffer size\n");
> + return -ENOMEM;
> + }
> + }
> +
> + /* DMAEND */
> + off += _emit_end(&buf[off]);
> +
> + ret = dma330_transfer_start(plat);
> + if (ret)
> + return ret;
> +
> + return dma330_transfer_finish(plat);
> +}
> +
> +/**
> + * dma330_transfer_zeroes - DMA transfer zeros.
> + *
> + * @plat: Pointer to struct dma_dma330_platdata.
> + *
> + * Used to write zeros to a memory chunk for memory scrubbing purpose.
> + *
> + * Return: Negative value for error or not successful. 0 for successful.
> + */
> +static int dma330_transfer_zeroes_setup(struct dma_dma330_platdata *plat)
Can this function be combined with the above somehow? It seems like a
lot of duplicate code.
> +{
> + /* Variable declaration */
> + /* Buffer offset clear to 0 */
> + int off = 0;
> + int ret = 0;
> + /* For DMALPEND */
> + u32 loopjmp0, loopjmp1;
> + /* Loop count 0 */
> + u32 lcnt0 = 0;
> + /* Loop count 1 */
> + u32 lcnt1 = 0;
> + u32 brst_size = 0;
> + u32 brst_len = 0;
> + u32 data_size_byte = plat->dma330.size_byte;
> + /* Strong order memory is required to store microcode command list */
> + u8 *buf = (u8 *)plat->dma330.buf;
> + /* Channel Control Register */
> + u32 ccr = 0;
> + struct dma330_reqcfg reqcfg;
> +
> + if (!buf) {
> + debug("ERROR DMA330 : DMA Microcode buffer pointer is NULL\n");
> + return -EINVAL;
> + }
> +
> + debug("INFO: Write zeros -> ");
> + debug("0x%x%x size=0x%08x\n",
> + (u32)(plat->dma330.dst_addr >> 32),
> + (u32)plat->dma330.dst_addr,
> + data_size_byte);
> +
> + plat->dma330.single_brst_size = 1;
> +
> + /* burst_size = 2 ^ plat->brst_size */
> + brst_size = 1 << plat->brst_size;
> +
> + /* Setup the command list */
> + /* DMAMOV DAR, x->dst_addr */
> + off += _emit_mov(&buf[off], DAR, plat->dma330.dst_addr);
> +
> + /* Preparing the CCR value */
> + /* Enable auto increment */
> + reqcfg.dst_inc = 1;
> + /* Disable auto increment (not applicable) */
> + reqcfg.src_inc = 0;
> + /* Noncacheable but bufferable */
> + reqcfg.dcctl = 0x1;
> + /* Noncacheable and bufferable */
> + reqcfg.scctl = 0x1;
> + /* 1 - Privileged */
> + reqcfg.privileged = 1;
> + /* 0 - Data access */
> + reqcfg.insnaccess = 0;
> + /* 0 - No endian swap */
> + reqcfg.swap = 0;
> +
> + if (!plat->dma330.secure)
> + reqcfg.nonsecure = 1; /* Non Secure mode */
> + else
> + reqcfg.nonsecure = 0; /* Secure mode */
> +
> + /* DMA burst length */
> + reqcfg.brst_len = plat->brst_len;
> + /* DMA burst size */
> + reqcfg.brst_size = plat->brst_size;
> + /* Preparing the CCR value */
> + ccr = _prepare_ccr(&reqcfg);
> + /* DMAMOV CCR, ccr */
> + off += _emit_mov(&buf[off], CCR, ccr);
> +
> + /* BURST */
> + /* Can initiate a burst? */
> + while (data_size_byte >= brst_size * plat->brst_len) {
> + lcnt0 = data_size_byte / (brst_size * plat->brst_len);
> + lcnt1 = 0;
> + if (lcnt0 >= 256 * 256) {
> + lcnt0 = 256;
> + lcnt1 = 256;
> + } else if (lcnt0 >= 256) {
> + lcnt1 = lcnt0 / 256;
> + lcnt0 = 256;
> + }
> +
> + data_size_byte = data_size_byte -
> + (brst_size * plat->brst_len * lcnt0 * lcnt1);
> +
> + debug("Transferring 0x%08x Remain 0x%08x\n", (brst_size *
> + plat->brst_len * lcnt0 * lcnt1), data_size_byte);
> + debug("Running burst - brst_size=2^%d, brst_len=%d,",
> + plat->brst_size, plat->brst_len);
> + debug("lcnt0=%d, lcnt1=%d\n", lcnt0, lcnt1);
> +
> + if (lcnt1) {
> + /* DMALP1 */
> + off += _emit_lp(&buf[off], 1, lcnt1);
> + loopjmp1 = off;
> + }
> + /* DMALP0 */
> + off += _emit_lp(&buf[off], 0, lcnt0);
> + loopjmp0 = off;
> + /* DMALSTZ */
> + off += _emit_stz(&buf[off]);
> +
> + WATCHDOG_RESET();
> +
> + /* DMALP0END */
> + struct _arg_lpend lpend;
> +
> + lpend.cond = ALWAYS;
> + lpend.forever = 0;
> + /* Loop cnt 0 */
> + lpend.loop = 0;
> + lpend.bjump = off - loopjmp0;
> + off += _emit_lpend(&buf[off], &lpend);
> + /* DMALP1END */
> + if (lcnt1) {
> + struct _arg_lpend lpend;
> +
> + lpend.cond = ALWAYS;
> + lpend.forever = 0;
> + /* Loop cnt 1*/
> + lpend.loop = 1;
> + lpend.bjump = off - loopjmp1;
> + off += _emit_lpend(&buf[off], &lpend);
> + }
> + /* Ensure the microcode don't exceed buffer size */
> + if (off > plat->buf_size) {
> + printf("off = %d\n", off);
> + debug("ERROR DMA330 : Exceed buffer size off %d\n",
> + off);
> + return -ENOMEM;
> + }
> + }
> +
> + /* SINGLE */
> + brst_len = 1;
> + /* brst_size = 2 ^ plat->dma330.single_brst_size */
> + brst_size = (1 << plat->dma330.single_brst_size);
> + lcnt0 = data_size_byte / (brst_size * brst_len);
> +
> + /* ensure all data will be transferred */
> + data_size_byte = data_size_byte -
> + (brst_size * brst_len * lcnt0);
> + if (data_size_byte) {
> + debug("ERROR DMA330 : Detected the possibility of ");
> + debug("untransfered data. Please ensure correct single ");
> + debug("burst size\n");
> + }
> +
> + if (lcnt0) {
> + debug("Transferring 0x%08x Remain 0x%08x\n", (brst_size *
> + brst_len * lcnt0), data_size_byte);
> + debug("Running single - brst_size=2^%d, brst_len=%d, ",
> + brst_size, brst_len);
> + debug("lcnt0=%d\n", lcnt0);
> +
> + /* Preparing the CCR value */
> + /* DMA burst length */
> + reqcfg.brst_len = brst_len;
> + /* DMA burst size */
> + reqcfg.brst_size = brst_size;
> + ccr = _prepare_ccr(&reqcfg);
> + /* DMAMOV CCR, ccr */
> + off += _emit_mov(&buf[off], CCR, ccr);
> +
> + /* DMALP0 */
> + off += _emit_lp(&buf[off], 0, lcnt0);
> + loopjmp0 = off;
> + /* DMALSTZ */
> + off += _emit_stz(&buf[off]);
> + /* DMALPEND */
> + struct _arg_lpend lpend1;
> +
> + lpend1.cond = ALWAYS;
> + lpend1.forever = 0;
> + /* Loop cnt 0 */
> + lpend1.loop = 0;
> + lpend1.bjump = off - loopjmp0;
> + off += _emit_lpend(&buf[off], &lpend1);
> + /* Ensure the microcode don't exceed buffer size */
> + if (off > plat->buf_size) {
> + debug("ERROR DMA330 : Exceed buffer size\n");
> + return -ENOMEM;
> + }
> + }
> +
> + /* DMAEND */
> + off += _emit_end(&buf[off]);
> +
> + ret = dma330_transfer_start(plat);
> + if (ret)
> + return ret;
> +
> + return dma330_transfer_finish(plat);
> +}
> +
> +static int dma330_transfer_zeroes(struct udevice *dev, void *dst, size_t len)
> +{
> + struct dma_dma330_platdata *plat = dev->platdata;
> + int ret = 0;
> +
> + /* If DMA access is set to secure, base change to DMA secure base */
> + if (plat->dma330.secure)
> + plat->base += 0x1000;
What is this? I suggest you set up the secure base separately in
'plat', e.g. with a base_secure member. You should not change platform
data while running.
You can pass the base address to dma330_transfer_zeroes_setup, perhaps.
> +
> + plat->dma330.dst_addr = (phys_size_t)(uintptr_t)dst;
> + plat->dma330.size_byte = len;
> +
> + ret = dma330_transfer_zeroes_setup(plat);
> +
> + /* Revert back to non secure DMA base */
> + if (plat->dma330.secure)
> + plat->base -= 0x1000;
> +
> + return ret;
> +}
> +
> +static int dma330_transfer(struct udevice *dev, int direction, void *dst,
> + void *src, size_t len)
> +{
> + struct dma_dma330_platdata *plat = dev->platdata;
> + int ret = 0;
> +
> + /* If DMA access is set to secure, base change to DMA secure base */
> + if (plat->dma330.secure)
> + plat->base += 0x1000;
Same as above
> +
> + plat->dma330.dst_addr = (phys_size_t)(uintptr_t)dst;
> + plat->dma330.src_addr = (phys_size_t)(uintptr_t)src;
> + plat->dma330.size_byte = len;
> +
> + switch (direction) {
> + case DMA_MEM_TO_MEM:
> + plat->dma330.transfer_type = DMA_SUPPORTS_MEM_TO_MEM;
> + break;
> + case DMA_MEM_TO_DEV:
> + plat->dma330.transfer_type = DMA_SUPPORTS_MEM_TO_DEV;
> + break;
> + case DMA_DEV_TO_MEM:
> + plat->dma330.transfer_type = DMA_SUPPORTS_DEV_TO_MEM;
> + break;
> + }
> +
> + ret = dma330_transfer_setup(plat);
> +
> + /* Revert back to non secure DMA base */
> + if (plat->dma330.secure)
> + plat->base -= 0x1000;
> +
> + return ret;
> +}
> +
> +static int dma330_ofdata_to_platdata(struct udevice *dev)
> +{
> + struct dma_dma330_platdata *plat = dev->platdata;
> + const void *blob = gd->fdt_blob;
> + int node = dev_of_offset(dev);
> +
> + plat->base = (void __iomem *)devfdt_get_addr(dev);
dev_read_addr()
> + plat->max_brst_size = fdtdec_get_uint(blob, node, "dma-max-burst-size",
> + 2);
Use dev_read API please (live tree)
> + plat->brst_size = fdtdec_get_uint(blob, node, "dma-burst-size", 2);
> + plat->brst_len = fdtdec_get_uint(blob, node, "dma-burst-length", 2);
> +
> + return 0;
> +}
> +
> +static int dma330_probe(struct udevice *adev)
> +{
> + struct dma_dev_priv *uc_priv = dev_get_uclass_priv(adev);
> +
> + uc_priv->supported = (DMA_SUPPORTS_MEM_TO_MEM |
> + DMA_SUPPORTS_MEM_TO_DEV |
> + DMA_SUPPORTS_DEV_TO_MEM);
> + return 0;
> +}
> +
> +static const struct dma_ops dma330_ops = {
> + .transfer = dma330_transfer,
> + .transfer_zeroes = dma330_transfer_zeroes,
> +};
> +
> +static const struct udevice_id dma330_ids[] = {
> + { .compatible = "arm,pl330" },
> + { .compatible = "arm,dma330" },
> + { /* sentinel */ }
> +};
> +
> +U_BOOT_DRIVER(dma_dma330) = {
> + .name = "dma_dma330",
> + .id = UCLASS_DMA,
> + .of_match = dma330_ids,
> + .ops = &dma330_ops,
> + .ofdata_to_platdata = dma330_ofdata_to_platdata,
> + .probe = dma330_probe,
> + .platdata_auto_alloc_size = sizeof(struct dma_dma330_platdata),
> +};
> diff --git a/include/dma330.h b/include/dma330.h
> new file mode 100644
> index 0000000..c054bf2
> --- /dev/null
> +++ b/include/dma330.h
> @@ -0,0 +1,136 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2018 Intel Corporation <www.intel.com>
> + */
> +
> +#include <dma.h>
> +#include <linux/sizes.h>
> +
> +#ifndef __DMA330_CORE_H
> +#define __DMA330_CORE_H
> +
> +#define DMA330_MAX_CHAN 8
> +#define DMA330_MAX_IRQS 32
> +#define DMA330_MAX_PERI 32
> +
> +#define DMA330_STATE_STOPPED BIT(0)
> +#define DMA330_STATE_EXECUTING BIT(1)
> +#define DMA330_STATE_WFE BIT(2)
> +#define DMA330_STATE_FAULTING BIT(3)
> +#define DMA330_STATE_COMPLETING BIT(4)
> +#define DMA330_STATE_WFP BIT(5)
> +#define DMA330_STATE_KILLING BIT(6)
> +#define DMA330_STATE_FAULT_COMPLETING BIT(7)
> +#define DMA330_STATE_CACHEMISS BIT(8)
> +#define DMA330_STATE_UPDTPC BIT(9)
> +#define DMA330_STATE_ATBARRIER BIT(10)
> +#define DMA330_STATE_QUEUEBUSY BIT(11))
> +#define DMA330_STATE_INVALID BIT(15)
> +
> +enum dma330_srccachectrl {
> + SCCTRL0 = 0, /* Noncacheable and nonbufferable */
> + SCCTRL1, /* Bufferable only */
> + SCCTRL2, /* Cacheable, but do not allocate */
> + SCCTRL3, /* Cacheable and bufferable, but do not allocate */
> + SINVALID1,
> + SINVALID2,
> + SCCTRL6, /* Cacheable write-through, allocate on reads only */
> + SCCTRL7, /* Cacheable write-back, allocate on reads only */
> +};
> +
> +enum dma330_dstcachectrl {
> + DCCTRL0 = 0, /* Noncacheable and nonbufferable */
> + DCCTRL1, /* Bufferable only */
> + DCCTRL2, /* Cacheable, but do not allocate */
> + DCCTRL3, /* Cacheable and bufferable, but do not allocate */
> + DINVALID1 = 8,
> + DINVALID2,
> + DCCTRL6, /* Cacheable write-through, allocate on writes only */
> + DCCTRL7, /* Cacheable write-back, allocate on writes only */
> +};
> +
> +enum dma330_byteswap {
> + SWAP_NO = 0,
> + SWAP_2,
> + SWAP_4,
> + SWAP_8,
> + SWAP_16,
> +};
> +
> +/**
> + * dma330_reqcfg - Request Configuration.
> + *
> + * The DMA330 core does not modify this and uses the last
> + * working configuration if the request doesn't provide any.
> + *
> + * The Client may want to provide this info only for the
> + * first request and a request with new settings.
> + */
> +struct dma330_reqcfg {
> + /* Address Incrementing */
> + u8 dst_inc;
> + u8 src_inc;
> +
> + /*
> + * For now, the SRC & DST protection levels
> + * and burst size/length are assumed same.
> + */
> + u8 nonsecure;
> + u8 privileged;
> + u8 insnaccess;
> + u8 brst_len;
> + u8 brst_size; /* in power of 2 */
> +
> + enum dma330_dstcachectrl dcctl;
> + enum dma330_srccachectrl scctl;
> + enum dma330_byteswap swap;
> +};
> +
> +/**
> + * dma330_transfer_struct - Structure to be passed in for dma330_transfer_x.
> + *
> + * @channel0_manager1: Switching configuration on DMA channel or DMA manager.
> + * @channel_num: Channel number assigned, valid from 0 to 7.
> + * @src_addr: Address to transfer from / source.
> + * @dst_addr: Address to transfer to / destination.
> + * @size_byte: Number of bytes to be transferred.
> + * @brst_size: Valid from 0 - 4,
> + * where 0 = 1 (2 ^ 0) bytes and 3 = 16 bytes (2 ^ 4).
> + * @max_brst_size: Max transfer size (from 0 - 4).
> + * @single_brst_size: Single transfer size (from 0 - 4).
> + * @brst_len: Valid from 1 - 16 where each burst can transfer 1 - 16
> + * Data chunk (each chunk size equivalent to brst_size).
> + * @peripheral_id: Assigned peripheral_id, valid from 0 to 31.
> + * @transfer_type: MEM2MEM, MEM2PERIPH or PERIPH2MEM.
> + * @enable_cache1: 1 for cache enabled for memory
> + * (cacheable and bufferable, but do not allocate).
> + * @buf: Buffer handler which will point to the memory
> + * allocated for dma microcode
> + * @secure: Set DMA channel in secure mode.
This is a good way to document structs. Please do it for the others.
> + *
> + * Description of the structure.
Yes?
> + */
> +struct dma330_transfer_struct {
> + u32 channel0_manager1;
> + u32 channel_num;
> + phys_size_t src_addr;
> + phys_size_t dst_addr;
> + u32 size_byte;
> + u32 single_brst_size;
> + u32 peripheral_id;
> + u32 transfer_type;
> + u32 enable_cache1;
> + u32 *buf;
> + u8 secure;
> +};
> +
> +struct dma_dma330_platdata {
> + void __iomem *base;
> + u32 buf_size;
> + u32 max_brst_size;
> + u32 brst_size;
> + u32 brst_len;
> + struct dma330_transfer_struct dma330;
> +};
> +
> +#endif /* __DMA330_CORE_H */
> --
> 2.2.0
>
Regards,
Simon
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/5] drivers: dma: Add function to zeroes a range of destination such as memory
2018-05-31 8:08 ` [U-Boot] [PATCH 2/5] drivers: dma: Add function to zeroes a range of destination such as memory tien.fong.chee at intel.com
@ 2018-06-01 14:24 ` Simon Glass
2018-06-04 7:34 ` Chee, Tien Fong
0 siblings, 1 reply; 27+ messages in thread
From: Simon Glass @ 2018-06-01 14:24 UTC (permalink / raw)
To: u-boot
Hi Tien,
On 31 May 2018 at 02:08, <tien.fong.chee@intel.com> wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
>
> This new DMA class function enables DMA being used for initializing
> a range of destination such as memory to zeros. This is quite useful to
> help accelerating the performance in scrubbing memory when ECC is enabled.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> ---
> drivers/dma/dma-uclass.c | 15 +++++++++++++++
> include/dma.h | 12 ++++++++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/drivers/dma/dma-uclass.c b/drivers/dma/dma-uclass.c
> index a33f7d5..cb83c24 100644
> --- a/drivers/dma/dma-uclass.c
> +++ b/drivers/dma/dma-uclass.c
> @@ -61,6 +61,21 @@ int dma_memcpy(void *dst, void *src, size_t len)
> return ops->transfer(dev, DMA_MEM_TO_MEM, dst, src, len);
> }
>
> +int dma_memcpy_zeroes(struct udevice *dev, void *dst, size_t len)
> +{
> + const struct dma_ops *ops;
> +
> + ops = device_get_ops(dev);
> + if (!ops->transfer_zeroes)
> + return -ENOSYS;
> +
> + /* Invalidate the area, so no writeback into the RAM races with DMA */
> + invalidate_dcache_range((unsigned long)dst, (unsigned long)dst +
> + roundup(len, ARCH_DMA_MINALIGN));
> +
> + return ops->transfer_zeroes(dev, dst, len);
> +}
> +
> UCLASS_DRIVER(dma) = {
> .id = UCLASS_DMA,
> .name = "dma",
> diff --git a/include/dma.h b/include/dma.h
> index 50e9652..6bad2264 100644
> --- a/include/dma.h
> +++ b/include/dma.h
> @@ -46,6 +46,7 @@ struct dma_ops {
> */
> int (*transfer)(struct udevice *dev, int direction, void *dst,
> void *src, size_t len);
> + int (*transfer_zeroes)(struct udevice *dev, void *dst, size_t len);
I wonder if this could be done by using transfer() with a src of NULL ?
> };
>
> /*
> @@ -82,4 +83,15 @@ int dma_get_device(u32 transfer_type, struct udevice **devp);
> */
> int dma_memcpy(void *dst, void *src, size_t len);
>
> +/*
> + * dma_memcpy_zeroes - Fill up destination with zeros through DMA.
> + *
> + * @dev: The DMA device
> + * @dst: destination pointer
> + * @len: length to be copied with zero
> + * @return: on successful transfer returns zero.
> + * on failure returns error code.
> + */
> +int dma_memcpy_zeroes(struct udevice *dev, void *dst, size_t len);
> +
> #endif /* _DMA_H_ */
> --
> 2.2.0
>
Regards,
Simon
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 3/5] drivers: dma: Factor out dma_get_device from DMA class function
2018-05-31 8:08 ` [U-Boot] [PATCH 3/5] drivers: dma: Factor out dma_get_device from DMA class function tien.fong.chee at intel.com
@ 2018-06-01 14:25 ` Simon Glass
0 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2018-06-01 14:25 UTC (permalink / raw)
To: u-boot
On 31 May 2018 at 02:08, <tien.fong.chee@intel.com> wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
>
> Factor out the dma_get_device from DMA class function so caller can
> set some configuration and changes on the DMA device structure which
> is return by calling dma_get_device before device instance is processed by
> DMA class functions.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> ---
> drivers/dma/dma-uclass.c | 8 +-------
> drivers/mtd/spi/spi_flash.c | 9 ++++++++-
> include/dma.h | 3 ++-
> 3 files changed, 11 insertions(+), 9 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 4/5] include: dma: Update the function description for dma_memcpy
2018-05-31 8:08 ` [U-Boot] [PATCH 4/5] include: dma: Update the function description for dma_memcpy tien.fong.chee at intel.com
@ 2018-06-01 14:25 ` Simon Glass
2018-06-04 7:14 ` Chee, Tien Fong
0 siblings, 1 reply; 27+ messages in thread
From: Simon Glass @ 2018-06-01 14:25 UTC (permalink / raw)
To: u-boot
Hi,
On 31 May 2018 at 02:08, <tien.fong.chee@intel.com> wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
>
> Update the dma_memcpy description on return argument for DMA330 driver.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> ---
> include/dma.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/dma.h b/include/dma.h
> index 0a0c9dd..b825c1e 100644
> --- a/include/dma.h
> +++ b/include/dma.h
> @@ -79,8 +79,8 @@ int dma_get_device(u32 transfer_type, struct udevice **devp);
> * @dst - destination pointer
> * @src - souce pointer
> * @len - data length to be copied
> - * @return - on successful transfer returns no of bytes
> - transferred and on failure return error code.
> + * @return - on successful transfer returns no of bytes or zero(for DMA330)
> + * transferred and on failure return error code.
This is a public API so you cannot change it just for one device. You
can change the API for everyone if you like.
But why would it want to return 0?
> */
> int dma_memcpy(struct udevice *dev, void *dst, void *src, size_t len);
>
> --
> 2.2.0
>
Regards,
Simon
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 5/5] arm: dts: socfpga: stratix10: update pdma
2018-05-31 8:08 ` [U-Boot] [PATCH 5/5] arm: dts: socfpga: stratix10: update pdma tien.fong.chee at intel.com
@ 2018-06-01 14:25 ` Simon Glass
2018-06-04 7:01 ` Chee, Tien Fong
2018-07-09 18:03 ` Dinh Nguyen
1 sibling, 1 reply; 27+ messages in thread
From: Simon Glass @ 2018-06-01 14:25 UTC (permalink / raw)
To: u-boot
On 31 May 2018 at 02:08, <tien.fong.chee@intel.com> wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
>
> Update pdma properties for Stratix 10
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> ---
> arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
Reviewed-by: Simon Glass <sjg@chromium.org>
Is there a DT binding file for this somewhere?
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 0/5] Add DMA driver for DMA330 controller
2018-05-31 9:58 ` Marek Vasut
@ 2018-06-04 6:47 ` Chee, Tien Fong
0 siblings, 0 replies; 27+ messages in thread
From: Chee, Tien Fong @ 2018-06-04 6:47 UTC (permalink / raw)
To: u-boot
On Thu, 2018-05-31 at 11:58 +0200, Marek Vasut wrote:
> On 05/31/2018 11:50 AM, See, Chin Liang wrote:
> >
> > On Thu, 2018-05-31 at 11:24 +0200, Marek Vasut wrote:
> > >
> > > On 05/31/2018 10:08 AM, tien.fong.chee at intel.com wrote:
> > > >
> > > >
> > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > >
> > > > This patchset contains dm driver for DMA330 controller.
> > > >
> > > > This series is working on top of u-boot-socfpga.git -
> > > > http://git.denx.de/u-boot.git .
> > > >
> > > > Tien Fong Chee (5):
> > > > drivers: dma: Enable DMA-330 driver support
> > > > drivers: dma: Add function to zeroes a range of destination
> > > > such
> > > > as
> > > > memory
> > > > drivers: dma: Factor out dma_get_device from DMA class
> > > > function
> > > > include: dma: Update the function description for dma_memcpy
> > > > arm: dts: socfpga: stratix10: update pdma
> > > >
> > > > arch/arm/dts/socfpga_stratix10.dtsi | 20 +
> > > > drivers/dma/Kconfig | 9 +-
> > > > drivers/dma/Makefile | 1 +
> > > > drivers/dma/dma-uclass.c | 23 +-
> > > > drivers/dma/dma330.c | 1514
> > > > +++++++++++++++++++++++++++++++++++
> > > > drivers/mtd/spi/spi_flash.c | 9 +-
> > > > include/dma.h | 19 +-
> > > > include/dma330.h | 136 ++++
> > > > 8 files changed, 1719 insertions(+), 12 deletions(-)
> > > > create mode 100644 drivers/dma/dma330.c
> > > > create mode 100644 include/dma330.h
> > > >
> > > I presume this is to zero-out the ECC RAM ? Just enable caches
> > > and
> > > use
> > > memset, it is much faster than this DMA witchcraft, at least on
> > > the
> > > A10.
> > Yes and no :)
> >
> > The patch enable DMA-330 support and also zero-out ECC RAM.
> Right, that's what I said ?
>
This driver also enable source to destination transferring through DMA.
> >
> > While for the speed, DMA still faster as it has larger burst size.
> It takes ~0.9s to erase 2 GiB of RAM on the A10. How fast is the DMA?
>
That is pretty fast. Using DMA dm on Stratix 10 also need around 3-4s
for 2GiB DRAM.
Would you mind to share us the codes for enabling the both cache and
MMU in SPL. I would like to replicate your changes at Stratix 10 for
the performance measurement.
> >
> > We
> > can also parallel the zero out task with other tasks such as flash
> > controller init.
> The flash controller also needs some zeroing out ?
>
I believe what Chin Liang saying is CPU is free to do other HW
initialization while DMA is zeroing the DRAM.
However, current driver is blocking, which means CPU has to wait the
DMA finish transferring.
I can pull the DMA transfer status check function out as a new class
function so user can check the DMA status at anytime.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 5/5] arm: dts: socfpga: stratix10: update pdma
2018-06-01 14:25 ` Simon Glass
@ 2018-06-04 7:01 ` Chee, Tien Fong
0 siblings, 0 replies; 27+ messages in thread
From: Chee, Tien Fong @ 2018-06-04 7:01 UTC (permalink / raw)
To: u-boot
On Fri, 2018-06-01 at 08:25 -0600, Simon Glass wrote:
> On 31 May 2018 at 02:08, <tien.fong.chee@intel.com> wrote:
> >
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> >
> > Update pdma properties for Stratix 10
> >
> > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > ---
> > arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Is there a DT binding file for this somewhere?
Yeah, it is in linux/Documentation
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/
bindings/dma/arm-pl330.txt
I can port over to U-boot.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 4/5] include: dma: Update the function description for dma_memcpy
2018-06-01 14:25 ` Simon Glass
@ 2018-06-04 7:14 ` Chee, Tien Fong
2018-06-08 21:59 ` Simon Glass
0 siblings, 1 reply; 27+ messages in thread
From: Chee, Tien Fong @ 2018-06-04 7:14 UTC (permalink / raw)
To: u-boot
On Fri, 2018-06-01 at 08:25 -0600, Simon Glass wrote:
> Hi,
>
> On 31 May 2018 at 02:08, <tien.fong.chee@intel.com> wrote:
> >
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> >
> > Update the dma_memcpy description on return argument for DMA330
> > driver.
> >
> > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > ---
> > include/dma.h | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/dma.h b/include/dma.h
> > index 0a0c9dd..b825c1e 100644
> > --- a/include/dma.h
> > +++ b/include/dma.h
> > @@ -79,8 +79,8 @@ int dma_get_device(u32 transfer_type, struct
> > udevice **devp);
> > * @dst - destination pointer
> > * @src - souce pointer
> > * @len - data length to be copied
> > - * @return - on successful transfer returns no of bytes
> > - transferred and on failure return error code.
> > + * @return - on successful transfer returns no of bytes or
> > zero(for DMA330)
> > + * transferred and on failure return error code.
> This is a public API so you cannot change it just for one device. You
> can change the API for everyone if you like.
>
> But why would it want to return 0?
>
Because we only able to check the DMA tranferring status, full transfer
or failed. I can return the len argument user set if full tranfer is
finished.
> >
> > */
> > int dma_memcpy(struct udevice *dev, void *dst, void *src, size_t
> > len);
> >
> > --
> > 2.2.0
> >
> Regards,
> Simon
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/5] drivers: dma: Add function to zeroes a range of destination such as memory
2018-06-01 14:24 ` Simon Glass
@ 2018-06-04 7:34 ` Chee, Tien Fong
0 siblings, 0 replies; 27+ messages in thread
From: Chee, Tien Fong @ 2018-06-04 7:34 UTC (permalink / raw)
To: u-boot
On Fri, 2018-06-01 at 08:24 -0600, Simon Glass wrote:
> Hi Tien,
>
> On 31 May 2018 at 02:08, <tien.fong.chee@intel.com> wrote:
> >
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> >
> > This new DMA class function enables DMA being used for initializing
> > a range of destination such as memory to zeros. This is quite
> > useful to
> > help accelerating the performance in scrubbing memory when ECC is
> > enabled.
> >
> > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > ---
> > drivers/dma/dma-uclass.c | 15 +++++++++++++++
> > include/dma.h | 12 ++++++++++++
> > 2 files changed, 27 insertions(+)
> >
> > diff --git a/drivers/dma/dma-uclass.c b/drivers/dma/dma-uclass.c
> > index a33f7d5..cb83c24 100644
> > --- a/drivers/dma/dma-uclass.c
> > +++ b/drivers/dma/dma-uclass.c
> > @@ -61,6 +61,21 @@ int dma_memcpy(void *dst, void *src, size_t len)
> > return ops->transfer(dev, DMA_MEM_TO_MEM, dst, src, len);
> > }
> >
> > +int dma_memcpy_zeroes(struct udevice *dev, void *dst, size_t len)
> > +{
> > + const struct dma_ops *ops;
> > +
> > + ops = device_get_ops(dev);
> > + if (!ops->transfer_zeroes)
> > + return -ENOSYS;
> > +
> > + /* Invalidate the area, so no writeback into the RAM races
> > with DMA */
> > + invalidate_dcache_range((unsigned long)dst, (unsigned
> > long)dst +
> > + roundup(len, ARCH_DMA_MINALIGN));
> > +
> > + return ops->transfer_zeroes(dev, dst, len);
> > +}
> > +
> > UCLASS_DRIVER(dma) = {
> > .id = UCLASS_DMA,
> > .name = "dma",
> > diff --git a/include/dma.h b/include/dma.h
> > index 50e9652..6bad2264 100644
> > --- a/include/dma.h
> > +++ b/include/dma.h
> > @@ -46,6 +46,7 @@ struct dma_ops {
> > */
> > int (*transfer)(struct udevice *dev, int direction, void
> > *dst,
> > void *src, size_t len);
> > + int (*transfer_zeroes)(struct udevice *dev, void *dst,
> > size_t len);
> I wonder if this could be done by using transfer() with a src of NULL
> ?
>
Yes, may be with some description about src of NULL(just for DMA330
driver) on the doc. Otherwise, it would confuse people, and other DMA
driver may treating src with NULL as error or address 0x0.
What do you think?
> >
> > };
> >
> > /*
> > @@ -82,4 +83,15 @@ int dma_get_device(u32 transfer_type, struct
> > udevice **devp);
> > */
> > int dma_memcpy(void *dst, void *src, size_t len);
> >
> > +/*
> > + * dma_memcpy_zeroes - Fill up destination with zeros through DMA.
> > + *
> > + * @dev: The DMA device
> > + * @dst: destination pointer
> > + * @len: length to be copied with zero
> > + * @return: on successful transfer returns zero.
> > + * on failure returns error code.
> > + */
> > +int dma_memcpy_zeroes(struct udevice *dev, void *dst, size_t len);
> > +
> > #endif /* _DMA_H_ */
> > --
> > 2.2.0
> >
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 1/5] drivers: dma: Enable DMA-330 driver support
2018-06-01 14:24 ` Simon Glass
@ 2018-06-06 5:22 ` Chee, Tien Fong
0 siblings, 0 replies; 27+ messages in thread
From: Chee, Tien Fong @ 2018-06-06 5:22 UTC (permalink / raw)
To: u-boot
On Fri, 2018-06-01 at 08:24 -0600, Simon Glass wrote:
> Hi Tien,
>
> On 31 May 2018 at 02:08, <tien.fong.chee@intel.com> wrote:
> >
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> >
> > Enable DMAC driver support for DMA-330 controller.
> > The driver is also compatible to PL330 product.
> >
> > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > ---
> > drivers/dma/Kconfig | 9 +-
> > drivers/dma/Makefile | 1 +
> > drivers/dma/dma330.c | 1514
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> > include/dma330.h | 136 +++++
> > 4 files changed, 1659 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/dma/dma330.c
> > create mode 100644 include/dma330.h
> >
> > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> > index 4ee6afa..6e77e07 100644
> > --- a/drivers/dma/Kconfig
> > +++ b/drivers/dma/Kconfig
> > @@ -2,7 +2,7 @@ menu "DMA Support"
> >
> > config DMA
> > bool "Enable Driver Model for DMA drivers"
> > - depends on DM
> > + depends on DM || SPL_DM
> > help
> > Enable driver model for DMA. DMA engines can do
> > asynchronous data transfers without involving the host
> > @@ -34,4 +34,11 @@ config APBH_DMA_BURST8
> >
> > endif
> >
> > +config DMA330_DMA
> > + bool "PL330/DMA-330 DMA Controller(DMAC) driver"
> > + depends on DMA
> > + help
> > + Enable the DMA controller driver for both PL330 and
> > + DMA-330 products.
> > +
> > endmenu # menu "DMA Support"
> > diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> > index 4eaef8a..bfad0dd 100644
> > --- a/drivers/dma/Makefile
> > +++ b/drivers/dma/Makefile
> > @@ -11,3 +11,4 @@ obj-$(CONFIG_FSL_DMA) += fsl_dma.o
> > obj-$(CONFIG_TI_KSNAV) += keystone_nav.o keystone_nav_cfg.o
> > obj-$(CONFIG_TI_EDMA3) += ti-edma3.o
> > obj-$(CONFIG_DMA_LPC32XX) += lpc32xx_dma.o
> > +obj-$(CONFIG_DMA330_DMA) += dma330.o
> > diff --git a/drivers/dma/dma330.c b/drivers/dma/dma330.c
> > new file mode 100644
> > index 0000000..66575d8
> > --- /dev/null
> > +++ b/drivers/dma/dma330.c
> > @@ -0,0 +1,1514 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018 Intel Corporation <www.intel.com>
> > + */
> > +
> > +#include <common.h>
> > +#include <asm/io.h>
> > +#include <dma330.h>
> > +#include <dm.h>
> > +#include <fdtdec.h>
> > +#include <wait_bit.h>
> > +#include <linux/unaligned/access_ok.h>
> > +
> > +/* Register and Bit field Definitions */
> > +
> > +/* DMA Status */
> > +#define DS 0x0
> > +#define DS_ST_STOP 0x0
> > +#define DS_ST_EXEC 0x1
> > +#define DS_ST_CMISS 0x2
> > +#define DS_ST_UPDTPC 0x3
> > +#define DS_ST_WFE 0x4
> > +#define DS_ST_ATBRR 0x5
> > +#define DS_ST_QBUSY 0x6
> > +#define DS_ST_WFP 0x7
> > +#define DS_ST_KILL 0x8
> > +#define DS_ST_CMPLT 0x9
> > +#define DS_ST_FLTCMP 0xe
> > +#define DS_ST_FAULT 0xf
> It is possible to use enum for some of these?
>
> enum {
> DS = 0,
> DS_ST_STOP,
> ...
> }
>
Okay.
> >
> > +
> > +/* DMA Program Count register */
> > +#define DPC 0x4
> > +/* Interrupt Enable register */
> > +#define INTEN 0x20
> > +/* event-Interrupt Raw Status register */
> > +#define ES 0x24
> > +/* Interrupt Status register */
> > +#define INTSTATUS 0x28
> > +/* Interrupt Clear register */
> > +#define INTCLR 0x2c
> > +/* Fault Status DMA Manager register */
> > +#define FSM 0x30
> > +/* Fault Status DMA Channel register */
> > +#define FSC 0x34
> > +/* Fault Type DMA Manager register */
> > +#define FTM 0x38
> > +
> > +/* Fault Type DMA Channel register */
> > +#define _FTC 0x40
> > +#define FTC(n) (_FTC + (n) * 0x4)
> > +
> > +/* Channel Status register */
> > +#define _CS 0x100
> > +#define CS(n) (_CS + (n) * 0x8)
> > +#define CS_CNS BIT(21)
> > +
> > +/* Channel Program Counter register */
> > +#define _CPC 0x104
> > +#define CPC(n) (_CPC + (n) * 0x8)
> > +
> > +/* Source Address register */
> > +#define _SA 0x400
> > +#define SA(n) (_SA + (n) * 0x20)
> > +
> > +/* Destination Address register */
> > +#define _DA 0x404
> > +#define DA(n) (_DA + (n) * 0x20)
> > +
> > +/* Channel Control register */
> > +#define _CC 0x408
> > +#define CC(n) (_CC + (n) * 0x20)
> > +
> > +/* Channel Control register (CCR) Setting */
> > +#define CC_SRCINC BIT(0)
> > +#define CC_DSTINC BIT(14)
> > +#define CC_SRCPRI BIT(8)
> > +#define CC_DSTPRI BIT(22)
> > +#define CC_SRCNS BIT(9)
> > +#define CC_DSTNS BIT(23)
> > +#define CC_SRCIA BIT(10)
> > +#define CC_DSTIA BIT(24)
> > +#define CC_SRCBRSTLEN_SHFT 4
> > +#define CC_DSTBRSTLEN_SHFT 18
> > +#define CC_SRCBRSTSIZE_SHFT 1
> > +#define CC_DSTBRSTSIZE_SHFT 15
> > +#define CC_SRCCCTRL_SHFT 11
> > +#define CC_SRCCCTRL_MASK 0x7
> Can you make the mask a shifted mask? Then it is easier to use in
> clrsetbits_le32(), for example.
>
> e.g.
>
> #define CC_SRCCCTRL_MASK (7 << CC_SRCCCTRL_SHFT)
>
Okay.
> >
> > +#define CC_DSTCCTRL_SHFT 25
> > +#define CC_DRCCCTRL_MASK 0x7
> > +#define CC_SWAP_SHFT 28
> > +
> > +/* Loop Counter 0 register */
> > +#define _LC0 0x40c
> > +#define LC0(n) (_LC0 + (n) * 0x20)
> > +
> > +/* Loop Counter 1 register */
> > +#define _LC1 0x410
> > +#define LC1(n) (_LC1 + (n) * 0x20)
> > +
> > +/* Debug Status register */
> > +#define DBGSTATUS 0xd00
> > +#define DBG_BUSY BIT(0)
> > +
> > +/* Debug Command register */
> > +#define DBGCMD 0xd04
> > +/* Debug Instruction 0 register */
> > +#define DBGINST0 0xd08
> > +/* Debug Instruction 1 register */
> > +#define DBGINST1 0xd0c
> > +
> > +/* Configuration register */
> > +#define CR0 0xe00
> > +#define CR1 0xe04
> > +#define CR2 0xe08
> > +#define CR3 0xe0c
> > +#define CR4 0xe10
> > +#define CRD 0xe14
> > +
> > +/* Peripheral Identification register */
> > +#define PERIPH_ID 0xfe0
> > +/* Component Identification register */
> > +#define PCELL_ID 0xff0
> > +
> > +/* Configuration register value */
> > +#define CR0_PERIPH_REQ_SET BIT(0)
> > +#define CR0_BOOT_EN_SET BIT(1)
> > +#define CR0_BOOT_MAN_NS BIT(2)
> > +#define CR0_NUM_CHANS_SHIFT 4
> > +#define CR0_NUM_CHANS_MASK 0x7
> > +#define CR0_NUM_PERIPH_SHIFT 12
> > +#define CR0_NUM_PERIPH_MASK 0x1f
> > +#define CR0_NUM_EVENTS_SHIFT 17
> > +#define CR0_NUM_EVENTS_MASK 0x1f
> > +
> > +/* Configuration register value */
> > +#define CR1_ICACHE_LEN_SHIFT 0
> > +#define CR1_ICACHE_LEN_MASK 0x7
> > +#define CR1_NUM_ICACHELINES_SHIFT 4
> > +#define CR1_NUM_ICACHELINES_MASK 0xf
> > +
> > +/* Configuration register value */
> > +#define CRD_DATA_WIDTH_SHIFT 0
> > +#define CRD_DATA_WIDTH_MASK 0x7
> > +#define CRD_WR_CAP_SHIFT 4
> > +#define CRD_WR_CAP_MASK 0x7
> > +#define CRD_WR_Q_DEP_SHIFT 8
> > +#define CRD_WR_Q_DEP_MASK 0xf
> > +#define CRD_RD_CAP_SHIFT 12
> > +#define CRD_RD_CAP_MASK 0x7
> > +#define CRD_RD_Q_DEP_SHIFT 16
> > +#define CRD_RD_Q_DEP_MASK 0xf
> > +#define CRD_DATA_BUFF_SHIFT 20
> > +#define CRD_DATA_BUFF_MASK 0x3ff
> > +
> > +/* Microcode opcode value */
> > +#define CMD_DMAADDH 0x54
> > +#define CMD_DMAEND 0x00
> > +#define CMD_DMAFLUSHP 0x35
> > +#define CMD_DMAGO 0xa0
> > +#define CMD_DMALD 0x04
> > +#define CMD_DMALDP 0x25
> > +#define CMD_DMALP 0x20
> > +#define CMD_DMALPEND 0x28
> > +#define CMD_DMAKILL 0x01
> > +#define CMD_DMAMOV 0xbc
> > +#define CMD_DMANOP 0x18
> > +#define CMD_DMARMB 0x12
> > +#define CMD_DMASEV 0x34
> > +#define CMD_DMAST 0x08
> > +#define CMD_DMASTP 0x29
> > +#define CMD_DMASTZ 0x0c
> > +#define CMD_DMAWFE 0x36
> > +#define CMD_DMAWFP 0x30
> > +#define CMD_DMAWMB 0x13
> > +
> > +/* the size of opcode plus opcode required settings */
> > +#define SZ_DMAADDH 3
> > +#define SZ_DMAEND 1
> > +#define SZ_DMAFLUSHP 2
> > +#define SZ_DMALD 1
> > +#define SZ_DMALDP 2
> > +#define SZ_DMALP 2
> > +#define SZ_DMALPEND 2
> > +#define SZ_DMAKILL 1
> > +#define SZ_DMAMOV 6
> > +#define SZ_DMANOP 1
> > +#define SZ_DMARMB 1
> > +#define SZ_DMASEV 2
> > +#define SZ_DMAST 1
> > +#define SZ_DMASTP 2
> > +#define SZ_DMASTZ 1
> > +#define SZ_DMAWFE 2
> > +#define SZ_DMAWFP 2
> > +#define SZ_DMAWMB 1
> > +#define SZ_DMAGO 6
> > +
> > +/* Use this _only_ to wait on transient states */
> > +#define UNTIL(t, s) do { \
> > + WATCHDOG_RESET(); \
> > + } while (!(dma330_getstate(t) & (s)))
> > +
> > +/* debug message printout */
> > +#ifdef DEBUG
> > +#define DMA330_DBGCMD_DUMP(off, x...) do { \
> > + printf("%x bytes:",
> > off); \
> > + printf(x); \
> > + WATCHDOG_RESET(); \
> > + } while (0)
> > +#else
> > +#define DMA330_DBGCMD_DUMP(off, x...) do {} while (0)
> > +#endif
> Can DMA330_DBGCMD_DUMP be a static function, and lower case? It's
> only
> used in one file, right?
>
Okay, yes.
> >
> > +
> > +/* Enum declaration */
> We know that :-)
>
> Can you please add comments explaining what these enums are for, and
> what the values mean?
>
Sure.
> >
> > +enum dmamov_dst {
> > + SAR = 0,
> > + CCR,
> > + DAR,
> > +};
> > +
> > +enum dma330_dst {
> > + SRC = 0,
> > + DST,
> > +};
> > +
> > +enum dma330_cond {
> > + SINGLE,
> > + BURST,
> > + ALWAYS,
> > +};
> > +
> > +/* Structure will be used by _emit_lpend function */
> > +struct _arg_lpend {
> > + enum dma330_cond cond;
> > + int forever;
> > + u32 loop;
> > + u8 bjump;
> > +};
> > +
> > +/* Structure will be used by _emit_go function */
> > +struct _arg_go {
> Please drop the _ on these struct names, and document the members
> here.
>
Okay.
> >
> > + u8 chan;
> > + u32 addr;
> > + u32 ns;
> > +};
> > +
> > +/*
> > + * _emit_end - Add opcode DMAEND into microcode (end).
> > + *
> > + * @buf: The buffer which stored the microcode program.
> > + *
> > + * Return: Size of opcode.
> > + */
> > +static inline u32 _emit_end(u8 buf[])
> Why is everything inline? The compiler will do it automatically when
> there is only one caller. Other than that, it just increases code
> size
> I think.
>
Okay, will convert to static function.
> >
> > +{
> > + buf[0] = CMD_DMAEND;
> > + DMA330_DBGCMD_DUMP(SZ_DMAEND, "\tDMAEND\n");
> > + return SZ_DMAEND;
> > +}
> > +
> > +/*
> > + * _emit_flushp - Add opcode DMAFLUSHP into microcode (flush
> > peripheral).
> > + *
> > + * @buf -> The buffer which stored the microcode program.
> > + * @peri -> Peripheral ID as listed in DMA NPP.
> > + *
> > + * Return: Size of opcode.
> > + */
> > +static inline u32 _emit_flushp(u8 buf[], u8 peri)
> > +{
> > + u8 *buffer = buf;
> > +
> > + buffer[0] = CMD_DMAFLUSHP;
> > + peri &= 0x1f;
> > + peri <<= 3;
> These should be enums too so we don't have open-coded values in the
> code, Please check throughout.
>
Okay, may be with #define or enum.
> >
> > + buffer[1] = peri;
> > + DMA330_DBGCMD_DUMP(SZ_DMAFLUSHP, "\tDMAFLUSHP %u\n", peri
> > >> 3);
> > + return SZ_DMAFLUSHP;
> > +}
> > +
> > +/**
> > + * _emit_ld - Add opcode DMALD into microcode (load).
> > + *
> > + * @buf: The buffer which stored the microcode program.
> > + * @cond: Execution criteria such as single, burst or always.
> > + *
> > + * Return: Size of opcode.
> > + */
> > +static inline u32 _emit_ld(u8 buf[], enum dma330_cond cond)
> > +{
> > + buf[0] = CMD_DMALD;
> > + if (cond == SINGLE)
> > + buf[0] |= (0 << 1) | (1 << 0);
> More open-coded things
>
Noted.
> >
> > + else if (cond == BURST)
> > + buf[0] |= (1 << 1) | (1 << 0);
> > + DMA330_DBGCMD_DUMP(SZ_DMALD, "\tDMALD%c\n",
> > + cond == SINGLE ? 'S' : (cond == BURST ?
> > 'B' : 'A'));
> > + return SZ_DMALD;
> > +}
> > +
> > +/**
> > + * _emit_lp - Add opcode DMALP into microcode (loop).
> > + *
> > + * @buf: The buffer which stored the microcode program.
> > + * @loop: Selection of using counter 0 or 1 (valid value 0 or 1).
> > + * @cnt: number of iteration (valid value 1-256).
> > + *
> > + * Return: Size of opcode.
> > + */
> > +static inline u32 _emit_lp(u8 buf[], u32 loop, u32 cnt)
> > +{
> > + u8 *buffer = buf;
> > +
> > + buffer[0] = CMD_DMALP;
> > + if (loop)
> > + buffer[0] |= (1 << 1);
> > + /* DMAC increments by 1 internally */
> > + cnt--;
> > + buffer[1] = cnt;
> > + DMA330_DBGCMD_DUMP(SZ_DMALP, "\tDMALP_%c %u\n", loop ? '1'
> > : '0', cnt);
> > + return SZ_DMALP;
> > +}
> > +
> > +/**
> > + * _emit_lpend - Add opcode DMALPEND into microcode (loop end).
> > + *
> > + * @buf: The buffer which stored the microcode program.
> > + * @arg: Structure _arg_lpend which contain all needed info.
> > + * arg->cond -> Execution criteria such as single, burst or
> > always
> > + * arg->forever -> Forever loop? used if DMALPFE started the
> > loop
> > + * arg->bjump -> Backwards jump (relative location of
> > + * 1st instruction in the loop.
> The members of the struct should be documented in the struct above,
> not here.
>
Okay.
> >
> > + *
> > + * Return: Size of opcode.
> > + */
> > +static inline u32 _emit_lpend(u8 buf[], const struct _arg_lpend
> > *arg)
> > +{
> > + u8 *buffer = buf;
> > + enum dma330_cond cond = arg->cond;
> > + int forever = arg->forever;
> > + u32 loop = arg->loop;
> > + u8 bjump = arg->bjump;
> Why have these? Can you not use arg-bjump, etc., directly?
>
Okay.
> >
> > +
> > + buffer[0] = CMD_DMALPEND;
> > + if (loop)
> > + buffer[0] |= (1 << 2);
> More open-coded things
>
noted.
> >
> > + if (!forever)
> > + buffer[0] |= (1 << 4);
> > + if (cond == SINGLE)
> > + buffer[0] |= (0 << 1) | (1 << 0);
> > + else if (cond == BURST)
> > + buffer[0] |= (1 << 1) | (1 << 0);
> > +
> > + buffer[1] = bjump;
> > + DMA330_DBGCMD_DUMP(SZ_DMALPEND, "\tDMALP%s%c_%c
> > bjmpto_%x\n",
> > + forever ? "FE" : "END",
> > + cond == SINGLE ? 'S' : (cond == BURST ?
> > 'B' : 'A'),
> > + loop ? '1' : '0',
> > + bjump);
> > + return SZ_DMALPEND;
> > +}
> > +
> > +/**
> > + * _emit_kill - Add opcode DMAKILL into microcode (kill).
> > + *
> > + * @buf: The buffer which stored the microcode program.
> > + *
> > + * Return: Size of opcode.
> > + */
> > +static inline u32 _emit_kill(u8 buf[])
> > +{
> > + buf[0] = CMD_DMAKILL;
> > + return SZ_DMAKILL;
> > +}
> > +
> > +/**
> > + * _emit_mov - Add opcode DMAMOV into microcode (move).
> > + *
> > + * @buf: The buffer which stored the microcode program.
> > + * @dst: Destination register (valid value SAR[0b000], DAR[0b010],
> > + * or CCR[0b001]).
> > + * @val: 32bit value that to be written into destination register.
> > + *
> > + * Return: Size of opcode.
> > + */
> > +static inline u32 _emit_mov(u8 buf[], enum dmamov_dst dst, u32
> > val)
> > +{
> > + u8 *buffer = buf;
> > +
> > + buffer[0] = CMD_DMAMOV;
> > + buffer[1] = dst;
> > + buffer[2] = val & 0xFF;
> > + buffer[3] = (val >> 8) & 0xFF;
> > + buffer[4] = (val >> 16) & 0xFF;
> > + buffer[5] = (val >> 24) & 0xFF;
> Can you use put_unaligned_le32() ? Also check the same thing below.
>
Okay.
>
> >
> > + DMA330_DBGCMD_DUMP(SZ_DMAMOV, "\tDMAMOV %s 0x%x\n",
> > + dst == SAR ? "SAR" : (dst == DAR ? "DAR"
> > : "CCR"),
> > + val);
> > + return SZ_DMAMOV;
> > +}
> > +
> > +/**
> > + * _emit_nop - Add opcode DMANOP into microcode (no operation).
> > + *
> > + * @buf: The buffer which stored the microcode program.
> > + *
> > + * Return: Size of opcode.
> > + */
> > +static inline u32 _emit_nop(u8 buf[])
> > +{
> > + buf[0] = CMD_DMANOP;
> > + DMA330_DBGCMD_DUMP(SZ_DMANOP, "\tDMANOP\n");
> please add a blank line before 'return'
>
Okay.
> >
> > + return SZ_DMANOP;
> > +}
> > +
> > +/**
> > + * _emit_rmb - Add opcode DMARMB into microcode (read memory
> > barrier).
> > + *
> > + * @buf: The buffer which stored the microcode program.
> > + *
> > + * Return: Size of opcode.
> > + */
> > +static inline u32 _emit_rmb(u8 buf[])
> > +{
> > + buf[0] = CMD_DMARMB;
> > + DMA330_DBGCMD_DUMP(SZ_DMARMB, "\tDMARMB\n");
> > + return SZ_DMARMB;
> > +}
> > +
> > +/**
> > + * _emit_sev - Add opcode DMASEV into microcode (send event).
> > + *
> > + * @buf: The buffer which stored the microcode program.
> > + * @ev: The event number (valid 0 - 31).
> > + *
> > + * Return: Size of opcode.
> > + */
> > +static inline u32 _emit_sev(u8 buf[], u8 ev)
> > +{
> > + u8 *buffer = buf;
> > +
> > + buffer[0] = CMD_DMASEV;
> > + ev &= 0x1f;
> > + ev <<= 3;
> > + buffer[1] = ev;
> > + DMA330_DBGCMD_DUMP(SZ_DMASEV, "\tDMASEV %u\n", ev >> 3);
> > + return SZ_DMASEV;
> > +}
> > +
> > +/**
> > + * _emit_st - Add opcode DMAST into microcode (store).
> > + *
> > + * @buf: The buffer which stored the microcode program.
> > + * @cond: Execution criteria such as single, burst or always.
> > + *
> > + * Return: Size of opcode.
> > + */
> > +static inline u32 _emit_st(u8 buf[], enum dma330_cond cond)
> > +{
> > + buf[0] = CMD_DMAST;
> > + if (cond == SINGLE)
> > + buf[0] |= (0 << 1) | (1 << 0);
> > + else if (cond == BURST)
> > + buf[0] |= (1 << 1) | (1 << 0);
> > +
> > + DMA330_DBGCMD_DUMP(SZ_DMAST, "\tDMAST%c\n",
> > + cond == SINGLE ? 'S' : (cond == BURST ? 'B' :
> > 'A'));
> > + return SZ_DMAST;
> > +}
> > +
> > +/**
> > + * _emit_stp - Add opcode DMASTP into microcode (store and notify
> > peripheral).
> > + *
> > + * @buf: The buffer which stored the microcode program.
> > + * @cond: Execution criteria such as single, burst or always.
> > + * @peri: Peripheral ID as listed in DMA NPP.
> > + *
> > + * Return: Size of opcode.
> > + */
> > +static inline u32 _emit_stp(u8 buf[], enum dma330_cond cond, u8
> > peri)
> > +{
> > + u8 *buffer = buf;
> > +
> > + buffer[0] = CMD_DMASTP;
> > + if (cond == BURST)
> > + buf[0] |= (1 << 1);
> > + peri &= 0x1f;
> > + peri <<= 3;
> > + buffer[1] = peri;
> > + DMA330_DBGCMD_DUMP(SZ_DMASTP, "\tDMASTP%c %u\n",
> > + cond == SINGLE ? 'S' : 'B', peri >> 3);
> > + return SZ_DMASTP;
> > +}
> > +
> > +/**
> > + * _emit_stz - Add opcode DMASTZ into microcode (store zeros).
> > + *
> > + * @buf -> The buffer which stored the microcode program.
> > + *
> > + * Return: Size of opcode.
> > + */
> > +static inline u32 _emit_stz(u8 buf[])
> > +{
> > + buf[0] = CMD_DMASTZ;
> > + DMA330_DBGCMD_DUMP(SZ_DMASTZ, "\tDMASTZ\n");
> > + return SZ_DMASTZ;
> > +}
> > +
> > +/**
> > + * _emit_wfp - Add opcode DMAWFP into microcode (wait for
> > peripheral).
> > + *
> > + * @buf: The buffer which stored the microcode program.
> > + * @cond: Execution criteria such as single, burst or always.
> > + * @peri: Peripheral ID as listed in DMA NPP.
> > + *
> > + * Return: Size of opcode.
> > + */
> > +static inline u32 _emit_wfp(u8 buf[], enum dma330_cond cond, u8
> > peri)
> > +{
> > + u8 *buffer = buf;
> > +
> > + buffer[0] = CMD_DMAWFP;
> > + if (cond == SINGLE)
> > + buffer[0] |= (0 << 1) | (0 << 0);
> > + else if (cond == BURST)
> > + buffer[0] |= (1 << 1) | (0 << 0);
> > + else
> > + buffer[0] |= (0 << 1) | (1 << 0);
> Perhaps these three values are the same for each command? If so, the
> three expressions (0, 1, 2) could go in an enum.
>
Okay.
> >
> > +
> > + peri &= 0x1f;
> > + peri <<= 3;
> > + buffer[1] = peri;
> > + DMA330_DBGCMD_DUMP(SZ_DMAWFP, "\tDMAWFP%c %u\n",
> > + cond == SINGLE ? 'S' : (cond == BURST ? 'B' : 'P'),
> > peri >> 3);
> > + return SZ_DMAWFP;
> > +}
> > +
> > +/**
> > + * _emit_wmb - Add opcode DMAWMB into microcode (write memory
> > barrier).
> > + *
> > + * @buf: The buffer which stored the microcode program.
> > + *
> > + * Return: Size of opcode.
> > + */
> > +static inline u32 _emit_wmb(u8 buf[])
> > +{
> > + buf[0] = CMD_DMAWMB;
> > + DMA330_DBGCMD_DUMP(SZ_DMAWMB, "\tDMAWMB\n");
> > + return SZ_DMAWMB;
> > +}
> > +
> > +/**
> > + * _emit_go - Add opcode DMALGO into microcode (go).
> > + *
> > + * @buf: The buffer which stored the microcode program.
> > + * @arg: structure _arg_go which contain all needed info
> > + * arg->chan -> channel number
> > + * arg->addr -> start address of the microcode program
> > + * which will be wrote into CPC register
> > + * arg->ns -> 1 for non secure, 0 for secure
> > + * (if only DMA Manager is in secure).
> > + *
> > + * Return: Size of opcode.
> > + */
> > +static inline u32 _emit_go(u8 buf[], const struct _arg_go *arg)
> > +{
> > + u8 *buffer = buf;
> > + u8 chan = arg->chan;
> > + u32 addr = arg->addr;
> > + u32 ns = arg->ns;
> These local vars seem pointless as the are only used once. Maybe it's
> worth it for 'addr', but even there you should use the put_unaligned
> thing.
Okay.
> >
> > +
> > + buffer[0] = CMD_DMAGO;
> > + buffer[0] |= (ns << 1);
> > + buffer[1] = chan & 0x7;
> > + buf[2] = addr & 0xFF;
> > + buf[3] = (addr >> 8) & 0xFF;
> > + buf[4] = (addr >> 16) & 0xFF;
> > + buf[5] = (addr >> 24) & 0xFF;
> > + return SZ_DMAGO;
> > +}
> > +
> > +/**
> > + * _prepare_ccr - Populate the CCR register.
> > + * @rqc: Request Configuration.
> > + *
> > + * Return: Channel Control register (CCR) Setting.
> > + */
> > +static inline u32 _prepare_ccr(const struct dma330_reqcfg *rqc)
> > +{
> > + u32 ccr = 0;
> > +
> > + if (rqc->src_inc)
> > + ccr |= CC_SRCINC;
> > + if (rqc->dst_inc)
> > + ccr |= CC_DSTINC;
> > +
> > + /* We set same protection levels for Src and DST for now */
> > + if (rqc->privileged)
> > + ccr |= CC_SRCPRI | CC_DSTPRI;
> > + if (rqc->nonsecure)
> > + ccr |= CC_SRCNS | CC_DSTNS;
> > + if (rqc->insnaccess)
> > + ccr |= CC_SRCIA | CC_DSTIA;
> > +
> > + ccr |= (((rqc->brst_len - 1) & 0xf) << CC_SRCBRSTLEN_SHFT);
> > + ccr |= (((rqc->brst_len - 1) & 0xf) << CC_DSTBRSTLEN_SHFT);
> > +
> > + ccr |= (rqc->brst_size << CC_SRCBRSTSIZE_SHFT);
> > + ccr |= (rqc->brst_size << CC_DSTBRSTSIZE_SHFT);
> > +
> > + ccr |= (rqc->scctl << CC_SRCCCTRL_SHFT);
> > + ccr |= (rqc->dcctl << CC_DSTCCTRL_SHFT);
> You don't need the outer brackets on these.
OKAY.
>
> >
> > +
> > + ccr |= (rqc->swap << CC_SWAP_SHFT);
> > + return ccr;
> > +}
> > +
> > +/**
> > + * dma330_until_dmac_idle - Wait until DMA Manager is idle.
> > + *
> > + * @plat: Pointer to struct dma_dma330_platdata.
> > + * @timeout_ms: Timeout (in miliseconds).
> > + *
> > + * Return: Negative value for error / timeout ocurred before idle,
> > + * 0 for successful.
> > + */
> > +static int dma330_until_dmac_idle(struct dma_dma330_platdata
> > *plat,
> > + const u32 timeout_ms)
> > +{
> > + void __iomem *regs = plat->base;
> > +
> > + return wait_for_bit(__func__,
> > + (const u32 *)(uintptr_t)(regs +
> > DBGSTATUS),
> > + DBG_BUSY, 0, timeout_ms, false);
> > +}
> > +
> > +/**
> > + * dma330_execute_dbginsn - Execute debug instruction such as
> > DMAGO and DMAKILL.
> > + *
> > + * @insn: The buffer which stored the debug instruction.
> > + * @plat: Pointer to struct dma_dma330_platdata.
> > + * @timeout_ms: Timeout (in miliseconds).
> > + *
> > + * Return: void.
> > + */
> > +static inline void dma330_execute_dbginsn(u8 insn[],
> > + struct
> > dma_dma330_platdata *plat,
> > + const u32 timeout_ms)
> > +{
> > + void __iomem *regs = plat->base;
> > + u32 val;
> > +
> > + val = (insn[0] << 16) | (insn[1] << 24);
> > + if (!plat->dma330.channel0_manager1)
> > + val |= (1 << 0);
> > + val |= (plat->dma330.channel_num << 8); /* Channel Number
> > */
> > + writel(val, regs + DBGINST0);
> > + val = insn[2];
> > + val = val | (insn[3] << 8);
> > + val = val | (insn[4] << 16);
> > + val = val | (insn[5] << 24);
> > + writel(val, regs + DBGINST1);
> > +
> > + /* If timed out due to halted state-machine */
> > + if (dma330_until_dmac_idle(plat, timeout_ms)) {
> > + debug("DMAC halted!\n");
> > + return;
> > + }
> > + /* Get going */
> > + writel(0, regs + DBGCMD);
> > +}
> > +
> > +/**
> > + * dma330_getstate - Get the status of current channel or manager.
> > + *
> > + * @plat: Pointer to struct dma_dma330_platdata.
> > + *
> > + * Return: Status state of current channel or current manager.
> > + */
> > +static inline u32 dma330_getstate(struct dma_dma330_platdata
> > *plat)
> > +{
> > + void __iomem *regs = plat->base;
> > + u32 val;
> > +
> > + if (plat->dma330.channel0_manager1)
> > + val = readl((uintptr_t)(regs + DS)) & 0xf;
> > + else
> > + val = readl((uintptr_t)(regs + CS(plat-
> > >dma330.channel_num))) &
> > + 0xf;
> > +
> > + switch (val) {
> > + case DS_ST_STOP:
> > + return DMA330_STATE_STOPPED;
> > + case DS_ST_EXEC:
> > + return DMA330_STATE_EXECUTING;
> > + case DS_ST_CMISS:
> > + return DMA330_STATE_CACHEMISS;
> > + case DS_ST_UPDTPC:
> > + return DMA330_STATE_UPDTPC;
> > + case DS_ST_WFE:
> > + return DMA330_STATE_WFE;
> > + case DS_ST_FAULT:
> > + return DMA330_STATE_FAULTING;
> > + case DS_ST_ATBRR:
> > + if (plat->dma330.channel0_manager1)
> > + return DMA330_STATE_INVALID;
> > + else
> > + return DMA330_STATE_ATBARRIER;
> > + case DS_ST_QBUSY:
> > + if (plat->dma330.channel0_manager1)
> > + return DMA330_STATE_INVALID;
> > + else
> > + return DMA330_STATE_QUEUEBUSY;
> > + case DS_ST_WFP:
> > + if (plat->dma330.channel0_manager1)
> > + return DMA330_STATE_INVALID;
> > + else
> > + return DMA330_STATE_WFP;
> > + case DS_ST_KILL:
> > + if (plat->dma330.channel0_manager1)
> > + return DMA330_STATE_INVALID;
> > + else
> > + return DMA330_STATE_KILLING;
> > + case DS_ST_CMPLT:
> > + if (plat->dma330.channel0_manager1)
> > + return DMA330_STATE_INVALID;
> > + else
> > + return DMA330_STATE_COMPLETING;
> > + case DS_ST_FLTCMP:
> > + if (plat->dma330.channel0_manager1)
> > + return DMA330_STATE_INVALID;
> > + else
> > + return DMA330_STATE_FAULT_COMPLETING;
> > + default:
> > + return DMA330_STATE_INVALID;
> > + }
> > +}
> > +
> > +/**
> > + * dma330_trigger - Execute the DMAGO through debug instruction.
> > + *
> > + * @plat: Pointer to struct dma_dma330_platdata.
> > + * @timeout_ms: Timeout (in miliseconds).
> > + *
> > + * When the DMA manager executes Go for a DMA channel that is in
> > the Stopped
> > + * state, it moves a 32-bit immediate into the program counter,
> > then setting
> > + * its security state and updating DMA channel to the Executing
> > state.
> > + *
> > + * Return: Negative value for error as DMA is not ready. 0 for
> > successful.
> > + */
> > +static int dma330_trigger(struct dma_dma330_platdata *plat,
> > + const u32 timeout_ms)
> > +{
> > + u8 insn[6] = {0, 0, 0, 0, 0, 0};
> > + struct _arg_go go;
> > +
> > + /*
> > + * Return if already ACTIVE. It will ensure DMAGO is
> > executed at
> > + * STOPPED state too
> > + */
> > + plat->dma330.channel0_manager1 = 0;
> > + if (dma330_getstate(plat) !=
> > + DMA330_STATE_STOPPED)
> > + return -EPERM;
> > +
> > + go.chan = plat->dma330.channel_num;
> > + go.addr = (uintptr_t)plat->dma330.buf;
> > +
> > + if (!plat->dma330.secure)
> > + go.ns = 1; /* non-secure condition */
> > + else
> > + go.ns = 0; /* secure condition */
> > +
> > + _emit_go(insn, &go);
> > +
> > + /* Only manager can execute GO */
> > + plat->dma330.channel0_manager1 = 1;
> > + dma330_execute_dbginsn(insn, plat, timeout_ms);
> > + return 0;
> > +}
> > +
> > +/**
> > + * dma330_stop - Stop the manager or channel.
> > + *
> > + * @plat: Pointer to struct dma_dma330_platdata.
> > + * @timeout_ms: Timeout (in miliseconds).
> > + *
> > + * Stop the manager/channel or killing them and ensure they reach
> > to stop
> > + * state.
> > + *
> > + * Return: void.
> > + */
> > +static void dma330_stop(struct dma_dma330_platdata *plat, const
> > u32 timeout_ms)
> > +{
> > + u8 insn[6] = {0, 0, 0, 0, 0, 0};
> > +
> > + /* If fault completing, wait until reach faulting and
> > killing state */
> > + if (dma330_getstate(plat) == DMA330_STATE_FAULT_COMPLETING)
> > + UNTIL(plat, DMA330_STATE_FAULTING |
> > DMA330_STATE_KILLING);
> > +
> > + /* Return if nothing needs to be done */
> > + if (dma330_getstate(plat) == DMA330_STATE_COMPLETING ||
> > + dma330_getstate(plat) == DMA330_STATE_KILLING ||
> > + dma330_getstate(plat) == DMA330_STATE_STOPPED)
> > + return;
> > +
> > + /* Kill it to ensure it reach to stop state */
> > + _emit_kill(insn);
> > +
> > + /* Execute the KILL instruction through debug registers */
> > + dma330_execute_dbginsn(insn, plat, timeout_ms);
> > +}
> > +
> > +/**
> > + * dma330_start - Execute the command list through DMAGO and debug
> > instruction.
> > + *
> > + * @plat: Pointer to struct dma_dma330_platdata.
> > + * @timeout_ms: Timeout (in miliseconds).
> > + *
> > + * Return: Negative value for error where DMA is not ready. 0 for
> > successful.
> > + */
> > +static int dma330_start(struct dma_dma330_platdata *plat, const
> > u32 timeout_ms)
> > +{
> > + debug("INFO: DMA BASE Address = 0x%08x\n",
> > + (u32)(uintptr_t)plat->base);
> > +
> > + switch (dma330_getstate(plat)) {
> > + case DMA330_STATE_FAULT_COMPLETING:
> > + UNTIL(plat, DMA330_STATE_FAULTING |
> > DMA330_STATE_KILLING);
> > +
> > + if (dma330_getstate(plat) == DMA330_STATE_KILLING)
> > + UNTIL(plat, DMA330_STATE_STOPPED);
> > +
> > + case DMA330_STATE_FAULTING:
> > + dma330_stop(plat, timeout_ms);
> > +
> > + case DMA330_STATE_KILLING:
> > + case DMA330_STATE_COMPLETING:
> > + UNTIL(plat, DMA330_STATE_STOPPED);
> > +
> > + case DMA330_STATE_STOPPED:
> > + return dma330_trigger(plat, timeout_ms);
> > +
> > + case DMA330_STATE_WFP:
> > + case DMA330_STATE_QUEUEBUSY:
> > + case DMA330_STATE_ATBARRIER:
> > + case DMA330_STATE_UPDTPC:
> > + case DMA330_STATE_CACHEMISS:
> > + case DMA330_STATE_EXECUTING:
> > + return -ESRCH;
> > + /* For RESUME, nothing yet */
> > + case DMA330_STATE_WFE:
> > + default:
> > + return -ESRCH;
> > + }
> > +}
> > +
> > +/**
> > + * dma330_transfer_start - DMA start to run.
> > + *
> > + * @plat: Pointer to struct dma_dma330_platdata.
> > + *
> > + * DMA start to excecute microcode command list.
> > + *
> > + * Return: Negative value for error or not successful. 0 for
> > successful.
> > + */
> > +static int dma330_transfer_start(struct dma_dma330_platdata *plat)
> > +{
> > + const u32 timeout_ms = 1000;
> > +
> > + /* Execute the command list */
> > + return dma330_start(plat, timeout_ms);
> > +}
> > +
> > +/**
> > + * dma330_transfer_finish - DMA polling until execution finish or
> > error.
> > + *
> > + * @plat: Pointer to struct dma_dma330_platdata.
> > + *
> > + * DMA polling until excution finish and checking the state
> > status.
> > + *
> > + * Return: Negative value for state error or not successful. 0 for
> > successful.
> > + */
> > +static int dma330_transfer_finish(struct dma_dma330_platdata
> > *plat)
> > +{
> > + void __iomem *regs = plat->base;
> > +
> > + if (!plat->dma330.buf) {
> > + debug("ERROR DMA330 : DMA Microcode buffer pointer
> > is NULL\n");
> > + return -EINVAL;
> > + }
> > +
> > + plat->dma330.channel0_manager1 = 0;
> > +
> > + /* Wait until finish execution to ensure we compared
> > correct result*/
> > + UNTIL(plat, DMA330_STATE_STOPPED | DMA330_STATE_FAULTING);
> > +
> > + /* check the state */
> > + if (dma330_getstate(plat) == DMA330_STATE_FAULTING) {
> > + debug("FAULT Mode: Channel %d Faulting, FTR =
> > 0x%08x,",
> > + plat->dma330.channel_num,
> > + readl(regs + FTC(plat->dma330.channel_num)));
> > + debug("CPC = 0x%08lx\n",
> > + (readl(regs + CPC(plat->dma330.channel_num))
> > + - (uintptr_t)plat->dma330.buf));
> > + return -EPROTO;
> > + }
> > + return 0;
> > +}
> > +
> > +/**
> > + * dma330_transfer_setup - DMA transfer setup (DMA_MEM_TO_MEM,
> > DMA_MEM_TO_DEV
> > + * or DMA_DEV_TO_MEM).
> > + *
> > + * @plat: Pointer to struct dma_dma330_platdata.
> > + *
> > + * For Peripheral transfer, the FIFO threshold value is expected
> > at
> > + * 2 ^ plat->brst_size * plat->brst_len.
> > + *
> > + * Return: Negative value for error or not successful. 0 for
> > successful.
> > + */
> > +static int dma330_transfer_setup(struct dma_dma330_platdata *plat)
> This function is extremely long, Can it be split into separete
> sub-functions which perform the work, if the work can be split
> sensibly into different parts?
>
Let me see how the work can be further split into different parts.
> >
> > +{
> > + /* Variable declaration */
> > + /* Buffer offset clear to 0 */
> > + int off = 0;
> > + int ret = 0;
> > + /* For DMALPEND */
> > + u32 loopjmp0, loopjmp1;
> > + /* Loop count 0 */
> > + u32 lcnt0 = 0;
> > + /* Loop count 1 */
> > + u32 lcnt1 = 0;
> > + u32 brst_size = 0;
> > + u32 brst_len = 0;
> > + u32 data_size_byte = plat->dma330.size_byte;
> > + /* Strong order memory is required to store microcode
> > command list */
> > + u8 *buf = (u8 *)plat->dma330.buf;
> > + /* Channel Control Register */
> > + u32 ccr = 0;
> > + struct dma330_reqcfg reqcfg;
> > +
> > + if (!buf) {
> > + debug("ERROR DMA330 : DMA Microcode buffer pointer
> > is NULL\n");
> > + return -EINVAL;
> > + }
> > +
> > + if (plat->dma330.transfer_type == DMA_MEM_TO_DEV)
> > + debug("INFO: mem2perip");
> > + else if (plat->dma330.transfer_type == DMA_DEV_TO_MEM)
> > + debug("INFO: perip2mem");
> > + else
> > + debug("INFO: DMA_MEM_TO_MEM");
> > +
> > + debug(" - 0x%x%x -> 0x%x%x\nsize=%08x brst_size=2^%d ",
> > + (u32)(plat->dma330.src_addr >> 32),
> > + (u32)plat->dma330.src_addr,
> > + (u32)(plat->dma330.dst_addr >> 32),
> > + (u32)plat->dma330.dst_addr,
> > + data_size_byte,
> > + plat->brst_size);
> > +
> > + debug("brst_len=%d singles_brst_size=2^%d\n", plat-
> > >brst_len,
> > + plat->dma330.single_brst_size);
> > +
> > + /* brst_size = 2 ^ plat->brst_size */
> > + brst_size = 1 << plat->brst_size;
> > +
> > + /* Fool proof checking */
> > + if (plat->brst_size < 0 || plat->brst_size >
> > + plat->max_brst_size) {
> > + debug("ERROR DMA330: brst_size must 0-%d (not
> > %d)\n",
> > + plat->max_brst_size, plat->brst_size);
> > + return -EINVAL;
> > + }
> > + if (plat->dma330.single_brst_size < 0 ||
> > + plat->dma330.single_brst_size > plat-
> > >max_brst_size) {
> > + debug("ERROR DMA330 : single_brst_size must 0-%d
> > (not %d)\n",
> > + plat->max_brst_size, plat-
> > >dma330.single_brst_size);
> > + return -EINVAL;
> > + }
> > + if (plat->brst_len < 1 || plat->brst_len > 16) {
> > + debug("ERROR DMA330 : brst_len must 1-16 (not
> > %d)\n",
> > + plat->brst_len);
> > + return -EINVAL;
> > + }
> > + if (plat->dma330.src_addr & (brst_size - 1)) {
> > + debug("ERROR DMA330 : Source address unaligned\n");
> > + return -EINVAL;
> > + }
> > + if (plat->dma330.dst_addr & (brst_size - 1)) {
> > + debug("ERROR DMA330 : Destination address
> > unaligned\n");
> > + return -EINVAL;
> > + }
> > +
> > + /* Setup the command list */
> > + /* DMAMOV SAR, x->src_addr */
> > + off += _emit_mov(&buf[off], SAR, plat->dma330.src_addr);
> > + /* DMAMOV DAR, x->dst_addr */
> > + off += _emit_mov(&buf[off], DAR, plat->dma330.dst_addr);
> > + /* DMAFLUSHP P(periheral_id) */
> > + if (plat->dma330.transfer_type != DMA_MEM_TO_MEM)
> > + off += _emit_flushp(&buf[off], plat-
> > >dma330.peripheral_id);
> > +
> > + /* Preparing the CCR value */
> > + if (plat->dma330.transfer_type == DMA_MEM_TO_DEV) {
> > + /* Disable auto increment */
> > + reqcfg.dst_inc = 0;
> > + /* Enable auto increment */
> > + reqcfg.src_inc = 1;
> > + } else if (plat->dma330.transfer_type == DMA_DEV_TO_MEM) {
> > + reqcfg.dst_inc = 1;
> > + reqcfg.src_inc = 0;
> > + } else {
> > + /* DMA_MEM_TO_MEM */
> > + reqcfg.dst_inc = 1;
> > + reqcfg.src_inc = 1;
> > + }
> > +
> > + if (!plat->dma330.secure)
> > + reqcfg.nonsecure = 1; /* Non Secure mode */
> > + else
> > + reqcfg.nonsecure = 0; /* Secure mode */
> > +
> > + if (plat->dma330.enable_cache1 == 0) {
> > + /* Noncacheable but bufferable */
> > + reqcfg.dcctl = 0x1;
> > + reqcfg.scctl = 0x1;
> > + } else {
> > + if (plat->dma330.transfer_type == DMA_MEM_TO_DEV) {
> > + reqcfg.dcctl = 0x1;
> > + /* Cacheable write back */
> > + reqcfg.scctl = 0x7;
> > + } else if (plat->dma330.transfer_type ==
> > DMA_DEV_TO_MEM) {
> > + reqcfg.dcctl = 0x7;
> > + reqcfg.scctl = 0x1;
> > + } else {
> > + reqcfg.dcctl = 0x7;
> > + reqcfg.scctl = 0x7;
> > + }
> > + }
> > + /* 1 - Privileged */
> > + reqcfg.privileged = 1;
> > + /* 0 - Data access */
> > + reqcfg.insnaccess = 0;
> > + /* 0 - No endian swap */
> I am not keen on these comments. The struct members should be
> documented when the struct is declared, not here.
>
Okay.
> >
> > + reqcfg.swap = 0;
> > + /* DMA burst length */
> > + reqcfg.brst_len = plat->brst_len;
> > + /* DMA burst size */
> > + reqcfg.brst_size = plat->brst_size;
> > + /* Preparing the CCR value */
> > + ccr = _prepare_ccr(&reqcfg);
> > + /* DMAMOV CCR, ccr */
> > + off += _emit_mov(&buf[off], CCR, ccr);
> > +
> > + /* BURST */
> > + /* Can initiate a burst? */
> These two comments should be joined
>
Okay. Actually comments helping to track the operation easily.
> >
> > + while (data_size_byte >= brst_size * plat->brst_len) {
> > + lcnt0 = data_size_byte / (brst_size * plat-
> > >brst_len);
> > + lcnt1 = 0;
> > + if (lcnt0 >= 256 * 256) {
> > + lcnt0 = 256;
> > + lcnt1 = 256;
> > + } else if (lcnt0 >= 256) {
> > + lcnt1 = lcnt0 / 256;
> > + lcnt0 = 256;
> > + }
> > + data_size_byte = data_size_byte -
> > + (brst_size * plat->brst_len * lcnt0 *
> > lcnt1);
> > +
> > + debug("Transferring 0x%08x Remain 0x%08x\n",
> > (brst_size *
> > + plat->brst_len * lcnt0 * lcnt1),
> > data_size_byte);
> > + debug("Running burst - brst_size=2^%d, brst_len=%d,
> > ",
> > + plat->brst_size, plat->brst_len);
> > + debug("lcnt0=%d, lcnt1=%d\n", lcnt0, lcnt1);
> > +
> > + if (lcnt1) {
> > + /* DMALP1 */
> > + off += _emit_lp(&buf[off], 1, lcnt1);
> > + loopjmp1 = off;
> > + }
> > + /* DMALP0 */
> What does this comment mean?
>
This means below _emit_lp would insert the microcode DMALP(looping)
into buffer[off]
> >
> > + off += _emit_lp(&buf[off], 0, lcnt0);
> > + loopjmp0 = off;
> > + /* DMAWFP periheral_id, burst */
> > + if (plat->dma330.transfer_type != DMA_MEM_TO_MEM)
> > + off += _emit_wfp(&buf[off], BURST,
> > + plat->dma330.peripheral_id);
> > + /* DMALD */
> > + off += _emit_ld(&buf[off], ALWAYS);
> > +
> > + WATCHDOG_RESET();
> > +
> > + /* DMARMB */
> > + off += _emit_rmb(&buf[off]);
> > + /* DMASTPB peripheral_id */
> > + if (plat->dma330.transfer_type != DMA_MEM_TO_MEM)
> > + off += _emit_stp(&buf[off], BURST,
> > + plat->dma330.peripheral_id);
> > + else
> > + off += _emit_st(&buf[off], ALWAYS);
> > + /* DMAWMB */
> > + off += _emit_wmb(&buf[off]);
> > + /* DMALP0END */
> > + struct _arg_lpend lpend;
> > +
> > + lpend.cond = ALWAYS;
> > + lpend.forever = 0;
> > + /* Loop cnt 0 */
> > + lpend.loop = 0;
> > + lpend.bjump = off - loopjmp0;
> > + off += _emit_lpend(&buf[off], &lpend);
> > + /* DMALP1END */
> > + if (lcnt1) {
> > + struct _arg_lpend lpend;
> > +
> > + lpend.cond = ALWAYS;
> > + lpend.forever = 0;
> > + lpend.loop = 1; /* loop cnt 1*/
> > + lpend.bjump = off - loopjmp1;
> > + off += _emit_lpend(&buf[off], &lpend);
> > + }
> > + /* Ensure the microcode don't exceed buffer size */
> > + if (off > plat->buf_size) {
> > + debug("ERROR DMA330 : Exceed buffer
> > size\n");
> > + return -ENOMEM;
> > + }
> > + }
> > +
> > + /* SINGLE */
> > + brst_len = 1;
> > + /* brst_size = 2 ^ plat->dma330.single_brst_size; */
> > + brst_size = (1 << plat->dma330.single_brst_size);
> > + lcnt0 = data_size_byte / (brst_size * brst_len);
> > +
> > + /* Ensure all data will be transferred */
> > + data_size_byte = data_size_byte -
> > + (brst_size * brst_len * lcnt0);
> > + if (data_size_byte) {
> > + debug("ERROR DMA330 : Detected the possibility of
> > ");
> > + debug("untransfered data. Please ensure correct
> > single ");
> > + debug("burst size\n");
> > + }
> > +
> > + if (lcnt0) {
> > + debug("Transferring 0x%08x Remain 0x%08x\n",
> > (brst_size *
> > + brst_len * lcnt0 * lcnt1), data_size_byte);
> > + debug("Running single - brst_size=2^%d,
> > brst_len=%d, ",
> > + brst_size, brst_len);
> > + debug("lcnt0=%d\n", lcnt0);
> > +
> > + /* Preparing the CCR value */
> > + /* DMA burst length */
> > + reqcfg.brst_len = brst_len;
> > + /* DMA burst size */
> > + reqcfg.brst_size = brst_size;
> > + ccr = _prepare_ccr(&reqcfg);
> > + /* DMAMOV CCR, ccr */
> > + off += _emit_mov(&buf[off], CCR, ccr);
> > +
> > + /* DMALP0 */
> > + off += _emit_lp(&buf[off], 0, lcnt0);
> > + loopjmp0 = off;
> > + /* DMAWFP peripheral_id, single */
> > + if (plat->dma330.transfer_type != DMA_MEM_TO_MEM)
> > + off += _emit_wfp(&buf[off], SINGLE,
> > + plat->dma330.peripheral_id);
> > +
> > + /* DMALD */
> > + off += _emit_ld(&buf[off], ALWAYS);
> > + /* DMARMB */
> > + off += _emit_rmb(&buf[off]);
> > + /* DMASTPS peripheral_id */
> > + if (plat->dma330.transfer_type != DMA_MEM_TO_MEM)
> > + off += _emit_stp(&buf[off], SINGLE,
> > + plat->dma330.peripheral_id);
> > + else
> > + off += _emit_st(&buf[off], ALWAYS);
> > +
> > + /* DMAWMB */
> > + off += _emit_wmb(&buf[off]);
> > + /* DMALPEND */
> > + struct _arg_lpend lpend1;
> > +
> > + lpend1.cond = ALWAYS;
> > + lpend1.forever = 0;
> > + /* loop cnt 0 */
> > + lpend1.loop = 0;
> > + lpend1.bjump = off - loopjmp0;
> > + off += _emit_lpend(&buf[off], &lpend1);
> > + /* Ensure the microcode don't exceed buffer size */
> > + if (off > plat->buf_size) {
> > + puts("ERROR DMA330 : Exceed buffer
> > size\n");
> > + return -ENOMEM;
> > + }
> > + }
> > +
> > + /* DMAEND */
> > + off += _emit_end(&buf[off]);
> > +
> > + ret = dma330_transfer_start(plat);
> > + if (ret)
> > + return ret;
> > +
> > + return dma330_transfer_finish(plat);
> > +}
> > +
> > +/**
> > + * dma330_transfer_zeroes - DMA transfer zeros.
> > + *
> > + * @plat: Pointer to struct dma_dma330_platdata.
> > + *
> > + * Used to write zeros to a memory chunk for memory scrubbing
> > purpose.
> > + *
> > + * Return: Negative value for error or not successful. 0 for
> > successful.
> > + */
> > +static int dma330_transfer_zeroes_setup(struct dma_dma330_platdata
> > *plat)
> Can this function be combined with the above somehow? It seems like a
> lot of duplicate code.
>
Let me see how to merge them.
> >
> > +{
> > + /* Variable declaration */
> > + /* Buffer offset clear to 0 */
> > + int off = 0;
> > + int ret = 0;
> > + /* For DMALPEND */
> > + u32 loopjmp0, loopjmp1;
> > + /* Loop count 0 */
> > + u32 lcnt0 = 0;
> > + /* Loop count 1 */
> > + u32 lcnt1 = 0;
> > + u32 brst_size = 0;
> > + u32 brst_len = 0;
> > + u32 data_size_byte = plat->dma330.size_byte;
> > + /* Strong order memory is required to store microcode
> > command list */
> > + u8 *buf = (u8 *)plat->dma330.buf;
> > + /* Channel Control Register */
> > + u32 ccr = 0;
> > + struct dma330_reqcfg reqcfg;
> > +
> > + if (!buf) {
> > + debug("ERROR DMA330 : DMA Microcode buffer pointer
> > is NULL\n");
> > + return -EINVAL;
> > + }
> > +
> > + debug("INFO: Write zeros -> ");
> > + debug("0x%x%x size=0x%08x\n",
> > + (u32)(plat->dma330.dst_addr >> 32),
> > + (u32)plat->dma330.dst_addr,
> > + data_size_byte);
> > +
> > + plat->dma330.single_brst_size = 1;
> > +
> > + /* burst_size = 2 ^ plat->brst_size */
> > + brst_size = 1 << plat->brst_size;
> > +
> > + /* Setup the command list */
> > + /* DMAMOV DAR, x->dst_addr */
> > + off += _emit_mov(&buf[off], DAR, plat->dma330.dst_addr);
> > +
> > + /* Preparing the CCR value */
> > + /* Enable auto increment */
> > + reqcfg.dst_inc = 1;
> > + /* Disable auto increment (not applicable) */
> > + reqcfg.src_inc = 0;
> > + /* Noncacheable but bufferable */
> > + reqcfg.dcctl = 0x1;
> > + /* Noncacheable and bufferable */
> > + reqcfg.scctl = 0x1;
> > + /* 1 - Privileged */
> > + reqcfg.privileged = 1;
> > + /* 0 - Data access */
> > + reqcfg.insnaccess = 0;
> > + /* 0 - No endian swap */
> > + reqcfg.swap = 0;
> > +
> > + if (!plat->dma330.secure)
> > + reqcfg.nonsecure = 1; /* Non Secure mode */
> > + else
> > + reqcfg.nonsecure = 0; /* Secure mode */
> > +
> > + /* DMA burst length */
> > + reqcfg.brst_len = plat->brst_len;
> > + /* DMA burst size */
> > + reqcfg.brst_size = plat->brst_size;
> > + /* Preparing the CCR value */
> > + ccr = _prepare_ccr(&reqcfg);
> > + /* DMAMOV CCR, ccr */
> > + off += _emit_mov(&buf[off], CCR, ccr);
> > +
> > + /* BURST */
> > + /* Can initiate a burst? */
> > + while (data_size_byte >= brst_size * plat->brst_len) {
> > + lcnt0 = data_size_byte / (brst_size * plat-
> > >brst_len);
> > + lcnt1 = 0;
> > + if (lcnt0 >= 256 * 256) {
> > + lcnt0 = 256;
> > + lcnt1 = 256;
> > + } else if (lcnt0 >= 256) {
> > + lcnt1 = lcnt0 / 256;
> > + lcnt0 = 256;
> > + }
> > +
> > + data_size_byte = data_size_byte -
> > + (brst_size * plat->brst_len * lcnt0 *
> > lcnt1);
> > +
> > + debug("Transferring 0x%08x Remain 0x%08x\n",
> > (brst_size *
> > + plat->brst_len * lcnt0 * lcnt1),
> > data_size_byte);
> > + debug("Running burst - brst_size=2^%d,
> > brst_len=%d,",
> > + plat->brst_size, plat->brst_len);
> > + debug("lcnt0=%d, lcnt1=%d\n", lcnt0, lcnt1);
> > +
> > + if (lcnt1) {
> > + /* DMALP1 */
> > + off += _emit_lp(&buf[off], 1, lcnt1);
> > + loopjmp1 = off;
> > + }
> > + /* DMALP0 */
> > + off += _emit_lp(&buf[off], 0, lcnt0);
> > + loopjmp0 = off;
> > + /* DMALSTZ */
> > + off += _emit_stz(&buf[off]);
> > +
> > + WATCHDOG_RESET();
> > +
> > + /* DMALP0END */
> > + struct _arg_lpend lpend;
> > +
> > + lpend.cond = ALWAYS;
> > + lpend.forever = 0;
> > + /* Loop cnt 0 */
> > + lpend.loop = 0;
> > + lpend.bjump = off - loopjmp0;
> > + off += _emit_lpend(&buf[off], &lpend);
> > + /* DMALP1END */
> > + if (lcnt1) {
> > + struct _arg_lpend lpend;
> > +
> > + lpend.cond = ALWAYS;
> > + lpend.forever = 0;
> > + /* Loop cnt 1*/
> > + lpend.loop = 1;
> > + lpend.bjump = off - loopjmp1;
> > + off += _emit_lpend(&buf[off], &lpend);
> > + }
> > + /* Ensure the microcode don't exceed buffer size */
> > + if (off > plat->buf_size) {
> > + printf("off = %d\n", off);
> > + debug("ERROR DMA330 : Exceed buffer size
> > off %d\n",
> > + off);
> > + return -ENOMEM;
> > + }
> > + }
> > +
> > + /* SINGLE */
> > + brst_len = 1;
> > + /* brst_size = 2 ^ plat->dma330.single_brst_size */
> > + brst_size = (1 << plat->dma330.single_brst_size);
> > + lcnt0 = data_size_byte / (brst_size * brst_len);
> > +
> > + /* ensure all data will be transferred */
> > + data_size_byte = data_size_byte -
> > + (brst_size * brst_len * lcnt0);
> > + if (data_size_byte) {
> > + debug("ERROR DMA330 : Detected the possibility of
> > ");
> > + debug("untransfered data. Please ensure correct
> > single ");
> > + debug("burst size\n");
> > + }
> > +
> > + if (lcnt0) {
> > + debug("Transferring 0x%08x Remain 0x%08x\n",
> > (brst_size *
> > + brst_len * lcnt0), data_size_byte);
> > + debug("Running single - brst_size=2^%d,
> > brst_len=%d, ",
> > + brst_size, brst_len);
> > + debug("lcnt0=%d\n", lcnt0);
> > +
> > + /* Preparing the CCR value */
> > + /* DMA burst length */
> > + reqcfg.brst_len = brst_len;
> > + /* DMA burst size */
> > + reqcfg.brst_size = brst_size;
> > + ccr = _prepare_ccr(&reqcfg);
> > + /* DMAMOV CCR, ccr */
> > + off += _emit_mov(&buf[off], CCR, ccr);
> > +
> > + /* DMALP0 */
> > + off += _emit_lp(&buf[off], 0, lcnt0);
> > + loopjmp0 = off;
> > + /* DMALSTZ */
> > + off += _emit_stz(&buf[off]);
> > + /* DMALPEND */
> > + struct _arg_lpend lpend1;
> > +
> > + lpend1.cond = ALWAYS;
> > + lpend1.forever = 0;
> > + /* Loop cnt 0 */
> > + lpend1.loop = 0;
> > + lpend1.bjump = off - loopjmp0;
> > + off += _emit_lpend(&buf[off], &lpend1);
> > + /* Ensure the microcode don't exceed buffer size */
> > + if (off > plat->buf_size) {
> > + debug("ERROR DMA330 : Exceed buffer
> > size\n");
> > + return -ENOMEM;
> > + }
> > + }
> > +
> > + /* DMAEND */
> > + off += _emit_end(&buf[off]);
> > +
> > + ret = dma330_transfer_start(plat);
> > + if (ret)
> > + return ret;
> > +
> > + return dma330_transfer_finish(plat);
> > +}
> > +
> > +static int dma330_transfer_zeroes(struct udevice *dev, void *dst,
> > size_t len)
> > +{
> > + struct dma_dma330_platdata *plat = dev->platdata;
> > + int ret = 0;
> > +
> > + /* If DMA access is set to secure, base change to DMA
> > secure base */
> > + if (plat->dma330.secure)
> > + plat->base += 0x1000;
> What is this? I suggest you set up the secure base separately in
> 'plat', e.g. with a base_secure member. You should not change
> platform
> data while running.
>
> You can pass the base address to dma330_transfer_zeroes_setup,
> perhaps.
>
Okay.
> >
> > +
> > + plat->dma330.dst_addr = (phys_size_t)(uintptr_t)dst;
> > + plat->dma330.size_byte = len;
> > +
> > + ret = dma330_transfer_zeroes_setup(plat);
> > +
> > + /* Revert back to non secure DMA base */
> > + if (plat->dma330.secure)
> > + plat->base -= 0x1000;
> > +
> > + return ret;
> > +}
> > +
> > +static int dma330_transfer(struct udevice *dev, int direction,
> > void *dst,
> > + void *src, size_t len)
> > +{
> > + struct dma_dma330_platdata *plat = dev->platdata;
> > + int ret = 0;
> > +
> > + /* If DMA access is set to secure, base change to DMA
> > secure base */
> > + if (plat->dma330.secure)
> > + plat->base += 0x1000;
> Same as above
>
Okay.
> >
> > +
> > + plat->dma330.dst_addr = (phys_size_t)(uintptr_t)dst;
> > + plat->dma330.src_addr = (phys_size_t)(uintptr_t)src;
> > + plat->dma330.size_byte = len;
> > +
> > + switch (direction) {
> > + case DMA_MEM_TO_MEM:
> > + plat->dma330.transfer_type =
> > DMA_SUPPORTS_MEM_TO_MEM;
> > + break;
> > + case DMA_MEM_TO_DEV:
> > + plat->dma330.transfer_type =
> > DMA_SUPPORTS_MEM_TO_DEV;
> > + break;
> > + case DMA_DEV_TO_MEM:
> > + plat->dma330.transfer_type =
> > DMA_SUPPORTS_DEV_TO_MEM;
> > + break;
> > + }
> > +
> > + ret = dma330_transfer_setup(plat);
> > +
> > + /* Revert back to non secure DMA base */
> > + if (plat->dma330.secure)
> > + plat->base -= 0x1000;
> > +
> > + return ret;
> > +}
> > +
> > +static int dma330_ofdata_to_platdata(struct udevice *dev)
> > +{
> > + struct dma_dma330_platdata *plat = dev->platdata;
> > + const void *blob = gd->fdt_blob;
> > + int node = dev_of_offset(dev);
> > +
> > + plat->base = (void __iomem *)devfdt_get_addr(dev);
> dev_read_addr()
>
Okay.
> >
> >
> > + plat->max_brst_size = fdtdec_get_uint(blob, node, "dma-max-
> > burst-size",
> > + 2);
> Use dev_read API please (live tree)
>
Okay.
> > + plat->brst_size = fdtdec_get_uint(blob, node, "dma-burst-
> > size", 2);
> > + plat->brst_len = fdtdec_get_uint(blob, node, "dma-burst-
> > length", 2);
> > +
> > + return 0;
> > +}
> > +
> > +static int dma330_probe(struct udevice *adev)
> > +{
> > + struct dma_dev_priv *uc_priv = dev_get_uclass_priv(adev);
> > +
> > + uc_priv->supported = (DMA_SUPPORTS_MEM_TO_MEM |
> > + DMA_SUPPORTS_MEM_TO_DEV |
> > + DMA_SUPPORTS_DEV_TO_MEM);
> > + return 0;
> > +}
> > +
> > +static const struct dma_ops dma330_ops = {
> > + .transfer = dma330_transfer,
> > + .transfer_zeroes = dma330_transfer_zeroes,
> > +};
> > +
> > +static const struct udevice_id dma330_ids[] = {
> > + { .compatible = "arm,pl330" },
> > + { .compatible = "arm,dma330" },
> > + { /* sentinel */ }
> > +};
> > +
> > +U_BOOT_DRIVER(dma_dma330) = {
> > + .name = "dma_dma330",
> > + .id = UCLASS_DMA,
> > + .of_match = dma330_ids,
> > + .ops = &dma330_ops,
> > + .ofdata_to_platdata = dma330_ofdata_to_platdata,
> > + .probe = dma330_probe,
> > + .platdata_auto_alloc_size = sizeof(struct
> > dma_dma330_platdata),
> > +};
> > diff --git a/include/dma330.h b/include/dma330.h
> > new file mode 100644
> > index 0000000..c054bf2
> > --- /dev/null
> > +++ b/include/dma330.h
> > @@ -0,0 +1,136 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2018 Intel Corporation <www.intel.com>
> > + */
> > +
> > +#include <dma.h>
> > +#include <linux/sizes.h>
> > +
> > +#ifndef __DMA330_CORE_H
> > +#define __DMA330_CORE_H
> > +
> > +#define DMA330_MAX_CHAN 8
> > +#define DMA330_MAX_IRQS 32
> > +#define DMA330_MAX_PERI 32
> > +
> > +#define DMA330_STATE_STOPPED BIT(0)
> > +#define DMA330_STATE_EXECUTING BIT(1)
> > +#define DMA330_STATE_WFE BIT(2)
> > +#define DMA330_STATE_FAULTING BIT(3)
> > +#define DMA330_STATE_COMPLETING BIT(4)
> > +#define DMA330_STATE_WFP BIT(5)
> > +#define DMA330_STATE_KILLING BIT(6)
> > +#define DMA330_STATE_FAULT_COMPLETING BIT(7)
> > +#define DMA330_STATE_CACHEMISS BIT(8)
> > +#define DMA330_STATE_UPDTPC BIT(9)
> > +#define DMA330_STATE_ATBARRIER BIT(10)
> > +#define DMA330_STATE_QUEUEBUSY BIT(11))
> > +#define DMA330_STATE_INVALID BIT(15)
> > +
> > +enum dma330_srccachectrl {
> > + SCCTRL0 = 0, /* Noncacheable and nonbufferable */
> > + SCCTRL1, /* Bufferable only */
> > + SCCTRL2, /* Cacheable, but do not allocate */
> > + SCCTRL3, /* Cacheable and bufferable, but do not
> > allocate */
> > + SINVALID1,
> > + SINVALID2,
> > + SCCTRL6, /* Cacheable write-through, allocate on
> > reads only */
> > + SCCTRL7, /* Cacheable write-back, allocate on reads
> > only */
> > +};
> > +
> > +enum dma330_dstcachectrl {
> > + DCCTRL0 = 0, /* Noncacheable and nonbufferable */
> > + DCCTRL1, /* Bufferable only */
> > + DCCTRL2, /* Cacheable, but do not allocate */
> > + DCCTRL3, /* Cacheable and bufferable, but do not
> > allocate */
> > + DINVALID1 = 8,
> > + DINVALID2,
> > + DCCTRL6, /* Cacheable write-through, allocate on
> > writes only */
> > + DCCTRL7, /* Cacheable write-back, allocate on writes
> > only */
> > +};
> > +
> > +enum dma330_byteswap {
> > + SWAP_NO = 0,
> > + SWAP_2,
> > + SWAP_4,
> > + SWAP_8,
> > + SWAP_16,
> > +};
> > +
> > +/**
> > + * dma330_reqcfg - Request Configuration.
> > + *
> > + * The DMA330 core does not modify this and uses the last
> > + * working configuration if the request doesn't provide any.
> > + *
> > + * The Client may want to provide this info only for the
> > + * first request and a request with new settings.
> > + */
> > +struct dma330_reqcfg {
> > + /* Address Incrementing */
> > + u8 dst_inc;
> > + u8 src_inc;
> > +
> > + /*
> > + * For now, the SRC & DST protection levels
> > + * and burst size/length are assumed same.
> > + */
> > + u8 nonsecure;
> > + u8 privileged;
> > + u8 insnaccess;
> > + u8 brst_len;
> > + u8 brst_size; /* in power of 2 */
> > +
> > + enum dma330_dstcachectrl dcctl;
> > + enum dma330_srccachectrl scctl;
> > + enum dma330_byteswap swap;
> > +};
> > +
> > +/**
> > + * dma330_transfer_struct - Structure to be passed in for
> > dma330_transfer_x.
> > + *
> > + * @channel0_manager1: Switching configuration on DMA channel or
> > DMA manager.
> > + * @channel_num: Channel number assigned, valid from 0 to 7.
> > + * @src_addr: Address to transfer from / source.
> > + * @dst_addr: Address to transfer to / destination.
> > + * @size_byte: Number of bytes to be transferred.
> > + * @brst_size: Valid from 0 - 4,
> > + * where 0 = 1 (2 ^ 0) bytes and 3 = 16 bytes (2 ^ 4).
> > + * @max_brst_size: Max transfer size (from 0 - 4).
> > + * @single_brst_size: Single transfer size (from 0 - 4).
> > + * @brst_len: Valid from 1 - 16 where each burst can transfer 1 -
> > 16
> > + * Data chunk (each chunk size equivalent to brst_size).
> > + * @peripheral_id: Assigned peripheral_id, valid from 0 to 31.
> > + * @transfer_type: MEM2MEM, MEM2PERIPH or PERIPH2MEM.
> > + * @enable_cache1: 1 for cache enabled for memory
> > + * (cacheable and bufferable, but do not allocate).
> > + * @buf: Buffer handler which will point to the memory
> > + * allocated for dma microcode
> > + * @secure: Set DMA channel in secure mode.
> This is a good way to document structs. Please do it for the others.
>
> >
> > + *
> > + * Description of the structure.
> Yes?
>
Sure.
> >
> > + */
> > +struct dma330_transfer_struct {
> > + u32 channel0_manager1;
> > + u32 channel_num;
> > + phys_size_t src_addr;
> > + phys_size_t dst_addr;
> > + u32 size_byte;
> > + u32 single_brst_size;
> > + u32 peripheral_id;
> > + u32 transfer_type;
> > + u32 enable_cache1;
> > + u32 *buf;
> > + u8 secure;
> > +};
> > +
> > +struct dma_dma330_platdata {
> > + void __iomem *base;
> > + u32 buf_size;
> > + u32 max_brst_size;
> > + u32 brst_size;
> > + u32 brst_len;
> > + struct dma330_transfer_struct dma330;
> > +};
> > +
> > +#endif /* __DMA330_CORE_H */
> > --
> > 2.2.0
> >
> Regards,
> Simon
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 4/5] include: dma: Update the function description for dma_memcpy
2018-06-04 7:14 ` Chee, Tien Fong
@ 2018-06-08 21:59 ` Simon Glass
2018-06-11 5:05 ` Chee, Tien Fong
0 siblings, 1 reply; 27+ messages in thread
From: Simon Glass @ 2018-06-08 21:59 UTC (permalink / raw)
To: u-boot
Hi,
On 3 June 2018 at 23:14, Chee, Tien Fong <tien.fong.chee@intel.com> wrote:
> On Fri, 2018-06-01 at 08:25 -0600, Simon Glass wrote:
>> Hi,
>>
>> On 31 May 2018 at 02:08, <tien.fong.chee@intel.com> wrote:
>> >
>> > From: Tien Fong Chee <tien.fong.chee@intel.com>
>> >
>> > Update the dma_memcpy description on return argument for DMA330
>> > driver.
>> >
>> > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
>> > ---
>> > include/dma.h | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/include/dma.h b/include/dma.h
>> > index 0a0c9dd..b825c1e 100644
>> > --- a/include/dma.h
>> > +++ b/include/dma.h
>> > @@ -79,8 +79,8 @@ int dma_get_device(u32 transfer_type, struct
>> > udevice **devp);
>> > * @dst - destination pointer
>> > * @src - souce pointer
>> > * @len - data length to be copied
>> > - * @return - on successful transfer returns no of bytes
>> > - transferred and on failure return error code.
>> > + * @return - on successful transfer returns no of bytes or
>> > zero(for DMA330)
>> > + * transferred and on failure return error code.
>> This is a public API so you cannot change it just for one device. You
>> can change the API for everyone if you like.
>>
>> But why would it want to return 0?
>>
> Because we only able to check the DMA tranferring status, full transfer
> or failed. I can return the len argument user set if full tranfer is
> finished.
OK. My concern is that you have a comment saying that the function
does something different for one device versus others. This is not the
place for such a comment. Here you can just document that it can
return two possible meanings. You should add comments explaining what
0 means too (e.g. completed, but length unknown?).
For something in progress, you should use -EINPROGRESS / -EAGAIN
Regards,
Simon
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 4/5] include: dma: Update the function description for dma_memcpy
2018-06-08 21:59 ` Simon Glass
@ 2018-06-11 5:05 ` Chee, Tien Fong
0 siblings, 0 replies; 27+ messages in thread
From: Chee, Tien Fong @ 2018-06-11 5:05 UTC (permalink / raw)
To: u-boot
On Fri, 2018-06-08 at 13:59 -0800, Simon Glass wrote:
> Hi,
>
> On 3 June 2018 at 23:14, Chee, Tien Fong <tien.fong.chee@intel.com>
> wrote:
> >
> > On Fri, 2018-06-01 at 08:25 -0600, Simon Glass wrote:
> > >
> > > Hi,
> > >
> > > On 31 May 2018 at 02:08, <tien.fong.chee@intel.com> wrote:
> > > >
> > > >
> > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > >
> > > > Update the dma_memcpy description on return argument for DMA330
> > > > driver.
> > > >
> > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > ---
> > > > include/dma.h | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/include/dma.h b/include/dma.h
> > > > index 0a0c9dd..b825c1e 100644
> > > > --- a/include/dma.h
> > > > +++ b/include/dma.h
> > > > @@ -79,8 +79,8 @@ int dma_get_device(u32 transfer_type, struct
> > > > udevice **devp);
> > > > * @dst - destination pointer
> > > > * @src - souce pointer
> > > > * @len - data length to be copied
> > > > - * @return - on successful transfer returns no of bytes
> > > > - transferred and on failure return error code.
> > > > + * @return - on successful transfer returns no of bytes or
> > > > zero(for DMA330)
> > > > + * transferred and on failure return error code.
> > > This is a public API so you cannot change it just for one device.
> > > You
> > > can change the API for everyone if you like.
> > >
> > > But why would it want to return 0?
> > >
> > Because we only able to check the DMA tranferring status, full
> > transfer
> > or failed. I can return the len argument user set if full tranfer
> > is
> > finished.
> OK. My concern is that you have a comment saying that the function
> does something different for one device versus others. This is not
> the
> place for such a comment. Here you can just document that it can
> return two possible meanings. You should add comments explaining what
> 0 means too (e.g. completed, but length unknown?).
>
> For something in progress, you should use -EINPROGRESS / -EAGAIN
>
Okay.
> Regards,
> Simon
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 5/5] arm: dts: socfpga: stratix10: update pdma
2018-05-31 8:08 ` [U-Boot] [PATCH 5/5] arm: dts: socfpga: stratix10: update pdma tien.fong.chee at intel.com
2018-06-01 14:25 ` Simon Glass
@ 2018-07-09 18:03 ` Dinh Nguyen
2018-07-09 20:28 ` Marek Vasut
1 sibling, 1 reply; 27+ messages in thread
From: Dinh Nguyen @ 2018-07-09 18:03 UTC (permalink / raw)
To: u-boot
On 05/31/2018 03:08 AM, tien.fong.chee at intel.com wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
>
> Update pdma properties for Stratix 10
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> ---
> arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/arch/arm/dts/socfpga_stratix10.dtsi b/arch/arm/dts/socfpga_stratix10.dtsi
> index ccd3f32..311ba09 100644
> --- a/arch/arm/dts/socfpga_stratix10.dtsi
> +++ b/arch/arm/dts/socfpga_stratix10.dtsi
> @@ -82,6 +82,26 @@
> ranges = <0 0 0 0xffffffff>;
> u-boot,dm-pre-reloc;
>
> + amba {
> + u-boot,dm-pre-reloc;
> + compatible = "arm,amba-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + pdma: pdma at ffda0000 {
> + u-boot,dm-pre-reloc;
> + compatible = "arm,pl330", "arm,dma330";
I think you got "arm,dma330" binding wrong. I don't see any binding with
that name.
Dinh
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 5/5] arm: dts: socfpga: stratix10: update pdma
2018-07-09 18:03 ` Dinh Nguyen
@ 2018-07-09 20:28 ` Marek Vasut
2018-07-10 13:11 ` Chee, Tien Fong
0 siblings, 1 reply; 27+ messages in thread
From: Marek Vasut @ 2018-07-09 20:28 UTC (permalink / raw)
To: u-boot
On 07/09/2018 08:03 PM, Dinh Nguyen wrote:
>
>
> On 05/31/2018 03:08 AM, tien.fong.chee at intel.com wrote:
>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>
>> Update pdma properties for Stratix 10
>>
>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
>> ---
>> arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/arch/arm/dts/socfpga_stratix10.dtsi b/arch/arm/dts/socfpga_stratix10.dtsi
>> index ccd3f32..311ba09 100644
>> --- a/arch/arm/dts/socfpga_stratix10.dtsi
>> +++ b/arch/arm/dts/socfpga_stratix10.dtsi
>> @@ -82,6 +82,26 @@
>> ranges = <0 0 0 0xffffffff>;
>> u-boot,dm-pre-reloc;
>>
>> + amba {
>> + u-boot,dm-pre-reloc;
>> + compatible = "arm,amba-bus";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges;
>> +
>> + pdma: pdma at ffda0000 {
>> + u-boot,dm-pre-reloc;
>> + compatible = "arm,pl330", "arm,dma330";
>
> I think you got "arm,dma330" binding wrong. I don't see any binding with
> that name.
I think the whole idea of using pl330 to scrub ECC DRAM is wrong. It
adds massive amount of code while a CPU can do the same and faster, cfr
arria10.
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 5/5] arm: dts: socfpga: stratix10: update pdma
2018-07-09 20:28 ` Marek Vasut
@ 2018-07-10 13:11 ` Chee, Tien Fong
2018-07-10 13:37 ` Dinh Nguyen
2018-07-10 16:13 ` Marek Vasut
0 siblings, 2 replies; 27+ messages in thread
From: Chee, Tien Fong @ 2018-07-10 13:11 UTC (permalink / raw)
To: u-boot
On Mon, 2018-07-09 at 22:28 +0200, Marek Vasut wrote:
> On 07/09/2018 08:03 PM, Dinh Nguyen wrote:
> >
> >
> >
> > On 05/31/2018 03:08 AM, tien.fong.chee at intel.com wrote:
> > >
> > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > >
> > > Update pdma properties for Stratix 10
> > >
> > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > > ---
> > > arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++
> > > 1 file changed, 20 insertions(+)
> > >
> > > diff --git a/arch/arm/dts/socfpga_stratix10.dtsi
> > > b/arch/arm/dts/socfpga_stratix10.dtsi
> > > index ccd3f32..311ba09 100644
> > > --- a/arch/arm/dts/socfpga_stratix10.dtsi
> > > +++ b/arch/arm/dts/socfpga_stratix10.dtsi
> > > @@ -82,6 +82,26 @@
> > > ranges = <0 0 0 0xffffffff>;
> > > u-boot,dm-pre-reloc;
> > >
> > > + amba {
> > > + u-boot,dm-pre-reloc;
> > > + compatible = "arm,amba-bus";
> > > + #address-cells = <1>;
> > > + #size-cells = <1>;
> > > + ranges;
> > > +
> > > + pdma: pdma at ffda0000 {
> > > + u-boot,dm-pre-reloc;
> > > + compatible =
> > > "arm,pl330", "arm,dma330";
> > I think you got "arm,dma330" binding wrong. I don't see any binding
> > with
> > that name.
Here https://patchwork.ozlabs.org/patch/923234/ .
> I think the whole idea of using pl330 to scrub ECC DRAM is wrong. It
> adds massive amount of code while a CPU can do the same and faster,
> cfr
> arria10.
>
I just measured the performance of initializing DRAM based on custodian
arria10_sdmmc, which is around 16sec with 1GB.
Using DMA to init the DDR, which is around 3-4sec for 2GB. I attached
screenshot for the print out based on arria10_sdmmc custodian.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sdram_ecc_timing.png
Type: image/png
Size: 119153 bytes
Desc: sdram_ecc_timing.png
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180710/bf2d3d41/attachment.png>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 5/5] arm: dts: socfpga: stratix10: update pdma
2018-07-10 13:11 ` Chee, Tien Fong
@ 2018-07-10 13:37 ` Dinh Nguyen
2018-07-11 2:55 ` Chee, Tien Fong
2018-07-10 16:13 ` Marek Vasut
1 sibling, 1 reply; 27+ messages in thread
From: Dinh Nguyen @ 2018-07-10 13:37 UTC (permalink / raw)
To: u-boot
On 07/10/2018 08:11 AM, Chee, Tien Fong wrote:
> On Mon, 2018-07-09 at 22:28 +0200, Marek Vasut wrote:
>> On 07/09/2018 08:03 PM, Dinh Nguyen wrote:
>>>
>>>
>>>
>>> On 05/31/2018 03:08 AM, tien.fong.chee at intel.com wrote:
>>>>
>>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>>
>>>> Update pdma properties for Stratix 10
>>>>
>>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
>>>> ---
>>>> arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++
>>>> 1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/arch/arm/dts/socfpga_stratix10.dtsi
>>>> b/arch/arm/dts/socfpga_stratix10.dtsi
>>>> index ccd3f32..311ba09 100644
>>>> --- a/arch/arm/dts/socfpga_stratix10.dtsi
>>>> +++ b/arch/arm/dts/socfpga_stratix10.dtsi
>>>> @@ -82,6 +82,26 @@
>>>> ranges = <0 0 0 0xffffffff>;
>>>> u-boot,dm-pre-reloc;
>>>>
>>>> + amba {
>>>> + u-boot,dm-pre-reloc;
>>>> + compatible = "arm,amba-bus";
>>>> + #address-cells = <1>;
>>>> + #size-cells = <1>;
>>>> + ranges;
>>>> +
>>>> + pdma: pdma at ffda0000 {
>>>> + u-boot,dm-pre-reloc;
>>>> + compatible =
>>>> "arm,pl330", "arm,dma330";
>>> I think you got "arm,dma330" binding wrong. I don't see any binding
>>> with
>>> that name.
> Here https://patchwork.ozlabs.org/patch/923234/ .
Oh okay...why do you need to add the additional binding "arm,dma330"?
Dinh
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 5/5] arm: dts: socfpga: stratix10: update pdma
2018-07-10 13:11 ` Chee, Tien Fong
2018-07-10 13:37 ` Dinh Nguyen
@ 2018-07-10 16:13 ` Marek Vasut
1 sibling, 0 replies; 27+ messages in thread
From: Marek Vasut @ 2018-07-10 16:13 UTC (permalink / raw)
To: u-boot
On 07/10/2018 03:11 PM, Chee, Tien Fong wrote:
> On Mon, 2018-07-09 at 22:28 +0200, Marek Vasut wrote:
>> On 07/09/2018 08:03 PM, Dinh Nguyen wrote:
>>>
>>>
>>>
>>> On 05/31/2018 03:08 AM, tien.fong.chee at intel.com wrote:
>>>>
>>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>>
>>>> Update pdma properties for Stratix 10
>>>>
>>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
>>>> ---
>>>> arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++
>>>> 1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/arch/arm/dts/socfpga_stratix10.dtsi
>>>> b/arch/arm/dts/socfpga_stratix10.dtsi
>>>> index ccd3f32..311ba09 100644
>>>> --- a/arch/arm/dts/socfpga_stratix10.dtsi
>>>> +++ b/arch/arm/dts/socfpga_stratix10.dtsi
>>>> @@ -82,6 +82,26 @@
>>>> ranges = <0 0 0 0xffffffff>;
>>>> u-boot,dm-pre-reloc;
>>>>
>>>> + amba {
>>>> + u-boot,dm-pre-reloc;
>>>> + compatible = "arm,amba-bus";
>>>> + #address-cells = <1>;
>>>> + #size-cells = <1>;
>>>> + ranges;
>>>> +
>>>> + pdma: pdma at ffda0000 {
>>>> + u-boot,dm-pre-reloc;
>>>> + compatible =
>>>> "arm,pl330", "arm,dma330";
>>> I think you got "arm,dma330" binding wrong. I don't see any binding
>>> with
>>> that name.
> Here https://patchwork.ozlabs.org/patch/923234/ .
>> I think the whole idea of using pl330 to scrub ECC DRAM is wrong. It
>> adds massive amount of code while a CPU can do the same and faster,
>> cfr
>> arria10.
>>
> I just measured the performance of initializing DRAM based on custodian
> arria10_sdmmc, which is around 16sec with 1GB.
Then you're doing something very wrong, it should be around 1 second.
See ie. "SoCFPGA PL330 DMA driver and ECC scrubbing" thread
> Using DMA to init the DDR, which is around 3-4sec for 2GB. I attached
> screenshot for the print out based on arria10_sdmmc custodian.
Well sure, if you do it wrong, it'll take longer for the CPU.
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 5/5] arm: dts: socfpga: stratix10: update pdma
2018-07-10 13:37 ` Dinh Nguyen
@ 2018-07-11 2:55 ` Chee, Tien Fong
0 siblings, 0 replies; 27+ messages in thread
From: Chee, Tien Fong @ 2018-07-11 2:55 UTC (permalink / raw)
To: u-boot
On Tue, 2018-07-10 at 08:37 -0500, Dinh Nguyen wrote:
>
> On 07/10/2018 08:11 AM, Chee, Tien Fong wrote:
> >
> > On Mon, 2018-07-09 at 22:28 +0200, Marek Vasut wrote:
> > >
> > > On 07/09/2018 08:03 PM, Dinh Nguyen wrote:
> > > >
> > > >
> > > >
> > > >
> > > > On 05/31/2018 03:08 AM, tien.fong.chee at intel.com wrote:
> > > > >
> > > > >
> > > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > >
> > > > > Update pdma properties for Stratix 10
> > > > >
> > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > > ---
> > > > > arch/arm/dts/socfpga_stratix10.dtsi | 20
> > > > > ++++++++++++++++++++
> > > > > 1 file changed, 20 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm/dts/socfpga_stratix10.dtsi
> > > > > b/arch/arm/dts/socfpga_stratix10.dtsi
> > > > > index ccd3f32..311ba09 100644
> > > > > --- a/arch/arm/dts/socfpga_stratix10.dtsi
> > > > > +++ b/arch/arm/dts/socfpga_stratix10.dtsi
> > > > > @@ -82,6 +82,26 @@
> > > > > ranges = <0 0 0 0xffffffff>;
> > > > > u-boot,dm-pre-reloc;
> > > > >
> > > > > + amba {
> > > > > + u-boot,dm-pre-reloc;
> > > > > + compatible = "arm,amba-bus";
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <1>;
> > > > > + ranges;
> > > > > +
> > > > > + pdma: pdma at ffda0000 {
> > > > > + u-boot,dm-pre-reloc;
> > > > > + compatible =
> > > > > "arm,pl330", "arm,dma330";
> > > > I think you got "arm,dma330" binding wrong. I don't see any
> > > > binding
> > > > with
> > > > that name.
> > Here https://patchwork.ozlabs.org/patch/923234/ .
> Oh okay...why do you need to add the additional binding "arm,dma330"?
pl330 is old name, now arm using dma330.
>
> Dinh
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2018-07-11 2:55 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-31 8:08 [U-Boot] [PATCH 0/5] Add DMA driver for DMA330 controller tien.fong.chee at intel.com
2018-05-31 8:08 ` [U-Boot] [PATCH 1/5] drivers: dma: Enable DMA-330 driver support tien.fong.chee at intel.com
2018-06-01 14:24 ` Simon Glass
2018-06-06 5:22 ` Chee, Tien Fong
2018-05-31 8:08 ` [U-Boot] [PATCH 2/5] drivers: dma: Add function to zeroes a range of destination such as memory tien.fong.chee at intel.com
2018-06-01 14:24 ` Simon Glass
2018-06-04 7:34 ` Chee, Tien Fong
2018-05-31 8:08 ` [U-Boot] [PATCH 3/5] drivers: dma: Factor out dma_get_device from DMA class function tien.fong.chee at intel.com
2018-06-01 14:25 ` Simon Glass
2018-05-31 8:08 ` [U-Boot] [PATCH 4/5] include: dma: Update the function description for dma_memcpy tien.fong.chee at intel.com
2018-06-01 14:25 ` Simon Glass
2018-06-04 7:14 ` Chee, Tien Fong
2018-06-08 21:59 ` Simon Glass
2018-06-11 5:05 ` Chee, Tien Fong
2018-05-31 8:08 ` [U-Boot] [PATCH 5/5] arm: dts: socfpga: stratix10: update pdma tien.fong.chee at intel.com
2018-06-01 14:25 ` Simon Glass
2018-06-04 7:01 ` Chee, Tien Fong
2018-07-09 18:03 ` Dinh Nguyen
2018-07-09 20:28 ` Marek Vasut
2018-07-10 13:11 ` Chee, Tien Fong
2018-07-10 13:37 ` Dinh Nguyen
2018-07-11 2:55 ` Chee, Tien Fong
2018-07-10 16:13 ` Marek Vasut
2018-05-31 9:24 ` [U-Boot] [PATCH 0/5] Add DMA driver for DMA330 controller Marek Vasut
2018-05-31 9:50 ` See, Chin Liang
2018-05-31 9:58 ` Marek Vasut
2018-06-04 6:47 ` Chee, Tien Fong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.