Add DMA driver for DMA330 controller
Subsystems (3)
cover Add DMA driver for DMA330 controller
From: Tien Fong Chee <[email protected]>
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
Signature · 1 line
2.2.0
PATCH 1/5 drivers: dma: Enable DMA-330 driver support
From: Tien Fong Chee <[email protected]>
Enable DMAC driver support for DMA-330 controller. The driver is also compatible to PL330 product.
--- 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
drivers/dma/Kconfig +8 -1
@@ -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"
drivers/dma/Makefile +1 -0
@@ -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
drivers/dma/dma330.c new file +1514 -0
1,514 lines not on this page · raw message
include/dma330.h new file +136 -0
136 lines not on this page · raw message
Signature · 1 line
2.2.0
PATCH 1/5 drivers: dma: Enable DMA-330 driver support
Hi Tien,
Quoted 74 lines · On 31 May 2018 at 02:08, <[email protected]> wrote:
From: Tien Fong Chee <[email protected]>
Enable DMAC driver support for DMA-330 controller. The driver is also compatible to PL330 product.
Signed-off-by Tien Fong Chee <[email protected]>--- 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.hdrivers/dma/Kconfig +8
-1@@ -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"drivers/dma/Makefile +1
-0@@ -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.odrivers/dma/dma330.c new file +19
-0@@ -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
drivers/dma/dma330.c line 29
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+#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,
...
}
Quoted 48 lines
drivers/dma/dma330.c new file +48
-0+ +/* 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)
drivers/dma/dma330.c line 87
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+#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)
Quoted 114 lines
drivers/dma/dma330.c new file +114
-0+#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))) +
drivers/dma/dma330.c line 211
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+/* 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?
drivers/dma/dma330.c line 213
Quoted 2 lines
drivers/dma/dma330.c new file +2
-0+ +/* Enum declaration */
We know that :-)
Can you please add comments explaining what these enums are for, and what the values mean?
Quoted 17 lines
drivers/dma/dma330.c new file +17
-0+enum dmamov_dst { +SAR =0 , +CCR , +DAR , +}; + +enum dma330_dst { +SRC =0 , +DST , +}; + +enum dma330_cond { +SINGLE , +BURST , +ALWAYS , +}; +
drivers/dma/dma330.c line 240
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+/* 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.
drivers/dma/dma330.c line 253
Quoted 13 lines
drivers/dma/dma330.c new file +13
-0+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.
Quoted 11 lines
drivers/dma/dma330.c new file +11
-0+{ + 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.
drivers/dma/dma330.c line 274
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+* +* 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.
Quoted 8 lines
drivers/dma/dma330.c new file +8
-0+ buffer[1 ] = peri; +DMA330_DBGCMD_DUMP (SZ_DMAFLUSHP ,"\tDMAFLUSHP %u\n" , peri >>3 ); +return SZ_DMAFLUSHP ; +} + +/** +* _emit_ld - Add opcode DMALD into microcode (load). +*
drivers/dma/dma330.c line 292
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+* @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
Quoted 29 lines
drivers/dma/dma330.c new file +29
-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 ; +}
drivers/dma/dma330.c line 331
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+ +/** +* _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.
drivers/dma/dma330.c line 341
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+* +* 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?
drivers/dma/dma330.c line 345
Quoted 4 lines
drivers/dma/dma330.c new file +4
-0+ + buffer[0 ] =CMD_DMALPEND ; +if (loop) + buffer[0 ] |= (1 <<2 );
More open-coded things
Quoted 39 lines
drivers/dma/dma330.c new file +39
-0+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. +*/
drivers/dma/dma330.c line 394
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+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.
Quoted 7 lines
drivers/dma/dma330.c new file +7
-0+DMA330_DBGCMD_DUMP (SZ_DMAMOV ,"\tDMAMOV %s 0x%x\n" , + dst ==SAR ?"SAR" : (dst ==DAR ?"DAR" :"CCR" ), + val); +return SZ_DMAMOV ; +} + +/**
drivers/dma/dma330.c line 411
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+* _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'
Quoted 106 lines
drivers/dma/dma330.c new file +106
-0+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)
drivers/dma/dma330.c line 527
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+{ +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.
Quoted 32 lines
drivers/dma/dma330.c new file +32
-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
drivers/dma/dma330.c line 569
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+* (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.
Quoted 32 lines
drivers/dma/dma330.c new file +32
-0+ + 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)
drivers/dma/dma330.c line 611
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+ 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.
Quoted 300 lines
drivers/dma/dma330.c new file +300
-0+ + 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
drivers/dma/dma330.c line 921
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+* 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?
Quoted 114 lines
drivers/dma/dma330.c new file +114
-0+{ +/* 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 ;
drivers/dma/dma330.c line 1045
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+ }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.
drivers/dma/dma330.c line 1057
Quoted 12 lines
drivers/dma/dma330.c new file +12
-0+ 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
Quoted 15 lines
drivers/dma/dma330.c new file +15
-0+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);
drivers/dma/dma330.c line 1082
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+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?
Quoted 125 lines
drivers/dma/dma330.c new file +125
-0+ 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); +} +
drivers/dma/dma330.c line 1217
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+/** +* 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.
Quoted 194 lines
drivers/dma/dma330.c new file +194
-0+{ +/* 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);
drivers/dma/dma330.c line 1421
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+} + +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.
Quoted 12 lines
drivers/dma/dma330.c new file +12
-0+ + 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; +}
drivers/dma/dma330.c line 1443
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+ +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
Quoted 23 lines
drivers/dma/dma330.c new file +23
-0+ + 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 ; +
drivers/dma/dma330.c line 1476
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+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()
drivers/dma/dma330.c line 1478
Quoted 2 lines
drivers/dma/dma330.c new file +2
-0+ plat->max_brst_size =fdtdec_get_uint (blob, node,"dma-max-burst-size" , +2 );
Use dev_read API please (live tree)
Quoted 141 lines
drivers/dma/dma330.c new file +36
-0+ 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 ), +};include/dma330.h new file +99
-0@@ -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).
include/dma330.h line 109
Quoted 10 lines
include/dma330.h new file +10
-0+* @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.
include/dma330.h line 111
Quoted 2 lines
include/dma330.h new file +2
-0+* +* Description of the structure.
Yes?
Quoted 26 lines
include/dma330.h new file +25
-1+*/ +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 */ --
Quoted 2 lines
2.2.0
Regards,
Simon
Chee
PATCH 1/5 drivers: dma: Enable DMA-330 driver support
Quoted 91 lines · Simon Glass
Hi Tien,
Quoted 76 lines · On 31 May 2018 at 02:08, <[email protected]> wrote:
From: Tien Fong Chee <[email protected]>
Enable DMAC driver support for DMA-330 controller. The driver is also compatible to PL330 product.
Signed-off-by Tien Fong Chee <[email protected]>---
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 100644include/dma330.hdrivers/dma/Kconfig +0
-0@@ -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 hostdrivers/dma/Kconfig +0
-0@@ -34,4 +34,11 @@ config APBH_DMA_BURST8endif
drivers/dma/Kconfig +7
-0+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"
drivers/dma/Makefile +0
-0@@ -11,3 +11,4 @@ obj-$(CONFIG_FSL_DMA) += fsl_dma.oobj-$(CONFIG_TI_KSNAV) += keystone_nav.o keystone_nav_cfg.o obj-$(CONFIG_TI_EDMA3) += ti-edma3.o obj-$(CONFIG_DMA_LPC32XX) += lpc32xx_dma.o
drivers/dma/Makefile +1
-0+obj-$(CONFIG_DMA330_DMA) += dma330.odrivers/dma/dma330.c new file +19
-0@@ -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 0x1drivers/dma/dma330.c line 29
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+#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 0xfIt is possible to use enum for some of these?
Quoted 6 lines
enum {
DS = 0,
DS_ST_STOP,
...
}
Okay.
Quoted 62 lines
Quoted 49 lines
drivers/dma/dma330.c new file +48
-0+ +/* 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)drivers/dma/dma330.c line 87
Quoted 10 lines
drivers/dma/dma330.c new file +10
-0+#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 0x7Can you make the mask a shifted mask? Then it is easier to use in
clrsetbits_le32(), for example.
Quoted 4 lines
e.g.
#define
CC_SRCCCTRL_MASK(7 <<CC_SRCCCTRL_SHFT)
Okay.
Quoted 121 lines
Quoted 121 lines
drivers/dma/dma330.c new file +118
-0+#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); \
Quoted 9 lines
drivers/dma/dma330.c line 211
Quoted 5 lines
+ WATCHDOG_RESET(); \
+ } while (0)drivers/dma/dma330.c new file +3
-0+#else +#define DMA330_DBGCMD_DUMP(off, x...) do {} while (0) +#endif Can
DMA330_DBGCMD_DUMPbe a static function, and lower case? It's only
used in one file, right?
Okay, yes.
Quoted 8 lines
drivers/dma/dma330.c
Quoted 3 lines
drivers/dma/dma330.c new file +2
-0+ +/* Enum declaration */ We know that :-)
Can you please add comments explaining what these enums are for, and what the values mean?
Sure.
Quoted 22 lines
Quoted 22 lines
drivers/dma/dma330.c new file +21
-0+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;
Quoted 9 lines
drivers/dma/dma330.c line 240
Quoted 6 lines
+ u32 loop;
+ u8 bjump;drivers/dma/dma330.c new file +4
-0+}; + +/* Structure will be used by _emit_go function */ +struct _arg_go {Please drop the _ on these struct names, and document the members here.
Okay.
Quoted 10 lines
Quoted 10 lines
+ u8 chan;
+ u32 addr;
+ u32 ns;drivers/dma/dma330.c new file +6
-0+}; + +/* +* _emit_end - Add opcode DMAEND into microcode (end). +* +* @buf: The buffer which stored the microcode program.
Quoted 9 lines
Quoted 4 lines
+ *
+ * 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.
Quoted 17 lines
Quoted 17 lines
drivers/dma/dma330.c new file +8
-0+{ + 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)
Quoted 9 lines
drivers/dma/dma330.c line 274
Quoted 6 lines
drivers/dma/dma330.c new file +6
-0+{ + 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.
Quoted 13 lines
Quoted 13 lines
+ buffer[1] = peri;
+ DMA330_DBGCMD_DUMP(SZ_DMAFLUSHP, "\tDMAFLUSHP %u\n", periQuoted 1 line
Quoted 1 line
3);
+ return
SZ_DMAFLUSHP;drivers/dma/dma330.c new file +8
-0+} + +/** +* _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. +*
Quoted 9 lines
drivers/dma/dma330.c line 292
Quoted 7 lines
+ * Return: Size of opcode.
+ */drivers/dma/dma330.c new file +5
-0+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.
Quoted 38 lines
Quoted 38 lines
+ 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'));
+ returnSZ_DMALD;drivers/dma/dma330.c new file +22
-0+} + +/** +* _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);
+ returnSZ_DMALP;drivers/dma/dma330.c new file +7
-0+} + +/** +* _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.
Quoted 9 lines
Quoted 6 lines
+ * 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.
Quoted 14 lines
Quoted 12 lines
+ *
+ * Return: Size of opcode.
+ */
+static inline u32_emit_lpend(u8 buf[], const struct _arg_lpend *arg)
+{
+ u8 *buffer = buf;
+ enumdma330_condcond =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.
Quoted 7 lines
Quoted 5 lines
+
+ buffer[0] =CMD_DMALPEND;
+ if (loop)
+ buffer[0] |= (1 << 2);More open-coded things
noted.
Quoted 46 lines
Quoted 46 lines
+ 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);
+ returnSZ_DMALPEND;drivers/dma/dma330.c new file +26
-0+} + +/** +* _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, u32val)
+{
+ u8 *buffer = buf;
Quoted 9 lines
Quoted 7 lines
+
+ 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.
Quoted 13 lines
Quoted 13 lines
+ DMA330_DBGCMD_DUMP(SZ_DMAMOV, "\tDMAMOV %s 0x%x\n", + dst == SAR ? "SAR" : (dst == DAR ? "DAR" : "CCR"),
+ val);
+ returnSZ_DMAMOV;drivers/dma/dma330.c new file +6
-0+} + +/** +* _emit_nop - Add opcode DMANOP into microcode (no operation). +* +* @buf: The buffer which stored the microcode program.
Quoted 9 lines
drivers/dma/dma330.c line 411
Quoted 7 lines
+ *
+ * Return: Size of opcode.
+ */drivers/dma/dma330.c new file +4
-0+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.
Quoted 117 lines
Quoted 117 lines
+ return
SZ_DMANOP;drivers/dma/dma330.c new file +4
-0+} + +/** +* _emit_rmb - Add opcode DMARMB into microcode (read memory barrier).
+ *
+ * @buf: The buffer which stored the microcode program.
+ *
+ * Return: Size of opcode.
+ */drivers/dma/dma330.c new file +45
-0+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'));
+ returnSZ_DMAST;drivers/dma/dma330.c new file +4
-0+} + +/** +* _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)drivers/dma/dma330.c new file +30
-0+{ + 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)drivers/dma/dma330.c new file +4
-0+{ + u8 *buffer = buf; + + buffer[0] = CMD_DMAWFP;
Quoted 9 lines
Quoted 6 lines
+ 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.
Quoted 40 lines
Quoted 40 lines
+
+ 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);
+ returnSZ_DMAWFP;drivers/dma/dma330.c new file +4
-0+} + +/** +* _emit_wmb - Add opcode DMAWMB into microcode (write memory barrier).
+ *
+ * @buf: The buffer which stored the microcode program.
+ *
+ * Return: Size of opcode.
+ */drivers/dma/dma330.c new file +21
-0+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)
Quoted 8 lines
Quoted 5 lines
+{
+ 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_unalignedthing.
Okay.
Quoted 36 lines
Quoted 36 lines
drivers/dma/dma330.c new file +35
-0+ + 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);
Quoted 8 lines
drivers/dma/dma330.c line 611
Quoted 7 lines
+ ccr |= (((
rqc->brst_len- 1) &0xf) <<CC_DSTBRSTLEN_SHFT);drivers/dma/dma330.c new file +6
-0+ + 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.
Quoted 338 lines
Quoted 338 lines
drivers/dma/dma330.c new file +15
-0+ + 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 u32timeout_ms)drivers/dma/dma330.c new file +5
-0+{ + void __iomem *regs = plat->base; + + return wait_for_bit(__func__, + (const u32 *)(uintptr_t)(regs +DBGSTATUS),
+ DBG_BUSY, 0,timeout_ms, false);drivers/dma/dma330.c new file +4
-0+} + +/** +* dma330_execute_dbginsn - Execute debug instruction such as DMAGO and DMAKILL.
+ *
+ * @insn: The buffer which stored the debug instruction.
+ * @plat: Pointer to structdma_dma330_platdata.
+ * @timeout_ms: Timeout (in miliseconds).
+ *
+ * Return: void.
+ */
+static inline void dma330_execute_dbginsn(u8 insn[], + structdma_dma330_platdata*plat,
+ const u32timeout_ms)drivers/dma/dma330.c new file +8
-0+{ + 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);
drivers/dma/dma330.c new file +18
-0+ + /* 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)
drivers/dma/dma330.c new file +8
-0+{ + 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-Quoted 1 line
dma330.channel_num))) &
+ 0xf;
drivers/dma/dma330.c new file +56
-0+ + 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.
+ */drivers/dma/dma330.c new file +8
-0+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;drivers/dma/dma330.c new file +24
-0+ + 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 voiddma330_stop(struct dma_dma330_platdata *plat, const u32 timeout_ms)drivers/dma/dma330.c new file +4
-0+{ + u8 insn[6] = {0, 0, 0, 0, 0, 0}; + + /* If fault completing, wait until reach faulting andkilling state */
+ if (dma330_getstate(plat)==DMA330_STATE_FAULT_COMPLETING) + UNTIL(plat,DMA330_STATE_FAULTING|DMA330_STATE_KILLING);drivers/dma/dma330.c new file +16
-0+ + /* 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 structdma_dma330_platdata.
+ * @timeout_ms: Timeout (in miliseconds).
+ *
+ * Return: Negative value for error where DMA is not ready. 0 for successful.
+ */
+static intdma330_start(struct dma_dma330_platdata *plat, const u32 timeout_ms)drivers/dma/dma330.c new file +7
-0+{ + 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);drivers/dma/dma330.c new file +36
-0+ + 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.
+ */drivers/dma/dma330.c new file +10
-0+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 structdma_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 intdma330_transfer_finish(struct dma_dma330_platdata *plat)drivers/dma/dma330.c new file +5
-0+{ + void __iomem *regs = plat->base; + + if (!plat->dma330.buf) { + debug("ERROR DMA330 : DMA Microcode buffer pointeris NULL\n");
+ return-EINVAL;
+ }drivers/dma/dma330.c new file +4
-0+ + plat->dma330.channel0_manager1 = 0; + + /* Wait until finish execution to ensure we comparedcorrect 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;drivers/dma/dma330.c new file +4
-0+} + +/** +* dma330_transfer_setup - DMA transfer setup (DMA_MEM_TO_MEM,
DMA_MEM_TO_DEV
+ * orDMA_DEV_TO_MEM).
+ *
+ * @plat: Pointer to structdma_dma330_platdata.
+ *
+ * For Peripheral transfer, the FIFO threshold value is expected at
+ * 2 ^plat->brst_size*plat->brst_len.
Quoted 9 lines
Quoted 5 lines
+ *
+ * Return: Negative value for error or not successful. 0 for successful.
+ */
+static intdma330_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.
Quoted 130 lines
Quoted 130 lines
+{
+ /* 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;
+ u32brst_size= 0;
+ u32brst_len= 0;
+ u32data_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;
+ structdma330_reqcfgreqcfg;
+
+ if (!buf) {
+ debug("ERROR DMA330 : DMA Microcode buffer pointer is NULL\n");
+ return-EINVAL;
+ }drivers/dma/dma330.c new file +17
-0+ + 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-Quoted 1 line
brst_len,+ plat->dma330.single_brst_size);
drivers/dma/dma330.c new file +8
-0+ + /* 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-Quoted 1 line
max_brst_size) {+ debug("ERROR DMA330 :
single_brst_sizemust 0-%d (not %d)\n",
+ plat->max_brst_size, plat-Quoted 1 line
dma330.single_brst_size);
+ return
-EINVAL;
+ }
+ if (plat->brst_len< 1 ||plat->brst_len> 16) { + debug("ERROR DMA330 :brst_lenmust 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); + /* DMAFLUSHPP(periheral_id)*/
+ if (plat->dma330.transfer_type!=DMA_MEM_TO_MEM) + off += _emit_flushp(&buf[off], plat-Quoted 1 line
dma330.peripheral_id);
drivers/dma/dma330.c new file +31
-0+ + /* 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;
+ }
Quoted 9 lines
Quoted 6 lines
+ }
+ /* 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.
Quoted 13 lines
Quoted 13 lines
+ 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? */
Quoted 2 lines
These two comments should be joined
Okay. Actually comments helping to track the operation easily.
Quoted 24 lines
Quoted 24 lines
+ while (
data_size_byte>=brst_size*plat->brst_len) { + lcnt0 =data_size_byte/ (brst_size* plat-Quoted 1 line
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);
Quoted 9 lines
Quoted 7 lines
+
+ 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]
Quoted 139 lines
Quoted 139 lines
+ off +=
_emit_lp(&buf[off], 0, lcnt0);
+ loopjmp0 = off;
+ /* DMAWFPperiheral_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);drivers/dma/dma330.c new file +35
-0+ + 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 buffersize\n");
+ return-ENOMEM;
+ }
+ }drivers/dma/dma330.c new file +12
-0+ + /* 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);drivers/dma/dma330.c new file +43
-0+ + /* 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 buffersize\n");
+ return-ENOMEM;
+ }
+ }drivers/dma/dma330.c new file +17
-0+ + /* 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.
Quoted 9 lines
Quoted 6 lines
+ *
+ * Return: Negative value for error or not successful. 0 for successful.
+ */
+static intdma330_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.
Quoted 225 lines
Quoted 217 lines
+{
+ /* 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;
+ u32brst_size= 0;
+ u32brst_len= 0;
+ u32data_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;
+ structdma330_reqcfgreqcfg;
+
+ if (!buf) {
+ debug("ERROR DMA330 : DMA Microcode buffer pointer is NULL\n");
+ return-EINVAL;
+ }drivers/dma/dma330.c new file +50
-0+ + 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-Quoted 1 line
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);drivers/dma/dma330.c new file +38
-0+ + 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 sizeoff %d\n",
+ off);
+ return-ENOMEM;
+ }
+ }drivers/dma/dma330.c new file +12
-0+ + /* 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);drivers/dma/dma330.c new file +27
-0+ + /* 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 buffersize\n");
+ return-ENOMEM;
+ }
+ }drivers/dma/dma330.c new file +12
-0+ + /* 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_tlen)drivers/dma/dma330.c new file +5
-0+{ + struct dma_dma330_platdata *plat = dev->platdata; + int ret = 0; + + /* If DMA access is set to secure, base change to DMAQuoted 3 lines
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_securemember. You should not change platform
data while running.
Quoted 3 lines
You can pass the base address to
dma330_transfer_zeroes_setup, perhaps.
Okay.
Quoted 18 lines
Quoted 18 lines
drivers/dma/dma330.c new file +14
-0+ + 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_tlen) +{
Quoted 9 lines
Quoted 7 lines
+ 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.
Quoted 30 lines
Quoted 30 lines
drivers/dma/dma330.c new file +8
-0+ + 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;
+ caseDMA_MEM_TO_DEV:
+ plat->dma330.transfer_type =DMA_SUPPORTS_MEM_TO_DEV;
+ break;
+ caseDMA_DEV_TO_MEM:
+ plat->dma330.transfer_type =DMA_SUPPORTS_DEV_TO_MEM;
+ break;
+ }drivers/dma/dma330.c new file +10
-0+ + ret = dma330_transfer_setup(plat); + + /* Revert back to non secure DMA base */ + if (plat->dma330.secure) + plat->base -= 0x1000; + + return ret; +} +
Quoted 9 lines
drivers/dma/dma330.c line 1476
Quoted 7 lines
drivers/dma/dma330.c new file +7
-0+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.
Quoted 7 lines
Quoted 5 lines
+ plat->max_brst_size = fdtdec_get_uint(blob, node, "dma-max- burst-size",
+ 2);Use
dev_readAPI please (live tree)
Okay.
Quoted 166 lines
Quoted 155 lines
+ plat->brst_size =
fdtdec_get_uint(blob, node, "dma-burst- size", 2);
+ plat->brst_len =fdtdec_get_uint(blob, node, "dma-burst- length", 2);drivers/dma/dma330.c new file +33
-0+ + 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),drivers/dma/dma330.c new file +1
-0+};include/dma330.h new file +34
-0@@ -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 notallocate */
+ SINVALID1,
+ SINVALID2,
+ SCCTRL6, /* Cacheable write-through, allocate on reads only */
+ SCCTRL7, /* Cacheable write-back, allocate on reads only */include/dma330.h new file +7
-0+}; + +enum dma330_dstcachectrl { + DCCTRL0 = 0, /* Noncacheable and nonbufferable */ + DCCTRL1, /* Bufferable only */ + DCCTRL2, /* Cacheable, but do not allocate */ + DCCTRL3, /* Cacheable and bufferable, but do notallocate */
+ DINVALID1 = 8,
+ DINVALID2,
+ DCCTRL6, /* Cacheable write-through, allocate on writes only */
+ DCCTRL7, /* Cacheable write-back, allocate on writes only */include/dma330.h new file +41
-0+}; + +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 - 16Quoted 8 lines
+ * Data chunk (each chunk size equivalent to
brst_size).
+ * @peripheral_id: Assignedperipheral_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.
Quoted 1 line
Quoted 4 lines
Quoted 2 lines
+ *
+ * Description of the structure.Yes?
Sure.
Quoted 31 lines
Quoted 27 lines
+ */
include/dma330.h new file +24
-1+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 */ --Quoted 2 lines
2.2.0
Regards,
Simon
PATCH 2/5 drivers: dma: Add function to zeroes a range of destination such as memory
In reply to tien.fong.chee at intel.comFrom: Tien Fong Chee <[email protected]>
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.
--- drivers/dma/dma-uclass.c | 15 +++++++++++++++ include/dma.h | 12 ++++++++++++ 2 files changed, 27 insertions(+)
drivers/dma/dma-uclass.c +15 -0
@@ -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" ,
include/dma.h +12 -0
@@ -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_ */
Signature · 1 line
2.2.0
PATCH 2/5 drivers: dma: Add function to zeroes a range of destination such as memory
Hi Tien,
Quoted 43 lines · On 31 May 2018 at 02:08, <[email protected]> wrote:
From: Tien Fong Chee <[email protected]>
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 <[email protected]>--- drivers/dma/dma-uclass.c | 15 +++++++++++++++ include/dma.h | 12 ++++++++++++ 2 files changed, 27 insertions(+)drivers/dma/dma-uclass.c +15
-0@@ -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" ,include/dma.h +0
-0
include/dma.h struct dma_ops { · line 49
Quoted 4 lines
include/dma.h +1
-0@@ -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 ?
Quoted 20 lines
include/dma.h +11
-1};/* @@ -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_ */ --
Quoted 2 lines
2.2.0
Regards,
Simon
Chee
PATCH 2/5 drivers: dma: Add function to zeroes a range of destination such as memory
Quoted 51 lines · Simon Glass
Hi Tien,
Quoted 48 lines · On 31 May 2018 at 02:08, <[email protected]> wrote:
From: Tien Fong Chee <[email protected]>
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 <[email protected]>---
drivers/dma/dma-uclass.c | 15 +++++++++++++++
include/dma.h | 12 ++++++++++++2 files changed, 27 insertions(+)drivers/dma/dma-uclass.c +9
-0@@ -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 raceswith DMA */
+ invalidate_dcache_range((unsigned long)dst, (unsigned long)dst +
+ roundup(len,ARCH_DMA_MINALIGN));drivers/dma/dma-uclass.c +4
-0+ + return ops->transfer_zeroes(dev, dst, len); +} +UCLASS_DRIVER(dma) = {
.id =UCLASS_DMA,
.name = "dma",include/dma.h +0
-0@@ -46,6 +46,7 @@ struct dma_ops {
Quoted 9 lines
Quoted 6 lines
include/dma.h +0
-0*/ int (*transfer)(struct udevice *dev, int direction, void *dst,
void *src,size_tlen);
+ int (*transfer_zeroes)(struct udevice *dev, void *dst,size_tlen);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?
Quoted 27 lines
Quoted 20 lines
};
/*
include/dma.h +11
-0@@ -82,4 +83,15 @@ int dma_get_device(u32 transfer_type, structudevice **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); +Quoted 4 lines
#endif /* _DMA_H_ */
--
2.2.0Regards,
Simon
PATCH 3/5 drivers: dma: Factor out dma_get_device from DMA class function
In reply to tien.fong.chee at intel.comFrom: Tien Fong Chee <[email protected]>
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.
--- drivers/dma/dma-uclass.c | 8 +------- drivers/mtd/spi/spi_flash.c | 9 ++++++++- include/dma.h | 3 ++- 3 files changed, 11 insertions(+), 9 deletions(-)
drivers/dma/dma-uclass.c +1 -7
@@ -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)
drivers/mtd/spi/spi_flash.c +8 -1
@@ -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);
include/dma.h +2 -1
@@ -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.
Signature · 1 line
2.2.0
PATCH 3/5 drivers: dma: Factor out dma_get_device from DMA class function
Quoted 7 lines · On 31 May 2018 at 02:08, <[email protected]> wrote:
From: Tien Fong Chee <[email protected]>
Factor out the
dma_get_devicefrom DMA class function so caller can set some configuration and changes on the DMA device structure which is return by callingdma_get_devicebefore device instance is processed by DMA class functions.
Quoted 6 lines
Signed-off-by Tien Fong Chee <[email protected]>--- drivers/dma/dma-uclass.c | 8 +------- drivers/mtd/spi/spi_flash.c | 9 ++++++++- include/dma.h | 3 ++- 3 files changed, 11 insertions(+), 9 deletions(-)
PATCH 4/5 include: dma: Update the function description for dma_memcpy
In reply to tien.fong.chee at intel.comFrom: Tien Fong Chee <[email protected]>
Update the dma_memcpy description on return argument for DMA330 driver.
--- include/dma.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
include/dma.h +2 -2
@@ -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);
Signature · 1 line
2.2.0
PATCH 4/5 include: dma: Update the function description for dma_memcpy
Hi,
Quoted 14 lines · On 31 May 2018 at 02:08, <[email protected]> wrote:
From: Tien Fong Chee <[email protected]>
Update the
dma_memcpydescription on return argument for DMA330 driver.Signed-off-by Tien Fong Chee <[email protected]>--- include/dma.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)include/dma.h +0
-0
include/dma.h int dma_get_device(u32 transfer_type, struct udevice **devp); · line 83
Quoted 7 lines
include/dma.h +2
-2@@ -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?
Quoted 6 lines
include/dma.h +0
-1*/ int dma_memcpy (struct udevice *dev,void *dst,void *src,size_t len); --2.2.0
Regards,
Simon
Chee
PATCH 4/5 include: dma: Update the function description for dma_memcpy
Quoted 31 lines · Simon Glass
Hi,
Quoted 16 lines · On 31 May 2018 at 02:08, <[email protected]> wrote:
From: Tien Fong Chee <[email protected]>
Update the
dma_memcpydescription on return argument for DMA330 driver.Signed-off-by Tien Fong Chee <[email protected]>---
include/dma.h | 4 ++--1 file changed, 2 insertions(+), 2 deletions(-)include/dma.h +0
-0include/dma.h int dma_get_device(u32 transfer_type, struct · line 85
Quoted 9 lines
include/dma.h +2
-2@@ -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.
Quoted 2 lines
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.
Quoted 10 lines
Quoted 8 lines
include/dma.h +0
-0*/ int
dma_memcpy(struct udevice *dev, void *dst, void *src, size_t len);--
2.2.0Regards,
Simon
PATCH 4/5 include: dma: Update the function description for dma_memcpy
Hi,
Quoted 32 lines · Tien Fong
Quoted 31 lines · Simon Glass
Hi,
Quoted 16 lines · On 31 May 2018 at 02:08, <[email protected]> wrote:
From: Tien Fong Chee <[email protected]>
Update the
dma_memcpydescription on return argument for DMA330 driver.Signed-off-by Tien Fong Chee <[email protected]>--- include/dma.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)include/dma.h +0
-0include/dma.h int dma_get_device(u32 transfer_type, struct · line 85
Quoted 9 lines
include/dma.h +2
-2@@ -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.
Quoted 5 lines
Quoted 2 lines
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
Chee
PATCH 4/5 include: dma: Update the function description for dma_memcpy
Quoted 47 lines · Simon Glass
Hi,
Quoted 36 lines · Tien Fong
Quoted 34 lines · Simon Glass
Hi,
Quoted 17 lines · On 31 May 2018 at 02:08, <[email protected]> wrote:
From: Tien Fong Chee <[email protected]>
Update the
dma_memcpydescription on return argument for DMA330 driver.Signed-off-by Tien Fong Chee <[email protected]>---
include/dma.h | 4 ++--1 file changed, 2 insertions(+), 2 deletions(-)include/dma.h +0
-0include/dma.h int dma_get_device(u32 transfer_type, struct · line 85
Quoted 9 lines
include/dma.h +2
-2@@ -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.Quoted 7 lines
Quoted 2 lines
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.
Quoted 9 lines
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.
Quoted 2 lines
Regards,
Simon
PATCH 5/5 arm: dts: socfpga: stratix10: update pdma
In reply to tien.fong.chee at intel.comFrom: Tien Fong Chee <[email protected]>
Update pdma properties for Stratix 10
--- arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
arch/arm/dts/socfpga_stratix10.dtsi +20 -0
@@ -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>;
Signature · 1 line
2.2.0
PATCH 5/5 arm: dts: socfpga: stratix10: update pdma
Quoted 8 lines · On 31 May 2018 at 02:08, <[email protected]> wrote:
From: Tien Fong Chee <[email protected]>
Update pdma properties for Stratix 10
Signed-off-by Tien Fong Chee <[email protected]>--- arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
Is there a DT binding file for this somewhere?
Chee
PATCH 5/5 arm: dts: socfpga: stratix10: update pdma
Quoted 6 lines · Simon Glass
Quoted 5 lines · On 31 May 2018 at 02:08, <[email protected]> wrote:
From: Tien Fong Chee <[email protected]>
Update pdma properties for Stratix 10
Quoted 7 lines
Quoted 4 lines
Signed-off-by Tien Fong Chee <[email protected]>---
arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++1 file changed, 20 insertions(+)Reviewed-by Simon Glass <[email protected]>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.
PATCH 5/5 arm: dts: socfpga: stratix10: update pdma
In reply to tien.fong.chee at intel.comQuoted 17 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
Update pdma properties for Stratix 10
Signed-off-by Tien Fong Chee <[email protected]>--- arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)arch/arm/dts/socfpga_stratix10.dtsi +0
-0@@ -82,6 +82,26 @@ ranges = <0 0 0 0xffffffff>; u-boot,dm-pre-reloc;
arch/arm/dts/socfpga_stratix10.dtsi line 94
Quoted 10 lines
arch/arm/dts/socfpga_stratix10.dtsi +10
-0+ 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
PATCH 5/5 arm: dts: socfpga: stratix10: update pdma
Quoted 20 lines · Dinh Nguyen
Quoted 17 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
Update pdma properties for Stratix 10
Signed-off-by Tien Fong Chee <[email protected]>--- arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)arch/arm/dts/socfpga_stratix10.dtsi +0
-0@@ -82,6 +82,26 @@ ranges = <0 0 0 0xffffffff>; u-boot,dm-pre-reloc;
Quoted 13 lines
arch/arm/dts/socfpga_stratix10.dtsi line 94
Quoted 10 lines
arch/arm/dts/socfpga_stratix10.dtsi +10
-0+ 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.
Signature · 2 lines
Best regards, Marek Vasut
Chee
PATCH 5/5 arm: dts: socfpga: stratix10: update pdma
Quoted 30 lines · Marek Vasut
Quoted 29 lines · Dinh Nguyen
Quoted 25 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
Update pdma properties for Stratix 10
Signed-off-by Tien Fong Chee <[email protected]>---
arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++1 file changed, 20 insertions(+)arch/arm/dts/socfpga_stratix10.dtsi +0
-0@@ -82,6 +82,26 @@ ranges = <0 0 0 0xffffffff>; u-boot,dm-pre-reloc;arch/arm/dts/socfpga_stratix10.dtsi +6
-0+ amba { + u-boot,dm-pre-reloc; + compatible = "arm,amba-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges;
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
arch/arm/dts/socfpga_stratix10.dtsi +4
-0+ + 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/ .
Quoted 5 lines
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>
PATCH 5/5 arm: dts: socfpga: stratix10: update pdma
Quoted 39 lines · Tien Fong
Quoted 30 lines · Marek Vasut
Quoted 29 lines · Dinh Nguyen
Quoted 25 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
Update pdma properties for Stratix 10
Signed-off-by Tien Fong Chee <[email protected]>---
arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++1 file changed, 20 insertions(+)arch/arm/dts/socfpga_stratix10.dtsi +0
-0@@ -82,6 +82,26 @@ ranges = <0 0 0 0xffffffff>; u-boot,dm-pre-reloc;arch/arm/dts/socfpga_stratix10.dtsi +6
-0+ amba { + u-boot,dm-pre-reloc; + compatible = "arm,amba-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges;Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
arch/arm/dts/socfpga_stratix10.dtsi +4
-0+ + 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.
Quoted 1 line
Oh okay...why do you need to add the additional binding "arm,dma330"?
Dinh
Chee
PATCH 5/5 arm: dts: socfpga: stratix10: update pdma
Quoted 47 lines · Dinh Nguyen
Quoted 45 lines · Tien Fong
Quoted 43 lines · Marek Vasut
Quoted 41 lines · Dinh Nguyen
Quoted 31 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
Update pdma properties for Stratix 10
Signed-off-by Tien Fong Chee <[email protected]>---
arch/arm/dts/socfpga_stratix10.dtsi | 20
++++++++++++++++++++1 file changed, 20 insertions(+)arch/arm/dts/socfpga_stratix10.dtsi +0
-0@@ -82,6 +82,26 @@ ranges = <0 0 0 0xffffffff>; u-boot,dm-pre-reloc;arch/arm/dts/socfpga_stratix10.dtsi +10
-0+ 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 =Quoted 1 line
"arm,pl330", "arm,dma330";
I think you got "arm,dma330" binding wrong. I don't see any binding
with
that name.
Quoted 2 lines
Quoted 1 line
Oh okay...why do you need to add the additional binding "arm,dma330"?
pl330 is old name, now arm using dma330.
Quoted 2 lines
Dinh
PATCH 5/5 arm: dts: socfpga: stratix10: update pdma
In reply to CheeQuoted 39 lines · Tien Fong
Quoted 30 lines · Marek Vasut
Quoted 29 lines · Dinh Nguyen
Quoted 25 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
Update pdma properties for Stratix 10
Signed-off-by Tien Fong Chee <[email protected]>---
arch/arm/dts/socfpga_stratix10.dtsi | 20 ++++++++++++++++++++1 file changed, 20 insertions(+)arch/arm/dts/socfpga_stratix10.dtsi +0
-0@@ -82,6 +82,26 @@ ranges = <0 0 0 0xffffffff>; u-boot,dm-pre-reloc;arch/arm/dts/socfpga_stratix10.dtsi +6
-0+ amba { + u-boot,dm-pre-reloc; + compatible = "arm,amba-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges;Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
arch/arm/dts/socfpga_stratix10.dtsi +4
-0+ + 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.
Quoted 8 lines
Here https://patchwork.ozlabs.org/patch/923234/ .
Quoted 5 lines
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
Quoted 2 lines
Using DMA to init the DDR, which is around 3-4sec for 2GB. I attached screenshot for the print out based on
arria10_sdmmccustodian.
Well sure, if you do it wrong, it'll take longer for the CPU.
Signature · 2 lines
Best regards, Marek Vasut
cover Add DMA driver for DMA330 controller
In reply to tien.fong.chee at intel.comQuoted 15 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 outdma_get_devicefrom DMA class function
include: dma: Update the function description fordma_memcpy
arm: dts: socfpga: stratix10: update pdma
Quoted 12 lines
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.
Signature · 2 lines
Best regards, Marek Vasut
See
cover Add DMA driver for DMA330 controller
Quoted 31 lines · Marek Vasut
Quoted 26 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 outdma_get_devicefrom DMA class function
include: dma: Update the function description fordma_memcpy
arm: dts: socfpga: stratix10: update pdmaarch/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 ++++Quoted 4 lines
8 files changed, 1719 insertions(+), 12 deletions(-)create mode 100644
drivers/dma/dma330.c
create mode 100644include/dma330.h
Quoted 4 lines
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
Quoted 1 line
cover Add DMA driver for DMA330 controller
Quoted 32 lines · Chin Liang
Quoted 31 lines · Marek Vasut
Quoted 30 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 outdma_get_devicefrom DMA class function
include: dma: Update the function description fordma_memcpy
arm: dts: socfpga: stratix10: update pdmaarch/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 100644include/dma330.h
Quoted 8 lines
Quoted 4 lines
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 ?
Quoted 1 line
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?
Quoted 3 lines
We
can also parallel the zero out task with other tasks such as flash controller init.
The flash controller also needs some zeroing out ?
Signature · 2 lines
Best regards, Marek Vasut
Chee
cover Add DMA driver for DMA330 controller
Quoted 44 lines · Marek Vasut
Quoted 43 lines · Chin Liang
Quoted 41 lines · Marek Vasut
Quoted 29 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 outdma_get_devicefrom DMA class function
include: dma: Update the function description fordma_memcpy
arm: dts: socfpga: stratix10: update pdmaarch/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 ++++Quoted 4 lines
8 files changed, 1719 insertions(+), 12 deletions(-)create mode 100644
drivers/dma/dma330.c
create mode 100644include/dma330.hI 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.
Quoted 5 lines
Quoted 3 lines
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.
Quoted 4 lines
Quoted 2 lines
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.
Quoted 6 lines
Quoted 4 lines
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.