| Message ID | 1335576594-25267-8-git-send-email-computersforpeace@gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
On 04/27/2012 08:29 PM, Brian Norris wrote: > @@ -717,7 +718,8 @@ static void fsl_ifc_write_page(struct mtd_info *mtd, struct nand_chip *chip, > const uint8_t *buf, int oob_required) > { > fsl_ifc_write_buf(mtd, buf, mtd->writesize); > - fsl_ifc_write_buf(mtd, chip->oob_poi, mtd->oobsize); > + if (oob_required) > + fsl_ifc_write_buf(mtd, chip->oob_poi, mtd->oobsize); > } This will result in writing junk to the non-ECC OOB bytes as opposed to leaving it alone. -Scott
On Mon, Apr 30, 2012 at 9:43 AM, Scott Wood <scottwood@freescale.com> wrote: > On 04/27/2012 08:29 PM, Brian Norris wrote: >> @@ -717,7 +718,8 @@ static void fsl_ifc_write_page(struct mtd_info *mtd, struct nand_chip *chip, >> const uint8_t *buf, int oob_required) >> { >> fsl_ifc_write_buf(mtd, buf, mtd->writesize); >> - fsl_ifc_write_buf(mtd, chip->oob_poi, mtd->oobsize); >> + if (oob_required) >> + fsl_ifc_write_buf(mtd, chip->oob_poi, mtd->oobsize); >> } > > This will result in writing junk to the non-ECC OOB bytes as opposed to > leaving it alone. Then I'll drop the write_page change from this patch. Is the read_page change sane? Did you review the (misspelled) eLBC patch? (patch 06/10) Thanks. Brian
On 04/30/2012 02:08 PM, Brian Norris wrote: > On Mon, Apr 30, 2012 at 9:43 AM, Scott Wood <scottwood@freescale.com> wrote: >> On 04/27/2012 08:29 PM, Brian Norris wrote: >>> @@ -717,7 +718,8 @@ static void fsl_ifc_write_page(struct mtd_info *mtd, struct nand_chip *chip, >>> const uint8_t *buf, int oob_required) >>> { >>> fsl_ifc_write_buf(mtd, buf, mtd->writesize); >>> - fsl_ifc_write_buf(mtd, chip->oob_poi, mtd->oobsize); >>> + if (oob_required) >>> + fsl_ifc_write_buf(mtd, chip->oob_poi, mtd->oobsize); >>> } >> >> This will result in writing junk to the non-ECC OOB bytes as opposed to >> leaving it alone. > > Then I'll drop the write_page change from this patch. > > Is the read_page change sane? It should be harmless. > Did you review the (misspelled) eLBC patch? (patch 06/10) No, that one wasn't CCed to me (I should probably get around to subscribing to linux-mtd...). It looks like the same situation as IFC. -Scott
On Mon, Apr 30, 2012 at 12:13 PM, Scott Wood <scottwood@freescale.com> wrote: > On 04/30/2012 02:08 PM, Brian Norris wrote: >> Is the read_page change sane? > > It should be harmless. Right, but I am now handling Shmulik's comments about auto-incremented NAND. It seems like there are few/no users of auto-incrementing page reads, but if any driver relied on reading page,oob,page,oob,etc. without a READ command in between, then we might cause problems by skipping the OOB buffer read. Of course, with various NAND controllers/drivers, this issue is hard for me to sort through. >> Did you review the (misspelled) eLBC patch? (patch 06/10) > > No, that one wasn't CCed to me (I should probably get around to > subscribing to linux-mtd...). scripts/get_maintainers.pl only goes so far. It's hard to track down the *real* experts here... > It looks like the same situation as IFC. OK, I'll fix it too then. Thanks. Brian
On 04/30/2012 02:23 PM, Brian Norris wrote: > On Mon, Apr 30, 2012 at 12:13 PM, Scott Wood <scottwood@freescale.com> wrote: >> On 04/30/2012 02:08 PM, Brian Norris wrote: >>> Is the read_page change sane? >> >> It should be harmless. > > Right, but I am now handling Shmulik's comments about auto-incremented > NAND. It seems like there are few/no users of auto-incrementing page > reads, but if any driver relied on reading page,oob,page,oob,etc. > without a READ command in between, then we might cause problems by > skipping the OOB buffer read. Of course, with various NAND > controllers/drivers, this issue is hard for me to sort through. This isn't an issue with eLBC/IFC -- besides not currently supporting autoincrement, the OOB is still read. We just wouldn't pull it out of the controller buffer. Not sure if the time saved is significant. -Scott
diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c index a8d1e87..2bbb9d5 100644 --- a/drivers/mtd/nand/fsl_ifc_nand.c +++ b/drivers/mtd/nand/fsl_ifc_nand.c @@ -699,7 +699,8 @@ static int fsl_ifc_read_page(struct mtd_info *mtd, struct nand_chip *chip, struct fsl_ifc_ctrl *ctrl = priv->ctrl; fsl_ifc_read_buf(mtd, buf, mtd->writesize); - fsl_ifc_read_buf(mtd, chip->oob_poi, mtd->oobsize); + if (oob_required) + fsl_ifc_read_buf(mtd, chip->oob_poi, mtd->oobsize); if (ctrl->nand_stat & IFC_NAND_EVTER_STAT_ECCER) dev_err(priv->dev, "NAND Flash ECC Uncorrectable Error\n"); @@ -717,7 +718,8 @@ static void fsl_ifc_write_page(struct mtd_info *mtd, struct nand_chip *chip, const uint8_t *buf, int oob_required) { fsl_ifc_write_buf(mtd, buf, mtd->writesize); - fsl_ifc_write_buf(mtd, chip->oob_poi, mtd->oobsize); + if (oob_required) + fsl_ifc_write_buf(mtd, chip->oob_poi, mtd->oobsize); } static int fsl_ifc_chip_init_tail(struct mtd_info *mtd)
Don't read/write OOB if the caller doesn't requre it. Signed-off-by: Brian Norris <computersforpeace@gmail.com> --- drivers/mtd/nand/fsl_ifc_nand.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)