[PATCH] mtd: nand: fixup kerneldoc, rename parameter

Gupta, Pekon pekon at ti.com
Thu Aug 8 23:25:51 EDT 2013


Hi Brian,

Thanks for review.. And few more thoughts and queries which came to my mind
while writing down subpage-write support. I think its time to address those also.

> First, the function argument is 'offset' not 'column'.
> 
> Second, the 'data_buf' name is inconsistent with the rest of this file.
> Just use 'buf'.
> 
> Signed-off-by: Brian Norris <computersforpeace at gmail.com>
> Cc: Gupta, Pekon <pekon at ti.com>
> ---
>  drivers/mtd/nand/nand_base.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 8f04fb0..49ca737 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -1980,13 +1980,14 @@ static int nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>   * nand_write_subpage_hwecc - [REPLACABLE] hardware ECC based subpage write
>   * @mtd:       mtd info structure
>   * @chip:      nand chip info structure
> - * @column:    column address of subpage within the page
> + * @offset:    column address of subpage within the page
>   * @data_len:  data length
> + * @buf:       data buffer
>   * @oob_required: must write chip->oob_poi to OOB
>   */
>  static int nand_write_subpage_hwecc(struct mtd_info *mtd,
>                                 struct nand_chip *chip, uint32_t offset,
> -                               uint32_t data_len, const uint8_t *data_buf,
> +                               uint32_t data_len, const uint8_t *buf,
>                                 int oob_required)
>  {
>         uint8_t *oob_buf  = chip->oob_poi;
> @@ -2005,20 +2006,20 @@ static int nand_write_subpage_hwecc(struct mtd_info *mtd,
>                 chip->ecc.hwctl(mtd, NAND_ECC_WRITE);
> 
>                 /* write data (untouched subpages already masked by 0xFF) */
> -               chip->write_buf(mtd, data_buf, ecc_size);
> +               chip->write_buf(mtd, buf, ecc_size);
> 
>                 /* mask ECC of un-touched subpages by padding 0xFF */
>                 if ((step < start_step) || (step > end_step))
>                         memset(ecc_calc, 0xff, ecc_bytes);

[Pekon]: In above implementation ..
even though only single subpage data needs to be written, still whole
page is programmed with other 'untouched' subpage data masked with 0xFF.
This is not the efficient way, as it does not give any performance benefit.
Is it possible tht specific sub-pages inside a page can be addressed, read & written
independently?
I have not found any NAND devices supporting subpages (partial pages),
and having a separate command 'SUBPAGE_PROGRAM' in addition to PAGE_PROGRAM.

If indepedent subpage programming was allowed (as per actual subpage definition),
and there was a independent NAND command SUBPAGE_PROGRAM then,
we could just just change mtd->writesize == subpage_size (partial-page-size),
and then eveything should work seemlessly.. correct ?

>                 else
> -                       chip->ecc.calculate(mtd, data_buf, ecc_calc);
> +                       chip->ecc.calculate(mtd, buf, ecc_calc);
> 
>                 /* mask OOB of un-touched subpages by padding 0xFF */
>                 /* if oob_required, preserve OOB metadata of written subpage */
>                 if (!oob_required || (step < start_step) || (step > end_step))
>                         memset(oob_buf, 0xff, oob_bytes);
> 
[Pekon]: same it the case for OOB data, untouched OOB bits are masked with 0xFF.
Also File-systems storing their metadata in OOB area might have some problem with using 
subpages, because currently our ECC layouts are not uniformly distributed across subpage
boundaries. Though this is also the problem with ECC also.
Hence going forward, I would suggest to have at-least _ecc_bytes_ eqully distributed
and aligned to subpage boundaries .. thoughts ??

> -               data_buf += ecc_size;
> +               buf += ecc_size;
>                 ecc_calc += ecc_bytes;
>                 oob_buf  += oob_bytes;
>         }
> --
> 1.8.1.2
> 

with regards, pekon


More information about the linux-mtd mailing list