| Message ID | 20190429063834.45967-5-xiaolei.li@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MTK NAND driver improvements and fixes | expand |
Hi Xiaolei, Xiaolei Li <xiaolei.li@mediatek.com> wrote on Mon, 29 Apr 2019 14:38:33 +0800: > One main goal of the function mtk_nfc_update_ecc_stats is to check > whether sectors are all empty. If they are empty, set these sectors's > data buffer and oob buffer as 0xff. > > But now, the sector oob buffer pointer is wrongly assigned. We always > do memset from sector 0. > > To fix this issue, pass start secotr number to make oob buffer pointer sector > be properly assigned. Please use upper case for plain English acronyms: NAND, ECC, OOB, etc. > > Fixes: 93db446a424c ("mtd: nand: move raw NAND related code to the raw/ subdir") Same comment as before, wrong commit pointed in the Fixes tag. > Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com> > --- > drivers/mtd/nand/raw/mtk_nand.c | 21 ++++++++++----------- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c > index 7a5e8c9cf61b..cf5e50e704ae 100644 > --- a/drivers/mtd/nand/raw/mtk_nand.c > +++ b/drivers/mtd/nand/raw/mtk_nand.c > @@ -873,19 +873,21 @@ static int mtk_nfc_write_oob_std(struct nand_chip *chip, int page) > return mtk_nfc_write_page_raw(chip, NULL, 1, page); > } > > -static int mtk_nfc_update_ecc_stats(struct mtd_info *mtd, u8 *buf, u32 sectors) > +static int mtk_nfc_update_ecc_stats(struct mtd_info *mtd, u8 *buf, u32 start, > + u32 sectors) > { > struct nand_chip *chip = mtd_to_nand(mtd); > struct mtk_nfc *nfc = nand_get_controller_data(chip); > struct mtk_nfc_nand_chip *mtk_nand = to_mtk_nand(chip); > struct mtk_ecc_stats stats; > + u32 reg_size = mtk_nand->fdm.reg_size; > int rc, i; > > rc = nfi_readl(nfc, NFI_STA) & STA_EMP_PAGE; > if (rc) { > memset(buf, 0xff, sectors * chip->ecc.size); > for (i = 0; i < sectors; i++) > - memset(oob_ptr(chip, i), 0xff, mtk_nand->fdm.reg_size); > + memset(oob_ptr(chip, start + i), 0xff, reg_size); > return 0; > } > > @@ -905,7 +907,7 @@ static int mtk_nfc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, > u32 spare = mtk_nand->spare_per_sector; > u32 column, sectors, start, end, reg; > dma_addr_t addr; > - int bitflips; > + int bitflips = 0; > size_t len; > u8 *buf; > int rc; > @@ -972,14 +974,11 @@ static int mtk_nfc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, > if (rc < 0) { > dev_err(nfc->dev, "subpage done timeout\n"); > bitflips = -EIO; > - } else { > - bitflips = 0; > - if (!raw) { > - rc = mtk_ecc_wait_done(nfc->ecc, ECC_DECODE); > - bitflips = rc < 0 ? -ETIMEDOUT : > - mtk_nfc_update_ecc_stats(mtd, buf, sectors); > - mtk_nfc_read_fdm(chip, start, sectors); > - } > + } else if (!raw) { > + rc = mtk_ecc_wait_done(nfc->ecc, ECC_DECODE); > + bitflips = rc < 0 ? -ETIMEDOUT : > + mtk_nfc_update_ecc_stats(mtd, buf, start, sectors); > + mtk_nfc_read_fdm(chip, start, sectors); > } > > dma_unmap_single(nfc->dev, addr, len, DMA_FROM_DEVICE); With this addressed: Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com> Thanks, Miquèl
Hi Miquel, On Mon, 2019-04-29 at 11:14 +0200, Miquel Raynal wrote: > Hi Xiaolei, > > Xiaolei Li <xiaolei.li@mediatek.com> wrote on Mon, 29 Apr 2019 14:38:33 > +0800: > > > One main goal of the function mtk_nfc_update_ecc_stats is to check > > whether sectors are all empty. If they are empty, set these sectors's > > data buffer and oob buffer as 0xff. > > > > But now, the sector oob buffer pointer is wrongly assigned. We always > > do memset from sector 0. > > > > To fix this issue, pass start secotr number to make oob buffer pointer > > sector Thanks. Will change this typo. > > > be properly assigned. > > Please use upper case for plain English acronyms: NAND, ECC, OOB, etc. OK. > > > > > Fixes: 93db446a424c ("mtd: nand: move raw NAND related code to the raw/ subdir") > > Same comment as before, wrong commit pointed in the Fixes tag. > > > Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com> > > --- > > drivers/mtd/nand/raw/mtk_nand.c | 21 ++++++++++----------- > > 1 file changed, 10 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c > > index 7a5e8c9cf61b..cf5e50e704ae 100644 > > --- a/drivers/mtd/nand/raw/mtk_nand.c > > +++ b/drivers/mtd/nand/raw/mtk_nand.c > > @@ -873,19 +873,21 @@ static int mtk_nfc_write_oob_std(struct nand_chip *chip, int page) > > return mtk_nfc_write_page_raw(chip, NULL, 1, page); > > } > > > > -static int mtk_nfc_update_ecc_stats(struct mtd_info *mtd, u8 *buf, u32 sectors) > > +static int mtk_nfc_update_ecc_stats(struct mtd_info *mtd, u8 *buf, u32 start, > > + u32 sectors) > > { > > struct nand_chip *chip = mtd_to_nand(mtd); > > struct mtk_nfc *nfc = nand_get_controller_data(chip); > > struct mtk_nfc_nand_chip *mtk_nand = to_mtk_nand(chip); > > struct mtk_ecc_stats stats; > > + u32 reg_size = mtk_nand->fdm.reg_size; > > int rc, i; > > > > rc = nfi_readl(nfc, NFI_STA) & STA_EMP_PAGE; > > if (rc) { > > memset(buf, 0xff, sectors * chip->ecc.size); > > for (i = 0; i < sectors; i++) > > - memset(oob_ptr(chip, i), 0xff, mtk_nand->fdm.reg_size); > > + memset(oob_ptr(chip, start + i), 0xff, reg_size); > > return 0; > > } > > > > @@ -905,7 +907,7 @@ static int mtk_nfc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, > > u32 spare = mtk_nand->spare_per_sector; > > u32 column, sectors, start, end, reg; > > dma_addr_t addr; > > - int bitflips; > > + int bitflips = 0; > > size_t len; > > u8 *buf; > > int rc; > > @@ -972,14 +974,11 @@ static int mtk_nfc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, > > if (rc < 0) { > > dev_err(nfc->dev, "subpage done timeout\n"); > > bitflips = -EIO; > > - } else { > > - bitflips = 0; > > - if (!raw) { > > - rc = mtk_ecc_wait_done(nfc->ecc, ECC_DECODE); > > - bitflips = rc < 0 ? -ETIMEDOUT : > > - mtk_nfc_update_ecc_stats(mtd, buf, sectors); > > - mtk_nfc_read_fdm(chip, start, sectors); > > - } > > + } else if (!raw) { > > + rc = mtk_ecc_wait_done(nfc->ecc, ECC_DECODE); > > + bitflips = rc < 0 ? -ETIMEDOUT : > > + mtk_nfc_update_ecc_stats(mtd, buf, start, sectors); > > + mtk_nfc_read_fdm(chip, start, sectors); > > } > > > > dma_unmap_single(nfc->dev, addr, len, DMA_FROM_DEVICE); > > With this addressed: > > Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > Thanks, > Miquèl
diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c index 7a5e8c9cf61b..cf5e50e704ae 100644 --- a/drivers/mtd/nand/raw/mtk_nand.c +++ b/drivers/mtd/nand/raw/mtk_nand.c @@ -873,19 +873,21 @@ static int mtk_nfc_write_oob_std(struct nand_chip *chip, int page) return mtk_nfc_write_page_raw(chip, NULL, 1, page); } -static int mtk_nfc_update_ecc_stats(struct mtd_info *mtd, u8 *buf, u32 sectors) +static int mtk_nfc_update_ecc_stats(struct mtd_info *mtd, u8 *buf, u32 start, + u32 sectors) { struct nand_chip *chip = mtd_to_nand(mtd); struct mtk_nfc *nfc = nand_get_controller_data(chip); struct mtk_nfc_nand_chip *mtk_nand = to_mtk_nand(chip); struct mtk_ecc_stats stats; + u32 reg_size = mtk_nand->fdm.reg_size; int rc, i; rc = nfi_readl(nfc, NFI_STA) & STA_EMP_PAGE; if (rc) { memset(buf, 0xff, sectors * chip->ecc.size); for (i = 0; i < sectors; i++) - memset(oob_ptr(chip, i), 0xff, mtk_nand->fdm.reg_size); + memset(oob_ptr(chip, start + i), 0xff, reg_size); return 0; } @@ -905,7 +907,7 @@ static int mtk_nfc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, u32 spare = mtk_nand->spare_per_sector; u32 column, sectors, start, end, reg; dma_addr_t addr; - int bitflips; + int bitflips = 0; size_t len; u8 *buf; int rc; @@ -972,14 +974,11 @@ static int mtk_nfc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, if (rc < 0) { dev_err(nfc->dev, "subpage done timeout\n"); bitflips = -EIO; - } else { - bitflips = 0; - if (!raw) { - rc = mtk_ecc_wait_done(nfc->ecc, ECC_DECODE); - bitflips = rc < 0 ? -ETIMEDOUT : - mtk_nfc_update_ecc_stats(mtd, buf, sectors); - mtk_nfc_read_fdm(chip, start, sectors); - } + } else if (!raw) { + rc = mtk_ecc_wait_done(nfc->ecc, ECC_DECODE); + bitflips = rc < 0 ? -ETIMEDOUT : + mtk_nfc_update_ecc_stats(mtd, buf, start, sectors); + mtk_nfc_read_fdm(chip, start, sectors); } dma_unmap_single(nfc->dev, addr, len, DMA_FROM_DEVICE);
One main goal of the function mtk_nfc_update_ecc_stats is to check whether sectors are all empty. If they are empty, set these sectors's data buffer and oob buffer as 0xff. But now, the sector oob buffer pointer is wrongly assigned. We always do memset from sector 0. To fix this issue, pass start secotr number to make oob buffer pointer be properly assigned. Fixes: 93db446a424c ("mtd: nand: move raw NAND related code to the raw/ subdir") Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com> --- drivers/mtd/nand/raw/mtk_nand.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)