| Message ID | 1363352378-2825-1-git-send-email-pekon@ti.com |
|---|---|
| State | New, archived |
| Headers | show |
On which controller was it tested ? The problem is that lot's of controller driver with hw ecc hack the ecc interface. For example the TI omap driver don't use the data pointer of ecc.calculate but use the data send on the nand interface. Matthieu Gupta, Pekon a écrit : > From: "Gupta, Pekon" <pekon@ti.com> > > This patch adds support for subpage (partial-page) writes when using > hardware based ECC schemes. > Advantages: > (1) reduces storage overhead when using file-systems like UBIFS, which > store LEB header at page-size granularity. > (2) allows independent subpage writes, thereby increasing NAND storage > efficiency for non-page aligned data. > > Signed-off-by: Gupta, Pekon <pekon@ti.com> > --- > drivers/mtd/nand/nand_base.c | 99 ++++++++++++++++++++++++++++++++++++++------ > include/linux/mtd/nand.h | 8 +++- > 2 files changed, 92 insertions(+), 15 deletions(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index 4321415..5138b6b 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -1127,7 +1127,7 @@ static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip, > } > > /** > - * nand_read_subpage - [REPLACEABLE] software ECC based sub-page read function > + * nand_read_subpage - [REPLACEABLE] ECC based sub-page read function > * @mtd: mtd info structure > * @chip: nand chip info structure > * @data_offs: offset of requested data within the page > @@ -1979,6 +1979,67 @@ static int nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip, > return 0; > } > > + > +/** > + * nand_write_subpage_hwecc - [REPLACABLE] hardware ECC based subpage write > + * @mtd: mtd info structure > + * @chip: nand chip info structure > + * @column: column address of subpage within the page > + * @data_len: data length > + * @oob_required: must write chip->oob_poi to OOB > + */ > +static int nand_write_subpage_hwecc(struct mtd_info *mtd, > + struct nand_chip *chip, uint32_t offset, > + uint32_t data_len, const uint8_t *data_buf, > + int oob_required) > +{ > + uint8_t *oob_buf = chip->oob_poi; > + uint8_t *ecc_calc = chip->buffers->ecccalc; > + int ecc_size = chip->ecc.size; > + int ecc_bytes = chip->ecc.bytes; > + int ecc_steps = chip->ecc.steps; > + uint32_t *eccpos = chip->ecc.layout->eccpos; > + uint32_t start_step = offset / ecc_size; > + uint32_t end_step = (offset + data_len - 1) / ecc_size; > + int oob_bytes = mtd->oobsize / ecc_steps; > + int step, i; > + > + for (step = 0; step < ecc_steps; step++) { > + /* configure controller for WRITE access */ > + chip->ecc.hwctl(mtd, NAND_ECC_WRITE); > + > + /* write data (untouched subpages already masked by 0xFF) */ > + chip->write_buf(mtd, data_buf, ecc_size); > + > + /* mask ECC of un-touched subpages by padding 0xFF */ > + if ((step < start_step) || (step > end_step)) > + memset(ecc_calc, 0xff, ecc_bytes); > + else > + chip->ecc.calculate(mtd, data_buf, ecc_calc); > + > + /* mask OOB of un-touched subpages by padding 0xFF */ > + /* if oob_required, preserve OOB metadata of written subpage */ > + if (!oob_required || (step < start_step) || (step > end_step)) > + memset(oob_buf, 0xff, oob_bytes); > + > + data_buf += ecc_size; > + ecc_calc += ecc_bytes; > + oob_buf += oob_bytes; > + } > + > + /* copy calculated ECC for whole page to chip->buffer->oob */ > + /* this include masked-value(0xFF) for unwritten subpages */ > + ecc_calc = chip->buffers->ecccalc; > + for (i = 0; i < chip->ecc.total; i++) > + chip->oob_poi[eccpos[i]] = ecc_calc[i]; > + > + /* write OOB buffer to NAND device */ > + chip->write_buf(mtd, chip->oob_poi, mtd->oobsize); > + > + return 0; > +} > + > + > /** > * nand_write_page_syndrome - [REPLACEABLE] hardware ECC syndrome based page write > * @mtd: mtd info structure > @@ -2031,6 +2092,8 @@ static int nand_write_page_syndrome(struct mtd_info *mtd, > * nand_write_page - [REPLACEABLE] write one page > * @mtd: MTD device structure > * @chip: NAND chip descriptor > + * @offset: address offset within the page > + * @data_len: length of actual data to be written > * @buf: the data to write > * @oob_required: must write chip->oob_poi to OOB > * @page: page number to write > @@ -2038,15 +2101,25 @@ static int nand_write_page_syndrome(struct mtd_info *mtd, > * @raw: use _raw version of write_page > */ > static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip, > - const uint8_t *buf, int oob_required, int page, > - int cached, int raw) > + uint32_t offset, int data_len, const uint8_t *buf, > + int oob_required, int page, int cached, int raw) > { > - int status; > + int status, subpage; > + > + if (!(chip->options & NAND_NO_SUBPAGE_WRITE) && > + chip->ecc.write_subpage) > + subpage = offset || (data_len < mtd->writesize); > + else > + subpage = 0; > > chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page); > > if (unlikely(raw)) > - status = chip->ecc.write_page_raw(mtd, chip, buf, oob_required); > + status = chip->ecc.write_page_raw(mtd, chip, buf, > + oob_required); > + else if (subpage) > + status = chip->ecc.write_subpage(mtd, chip, offset, data_len, > + buf, oob_required); > else > status = chip->ecc.write_page(mtd, chip, buf, oob_required); > > @@ -2160,7 +2233,7 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to, > > uint8_t *oob = ops->oobbuf; > uint8_t *buf = ops->datbuf; > - int ret, subpage; > + int ret; > int oob_required = oob ? 1 : 0; > > ops->retlen = 0; > @@ -2175,10 +2248,6 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to, > } > > column = to & (mtd->writesize - 1); > - subpage = column || (writelen & (mtd->writesize - 1)); > - > - if (subpage && oob) > - return -EINVAL; > > chipnr = (int)(to >> chip->chip_shift); > chip->select_chip(mtd, chipnr); > @@ -2227,9 +2296,9 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to, > /* We still need to erase leftover OOB data */ > memset(chip->oob_poi, 0xff, mtd->oobsize); > } > - > - ret = chip->write_page(mtd, chip, wbuf, oob_required, page, > - cached, (ops->mode == MTD_OPS_RAW)); > + ret = chip->write_page(mtd, chip, column, bytes, wbuf, > + oob_required, page, cached, > + (ops->mode == MTD_OPS_RAW)); > if (ret) > break; > > @@ -3458,6 +3527,10 @@ int nand_scan_tail(struct mtd_info *mtd) > chip->ecc.read_oob = nand_read_oob_std; > if (!chip->ecc.write_oob) > chip->ecc.write_oob = nand_write_oob_std; > + if (!chip->ecc.read_subpage) > + chip->ecc.read_subpage = nand_read_subpage; > + if (!chip->ecc.write_subpage) > + chip->ecc.write_subpage = nand_write_subpage_hwecc; > > case NAND_ECC_HW_SYNDROME: > if ((!chip->ecc.calculate || !chip->ecc.correct || > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index 7ccb3c5..7d81403f 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -354,6 +354,7 @@ struct nand_hw_control { > * any single ECC step, 0 if bitflips uncorrectable, -EIO hw error > * @read_subpage: function to read parts of the page covered by ECC; > * returns same as read_page() > + * @write_subpage: function to write parts of the page covered by ECC. > * @write_page: function to write a page according to the ECC generator > * requirements. > * @write_oob_raw: function to write chip OOB data without ECC > @@ -385,6 +386,9 @@ struct nand_ecc_ctrl { > uint8_t *buf, int oob_required, int page); > int (*read_subpage)(struct mtd_info *mtd, struct nand_chip *chip, > uint32_t offs, uint32_t len, uint8_t *buf); > + int (*write_subpage)(struct mtd_info *mtd, struct nand_chip *chip, > + uint32_t offset, uint32_t data_len, > + const uint8_t *data_buf, int oob_required); > int (*write_page)(struct mtd_info *mtd, struct nand_chip *chip, > const uint8_t *buf, int oob_required); > int (*write_oob_raw)(struct mtd_info *mtd, struct nand_chip *chip, > @@ -520,8 +524,8 @@ struct nand_chip { > int (*errstat)(struct mtd_info *mtd, struct nand_chip *this, int state, > int status, int page); > int (*write_page)(struct mtd_info *mtd, struct nand_chip *chip, > - const uint8_t *buf, int oob_required, int page, > - int cached, int raw); > + uint32_t offset, int data_len, const uint8_t *buf, > + int oob_required, int page, int cached, int raw); > int (*onfi_set_features)(struct mtd_info *mtd, struct nand_chip *chip, > int feature_addr, uint8_t *subfeature_para); > int (*onfi_get_features)(struct mtd_info *mtd, struct nand_chip *chip,
Hi, > On which controller was it tested ? [Pekon]: I tested it on TI's omap controller itself :-) However I ran following, to confirm.. (1) mtd_subpagetest: part of MTD tests (2) ubiattach & ubiformat of ubi image built with Volume Header at 512 offset that is using -O 512. (3) nand raw read / write with non-page aligned data. Do you have any other tests in mind ? > The problem is that lot's of controller driver with hw ecc hack the ecc interface. > For example the TI omap driver don't use the data pointer of ecc.calculate but > use the data send on the nand interface. [Pekon]: Yes i agree. But what I see in TI's hack also that ECC is calculated for each subpage separately. Its just that instead of using data in chip->buffers->databuf TI's driver uses data which is present in controller's internal buffers, which should be the way if we are depending on Hardware (controller) to do ECC. Is this something different from other Hardware based ECC implementations? [Pekon]: Also, its not just doing sub-page writes, rather its actually masking other sub-pages with 0xFF. And also their corresponding ECC . This is done, because even though some NANDs support sub-page accesses, but the actual PAGE-PROGRAM NAND command which actually flashes the data present in device buffers to array cells, works at page boundaries. I'm following similar to what is given at http://www.linux-mtd.infradead.org/doc/ubi.html#L_subpage > Matthieu with regards, pekon
Gupta, Pekon a écrit : > Hi, > >> On which controller was it tested ? > > [Pekon]: I tested it on TI's omap controller itself :-) > However I ran following, to confirm.. > (1) mtd_subpagetest: part of MTD tests > (2) ubiattach & ubiformat of ubi image built with Volume Header at 512 offset > that is using -O 512. > (3) nand raw read / write with non-page aligned data. > > Do you have any other tests in mind ? > > >> The problem is that lot's of controller driver with hw ecc hack the ecc interface. > >> For example the TI omap driver don't use the data pointer of ecc.calculate but >> use the data send on the nand interface. > > [Pekon]: Yes i agree. But what I see in TI's hack also that ECC is calculated for > each subpage separately. Its just that instead of using data in > chip->buffers->databuf > TI's driver uses data which is present in controller's internal buffers, which > should be the way if we are depending on Hardware (controller) to do ECC. > Is this something different from other Hardware based ECC implementations? Yes for example some controller have ecc that for a 0xff page is not 0xff. And this ecc is generated on the fly : you can't xor it before writing it to the flash. I know that omap driver with ELM error correction did not generated 0xff ecc for a black page. I wonder if it works why your patch. Matthieu
Hi Matthieu, I have tried to elaborate on my patch below. Hopefully this explains intend & implementation. The patch adds interface for chip->ecc.write_subpage to generic driver, so that any driver can extend or reuse it for supporting subpage writes. Current generic driver only supports subpage reads. > >> The problem is that lot's of controller driver with hw ecc hack the ecc > interface. > > > >> For example the TI omap driver don't use the data pointer of > ecc.calculate but > >> use the data send on the nand interface. > > > > [Pekon]: Yes i agree. But what I see in TI's hack also that ECC is > calculated for > > each subpage separately. Its just that instead of using data in > > chip->buffers->databuf > > TI's driver uses data which is present in controller's internal buffers, > which > > should be the way if we are depending on Hardware (controller) to do > ECC. > > Is this something different from other Hardware based ECC > implementations? > Yes for example some controller have ecc that for a 0xff page is not 0xff. > And this ecc is generated on the fly : you can't xor it before writing it to > the > flash. > [Pekon]: TI GPMC controller also behaves in same way. It also generates non 0xff ECC for a blank page, therefore in function 'nand_write_subpage_hwecc()', I'm explicitly replacing h/w controller's ECC with 0xff for blank | untouched subpages. So that existing data is not mangled. > I know that omap driver with ELM error correction did not generated 0xff > ecc for > a black page. I wonder if it works why your patch. > [Pekon]: ELM is used on read-path (ECC-correction), while my patch below is for write-path, writing at granularity of sub-pages.Also, the patch is in generic nand-driver (nand_base.c). not limited to usage with TI OMAP. I think there is some confusion, so I'll try to elaborate the patch: +static int nand_write_subpage_hwecc(struct mtd_info *mtd, + struct nand_chip *chip, uint32_t offset, + uint32_t data_len, const uint8_t *data_buf, + int oob_required) +{ + uint8_t *oob_buf = chip->oob_poi; + uint8_t *ecc_calc = chip->buffers->ecccalc; + int ecc_size = chip->ecc.size; + int ecc_bytes = chip->ecc.bytes; + int ecc_steps = chip->ecc.steps; + uint32_t *eccpos = chip->ecc.layout->eccpos; + uint32_t start_step = offset / ecc_size; + uint32_t end_step = (offset + data_len - 1) / ecc_size; + int oob_bytes = mtd->oobsize / ecc_steps; + int step, i; + + for (step = 0; step < ecc_steps; step++) { + /* configure controller for WRITE access */ + chip->ecc.hwctl(mtd, NAND_ECC_WRITE); + + /* write data (untouched subpages already masked by 0xFF) */ + chip->write_buf(mtd, data_buf, ecc_size); [Pekon]: writing main area of flash sub-page, which is already padded with 0xff to make it of page-size, in nand_do_write_ops(): // partial page write if (unlikely(column || writelen < (mtd->writesize - 1))) { .... memset(chip->buffers->databuf, 0xff, mtd->writesize); memcpy(&chip->buffers->databuf[column], buf, bytes); ... } + + /* mask ECC of un-touched subpages by padding 0xFF */ + if ((step < start_step) || (step > end_step)) + memset(ecc_calc, 0xff, ecc_bytes); + else + chip->ecc.calculate(mtd, data_buf, ecc_calc); + [Pekon]: As mentioned earlier, many H/W controllers generate non-0xff ECC for blank pages, so above code will explicitly pad ECC with 0xff for un-touched | blank sub-pages, so that their ECC is not mangled. + /* mask OOB of un-touched subpages by padding 0xFF */ + /* if oob_required, preserve OOB metadata of written subpage */ + if (!oob_required || (step < start_step) || (step > end_step)) + memset(oob_buf, 0xff, oob_bytes); + [Pekon]: MTD layer may also provide some, file-system metadata to be written in OOB area corresponding to written sub-page. Above code pads 0xff to OOB buffer corresponding to un-touched | blank subpage to preserve existing OOB data from being overwritten. Usage: -> UBIFS : does not have any FileSystem meta-data stored OOB. -> JFFS2: has clean-markers written to OOB. -> YFFS2: has statistical metadata written to OOB. -> LOGFS: (don't know) [Pekon]: However, it should be noted here that both FS meta-data and ECC can be scattered at any byte-positions, depending on OOB layout. So, alternative is to completely block any File-system metadata while using sub-page format. + data_buf += ecc_size; + ecc_calc += ecc_bytes; + oob_buf += oob_bytes; + } + + /* copy calculated ECC for whole page to chip->buffer->oob */ + /* this include masked-value(0xFF) for unwritten subpages */ + ecc_calc = chip->buffers->ecccalc; + for (i = 0; i < chip->ecc.total; i++) + chip->oob_poi[eccpos[i]] = ecc_calc[i]; + [Pekon]: Here ECC is copied to OOB buffer for flashing. + /* write OOB buffer to NAND device */ + chip->write_buf(mtd, chip->oob_poi, mtd->oobsize); + [Pekon]: Actual ECC is flashed to OOB area here, Thus even if the controller generates non-0xff ECC for blank-pages, we above code handles it. + return 0; +} static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip, - const uint8_t *buf, int oob_required, int page, - int cached, int raw) + uint32_t offset, int data_len, const uint8_t *buf, + int oob_required, int page, int cached, int raw) { - int status; + int status, subpage; + + if (!(chip->options & NAND_NO_SUBPAGE_WRITE) && + chip->ecc.write_subpage) + subpage = offset || (data_len < mtd->writesize); + else + [Pekon]: modifying existing 'nand_write_page' to accommodate 'column' address for adding nand_write_subpage support. @@ -3458,6 +3527,10 @@ int nand_scan_tail(struct mtd_info *mtd) chip->ecc.read_oob = nand_read_oob_std; if (!chip->ecc.write_oob) chip->ecc.write_oob = nand_write_oob_std; + if (!chip->ecc.read_subpage) + chip->ecc.read_subpage = nand_read_subpage; + if (!chip->ecc.write_subpage) + chip->ecc.write_subpage = nand_write_subpage_hwecc; [Pekon]: populating functions for subpage support. -> reusing existing nand_read_subpage. -> adding new nand_write_subpage. [Pekon]: The problem is that I only have access only to TI OMAP boards, so I'm unable to test this patch across different manufacturers. So, in case anyone can help me with testing on non TI board, it would be great. with regards, pekon > > Matthieu
Hi, Can someone please test following patch on non-TI devices ? I have tested this on Micron NAND using TI-OMAP controller: MT29F2G08AADWP: 2K/64 (page=2048, subpage=512) MT29F4G08ABxx: 4K/224 (page=4096, subpage=1024) (1) using mtd/tests/mtd_subpage.ko (2) using UBIFS image of different sizes (attaching a generic script to create UBIFS image.) (3) using loosely written nand-raw test, which writes in multiples of subpage lengths But due to un-availability I couldn't test it on other boards. So, any ACKs | NAKs will help to improve this further. with regards, pekon > From: "Gupta, Pekon" <pekon@ti.com> > > This patch adds support for subpage (partial-page) writes when using > hardware based ECC schemes. > Advantages: > (1) reduces storage overhead when using file-systems like UBIFS, which > store LEB header at page-size granularity. > (2) allows independent subpage writes, thereby increasing NAND > storage > efficiency for non-page aligned data. > > Signed-off-by: Gupta, Pekon <pekon@ti.com> > --- > drivers/mtd/nand/nand_base.c | 99 > ++++++++++++++++++++++++++++++++++++++------ > include/linux/mtd/nand.h | 8 +++- > 2 files changed, 92 insertions(+), 15 deletions(-) > > diff --git a/drivers/mtd/nand/nand_base.c > b/drivers/mtd/nand/nand_base.c > index 4321415..5138b6b 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -1127,7 +1127,7 @@ static int nand_read_page_swecc(struct > mtd_info *mtd, struct nand_chip *chip, > } > > /** > - * nand_read_subpage - [REPLACEABLE] software ECC based sub-page > read function > + * nand_read_subpage - [REPLACEABLE] ECC based sub-page read > function > * @mtd: mtd info structure > * @chip: nand chip info structure > * @data_offs: offset of requested data within the page > @@ -1979,6 +1979,67 @@ static int nand_write_page_hwecc(struct > mtd_info *mtd, struct nand_chip *chip, > return 0; > } > > + > +/** > + * nand_write_subpage_hwecc - [REPLACABLE] hardware ECC based > subpage write > + * @mtd: mtd info structure > + * @chip: nand chip info structure > + * @column: column address of subpage within the page > + * @data_len: data length > + * @oob_required: must write chip->oob_poi to OOB > + */ > +static int nand_write_subpage_hwecc(struct mtd_info *mtd, > + struct nand_chip *chip, uint32_t offset, > + uint32_t data_len, const uint8_t > *data_buf, > + int oob_required) > +{ > + uint8_t *oob_buf = chip->oob_poi; > + uint8_t *ecc_calc = chip->buffers->ecccalc; > + int ecc_size = chip->ecc.size; > + int ecc_bytes = chip->ecc.bytes; > + int ecc_steps = chip->ecc.steps; > + uint32_t *eccpos = chip->ecc.layout->eccpos; > + uint32_t start_step = offset / ecc_size; > + uint32_t end_step = (offset + data_len - 1) / ecc_size; > + int oob_bytes = mtd->oobsize / ecc_steps; > + int step, i; > + > + for (step = 0; step < ecc_steps; step++) { > + /* configure controller for WRITE access */ > + chip->ecc.hwctl(mtd, NAND_ECC_WRITE); > + > + /* write data (untouched subpages already masked by > 0xFF) */ > + chip->write_buf(mtd, data_buf, ecc_size); > + > + /* mask ECC of un-touched subpages by padding 0xFF */ > + if ((step < start_step) || (step > end_step)) > + memset(ecc_calc, 0xff, ecc_bytes); > + else > + chip->ecc.calculate(mtd, data_buf, ecc_calc); > + > + /* mask OOB of un-touched subpages by padding 0xFF > */ > + /* if oob_required, preserve OOB metadata of written > subpage */ > + if (!oob_required || (step < start_step) || (step > > end_step)) > + memset(oob_buf, 0xff, oob_bytes); > + > + data_buf += ecc_size; > + ecc_calc += ecc_bytes; > + oob_buf += oob_bytes; > + } > + > + /* copy calculated ECC for whole page to chip->buffer->oob */ > + /* this include masked-value(0xFF) for unwritten subpages */ > + ecc_calc = chip->buffers->ecccalc; > + for (i = 0; i < chip->ecc.total; i++) > + chip->oob_poi[eccpos[i]] = ecc_calc[i]; > + > + /* write OOB buffer to NAND device */ > + chip->write_buf(mtd, chip->oob_poi, mtd->oobsize); > + > + return 0; > +} > + > + > /** > * nand_write_page_syndrome - [REPLACEABLE] hardware ECC > syndrome based page write > * @mtd: mtd info structure > @@ -2031,6 +2092,8 @@ static int nand_write_page_syndrome(struct > mtd_info *mtd, > * nand_write_page - [REPLACEABLE] write one page > * @mtd: MTD device structure > * @chip: NAND chip descriptor > + * @offset: address offset within the page > + * @data_len: length of actual data to be written > * @buf: the data to write > * @oob_required: must write chip->oob_poi to OOB > * @page: page number to write > @@ -2038,15 +2101,25 @@ static int nand_write_page_syndrome(struct > mtd_info *mtd, > * @raw: use _raw version of write_page > */ > static int nand_write_page(struct mtd_info *mtd, struct nand_chip > *chip, > - const uint8_t *buf, int oob_required, int page, > - int cached, int raw) > + uint32_t offset, int data_len, const uint8_t *buf, > + int oob_required, int page, int cached, int raw) > { > - int status; > + int status, subpage; > + > + if (!(chip->options & NAND_NO_SUBPAGE_WRITE) && > + chip->ecc.write_subpage) > + subpage = offset || (data_len < mtd->writesize); > + else > + subpage = 0; > > chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page); > > if (unlikely(raw)) > - status = chip->ecc.write_page_raw(mtd, chip, buf, > oob_required); > + status = chip->ecc.write_page_raw(mtd, chip, buf, > + oob_required); > + else if (subpage) > + status = chip->ecc.write_subpage(mtd, chip, offset, > data_len, > + buf, > oob_required); > else > status = chip->ecc.write_page(mtd, chip, buf, > oob_required); > > @@ -2160,7 +2233,7 @@ static int nand_do_write_ops(struct mtd_info > *mtd, loff_t to, > > uint8_t *oob = ops->oobbuf; > uint8_t *buf = ops->datbuf; > - int ret, subpage; > + int ret; > int oob_required = oob ? 1 : 0; > > ops->retlen = 0; > @@ -2175,10 +2248,6 @@ static int nand_do_write_ops(struct mtd_info > *mtd, loff_t to, > } > > column = to & (mtd->writesize - 1); > - subpage = column || (writelen & (mtd->writesize - 1)); > - > - if (subpage && oob) > - return -EINVAL; > > chipnr = (int)(to >> chip->chip_shift); > chip->select_chip(mtd, chipnr); > @@ -2227,9 +2296,9 @@ static int nand_do_write_ops(struct mtd_info > *mtd, loff_t to, > /* We still need to erase leftover OOB data */ > memset(chip->oob_poi, 0xff, mtd->oobsize); > } > - > - ret = chip->write_page(mtd, chip, wbuf, oob_required, > page, > - cached, (ops->mode == > MTD_OPS_RAW)); > + ret = chip->write_page(mtd, chip, column, bytes, wbuf, > + oob_required, page, cached, > + (ops->mode == > MTD_OPS_RAW)); > if (ret) > break; > > @@ -3458,6 +3527,10 @@ int nand_scan_tail(struct mtd_info *mtd) > chip->ecc.read_oob = nand_read_oob_std; > if (!chip->ecc.write_oob) > chip->ecc.write_oob = nand_write_oob_std; > + if (!chip->ecc.read_subpage) > + chip->ecc.read_subpage = nand_read_subpage; > + if (!chip->ecc.write_subpage) > + chip->ecc.write_subpage = > nand_write_subpage_hwecc; > > case NAND_ECC_HW_SYNDROME: > if ((!chip->ecc.calculate || !chip->ecc.correct || > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index 7ccb3c5..7d81403f 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -354,6 +354,7 @@ struct nand_hw_control { > * any single ECC step, 0 if bitflips uncorrectable, -EIO hw > error > * @read_subpage: function to read parts of the page covered by > ECC; > * returns same as read_page() > + * @write_subpage: function to write parts of the page covered by > ECC. > * @write_page: function to write a page according to the ECC > generator > * requirements. > * @write_oob_raw: function to write chip OOB data without ECC > @@ -385,6 +386,9 @@ struct nand_ecc_ctrl { > uint8_t *buf, int oob_required, int page); > int (*read_subpage)(struct mtd_info *mtd, struct nand_chip > *chip, > uint32_t offs, uint32_t len, uint8_t *buf); > + int (*write_subpage)(struct mtd_info *mtd, struct nand_chip > *chip, > + uint32_t offset, uint32_t data_len, > + const uint8_t *data_buf, int oob_required); > int (*write_page)(struct mtd_info *mtd, struct nand_chip *chip, > const uint8_t *buf, int oob_required); > int (*write_oob_raw)(struct mtd_info *mtd, struct nand_chip > *chip, > @@ -520,8 +524,8 @@ struct nand_chip { > int (*errstat)(struct mtd_info *mtd, struct nand_chip *this, int > state, > int status, int page); > int (*write_page)(struct mtd_info *mtd, struct nand_chip *chip, > - const uint8_t *buf, int oob_required, int page, > - int cached, int raw); > + uint32_t offset, int data_len, const uint8_t *buf, > + int oob_required, int page, int cached, int raw); > int (*onfi_set_features)(struct mtd_info *mtd, struct nand_chip > *chip, > int feature_addr, uint8_t *subfeature_para); > int (*onfi_get_features)(struct mtd_info *mtd, struct nand_chip > *chip, > -- > 1.8.1
On Fri, 2013-03-15 at 18:29 +0530, Gupta, Pekon wrote: > int (*write_page)(struct mtd_info *mtd, struct nand_chip *chip, > - const uint8_t *buf, int oob_required, int page, > - int cached, int raw); > + uint32_t offset, int data_len, const uint8_t *buf, > + int oob_required, int page, int cached, int raw); You need to update cafe_nand.c accordingly. Aiaiai detected this: --- before_patching.log +++ after_patching.log @@ @@ +drivers/mtd/nand/cafe_nand.c: In function ‘cafe_nand_probe’: +drivers/mtd/nand/cafe_nand.c:787:24: warning: assignment from incompatible pointer type [enabled by default]
> You need to update cafe_nand.c accordingly. Aiaiai detected this: > > Artem Bityutskiy Thanks Aiaiai, Artem, [version-2] + updated cafe_nand and lpc32xx_mlc NAND drivers for change in chip->write_page interface. + verified with linux-3.9-rc5 with regards, pekon
On Fri, 2013-04-05 at 09:07 +0000, Gupta, Pekon wrote: > > You need to update cafe_nand.c accordingly. Aiaiai detected this: > > > > Artem Bityutskiy > > Thanks Aiaiai, Artem, Roland, heads up, I am going to apply this patch to the MTD tree. It touches your driver.
On Fri, 2013-04-05 at 09:07 +0000, Gupta, Pekon wrote: > > You need to update cafe_nand.c accordingly. Aiaiai detected this: > > > > Artem Bityutskiy > > Thanks Aiaiai, Artem, Pushed this one to l2-mtd.git, thanks!
On 04/05/2013 12:43 PM, Artem Bityutskiy wrote: > On Fri, 2013-04-05 at 09:07 +0000, Gupta, Pekon wrote: >> > You need to update cafe_nand.c accordingly. Aiaiai detected this: >>> >>> Artem Bityutskiy >> >> Thanks Aiaiai, Artem, > > Roland, heads up, I am going to apply this patch to the MTD tree. It > touches your driver. Tested-by: Roland Stigge <stigge@antcom.de> Thanks for the note! (Sorry, haven't followed this issue.) Works fine so far. As far as I understand it, there is only an interface change regarding LPC32xx - subpage writing isn't officially supported by this controller hardware (with >=2k pages + ecc applied to 512 byte "subpages" - not the real subpages as reported by the flash chip). So I guess it's ok to ignore the new "uint32_t offset, int data_len" arguments in lpc32xx_write_page. Roland
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 4321415..5138b6b 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -1127,7 +1127,7 @@ static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip, } /** - * nand_read_subpage - [REPLACEABLE] software ECC based sub-page read function + * nand_read_subpage - [REPLACEABLE] ECC based sub-page read function * @mtd: mtd info structure * @chip: nand chip info structure * @data_offs: offset of requested data within the page @@ -1979,6 +1979,67 @@ static int nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip, return 0; } + +/** + * nand_write_subpage_hwecc - [REPLACABLE] hardware ECC based subpage write + * @mtd: mtd info structure + * @chip: nand chip info structure + * @column: column address of subpage within the page + * @data_len: data length + * @oob_required: must write chip->oob_poi to OOB + */ +static int nand_write_subpage_hwecc(struct mtd_info *mtd, + struct nand_chip *chip, uint32_t offset, + uint32_t data_len, const uint8_t *data_buf, + int oob_required) +{ + uint8_t *oob_buf = chip->oob_poi; + uint8_t *ecc_calc = chip->buffers->ecccalc; + int ecc_size = chip->ecc.size; + int ecc_bytes = chip->ecc.bytes; + int ecc_steps = chip->ecc.steps; + uint32_t *eccpos = chip->ecc.layout->eccpos; + uint32_t start_step = offset / ecc_size; + uint32_t end_step = (offset + data_len - 1) / ecc_size; + int oob_bytes = mtd->oobsize / ecc_steps; + int step, i; + + for (step = 0; step < ecc_steps; step++) { + /* configure controller for WRITE access */ + chip->ecc.hwctl(mtd, NAND_ECC_WRITE); + + /* write data (untouched subpages already masked by 0xFF) */ + chip->write_buf(mtd, data_buf, ecc_size); + + /* mask ECC of un-touched subpages by padding 0xFF */ + if ((step < start_step) || (step > end_step)) + memset(ecc_calc, 0xff, ecc_bytes); + else + chip->ecc.calculate(mtd, data_buf, ecc_calc); + + /* mask OOB of un-touched subpages by padding 0xFF */ + /* if oob_required, preserve OOB metadata of written subpage */ + if (!oob_required || (step < start_step) || (step > end_step)) + memset(oob_buf, 0xff, oob_bytes); + + data_buf += ecc_size; + ecc_calc += ecc_bytes; + oob_buf += oob_bytes; + } + + /* copy calculated ECC for whole page to chip->buffer->oob */ + /* this include masked-value(0xFF) for unwritten subpages */ + ecc_calc = chip->buffers->ecccalc; + for (i = 0; i < chip->ecc.total; i++) + chip->oob_poi[eccpos[i]] = ecc_calc[i]; + + /* write OOB buffer to NAND device */ + chip->write_buf(mtd, chip->oob_poi, mtd->oobsize); + + return 0; +} + + /** * nand_write_page_syndrome - [REPLACEABLE] hardware ECC syndrome based page write * @mtd: mtd info structure @@ -2031,6 +2092,8 @@ static int nand_write_page_syndrome(struct mtd_info *mtd, * nand_write_page - [REPLACEABLE] write one page * @mtd: MTD device structure * @chip: NAND chip descriptor + * @offset: address offset within the page + * @data_len: length of actual data to be written * @buf: the data to write * @oob_required: must write chip->oob_poi to OOB * @page: page number to write @@ -2038,15 +2101,25 @@ static int nand_write_page_syndrome(struct mtd_info *mtd, * @raw: use _raw version of write_page */ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip, - const uint8_t *buf, int oob_required, int page, - int cached, int raw) + uint32_t offset, int data_len, const uint8_t *buf, + int oob_required, int page, int cached, int raw) { - int status; + int status, subpage; + + if (!(chip->options & NAND_NO_SUBPAGE_WRITE) && + chip->ecc.write_subpage) + subpage = offset || (data_len < mtd->writesize); + else + subpage = 0; chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page); if (unlikely(raw)) - status = chip->ecc.write_page_raw(mtd, chip, buf, oob_required); + status = chip->ecc.write_page_raw(mtd, chip, buf, + oob_required); + else if (subpage) + status = chip->ecc.write_subpage(mtd, chip, offset, data_len, + buf, oob_required); else status = chip->ecc.write_page(mtd, chip, buf, oob_required); @@ -2160,7 +2233,7 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to, uint8_t *oob = ops->oobbuf; uint8_t *buf = ops->datbuf; - int ret, subpage; + int ret; int oob_required = oob ? 1 : 0; ops->retlen = 0; @@ -2175,10 +2248,6 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to, } column = to & (mtd->writesize - 1); - subpage = column || (writelen & (mtd->writesize - 1)); - - if (subpage && oob) - return -EINVAL; chipnr = (int)(to >> chip->chip_shift); chip->select_chip(mtd, chipnr); @@ -2227,9 +2296,9 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to, /* We still need to erase leftover OOB data */ memset(chip->oob_poi, 0xff, mtd->oobsize); } - - ret = chip->write_page(mtd, chip, wbuf, oob_required, page, - cached, (ops->mode == MTD_OPS_RAW)); + ret = chip->write_page(mtd, chip, column, bytes, wbuf, + oob_required, page, cached, + (ops->mode == MTD_OPS_RAW)); if (ret) break; @@ -3458,6 +3527,10 @@ int nand_scan_tail(struct mtd_info *mtd) chip->ecc.read_oob = nand_read_oob_std; if (!chip->ecc.write_oob) chip->ecc.write_oob = nand_write_oob_std; + if (!chip->ecc.read_subpage) + chip->ecc.read_subpage = nand_read_subpage; + if (!chip->ecc.write_subpage) + chip->ecc.write_subpage = nand_write_subpage_hwecc; case NAND_ECC_HW_SYNDROME: if ((!chip->ecc.calculate || !chip->ecc.correct || diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index 7ccb3c5..7d81403f 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -354,6 +354,7 @@ struct nand_hw_control { * any single ECC step, 0 if bitflips uncorrectable, -EIO hw error * @read_subpage: function to read parts of the page covered by ECC; * returns same as read_page() + * @write_subpage: function to write parts of the page covered by ECC. * @write_page: function to write a page according to the ECC generator * requirements. * @write_oob_raw: function to write chip OOB data without ECC @@ -385,6 +386,9 @@ struct nand_ecc_ctrl { uint8_t *buf, int oob_required, int page); int (*read_subpage)(struct mtd_info *mtd, struct nand_chip *chip, uint32_t offs, uint32_t len, uint8_t *buf); + int (*write_subpage)(struct mtd_info *mtd, struct nand_chip *chip, + uint32_t offset, uint32_t data_len, + const uint8_t *data_buf, int oob_required); int (*write_page)(struct mtd_info *mtd, struct nand_chip *chip, const uint8_t *buf, int oob_required); int (*write_oob_raw)(struct mtd_info *mtd, struct nand_chip *chip, @@ -520,8 +524,8 @@ struct nand_chip { int (*errstat)(struct mtd_info *mtd, struct nand_chip *this, int state, int status, int page); int (*write_page)(struct mtd_info *mtd, struct nand_chip *chip, - const uint8_t *buf, int oob_required, int page, - int cached, int raw); + uint32_t offset, int data_len, const uint8_t *buf, + int oob_required, int page, int cached, int raw); int (*onfi_set_features)(struct mtd_info *mtd, struct nand_chip *chip, int feature_addr, uint8_t *subfeature_para); int (*onfi_get_features)(struct mtd_info *mtd, struct nand_chip *chip,