| Message ID | 20180124004454.5759-6-miquel.raynal@free-electrons.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | Bring NAND support to Nintendo NES Classic | expand |
Hi, On Wed, Jan 24, 2018 at 01:44:51AM +0100, Miquel Raynal wrote: > Because using DMA implementation is not generic and was not developped > to work on SoCs like A33, migrate the SPL to use PIO. > > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> I guess the issue is slightly different than what you're saying, or at least should be expressed differently. The issue is that the SPL support has be done to support only the earlier generations of Allwinner SoCs, and only really enabled on the A13 / GR8. However, those old SoCs had a DMA engine that has been replaced since the A31 by another DMA controller that is no longer compatible. Since the code directly uses that DMA controller, it cannot operate properly on the later SoCs, while the NAND controller hasn't changed. There's two pathes forward, the first one would be to add support for that DMA controller too, or you can just remove the DMA usage entirely and rely on PIO that will make the driver simpler. Explaining why you chose PIO would be great too. > --- > board/sunxi/board.c | 4 +- > drivers/mtd/nand/sunxi_nand_spl.c | 167 +++++++++++++++++--------------------- > 2 files changed, 79 insertions(+), 92 deletions(-) > > 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 = > (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; > > - 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)); That has nothing to do with PIO vs DMA. It should be in another patch. > #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)); So you're de-asserting the reset line for the DMA controller, while the commit is about removing the need for that DMA controller ? :) That whole block can actually be removed (in a separate patch) now that you're not using DMA anymore. > #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 <common.h> > #include <config.h> > #include <nand.h> > +#include <linux/ctype.h> > > /* 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) > > - > #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) > - > > #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) > > #define NFC_ST_CMD_INT_FLAG (1 << 1) > -#define NFC_ST_DMA_INT_FLAG (1 << 2) > +#define NFC_ST_CMD_FIFO_STAT (1 << 3) > > #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 > > -#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) > - We should probably keep those values. They're not used anymore, but the NAND controller has been under-documented on most of the datasheet we've had. It's good to keep it at least for reference. > struct nfc_config { > int page_size; > int ecc_strength; > @@ -254,7 +229,23 @@ static int nand_reset_column(void) > SUNXI_NFC_BASE + NFC_CMD); > > return nand_wait_int(); > +} > > +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(); > } > > static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116}; > @@ -262,86 +253,80 @@ static const int ecc_bytes[] = {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 = (dma_addr_t)dest; > int nsectors = len / conf->ecc_size; > u16 rand_seed = 0; > - u32 val; > - int page; > - > - page = offs / conf->page_size; > + int oob_chunk_sz = ecc_bytes[conf->ecc_strength]; > + int page = offs / conf->page_size; > + u32 ecc_st; > + int i; > > if (offs % conf->page_size || len % conf->ecc_size || > len > conf->page_size || len < 0) > return -EINVAL; > > - /* clear ecc status */ > - writel(0, SUNXI_NFC_BASE + NFC_ECC_ST); > - > /* Choose correct seed if randomized */ > if (conf->randomize) > rand_seed = random_seed[page % conf->nseeds]; > > - writel((rand_seed << 16) | (conf->ecc_strength << 12) | > - (conf->randomize ? NFC_ECC_RANDOM_EN : 0) | > - (conf->ecc_size == 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 = 0; i < nsectors; i++) { > + int data_off = i * conf->ecc_size; > + int oob_off = conf->page_size + (i * oob_chunk_sz); > + u8 *data = 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 == 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(); > > - 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); It seems that you're also converting this code to some of your new helpers. This should be done in the patch introducing them. > + /* > + * 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(); > > - 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; > - } Ditto. Thanks! Maxime
On Wed, 24 Jan 2018 01:44:51 +0100 Miquel Raynal <miquel.raynal@free-electrons.com> wrote: > Because using DMA implementation is not generic and was not developped > to work on SoCs like A33, migrate the SPL to use PIO. > > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> > --- > board/sunxi/board.c | 4 +- > drivers/mtd/nand/sunxi_nand_spl.c | 167 +++++++++++++++++--------------------- > 2 files changed, 79 insertions(+), 92 deletions(-) > > 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 = > (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; > > - 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 <common.h> > #include <config.h> > #include <nand.h> > +#include <linux/ctype.h> > > /* 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) > > - > #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) > - > > #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) > > #define NFC_ST_CMD_INT_FLAG (1 << 1) > -#define NFC_ST_DMA_INT_FLAG (1 << 2) > +#define NFC_ST_CMD_FIFO_STAT (1 << 3) > > #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 > > -#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); > > return nand_wait_int(); > +} > > +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(); > } 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. > > static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116}; > @@ -262,86 +253,80 @@ static const int ecc_bytes[] = {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 = (dma_addr_t)dest; > int nsectors = len / conf->ecc_size; > u16 rand_seed = 0; > - u32 val; > - int page; > - > - page = offs / conf->page_size; > + int oob_chunk_sz = ecc_bytes[conf->ecc_strength]; > + int page = offs / conf->page_size; > + u32 ecc_st; > + int i; > > if (offs % conf->page_size || len % conf->ecc_size || > len > conf->page_size || len < 0) > return -EINVAL; > > - /* clear ecc status */ > - writel(0, SUNXI_NFC_BASE + NFC_ECC_ST); > - > /* Choose correct seed if randomized */ > if (conf->randomize) > rand_seed = random_seed[page % conf->nseeds]; > > - writel((rand_seed << 16) | (conf->ecc_strength << 12) | > - (conf->randomize ? NFC_ECC_RANDOM_EN : 0) | > - (conf->ecc_size == 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 = 0; i < nsectors; i++) { > + int data_off = i * conf->ecc_size; > + int oob_off = conf->page_size + (i * oob_chunk_sz); > + u8 *data = 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 == 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(); > > - 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(); > > - 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 = readl(SUNXI_NFC_BASE + NFC_ECC_ST); > > - 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); 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. > > - val = 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); > > - /* ECC error detected. */ > - if (val & 0xffff) > - return -EIO; > + /* ECC error detected. */ > + if (ecc_st & 0xffff) > + return -EIO; > > - /* > - * 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; This is no longer valid, because now you iterate over all ECC chunks manually. The test should be: if (!i && (ecc_st & 0x10000)) > + > + if (data_off + conf->ecc_size >= len) > + break; > + } > + > + return 0; > } > > static int nand_max_ecc_strength(struct nfc_config *conf)
On Wed, 24 Jan 2018 09:06:26 +0100 Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > Hi, > > On Wed, Jan 24, 2018 at 01:44:51AM +0100, Miquel Raynal wrote: > > Because using DMA implementation is not generic and was not developped > > to work on SoCs like A33, migrate the SPL to use PIO. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> > > I guess the issue is slightly different than what you're saying, or at > least should be expressed differently. > > The issue is that the SPL support has be done to support only the > earlier generations of Allwinner SoCs, and only really enabled on the > A13 / GR8. However, those old SoCs had a DMA engine that has been > replaced since the A31 by another DMA controller that is no longer > compatible. > > Since the code directly uses that DMA controller, it cannot operate > properly on the later SoCs, while the NAND controller hasn't changed. > > There's two pathes forward, the first one would be to add support for > that DMA controller too, or you can just remove the DMA usage entirely > and rely on PIO that will make the driver simpler. > > Explaining why you chose PIO would be great too. > > > --- > > board/sunxi/board.c | 4 +- > > drivers/mtd/nand/sunxi_nand_spl.c | 167 +++++++++++++++++--------------------- > > 2 files changed, 79 insertions(+), 92 deletions(-) > > > > 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 = > > (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; > > > > - 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)); > > That has nothing to do with PIO vs DMA. It should be in another patch. > > > #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)); > > So you're de-asserting the reset line for the DMA controller, while > the commit is about removing the need for that DMA controller ? :) > > That whole block can actually be removed (in a separate patch) now > that you're not using DMA anymore. > > > #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 <common.h> > > #include <config.h> > > #include <nand.h> > > +#include <linux/ctype.h> > > > > /* 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) > > > > - > > #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) > > - > > > > #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) > > > > #define NFC_ST_CMD_INT_FLAG (1 << 1) > > -#define NFC_ST_DMA_INT_FLAG (1 << 2) > > +#define NFC_ST_CMD_FIFO_STAT (1 << 3) Oh, I didn't notice you were removing some definitions. I agree what Maxime says below, it's definitely not a good move. > > > > #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 > > > > -#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) > > - > > We should probably keep those values. They're not used anymore, but > the NAND controller has been under-documented on most of the datasheet > we've had. It's good to keep it at least for reference. > Hm, your statement is valid for the NFC_ defs, but these are purely DMA engine related definitions, so I think we can get rid of them.
On Wed, Jan 24, 2018 at 09:26:37AM +0100, Boris Brezillon wrote: > > > > > > #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 > > > > > > -#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) > > > - > > > > We should probably keep those values. They're not used anymore, but > > the NAND controller has been under-documented on most of the datasheet > > we've had. It's good to keep it at least for reference. > > > > Hm, your statement is valid for the NFC_ defs, but these are purely DMA > engine related definitions, so I think we can get rid of them. Oh, right, we can totally remove those. Maxime
Hi Maxime, I have some doubts about your comments, see below. On Wed, 24 Jan 2018 09:06:26 +0100 Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > Hi, > > On Wed, Jan 24, 2018 at 01:44:51AM +0100, Miquel Raynal wrote: > > Because using DMA implementation is not generic and was not developped > > to work on SoCs like A33, migrate the SPL to use PIO. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> > > I guess the issue is slightly different than what you're saying, or at > least should be expressed differently. > > The issue is that the SPL support has be done to support only the > earlier generations of Allwinner SoCs, and only really enabled on the > A13 / GR8. However, those old SoCs had a DMA engine that has been > replaced since the A31 by another DMA controller that is no longer > compatible. > > Since the code directly uses that DMA controller, it cannot operate > properly on the later SoCs, while the NAND controller hasn't changed. > > There's two pathes forward, the first one would be to add support for > that DMA controller too, or you can just remove the DMA usage entirely > and rely on PIO that will make the driver simpler. > > Explaining why you chose PIO would be great too. > > > --- > > board/sunxi/board.c | 4 +- > > drivers/mtd/nand/sunxi_nand_spl.c | 167 +++++++++++++++++--------------------- > > 2 files changed, 79 insertions(+), 92 deletions(-) > > > > 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 = > > (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; > > > > - 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)); > > That has nothing to do with PIO vs DMA. It should be in another patch. Ok. > > > #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)); > > So you're de-asserting the reset line for the DMA controller, while > the commit is about removing the need for that DMA controller ? :) > > That whole block can actually be removed (in a separate patch) now > that you're not using DMA anymore. Shall I remove only what is after the else statement? Or does MACH_SUN9I use the same code so I can remove the whole #ifdef/#else/#endif statement? > > > #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 <common.h> > > #include <config.h> > > #include <nand.h> > > +#include <linux/ctype.h> > > > > /* 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) > > > > - > > #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) > > - > > > > #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) > > > > #define NFC_ST_CMD_INT_FLAG (1 << 1) > > -#define NFC_ST_DMA_INT_FLAG (1 << 2) > > +#define NFC_ST_CMD_FIFO_STAT (1 << 3) > > > > #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 > > > > -#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) > > - > > We should probably keep those values. They're not used anymore, but > the NAND controller has been under-documented on most of the datasheet > we've had. It's good to keep it at least for reference. I will remove DMA related defines, and keep the other ones. > > > struct nfc_config { > > int page_size; > > int ecc_strength; > > @@ -254,7 +229,23 @@ static int nand_reset_column(void) > > SUNXI_NFC_BASE + NFC_CMD); > > > > return nand_wait_int(); > > +} > > > > +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(); > > } > > > > static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116}; > > @@ -262,86 +253,80 @@ static const int ecc_bytes[] = {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 = (dma_addr_t)dest; > > int nsectors = len / conf->ecc_size; > > u16 rand_seed = 0; > > - u32 val; > > - int page; > > - > > - page = offs / conf->page_size; > > + int oob_chunk_sz = ecc_bytes[conf->ecc_strength]; > > + int page = offs / conf->page_size; > > + u32 ecc_st; > > + int i; > > > > if (offs % conf->page_size || len % conf->ecc_size || > > len > conf->page_size || len < 0) > > return -EINVAL; > > > > - /* clear ecc status */ > > - writel(0, SUNXI_NFC_BASE + NFC_ECC_ST); > > - > > /* Choose correct seed if randomized */ > > if (conf->randomize) > > rand_seed = random_seed[page % conf->nseeds]; > > > > - writel((rand_seed << 16) | (conf->ecc_strength << 12) | > > - (conf->randomize ? NFC_ECC_RANDOM_EN : 0) | > > - (conf->ecc_size == 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 = 0; i < nsectors; i++) { > > + int data_off = i * conf->ecc_size; > > + int oob_off = conf->page_size + (i * oob_chunk_sz); > > + u8 *data = 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 == 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(); > > > > - 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); > > It seems that you're also converting this code to some of your new > helpers. This should be done in the patch introducing them. Actually no, it is close but this is DMA specific, and does not fit with the previously created helpers. The first reason why this block is removed is because it is related to DMA. > > > + /* > > + * 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(); > > > > - 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; > > - } > > Ditto. Same as above. Thanks for reviewing, Miquèl
Hi Boris, On Wed, 24 Jan 2018 09:16:19 +0100 Boris Brezillon <boris.brezillon@free-electrons.com> wrote: > On Wed, 24 Jan 2018 01:44:51 +0100 > Miquel Raynal <miquel.raynal@free-electrons.com> wrote: > > > Because using DMA implementation is not generic and was not developped > > to work on SoCs like A33, migrate the SPL to use PIO. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> > > --- > > board/sunxi/board.c | 4 +- > > drivers/mtd/nand/sunxi_nand_spl.c | 167 +++++++++++++++++--------------------- > > 2 files changed, 79 insertions(+), 92 deletions(-) > > > > 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 = > > (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; > > > > - 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 <common.h> > > #include <config.h> > > #include <nand.h> > > +#include <linux/ctype.h> > > > > /* 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) > > > > - > > #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) > > - > > > > #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) > > > > #define NFC_ST_CMD_INT_FLAG (1 << 1) > > -#define NFC_ST_DMA_INT_FLAG (1 << 2) > > +#define NFC_ST_CMD_FIFO_STAT (1 << 3) > > > > #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 > > > > -#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); > > > > return nand_wait_int(); > > +} > > > > +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(); > > } > > 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. > > > > > static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116}; > > @@ -262,86 +253,80 @@ static const int ecc_bytes[] = {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 = (dma_addr_t)dest; > > int nsectors = len / conf->ecc_size; > > u16 rand_seed = 0; > > - u32 val; > > - int page; > > - > > - page = offs / conf->page_size; > > + int oob_chunk_sz = ecc_bytes[conf->ecc_strength]; > > + int page = offs / conf->page_size; > > + u32 ecc_st; > > + int i; > > > > if (offs % conf->page_size || len % conf->ecc_size || > > len > conf->page_size || len < 0) > > return -EINVAL; > > > > - /* clear ecc status */ > > - writel(0, SUNXI_NFC_BASE + NFC_ECC_ST); > > - > > /* Choose correct seed if randomized */ > > if (conf->randomize) > > rand_seed = random_seed[page % conf->nseeds]; > > > > - writel((rand_seed << 16) | (conf->ecc_strength << 12) | > > - (conf->randomize ? NFC_ECC_RANDOM_EN : 0) | > > - (conf->ecc_size == 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 = 0; i < nsectors; i++) { > > + int data_off = i * conf->ecc_size; > > + int oob_off = conf->page_size + (i * oob_chunk_sz); > > + u8 *data = 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 == 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(); > > > > - 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(); > > > > - 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 = readl(SUNXI_NFC_BASE + NFC_ECC_ST); > > > > - 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); > > 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. > > > > > - val = 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); > > > > - /* ECC error detected. */ > > - if (val & 0xffff) > > - return -EIO; > > + /* ECC error detected. */ > > + if (ecc_st & 0xffff) > > + return -EIO; > > > > - /* > > - * 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; > > This is no longer valid, because now you iterate over all ECC chunks > manually. The test should be: > > if (!i && (ecc_st & 0x10000)) > That is right and could have caused troubles, thanks for spotting it! > > + > > + if (data_off + conf->ecc_size >= len) > > + break; > > + } > > + > > + return 0; > > } > > > > static int nand_max_ecc_strength(struct nfc_config *conf) > Thanks for reviewing, Miquèl
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 = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; - 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 <common.h> #include <config.h> #include <nand.h> +#include <linux/ctype.h> /* 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) - #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) - #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) #define NFC_ST_CMD_INT_FLAG (1 << 1) -#define NFC_ST_DMA_INT_FLAG (1 << 2) +#define NFC_ST_CMD_FIFO_STAT (1 << 3) #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 -#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); return nand_wait_int(); +} +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(); } static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116}; @@ -262,86 +253,80 @@ static const int ecc_bytes[] = {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 = (dma_addr_t)dest; int nsectors = len / conf->ecc_size; u16 rand_seed = 0; - u32 val; - int page; - - page = offs / conf->page_size; + int oob_chunk_sz = ecc_bytes[conf->ecc_strength]; + int page = offs / conf->page_size; + u32 ecc_st; + int i; if (offs % conf->page_size || len % conf->ecc_size || len > conf->page_size || len < 0) return -EINVAL; - /* clear ecc status */ - writel(0, SUNXI_NFC_BASE + NFC_ECC_ST); - /* Choose correct seed if randomized */ if (conf->randomize) rand_seed = random_seed[page % conf->nseeds]; - writel((rand_seed << 16) | (conf->ecc_strength << 12) | - (conf->randomize ? NFC_ECC_RANDOM_EN : 0) | - (conf->ecc_size == 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 = 0; i < nsectors; i++) { + int data_off = i * conf->ecc_size; + int oob_off = conf->page_size + (i * oob_chunk_sz); + u8 *data = 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 == 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(); - 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(); - 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 = readl(SUNXI_NFC_BASE + NFC_ECC_ST); - 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); - val = 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); - /* ECC error detected. */ - if (val & 0xffff) - return -EIO; + /* ECC error detected. */ + if (ecc_st & 0xffff) + return -EIO; - /* - * 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; + + if (data_off + conf->ecc_size >= len) + break; + } + + return 0; } static int nand_max_ecc_strength(struct nfc_config *conf)
Because using DMA implementation is not generic and was not developped to work on SoCs like A33, migrate the SPL to use PIO. Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> --- board/sunxi/board.c | 4 +- drivers/mtd/nand/sunxi_nand_spl.c | 167 +++++++++++++++++--------------------- 2 files changed, 79 insertions(+), 92 deletions(-)