[PATCH v3 09/10] mtd: nand: utilize oob_required parameter

[Shmulik Ladkani]

Hi Brian,

On Fri, 27 Apr 2012 18:29:53 -0700 Brian Norris <computersforpeace at gmail.com> wrote:
> Don't read/write OOB if the caller doesn't requre it.
> 
> Signed-off-by: Brian Norris <computersforpeace at gmail.com>
> ---
>  drivers/mtd/nand/nand_base.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 13a6355..5b390ae 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -1075,7 +1075,8 @@ static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>  			      uint8_t *buf, int oob_required, int page)
>  {
>  	chip->read_buf(mtd, buf, mtd->writesize);
> -	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
> +	if (oob_required)
> +		chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
>  	return 0;
>  }

Re-thinking this, I think this might be incorrect.
Sorry I havn't noticed this earlier.

Suppose the nand chip is working in "sequential" (aka "incremental")
mode.
Meaning, a NAND_CMD_READ0 command is issued, and then adjacent pages are
read sequencially, without having to re-issue NAND_CMD_READ0.

(see for example the loop within 'nand_do_read_ops', especially the
'sndcmd' variable).

In that case, we MUST read the 'oob', because the OOB bytes will always
arrive on the bus following the inband data.
(on bus, data will appear as: page, spare, page, spare, and so on...)

If we do not read into 'oob_poi', then the OOB bytes arriving on the bus
will be set by the software into next page's 'buf'...

I have no idea if this issue is also relevant for 'read_page' implementors
other than of nand_base.c; should carefully review.

There are 2 options for fixing the issue in nand_base.c:

- Disregard the 'oob_required' parameter in all nand_base's
  'read_page' implementations (not future proof, someone might miss the
  nuance; also bug might still exist in the non-default implementations)

- Fix your 02/10 patch, in away that the passed 'oob_required' argument
  will be somehow set according to the 'snd_cmd'

Regards,
Shmulik