[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