From mboxrd@z Thu Jan  1 00:00:00 1970
From: Tom <Tom.Rix@windriver.com>
Date: Wed, 30 Sep 2009 14:22:09 -0500
Subject: [U-Boot] [PATCH] Nand: Implement raw read/write and biterr
In-Reply-To: <1254334299-9583-1-git-send-email-jrigby@control4.com>
References: <1254334299-9583-1-git-send-email-jrigby@control4.com>
Message-ID: <4AC3AFE1.2030506@windriver.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

John Rigby wrote:
> New commands nand read.raw and write.raw read/write
> main and oob area.
> 
> Implement previously stubbed nand biterr command.
> 
> Document the above and also the previously undocumented
> read.oob and write.oob.
> 
> Signed-off-by: John Rigby <jrigby@control4.com>
> ---
>  common/cmd_nand.c |  115 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 112 insertions(+), 3 deletions(-)
> 
> diff --git a/common/cmd_nand.c b/common/cmd_nand.c
> index 158a55f..a488038 100644
> --- a/common/cmd_nand.c
> +++ b/common/cmd_nand.c
> @@ -90,6 +90,95 @@ static int nand_dump(nand_info_t *nand, ulong off, int only_oob)
>  	return 0;
>  }
>  
> +#define NAND_RW_RAW_READ 0
> +#define NAND_RW_RAW_WRITE 1
> +
> +static int nand_rdwr_raw(int rdwr, nand_info_t *nand, ulong off, u_char *buf,
> +				size_t size)
> +{
> +	struct mtd_oob_ops ops = {
> +		.len = nand->writesize,
> +		.ooblen = nand->oobsize,
> +		.mode = MTD_OOB_RAW,
> +	};
> +	int i;
> +	int nrblocks = size / nand->writesize;
> +	loff_t addr = (loff_t)(off & ~(nand->writesize - 1));

Silently dropping bytes.
Would it be better to require the size to be a multiple of the block size ?
Or at least warn of the dropped bytes.

> +
> +	while (nrblocks--) {
> +		ops.datbuf = buf;

<snip>

>  	}
>  
> @@ -494,13 +600,16 @@ U_BOOT_CMD(nand, CONFIG_SYS_MAXARGS, 1, do_nand,
>  	"nand write - addr off|partition size\n"
>  	"    read/write 'size' bytes starting at offset 'off'\n"
>  	"    to/from memory address 'addr', skipping bad blocks.\n"
> +	"     .oob - reads/writes oob only.\n"
> +	"     .raw - reads/writes both main and oob with no error\n"
> +	"            detection or correction\n"

Writing the oob area directly is UNSAFE.
Having done to myself, it was a pain to undo.
You should put at least document that.

Tom