diff mbox

[1/2] mtd: nand: add OOB argument to NAND {read, write}_page interfaces

Message ID 1334615755-15418-2-git-send-email-computersforpeace@gmail.com
State New, archived
Headers show

Commit Message

Brian Norris April 16, 2012, 10:35 p.m. UTC
New NAND controllers can perform read/write via HW engines which don't expose
OOB data in their DMA mode. To reflect this, we should rework the nand_chip /
nand_ecc_ctrl interfaces that assume that drivers will always read/write OOB
data in the nand_chip.oob_poi buffer. A better interface would pass the
appropriate buffer explicitly when OOB data is requested and otherwise pass a
NULL pointer, meaning that no OOB data is needed.

This patch adds the 'oob' parameter to each relevant {read,write}_page
interface; all 'oob' parameters are left unused and NULL for now. The next
patch will set the parameter properly. Perhaps it should be utilized eventually
on some of these drivers, but this is a slow process of hacking each driver
function that uses these interfaces.

I renamed many 'oob' buffers to 'oobbuf' so that we can use 'oob' as an
argument to these interface functions. I think the consistency can help in the
long run.

Note: I couldn't compile-test all of these easily, as some had ARCH
dependencies.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/atmel_nand.c          |    7 ++-
 drivers/mtd/nand/bcm_umi_bch.c         |   10 ++-
 drivers/mtd/nand/bcm_umi_nand.c        |    2 +-
 drivers/mtd/nand/bf5xx_nand.c          |    4 +-
 drivers/mtd/nand/cafe_nand.c           |   26 ++++---
 drivers/mtd/nand/denali.c              |    8 +-
 drivers/mtd/nand/docg4.c               |   12 ++--
 drivers/mtd/nand/fsl_elbc_nand.c       |    7 +-
 drivers/mtd/nand/fsl_ifc_nand.c        |    4 +-
 drivers/mtd/nand/fsmc_nand.c           |    3 +-
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    5 +-
 drivers/mtd/nand/nand_base.c           |  123 ++++++++++++++++++--------------
 drivers/mtd/nand/pxa3xx_nand.c         |    4 +-
 drivers/mtd/nand/sh_flctl.c            |    4 +-
 include/linux/mtd/nand.h               |   11 ++--
 15 files changed, 127 insertions(+), 103 deletions(-)

Comments

Matthieu CASTET April 17, 2012, 7:50 a.m. UTC | #1
Brian Norris a écrit :
> New NAND controllers can perform read/write via HW engines which don't expose
> OOB data in their DMA mode. To reflect this, we should rework the nand_chip /
> nand_ecc_ctrl interfaces that assume that drivers will always read/write OOB
> data in the nand_chip.oob_poi buffer. A better interface would pass the
> appropriate buffer explicitly when OOB data is requested and otherwise pass a
> NULL pointer, meaning that no OOB data is needed.

If I understand correctly you propose that these driver will fetch oob (via pio
mode) only when needed ?
Do you have an example of such controller ?

Matthieu
Shmulik Ladkani April 17, 2012, 2:29 p.m. UTC | #2
Hi Brian,

I have no inputs regarding the concept, others probably know better.

Only verified execution in nand_base.c and nand.h, see below.

On Mon, 16 Apr 2012 15:35:54 -0700 Brian Norris <computersforpeace@gmail.com> wrote:
>  static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
> -				   uint8_t *buf, int page)
> +				   uint8_t *buf, uint8_t *oob, int page)
>  {
>  	int i, eccsize = chip->ecc.size;
>  	int eccbytes = chip->ecc.bytes;
>  	int eccsteps = chip->ecc.steps;
>  	uint8_t *p = buf;
> -	uint8_t *oob = chip->oob_poi;
> +	uint8_t *oobbuf = chip->oob_poi;
>  
>  	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
>  		int stat;
> @@ -1371,23 +1376,23 @@ static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
>  		chip->read_buf(mtd, p, eccsize);
>  
>  		if (chip->ecc.prepad) {
> -			chip->read_buf(mtd, oob, chip->ecc.prepad);
> -			oob += chip->ecc.prepad;
> +			chip->read_buf(mtd, oobbuf, chip->ecc.prepad);
> +			oobbuf += chip->ecc.prepad;
>  		}
>  
>  		chip->ecc.hwctl(mtd, NAND_ECC_READSYN);
> -		chip->read_buf(mtd, oob, eccbytes);
> -		stat = chip->ecc.correct(mtd, p, oob, NULL);
> +		chip->read_buf(mtd, oobbuf, eccbytes);
> +		stat = chip->ecc.correct(mtd, p, oobbuf, NULL);
>  
>  		if (stat < 0)
>  			mtd->ecc_stats.failed++;
>  		else
>  			mtd->ecc_stats.corrected += stat;
>  
> -		oob += eccbytes;
> +		oobbuf += eccbytes;
>  
>  		if (chip->ecc.postpad) {
> -			chip->read_buf(mtd, oob, chip->ecc.postpad);
> +			chip->read_buf(mtd, oobbuf, chip->ecc.postpad);
>  			oob += chip->ecc.postpad;

Missed convertion of 'oob' in the last line above.
s/oob/oobbuf/

Same problem in the following lines of 'nand_read_page_syndrome'.
Should be:

@@ -1393,9 +1393,9 @@ static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
        }
 
        /* Calculate remaining oob bytes */
-       i = mtd->oobsize - (oob - chip->oob_poi);
+       i = mtd->oobsize - (oobbuf - chip->oob_poi);
        if (i)
-               chip->read_buf(mtd, oob, i);
+               chip->read_buf(mtd, oobbuf, i);
 
        return 0;
 }

> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 1482340..be9ee1f 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -363,15 +363,15 @@ struct nand_ecc_ctrl {
>  	int (*correct)(struct mtd_info *mtd, uint8_t *dat, uint8_t *read_ecc,
>  			uint8_t *calc_ecc);
>  	int (*read_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
> -			uint8_t *buf, int page);
> +			uint8_t *buf, uint8_t *oob, int page);
>  	void (*write_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
> -			const uint8_t *buf);
> +			const uint8_t *buf, const uint8_t *oob);
>  	int (*read_page)(struct mtd_info *mtd, struct nand_chip *chip,
> -			uint8_t *buf, int page);
> +			uint8_t *buf, uint8_t *oob, int page);
>  	int (*read_subpage)(struct mtd_info *mtd, struct nand_chip *chip,
>  			uint32_t offs, uint32_t len, uint8_t *buf);

No need to pass 'oob' to 'read_subpage'?

Regards,
Shmulik
Brian Norris April 18, 2012, 3:44 a.m. UTC | #3
Hi Matthieu,

On Tue, Apr 17, 2012 at 12:50 AM, Matthieu CASTET
<matthieu.castet@parrot.com> wrote:
> Brian Norris a écrit :
>> New NAND controllers can perform read/write via HW engines which don't expose
>> OOB data in their DMA mode. To reflect this, we should rework the nand_chip /
>> nand_ecc_ctrl interfaces that assume that drivers will always read/write OOB
>> data in the nand_chip.oob_poi buffer. A better interface would pass the
>> appropriate buffer explicitly when OOB data is requested and otherwise pass a
>> NULL pointer, meaning that no OOB data is needed.
>
> If I understand correctly you propose that these driver will fetch oob (via pio
> mode) only when needed ?

Sure, that's the idea.

> Do you have an example of such controller ?

Yes. I'm working with a new ASIC which has a DMA mode as well as a PIO
mode. For DMA transactions (the preferred interface), I fill out a
64-byte descriptor (format specified below) then point the controller
to that DRAM address via a register. The "command" field only supports
page program, page read, and block erase, though.

So for instance, if I want to perform a page program, I can only
specify a single buffer (via dram_addr/dram_addr_ext) for the in-band
data; the OOB data is automatically filled with 0xFF + HW ECC
(calculated on the fly, internally, by HW controller). And on page
read, I only receive the in-band data in my DRAM buffer.

Now, in future revisions of this ASIC, it may be possible to access
OOB via DMA as well, but even if this happens, it doesn't make a lot
of sense on this hardware to *always* pull OOB data. As mentioned
previously, most normal applications (i.e., UBI(FS)) don't need to
access this OOB data at all, so it seems silly to go to extra work to
have the DMA controller return it to the MTD/NAND layers. I'm not
familiar with the OOB/ECC schemes on enough other hardware to
determine whether other drivers could make use of this same
optimization. It would require hardware with internal buffers for
error correction and an access mode that easily returns page data
only...

Brian

struct nand_dma_desc {
       __le32 next_desc;
       __le32 next_desc_ext;
       u8 type_E;
       u8 irq_swap;
       u8 __pad1;
       u8 command;
       __le32 dram_addr;
       __le32 dram_addr_ext;
       __le32 tfr_len;
       __le32 total_len;
       __le32 flash_addr;
       __le32 flash_addr_ext;
       __le32 cs;
       u32 __pad[5];
       u8 valid;
       u8 ecc_status;
       u8 flash_status;
       u8 dma_status;
} __attribute__((packed));
Brian Norris April 18, 2012, 4:11 a.m. UTC | #4
Hi Shmulik,

On Tue, Apr 17, 2012 at 7:29 AM, Shmulik Ladkani
<shmulik.ladkani@gmail.com> wrote:
> Only verified execution in nand_base.c and nand.h, see below.

Thanks, this is helpful!

> On Mon, 16 Apr 2012 15:35:54 -0700 Brian Norris <computersforpeace@gmail.com> wrote:
>> @@ -1371,23 +1376,23 @@ static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
>>               chip->read_buf(mtd, p, eccsize);
>>
>>               if (chip->ecc.prepad) {
>> -                     chip->read_buf(mtd, oob, chip->ecc.prepad);
>> -                     oob += chip->ecc.prepad;
>> +                     chip->read_buf(mtd, oobbuf, chip->ecc.prepad);
>> +                     oobbuf += chip->ecc.prepad;
>>               }
>>
>>               chip->ecc.hwctl(mtd, NAND_ECC_READSYN);
>> -             chip->read_buf(mtd, oob, eccbytes);
>> -             stat = chip->ecc.correct(mtd, p, oob, NULL);
>> +             chip->read_buf(mtd, oobbuf, eccbytes);
>> +             stat = chip->ecc.correct(mtd, p, oobbuf, NULL);
>>
>>               if (stat < 0)
>>                       mtd->ecc_stats.failed++;
>>               else
>>                       mtd->ecc_stats.corrected += stat;
>>
>> -             oob += eccbytes;
>> +             oobbuf += eccbytes;
>>
>>               if (chip->ecc.postpad) {
>> -                     chip->read_buf(mtd, oob, chip->ecc.postpad);
>> +                     chip->read_buf(mtd, oobbuf, chip->ecc.postpad);
>>                       oob += chip->ecc.postpad;
>
> Missed convertion of 'oob' in the last line above.
> s/oob/oobbuf/

Yes, good catch. Thanks.

> Same problem in the following lines of 'nand_read_page_syndrome'.
> Should be:
>
> @@ -1393,9 +1393,9 @@ static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
>        }
>
>        /* Calculate remaining oob bytes */
> -       i = mtd->oobsize - (oob - chip->oob_poi);
> +       i = mtd->oobsize - (oobbuf - chip->oob_poi);
>        if (i)
> -               chip->read_buf(mtd, oob, i);
> +               chip->read_buf(mtd, oobbuf, i);
>
>        return 0;
>  }
>
>> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
>> index 1482340..be9ee1f 100644
>> --- a/include/linux/mtd/nand.h
>> +++ b/include/linux/mtd/nand.h
>> @@ -363,15 +363,15 @@ struct nand_ecc_ctrl {
>>       int (*correct)(struct mtd_info *mtd, uint8_t *dat, uint8_t *read_ecc,
>>                       uint8_t *calc_ecc);
>>       int (*read_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
>> -                     uint8_t *buf, int page);
>> +                     uint8_t *buf, uint8_t *oob, int page);
>>       void (*write_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
>> -                     const uint8_t *buf);
>> +                     const uint8_t *buf, const uint8_t *oob);
>>       int (*read_page)(struct mtd_info *mtd, struct nand_chip *chip,
>> -                     uint8_t *buf, int page);
>> +                     uint8_t *buf, uint8_t *oob, int page);
>>       int (*read_subpage)(struct mtd_info *mtd, struct nand_chip *chip,
>>                       uint32_t offs, uint32_t len, uint8_t *buf);
>
> No need to pass 'oob' to 'read_subpage'?

No. It seems like 'read_subpage' is already assumed not to read OOB
data; it only has one user, in nand_base.c, and this user has the
condition '!oob'. (My driver simply avoids subpage read/write
entirely, BTW, so I'm not too familiar with subpages.)

Thanks a lot for the review! This lets me know I should take a closer
look at my patch submissions...this pair of patches went through a lot
of revision before sending, so the 'oob' vs. 'oobbuf' thing got a
little convoluted.

Brian
Bastian Hecht April 18, 2012, 7:56 a.m. UTC | #5
Hello Brian,

I'm currently working on the hardware ECC part of the SH Mobile flctl
and I like the patchset as it makes things more explicit when OOB data
is requested or not and such things. It's cleaner and better, thanks!

Best regards,

 Bastian Hecht


2012/4/17 Brian Norris <computersforpeace@gmail.com>:
> New NAND controllers can perform read/write via HW engines which don't expose
> OOB data in their DMA mode. To reflect this, we should rework the nand_chip /
> nand_ecc_ctrl interfaces that assume that drivers will always read/write OOB
> data in the nand_chip.oob_poi buffer. A better interface would pass the
> appropriate buffer explicitly when OOB data is requested and otherwise pass a
> NULL pointer, meaning that no OOB data is needed.
>
> This patch adds the 'oob' parameter to each relevant {read,write}_page
> interface; all 'oob' parameters are left unused and NULL for now. The next
> patch will set the parameter properly. Perhaps it should be utilized eventually
> on some of these drivers, but this is a slow process of hacking each driver
> function that uses these interfaces.
>
> I renamed many 'oob' buffers to 'oobbuf' so that we can use 'oob' as an
> argument to these interface functions. I think the consistency can help in the
> long run.
>
> Note: I couldn't compile-test all of these easily, as some had ARCH
> dependencies.
>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>  drivers/mtd/nand/atmel_nand.c          |    7 ++-
>  drivers/mtd/nand/bcm_umi_bch.c         |   10 ++-
>  drivers/mtd/nand/bcm_umi_nand.c        |    2 +-
>  drivers/mtd/nand/bf5xx_nand.c          |    4 +-
>  drivers/mtd/nand/cafe_nand.c           |   26 ++++---
>  drivers/mtd/nand/denali.c              |    8 +-
>  drivers/mtd/nand/docg4.c               |   12 ++--
>  drivers/mtd/nand/fsl_elbc_nand.c       |    7 +-
>  drivers/mtd/nand/fsl_ifc_nand.c        |    4 +-
>  drivers/mtd/nand/fsmc_nand.c           |    3 +-
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    5 +-
>  drivers/mtd/nand/nand_base.c           |  123 ++++++++++++++++++--------------
>  drivers/mtd/nand/pxa3xx_nand.c         |    4 +-
>  drivers/mtd/nand/sh_flctl.c            |    4 +-
>  include/linux/mtd/nand.h               |   11 ++--
>  15 files changed, 127 insertions(+), 103 deletions(-)
>
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index 2165576..95838a6 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -324,18 +324,21 @@ static int atmel_nand_calculate(struct mtd_info *mtd,
>  * mtd:        mtd info structure
>  * chip:       nand chip info structure
>  * buf:        buffer to store read data
> + * oob:        buffer to store read OOB data
>  */
>  static int atmel_nand_read_page(struct mtd_info *mtd,
> -               struct nand_chip *chip, uint8_t *buf, int page)
> +               struct nand_chip *chip, uint8_t *buf, uint8_t *oob, int page)
>  {
>        int eccsize = chip->ecc.size;
>        int eccbytes = chip->ecc.bytes;
>        uint32_t *eccpos = chip->ecc.layout->eccpos;
>        uint8_t *p = buf;
> -       uint8_t *oob = chip->oob_poi;
>        uint8_t *ecc_pos;
>        int stat;
>
> +       if (!oob)
> +               oob = chip->oob_poi;
> +
>        /*
>         * Errata: ALE is incorrectly wired up to the ECC controller
>         * on the AP7000, so it will include the address cycles in the
> diff --git a/drivers/mtd/nand/bcm_umi_bch.c b/drivers/mtd/nand/bcm_umi_bch.c
> index a930666..3f4e845 100644
> --- a/drivers/mtd/nand/bcm_umi_bch.c
> +++ b/drivers/mtd/nand/bcm_umi_bch.c
> @@ -22,9 +22,9 @@
>
>  /* ---- Private Function Prototypes -------------------------------------- */
>  static int bcm_umi_bch_read_page_hwecc(struct mtd_info *mtd,
> -       struct nand_chip *chip, uint8_t *buf, int page);
> +       struct nand_chip *chip, uint8_t *buf, uint8_t *oob, int page);
>  static void bcm_umi_bch_write_page_hwecc(struct mtd_info *mtd,
> -       struct nand_chip *chip, const uint8_t *buf);
> +       struct nand_chip *chip, const uint8_t *buf, const uint8_t *oob);
>
>  /* ---- Private Variables ------------------------------------------------ */
>
> @@ -103,11 +103,12 @@ static struct nand_ecclayout nand_hw_eccoob_4096 = {
>  *  @mtd:       mtd info structure
>  *  @chip:      nand chip info structure
>  *  @buf:       buffer to store read data
> +*  @oob:       buffer to store read OOB data
>  *
>  ***************************************************************************/
>  static int bcm_umi_bch_read_page_hwecc(struct mtd_info *mtd,
>                                       struct nand_chip *chip, uint8_t * buf,
> -                                                int page)
> +                                      uint8_t *oob, int page)
>  {
>        int sectorIdx = 0;
>        int eccsize = chip->ecc.size;
> @@ -188,10 +189,11 @@ static int bcm_umi_bch_read_page_hwecc(struct mtd_info *mtd,
>  *  @mtd:       mtd info structure
>  *  @chip:      nand chip info structure
>  *  @buf:       data buffer
> +*  @oob:       OOB buffer
>  *
>  ***************************************************************************/
>  static void bcm_umi_bch_write_page_hwecc(struct mtd_info *mtd,
> -       struct nand_chip *chip, const uint8_t *buf)
> +       struct nand_chip *chip, const uint8_t *buf, const uint8_t *oob)
>  {
>        int sectorIdx = 0;
>        int eccsize = chip->ecc.size;
> diff --git a/drivers/mtd/nand/bcm_umi_nand.c b/drivers/mtd/nand/bcm_umi_nand.c
> index 6908cdd..ac7a027 100644
> --- a/drivers/mtd/nand/bcm_umi_nand.c
> +++ b/drivers/mtd/nand/bcm_umi_nand.c
> @@ -341,7 +341,7 @@ static int bcm_umi_nand_verify_buf(struct mtd_info *mtd, const u_char * buf,
>         * for MLC parts which may have permanently stuck bits.
>         */
>        struct nand_chip *chip = mtd->priv;
> -       int ret = chip->ecc.read_page(mtd, chip, readbackbuf, 0);
> +       int ret = chip->ecc.read_page(mtd, chip, readbackbuf, NULL, 0);
>        if (ret < 0)
>                return -EFAULT;
>        else {
> diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
> index d7b86b9..d25ab1a 100644
> --- a/drivers/mtd/nand/bf5xx_nand.c
> +++ b/drivers/mtd/nand/bf5xx_nand.c
> @@ -558,7 +558,7 @@ static void bf5xx_nand_dma_write_buf(struct mtd_info *mtd,
>  }
>
>  static int bf5xx_nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> -               uint8_t *buf, int page)
> +               uint8_t *buf, uint8_t *oob, int page)
>  {
>        bf5xx_nand_read_buf(mtd, buf, mtd->writesize);
>        bf5xx_nand_read_buf(mtd, chip->oob_poi, mtd->oobsize);
> @@ -567,7 +567,7 @@ static int bf5xx_nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip
>  }
>
>  static void bf5xx_nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> -               const uint8_t *buf)
> +               const uint8_t *buf, const uint8_t *oob)
>  {
>        bf5xx_nand_write_buf(mtd, buf, mtd->writesize);
>        bf5xx_nand_write_buf(mtd, chip->oob_poi, mtd->oobsize);
> diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c
> index 2973d97..e4103c5 100644
> --- a/drivers/mtd/nand/cafe_nand.c
> +++ b/drivers/mtd/nand/cafe_nand.c
> @@ -375,12 +375,13 @@ static int cafe_nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
>  * @mtd:       mtd info structure
>  * @chip:      nand chip info structure
>  * @buf:       buffer to store read data
> + * @oob:       buffer to store OOB data
>  *
>  * The hw generator calculates the error syndrome automatically. Therefor
>  * we need a special oob layout and handling.
>  */
>  static int cafe_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> -                              uint8_t *buf, int page)
> +                              uint8_t *buf, uint8_t *oob, int page)
>  {
>        struct cafe_priv *cafe = mtd->priv;
>
> @@ -394,7 +395,7 @@ static int cafe_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>        if (checkecc && cafe_readl(cafe, NAND_ECC_RESULT) & (1<<18)) {
>                unsigned short syn[8], pat[4];
>                int pos[4];
> -               u8 *oob = chip->oob_poi;
> +               u8 *oobbuf = chip->oob_poi;
>                int i, n;
>
>                for (i=0; i<8; i+=2) {
> @@ -422,14 +423,14 @@ static int cafe_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>                                        buf[0] ^= pat[i];
>                        } else if (p == 1365) {
>                                buf[2047] ^= pat[i] >> 4;
> -                               oob[0] ^= pat[i] << 4;
> +                               oobbuf[0] ^= pat[i] << 4;
>                        } else if (p > 1365) {
>                                if ((p & 1) == 1) {
> -                                       oob[3*p/2 - 2048] ^= pat[i] >> 4;
> -                                       oob[3*p/2 - 2047] ^= pat[i] << 4;
> +                                       oobbuf[3*p/2 - 2048] ^= pat[i] >> 4;
> +                                       oobbuf[3*p/2 - 2047] ^= pat[i] << 4;
>                                } else {
> -                                       oob[3*p/2 - 2049] ^= pat[i] >> 8;
> -                                       oob[3*p/2 - 2048] ^= pat[i];
> +                                       oobbuf[3*p/2 - 2049] ^= pat[i] >> 8;
> +                                       oobbuf[3*p/2 - 2048] ^= pat[i];
>                                }
>                        } else if ((p & 1) == 1) {
>                                buf[3*p/2] ^= pat[i] >> 4;
> @@ -518,7 +519,9 @@ static struct nand_bbt_descr cafe_bbt_mirror_descr_512 = {
>
>
>  static void cafe_nand_write_page_lowlevel(struct mtd_info *mtd,
> -                                         struct nand_chip *chip, const uint8_t *buf)
> +                                         struct nand_chip *chip,
> +                                         const uint8_t *buf,
> +                                         const uint8_t *oob)
>  {
>        struct cafe_priv *cafe = mtd->priv;
>
> @@ -530,16 +533,17 @@ static void 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 page, int cached, int raw)
> +                               const uint8_t *buf, const uint8_t *oob,
> +                               int page, int cached, int raw)
>  {
>        int status;
>
>        chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
>
>        if (unlikely(raw))
> -               chip->ecc.write_page_raw(mtd, chip, buf);
> +               chip->ecc.write_page_raw(mtd, chip, buf, oob);
>        else
> -               chip->ecc.write_page(mtd, chip, buf);
> +               chip->ecc.write_page(mtd, chip, buf, oob);
>
>        /*
>         * Cached progamming disabled for now, Not sure if its worth the
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index a1048c7..d078470 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -1084,7 +1084,7 @@ static void write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  * by write_page above.
>  * */
>  static void denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> -                               const uint8_t *buf)
> +                               const uint8_t *buf, const uint8_t *oob)
>  {
>        /* for regular page writes, we let HW handle all the ECC
>         * data written to the device. */
> @@ -1096,7 +1096,7 @@ static void denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  * write_page() function above.
>  */
>  static void denali_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> -                                       const uint8_t *buf)
> +                                       const uint8_t *buf, const uint8_t *oob)
>  {
>        /* for raw page writes, we want to disable ECC and simply write
>           whatever data is in the buffer. */
> @@ -1119,7 +1119,7 @@ static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
>  }
>
>  static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> -                           uint8_t *buf, int page)
> +                           uint8_t *buf, uint8_t *oob, int page)
>  {
>        struct denali_nand_info *denali = mtd_to_denali(mtd);
>
> @@ -1171,7 +1171,7 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>  }
>
>  static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> -                               uint8_t *buf, int page)
> +                               uint8_t *buf, uint8_t *oob, int page)
>  {
>        struct denali_nand_info *denali = mtd_to_denali(mtd);
>
> diff --git a/drivers/mtd/nand/docg4.c b/drivers/mtd/nand/docg4.c
> index b082026..7ff99c9 100644
> --- a/drivers/mtd/nand/docg4.c
> +++ b/drivers/mtd/nand/docg4.c
> @@ -786,13 +786,13 @@ static int read_page(struct mtd_info *mtd, struct nand_chip *nand,
>
>
>  static int docg4_read_page_raw(struct mtd_info *mtd, struct nand_chip *nand,
> -                              uint8_t *buf, int page)
> +                              uint8_t *buf, uint8_t *oob, int page)
>  {
>        return read_page(mtd, nand, buf, page, false);
>  }
>
>  static int docg4_read_page(struct mtd_info *mtd, struct nand_chip *nand,
> -                          uint8_t *buf, int page)
> +                          uint8_t *buf, uint8_t *oob, int page)
>  {
>        return read_page(mtd, nand, buf, page, true);
>  }
> @@ -952,13 +952,13 @@ static void write_page(struct mtd_info *mtd, struct nand_chip *nand,
>  }
>
>  static void docg4_write_page_raw(struct mtd_info *mtd, struct nand_chip *nand,
> -                                const uint8_t *buf)
> +                                const uint8_t *buf, const uint8_t *oob)
>  {
>        return write_page(mtd, nand, buf, false);
>  }
>
>  static void docg4_write_page(struct mtd_info *mtd, struct nand_chip *nand,
> -                            const uint8_t *buf)
> +                            const uint8_t *buf, const uint8_t *oob)
>  {
>        return write_page(mtd, nand, buf, true);
>  }
> @@ -1002,7 +1002,7 @@ static int __init read_factory_bbt(struct mtd_info *mtd)
>                return -ENOMEM;
>
>        read_page_prologue(mtd, g4_addr);
> -       status = docg4_read_page(mtd, nand, buf, DOCG4_FACTORY_BBT_PAGE);
> +       status = docg4_read_page(mtd, nand, buf, NULL, DOCG4_FACTORY_BBT_PAGE);
>        if (status)
>                goto exit;
>
> @@ -1079,7 +1079,7 @@ static int docg4_block_markbad(struct mtd_info *mtd, loff_t ofs)
>
>        /* write first page of block */
>        write_page_prologue(mtd, g4_addr);
> -       docg4_write_page(mtd, nand, buf);
> +       docg4_write_page(mtd, nand, buf, nand->oob_poi);
>        ret = pageprog(mtd);
>        if (!ret)
>                mtd->ecc_stats.badblocks++;
> diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
> index 80b5264..b7d9bfd 100644
> --- a/drivers/mtd/nand/fsl_elbc_nand.c
> +++ b/drivers/mtd/nand/fsl_elbc_nand.c
> @@ -740,7 +740,7 @@ static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
>
>  static int fsl_elbc_read_page(struct mtd_info *mtd,
>                               struct nand_chip *chip,
> -                             uint8_t *buf,
> +                             uint8_t *buf, uint8_t *oob,
>                              int page)
>  {
>        fsl_elbc_read_buf(mtd, buf, mtd->writesize);
> @@ -755,9 +755,8 @@ static int fsl_elbc_read_page(struct mtd_info *mtd,
>  /* ECC will be calculated automatically, and errors will be detected in
>  * waitfunc.
>  */
> -static void fsl_elbc_write_page(struct mtd_info *mtd,
> -                                struct nand_chip *chip,
> -                                const uint8_t *buf)
> +static void fsl_elbc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> +                               const uint8_t *buf, const uint8_t *oob)
>  {
>        fsl_elbc_write_buf(mtd, buf, mtd->writesize);
>        fsl_elbc_write_buf(mtd, chip->oob_poi, mtd->oobsize);
> diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
> index c30ac7b..f8a6e3e 100644
> --- a/drivers/mtd/nand/fsl_ifc_nand.c
> +++ b/drivers/mtd/nand/fsl_ifc_nand.c
> @@ -684,7 +684,7 @@ static int fsl_ifc_wait(struct mtd_info *mtd, struct nand_chip *chip)
>
>  static int fsl_ifc_read_page(struct mtd_info *mtd,
>                              struct nand_chip *chip,
> -                             uint8_t *buf, int page)
> +                             uint8_t *buf, uint8_t *oob, int page)
>  {
>        struct fsl_ifc_mtd *priv = chip->priv;
>        struct fsl_ifc_ctrl *ctrl = priv->ctrl;
> @@ -706,7 +706,7 @@ static int fsl_ifc_read_page(struct mtd_info *mtd,
>  */
>  static void fsl_ifc_write_page(struct mtd_info *mtd,
>                                struct nand_chip *chip,
> -                               const uint8_t *buf)
> +                               const uint8_t *buf, const uint8_t *oob)
>  {
>        fsl_ifc_write_buf(mtd, buf, mtd->writesize);
>        fsl_ifc_write_buf(mtd, chip->oob_poi, mtd->oobsize);
> diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
> index 1b8330e..8bb731b 100644
> --- a/drivers/mtd/nand/fsmc_nand.c
> +++ b/drivers/mtd/nand/fsmc_nand.c
> @@ -692,6 +692,7 @@ static void fsmc_write_buf_dma(struct mtd_info *mtd, const uint8_t *buf,
>  * @mtd:       mtd info structure
>  * @chip:      nand chip info structure
>  * @buf:       buffer to store read data
> + * @oob:       buffer to store read OOB data
>  * @page:      page number to read
>  *
>  * This routine is needed for fsmc version 8 as reading from NAND chip has to be
> @@ -701,7 +702,7 @@ static void fsmc_write_buf_dma(struct mtd_info *mtd, const uint8_t *buf,
>  * max of 8 bits)
>  */
>  static int fsmc_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
> -                                uint8_t *buf, int page)
> +                                uint8_t *buf, uint8_t *oob, int page)
>  {
>        struct fsmc_nand_data *host = container_of(mtd,
>                                        struct fsmc_nand_data, mtd);
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 75b1dde..949c761 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -841,7 +841,7 @@ static void block_mark_swapping(struct gpmi_nand_data *this,
>  }
>
>  static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> -                               uint8_t *buf, int page)
> +                               uint8_t *buf, uint8_t *oob, int page)
>  {
>        struct gpmi_nand_data *this = chip->priv;
>        struct bch_geometry *nfc_geo = &this->bch_geometry;
> @@ -928,7 +928,8 @@ exit_nfc:
>  }
>
>  static void gpmi_ecc_write_page(struct mtd_info *mtd,
> -                               struct nand_chip *chip, const uint8_t *buf)
> +                               struct nand_chip *chip, const uint8_t *buf,
> +                               const uint8_t *oob)
>  {
>        struct gpmi_nand_data *this = chip->priv;
>        struct bch_geometry *nfc_geo = &this->bch_geometry;
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 47b19c0..95ba987 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -1066,12 +1066,13 @@ EXPORT_SYMBOL(nand_lock);
>  * @mtd: mtd info structure
>  * @chip: nand chip info structure
>  * @buf: buffer to store read data
> + * @oob: buffer to store read OOB data
>  * @page: page number to read
>  *
>  * Not for syndrome calculating ECC controllers, which use a special oob layout.
>  */
>  static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> -                             uint8_t *buf, int page)
> +                             uint8_t *buf, uint8_t *oob, int page)
>  {
>        chip->read_buf(mtd, buf, mtd->writesize);
>        chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
> @@ -1083,17 +1084,18 @@ static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>  * @mtd: mtd info structure
>  * @chip: nand chip info structure
>  * @buf: buffer to store read data
> + * @oob: buffer to store read OOB
>  * @page: page number to read
>  *
>  * We need a special oob layout and handling even when OOB isn't used.
>  */
>  static int nand_read_page_raw_syndrome(struct mtd_info *mtd,
>                                        struct nand_chip *chip,
> -                                       uint8_t *buf, int page)
> +                                       uint8_t *buf, uint8_t *oob, int page)
>  {
>        int eccsize = chip->ecc.size;
>        int eccbytes = chip->ecc.bytes;
> -       uint8_t *oob = chip->oob_poi;
> +       uint8_t *oobbuf = chip->oob_poi;
>        int steps, size;
>
>        for (steps = chip->ecc.steps; steps > 0; steps--) {
> @@ -1101,22 +1103,22 @@ static int nand_read_page_raw_syndrome(struct mtd_info *mtd,
>                buf += eccsize;
>
>                if (chip->ecc.prepad) {
> -                       chip->read_buf(mtd, oob, chip->ecc.prepad);
> -                       oob += chip->ecc.prepad;
> +                       chip->read_buf(mtd, oobbuf, chip->ecc.prepad);
> +                       oobbuf += chip->ecc.prepad;
>                }
>
> -               chip->read_buf(mtd, oob, eccbytes);
> -               oob += eccbytes;
> +               chip->read_buf(mtd, oobbuf, eccbytes);
> +               oobbuf += eccbytes;
>
>                if (chip->ecc.postpad) {
> -                       chip->read_buf(mtd, oob, chip->ecc.postpad);
> -                       oob += chip->ecc.postpad;
> +                       chip->read_buf(mtd, oobbuf, chip->ecc.postpad);
> +                       oobbuf += chip->ecc.postpad;
>                }
>        }
>
> -       size = mtd->oobsize - (oob - chip->oob_poi);
> +       size = mtd->oobsize - (oobbuf - chip->oob_poi);
>        if (size)
> -               chip->read_buf(mtd, oob, size);
> +               chip->read_buf(mtd, oobbuf, size);
>
>        return 0;
>  }
> @@ -1129,7 +1131,7 @@ static int nand_read_page_raw_syndrome(struct mtd_info *mtd,
>  * @page: page number to read
>  */
>  static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
> -                               uint8_t *buf, int page)
> +                               uint8_t *buf, uint8_t *oob, int page)
>  {
>        int i, eccsize = chip->ecc.size;
>        int eccbytes = chip->ecc.bytes;
> @@ -1139,7 +1141,7 @@ static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
>        uint8_t *ecc_code = chip->buffers->ecccode;
>        uint32_t *eccpos = chip->ecc.layout->eccpos;
>
> -       chip->ecc.read_page_raw(mtd, chip, buf, page);
> +       chip->ecc.read_page_raw(mtd, chip, buf, oob, page);
>
>        for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
>                chip->ecc.calculate(mtd, p, &ecc_calc[i]);
> @@ -1257,12 +1259,13 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
>  * @mtd: mtd info structure
>  * @chip: nand chip info structure
>  * @buf: buffer to store read data
> + * @oob: buffer to store read OOB
>  * @page: page number to read
>  *
>  * Not for syndrome calculating ECC controllers which need a special oob layout.
>  */
>  static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
> -                               uint8_t *buf, int page)
> +                               uint8_t *buf, uint8_t *oob, int page)
>  {
>        int i, eccsize = chip->ecc.size;
>        int eccbytes = chip->ecc.bytes;
> @@ -1302,6 +1305,7 @@ static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>  * @mtd: mtd info structure
>  * @chip: nand chip info structure
>  * @buf: buffer to store read data
> + * @oob: buffer to store read OOB
>  * @page: page number to read
>  *
>  * Hardware ECC for large page chips, require OOB to be read first. For this
> @@ -1311,7 +1315,7 @@ static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>  * the data area, by overwriting the NAND manufacturer bad block markings.
>  */
>  static int nand_read_page_hwecc_oob_first(struct mtd_info *mtd,
> -       struct nand_chip *chip, uint8_t *buf, int page)
> +       struct nand_chip *chip, uint8_t *buf, uint8_t *oob, int page)
>  {
>        int i, eccsize = chip->ecc.size;
>        int eccbytes = chip->ecc.bytes;
> @@ -1350,19 +1354,20 @@ static int nand_read_page_hwecc_oob_first(struct mtd_info *mtd,
>  * @mtd: mtd info structure
>  * @chip: nand chip info structure
>  * @buf: buffer to store read data
> + * @oob: buffer to store read OOB
>  * @page: page number to read
>  *
>  * The hw generator calculates the error syndrome automatically. Therefore we
>  * need a special oob layout and handling.
>  */
>  static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
> -                                  uint8_t *buf, int page)
> +                                  uint8_t *buf, uint8_t *oob, int page)
>  {
>        int i, eccsize = chip->ecc.size;
>        int eccbytes = chip->ecc.bytes;
>        int eccsteps = chip->ecc.steps;
>        uint8_t *p = buf;
> -       uint8_t *oob = chip->oob_poi;
> +       uint8_t *oobbuf = chip->oob_poi;
>
>        for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
>                int stat;
> @@ -1371,23 +1376,23 @@ static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
>                chip->read_buf(mtd, p, eccsize);
>
>                if (chip->ecc.prepad) {
> -                       chip->read_buf(mtd, oob, chip->ecc.prepad);
> -                       oob += chip->ecc.prepad;
> +                       chip->read_buf(mtd, oobbuf, chip->ecc.prepad);
> +                       oobbuf += chip->ecc.prepad;
>                }
>
>                chip->ecc.hwctl(mtd, NAND_ECC_READSYN);
> -               chip->read_buf(mtd, oob, eccbytes);
> -               stat = chip->ecc.correct(mtd, p, oob, NULL);
> +               chip->read_buf(mtd, oobbuf, eccbytes);
> +               stat = chip->ecc.correct(mtd, p, oobbuf, NULL);
>
>                if (stat < 0)
>                        mtd->ecc_stats.failed++;
>                else
>                        mtd->ecc_stats.corrected += stat;
>
> -               oob += eccbytes;
> +               oobbuf += eccbytes;
>
>                if (chip->ecc.postpad) {
> -                       chip->read_buf(mtd, oob, chip->ecc.postpad);
> +                       chip->read_buf(mtd, oobbuf, chip->ecc.postpad);
>                        oob += chip->ecc.postpad;
>                }
>        }
> @@ -1500,14 +1505,14 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
>
>                        /* Now read the page into the buffer */
>                        if (unlikely(ops->mode == MTD_OPS_RAW))
> -                               ret = chip->ecc.read_page_raw(mtd, chip,
> -                                                             bufpoi, page);
> +                               ret = chip->ecc.read_page_raw(mtd, chip, bufpoi,
> +                                                             NULL, page);
>                        else if (!aligned && NAND_SUBPAGE_READ(chip) && !oob)
>                                ret = chip->ecc.read_subpage(mtd, chip,
>                                                        col, bytes, bufpoi);
>                        else
>                                ret = chip->ecc.read_page(mtd, chip, bufpoi,
> -                                                         page);
> +                                                         NULL, page);
>                        if (ret < 0) {
>                                if (!aligned)
>                                        /* Invalidate page cache */
> @@ -1919,11 +1924,12 @@ out:
>  * @mtd: mtd info structure
>  * @chip: nand chip info structure
>  * @buf: data buffer
> + * @oob: OOB buffer
>  *
>  * Not for syndrome calculating ECC controllers, which use a special oob layout.
>  */
>  static void nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> -                               const uint8_t *buf)
> +                               const uint8_t *buf, const uint8_t *oob)
>  {
>        chip->write_buf(mtd, buf, mtd->writesize);
>        chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
> @@ -1934,16 +1940,18 @@ static void nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>  * @mtd: mtd info structure
>  * @chip: nand chip info structure
>  * @buf: data buffer
> + * @oob: OOB buffer
>  *
>  * We need a special oob layout and handling even when ECC isn't checked.
>  */
>  static void nand_write_page_raw_syndrome(struct mtd_info *mtd,
>                                        struct nand_chip *chip,
> -                                       const uint8_t *buf)
> +                                       const uint8_t *buf,
> +                                       const uint8_t *oob)
>  {
>        int eccsize = chip->ecc.size;
>        int eccbytes = chip->ecc.bytes;
> -       uint8_t *oob = chip->oob_poi;
> +       uint8_t *oobbuf = chip->oob_poi;
>        int steps, size;
>
>        for (steps = chip->ecc.steps; steps > 0; steps--) {
> @@ -1951,31 +1959,32 @@ static void nand_write_page_raw_syndrome(struct mtd_info *mtd,
>                buf += eccsize;
>
>                if (chip->ecc.prepad) {
> -                       chip->write_buf(mtd, oob, chip->ecc.prepad);
> -                       oob += chip->ecc.prepad;
> +                       chip->write_buf(mtd, oobbuf, chip->ecc.prepad);
> +                       oobbuf += chip->ecc.prepad;
>                }
>
> -               chip->read_buf(mtd, oob, eccbytes);
> -               oob += eccbytes;
> +               chip->read_buf(mtd, oobbuf, eccbytes);
> +               oobbuf += eccbytes;
>
>                if (chip->ecc.postpad) {
> -                       chip->write_buf(mtd, oob, chip->ecc.postpad);
> -                       oob += chip->ecc.postpad;
> +                       chip->write_buf(mtd, oobbuf, chip->ecc.postpad);
> +                       oobbuf += chip->ecc.postpad;
>                }
>        }
>
> -       size = mtd->oobsize - (oob - chip->oob_poi);
> +       size = mtd->oobsize - (oobbuf - chip->oob_poi);
>        if (size)
> -               chip->write_buf(mtd, oob, size);
> +               chip->write_buf(mtd, oobbuf, size);
>  }
>  /**
>  * nand_write_page_swecc - [REPLACEABLE] software ECC based page write function
>  * @mtd: mtd info structure
>  * @chip: nand chip info structure
>  * @buf: data buffer
> + * @oob: OOB buffer
>  */
>  static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
> -                                 const uint8_t *buf)
> +                                 const uint8_t *buf, const uint8_t *oob)
>  {
>        int i, eccsize = chip->ecc.size;
>        int eccbytes = chip->ecc.bytes;
> @@ -1991,7 +2000,7 @@ static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
>        for (i = 0; i < chip->ecc.total; i++)
>                chip->oob_poi[eccpos[i]] = ecc_calc[i];
>
> -       chip->ecc.write_page_raw(mtd, chip, buf);
> +       chip->ecc.write_page_raw(mtd, chip, buf, oob);
>  }
>
>  /**
> @@ -1999,9 +2008,10 @@ static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
>  * @mtd: mtd info structure
>  * @chip: nand chip info structure
>  * @buf: data buffer
> + * @oob: OOB buffer
>  */
>  static void nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
> -                                 const uint8_t *buf)
> +                                 const uint8_t *buf, const uint8_t *oob)
>  {
>        int i, eccsize = chip->ecc.size;
>        int eccbytes = chip->ecc.bytes;
> @@ -2027,18 +2037,20 @@ static void nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>  * @mtd: mtd info structure
>  * @chip: nand chip info structure
>  * @buf: data buffer
> + * @oob: OOB data buffer
>  *
>  * The hw generator calculates the error syndrome automatically. Therefore we
>  * need a special oob layout and handling.
>  */
>  static void nand_write_page_syndrome(struct mtd_info *mtd,
> -                                   struct nand_chip *chip, const uint8_t *buf)
> +                                   struct nand_chip *chip, const uint8_t *buf,
> +                                   const uint8_t *oob)
>  {
>        int i, eccsize = chip->ecc.size;
>        int eccbytes = chip->ecc.bytes;
>        int eccsteps = chip->ecc.steps;
>        const uint8_t *p = buf;
> -       uint8_t *oob = chip->oob_poi;
> +       uint8_t *oobbuf = chip->oob_poi;
>
>        for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
>
> @@ -2046,24 +2058,24 @@ static void nand_write_page_syndrome(struct mtd_info *mtd,
>                chip->write_buf(mtd, p, eccsize);
>
>                if (chip->ecc.prepad) {
> -                       chip->write_buf(mtd, oob, chip->ecc.prepad);
> -                       oob += chip->ecc.prepad;
> +                       chip->write_buf(mtd, oobbuf, chip->ecc.prepad);
> +                       oobbuf += chip->ecc.prepad;
>                }
>
> -               chip->ecc.calculate(mtd, p, oob);
> -               chip->write_buf(mtd, oob, eccbytes);
> -               oob += eccbytes;
> +               chip->ecc.calculate(mtd, p, oobbuf);
> +               chip->write_buf(mtd, oobbuf, eccbytes);
> +               oobbuf += eccbytes;
>
>                if (chip->ecc.postpad) {
> -                       chip->write_buf(mtd, oob, chip->ecc.postpad);
> -                       oob += chip->ecc.postpad;
> +                       chip->write_buf(mtd, oobbuf, chip->ecc.postpad);
> +                       oobbuf += chip->ecc.postpad;
>                }
>        }
>
>        /* Calculate remaining oob bytes */
> -       i = mtd->oobsize - (oob - chip->oob_poi);
> +       i = mtd->oobsize - (oobbuf - chip->oob_poi);
>        if (i)
> -               chip->write_buf(mtd, oob, i);
> +               chip->write_buf(mtd, oobbuf, i);
>  }
>
>  /**
> @@ -2076,16 +2088,17 @@ static void 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 page, int cached, int raw)
> +                          const uint8_t *buf, const uint8_t *oob, int page,
> +                          int cached, int raw)
>  {
>        int status;
>
>        chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
>
>        if (unlikely(raw))
> -               chip->ecc.write_page_raw(mtd, chip, buf);
> +               chip->ecc.write_page_raw(mtd, chip, buf, oob);
>        else
> -               chip->ecc.write_page(mtd, chip, buf);
> +               chip->ecc.write_page(mtd, chip, buf, oob);
>
>        /*
>         * Cached progamming disabled for now. Not sure if it's worth the
> @@ -2264,7 +2277,7 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
>                        memset(chip->oob_poi, 0xff, mtd->oobsize);
>                }
>
> -               ret = chip->write_page(mtd, chip, wbuf, page, cached,
> +               ret = chip->write_page(mtd, chip, wbuf, NULL, page, cached,
>                                       (ops->mode == MTD_OPS_RAW));
>                if (ret)
>                        break;
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index def50ca..92d2eae 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -682,14 +682,14 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
>  }
>
>  static void pxa3xx_nand_write_page_hwecc(struct mtd_info *mtd,
> -               struct nand_chip *chip, const uint8_t *buf)
> +               struct nand_chip *chip, const uint8_t *buf, const uint8_t *oob)
>  {
>        chip->write_buf(mtd, buf, mtd->writesize);
>        chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
>  }
>
>  static int pxa3xx_nand_read_page_hwecc(struct mtd_info *mtd,
> -               struct nand_chip *chip, uint8_t *buf, int page)
> +               struct nand_chip *chip, uint8_t *buf, uint8_t *oob, int page)
>  {
>        struct pxa3xx_nand_host *host = mtd->priv;
>        struct pxa3xx_nand_info *info = host->info_data;
> diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
> index e9b2b26..d1b2731 100644
> --- a/drivers/mtd/nand/sh_flctl.c
> +++ b/drivers/mtd/nand/sh_flctl.c
> @@ -344,7 +344,7 @@ static void set_cmd_regs(struct mtd_info *mtd, uint32_t cmd, uint32_t flcmcdr_va
>  }
>
>  static int flctl_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
> -                               uint8_t *buf, int page)
> +                               uint8_t *buf, uint8_t *oob, int page)
>  {
>        int i, eccsize = chip->ecc.size;
>        int eccbytes = chip->ecc.bytes;
> @@ -366,7 +366,7 @@ static int flctl_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>  }
>
>  static void flctl_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
> -                                  const uint8_t *buf)
> +                                  const uint8_t *buf, const uint8_t *oob)
>  {
>        int i, eccsize = chip->ecc.size;
>        int eccbytes = chip->ecc.bytes;
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 1482340..be9ee1f 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -363,15 +363,15 @@ struct nand_ecc_ctrl {
>        int (*correct)(struct mtd_info *mtd, uint8_t *dat, uint8_t *read_ecc,
>                        uint8_t *calc_ecc);
>        int (*read_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
> -                       uint8_t *buf, int page);
> +                       uint8_t *buf, uint8_t *oob, int page);
>        void (*write_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
> -                       const uint8_t *buf);
> +                       const uint8_t *buf, const uint8_t *oob);
>        int (*read_page)(struct mtd_info *mtd, struct nand_chip *chip,
> -                       uint8_t *buf, int page);
> +                       uint8_t *buf, uint8_t *oob, int page);
>        int (*read_subpage)(struct mtd_info *mtd, struct nand_chip *chip,
>                        uint32_t offs, uint32_t len, uint8_t *buf);
>        void (*write_page)(struct mtd_info *mtd, struct nand_chip *chip,
> -                       const uint8_t *buf);
> +                       const uint8_t *buf, const uint8_t *oob);
>        int (*write_oob_raw)(struct mtd_info *mtd, struct nand_chip *chip,
>                        int page);
>        int (*read_oob_raw)(struct mtd_info *mtd, struct nand_chip *chip,
> @@ -505,7 +505,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 page, int cached, int raw);
> +                       const uint8_t *buf, const uint8_t *oob, int page,
> +                       int cached, int raw);
>
>        int chip_delay;
>        unsigned int options;
> --
> 1.7.5.4.2.g519b1
>
Bastian Hecht April 18, 2012, 9:37 a.m. UTC | #6
2012/4/18 Bastian Hecht <hechtb@googlemail.com>:
> Hello Brian,
>
> I'm currently working on the hardware ECC part of the SH Mobile flctl
> and I like the patchset as it makes things more explicit when OOB data
> is requested or not and such things. It's cleaner and better, thanks!

To be a bit more of a concrete help, if you can give me a use case to
test the oob reads/writes I can modify my driver to work with your
patches. Right now I rely on nandwrite, nanddump, nandtest, the kernel
test modules and ubi. With none of them I produced an error so far
although the current implementation of the hardware ECC completely
ignores OOB data other than ECC.

Best regards,

 Bastian Hecht


> Best regards,
>
>  Bastian Hecht
>
>
> 2012/4/17 Brian Norris <computersforpeace@gmail.com>:
>> New NAND controllers can perform read/write via HW engines which don't expose
>> OOB data in their DMA mode. To reflect this, we should rework the nand_chip /
>> nand_ecc_ctrl interfaces that assume that drivers will always read/write OOB
>> data in the nand_chip.oob_poi buffer. A better interface would pass the
>> appropriate buffer explicitly when OOB data is requested and otherwise pass a
>> NULL pointer, meaning that no OOB data is needed.
>>
>> This patch adds the 'oob' parameter to each relevant {read,write}_page
>> interface; all 'oob' parameters are left unused and NULL for now. The next
>> patch will set the parameter properly. Perhaps it should be utilized eventually
>> on some of these drivers, but this is a slow process of hacking each driver
>> function that uses these interfaces.
>>
>> I renamed many 'oob' buffers to 'oobbuf' so that we can use 'oob' as an
>> argument to these interface functions. I think the consistency can help in the
>> long run.
>>
>> Note: I couldn't compile-test all of these easily, as some had ARCH
>> dependencies.
>>
>> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
>> ---
>>  drivers/mtd/nand/atmel_nand.c          |    7 ++-
>>  drivers/mtd/nand/bcm_umi_bch.c         |   10 ++-
>>  drivers/mtd/nand/bcm_umi_nand.c        |    2 +-
>>  drivers/mtd/nand/bf5xx_nand.c          |    4 +-
>>  drivers/mtd/nand/cafe_nand.c           |   26 ++++---
>>  drivers/mtd/nand/denali.c              |    8 +-
>>  drivers/mtd/nand/docg4.c               |   12 ++--
>>  drivers/mtd/nand/fsl_elbc_nand.c       |    7 +-
>>  drivers/mtd/nand/fsl_ifc_nand.c        |    4 +-
>>  drivers/mtd/nand/fsmc_nand.c           |    3 +-
>>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    5 +-
>>  drivers/mtd/nand/nand_base.c           |  123 ++++++++++++++++++--------------
>>  drivers/mtd/nand/pxa3xx_nand.c         |    4 +-
>>  drivers/mtd/nand/sh_flctl.c            |    4 +-
>>  include/linux/mtd/nand.h               |   11 ++--
>>  15 files changed, 127 insertions(+), 103 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
>> index 2165576..95838a6 100644
>> --- a/drivers/mtd/nand/atmel_nand.c
>> +++ b/drivers/mtd/nand/atmel_nand.c
>> @@ -324,18 +324,21 @@ static int atmel_nand_calculate(struct mtd_info *mtd,
>>  * mtd:        mtd info structure
>>  * chip:       nand chip info structure
>>  * buf:        buffer to store read data
>> + * oob:        buffer to store read OOB data
>>  */
>>  static int atmel_nand_read_page(struct mtd_info *mtd,
>> -               struct nand_chip *chip, uint8_t *buf, int page)
>> +               struct nand_chip *chip, uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        int eccsize = chip->ecc.size;
>>        int eccbytes = chip->ecc.bytes;
>>        uint32_t *eccpos = chip->ecc.layout->eccpos;
>>        uint8_t *p = buf;
>> -       uint8_t *oob = chip->oob_poi;
>>        uint8_t *ecc_pos;
>>        int stat;
>>
>> +       if (!oob)
>> +               oob = chip->oob_poi;
>> +
>>        /*
>>         * Errata: ALE is incorrectly wired up to the ECC controller
>>         * on the AP7000, so it will include the address cycles in the
>> diff --git a/drivers/mtd/nand/bcm_umi_bch.c b/drivers/mtd/nand/bcm_umi_bch.c
>> index a930666..3f4e845 100644
>> --- a/drivers/mtd/nand/bcm_umi_bch.c
>> +++ b/drivers/mtd/nand/bcm_umi_bch.c
>> @@ -22,9 +22,9 @@
>>
>>  /* ---- Private Function Prototypes -------------------------------------- */
>>  static int bcm_umi_bch_read_page_hwecc(struct mtd_info *mtd,
>> -       struct nand_chip *chip, uint8_t *buf, int page);
>> +       struct nand_chip *chip, uint8_t *buf, uint8_t *oob, int page);
>>  static void bcm_umi_bch_write_page_hwecc(struct mtd_info *mtd,
>> -       struct nand_chip *chip, const uint8_t *buf);
>> +       struct nand_chip *chip, const uint8_t *buf, const uint8_t *oob);
>>
>>  /* ---- Private Variables ------------------------------------------------ */
>>
>> @@ -103,11 +103,12 @@ static struct nand_ecclayout nand_hw_eccoob_4096 = {
>>  *  @mtd:       mtd info structure
>>  *  @chip:      nand chip info structure
>>  *  @buf:       buffer to store read data
>> +*  @oob:       buffer to store read OOB data
>>  *
>>  ***************************************************************************/
>>  static int bcm_umi_bch_read_page_hwecc(struct mtd_info *mtd,
>>                                       struct nand_chip *chip, uint8_t * buf,
>> -                                                int page)
>> +                                      uint8_t *oob, int page)
>>  {
>>        int sectorIdx = 0;
>>        int eccsize = chip->ecc.size;
>> @@ -188,10 +189,11 @@ static int bcm_umi_bch_read_page_hwecc(struct mtd_info *mtd,
>>  *  @mtd:       mtd info structure
>>  *  @chip:      nand chip info structure
>>  *  @buf:       data buffer
>> +*  @oob:       OOB buffer
>>  *
>>  ***************************************************************************/
>>  static void bcm_umi_bch_write_page_hwecc(struct mtd_info *mtd,
>> -       struct nand_chip *chip, const uint8_t *buf)
>> +       struct nand_chip *chip, const uint8_t *buf, const uint8_t *oob)
>>  {
>>        int sectorIdx = 0;
>>        int eccsize = chip->ecc.size;
>> diff --git a/drivers/mtd/nand/bcm_umi_nand.c b/drivers/mtd/nand/bcm_umi_nand.c
>> index 6908cdd..ac7a027 100644
>> --- a/drivers/mtd/nand/bcm_umi_nand.c
>> +++ b/drivers/mtd/nand/bcm_umi_nand.c
>> @@ -341,7 +341,7 @@ static int bcm_umi_nand_verify_buf(struct mtd_info *mtd, const u_char * buf,
>>         * for MLC parts which may have permanently stuck bits.
>>         */
>>        struct nand_chip *chip = mtd->priv;
>> -       int ret = chip->ecc.read_page(mtd, chip, readbackbuf, 0);
>> +       int ret = chip->ecc.read_page(mtd, chip, readbackbuf, NULL, 0);
>>        if (ret < 0)
>>                return -EFAULT;
>>        else {
>> diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
>> index d7b86b9..d25ab1a 100644
>> --- a/drivers/mtd/nand/bf5xx_nand.c
>> +++ b/drivers/mtd/nand/bf5xx_nand.c
>> @@ -558,7 +558,7 @@ static void bf5xx_nand_dma_write_buf(struct mtd_info *mtd,
>>  }
>>
>>  static int bf5xx_nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>> -               uint8_t *buf, int page)
>> +               uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        bf5xx_nand_read_buf(mtd, buf, mtd->writesize);
>>        bf5xx_nand_read_buf(mtd, chip->oob_poi, mtd->oobsize);
>> @@ -567,7 +567,7 @@ static int bf5xx_nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip
>>  }
>>
>>  static void bf5xx_nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>> -               const uint8_t *buf)
>> +               const uint8_t *buf, const uint8_t *oob)
>>  {
>>        bf5xx_nand_write_buf(mtd, buf, mtd->writesize);
>>        bf5xx_nand_write_buf(mtd, chip->oob_poi, mtd->oobsize);
>> diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c
>> index 2973d97..e4103c5 100644
>> --- a/drivers/mtd/nand/cafe_nand.c
>> +++ b/drivers/mtd/nand/cafe_nand.c
>> @@ -375,12 +375,13 @@ static int cafe_nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
>>  * @mtd:       mtd info structure
>>  * @chip:      nand chip info structure
>>  * @buf:       buffer to store read data
>> + * @oob:       buffer to store OOB data
>>  *
>>  * The hw generator calculates the error syndrome automatically. Therefor
>>  * we need a special oob layout and handling.
>>  */
>>  static int cafe_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>> -                              uint8_t *buf, int page)
>> +                              uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        struct cafe_priv *cafe = mtd->priv;
>>
>> @@ -394,7 +395,7 @@ static int cafe_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>>        if (checkecc && cafe_readl(cafe, NAND_ECC_RESULT) & (1<<18)) {
>>                unsigned short syn[8], pat[4];
>>                int pos[4];
>> -               u8 *oob = chip->oob_poi;
>> +               u8 *oobbuf = chip->oob_poi;
>>                int i, n;
>>
>>                for (i=0; i<8; i+=2) {
>> @@ -422,14 +423,14 @@ static int cafe_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>>                                        buf[0] ^= pat[i];
>>                        } else if (p == 1365) {
>>                                buf[2047] ^= pat[i] >> 4;
>> -                               oob[0] ^= pat[i] << 4;
>> +                               oobbuf[0] ^= pat[i] << 4;
>>                        } else if (p > 1365) {
>>                                if ((p & 1) == 1) {
>> -                                       oob[3*p/2 - 2048] ^= pat[i] >> 4;
>> -                                       oob[3*p/2 - 2047] ^= pat[i] << 4;
>> +                                       oobbuf[3*p/2 - 2048] ^= pat[i] >> 4;
>> +                                       oobbuf[3*p/2 - 2047] ^= pat[i] << 4;
>>                                } else {
>> -                                       oob[3*p/2 - 2049] ^= pat[i] >> 8;
>> -                                       oob[3*p/2 - 2048] ^= pat[i];
>> +                                       oobbuf[3*p/2 - 2049] ^= pat[i] >> 8;
>> +                                       oobbuf[3*p/2 - 2048] ^= pat[i];
>>                                }
>>                        } else if ((p & 1) == 1) {
>>                                buf[3*p/2] ^= pat[i] >> 4;
>> @@ -518,7 +519,9 @@ static struct nand_bbt_descr cafe_bbt_mirror_descr_512 = {
>>
>>
>>  static void cafe_nand_write_page_lowlevel(struct mtd_info *mtd,
>> -                                         struct nand_chip *chip, const uint8_t *buf)
>> +                                         struct nand_chip *chip,
>> +                                         const uint8_t *buf,
>> +                                         const uint8_t *oob)
>>  {
>>        struct cafe_priv *cafe = mtd->priv;
>>
>> @@ -530,16 +533,17 @@ static void 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 page, int cached, int raw)
>> +                               const uint8_t *buf, const uint8_t *oob,
>> +                               int page, int cached, int raw)
>>  {
>>        int status;
>>
>>        chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
>>
>>        if (unlikely(raw))
>> -               chip->ecc.write_page_raw(mtd, chip, buf);
>> +               chip->ecc.write_page_raw(mtd, chip, buf, oob);
>>        else
>> -               chip->ecc.write_page(mtd, chip, buf);
>> +               chip->ecc.write_page(mtd, chip, buf, oob);
>>
>>        /*
>>         * Cached progamming disabled for now, Not sure if its worth the
>> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
>> index a1048c7..d078470 100644
>> --- a/drivers/mtd/nand/denali.c
>> +++ b/drivers/mtd/nand/denali.c
>> @@ -1084,7 +1084,7 @@ static void write_page(struct mtd_info *mtd, struct nand_chip *chip,
>>  * by write_page above.
>>  * */
>>  static void denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>> -                               const uint8_t *buf)
>> +                               const uint8_t *buf, const uint8_t *oob)
>>  {
>>        /* for regular page writes, we let HW handle all the ECC
>>         * data written to the device. */
>> @@ -1096,7 +1096,7 @@ static void denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>>  * write_page() function above.
>>  */
>>  static void denali_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>> -                                       const uint8_t *buf)
>> +                                       const uint8_t *buf, const uint8_t *oob)
>>  {
>>        /* for raw page writes, we want to disable ECC and simply write
>>           whatever data is in the buffer. */
>> @@ -1119,7 +1119,7 @@ static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
>>  }
>>
>>  static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>> -                           uint8_t *buf, int page)
>> +                           uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        struct denali_nand_info *denali = mtd_to_denali(mtd);
>>
>> @@ -1171,7 +1171,7 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>>  }
>>
>>  static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>> -                               uint8_t *buf, int page)
>> +                               uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        struct denali_nand_info *denali = mtd_to_denali(mtd);
>>
>> diff --git a/drivers/mtd/nand/docg4.c b/drivers/mtd/nand/docg4.c
>> index b082026..7ff99c9 100644
>> --- a/drivers/mtd/nand/docg4.c
>> +++ b/drivers/mtd/nand/docg4.c
>> @@ -786,13 +786,13 @@ static int read_page(struct mtd_info *mtd, struct nand_chip *nand,
>>
>>
>>  static int docg4_read_page_raw(struct mtd_info *mtd, struct nand_chip *nand,
>> -                              uint8_t *buf, int page)
>> +                              uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        return read_page(mtd, nand, buf, page, false);
>>  }
>>
>>  static int docg4_read_page(struct mtd_info *mtd, struct nand_chip *nand,
>> -                          uint8_t *buf, int page)
>> +                          uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        return read_page(mtd, nand, buf, page, true);
>>  }
>> @@ -952,13 +952,13 @@ static void write_page(struct mtd_info *mtd, struct nand_chip *nand,
>>  }
>>
>>  static void docg4_write_page_raw(struct mtd_info *mtd, struct nand_chip *nand,
>> -                                const uint8_t *buf)
>> +                                const uint8_t *buf, const uint8_t *oob)
>>  {
>>        return write_page(mtd, nand, buf, false);
>>  }
>>
>>  static void docg4_write_page(struct mtd_info *mtd, struct nand_chip *nand,
>> -                            const uint8_t *buf)
>> +                            const uint8_t *buf, const uint8_t *oob)
>>  {
>>        return write_page(mtd, nand, buf, true);
>>  }
>> @@ -1002,7 +1002,7 @@ static int __init read_factory_bbt(struct mtd_info *mtd)
>>                return -ENOMEM;
>>
>>        read_page_prologue(mtd, g4_addr);
>> -       status = docg4_read_page(mtd, nand, buf, DOCG4_FACTORY_BBT_PAGE);
>> +       status = docg4_read_page(mtd, nand, buf, NULL, DOCG4_FACTORY_BBT_PAGE);
>>        if (status)
>>                goto exit;
>>
>> @@ -1079,7 +1079,7 @@ static int docg4_block_markbad(struct mtd_info *mtd, loff_t ofs)
>>
>>        /* write first page of block */
>>        write_page_prologue(mtd, g4_addr);
>> -       docg4_write_page(mtd, nand, buf);
>> +       docg4_write_page(mtd, nand, buf, nand->oob_poi);
>>        ret = pageprog(mtd);
>>        if (!ret)
>>                mtd->ecc_stats.badblocks++;
>> diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
>> index 80b5264..b7d9bfd 100644
>> --- a/drivers/mtd/nand/fsl_elbc_nand.c
>> +++ b/drivers/mtd/nand/fsl_elbc_nand.c
>> @@ -740,7 +740,7 @@ static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
>>
>>  static int fsl_elbc_read_page(struct mtd_info *mtd,
>>                               struct nand_chip *chip,
>> -                             uint8_t *buf,
>> +                             uint8_t *buf, uint8_t *oob,
>>                              int page)
>>  {
>>        fsl_elbc_read_buf(mtd, buf, mtd->writesize);
>> @@ -755,9 +755,8 @@ static int fsl_elbc_read_page(struct mtd_info *mtd,
>>  /* ECC will be calculated automatically, and errors will be detected in
>>  * waitfunc.
>>  */
>> -static void fsl_elbc_write_page(struct mtd_info *mtd,
>> -                                struct nand_chip *chip,
>> -                                const uint8_t *buf)
>> +static void fsl_elbc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>> +                               const uint8_t *buf, const uint8_t *oob)
>>  {
>>        fsl_elbc_write_buf(mtd, buf, mtd->writesize);
>>        fsl_elbc_write_buf(mtd, chip->oob_poi, mtd->oobsize);
>> diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
>> index c30ac7b..f8a6e3e 100644
>> --- a/drivers/mtd/nand/fsl_ifc_nand.c
>> +++ b/drivers/mtd/nand/fsl_ifc_nand.c
>> @@ -684,7 +684,7 @@ static int fsl_ifc_wait(struct mtd_info *mtd, struct nand_chip *chip)
>>
>>  static int fsl_ifc_read_page(struct mtd_info *mtd,
>>                              struct nand_chip *chip,
>> -                             uint8_t *buf, int page)
>> +                             uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        struct fsl_ifc_mtd *priv = chip->priv;
>>        struct fsl_ifc_ctrl *ctrl = priv->ctrl;
>> @@ -706,7 +706,7 @@ static int fsl_ifc_read_page(struct mtd_info *mtd,
>>  */
>>  static void fsl_ifc_write_page(struct mtd_info *mtd,
>>                                struct nand_chip *chip,
>> -                               const uint8_t *buf)
>> +                               const uint8_t *buf, const uint8_t *oob)
>>  {
>>        fsl_ifc_write_buf(mtd, buf, mtd->writesize);
>>        fsl_ifc_write_buf(mtd, chip->oob_poi, mtd->oobsize);
>> diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
>> index 1b8330e..8bb731b 100644
>> --- a/drivers/mtd/nand/fsmc_nand.c
>> +++ b/drivers/mtd/nand/fsmc_nand.c
>> @@ -692,6 +692,7 @@ static void fsmc_write_buf_dma(struct mtd_info *mtd, const uint8_t *buf,
>>  * @mtd:       mtd info structure
>>  * @chip:      nand chip info structure
>>  * @buf:       buffer to store read data
>> + * @oob:       buffer to store read OOB data
>>  * @page:      page number to read
>>  *
>>  * This routine is needed for fsmc version 8 as reading from NAND chip has to be
>> @@ -701,7 +702,7 @@ static void fsmc_write_buf_dma(struct mtd_info *mtd, const uint8_t *buf,
>>  * max of 8 bits)
>>  */
>>  static int fsmc_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>> -                                uint8_t *buf, int page)
>> +                                uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        struct fsmc_nand_data *host = container_of(mtd,
>>                                        struct fsmc_nand_data, mtd);
>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> index 75b1dde..949c761 100644
>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> @@ -841,7 +841,7 @@ static void block_mark_swapping(struct gpmi_nand_data *this,
>>  }
>>
>>  static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>> -                               uint8_t *buf, int page)
>> +                               uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        struct gpmi_nand_data *this = chip->priv;
>>        struct bch_geometry *nfc_geo = &this->bch_geometry;
>> @@ -928,7 +928,8 @@ exit_nfc:
>>  }
>>
>>  static void gpmi_ecc_write_page(struct mtd_info *mtd,
>> -                               struct nand_chip *chip, const uint8_t *buf)
>> +                               struct nand_chip *chip, const uint8_t *buf,
>> +                               const uint8_t *oob)
>>  {
>>        struct gpmi_nand_data *this = chip->priv;
>>        struct bch_geometry *nfc_geo = &this->bch_geometry;
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index 47b19c0..95ba987 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -1066,12 +1066,13 @@ EXPORT_SYMBOL(nand_lock);
>>  * @mtd: mtd info structure
>>  * @chip: nand chip info structure
>>  * @buf: buffer to store read data
>> + * @oob: buffer to store read OOB data
>>  * @page: page number to read
>>  *
>>  * Not for syndrome calculating ECC controllers, which use a special oob layout.
>>  */
>>  static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>> -                             uint8_t *buf, int page)
>> +                             uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        chip->read_buf(mtd, buf, mtd->writesize);
>>        chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
>> @@ -1083,17 +1084,18 @@ static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>>  * @mtd: mtd info structure
>>  * @chip: nand chip info structure
>>  * @buf: buffer to store read data
>> + * @oob: buffer to store read OOB
>>  * @page: page number to read
>>  *
>>  * We need a special oob layout and handling even when OOB isn't used.
>>  */
>>  static int nand_read_page_raw_syndrome(struct mtd_info *mtd,
>>                                        struct nand_chip *chip,
>> -                                       uint8_t *buf, int page)
>> +                                       uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        int eccsize = chip->ecc.size;
>>        int eccbytes = chip->ecc.bytes;
>> -       uint8_t *oob = chip->oob_poi;
>> +       uint8_t *oobbuf = chip->oob_poi;
>>        int steps, size;
>>
>>        for (steps = chip->ecc.steps; steps > 0; steps--) {
>> @@ -1101,22 +1103,22 @@ static int nand_read_page_raw_syndrome(struct mtd_info *mtd,
>>                buf += eccsize;
>>
>>                if (chip->ecc.prepad) {
>> -                       chip->read_buf(mtd, oob, chip->ecc.prepad);
>> -                       oob += chip->ecc.prepad;
>> +                       chip->read_buf(mtd, oobbuf, chip->ecc.prepad);
>> +                       oobbuf += chip->ecc.prepad;
>>                }
>>
>> -               chip->read_buf(mtd, oob, eccbytes);
>> -               oob += eccbytes;
>> +               chip->read_buf(mtd, oobbuf, eccbytes);
>> +               oobbuf += eccbytes;
>>
>>                if (chip->ecc.postpad) {
>> -                       chip->read_buf(mtd, oob, chip->ecc.postpad);
>> -                       oob += chip->ecc.postpad;
>> +                       chip->read_buf(mtd, oobbuf, chip->ecc.postpad);
>> +                       oobbuf += chip->ecc.postpad;
>>                }
>>        }
>>
>> -       size = mtd->oobsize - (oob - chip->oob_poi);
>> +       size = mtd->oobsize - (oobbuf - chip->oob_poi);
>>        if (size)
>> -               chip->read_buf(mtd, oob, size);
>> +               chip->read_buf(mtd, oobbuf, size);
>>
>>        return 0;
>>  }
>> @@ -1129,7 +1131,7 @@ static int nand_read_page_raw_syndrome(struct mtd_info *mtd,
>>  * @page: page number to read
>>  */
>>  static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
>> -                               uint8_t *buf, int page)
>> +                               uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        int i, eccsize = chip->ecc.size;
>>        int eccbytes = chip->ecc.bytes;
>> @@ -1139,7 +1141,7 @@ static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
>>        uint8_t *ecc_code = chip->buffers->ecccode;
>>        uint32_t *eccpos = chip->ecc.layout->eccpos;
>>
>> -       chip->ecc.read_page_raw(mtd, chip, buf, page);
>> +       chip->ecc.read_page_raw(mtd, chip, buf, oob, page);
>>
>>        for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
>>                chip->ecc.calculate(mtd, p, &ecc_calc[i]);
>> @@ -1257,12 +1259,13 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
>>  * @mtd: mtd info structure
>>  * @chip: nand chip info structure
>>  * @buf: buffer to store read data
>> + * @oob: buffer to store read OOB
>>  * @page: page number to read
>>  *
>>  * Not for syndrome calculating ECC controllers which need a special oob layout.
>>  */
>>  static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>> -                               uint8_t *buf, int page)
>> +                               uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        int i, eccsize = chip->ecc.size;
>>        int eccbytes = chip->ecc.bytes;
>> @@ -1302,6 +1305,7 @@ static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>>  * @mtd: mtd info structure
>>  * @chip: nand chip info structure
>>  * @buf: buffer to store read data
>> + * @oob: buffer to store read OOB
>>  * @page: page number to read
>>  *
>>  * Hardware ECC for large page chips, require OOB to be read first. For this
>> @@ -1311,7 +1315,7 @@ static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>>  * the data area, by overwriting the NAND manufacturer bad block markings.
>>  */
>>  static int nand_read_page_hwecc_oob_first(struct mtd_info *mtd,
>> -       struct nand_chip *chip, uint8_t *buf, int page)
>> +       struct nand_chip *chip, uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        int i, eccsize = chip->ecc.size;
>>        int eccbytes = chip->ecc.bytes;
>> @@ -1350,19 +1354,20 @@ static int nand_read_page_hwecc_oob_first(struct mtd_info *mtd,
>>  * @mtd: mtd info structure
>>  * @chip: nand chip info structure
>>  * @buf: buffer to store read data
>> + * @oob: buffer to store read OOB
>>  * @page: page number to read
>>  *
>>  * The hw generator calculates the error syndrome automatically. Therefore we
>>  * need a special oob layout and handling.
>>  */
>>  static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
>> -                                  uint8_t *buf, int page)
>> +                                  uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        int i, eccsize = chip->ecc.size;
>>        int eccbytes = chip->ecc.bytes;
>>        int eccsteps = chip->ecc.steps;
>>        uint8_t *p = buf;
>> -       uint8_t *oob = chip->oob_poi;
>> +       uint8_t *oobbuf = chip->oob_poi;
>>
>>        for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
>>                int stat;
>> @@ -1371,23 +1376,23 @@ static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
>>                chip->read_buf(mtd, p, eccsize);
>>
>>                if (chip->ecc.prepad) {
>> -                       chip->read_buf(mtd, oob, chip->ecc.prepad);
>> -                       oob += chip->ecc.prepad;
>> +                       chip->read_buf(mtd, oobbuf, chip->ecc.prepad);
>> +                       oobbuf += chip->ecc.prepad;
>>                }
>>
>>                chip->ecc.hwctl(mtd, NAND_ECC_READSYN);
>> -               chip->read_buf(mtd, oob, eccbytes);
>> -               stat = chip->ecc.correct(mtd, p, oob, NULL);
>> +               chip->read_buf(mtd, oobbuf, eccbytes);
>> +               stat = chip->ecc.correct(mtd, p, oobbuf, NULL);
>>
>>                if (stat < 0)
>>                        mtd->ecc_stats.failed++;
>>                else
>>                        mtd->ecc_stats.corrected += stat;
>>
>> -               oob += eccbytes;
>> +               oobbuf += eccbytes;
>>
>>                if (chip->ecc.postpad) {
>> -                       chip->read_buf(mtd, oob, chip->ecc.postpad);
>> +                       chip->read_buf(mtd, oobbuf, chip->ecc.postpad);
>>                        oob += chip->ecc.postpad;
>>                }
>>        }
>> @@ -1500,14 +1505,14 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
>>
>>                        /* Now read the page into the buffer */
>>                        if (unlikely(ops->mode == MTD_OPS_RAW))
>> -                               ret = chip->ecc.read_page_raw(mtd, chip,
>> -                                                             bufpoi, page);
>> +                               ret = chip->ecc.read_page_raw(mtd, chip, bufpoi,
>> +                                                             NULL, page);
>>                        else if (!aligned && NAND_SUBPAGE_READ(chip) && !oob)
>>                                ret = chip->ecc.read_subpage(mtd, chip,
>>                                                        col, bytes, bufpoi);
>>                        else
>>                                ret = chip->ecc.read_page(mtd, chip, bufpoi,
>> -                                                         page);
>> +                                                         NULL, page);
>>                        if (ret < 0) {
>>                                if (!aligned)
>>                                        /* Invalidate page cache */
>> @@ -1919,11 +1924,12 @@ out:
>>  * @mtd: mtd info structure
>>  * @chip: nand chip info structure
>>  * @buf: data buffer
>> + * @oob: OOB buffer
>>  *
>>  * Not for syndrome calculating ECC controllers, which use a special oob layout.
>>  */
>>  static void nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>> -                               const uint8_t *buf)
>> +                               const uint8_t *buf, const uint8_t *oob)
>>  {
>>        chip->write_buf(mtd, buf, mtd->writesize);
>>        chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
>> @@ -1934,16 +1940,18 @@ static void nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>>  * @mtd: mtd info structure
>>  * @chip: nand chip info structure
>>  * @buf: data buffer
>> + * @oob: OOB buffer
>>  *
>>  * We need a special oob layout and handling even when ECC isn't checked.
>>  */
>>  static void nand_write_page_raw_syndrome(struct mtd_info *mtd,
>>                                        struct nand_chip *chip,
>> -                                       const uint8_t *buf)
>> +                                       const uint8_t *buf,
>> +                                       const uint8_t *oob)
>>  {
>>        int eccsize = chip->ecc.size;
>>        int eccbytes = chip->ecc.bytes;
>> -       uint8_t *oob = chip->oob_poi;
>> +       uint8_t *oobbuf = chip->oob_poi;
>>        int steps, size;
>>
>>        for (steps = chip->ecc.steps; steps > 0; steps--) {
>> @@ -1951,31 +1959,32 @@ static void nand_write_page_raw_syndrome(struct mtd_info *mtd,
>>                buf += eccsize;
>>
>>                if (chip->ecc.prepad) {
>> -                       chip->write_buf(mtd, oob, chip->ecc.prepad);
>> -                       oob += chip->ecc.prepad;
>> +                       chip->write_buf(mtd, oobbuf, chip->ecc.prepad);
>> +                       oobbuf += chip->ecc.prepad;
>>                }
>>
>> -               chip->read_buf(mtd, oob, eccbytes);
>> -               oob += eccbytes;
>> +               chip->read_buf(mtd, oobbuf, eccbytes);
>> +               oobbuf += eccbytes;
>>
>>                if (chip->ecc.postpad) {
>> -                       chip->write_buf(mtd, oob, chip->ecc.postpad);
>> -                       oob += chip->ecc.postpad;
>> +                       chip->write_buf(mtd, oobbuf, chip->ecc.postpad);
>> +                       oobbuf += chip->ecc.postpad;
>>                }
>>        }
>>
>> -       size = mtd->oobsize - (oob - chip->oob_poi);
>> +       size = mtd->oobsize - (oobbuf - chip->oob_poi);
>>        if (size)
>> -               chip->write_buf(mtd, oob, size);
>> +               chip->write_buf(mtd, oobbuf, size);
>>  }
>>  /**
>>  * nand_write_page_swecc - [REPLACEABLE] software ECC based page write function
>>  * @mtd: mtd info structure
>>  * @chip: nand chip info structure
>>  * @buf: data buffer
>> + * @oob: OOB buffer
>>  */
>>  static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
>> -                                 const uint8_t *buf)
>> +                                 const uint8_t *buf, const uint8_t *oob)
>>  {
>>        int i, eccsize = chip->ecc.size;
>>        int eccbytes = chip->ecc.bytes;
>> @@ -1991,7 +2000,7 @@ static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
>>        for (i = 0; i < chip->ecc.total; i++)
>>                chip->oob_poi[eccpos[i]] = ecc_calc[i];
>>
>> -       chip->ecc.write_page_raw(mtd, chip, buf);
>> +       chip->ecc.write_page_raw(mtd, chip, buf, oob);
>>  }
>>
>>  /**
>> @@ -1999,9 +2008,10 @@ static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
>>  * @mtd: mtd info structure
>>  * @chip: nand chip info structure
>>  * @buf: data buffer
>> + * @oob: OOB buffer
>>  */
>>  static void nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>> -                                 const uint8_t *buf)
>> +                                 const uint8_t *buf, const uint8_t *oob)
>>  {
>>        int i, eccsize = chip->ecc.size;
>>        int eccbytes = chip->ecc.bytes;
>> @@ -2027,18 +2037,20 @@ static void nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>>  * @mtd: mtd info structure
>>  * @chip: nand chip info structure
>>  * @buf: data buffer
>> + * @oob: OOB data buffer
>>  *
>>  * The hw generator calculates the error syndrome automatically. Therefore we
>>  * need a special oob layout and handling.
>>  */
>>  static void nand_write_page_syndrome(struct mtd_info *mtd,
>> -                                   struct nand_chip *chip, const uint8_t *buf)
>> +                                   struct nand_chip *chip, const uint8_t *buf,
>> +                                   const uint8_t *oob)
>>  {
>>        int i, eccsize = chip->ecc.size;
>>        int eccbytes = chip->ecc.bytes;
>>        int eccsteps = chip->ecc.steps;
>>        const uint8_t *p = buf;
>> -       uint8_t *oob = chip->oob_poi;
>> +       uint8_t *oobbuf = chip->oob_poi;
>>
>>        for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
>>
>> @@ -2046,24 +2058,24 @@ static void nand_write_page_syndrome(struct mtd_info *mtd,
>>                chip->write_buf(mtd, p, eccsize);
>>
>>                if (chip->ecc.prepad) {
>> -                       chip->write_buf(mtd, oob, chip->ecc.prepad);
>> -                       oob += chip->ecc.prepad;
>> +                       chip->write_buf(mtd, oobbuf, chip->ecc.prepad);
>> +                       oobbuf += chip->ecc.prepad;
>>                }
>>
>> -               chip->ecc.calculate(mtd, p, oob);
>> -               chip->write_buf(mtd, oob, eccbytes);
>> -               oob += eccbytes;
>> +               chip->ecc.calculate(mtd, p, oobbuf);
>> +               chip->write_buf(mtd, oobbuf, eccbytes);
>> +               oobbuf += eccbytes;
>>
>>                if (chip->ecc.postpad) {
>> -                       chip->write_buf(mtd, oob, chip->ecc.postpad);
>> -                       oob += chip->ecc.postpad;
>> +                       chip->write_buf(mtd, oobbuf, chip->ecc.postpad);
>> +                       oobbuf += chip->ecc.postpad;
>>                }
>>        }
>>
>>        /* Calculate remaining oob bytes */
>> -       i = mtd->oobsize - (oob - chip->oob_poi);
>> +       i = mtd->oobsize - (oobbuf - chip->oob_poi);
>>        if (i)
>> -               chip->write_buf(mtd, oob, i);
>> +               chip->write_buf(mtd, oobbuf, i);
>>  }
>>
>>  /**
>> @@ -2076,16 +2088,17 @@ static void 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 page, int cached, int raw)
>> +                          const uint8_t *buf, const uint8_t *oob, int page,
>> +                          int cached, int raw)
>>  {
>>        int status;
>>
>>        chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
>>
>>        if (unlikely(raw))
>> -               chip->ecc.write_page_raw(mtd, chip, buf);
>> +               chip->ecc.write_page_raw(mtd, chip, buf, oob);
>>        else
>> -               chip->ecc.write_page(mtd, chip, buf);
>> +               chip->ecc.write_page(mtd, chip, buf, oob);
>>
>>        /*
>>         * Cached progamming disabled for now. Not sure if it's worth the
>> @@ -2264,7 +2277,7 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
>>                        memset(chip->oob_poi, 0xff, mtd->oobsize);
>>                }
>>
>> -               ret = chip->write_page(mtd, chip, wbuf, page, cached,
>> +               ret = chip->write_page(mtd, chip, wbuf, NULL, page, cached,
>>                                       (ops->mode == MTD_OPS_RAW));
>>                if (ret)
>>                        break;
>> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
>> index def50ca..92d2eae 100644
>> --- a/drivers/mtd/nand/pxa3xx_nand.c
>> +++ b/drivers/mtd/nand/pxa3xx_nand.c
>> @@ -682,14 +682,14 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
>>  }
>>
>>  static void pxa3xx_nand_write_page_hwecc(struct mtd_info *mtd,
>> -               struct nand_chip *chip, const uint8_t *buf)
>> +               struct nand_chip *chip, const uint8_t *buf, const uint8_t *oob)
>>  {
>>        chip->write_buf(mtd, buf, mtd->writesize);
>>        chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
>>  }
>>
>>  static int pxa3xx_nand_read_page_hwecc(struct mtd_info *mtd,
>> -               struct nand_chip *chip, uint8_t *buf, int page)
>> +               struct nand_chip *chip, uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        struct pxa3xx_nand_host *host = mtd->priv;
>>        struct pxa3xx_nand_info *info = host->info_data;
>> diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
>> index e9b2b26..d1b2731 100644
>> --- a/drivers/mtd/nand/sh_flctl.c
>> +++ b/drivers/mtd/nand/sh_flctl.c
>> @@ -344,7 +344,7 @@ static void set_cmd_regs(struct mtd_info *mtd, uint32_t cmd, uint32_t flcmcdr_va
>>  }
>>
>>  static int flctl_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>> -                               uint8_t *buf, int page)
>> +                               uint8_t *buf, uint8_t *oob, int page)
>>  {
>>        int i, eccsize = chip->ecc.size;
>>        int eccbytes = chip->ecc.bytes;
>> @@ -366,7 +366,7 @@ static int flctl_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>>  }
>>
>>  static void flctl_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>> -                                  const uint8_t *buf)
>> +                                  const uint8_t *buf, const uint8_t *oob)
>>  {
>>        int i, eccsize = chip->ecc.size;
>>        int eccbytes = chip->ecc.bytes;
>> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
>> index 1482340..be9ee1f 100644
>> --- a/include/linux/mtd/nand.h
>> +++ b/include/linux/mtd/nand.h
>> @@ -363,15 +363,15 @@ struct nand_ecc_ctrl {
>>        int (*correct)(struct mtd_info *mtd, uint8_t *dat, uint8_t *read_ecc,
>>                        uint8_t *calc_ecc);
>>        int (*read_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
>> -                       uint8_t *buf, int page);
>> +                       uint8_t *buf, uint8_t *oob, int page);
>>        void (*write_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
>> -                       const uint8_t *buf);
>> +                       const uint8_t *buf, const uint8_t *oob);
>>        int (*read_page)(struct mtd_info *mtd, struct nand_chip *chip,
>> -                       uint8_t *buf, int page);
>> +                       uint8_t *buf, uint8_t *oob, int page);
>>        int (*read_subpage)(struct mtd_info *mtd, struct nand_chip *chip,
>>                        uint32_t offs, uint32_t len, uint8_t *buf);
>>        void (*write_page)(struct mtd_info *mtd, struct nand_chip *chip,
>> -                       const uint8_t *buf);
>> +                       const uint8_t *buf, const uint8_t *oob);
>>        int (*write_oob_raw)(struct mtd_info *mtd, struct nand_chip *chip,
>>                        int page);
>>        int (*read_oob_raw)(struct mtd_info *mtd, struct nand_chip *chip,
>> @@ -505,7 +505,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 page, int cached, int raw);
>> +                       const uint8_t *buf, const uint8_t *oob, int page,
>> +                       int cached, int raw);
>>
>>        int chip_delay;
>>        unsigned int options;
>> --
>> 1.7.5.4.2.g519b1
>>
Brian Norris April 18, 2012, 4:22 p.m. UTC | #7
Hi Bastian,

On 4/18/2012 2:37 AM, Bastian Hecht wrote:
> 2012/4/18 Bastian Hecht<hechtb@googlemail.com>:
>> I'm currently working on the hardware ECC part of the SH Mobile flctl
>> and I like the patchset as it makes things more explicit when OOB data
>> is requested or not and such things. It's cleaner and better, thanks!

Glad it will help! It's good to know other drivers will see a benefit.

> To be a bit more of a concrete help, if you can give me a use case to
> test the oob reads/writes I can modify my driver to work with your
> patches. Right now I rely on nandwrite, nanddump, nandtest, the kernel
> test modules and ubi. With none of them I produced an error so far
> although the current implementation of the hardware ECC completely
> ignores OOB data other than ECC.

Well, for me the big "use case" is that I can support my new DMA 
controller (that doesn't read/write OOB) without the rest of MTD/NAND 
choking on anything or returning junk data. So I mostly make sure that 
nanddump/nandwrite work both with and without OOB (the -o flag) or ECC 
(the -n flag); that the kernel test modules work; and that UBI/UBIFS 
work on top of MTD.

I don't know if there are any more specific tests you would need to run; 
are you looking for something else?

Brian
Bastian Hecht April 19, 2012, 9:26 a.m. UTC | #8
Hi Brian,

2012/4/18 Brian Norris <computersforpeace@gmail.com>:
> Hi Bastian,
>
> On 4/18/2012 2:37 AM, Bastian Hecht wrote:
>>
>> 2012/4/18 Bastian Hecht<hechtb@googlemail.com>:
>>
>>> I'm currently working on the hardware ECC part of the SH Mobile flctl
>>> and I like the patchset as it makes things more explicit when OOB data
>>> is requested or not and such things. It's cleaner and better, thanks!
>
>
> Glad it will help! It's good to know other drivers will see a benefit.
>
>
>> To be a bit more of a concrete help, if you can give me a use case to
>> test the oob reads/writes I can modify my driver to work with your
>> patches. Right now I rely on nandwrite, nanddump, nandtest, the kernel
>> test modules and ubi. With none of them I produced an error so far
>> although the current implementation of the hardware ECC completely
>> ignores OOB data other than ECC.
>
>
> Well, for me the big "use case" is that I can support my new DMA controller
> (that doesn't read/write OOB) without the rest of MTD/NAND choking on
> anything or returning junk data. So I mostly make sure that
> nanddump/nandwrite work both with and without OOB (the -o flag) or ECC (the
> -n flag); that the kernel test modules work; and that UBI/UBIFS work on top
> of MTD.

Ah very nice! You made me look at the nandwrite -o thing once again. I
used it before without success, there was no write at all when I
issued it, but now ( - I compiled the git version of mtd-utils and
replaced the debian prehistoric prepackaged one with it - ) it works!
Now I can finish my work on the flctl ECC part. Thanks!

> I don't know if there are any more specific tests you would need to run; are
> you looking for something else?

Nope, thanks.

cheers,

 Bastian Hecht


> Brian
Mike Dunn April 19, 2012, 4:50 p.m. UTC | #9
Hi Brian,

On 04/17/2012 08:44 PM, Brian Norris wrote:

> Now, in future revisions of this ASIC, it may be possible to access
> OOB via DMA as well, but even if this happens, it doesn't make a lot
> of sense on this hardware to *always* pull OOB data. 


No, it doesn't.  In fact, I'm not aware of any code within or on top of mtd that
does anything with the oob data when a page is read.  If oob is needed,
mtd_read_oob() is used.  Coincidentally, I recxently discovered that my docg4
driver is technically broken because I don't fill the chip->oob_poi buffer when
I read a page, but it never caused a problem with UBI/ubifs.  And the mtdutils
are fine because mtdchar requires use of an ioctl for oob access, and the
handler for this ioctl uses mtd_read_oob().


> As mentioned
> previously, most normal applications (i.e., UBI(FS)) don't need to
> access this OOB data at all, so it seems silly to go to extra work to
> have the DMA controller return it to the MTD/NAND layers. I'm not
> familiar with the OOB/ECC schemes on enough other hardware to
> determine whether other drivers could make use of this same
> optimization. It would require hardware with internal buffers for
> error correction and an access mode that easily returns page data
> only...


The Freescale nand controllers might fall into this category.  Hardware handles
error detction *and* correction, so there's no need to read the oob at all if
it's not needed.  And fsl_ifc_nand was just mainlined, BTW.

Thanks,
Mike
Brian Norris April 19, 2012, 10:06 p.m. UTC | #10
Hi Mike,

On Thu, Apr 19, 2012 at 9:50 AM, Mike Dunn <mikedunn@newsguy.com> wrote:
> On 04/17/2012 08:44 PM, Brian Norris wrote:
>
>> Now, in future revisions of this ASIC, it may be possible to access
>> OOB via DMA as well, but even if this happens, it doesn't make a lot
>> of sense on this hardware to *always* pull OOB data.
>
>
> No, it doesn't.  In fact, I'm not aware of any code within or on top of mtd that
> does anything with the oob data when a page is read.  If oob is needed,
> mtd_read_oob() is used.

I guess this may not be an issue for page read, but I know one use for
write_page data+OOB. MLC NAND, for instance, requires that you write
*once* to a page, so I introduced ioctl(MEMWRITE) which generically
allows page, OOB, or both to be written. This trickles down to the
nand_ecc_ctrl.write_page function, I think. There are probably other
cases that I'm not really thinking of right now.

> Coincidentally, I recxently discovered that my docg4
> driver is technically broken because I don't fill the chip->oob_poi buffer when
> I read a page, but it never caused a problem with UBI/ubifs.

If, as you claim, chip->oob_poi is never used on page read, then why
do you claim this is "broken" for docg4? (Note that I am not 100% sure
of your claim. There are *potential* users through the mtd_read_oob()
API, which can specify both oobbuf and datbuf.)

> And the mtdutils
> are fine because mtdchar requires use of an ioctl for oob access, and the
> handler for this ioctl uses mtd_read_oob().
>
>> As mentioned
>> previously, most normal applications (i.e., UBI(FS)) don't need to
>> access this OOB data at all, so it seems silly to go to extra work to
>> have the DMA controller return it to the MTD/NAND layers. I'm not
>> familiar with the OOB/ECC schemes on enough other hardware to
>> determine whether other drivers could make use of this same
>> optimization. It would require hardware with internal buffers for
>> error correction and an access mode that easily returns page data
>> only...
>
>
> The Freescale nand controllers might fall into this category.  Hardware handles
> error detction *and* correction, so there's no need to read the oob at all if
> it's not needed.  And fsl_ifc_nand was just mainlined, BTW.

Right, I noticed the new driver (merge in 3.4-rc, right?) but didn't
study the details. From your description, it sounds like it could use
this change. CC'ing Prabhakar, author of fsl_ifc_nand.c.

And have you seen the thread (on patch 2/2) in which Shmulik suggests
using a boolean (has_oob) argument instead of a buffer (oob) argument?
Unless there are objections, I plan to rewrite v2 under his
suggestion.

Thanks for your thoughts!

Brian

P.S. It seems, Mike, like you dropped the CC list. This may or may not
be intentional, but either way I suppose this discussion isn't
particularly important for all the CC'd members...
Jon Povey April 20, 2012, 1:10 a.m. UTC | #11
linux-mtd-bounces@lists.infradead.org wrote:
> {read, write}_page interfaces
>
> Hi Mike,
>
> On Thu, Apr 19, 2012 at 9:50 AM, Mike Dunn
> <mikedunn@newsguy.com> wrote:
>> On 04/17/2012 08:44 PM, Brian Norris wrote:
>>
>>> Now, in future revisions of this ASIC, it may be possible to access
>>> OOB via DMA as well, but even if this happens, it doesn't make a lot
>>> of sense on this hardware to *always* pull OOB data.
>>
>>
>> No, it doesn't.  In fact, I'm not aware of any code within or on top
>> of mtd that does anything with the oob data when a page is read.  If
>> oob is needed, mtd_read_oob() is used.
>
> I guess this may not be an issue for page read, but I know one use for
> write_page data+OOB. MLC NAND, for instance, requires that you write
> *once* to a page, so I introduced ioctl(MEMWRITE) which generically
> allows page, OOB, or both to be written. This trickles down to the
> nand_ecc_ctrl.write_page function, I think. There are probably other
> cases that I'm not really thinking of right now.

Ability to write raw OOB including userland software-generated ECC is
needed for Linux to be able to update the bootloader on certain TI
DaVinci chips at least. This is because the ROM bootloader in the chip
has a wacky/bugged idea of OOB layout.

If you can do that you can also write MTD partitions in a different layout
from the one you are supporting, e.g. as part of a firmware update to a
kernel with a different filesystem and OOB layout. I have used this to
good effect.

I can think of a slightly obscure use for reading OOB on these TI chips:
Once booted to Linux, you could read and check in software the ECC for
the ROM bootloader looking for bitflips, and rewrite the bootloader if
it was starting to degrade. The bootloader has no way to rewrite or report
ECC corrections, you see. I have not done this myself.

In theory nobody should ever need to worry about OOB+ECC but being able
to fiddle with it has its uses and I hope that ability doesn't go away.

--
Jon Povey
jon.povey@racelogic.co.uk

Racelogic is a limited company registered in England. Registered number 2743719 .
Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB .

The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network
Mike Dunn April 20, 2012, 4:17 p.m. UTC | #12
Hi Brian,

On 04/19/2012 03:06 PM, Brian Norris wrote:
> Hi Mike,
> 
> On Thu, Apr 19, 2012 at 9:50 AM, Mike Dunn <mikedunn@newsguy.com> wrote:
>> On 04/17/2012 08:44 PM, Brian Norris wrote:
>>
>>> Now, in future revisions of this ASIC, it may be possible to access
>>> OOB via DMA as well, but even if this happens, it doesn't make a lot
>>> of sense on this hardware to *always* pull OOB data.
>>
>>
>> No, it doesn't.  In fact, I'm not aware of any code within or on top of mtd that
>> does anything with the oob data when a page is read.  If oob is needed,
>> mtd_read_oob() is used.
> 
> I guess this may not be an issue for page read, but I know one use for
> write_page data+OOB. MLC NAND, for instance, requires that you write
> *once* to a page, so I introduced ioctl(MEMWRITE) which generically
> allows page, OOB, or both to be written. This trickles down to the
> nand_ecc_ctrl.write_page function, I think. There are probably other
> cases that I'm not really thinking of right now.


I was just thinking of read.


> 
>> Coincidentally, I recxently discovered that my docg4
>> driver is technically broken because I don't fill the chip->oob_poi buffer when
>> I read a page, but it never caused a problem with UBI/ubifs.
> 
> If, as you claim, chip->oob_poi is never used on page read, then why
> do you claim this is "broken" for docg4? (Note that I am not 100% sure
> of your claim. There are *potential* users through the mtd_read_oob()
> API, which can specify both oobbuf and datbuf.)


Broken because it's *supposed* to fill it, creating the *potential* users, as
you point out.  My point was just to illustrate that you are correct: the need
to read oob along with the page data need not be assumed.


> 
>> And the mtdutils
>> are fine because mtdchar requires use of an ioctl for oob access, and the
>> handler for this ioctl uses mtd_read_oob().
>>
>>> As mentioned
>>> previously, most normal applications (i.e., UBI(FS)) don't need to
>>> access this OOB data at all, so it seems silly to go to extra work to
>>> have the DMA controller return it to the MTD/NAND layers. I'm not
>>> familiar with the OOB/ECC schemes on enough other hardware to
>>> determine whether other drivers could make use of this same
>>> optimization. It would require hardware with internal buffers for
>>> error correction and an access mode that easily returns page data
>>> only...
>>
>>
>> The Freescale nand controllers might fall into this category.  Hardware handles
>> error detction *and* correction, so there's no need to read the oob at all if
>> it's not needed.  And fsl_ifc_nand was just mainlined, BTW.
> 
> Right, I noticed the new driver (merge in 3.4-rc, right?) but didn't
> study the details. From your description, it sounds like it could use
> this change. CC'ing Prabhakar, author of fsl_ifc_nand.c.
> 
> And have you seen the thread (on patch 2/2) in which Shmulik suggests
> using a boolean (has_oob) argument instead of a buffer (oob) argument?
> Unless there are objections, I plan to rewrite v2 under his
> suggestion.


Yes (after sending my post).  I think it's a very good suggestion.  Keeps the
code from becoming even more confusing than it already is.


> 
> Thanks for your thoughts!
> 
> Brian
> 
> P.S. It seems, Mike, like you dropped the CC list. This may or may not
> be intentional, but either way I suppose this discussion isn't
> particularly important for all the CC'd members...


Done because my smtp server has a recipient limit of 20 :(  Otherwise I would
have "replied all".

Thanks,
Mike
Mike Dunn April 20, 2012, 4:25 p.m. UTC | #13
On 04/19/2012 06:10 PM, Jon Povey wrote:
> 
> In theory nobody should ever need to worry about OOB+ECC but being able
> to fiddle with it has its uses and I hope that ability doesn't go away.


I think we're just trying to figure out how to make it optional.


Thanks,
Mike
Brian Norris April 20, 2012, 7:19 p.m. UTC | #14
On Fri, Apr 20, 2012 at 9:25 AM, Mike Dunn <mikedunn@newsguy.com> wrote:
> On 04/19/2012 06:10 PM, Jon Povey wrote:
>>
>> In theory nobody should ever need to worry about OOB+ECC but being able
>> to fiddle with it has its uses and I hope that ability doesn't go away.
>
>
> I think we're just trying to figure out how to make it optional.

Right, optional.

Brian
Shmulik Ladkani April 22, 2012, 7:58 a.m. UTC | #15
On Thu, 19 Apr 2012 15:06:33 -0700 Brian Norris <computersforpeace@gmail.com> wrote:
> On Thu, Apr 19, 2012 at 9:50 AM, Mike Dunn <mikedunn@newsguy.com> wrote:
> > On 04/17/2012 08:44 PM, Brian Norris wrote:
> >
> >> Now, in future revisions of this ASIC, it may be possible to access
> >> OOB via DMA as well, but even if this happens, it doesn't make a lot
> >> of sense on this hardware to *always* pull OOB data.
> >
> >
> > No, it doesn't. In fact, I'm not aware of any code within or on top of mtd that
> > does anything with the oob data when a page is read.  If oob is needed,
> > mtd_read_oob() is used.
> 
> I guess this may not be an issue for page read, but I know one use for
> write_page data+OOB. MLC NAND, for instance, requires that you write
> *once* to a page, so I introduced ioctl(MEMWRITE) which generically
> allows page, OOB, or both to be written. This trickles down to the
> nand_ecc_ctrl.write_page function, I think. There are probably other
> cases that I'm not really thinking of right now.

From MTD user's perspective, mtd_read() and mtd_write() interfaces
do not expose the OOB to the user at all.

OTHO, mtd_read_oob() and mtd_write_oob() allow the user to read/write
either the OOB on its own (NULL 'ops->datbuf'), or the page data along
with its accompanying OOB (non-null 'ops->datbuf' and 'ops->oobbuf').

There are few users of the mtd_read_oob/mtd_write_oob interfaces that
provide a non-null 'ops->datbuf':
- nand_bbt.c
- MEMWRITE mtdchar ioctl
- mtdswap.c
- sm_ftl.c
- yaffs2 (out-of-tree)

Regards,
Shmulik
Bastian Hecht April 23, 2012, 9:14 a.m. UTC | #16
2012/4/22 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> On Thu, 19 Apr 2012 15:06:33 -0700 Brian Norris <computersforpeace@gmail.com> wrote:
>> On Thu, Apr 19, 2012 at 9:50 AM, Mike Dunn <mikedunn@newsguy.com> wrote:
>> > On 04/17/2012 08:44 PM, Brian Norris wrote:
>> I guess this may not be an issue for page read, but I know one use for
>> write_page data+OOB. MLC NAND, for instance, requires that you write
>> *once* to a page, so I introduced ioctl(MEMWRITE) which generically
>> allows page, OOB, or both to be written. This trickles down to the
>> nand_ecc_ctrl.write_page function, I think. There are probably other
>> cases that I'm not really thinking of right now.
>
> From MTD user's perspective, mtd_read() and mtd_write() interfaces
> do not expose the OOB to the user at all.
>
> OTHO, mtd_read_oob() and mtd_write_oob() allow the user to read/write
> either the OOB on its own (NULL 'ops->datbuf'), or the page data along
> with its accompanying OOB (non-null 'ops->datbuf' and 'ops->oobbuf').
>
> There are few users of the mtd_read_oob/mtd_write_oob interfaces that
> provide a non-null 'ops->datbuf':
> - nand_bbt.c
> - MEMWRITE mtdchar ioctl
> - mtdswap.c
> - sm_ftl.c
> - yaffs2 (out-of-tree)

Following this thread, I wondered how mtd_write_oob is meant to work.
The sh_flctl code simply writes the OOB data 1 to 1 into the oob area
of the nand chip when using hardware ECC. So when reading this, it
should result in a corrupted page. Is the driver meant to OR the ECC
code to the buffer? Then the driver would need a page read and write
it back applying the oob data, no?

Best regards,

 Bastian Hecht

> Regards,
> Shmulik
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
Mike Dunn April 23, 2012, 5:14 p.m. UTC | #17
On 04/23/2012 02:14 AM, Bastian Hecht wrote:
> 
> Following this thread, I wondered how mtd_write_oob is meant to work.
> The sh_flctl code simply writes the OOB data 1 to 1 into the oob area
> of the nand chip when using hardware ECC. So when reading this, it
> should result in a corrupted page. Is the driver meant to OR the ECC
> code to the buffer? Then the driver would need a page read and write
> it back applying the oob data, no?


Good question.  I would guess that one-to-one placement is probably only
appropriate for MTD_OPS_RAW.

Mike
Shmulik Ladkani April 24, 2012, 6:02 a.m. UTC | #18
Hi Bastian,

On Mon, 23 Apr 2012 11:14:39 +0200 Bastian Hecht <hechtb@googlemail.com> wrote:
> Following this thread, I wondered how mtd_write_oob is meant to work.
> The sh_flctl code simply writes the OOB data 1 to 1 into the oob area
> of the nand chip when using hardware ECC. So when reading this, it
> should result in a corrupted page. Is the driver meant to OR the ECC
> code to the buffer? Then the driver would need a page read and write
> it back applying the oob data, no?

The beahvior is dependent on mtd_oob_ops->mode.

If mode is MTD_OPS_AUTO_OOB, 'nand_fill_oob' places the user provided
'oobbuf' into the "free" locations within the OOB layout (as defined by
'chip->ecc.layout->oobfree').
Implementor of 'chip->ecc.write_page' is expected to take care of ECC
caclulation and fill the ECC bytes (driver's or HW responsibility).

If the mode is MTD_OPS_RAW, user provided 'oobbuf' should be transferred
as-is, without error correction ('chip->ecc.write_page_raw' will be
invoked).

If the mode is MTD_OPS_PLACE_OOB, then 'nand_fill_oob' copies user's
'oobbuf' onto 'oob_poi'.
As with MTD_OPS_AUTO_OOB, implementor of 'chip->ecc.write_page' is
expected to take care of ECC calculation; the ECC locations are assumed
to be overwritten (ignoring user's bytes at the ECC locations).

This is according to the nand_base.c default methods.
For example, you can follow nand_do_write_ops, nand_write_page,
nand_write_page_swecc.

Regards,
Shmulik
Bastian Hecht April 25, 2012, 1:17 p.m. UTC | #19
2012/4/24 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> Hi Bastian,
>
> On Mon, 23 Apr 2012 11:14:39 +0200 Bastian Hecht <hechtb@googlemail.com> wrote:
>> Following this thread, I wondered how mtd_write_oob is meant to work.
>> The sh_flctl code simply writes the OOB data 1 to 1 into the oob area
>> of the nand chip when using hardware ECC. So when reading this, it
>> should result in a corrupted page. Is the driver meant to OR the ECC
>> code to the buffer? Then the driver would need a page read and write
>> it back applying the oob data, no?

Hello Schmulik,

> The beahvior is dependent on mtd_oob_ops->mode.
>
> If mode is MTD_OPS_AUTO_OOB, 'nand_fill_oob' places the user provided
> 'oobbuf' into the "free" locations within the OOB layout (as defined by
> 'chip->ecc.layout->oobfree').
> Implementor of 'chip->ecc.write_page' is expected to take care of ECC
> caclulation and fill the ECC bytes (driver's or HW responsibility).
>
> If the mode is MTD_OPS_RAW, user provided 'oobbuf' should be transferred
> as-is, without error correction ('chip->ecc.write_page_raw' will be
> invoked).

Ahaa. Thanks for elaborating this, now I understand how things are
supposed to work. You helped me to catch a flaw in the code, thanks! I
have to introduce a flag or similar to turn off the on-the-fly ECC
generation when the _raw functions are used.

Best regards,

 Bastian Hecht

> If the mode is MTD_OPS_PLACE_OOB, then 'nand_fill_oob' copies user's
> 'oobbuf' onto 'oob_poi'.
> As with MTD_OPS_AUTO_OOB, implementor of 'chip->ecc.write_page' is
> expected to take care of ECC calculation; the ECC locations are assumed
> to be overwritten (ignoring user's bytes at the ECC locations).
>
> This is according to the nand_base.c default methods.
> For example, you can follow nand_do_write_ops, nand_write_page,
> nand_write_page_swecc.
>
> Regards,
> Shmulik
diff mbox

Patch

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 2165576..95838a6 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -324,18 +324,21 @@  static int atmel_nand_calculate(struct mtd_info *mtd,
  * mtd:        mtd info structure
  * chip:       nand chip info structure
  * buf:        buffer to store read data
+ * oob:        buffer to store read OOB data
  */
 static int atmel_nand_read_page(struct mtd_info *mtd,
-		struct nand_chip *chip, uint8_t *buf, int page)
+		struct nand_chip *chip, uint8_t *buf, uint8_t *oob, int page)
 {
 	int eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
 	uint32_t *eccpos = chip->ecc.layout->eccpos;
 	uint8_t *p = buf;
-	uint8_t *oob = chip->oob_poi;
 	uint8_t *ecc_pos;
 	int stat;
 
+	if (!oob)
+		oob = chip->oob_poi;
+
 	/*
 	 * Errata: ALE is incorrectly wired up to the ECC controller
 	 * on the AP7000, so it will include the address cycles in the
diff --git a/drivers/mtd/nand/bcm_umi_bch.c b/drivers/mtd/nand/bcm_umi_bch.c
index a930666..3f4e845 100644
--- a/drivers/mtd/nand/bcm_umi_bch.c
+++ b/drivers/mtd/nand/bcm_umi_bch.c
@@ -22,9 +22,9 @@ 
 
 /* ---- Private Function Prototypes -------------------------------------- */
 static int bcm_umi_bch_read_page_hwecc(struct mtd_info *mtd,
-	struct nand_chip *chip, uint8_t *buf, int page);
+	struct nand_chip *chip, uint8_t *buf, uint8_t *oob, int page);
 static void bcm_umi_bch_write_page_hwecc(struct mtd_info *mtd,
-	struct nand_chip *chip, const uint8_t *buf);
+	struct nand_chip *chip, const uint8_t *buf, const uint8_t *oob);
 
 /* ---- Private Variables ------------------------------------------------ */
 
@@ -103,11 +103,12 @@  static struct nand_ecclayout nand_hw_eccoob_4096 = {
 *  @mtd:	mtd info structure
 *  @chip:	nand chip info structure
 *  @buf:	buffer to store read data
+*  @oob:	buffer to store read OOB data
 *
 ***************************************************************************/
 static int bcm_umi_bch_read_page_hwecc(struct mtd_info *mtd,
 				       struct nand_chip *chip, uint8_t * buf,
-						 int page)
+				       uint8_t *oob, int page)
 {
 	int sectorIdx = 0;
 	int eccsize = chip->ecc.size;
@@ -188,10 +189,11 @@  static int bcm_umi_bch_read_page_hwecc(struct mtd_info *mtd,
 *  @mtd:	mtd info structure
 *  @chip:	nand chip info structure
 *  @buf:	data buffer
+*  @oob:	OOB buffer
 *
 ***************************************************************************/
 static void bcm_umi_bch_write_page_hwecc(struct mtd_info *mtd,
-	struct nand_chip *chip, const uint8_t *buf)
+	struct nand_chip *chip, const uint8_t *buf, const uint8_t *oob)
 {
 	int sectorIdx = 0;
 	int eccsize = chip->ecc.size;
diff --git a/drivers/mtd/nand/bcm_umi_nand.c b/drivers/mtd/nand/bcm_umi_nand.c
index 6908cdd..ac7a027 100644
--- a/drivers/mtd/nand/bcm_umi_nand.c
+++ b/drivers/mtd/nand/bcm_umi_nand.c
@@ -341,7 +341,7 @@  static int bcm_umi_nand_verify_buf(struct mtd_info *mtd, const u_char * buf,
 	 * for MLC parts which may have permanently stuck bits.
 	 */
 	struct nand_chip *chip = mtd->priv;
-	int ret = chip->ecc.read_page(mtd, chip, readbackbuf, 0);
+	int ret = chip->ecc.read_page(mtd, chip, readbackbuf, NULL, 0);
 	if (ret < 0)
 		return -EFAULT;
 	else {
diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
index d7b86b9..d25ab1a 100644
--- a/drivers/mtd/nand/bf5xx_nand.c
+++ b/drivers/mtd/nand/bf5xx_nand.c
@@ -558,7 +558,7 @@  static void bf5xx_nand_dma_write_buf(struct mtd_info *mtd,
 }
 
 static int bf5xx_nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-		uint8_t *buf, int page)
+		uint8_t *buf, uint8_t *oob, int page)
 {
 	bf5xx_nand_read_buf(mtd, buf, mtd->writesize);
 	bf5xx_nand_read_buf(mtd, chip->oob_poi, mtd->oobsize);
@@ -567,7 +567,7 @@  static int bf5xx_nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip
 }
 
 static void bf5xx_nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-		const uint8_t *buf)
+		const uint8_t *buf, const uint8_t *oob)
 {
 	bf5xx_nand_write_buf(mtd, buf, mtd->writesize);
 	bf5xx_nand_write_buf(mtd, chip->oob_poi, mtd->oobsize);
diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c
index 2973d97..e4103c5 100644
--- a/drivers/mtd/nand/cafe_nand.c
+++ b/drivers/mtd/nand/cafe_nand.c
@@ -375,12 +375,13 @@  static int cafe_nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd:	mtd info structure
  * @chip:	nand chip info structure
  * @buf:	buffer to store read data
+ * @oob:	buffer to store OOB data
  *
  * The hw generator calculates the error syndrome automatically. Therefor
  * we need a special oob layout and handling.
  */
 static int cafe_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
-			       uint8_t *buf, int page)
+			       uint8_t *buf, uint8_t *oob, int page)
 {
 	struct cafe_priv *cafe = mtd->priv;
 
@@ -394,7 +395,7 @@  static int cafe_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	if (checkecc && cafe_readl(cafe, NAND_ECC_RESULT) & (1<<18)) {
 		unsigned short syn[8], pat[4];
 		int pos[4];
-		u8 *oob = chip->oob_poi;
+		u8 *oobbuf = chip->oob_poi;
 		int i, n;
 
 		for (i=0; i<8; i+=2) {
@@ -422,14 +423,14 @@  static int cafe_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 					buf[0] ^= pat[i];
 			} else if (p == 1365) {
 				buf[2047] ^= pat[i] >> 4;
-				oob[0] ^= pat[i] << 4;
+				oobbuf[0] ^= pat[i] << 4;
 			} else if (p > 1365) {
 				if ((p & 1) == 1) {
-					oob[3*p/2 - 2048] ^= pat[i] >> 4;
-					oob[3*p/2 - 2047] ^= pat[i] << 4;
+					oobbuf[3*p/2 - 2048] ^= pat[i] >> 4;
+					oobbuf[3*p/2 - 2047] ^= pat[i] << 4;
 				} else {
-					oob[3*p/2 - 2049] ^= pat[i] >> 8;
-					oob[3*p/2 - 2048] ^= pat[i];
+					oobbuf[3*p/2 - 2049] ^= pat[i] >> 8;
+					oobbuf[3*p/2 - 2048] ^= pat[i];
 				}
 			} else if ((p & 1) == 1) {
 				buf[3*p/2] ^= pat[i] >> 4;
@@ -518,7 +519,9 @@  static struct nand_bbt_descr cafe_bbt_mirror_descr_512 = {
 
 
 static void cafe_nand_write_page_lowlevel(struct mtd_info *mtd,
-					  struct nand_chip *chip, const uint8_t *buf)
+					  struct nand_chip *chip,
+					  const uint8_t *buf,
+					  const uint8_t *oob)
 {
 	struct cafe_priv *cafe = mtd->priv;
 
@@ -530,16 +533,17 @@  static void 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 page, int cached, int raw)
+				const uint8_t *buf, const uint8_t *oob,
+				int page, int cached, int raw)
 {
 	int status;
 
 	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
 
 	if (unlikely(raw))
-		chip->ecc.write_page_raw(mtd, chip, buf);
+		chip->ecc.write_page_raw(mtd, chip, buf, oob);
 	else
-		chip->ecc.write_page(mtd, chip, buf);
+		chip->ecc.write_page(mtd, chip, buf, oob);
 
 	/*
 	 * Cached progamming disabled for now, Not sure if its worth the
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index a1048c7..d078470 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1084,7 +1084,7 @@  static void write_page(struct mtd_info *mtd, struct nand_chip *chip,
  * by write_page above.
  * */
 static void denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
-				const uint8_t *buf)
+				const uint8_t *buf, const uint8_t *oob)
 {
 	/* for regular page writes, we let HW handle all the ECC
 	 * data written to the device. */
@@ -1096,7 +1096,7 @@  static void denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
  * write_page() function above.
  */
 static void denali_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-					const uint8_t *buf)
+					const uint8_t *buf, const uint8_t *oob)
 {
 	/* for raw page writes, we want to disable ECC and simply write
 	   whatever data is in the buffer. */
@@ -1119,7 +1119,7 @@  static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
-			    uint8_t *buf, int page)
+			    uint8_t *buf, uint8_t *oob, int page)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
 
@@ -1171,7 +1171,7 @@  static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-				uint8_t *buf, int page)
+				uint8_t *buf, uint8_t *oob, int page)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
 
diff --git a/drivers/mtd/nand/docg4.c b/drivers/mtd/nand/docg4.c
index b082026..7ff99c9 100644
--- a/drivers/mtd/nand/docg4.c
+++ b/drivers/mtd/nand/docg4.c
@@ -786,13 +786,13 @@  static int read_page(struct mtd_info *mtd, struct nand_chip *nand,
 
 
 static int docg4_read_page_raw(struct mtd_info *mtd, struct nand_chip *nand,
-			       uint8_t *buf, int page)
+			       uint8_t *buf, uint8_t *oob, int page)
 {
 	return read_page(mtd, nand, buf, page, false);
 }
 
 static int docg4_read_page(struct mtd_info *mtd, struct nand_chip *nand,
-			   uint8_t *buf, int page)
+			   uint8_t *buf, uint8_t *oob, int page)
 {
 	return read_page(mtd, nand, buf, page, true);
 }
@@ -952,13 +952,13 @@  static void write_page(struct mtd_info *mtd, struct nand_chip *nand,
 }
 
 static void docg4_write_page_raw(struct mtd_info *mtd, struct nand_chip *nand,
-				 const uint8_t *buf)
+				 const uint8_t *buf, const uint8_t *oob)
 {
 	return write_page(mtd, nand, buf, false);
 }
 
 static void docg4_write_page(struct mtd_info *mtd, struct nand_chip *nand,
-			     const uint8_t *buf)
+			     const uint8_t *buf, const uint8_t *oob)
 {
 	return write_page(mtd, nand, buf, true);
 }
@@ -1002,7 +1002,7 @@  static int __init read_factory_bbt(struct mtd_info *mtd)
 		return -ENOMEM;
 
 	read_page_prologue(mtd, g4_addr);
-	status = docg4_read_page(mtd, nand, buf, DOCG4_FACTORY_BBT_PAGE);
+	status = docg4_read_page(mtd, nand, buf, NULL, DOCG4_FACTORY_BBT_PAGE);
 	if (status)
 		goto exit;
 
@@ -1079,7 +1079,7 @@  static int docg4_block_markbad(struct mtd_info *mtd, loff_t ofs)
 
 	/* write first page of block */
 	write_page_prologue(mtd, g4_addr);
-	docg4_write_page(mtd, nand, buf);
+	docg4_write_page(mtd, nand, buf, nand->oob_poi);
 	ret = pageprog(mtd);
 	if (!ret)
 		mtd->ecc_stats.badblocks++;
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index 80b5264..b7d9bfd 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -740,7 +740,7 @@  static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
 
 static int fsl_elbc_read_page(struct mtd_info *mtd,
                               struct nand_chip *chip,
-			      uint8_t *buf,
+			      uint8_t *buf, uint8_t *oob,
 			      int page)
 {
 	fsl_elbc_read_buf(mtd, buf, mtd->writesize);
@@ -755,9 +755,8 @@  static int fsl_elbc_read_page(struct mtd_info *mtd,
 /* ECC will be calculated automatically, and errors will be detected in
  * waitfunc.
  */
-static void fsl_elbc_write_page(struct mtd_info *mtd,
-                                struct nand_chip *chip,
-                                const uint8_t *buf)
+static void fsl_elbc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
+				const uint8_t *buf, const uint8_t *oob)
 {
 	fsl_elbc_write_buf(mtd, buf, mtd->writesize);
 	fsl_elbc_write_buf(mtd, chip->oob_poi, mtd->oobsize);
diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
index c30ac7b..f8a6e3e 100644
--- a/drivers/mtd/nand/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/fsl_ifc_nand.c
@@ -684,7 +684,7 @@  static int fsl_ifc_wait(struct mtd_info *mtd, struct nand_chip *chip)
 
 static int fsl_ifc_read_page(struct mtd_info *mtd,
 			      struct nand_chip *chip,
-			      uint8_t *buf, int page)
+			      uint8_t *buf, uint8_t *oob, int page)
 {
 	struct fsl_ifc_mtd *priv = chip->priv;
 	struct fsl_ifc_ctrl *ctrl = priv->ctrl;
@@ -706,7 +706,7 @@  static int fsl_ifc_read_page(struct mtd_info *mtd,
  */
 static void fsl_ifc_write_page(struct mtd_info *mtd,
 				struct nand_chip *chip,
-				const uint8_t *buf)
+				const uint8_t *buf, const uint8_t *oob)
 {
 	fsl_ifc_write_buf(mtd, buf, mtd->writesize);
 	fsl_ifc_write_buf(mtd, chip->oob_poi, mtd->oobsize);
diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index 1b8330e..8bb731b 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -692,6 +692,7 @@  static void fsmc_write_buf_dma(struct mtd_info *mtd, const uint8_t *buf,
  * @mtd:	mtd info structure
  * @chip:	nand chip info structure
  * @buf:	buffer to store read data
+ * @oob:	buffer to store read OOB data
  * @page:	page number to read
  *
  * This routine is needed for fsmc version 8 as reading from NAND chip has to be
@@ -701,7 +702,7 @@  static void fsmc_write_buf_dma(struct mtd_info *mtd, const uint8_t *buf,
  * max of 8 bits)
  */
 static int fsmc_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
-				 uint8_t *buf, int page)
+				 uint8_t *buf, uint8_t *oob, int page)
 {
 	struct fsmc_nand_data *host = container_of(mtd,
 					struct fsmc_nand_data, mtd);
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 75b1dde..949c761 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -841,7 +841,7 @@  static void block_mark_swapping(struct gpmi_nand_data *this,
 }
 
 static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
-				uint8_t *buf, int page)
+				uint8_t *buf, uint8_t *oob, int page)
 {
 	struct gpmi_nand_data *this = chip->priv;
 	struct bch_geometry *nfc_geo = &this->bch_geometry;
@@ -928,7 +928,8 @@  exit_nfc:
 }
 
 static void gpmi_ecc_write_page(struct mtd_info *mtd,
-				struct nand_chip *chip, const uint8_t *buf)
+				struct nand_chip *chip, const uint8_t *buf,
+				const uint8_t *oob)
 {
 	struct gpmi_nand_data *this = chip->priv;
 	struct bch_geometry *nfc_geo = &this->bch_geometry;
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 47b19c0..95ba987 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1066,12 +1066,13 @@  EXPORT_SYMBOL(nand_lock);
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: buffer to store read data
+ * @oob: buffer to store read OOB data
  * @page: page number to read
  *
  * Not for syndrome calculating ECC controllers, which use a special oob layout.
  */
 static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-			      uint8_t *buf, int page)
+			      uint8_t *buf, uint8_t *oob, int page)
 {
 	chip->read_buf(mtd, buf, mtd->writesize);
 	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
@@ -1083,17 +1084,18 @@  static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: buffer to store read data
+ * @oob: buffer to store read OOB
  * @page: page number to read
  *
  * We need a special oob layout and handling even when OOB isn't used.
  */
 static int nand_read_page_raw_syndrome(struct mtd_info *mtd,
 					struct nand_chip *chip,
-					uint8_t *buf, int page)
+					uint8_t *buf, uint8_t *oob, int page)
 {
 	int eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
-	uint8_t *oob = chip->oob_poi;
+	uint8_t *oobbuf = chip->oob_poi;
 	int steps, size;
 
 	for (steps = chip->ecc.steps; steps > 0; steps--) {
@@ -1101,22 +1103,22 @@  static int nand_read_page_raw_syndrome(struct mtd_info *mtd,
 		buf += eccsize;
 
 		if (chip->ecc.prepad) {
-			chip->read_buf(mtd, oob, chip->ecc.prepad);
-			oob += chip->ecc.prepad;
+			chip->read_buf(mtd, oobbuf, chip->ecc.prepad);
+			oobbuf += chip->ecc.prepad;
 		}
 
-		chip->read_buf(mtd, oob, eccbytes);
-		oob += eccbytes;
+		chip->read_buf(mtd, oobbuf, eccbytes);
+		oobbuf += eccbytes;
 
 		if (chip->ecc.postpad) {
-			chip->read_buf(mtd, oob, chip->ecc.postpad);
-			oob += chip->ecc.postpad;
+			chip->read_buf(mtd, oobbuf, chip->ecc.postpad);
+			oobbuf += chip->ecc.postpad;
 		}
 	}
 
-	size = mtd->oobsize - (oob - chip->oob_poi);
+	size = mtd->oobsize - (oobbuf - chip->oob_poi);
 	if (size)
-		chip->read_buf(mtd, oob, size);
+		chip->read_buf(mtd, oobbuf, size);
 
 	return 0;
 }
@@ -1129,7 +1131,7 @@  static int nand_read_page_raw_syndrome(struct mtd_info *mtd,
  * @page: page number to read
  */
 static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
-				uint8_t *buf, int page)
+				uint8_t *buf, uint8_t *oob, int page)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -1139,7 +1141,7 @@  static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
 	uint8_t *ecc_code = chip->buffers->ecccode;
 	uint32_t *eccpos = chip->ecc.layout->eccpos;
 
-	chip->ecc.read_page_raw(mtd, chip, buf, page);
+	chip->ecc.read_page_raw(mtd, chip, buf, oob, page);
 
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
 		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
@@ -1257,12 +1259,13 @@  static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: buffer to store read data
+ * @oob: buffer to store read OOB
  * @page: page number to read
  *
  * Not for syndrome calculating ECC controllers which need a special oob layout.
  */
 static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
-				uint8_t *buf, int page)
+				uint8_t *buf, uint8_t *oob, int page)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -1302,6 +1305,7 @@  static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: buffer to store read data
+ * @oob: buffer to store read OOB
  * @page: page number to read
  *
  * Hardware ECC for large page chips, require OOB to be read first. For this
@@ -1311,7 +1315,7 @@  static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
  * the data area, by overwriting the NAND manufacturer bad block markings.
  */
 static int nand_read_page_hwecc_oob_first(struct mtd_info *mtd,
-	struct nand_chip *chip, uint8_t *buf, int page)
+	struct nand_chip *chip, uint8_t *buf, uint8_t *oob, int page)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -1350,19 +1354,20 @@  static int nand_read_page_hwecc_oob_first(struct mtd_info *mtd,
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: buffer to store read data
+ * @oob: buffer to store read OOB
  * @page: page number to read
  *
  * The hw generator calculates the error syndrome automatically. Therefore we
  * need a special oob layout and handling.
  */
 static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
-				   uint8_t *buf, int page)
+				   uint8_t *buf, uint8_t *oob, int page)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
 	int eccsteps = chip->ecc.steps;
 	uint8_t *p = buf;
-	uint8_t *oob = chip->oob_poi;
+	uint8_t *oobbuf = chip->oob_poi;
 
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
 		int stat;
@@ -1371,23 +1376,23 @@  static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
 		chip->read_buf(mtd, p, eccsize);
 
 		if (chip->ecc.prepad) {
-			chip->read_buf(mtd, oob, chip->ecc.prepad);
-			oob += chip->ecc.prepad;
+			chip->read_buf(mtd, oobbuf, chip->ecc.prepad);
+			oobbuf += chip->ecc.prepad;
 		}
 
 		chip->ecc.hwctl(mtd, NAND_ECC_READSYN);
-		chip->read_buf(mtd, oob, eccbytes);
-		stat = chip->ecc.correct(mtd, p, oob, NULL);
+		chip->read_buf(mtd, oobbuf, eccbytes);
+		stat = chip->ecc.correct(mtd, p, oobbuf, NULL);
 
 		if (stat < 0)
 			mtd->ecc_stats.failed++;
 		else
 			mtd->ecc_stats.corrected += stat;
 
-		oob += eccbytes;
+		oobbuf += eccbytes;
 
 		if (chip->ecc.postpad) {
-			chip->read_buf(mtd, oob, chip->ecc.postpad);
+			chip->read_buf(mtd, oobbuf, chip->ecc.postpad);
 			oob += chip->ecc.postpad;
 		}
 	}
@@ -1500,14 +1505,14 @@  static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 
 			/* Now read the page into the buffer */
 			if (unlikely(ops->mode == MTD_OPS_RAW))
-				ret = chip->ecc.read_page_raw(mtd, chip,
-							      bufpoi, page);
+				ret = chip->ecc.read_page_raw(mtd, chip, bufpoi,
+							      NULL, page);
 			else if (!aligned && NAND_SUBPAGE_READ(chip) && !oob)
 				ret = chip->ecc.read_subpage(mtd, chip,
 							col, bytes, bufpoi);
 			else
 				ret = chip->ecc.read_page(mtd, chip, bufpoi,
-							  page);
+							  NULL, page);
 			if (ret < 0) {
 				if (!aligned)
 					/* Invalidate page cache */
@@ -1919,11 +1924,12 @@  out:
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: data buffer
+ * @oob: OOB buffer
  *
  * Not for syndrome calculating ECC controllers, which use a special oob layout.
  */
 static void nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-				const uint8_t *buf)
+				const uint8_t *buf, const uint8_t *oob)
 {
 	chip->write_buf(mtd, buf, mtd->writesize);
 	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
@@ -1934,16 +1940,18 @@  static void nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: data buffer
+ * @oob: OOB buffer
  *
  * We need a special oob layout and handling even when ECC isn't checked.
  */
 static void nand_write_page_raw_syndrome(struct mtd_info *mtd,
 					struct nand_chip *chip,
-					const uint8_t *buf)
+					const uint8_t *buf,
+					const uint8_t *oob)
 {
 	int eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
-	uint8_t *oob = chip->oob_poi;
+	uint8_t *oobbuf = chip->oob_poi;
 	int steps, size;
 
 	for (steps = chip->ecc.steps; steps > 0; steps--) {
@@ -1951,31 +1959,32 @@  static void nand_write_page_raw_syndrome(struct mtd_info *mtd,
 		buf += eccsize;
 
 		if (chip->ecc.prepad) {
-			chip->write_buf(mtd, oob, chip->ecc.prepad);
-			oob += chip->ecc.prepad;
+			chip->write_buf(mtd, oobbuf, chip->ecc.prepad);
+			oobbuf += chip->ecc.prepad;
 		}
 
-		chip->read_buf(mtd, oob, eccbytes);
-		oob += eccbytes;
+		chip->read_buf(mtd, oobbuf, eccbytes);
+		oobbuf += eccbytes;
 
 		if (chip->ecc.postpad) {
-			chip->write_buf(mtd, oob, chip->ecc.postpad);
-			oob += chip->ecc.postpad;
+			chip->write_buf(mtd, oobbuf, chip->ecc.postpad);
+			oobbuf += chip->ecc.postpad;
 		}
 	}
 
-	size = mtd->oobsize - (oob - chip->oob_poi);
+	size = mtd->oobsize - (oobbuf - chip->oob_poi);
 	if (size)
-		chip->write_buf(mtd, oob, size);
+		chip->write_buf(mtd, oobbuf, size);
 }
 /**
  * nand_write_page_swecc - [REPLACEABLE] software ECC based page write function
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: data buffer
+ * @oob: OOB buffer
  */
 static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
-				  const uint8_t *buf)
+				  const uint8_t *buf, const uint8_t *oob)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -1991,7 +2000,7 @@  static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
 	for (i = 0; i < chip->ecc.total; i++)
 		chip->oob_poi[eccpos[i]] = ecc_calc[i];
 
-	chip->ecc.write_page_raw(mtd, chip, buf);
+	chip->ecc.write_page_raw(mtd, chip, buf, oob);
 }
 
 /**
@@ -1999,9 +2008,10 @@  static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: data buffer
+ * @oob: OOB buffer
  */
 static void nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
-				  const uint8_t *buf)
+				  const uint8_t *buf, const uint8_t *oob)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -2027,18 +2037,20 @@  static void nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: data buffer
+ * @oob: OOB data buffer
  *
  * The hw generator calculates the error syndrome automatically. Therefore we
  * need a special oob layout and handling.
  */
 static void nand_write_page_syndrome(struct mtd_info *mtd,
-				    struct nand_chip *chip, const uint8_t *buf)
+				    struct nand_chip *chip, const uint8_t *buf,
+				    const uint8_t *oob)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
 	int eccsteps = chip->ecc.steps;
 	const uint8_t *p = buf;
-	uint8_t *oob = chip->oob_poi;
+	uint8_t *oobbuf = chip->oob_poi;
 
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
 
@@ -2046,24 +2058,24 @@  static void nand_write_page_syndrome(struct mtd_info *mtd,
 		chip->write_buf(mtd, p, eccsize);
 
 		if (chip->ecc.prepad) {
-			chip->write_buf(mtd, oob, chip->ecc.prepad);
-			oob += chip->ecc.prepad;
+			chip->write_buf(mtd, oobbuf, chip->ecc.prepad);
+			oobbuf += chip->ecc.prepad;
 		}
 
-		chip->ecc.calculate(mtd, p, oob);
-		chip->write_buf(mtd, oob, eccbytes);
-		oob += eccbytes;
+		chip->ecc.calculate(mtd, p, oobbuf);
+		chip->write_buf(mtd, oobbuf, eccbytes);
+		oobbuf += eccbytes;
 
 		if (chip->ecc.postpad) {
-			chip->write_buf(mtd, oob, chip->ecc.postpad);
-			oob += chip->ecc.postpad;
+			chip->write_buf(mtd, oobbuf, chip->ecc.postpad);
+			oobbuf += chip->ecc.postpad;
 		}
 	}
 
 	/* Calculate remaining oob bytes */
-	i = mtd->oobsize - (oob - chip->oob_poi);
+	i = mtd->oobsize - (oobbuf - chip->oob_poi);
 	if (i)
-		chip->write_buf(mtd, oob, i);
+		chip->write_buf(mtd, oobbuf, i);
 }
 
 /**
@@ -2076,16 +2088,17 @@  static void 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 page, int cached, int raw)
+			   const uint8_t *buf, const uint8_t *oob, int page,
+			   int cached, int raw)
 {
 	int status;
 
 	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
 
 	if (unlikely(raw))
-		chip->ecc.write_page_raw(mtd, chip, buf);
+		chip->ecc.write_page_raw(mtd, chip, buf, oob);
 	else
-		chip->ecc.write_page(mtd, chip, buf);
+		chip->ecc.write_page(mtd, chip, buf, oob);
 
 	/*
 	 * Cached progamming disabled for now. Not sure if it's worth the
@@ -2264,7 +2277,7 @@  static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 			memset(chip->oob_poi, 0xff, mtd->oobsize);
 		}
 
-		ret = chip->write_page(mtd, chip, wbuf, page, cached,
+		ret = chip->write_page(mtd, chip, wbuf, NULL, page, cached,
 				       (ops->mode == MTD_OPS_RAW));
 		if (ret)
 			break;
diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index def50ca..92d2eae 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -682,14 +682,14 @@  static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 }
 
 static void pxa3xx_nand_write_page_hwecc(struct mtd_info *mtd,
-		struct nand_chip *chip, const uint8_t *buf)
+		struct nand_chip *chip, const uint8_t *buf, const uint8_t *oob)
 {
 	chip->write_buf(mtd, buf, mtd->writesize);
 	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
 }
 
 static int pxa3xx_nand_read_page_hwecc(struct mtd_info *mtd,
-		struct nand_chip *chip, uint8_t *buf, int page)
+		struct nand_chip *chip, uint8_t *buf, uint8_t *oob, int page)
 {
 	struct pxa3xx_nand_host *host = mtd->priv;
 	struct pxa3xx_nand_info *info = host->info_data;
diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index e9b2b26..d1b2731 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -344,7 +344,7 @@  static void set_cmd_regs(struct mtd_info *mtd, uint32_t cmd, uint32_t flcmcdr_va
 }
 
 static int flctl_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
-				uint8_t *buf, int page)
+				uint8_t *buf, uint8_t *oob, int page)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -366,7 +366,7 @@  static int flctl_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 static void flctl_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
-				   const uint8_t *buf)
+				   const uint8_t *buf, const uint8_t *oob)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 1482340..be9ee1f 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -363,15 +363,15 @@  struct nand_ecc_ctrl {
 	int (*correct)(struct mtd_info *mtd, uint8_t *dat, uint8_t *read_ecc,
 			uint8_t *calc_ecc);
 	int (*read_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
-			uint8_t *buf, int page);
+			uint8_t *buf, uint8_t *oob, int page);
 	void (*write_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
-			const uint8_t *buf);
+			const uint8_t *buf, const uint8_t *oob);
 	int (*read_page)(struct mtd_info *mtd, struct nand_chip *chip,
-			uint8_t *buf, int page);
+			uint8_t *buf, uint8_t *oob, int page);
 	int (*read_subpage)(struct mtd_info *mtd, struct nand_chip *chip,
 			uint32_t offs, uint32_t len, uint8_t *buf);
 	void (*write_page)(struct mtd_info *mtd, struct nand_chip *chip,
-			const uint8_t *buf);
+			const uint8_t *buf, const uint8_t *oob);
 	int (*write_oob_raw)(struct mtd_info *mtd, struct nand_chip *chip,
 			int page);
 	int (*read_oob_raw)(struct mtd_info *mtd, struct nand_chip *chip,
@@ -505,7 +505,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 page, int cached, int raw);
+			const uint8_t *buf, const uint8_t *oob, int page,
+			int cached, int raw);
 
 	int chip_delay;
 	unsigned int options;