From mboxrd@z Thu Jan 1 00:00:00 1970 From: Miquel Raynal Date: Fri, 26 Jan 2018 01:09:38 +0100 Subject: [U-Boot] [PATCH 5/8] spl: nand: sunxi: use PIO instead of DMA In-Reply-To: <20180124091619.40d29d03@bbrezillon> References: <20180124004454.5759-1-miquel.raynal@free-electrons.com> <20180124004454.5759-6-miquel.raynal@free-electrons.com> <20180124091619.40d29d03@bbrezillon> Message-ID: <20180126010938.3ee64eba@xps13> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: u-boot@lists.denx.de Hi Boris, On Wed, 24 Jan 2018 09:16:19 +0100 Boris Brezillon wrote: > On Wed, 24 Jan 2018 01:44:51 +0100 > Miquel Raynal wrote: >=20 > > Because using DMA implementation is not generic and was not developped > > to work on SoCs like A33, migrate the SPL to use PIO. > >=20 > > Signed-off-by: Miquel Raynal > > --- > > board/sunxi/board.c | 4 +- > > drivers/mtd/nand/sunxi_nand_spl.c | 167 +++++++++++++++++-------------= -------- > > 2 files changed, 79 insertions(+), 92 deletions(-) > >=20 > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c > > index 70e01437c4..512e2c17a6 100644 > > --- a/board/sunxi/board.c > > +++ b/board/sunxi/board.c > > @@ -266,11 +266,13 @@ static void nand_clock_setup(void) > > struct sunxi_ccm_reg *const ccm =3D > > (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; > > =20 > > - setbits_le32(&ccm->ahb_gate0, (CLK_GATE_OPEN << AHB_GATE_OFFSET_NAND0= )); > > + setbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_NAND0)); > > + setbits_le32(&ccm->ahb_reset0_cfg, (1 << AHB_GATE_OFFSET_NAND0)); > > #ifdef CONFIG_MACH_SUN9I > > setbits_le32(&ccm->ahb_gate1, (1 << AHB_GATE_OFFSET_DMA)); > > #else > > setbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_DMA)); > > + setbits_le32(&ccm->ahb_reset0_cfg, (1 << AHB_GATE_OFFSET_DMA)); > > #endif > > setbits_le32(&ccm->nand0_clk_cfg, CCM_NAND_CTRL_ENABLE | AHB_DIV_1); > > } > > diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi= _nand_spl.c > > index 2488d5cb51..5de6825994 100644 > > --- a/drivers/mtd/nand/sunxi_nand_spl.c > > +++ b/drivers/mtd/nand/sunxi_nand_spl.c > > @@ -10,6 +10,7 @@ > > #include > > #include > > #include > > +#include > > =20 > > /* registers */ > > #define NFC_CTL 0x00000000 > > @@ -45,32 +46,22 @@ > > #define NFC_CTL_PAGE_SIZE_MASK (0xf << 8) > > #define NFC_CTL_PAGE_SIZE(a) ((fls(a) - 11) << 8) > > =20 > > - > > #define NFC_ECC_EN (1 << 0) > > -#define NFC_ECC_PIPELINE (1 << 3) > > #define NFC_ECC_EXCEPTION (1 << 4) > > #define NFC_ECC_BLOCK_SIZE (1 << 5) > > #define NFC_ECC_RANDOM_EN (1 << 9) > > -#define NFC_ECC_RANDOM_DIRECTION (1 << 10) > > - > > =20 > > #define NFC_ADDR_NUM_OFFSET 16 > > -#define NFC_ACCESS_DIR (1 << 20) > > #define NFC_SEND_ADDR (1 << 19) > > #define NFC_DATA_TRANS (1 << 21) > > #define NFC_SEND_CMD1 (1 << 22) > > #define NFC_WAIT_FLAG (1 << 23) > > #define NFC_SEND_CMD2 (1 << 24) > > -#define NFC_SEQ (1 << 25) > > -#define NFC_DATA_SWAP_METHOD (1 << 26) > > -#define NFC_ROW_AUTO_INC (1 << 27) > > -#define NFC_SEND_CMD3 (1 << 28) > > -#define NFC_SEND_CMD4 (1 << 29) > > #define NFC_RAW_CMD (0 << 30) > > -#define NFC_PAGE_CMD (2 << 30) > > +#define NFC_ECC_CMD (1 << 30) > > =20 > > #define NFC_ST_CMD_INT_FLAG (1 << 1) > > -#define NFC_ST_DMA_INT_FLAG (1 << 2) > > +#define NFC_ST_CMD_FIFO_STAT (1 << 3) > > =20 > > #define NFC_READ_CMD_OFFSET 0 > > #define NFC_RANDOM_READ_CMD0_OFFSET 8 > > @@ -80,22 +71,6 @@ > > #define NFC_CMD_RNDOUT 0x05 > > #define NFC_CMD_READSTART 0x30 > > =20 > > -#define SUNXI_DMA_CFG_REG0 0x300 > > -#define SUNXI_DMA_SRC_START_ADDR_REG0 0x304 > > -#define SUNXI_DMA_DEST_START_ADDRR_REG0 0x308 > > -#define SUNXI_DMA_DDMA_BC_REG0 0x30C > > -#define SUNXI_DMA_DDMA_PARA_REG0 0x318 > > - > > -#define SUNXI_DMA_DDMA_CFG_REG_LOADING (1 << 31) > > -#define SUNXI_DMA_DDMA_CFG_REG_DMA_DEST_DATA_WIDTH_32 (2 << 25) > > -#define SUNXI_DMA_DDMA_CFG_REG_DDMA_DST_DRQ_TYPE_DRAM (1 << 16) > > -#define SUNXI_DMA_DDMA_CFG_REG_DMA_SRC_DATA_WIDTH_32 (2 << 9) > > -#define SUNXI_DMA_DDMA_CFG_REG_DMA_SRC_ADDR_MODE_IO (1 << 5) > > -#define SUNXI_DMA_DDMA_CFG_REG_DDMA_SRC_DRQ_TYPE_NFC (3 << 0) > > - > > -#define SUNXI_DMA_DDMA_PARA_REG_SRC_WAIT_CYC (0x0F << 0) > > -#define SUNXI_DMA_DDMA_PARA_REG_SRC_BLK_SIZE (0x7F << 8) > > - > > struct nfc_config { > > int page_size; > > int ecc_strength; > > @@ -254,7 +229,23 @@ static int nand_reset_column(void) > > SUNXI_NFC_BASE + NFC_CMD); > > =20 > > return nand_wait_int(); > > +} > > =20 > > +static int nand_change_column(u16 column) > > +{ > > + nand_wait_cmd_fifo_empty(); > > + > > + writel((NFC_CMD_RNDOUTSTART << NFC_RANDOM_READ_CMD1_OFFSET) | > > + (NFC_CMD_RNDOUT << NFC_RANDOM_READ_CMD0_OFFSET) | > > + (NFC_CMD_RNDOUTSTART << NFC_READ_CMD_OFFSET), > > + SUNXI_NFC_BASE + NFC_RCMD_SET); > > + writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); > > + writel(column, SUNXI_NFC_BASE + NFC_ADDR_LOW); > > + writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD | > > + (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR | NFC_CMD_RNDOUT, > > + SUNXI_NFC_BASE + NFC_CMD); > > + > > + return nand_wait_int(); > > } =20 >=20 > nand_reset_column() and nand_change_column() are almost the same, > except one is taking an argument to specify where you want to place the > pointer. I'd recommend getting rid of nand_reset_column() entirely and > patching all call sites to call nand_change_column(0) instead. Done in a separate commit. >=20 > > =20 > > static const int ecc_bytes[] =3D {32, 46, 54, 60, 74, 88, 102, 110, 11= 6}; > > @@ -262,86 +253,80 @@ static const int ecc_bytes[] =3D {32, 46, 54, 60,= 74, 88, 102, 110, 116}; > > static int nand_read_page(const struct nfc_config *conf, u32 offs, > > void *dest, int len) > > { > > - dma_addr_t dst =3D (dma_addr_t)dest; > > int nsectors =3D len / conf->ecc_size; > > u16 rand_seed =3D 0; > > - u32 val; > > - int page; > > - > > - page =3D offs / conf->page_size; > > + int oob_chunk_sz =3D ecc_bytes[conf->ecc_strength]; > > + int page =3D offs / conf->page_size; > > + u32 ecc_st; > > + int i; > > =20 > > if (offs % conf->page_size || len % conf->ecc_size || > > len > conf->page_size || len < 0) > > return -EINVAL; > > =20 > > - /* clear ecc status */ > > - writel(0, SUNXI_NFC_BASE + NFC_ECC_ST); > > - > > /* Choose correct seed if randomized */ > > if (conf->randomize) > > rand_seed =3D random_seed[page % conf->nseeds]; > > =20 > > - writel((rand_seed << 16) | (conf->ecc_strength << 12) | > > - (conf->randomize ? NFC_ECC_RANDOM_EN : 0) | > > - (conf->ecc_size =3D=3D 512 ? NFC_ECC_BLOCK_SIZE : 0) | > > - NFC_ECC_EN | NFC_ECC_PIPELINE | NFC_ECC_EXCEPTION, > > - SUNXI_NFC_BASE + NFC_ECC_CTL); > > - > > - flush_dcache_range(dst, ALIGN(dst + conf->ecc_size, ARCH_DMA_MINALIGN= )); > > - > > - /* SUNXI_DMA */ > > - writel(0x0, SUNXI_DMA_BASE + SUNXI_DMA_CFG_REG0); /* clr dma cmd */ > > - /* read from REG_IO_DATA */ > > - writel(SUNXI_NFC_BASE + NFC_IO_DATA, > > - SUNXI_DMA_BASE + SUNXI_DMA_SRC_START_ADDR_REG0); > > - /* read to RAM */ > > - writel(dst, SUNXI_DMA_BASE + SUNXI_DMA_DEST_START_ADDRR_REG0); > > - writel(SUNXI_DMA_DDMA_PARA_REG_SRC_WAIT_CYC | > > - SUNXI_DMA_DDMA_PARA_REG_SRC_BLK_SIZE, > > - SUNXI_DMA_BASE + SUNXI_DMA_DDMA_PARA_REG0); > > - writel(len, SUNXI_DMA_BASE + SUNXI_DMA_DDMA_BC_REG0); > > - writel(SUNXI_DMA_DDMA_CFG_REG_LOADING | > > - SUNXI_DMA_DDMA_CFG_REG_DMA_DEST_DATA_WIDTH_32 | > > - SUNXI_DMA_DDMA_CFG_REG_DDMA_DST_DRQ_TYPE_DRAM | > > - SUNXI_DMA_DDMA_CFG_REG_DMA_SRC_DATA_WIDTH_32 | > > - SUNXI_DMA_DDMA_CFG_REG_DMA_SRC_ADDR_MODE_IO | > > - SUNXI_DMA_DDMA_CFG_REG_DDMA_SRC_DRQ_TYPE_NFC, > > - SUNXI_DMA_BASE + SUNXI_DMA_CFG_REG0); > > - > > - writel(nsectors, SUNXI_NFC_BASE + NFC_SECTOR_NUM); > > - writel(NFC_ST_DMA_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); > > - writel(NFC_DATA_TRANS | NFC_PAGE_CMD | NFC_DATA_SWAP_METHOD, > > - SUNXI_NFC_BASE + NFC_CMD); > > + /* Retrieve data from SRAM (PIO) */ > > + for (i =3D 0; i < nsectors; i++) { > > + int data_off =3D i * conf->ecc_size; > > + int oob_off =3D conf->page_size + (i * oob_chunk_sz); > > + u8 *data =3D dest + data_off; > > + > > + /* Clear ECC status and restart ECC engine */ > > + writel(0, SUNXI_NFC_BASE + NFC_ECC_ST); > > + writel((rand_seed << 16) | (conf->ecc_strength << 12) | > > + (conf->randomize ? NFC_ECC_RANDOM_EN : 0) | > > + (conf->ecc_size =3D=3D 512 ? NFC_ECC_BLOCK_SIZE : 0) | > > + NFC_ECC_EN | NFC_ECC_EXCEPTION, > > + SUNXI_NFC_BASE + NFC_ECC_CTL); > > + > > + /* Move the data in SRAM */ > > + nand_change_column(data_off); > > + nand_wait_cmd_fifo_empty(); > > + writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); > > + writel(conf->ecc_size, SUNXI_NFC_BASE + NFC_CNT); > > + writel(NFC_DATA_TRANS, SUNXI_NFC_BASE + NFC_CMD); > > + nand_wait_int(); > > =20 > > - if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_DMA_INT_FLAG, > > - DEFAULT_TIMEOUT_US)) { > > - printf("Error while initializing dma interrupt\n"); > > - return -EIO; > > - } > > - writel(NFC_ST_DMA_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); > > + /* > > + * Let the ECC engine consume the ECC bytes and possibly correct > > + * the data. > > + */ > > + nand_change_column(oob_off); > > + nand_wait_cmd_fifo_empty(); > > + writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); > > + writel(NFC_DATA_TRANS | NFC_ECC_CMD, SUNXI_NFC_BASE + NFC_CMD); > > + nand_wait_int(); > > =20 > > - if (!check_value_negated(SUNXI_DMA_BASE + SUNXI_DMA_CFG_REG0, > > - SUNXI_DMA_DDMA_CFG_REG_LOADING, > > - DEFAULT_TIMEOUT_US)) { > > - printf("Error while waiting for dma transfer to finish\n"); > > - return -EIO; > > - } > > + /* Get the ECC status */ > > + ecc_st =3D readl(SUNXI_NFC_BASE + NFC_ECC_ST); > > =20 > > - invalidate_dcache_range(dst, > > - ALIGN(dst + conf->ecc_size, ARCH_DMA_MINALIGN)); > > + /* Retrieve the data from SRAM */ > > + memcpy_fromio(data, SUNXI_NFC_BASE + NFC_RAM0_BASE, > > + conf->ecc_size); =20 >=20 > Not a big deal, but should we really copy the data if the ECC engine > reports an error? Maybe you can move this memcpy_fromio() after checking > ecc_st. Ok. >=20 > > =20 > > - val =3D readl(SUNXI_NFC_BASE + NFC_ECC_ST); > > + /* Stop ECC engine */ > > + writel(readl(SUNXI_NFC_BASE + NFC_ECC_CTL) & ~NFC_ECC_EN, > > + SUNXI_NFC_BASE + NFC_ECC_CTL); > > =20 > > - /* ECC error detected. */ > > - if (val & 0xffff) > > - return -EIO; > > + /* ECC error detected. */ > > + if (ecc_st & 0xffff) > > + return -EIO; > > =20 > > - /* > > - * Return 1 if the page is empty. > > - * We consider the page as empty if the first ECC block is marked > > - * empty. > > - */ > > - return (val & 0x10000) ? 1 : 0; > > + /* > > + * Return 1 if the page is empty. We consider the page as empty > > + * if the first ECC block is marked empty. > > + */ > > + if (ecc_st & 0x10000) > > + return 1; =20 >=20 > This is no longer valid, because now you iterate over all ECC chunks > manually. The test should be: >=20 > if (!i && (ecc_st & 0x10000)) >=20 That is right and could have caused troubles, thanks for spotting it! > > + > > + if (data_off + conf->ecc_size >=3D len) > > + break; > > + } > > + > > + return 0; > > } > > =20 > > static int nand_max_ecc_strength(struct nfc_config *conf) =20 >=20 Thanks for reviewing, Miqu=C3=A8l --=20 Miquel Raynal, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com