Hi Boris,
2017-06-07 22:33 GMT+09:00 Boris Brezillon <[email protected]>: > On Wed, 7 Jun 2017 20:52:16 +0900 > Masahiro Yamada <[email protected]> wrote: > >> Currently, the error handling of denali_write_page(_raw) is a bit >> complicated. If the program command fails, NAND_STATUS_FAIL is set >> to the driver internal denali->status, then read out later by >> denali_waitfunc(). >> >> We can avoid it by exploiting the nand_write_page() implementation. >> If chip->ecc.write_page(_raw) returns negative code (i.e. -EIO), it >> errors out immediately. This gives the same result as returning >> NAND_STATUS_FAIL from chip->waitfunc. In either way, -EIO is >> returned to the upper MTD layer. > > Actually, this is how it's supposed to work now (when they set > the NAND_ECC_CUSTOM_PAGE_ACCESS flag, drivers are expected to wait for > the program operation to finish and return -EIO if it failed), so you're > all good ;-). > >> >> Signed-off-by: Masahiro Yamada <[email protected]> >> --- >> >> Changes in v5: None >> Changes in v4: None >> Changes in v3: None >> Changes in v2: >> - Newly added >> >> drivers/mtd/nand/denali.c | 12 ++++-------- >> drivers/mtd/nand/denali.h | 1 - >> 2 files changed, 4 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c >> index 1897fe238290..22acfc34b546 100644 >> --- a/drivers/mtd/nand/denali.c >> +++ b/drivers/mtd/nand/denali.c >> @@ -1005,6 +1005,7 @@ static int write_page(struct mtd_info *mtd, struct >> nand_chip *chip, >> size_t size = mtd->writesize + mtd->oobsize; >> uint32_t irq_status; >> uint32_t irq_mask = INTR__DMA_CMD_COMP | INTR__PROGRAM_FAIL; > > As mentioned in my previous patch, I think you should wait for > INTR__PROGRAM_COMP | INTR__PROGRAM_FAIL here. No. It is intentional to use INTR__DMA_CMD_COMP instead of INTR__PROGRAM_COMP here. This is very strange of this IP, INTR__PROGRAM_COMP is never set when DMA mode is being used. (INTR__DMA_CMD_COMP is set instead.) As far as I tested this IP, INTR__PROGRAM_COMP is set only when data are written by PIO mode. I introduced PIO transfer in http://patchwork.ozlabs.org/patch/772398/ I used INTR__PROGRAM_COMP in denali_pio_write(). >> + int ret = 0; >> >> denali->page = page; >> >> @@ -1038,13 +1039,13 @@ static int write_page(struct mtd_info *mtd, struct >> nand_chip *chip, >> if (irq_status == 0) { >> dev_err(denali->dev, "timeout on write_page (type = %d)\n", >> raw_xfer); >> - denali->status = NAND_STATUS_FAIL; >> + ret = -EIO; >> } > > if (irq_status & INTR__PROGRAM_FAIL) { > dev_err(denali->dev, "page program failed (type = %d)\n", > raw_xfer); > ret = -EIO; > } This will be fixed anyway by http://patchwork.ozlabs.org/patch/772414/ I do not want to include unrelated change. -- Best Regards Masahiro Yamada
