| Message ID | 20180124004454.5759-5-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:50AM +0100, Miquel Raynal wrote: > Do some cleaning in sunxi NAND SPL driver like adding helpers and > clearing flags at the right spot > > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> I'm not really fond of these kind of wildcard patches, especially since.. > --- > drivers/mtd/nand/sunxi_nand_spl.c | 64 +++++++++++++++++++++++++-------------- > 1 file changed, 41 insertions(+), 23 deletions(-) > > diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c > index 06695fc15f..2488d5cb51 100644 > --- a/drivers/mtd/nand/sunxi_nand_spl.c > +++ b/drivers/mtd/nand/sunxi_nand_spl.c > @@ -55,8 +55,8 @@ > > > #define NFC_ADDR_NUM_OFFSET 16 > -#define NFC_SEND_ADR (1 << 19) > #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) ... here you reorder a single value, which means that the set of these values is not sorted anymore ... > @@ -155,6 +155,30 @@ static inline int check_value_negated(int offset, int unexpected_bits, > return check_value_inner(offset, unexpected_bits, timeout_us, 1); > } > > +static int nand_wait_cmd_fifo_empty(void) > +{ > + if (!check_value_negated(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_FIFO_STAT, > + DEFAULT_TIMEOUT_US)) { > + printf("nand: timeout waiting for empty cmd FIFO\n"); > + return -ETIMEDOUT; > + } > + > + return 0; > +} > + > +static int nand_wait_int(void) > +{ > + if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, > + DEFAULT_TIMEOUT_US)) { > + printf("nand: timeout waiting for interruption\n"); > + return -ETIMEDOUT; > + } > + > + udelay(1); > + > + return 0; > +} > + ... here you add the helpers that you talked about in your commit log, good. but ... > void nand_init(void) > { > uint32_t val; > @@ -172,22 +196,21 @@ void nand_init(void) > } > > /* reset NAND */ > + nand_wait_cmd_fifo_empty(); > + ... here you call one of these helpers where you didn't have that code before, and that's not mentionned or explained in your commit log. > writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); > writel(NFC_SEND_CMD1 | NFC_WAIT_FLAG | NAND_CMD_RESET, > SUNXI_NFC_BASE + NFC_CMD); > > - if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, > - DEFAULT_TIMEOUT_US)) { > - printf("Error timeout waiting for nand reset\n"); > - return; > - } > - writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); > + nand_wait_int(); > } > > static void nand_apply_config(const struct nfc_config *conf) > { > u32 val; > > + nand_wait_cmd_fifo_empty(); > + Ditto. > val = readl(SUNXI_NFC_BASE + NFC_CTL); > val &= ~NFC_CTL_PAGE_SIZE_MASK; > writel(val | NFC_CTL_RAM_METHOD | NFC_CTL_PAGE_SIZE(conf->page_size), > @@ -200,6 +223,8 @@ static int nand_load_page(const struct nfc_config *conf, u32 offs) > { > int page = offs / conf->page_size; > > + nand_wait_cmd_fifo_empty(); > + Ditto. > writel((NFC_CMD_RNDOUTSTART << NFC_RANDOM_READ_CMD1_OFFSET) | > (NFC_CMD_RNDOUT << NFC_RANDOM_READ_CMD0_OFFSET) | > (NFC_CMD_READSTART << NFC_READ_CMD_OFFSET), > @@ -208,38 +233,32 @@ static int nand_load_page(const struct nfc_config *conf, u32 offs) > writel((page >> 16) & 0xFF, SUNXI_NFC_BASE + NFC_ADDR_HIGH); > writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); > writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD | NFC_WAIT_FLAG | > - ((conf->addr_cycles - 1) << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADR, > + ((conf->addr_cycles - 1) << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR, I guess it was a typo fix then? > SUNXI_NFC_BASE + NFC_CMD); > > - if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, > - DEFAULT_TIMEOUT_US)) { > - printf("Error while initializing dma interrupt\n"); > - return -EIO; > - } > - > - return 0; > + return nand_wait_int(); > } > > static int nand_reset_column(void) > { > + nand_wait_cmd_fifo_empty(); > + Not mentionned or explained in your commit log. > 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); Ditto > writel(0, SUNXI_NFC_BASE + NFC_ADDR_LOW); > writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD | > - (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADR | NFC_CMD_RNDOUT, > + (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR | NFC_CMD_RNDOUT, > SUNXI_NFC_BASE + NFC_CMD); > > - if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, > - DEFAULT_TIMEOUT_US)) { > - printf("Error while initializing dma interrupt\n"); > - return -1; > - } > + return nand_wait_int(); > > - return 0; > } > > +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) > { > @@ -327,7 +346,6 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs, > > static int nand_max_ecc_strength(struct nfc_config *conf) > { > - static const int ecc_bytes[] = { 32, 46, 54, 60, 74, 88, 102, 110, 116 }; Ditto. Overall, I think it would be great to split that patch into 4: - the first one to fix the ADR vs ADDR typo (without changing the order) - the second one to introduce the nand_wait_int helper - the third one to introduce the nand_wait_cmd_fifo_empty, with a proper commit log explaining why this needs to be done and which issues is it fixing - And maybe if justified moving the ecc_bytes array to global scope Thanks! Maxime
On Wed, 24 Jan 2018 01:44:50 +0100 Miquel Raynal <miquel.raynal@free-electrons.com> wrote: > Do some cleaning in sunxi NAND SPL driver like adding helpers and > clearing flags at the right spot > > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> > --- > drivers/mtd/nand/sunxi_nand_spl.c | 64 +++++++++++++++++++++++++-------------- > 1 file changed, 41 insertions(+), 23 deletions(-) > > diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c > index 06695fc15f..2488d5cb51 100644 > --- a/drivers/mtd/nand/sunxi_nand_spl.c > +++ b/drivers/mtd/nand/sunxi_nand_spl.c > @@ -55,8 +55,8 @@ > > > #define NFC_ADDR_NUM_OFFSET 16 > -#define NFC_SEND_ADR (1 << 19) > #define NFC_ACCESS_DIR (1 << 20) > +#define NFC_SEND_ADDR (1 << 19) This typo fix probably deserves a separate patch. > #define NFC_DATA_TRANS (1 << 21) > #define NFC_SEND_CMD1 (1 << 22) > #define NFC_WAIT_FLAG (1 << 23) > @@ -155,6 +155,30 @@ static inline int check_value_negated(int offset, int unexpected_bits, > return check_value_inner(offset, unexpected_bits, timeout_us, 1); > } > > +static int nand_wait_cmd_fifo_empty(void) > +{ > + if (!check_value_negated(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_FIFO_STAT, > + DEFAULT_TIMEOUT_US)) { > + printf("nand: timeout waiting for empty cmd FIFO\n"); > + return -ETIMEDOUT; > + } > + > + return 0; > +} > + > +static int nand_wait_int(void) > +{ > + if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, > + DEFAULT_TIMEOUT_US)) { > + printf("nand: timeout waiting for interruption\n"); > + return -ETIMEDOUT; > + } > + > + udelay(1); I'm pretty sure this udelay() is not required. Probably some leftover of your debug session ;-). > + > + return 0; > +} > + > void nand_init(void) > { > uint32_t val; > @@ -172,22 +196,21 @@ void nand_init(void) > } > > /* reset NAND */ > + nand_wait_cmd_fifo_empty(); > + > writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); > writel(NFC_SEND_CMD1 | NFC_WAIT_FLAG | NAND_CMD_RESET, > SUNXI_NFC_BASE + NFC_CMD); > > - if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, > - DEFAULT_TIMEOUT_US)) { > - printf("Error timeout waiting for nand reset\n"); > - return; > - } > - writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); > + nand_wait_int(); I would go even further and create an helper that takes care of the 'wait_fifo_empty+clear_int+write_cmd+wait_int' sequence: static int nand_exec_cmd(u32 cmd) { int ret; ret = nand_wait_cmd_fifo_empty(); if (ret) return ret; writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); writel(cmd, SUNXI_NFC_BASE + NFC_CMD); return nand_wait_int(); } > } > > static void nand_apply_config(const struct nfc_config *conf) > { > u32 val; > > + nand_wait_cmd_fifo_empty(); > + > val = readl(SUNXI_NFC_BASE + NFC_CTL); > val &= ~NFC_CTL_PAGE_SIZE_MASK; > writel(val | NFC_CTL_RAM_METHOD | NFC_CTL_PAGE_SIZE(conf->page_size), > @@ -200,6 +223,8 @@ static int nand_load_page(const struct nfc_config *conf, u32 offs) > { > int page = offs / conf->page_size; > > + nand_wait_cmd_fifo_empty(); > + > writel((NFC_CMD_RNDOUTSTART << NFC_RANDOM_READ_CMD1_OFFSET) | > (NFC_CMD_RNDOUT << NFC_RANDOM_READ_CMD0_OFFSET) | > (NFC_CMD_READSTART << NFC_READ_CMD_OFFSET), > @@ -208,38 +233,32 @@ static int nand_load_page(const struct nfc_config *conf, u32 offs) > writel((page >> 16) & 0xFF, SUNXI_NFC_BASE + NFC_ADDR_HIGH); > writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); > writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD | NFC_WAIT_FLAG | > - ((conf->addr_cycles - 1) << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADR, > + ((conf->addr_cycles - 1) << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR, > SUNXI_NFC_BASE + NFC_CMD); > > - if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, > - DEFAULT_TIMEOUT_US)) { > - printf("Error while initializing dma interrupt\n"); > - return -EIO; > - } > - > - return 0; > + return nand_wait_int(); > } > > static int nand_reset_column(void) > { > + 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(0, SUNXI_NFC_BASE + NFC_ADDR_LOW); > writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD | > - (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADR | NFC_CMD_RNDOUT, > + (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR | NFC_CMD_RNDOUT, > SUNXI_NFC_BASE + NFC_CMD); > > - if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, > - DEFAULT_TIMEOUT_US)) { > - printf("Error while initializing dma interrupt\n"); > - return -1; > - } > + return nand_wait_int(); Here, I think you should wait, just to make sure we don't read data before tCCS. udelay(1) should be enough. > > - return 0; > } > > +static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116}; > + Putting ecc_bytes out of nand_max_ecc_strength() is not really related to this patch, and should probably go in patch 5. > static int nand_read_page(const struct nfc_config *conf, u32 offs, > void *dest, int len) > { > @@ -327,7 +346,6 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs, > > static int nand_max_ecc_strength(struct nfc_config *conf) > { > - static const int ecc_bytes[] = { 32, 46, 54, 60, 74, 88, 102, 110, 116 }; > int max_oobsize, max_ecc_bytes; > int nsectors = conf->page_size / conf->ecc_size; > int i;
Hi Maxime, On Wed, 24 Jan 2018 08:56:38 +0100 Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > Hi, > > On Wed, Jan 24, 2018 at 01:44:50AM +0100, Miquel Raynal wrote: > > Do some cleaning in sunxi NAND SPL driver like adding helpers and > > clearing flags at the right spot > > > > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> > > I'm not really fond of these kind of wildcard patches, especially > since.. > [...] > > Overall, I think it would be great to split that patch into 4: > - the first one to fix the ADR vs ADDR typo (without changing the order) > - the second one to introduce the nand_wait_int helper > - the third one to introduce the nand_wait_cmd_fifo_empty, with a > proper commit log explaining why this needs to be done and which > issues is it fixing > - And maybe if justified moving the ecc_bytes array to global scope I split this patch into 4 logical peaces, as requested. Thanks, Miquèl
Hi Boris, On Wed, 24 Jan 2018 09:06:38 +0100 Boris Brezillon <boris.brezillon@free-electrons.com> wrote: > On Wed, 24 Jan 2018 01:44:50 +0100 > Miquel Raynal <miquel.raynal@free-electrons.com> wrote: > > > Do some cleaning in sunxi NAND SPL driver like adding helpers and > > clearing flags at the right spot > > > > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> > > --- > > drivers/mtd/nand/sunxi_nand_spl.c | 64 +++++++++++++++++++++++++-------------- > > 1 file changed, 41 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c > > index 06695fc15f..2488d5cb51 100644 > > --- a/drivers/mtd/nand/sunxi_nand_spl.c > > +++ b/drivers/mtd/nand/sunxi_nand_spl.c > > @@ -55,8 +55,8 @@ > > > > > > #define NFC_ADDR_NUM_OFFSET 16 > > -#define NFC_SEND_ADR (1 << 19) > > #define NFC_ACCESS_DIR (1 << 20) > > +#define NFC_SEND_ADDR (1 << 19) > > This typo fix probably deserves a separate patch. Indeed. > > > #define NFC_DATA_TRANS (1 << 21) > > #define NFC_SEND_CMD1 (1 << 22) > > #define NFC_WAIT_FLAG (1 << 23) > > @@ -155,6 +155,30 @@ static inline int check_value_negated(int offset, int unexpected_bits, > > return check_value_inner(offset, unexpected_bits, timeout_us, 1); > > } > > > > +static int nand_wait_cmd_fifo_empty(void) > > +{ > > + if (!check_value_negated(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_FIFO_STAT, > > + DEFAULT_TIMEOUT_US)) { > > + printf("nand: timeout waiting for empty cmd FIFO\n"); > > + return -ETIMEDOUT; > > + } > > + > > + return 0; > > +} > > + > > +static int nand_wait_int(void) > > +{ > > + if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, > > + DEFAULT_TIMEOUT_US)) { > > + printf("nand: timeout waiting for interruption\n"); > > + return -ETIMEDOUT; > > + } > > + > > + udelay(1); > > I'm pretty sure this udelay() is not required. Probably some leftover > of your debug session ;-). That is right -> removed. > > > + > > + return 0; > > +} > > + > > void nand_init(void) > > { > > uint32_t val; > > @@ -172,22 +196,21 @@ void nand_init(void) > > } > > > > /* reset NAND */ > > + nand_wait_cmd_fifo_empty(); > > + > > writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); > > writel(NFC_SEND_CMD1 | NFC_WAIT_FLAG | NAND_CMD_RESET, > > SUNXI_NFC_BASE + NFC_CMD); > > > > - if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, > > - DEFAULT_TIMEOUT_US)) { > > - printf("Error timeout waiting for nand reset\n"); > > - return; > > - } > > - writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); > > + nand_wait_int(); > > I would go even further and create an helper that takes care of the > 'wait_fifo_empty+clear_int+write_cmd+wait_int' sequence: > > static int nand_exec_cmd(u32 cmd) > { > int ret; > > ret = nand_wait_cmd_fifo_empty(); > if (ret) > return ret; > > writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); > writel(cmd, SUNXI_NFC_BASE + NFC_CMD); > > return nand_wait_int(); > } It makes sense, I added another patch to introduce this helper. > > > } > > > > static void nand_apply_config(const struct nfc_config *conf) > > { > > u32 val; > > > > + nand_wait_cmd_fifo_empty(); > > + > > val = readl(SUNXI_NFC_BASE + NFC_CTL); > > val &= ~NFC_CTL_PAGE_SIZE_MASK; > > writel(val | NFC_CTL_RAM_METHOD | NFC_CTL_PAGE_SIZE(conf->page_size), > > @@ -200,6 +223,8 @@ static int nand_load_page(const struct nfc_config *conf, u32 offs) > > { > > int page = offs / conf->page_size; > > > > + nand_wait_cmd_fifo_empty(); > > + > > writel((NFC_CMD_RNDOUTSTART << NFC_RANDOM_READ_CMD1_OFFSET) | > > (NFC_CMD_RNDOUT << NFC_RANDOM_READ_CMD0_OFFSET) | > > (NFC_CMD_READSTART << NFC_READ_CMD_OFFSET), > > @@ -208,38 +233,32 @@ static int nand_load_page(const struct nfc_config *conf, u32 offs) > > writel((page >> 16) & 0xFF, SUNXI_NFC_BASE + NFC_ADDR_HIGH); > > writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); > > writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD | NFC_WAIT_FLAG | > > - ((conf->addr_cycles - 1) << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADR, > > + ((conf->addr_cycles - 1) << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR, > > SUNXI_NFC_BASE + NFC_CMD); > > > > - if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, > > - DEFAULT_TIMEOUT_US)) { > > - printf("Error while initializing dma interrupt\n"); > > - return -EIO; > > - } > > - > > - return 0; > > + return nand_wait_int(); > > } > > > > static int nand_reset_column(void) > > { > > + 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(0, SUNXI_NFC_BASE + NFC_ADDR_LOW); > > writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD | > > - (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADR | NFC_CMD_RNDOUT, > > + (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR | NFC_CMD_RNDOUT, > > SUNXI_NFC_BASE + NFC_CMD); > > > > - if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, > > - DEFAULT_TIMEOUT_US)) { > > - printf("Error while initializing dma interrupt\n"); > > - return -1; > > - } > > + return nand_wait_int(); > > Here, I think you should wait, just to make sure we don't read data > before tCCS. udelay(1) should be enough. Done and explained in another patch. > > > > > - return 0; > > } > > > > +static const int ecc_bytes[] = {32, 46, 54, 60, 74, 88, 102, 110, 116}; > > + > > Putting ecc_bytes out of nand_max_ecc_strength() is not really related > to this patch, and should probably go in patch 5. Right, I will move this to the patch where it belongs. > > > static int nand_read_page(const struct nfc_config *conf, u32 offs, > > void *dest, int len) > > { > > @@ -327,7 +346,6 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs, > > > > static int nand_max_ecc_strength(struct nfc_config *conf) > > { > > - static const int ecc_bytes[] = { 32, 46, 54, 60, 74, 88, 102, 110, 116 }; > > int max_oobsize, max_ecc_bytes; > > int nsectors = conf->page_size / conf->ecc_size; > > int i; > Thanks, Miquèl
diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c index 06695fc15f..2488d5cb51 100644 --- a/drivers/mtd/nand/sunxi_nand_spl.c +++ b/drivers/mtd/nand/sunxi_nand_spl.c @@ -55,8 +55,8 @@ #define NFC_ADDR_NUM_OFFSET 16 -#define NFC_SEND_ADR (1 << 19) #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) @@ -155,6 +155,30 @@ static inline int check_value_negated(int offset, int unexpected_bits, return check_value_inner(offset, unexpected_bits, timeout_us, 1); } +static int nand_wait_cmd_fifo_empty(void) +{ + if (!check_value_negated(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_FIFO_STAT, + DEFAULT_TIMEOUT_US)) { + printf("nand: timeout waiting for empty cmd FIFO\n"); + return -ETIMEDOUT; + } + + return 0; +} + +static int nand_wait_int(void) +{ + if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, + DEFAULT_TIMEOUT_US)) { + printf("nand: timeout waiting for interruption\n"); + return -ETIMEDOUT; + } + + udelay(1); + + return 0; +} + void nand_init(void) { uint32_t val; @@ -172,22 +196,21 @@ void nand_init(void) } /* reset NAND */ + nand_wait_cmd_fifo_empty(); + writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); writel(NFC_SEND_CMD1 | NFC_WAIT_FLAG | NAND_CMD_RESET, SUNXI_NFC_BASE + NFC_CMD); - if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, - DEFAULT_TIMEOUT_US)) { - printf("Error timeout waiting for nand reset\n"); - return; - } - writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); + nand_wait_int(); } static void nand_apply_config(const struct nfc_config *conf) { u32 val; + nand_wait_cmd_fifo_empty(); + val = readl(SUNXI_NFC_BASE + NFC_CTL); val &= ~NFC_CTL_PAGE_SIZE_MASK; writel(val | NFC_CTL_RAM_METHOD | NFC_CTL_PAGE_SIZE(conf->page_size), @@ -200,6 +223,8 @@ static int nand_load_page(const struct nfc_config *conf, u32 offs) { int page = offs / conf->page_size; + nand_wait_cmd_fifo_empty(); + writel((NFC_CMD_RNDOUTSTART << NFC_RANDOM_READ_CMD1_OFFSET) | (NFC_CMD_RNDOUT << NFC_RANDOM_READ_CMD0_OFFSET) | (NFC_CMD_READSTART << NFC_READ_CMD_OFFSET), @@ -208,38 +233,32 @@ static int nand_load_page(const struct nfc_config *conf, u32 offs) writel((page >> 16) & 0xFF, SUNXI_NFC_BASE + NFC_ADDR_HIGH); writel(NFC_ST_CMD_INT_FLAG, SUNXI_NFC_BASE + NFC_ST); writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD | NFC_WAIT_FLAG | - ((conf->addr_cycles - 1) << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADR, + ((conf->addr_cycles - 1) << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR, SUNXI_NFC_BASE + NFC_CMD); - if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, - DEFAULT_TIMEOUT_US)) { - printf("Error while initializing dma interrupt\n"); - return -EIO; - } - - return 0; + return nand_wait_int(); } static int nand_reset_column(void) { + 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(0, SUNXI_NFC_BASE + NFC_ADDR_LOW); writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD | - (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADR | NFC_CMD_RNDOUT, + (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADDR | NFC_CMD_RNDOUT, SUNXI_NFC_BASE + NFC_CMD); - if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, - DEFAULT_TIMEOUT_US)) { - printf("Error while initializing dma interrupt\n"); - return -1; - } + return nand_wait_int(); - return 0; } +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) { @@ -327,7 +346,6 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs, static int nand_max_ecc_strength(struct nfc_config *conf) { - static const int ecc_bytes[] = { 32, 46, 54, 60, 74, 88, 102, 110, 116 }; int max_oobsize, max_ecc_bytes; int nsectors = conf->page_size / conf->ecc_size; int i;
Do some cleaning in sunxi NAND SPL driver like adding helpers and clearing flags at the right spot Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> --- drivers/mtd/nand/sunxi_nand_spl.c | 64 +++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 23 deletions(-)