| Message ID | 1265326257-4446-11-git-send-email-maximlevitsky@gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
Maxim Levitsky wrote: > This changes the behavier of MTD_OOB_RAW. It used to read both OOB and data > to the data buffer, however you would still need to specify the dummy oob buffer. > > This is only used in one place, but makes it hard to read data+oob without ECC > test, thus I removed that behavier, and fixed the user. > > Now MTD_OOB_RAW behaves like MTD_OOB_PLACE, but doesn't do ECC validation > > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com> > --- > drivers/mtd/nand/nand_base.c | 5 ----- > drivers/mtd/nand/nand_bbt.c | 26 ++++++++++++++++++++++---- > include/linux/mtd/mtd.h | 4 +--- > 3 files changed, 23 insertions(+), 12 deletions(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index 405c538..8ff36be 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -1286,8 +1286,6 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from, > > if (unlikely(oob)) { > > - /* Raw mode does data:oob:data:oob */ > - if (ops->mode != MTD_OOB_RAW) { > I am not sure if it is legal to modify the behavior of MTD_OOB_RAW. But if you remove the "if" command, the following code should be indent again. Stanley. > int toread = min(oobreadlen, > max_oobsize); > if (toread) { > @@ -1295,9 +1293,6 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from, > oob, ops, toread); > oobreadlen -= toread; > } > - } else > - buf = nand_transfer_oob(chip, > - buf, ops, mtd->oobsize); > } > > if (!(chip->options & NAND_NO_READRDY)) { > diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c > index 55c23e5..387c45c 100644 > --- a/drivers/mtd/nand/nand_bbt.c > +++ b/drivers/mtd/nand/nand_bbt.c > @@ -237,15 +237,33 @@ static int scan_read_raw(struct mtd_info *mtd, uint8_t *buf, loff_t offs, > size_t len) > { > struct mtd_oob_ops ops; > + int res; > > ops.mode = MTD_OOB_RAW; > ops.ooboffs = 0; > ops.ooblen = mtd->oobsize; > - ops.oobbuf = buf; > - ops.datbuf = buf; > - ops.len = len; > > - return mtd->read_oob(mtd, offs, &ops); > + > + while (len > 0) { > + if (len <= mtd->writesize) { > + ops.oobbuf = buf + len; > + ops.datbuf = buf; > + ops.len = len; > + return mtd->read_oob(mtd, offs, &ops); > + } else { > + ops.oobbuf = buf + mtd->writesize; > + ops.datbuf = buf; > + ops.len = mtd->writesize; > + res = mtd->read_oob(mtd, offs, &ops); > + > + if (res) > + return res; > + } > + > + buf += mtd->oobsize + mtd->writesize; > + len -= mtd->writesize; > + } > + return 0; > } > > /* > diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h > index 662d747..84bb375 100644 > --- a/include/linux/mtd/mtd.h > +++ b/include/linux/mtd/mtd.h > @@ -61,9 +61,7 @@ struct mtd_erase_region_info { > * MTD_OOB_PLACE: oob data are placed at the given offset > * MTD_OOB_AUTO: oob data are automatically placed at the free areas > * which are defined by the ecclayout > - * MTD_OOB_RAW: mode to read raw data+oob in one chunk. The oob data > - * is inserted into the data. Thats a raw image of the > - * flash contents. > + * MTD_OOB_RAW: mode to read oob and data without doing ECC checking > */ > typedef enum { > MTD_OOB_PLACE, >
On Fri, Feb 5, 2010 at 12:30 AM, Maxim Levitsky <maximlevitsky@gmail.com> wrote: > This changes the behavier of MTD_OOB_RAW. It used to read both OOB and data > to the data buffer, however you would still need to specify the dummy oob buffer. > > This is only used in one place, but makes it hard to read data+oob without ECC > test, thus I removed that behavier, and fixed the user. > > Now MTD_OOB_RAW behaves like MTD_OOB_PLACE, but doesn't do ECC validation > > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com> > --- > drivers/mtd/nand/nand_base.c | 5 ----- > drivers/mtd/nand/nand_bbt.c | 26 ++++++++++++++++++++++---- > include/linux/mtd/mtd.h | 4 +--- > 3 files changed, 23 insertions(+), 12 deletions(-) Could you please provide a clearer description of why you think this is necessary? Thanks, Vitaly
On Fri, 2010-02-05 at 09:56 +0100, Vitaly Wool wrote: > On Fri, Feb 5, 2010 at 12:30 AM, Maxim Levitsky <maximlevitsky@gmail.com> wrote: > > This changes the behavier of MTD_OOB_RAW. It used to read both OOB and data > > to the data buffer, however you would still need to specify the dummy oob buffer. > > > > This is only used in one place, but makes it hard to read data+oob without ECC > > test, thus I removed that behavier, and fixed the user. > > > > Now MTD_OOB_RAW behaves like MTD_OOB_PLACE, but doesn't do ECC validation > > > > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com> > > --- > > drivers/mtd/nand/nand_base.c | 5 ----- > > drivers/mtd/nand/nand_bbt.c | 26 ++++++++++++++++++++++---- > > include/linux/mtd/mtd.h | 4 +--- > > 3 files changed, 23 insertions(+), 12 deletions(-) > > Could you please provide a clearer description of why you think this > is necessary? Sure. I have two distinct buffers. One holds the data and is 512 bytes long, other is the oob and is 16 bytes long. I want to write these buffers to flash but without ECC Currently I would have to allocate new buffer and copy both buffers there. Why I need it? To make my SmartMedia FTL complete I need to add support for 256 byte nands. These have 256 bytes per page and 8 bytes of oob. However, pages are always written in pairs, forming 512 bytes sector. Combined oob of first and second page is identical to the one that is written to 512 byte page, thus its layout depends on page number. Also I want the nand driver to have as little as possible smartmedia dependencies, thus allow it to be used with any FTL, UBI, jffs2, etc.. Therefore I decided to make nand driver use standard oob layout and ECC in case of 256 byte flash, but make my FTL bypass it. It also happens to work with nandsim unmodified... :-) Also, this makes the interface more orthogonal. This feature is used in just one place too. Best regards, Maxim Levitsky
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 405c538..8ff36be 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -1286,8 +1286,6 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from, if (unlikely(oob)) { - /* Raw mode does data:oob:data:oob */ - if (ops->mode != MTD_OOB_RAW) { int toread = min(oobreadlen, max_oobsize); if (toread) { @@ -1295,9 +1293,6 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from, oob, ops, toread); oobreadlen -= toread; } - } else - buf = nand_transfer_oob(chip, - buf, ops, mtd->oobsize); } if (!(chip->options & NAND_NO_READRDY)) { diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c index 55c23e5..387c45c 100644 --- a/drivers/mtd/nand/nand_bbt.c +++ b/drivers/mtd/nand/nand_bbt.c @@ -237,15 +237,33 @@ static int scan_read_raw(struct mtd_info *mtd, uint8_t *buf, loff_t offs, size_t len) { struct mtd_oob_ops ops; + int res; ops.mode = MTD_OOB_RAW; ops.ooboffs = 0; ops.ooblen = mtd->oobsize; - ops.oobbuf = buf; - ops.datbuf = buf; - ops.len = len; - return mtd->read_oob(mtd, offs, &ops); + + while (len > 0) { + if (len <= mtd->writesize) { + ops.oobbuf = buf + len; + ops.datbuf = buf; + ops.len = len; + return mtd->read_oob(mtd, offs, &ops); + } else { + ops.oobbuf = buf + mtd->writesize; + ops.datbuf = buf; + ops.len = mtd->writesize; + res = mtd->read_oob(mtd, offs, &ops); + + if (res) + return res; + } + + buf += mtd->oobsize + mtd->writesize; + len -= mtd->writesize; + } + return 0; } /* diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index 662d747..84bb375 100644 --- a/include/linux/mtd/mtd.h +++ b/include/linux/mtd/mtd.h @@ -61,9 +61,7 @@ struct mtd_erase_region_info { * MTD_OOB_PLACE: oob data are placed at the given offset * MTD_OOB_AUTO: oob data are automatically placed at the free areas * which are defined by the ecclayout - * MTD_OOB_RAW: mode to read raw data+oob in one chunk. The oob data - * is inserted into the data. Thats a raw image of the - * flash contents. + * MTD_OOB_RAW: mode to read oob and data without doing ECC checking */ typedef enum { MTD_OOB_PLACE,
This changes the behavier of MTD_OOB_RAW. It used to read both OOB and data to the data buffer, however you would still need to specify the dummy oob buffer. This is only used in one place, but makes it hard to read data+oob without ECC test, thus I removed that behavier, and fixed the user. Now MTD_OOB_RAW behaves like MTD_OOB_PLACE, but doesn't do ECC validation Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com> --- drivers/mtd/nand/nand_base.c | 5 ----- drivers/mtd/nand/nand_bbt.c | 26 ++++++++++++++++++++++---- include/linux/mtd/mtd.h | 4 +--- 3 files changed, 23 insertions(+), 12 deletions(-)