Re: [RFC 1/5] mtd: support MTD_MODE_RAW for writing OOB

[Artem Bityutskiy <dedekind1@gmail.com>]

On Wed, 2011-08-17 at 16:50 -0700, Brian Norris wrote:
> mtd_do_write_oob does not pass information about whether to write in RAW
> mode to the lower layers (e.g., NAND). We should pass MTD_OOB_RAW or
> MTD_OOB_PLACE based on the MTD file mode.
> 
> This fixes issues with `nandwrite -n' and the MEMWRITEOOB[64] ioctls.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

I guess this patch deserves to be non-RFC? Should it be pushed to
l2-mtd-2.6.git? Should it even have "Cc: stable@kernel.org [kernel
version +] ?

> ---
>  drivers/mtd/mtdchar.c        |    3 ++-
>  drivers/mtd/nand/nand_base.c |    9 ++++++++-
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
> index a75d555..7ca4361 100644
> --- a/drivers/mtd/mtdchar.c
> +++ b/drivers/mtd/mtdchar.c
> @@ -391,6 +391,7 @@ static int mtd_do_writeoob(struct file *file, struct mtd_info *mtd,
>  	uint64_t start, uint32_t length, void __user *ptr,
>  	uint32_t __user *retp)
>  {
> +	struct mtd_file_info *mfi = file->private_data;
>  	struct mtd_oob_ops ops;
>  	uint32_t retlen;
>  	int ret = 0;
> @@ -412,7 +413,7 @@ static int mtd_do_writeoob(struct file *file, struct mtd_info *mtd,
>  	ops.ooblen = length;
>  	ops.ooboffs = start & (mtd->oobsize - 1);
>  	ops.datbuf = NULL;
> -	ops.mode = MTD_OOB_PLACE;
> +	ops.mode = (mfi->mode == MTD_MODE_RAW) ? MTD_OOB_RAW : MTD_OOB_PLACE;

Would be really great to document things while you are in it - e.g.,
putting a little note in include/linux/mtd/mtd.h that MTD_OOB_PLACE is
the default mode. And of course documenting the modes in this file a bit
more would be also great. I encourage you to store the wisdom you gain
in the comments :-)

>  
>  	if (ops.ooboffs && ops.ooblen > (mtd->oobsize - ops.ooboffs))
>  		return -EINVAL;
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index d2ee68a..e2b1c90 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2401,7 +2401,14 @@ static int nand_do_write_oob(struct mtd_info *mtd, loff_t to,
>  		chip->pagebuf = -1;
>  
>  	nand_fill_oob(mtd, ops->oobbuf, ops->ooblen, ops);
> -	status = chip->ecc.write_oob(mtd, chip, page & chip->pagemask);
> +
> +	if (ops->mode == MTD_OOB_RAW) {
> +		status = 0;
> +		chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
> +		chip->ecc.write_page_raw(mtd, chip, NULL);

Strange that write_page_raw does not return an error code - it cannot
fail? But this is a separate issue anyway.

> +	} else {
> +		status = chip->ecc.write_oob(mtd, chip, page & chip->pagemask);
> +	}
>  
>  	if (status)
>  		return status;


-- 
Best Regards,
Artem Bityutskiy