[PATCH] mtd: nand: subpage write support for hardware based ECC schemes

[PATCH] mtd: nand: subpage write support for hardware based ECC schemes

[Gupta, 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

Re: [PATCH] mtd: nand: subpage write support for hardware based ECC schemes

[Matthieu CASTET]

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,

RE: [PATCH] mtd: nand: subpage write support for hardware based ECC schemes

[Gupta, Pekon]

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

Re: [PATCH] mtd: nand: subpage write support for hardware based ECC schemes

[Matthieu CASTET]

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

RE: [PATCH] mtd: nand: subpage write support for hardware based ECC schemes

[Gupta, Pekon]

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

RE: [PATCH] mtd: nand: subpage write support for hardware based ECC schemes

[Gupta, Pekon]

[-- Attachment #1: Type: text/plain, Size: 9309 bytes --]

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


[-- Attachment #2: create_ubi_image.sh --]
[-- Type: application/octet-stream, Size: 2062 bytes --]

#!/bin/sh
#---------------------------------------------------------------------- 
# Description: generic script to create UBI image for NAND devices
#		with different page sizes.
#
# Author: 	Pekon Gupta (pekon@ti.com)
# Usage:	create_ubi_image.sh <PAGE_SIZE> <SUBPAGE_SIZE> \
#						<PARTITION_SIZE>
# History:
# version 1.1	pekon
#		added calculation of LEB_SIZE while using subpages
#
#---------------------------------------------------------------------- 
#-- remove old files --
rm -v ubi.ubifs  ubi.img ubinize.cfg

#-- user inputs --
PAGE_SIZE=$1
SUBPAGE_SIZE=$2
PARTITION_SIZE=$3

if test -z $1 || test -z $2  || test -z $3
then
echo "Error: incomplete inputs"
echo "Usage: create_ubi_image.sh <PAGE_SIZE> <SUBPAGE_SIZE> <PARTITION_SIZE>"
exit
fi


#-- calculate UBI parameter --
PEB_SIZE=$(( 64 \* $PAGE_SIZE ))

# LEB size =	PEB_SIZE - number_of_pages_used_for_UBI_headers * PAGE_SIZE
#		number_of_pages_used_for_UBI_header = 1 if using sub-pages
#		number_of_pages_used_for_UBI_header = 2 if not using sub-pages
if test $PAGE_SIZE -ne $SUBPAGE_SIZE
then
	LEB_SIZE=$(( $PEB_SIZE - (1 \* $PAGE_SIZE) ))
else
	LEB_SIZE=$(( $PEB_SIZE - (2 \* $PAGE_SIZE) ))
fi

LEB_COUNT=$(( $PARTITION_SIZE / $LEB_SIZE ))

echo "LOG: PAGE_SIZE	=	$PAGE_SIZE"
echo "LOG: SUBPAGE_SIZE	=	$SUBPAGE_SIZE"
echo "LOG: PEB_SIZE	=	$PEB_SIZE"
echo "LOG: LEB_SIZE	=	$LEB_SIZE"
echo "LOG: LEB_COUNT	=	$LEB_COUNT"


#-- generate ubinize.cfg --
echo "[ubifs]"                	>> ubinize.cfg 
echo "mode=ubi"               	>> ubinize.cfg 
echo "image=ubi.ubifs"        	>> ubinize.cfg 
echo "vol_id=0"               	>> ubinize.cfg 
echo "vol_size=$PARTITION_SIZE"	>> ubinize.cfg 
echo "vol_type=dynamic"       	>> ubinize.cfg 
echo "vol_name=test"         	>> ubinize.cfg 
echo "vol_flags='autoresize'" 	>> ubinize.cfg 
echo "vol_alignment=1"        	>> ubinize.cfg 

#-- generate UBI image --
mkfs.ubifs  -o ubi.ubifs  -m $PAGE_SIZE  -e $LEB_SIZE  -c $LEB_COUNT  -r rootfs  
ubinize     -o ubi.img    -m $PAGE_SIZE  -p $PEB_SIZE  -s $SUBPAGE_SIZE  -O $SUBPAGE_SIZE  -v  ubinize.cfg

Re: [PATCH] mtd: nand: subpage write support for hardware based ECC schemes

[Artem Bityutskiy]

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]

-- 
Best Regards,
Artem Bityutskiy

RE: [PATCH] mtd: nand: subpage write support for hardware based ECC schemes

[Gupta, Pekon]

[-- Attachment #1: Type: text/plain, Size: 286 bytes --]

 
> 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

[-- Attachment #2: mtd-nand-subpage-write-support-for-hardware-based-EC.patch --]
[-- Type: application/octet-stream, Size: 9425 bytes --]

From d6d51765d11bb0a80eaca9f4f586f34612e2139c Mon Sep 17 00:00:00 2001
From: "Gupta, Pekon" <pekon@ti.com>
Date: Fri, 15 Mar 2013 17:55:53 +0530
Subject: [PATCH] mtd: nand: subpage write support for hardware based ECC
 schemes

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.
+ updated cafe_nand and lpc32xx_mlc NAND drivers for change in
chip->write_page interface.

Signed-off-by: Gupta, Pekon <pekon@ti.com>
---
 drivers/mtd/nand/cafe_nand.c   |  4 +-
 drivers/mtd/nand/lpc32xx_mlc.c |  4 +-
 drivers/mtd/nand/nand_base.c   | 99 ++++++++++++++++++++++++++++++++++++------
 include/linux/mtd/nand.h       |  8 +++-
 4 files changed, 96 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c
index 010d612..0d80b2e 100644
--- a/drivers/mtd/nand/cafe_nand.c
+++ b/drivers/mtd/nand/cafe_nand.c
@@ -536,8 +536,8 @@ static int cafe_nand_write_page_lowlevel(struct mtd_info *mtd,
 }
 
 static int cafe_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;
 
diff --git a/drivers/mtd/nand/lpc32xx_mlc.c b/drivers/mtd/nand/lpc32xx_mlc.c
index 0ca22ae..a94facb 100644
--- a/drivers/mtd/nand/lpc32xx_mlc.c
+++ b/drivers/mtd/nand/lpc32xx_mlc.c
@@ -540,8 +540,8 @@ static int lpc32xx_write_page_lowlevel(struct mtd_info *mtd,
 }
 
 static int lpc32xx_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 res;
 
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 42c6392..8433b71 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
@@ -1995,6 +1995,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
@@ -2047,6 +2108,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
@@ -2054,15 +2117,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);
 
@@ -2176,7 +2249,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;
@@ -2191,10 +2264,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);
@@ -2243,9 +2312,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;
 
@@ -3474,6 +3543,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 ef52d9c..fdcc3a0 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -361,6 +361,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
@@ -392,6 +393,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,
@@ -527,8 +531,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

Re: [PATCH] mtd: nand: subpage write support for hardware based ECC schemes

[Artem Bityutskiy]

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.

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH] mtd: nand: subpage write support for hardware based ECC schemes

[Artem Bityutskiy]

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!

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH] mtd: nand: subpage write support for hardware based ECC schemes

[Roland Stigge]

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