| Message ID | 1396308537-16013-5-git-send-email-davidm@egauge.net |
|---|---|
| State | Rejected |
| Headers | show |
On Mon, Mar 31, 2014 at 05:28:56PM -0600, David Mosberger wrote: > To avoid unnecessary rewrites, it is necessary to count the number of > actual bitflips that occurred when the NAND_STATUS_REWRITE bit is set. > This patch introduces the extra buffers needed to implement that > counting. The actual counting is in the next patch. > > Signed-off-by: David Mosberger <davidm@egauge.net> > --- > drivers/mtd/nand/nand_base.c | 13 ++++++++++++- > include/linux/mtd/nand.h | 2 ++ > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index 435ef44..834352a 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -3935,13 +3935,24 @@ int nand_scan_tail(struct mtd_info *mtd) > !(chip->bbt_options & NAND_BBT_USE_FLASH)); > > if (!(chip->options & NAND_OWN_BUFFERS)) { > + size_t on_die_bufsz = 0; > + > + if (chip->ecc.mode == NAND_ECC_HW_ON_DIE) > + on_die_bufsz = 2*(mtd->writesize + mtd->oobsize); Spacing: on_die_bufsz = 2 * (mtd->writesize + mtd->oobsize); > + > nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize > - + mtd->oobsize * 3, GFP_KERNEL); > + + mtd->oobsize * 3 + on_die_bufsz, GFP_KERNEL); > if (!nbuf) > return -ENOMEM; > nbuf->ecccalc = (uint8_t *)(nbuf + 1); > nbuf->ecccode = nbuf->ecccalc + mtd->oobsize; > nbuf->databuf = nbuf->ecccode + mtd->oobsize; > + if (chip->ecc.mode == NAND_ECC_HW_ON_DIE) { > + nbuf->chkbuf = (nbuf->databuf + mtd->writesize > + + mtd->oobsize); > + nbuf->rawbuf = (nbuf->chkbuf + mtd->writesize > + + mtd->oobsize); > + } > > chip->buffers = nbuf; > } else { > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index dbb99b3..456809b 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -533,6 +533,8 @@ struct nand_buffers { > uint8_t *ecccalc; > uint8_t *ecccode; > uint8_t *databuf; > + uint8_t *chkbuf; > + uint8_t *rawbuf; Do you really need two additional buffers? Can you get by with just one of them? > }; > > /** Brian
Hi Brian, >From: Brian Norris [mailto:computersforpeace@gmail.com] >>On Mon, Mar 31, 2014 at 05:28:56PM -0600, David Mosberger wrote: [...] >> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h >> index dbb99b3..456809b 100644 >> --- a/include/linux/mtd/nand.h >> +++ b/include/linux/mtd/nand.h >> @@ -533,6 +533,8 @@ struct nand_buffers { >> uint8_t *ecccalc; >> uint8_t *ecccode; >> uint8_t *databuf; >> + uint8_t *chkbuf; >> + uint8_t *rawbuf; > >Do you really need two additional buffers? Can you get by with just one >of them? > Some similar comments have been provided in the previous versions of the patch. But due to in-consistency in $subject, they might not be visible as a thread of same discussion. You may like to review and comment on below threads also. [PATCH] mtd: nand: Add support for Micron on-die ECC controller (rev2). http://lists.infradead.org/pipermail/linux-mtd/2014-March/052949.html [RFC] mtd: nand: Preparatory patch for adding on-die ECC controller support ... http://lists.infradead.org/pipermail/linux-mtd/2014-March/052969.html with regards, pekon
Hi Pekon, On Tue, Apr 01, 2014 at 07:37:46AM +0000, Pekon Gupta wrote: > >From: Brian Norris [mailto:computersforpeace@gmail.com] > >>On Mon, Mar 31, 2014 at 05:28:56PM -0600, David Mosberger wrote: > [...] > >> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > >> index dbb99b3..456809b 100644 > >> --- a/include/linux/mtd/nand.h > >> +++ b/include/linux/mtd/nand.h > >> @@ -533,6 +533,8 @@ struct nand_buffers { > >> uint8_t *ecccalc; > >> uint8_t *ecccode; > >> uint8_t *databuf; > >> + uint8_t *chkbuf; > >> + uint8_t *rawbuf; > > > >Do you really need two additional buffers? Can you get by with just one > >of them? > > > > Some similar comments have been provided in the previous versions > of the patch. But due to in-consistency in $subject, they might not > be visible as a thread of same discussion. Yeah, I didn't review every iteration so far. I'm possibly repeating some things. I'll see where to respond on the old threads. But here's a note to David: it looks like you sent 4 revisions in one week. That's a bit fast. Perhaps you can wait a few more days on spinning patch sets, especially when review is straddling a weekend like that. > You may like to review and comment on below threads also. > > [PATCH] mtd: nand: Add support for Micron on-die ECC controller (rev2). > http://lists.infradead.org/pipermail/linux-mtd/2014-March/052949.html > > [RFC] mtd: nand: Preparatory patch for adding on-die ECC controller support ... > http://lists.infradead.org/pipermail/linux-mtd/2014-March/052969.html Brian
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 435ef44..834352a 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -3935,13 +3935,24 @@ int nand_scan_tail(struct mtd_info *mtd) !(chip->bbt_options & NAND_BBT_USE_FLASH)); if (!(chip->options & NAND_OWN_BUFFERS)) { + size_t on_die_bufsz = 0; + + if (chip->ecc.mode == NAND_ECC_HW_ON_DIE) + on_die_bufsz = 2*(mtd->writesize + mtd->oobsize); + nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize - + mtd->oobsize * 3, GFP_KERNEL); + + mtd->oobsize * 3 + on_die_bufsz, GFP_KERNEL); if (!nbuf) return -ENOMEM; nbuf->ecccalc = (uint8_t *)(nbuf + 1); nbuf->ecccode = nbuf->ecccalc + mtd->oobsize; nbuf->databuf = nbuf->ecccode + mtd->oobsize; + if (chip->ecc.mode == NAND_ECC_HW_ON_DIE) { + nbuf->chkbuf = (nbuf->databuf + mtd->writesize + + mtd->oobsize); + nbuf->rawbuf = (nbuf->chkbuf + mtd->writesize + + mtd->oobsize); + } chip->buffers = nbuf; } else { diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index dbb99b3..456809b 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -533,6 +533,8 @@ struct nand_buffers { uint8_t *ecccalc; uint8_t *ecccode; uint8_t *databuf; + uint8_t *chkbuf; + uint8_t *rawbuf; }; /**
To avoid unnecessary rewrites, it is necessary to count the number of actual bitflips that occurred when the NAND_STATUS_REWRITE bit is set. This patch introduces the extra buffers needed to implement that counting. The actual counting is in the next patch. Signed-off-by: David Mosberger <davidm@egauge.net> --- drivers/mtd/nand/nand_base.c | 13 ++++++++++++- include/linux/mtd/nand.h | 2 ++ 2 files changed, 14 insertions(+), 1 deletion(-)