| Message ID | 20200427082028.394719-12-boris.brezillon@collabora.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Miquel Raynal |
| Headers | show |
| Series | mtd: rawnand: cafe: Convert to exec_op() (and more) | expand |
Hi Boris, Boris Brezillon <boris.brezillon@collabora.com> wrote on Mon, 27 Apr 2020 10:20:21 +0200: > cafe_nand_write_page_lowlevel() sets the ECC auto-generation flag but IIRC you renamed this function in patch 1 so now it is named "_write_page()". > never clears it, thus forcing the cafe_nand_cmdfunc() to clear it > in certain circumstances. Let's just clear this flag in > cafe_nand_write_page_lowlevel() instead. Same here ^ > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > drivers/mtd/nand/raw/cafe_nand.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/nand/raw/cafe_nand.c b/drivers/mtd/nand/raw/cafe_nand.c > index 2825489a71b8..31493a201a02 100644 > --- a/drivers/mtd/nand/raw/cafe_nand.c > +++ b/drivers/mtd/nand/raw/cafe_nand.c > @@ -261,7 +261,6 @@ static void cafe_nand_cmdfunc(struct nand_chip *chip, unsigned command, > CAFE_FIELD_PREP(NAND_CTRL2, CMD2, command), > NAND_CTRL2); > ctl1 = cafe->ctl1; > - cafe->ctl2 &= ~CAFE_NAND_CTRL2_AUTO_WRITE_ECC; > dev_dbg(&cafe->pdev->dev, "Continue command, ctl1 %08x, #data %d\n", > cafe->ctl1, cafe->nr_data); > goto do_command; > @@ -643,6 +642,7 @@ static int cafe_nand_write_page(struct nand_chip *chip, > { > struct mtd_info *mtd = nand_to_mtd(chip); > struct cafe_priv *cafe = nand_get_controller_data(chip); > + int ret; > > nand_prog_page_begin_op(chip, page, 0, buf, mtd->writesize); > chip->legacy.write_buf(chip, chip->oob_poi, mtd->oobsize); > @@ -650,7 +650,14 @@ static int cafe_nand_write_page(struct nand_chip *chip, > /* Set up ECC autogeneration */ > cafe->ctl2 |= CAFE_NAND_CTRL2_AUTO_WRITE_ECC; > > - return nand_prog_page_end_op(chip); > + ret = nand_prog_page_end_op(chip); > + > + /* > + * And clear it before returning so that following write operations > + * that do not involve ECC don't generate ECC bytes. > + */ > + cafe->ctl2 &= ~CAFE_NAND_CTRL2_AUTO_WRITE_ECC; I like spaces before returning, but again it's really a nitpick, you can ignore that :) > + return ret; > } > > /* F_2[X]/(X**6+X+1) */ With the commit log updated Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com> Thanks, Miquèl
On Mon, 27 Apr 2020 21:51:32 +0200 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > Hi Boris, > > Boris Brezillon <boris.brezillon@collabora.com> wrote on Mon, 27 Apr > 2020 10:20:21 +0200: > > > cafe_nand_write_page_lowlevel() sets the ECC auto-generation flag but > > IIRC you renamed this function in patch 1 so now it is named > "_write_page()". > > > never clears it, thus forcing the cafe_nand_cmdfunc() to clear it > > in certain circumstances. Let's just clear this flag in > > cafe_nand_write_page_lowlevel() instead. > > Same here ^ > Right. That's what happens when you re-order commits without paying attention to the commit message. > > - return nand_prog_page_end_op(chip); > > + ret = nand_prog_page_end_op(chip); > > + > > + /* > > + * And clear it before returning so that following write operations > > + * that do not involve ECC don't generate ECC bytes. > > + */ > > + cafe->ctl2 &= ~CAFE_NAND_CTRL2_AUTO_WRITE_ECC; > > I like spaces before returning, but again it's really a nitpick, you > can ignore that :) > Will add a blank line here as well. > > + return ret; > > } > > > > /* F_2[X]/(X**6+X+1) */
diff --git a/drivers/mtd/nand/raw/cafe_nand.c b/drivers/mtd/nand/raw/cafe_nand.c index 2825489a71b8..31493a201a02 100644 --- a/drivers/mtd/nand/raw/cafe_nand.c +++ b/drivers/mtd/nand/raw/cafe_nand.c @@ -261,7 +261,6 @@ static void cafe_nand_cmdfunc(struct nand_chip *chip, unsigned command, CAFE_FIELD_PREP(NAND_CTRL2, CMD2, command), NAND_CTRL2); ctl1 = cafe->ctl1; - cafe->ctl2 &= ~CAFE_NAND_CTRL2_AUTO_WRITE_ECC; dev_dbg(&cafe->pdev->dev, "Continue command, ctl1 %08x, #data %d\n", cafe->ctl1, cafe->nr_data); goto do_command; @@ -643,6 +642,7 @@ static int cafe_nand_write_page(struct nand_chip *chip, { struct mtd_info *mtd = nand_to_mtd(chip); struct cafe_priv *cafe = nand_get_controller_data(chip); + int ret; nand_prog_page_begin_op(chip, page, 0, buf, mtd->writesize); chip->legacy.write_buf(chip, chip->oob_poi, mtd->oobsize); @@ -650,7 +650,14 @@ static int cafe_nand_write_page(struct nand_chip *chip, /* Set up ECC autogeneration */ cafe->ctl2 |= CAFE_NAND_CTRL2_AUTO_WRITE_ECC; - return nand_prog_page_end_op(chip); + ret = nand_prog_page_end_op(chip); + + /* + * And clear it before returning so that following write operations + * that do not involve ECC don't generate ECC bytes. + */ + cafe->ctl2 &= ~CAFE_NAND_CTRL2_AUTO_WRITE_ECC; + return ret; } /* F_2[X]/(X**6+X+1) */
cafe_nand_write_page_lowlevel() sets the ECC auto-generation flag but never clears it, thus forcing the cafe_nand_cmdfunc() to clear it in certain circumstances. Let's just clear this flag in cafe_nand_write_page_lowlevel() instead. Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> --- drivers/mtd/nand/raw/cafe_nand.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)