[PATCH 2/2] mtd: nand: nand_do_{read,write}_ops - pass OOB buffer through

[Shmulik Ladkani]

Hi Brian,

On Mon, 16 Apr 2012 15:35:55 -0700 Brian Norris <computersforpeace at gmail.com> wrote:
> Now that we have a function parameter for the OOB buffer, we can pass the OOB
> buffer as an argument to the nand_ecc_ctrl functions. This allows drivers to
> know when OOB data must be returned to the upper layers and when it is simply
> needed for internal calculations, potentially saving time for NAND HW/SW that
> can simply avoid reading the OOB data.

I think for consistency sake, existing chip->ecc.{read,write}_page_xxx
methods do need to be ported to support the new 'oob' parameter.

> @@ -2272,12 +2272,14 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
>  			size_t len = min(oobwritelen, oobmaxlen);
>  			oob = nand_fill_oob(mtd, oob, len, ops);
>  			oobwritelen -= len;
> +			oobpoi = chip->oob_poi;
>  		} else {
> +			oobpoi = NULL;
>  			/* We still need to erase leftover OOB data */
>  			memset(chip->oob_poi, 0xff, mtd->oobsize);
>  		}
>  
> -		ret = chip->write_page(mtd, chip, wbuf, NULL, page, cached,
> +		ret = chip->write_page(mtd, chip, wbuf, oobpoi, page, cached,
>  				       (ops->mode == MTD_OPS_RAW));
>  		if (ret)
>  			break;

The 'write_page' interface is problematic, as the meaning of 'oob'
parameter is a bit inconsistent:
- A NULL 'oob' actually states "no OOB buffer to write"
- Your driver instructs HW to write the page (ECC taken care of by HW)
- However default chip->ecc.write_page_xxx methods do need a temp buffer
  for OOB ECC calculation (hence will probably use the internal
  chip->oob_poi buffer)
- But when non-null 'oob' is passed to the default methods, they should
  probably use the given 'oob' buffer (and not a temp buffer)

(This is same for the read interface.)

So the 'oob' parameter is more of a boolean than an actual buffer to be
used by the various ecc.{read,write}_page implementors.

Any reason not to pass a boolean instead?

Regards,
Shmulik