[U-Boot] [PATCH] OneNAND partial read/write support

[Scott Wood]

On Mon, Oct 12, 2009 at 04:27:10PM +0900, Kyungmin Park wrote:
> Now OneNAND handles block operation only.
> With this patch OneNAND handles all read/write size.
> 
> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> ---
> diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c
> index 9090940..2b8f01b 100644
> --- a/common/cmd_onenand.c
> +++ b/common/cmd_onenand.c
> @@ -36,7 +36,7 @@ static inline int str2long(char *p, ulong *num)
>  	return (*p != '\0' && *endptr == '\0') ? 1 : 0;
>  }
>  
> -static int arg_off_size(int argc, char *argv[], ulong *off, size_t *size)
> +static int arg_off_size(int argc, char *argv[], ulong *off, ssize_t *size)
>  {
>  	if (argc >= 1) {
>  		if (!(str2long(argv[0], off))) {

Are you expecting negative sizes?

> @@ -69,61 +69,65 @@ static int arg_off_size(int argc, char *argv[], ulong *off, size_t *size)
>  	return 0;
>  }
>  
> -static int onenand_block_read(loff_t from, size_t len,
> -			      size_t *retlen, u_char *buf, int oob)
> +static int onenand_block_read(loff_t from, ssize_t len,
> +			      ssize_t *retlen, u_char *buf, int oob)
>  {

Is it still onenand_block_read if you don't have to read a whole block?

>  	struct onenand_chip *this = mtd->priv;
> -	int blocks = (int) len >> this->erase_shift;
>  	int blocksize = (1 << this->erase_shift);
>  	loff_t ofs = from;
>  	struct mtd_oob_ops ops = {
>  		.retlen		= 0,
>  	};
> +	ssize_t thislen;
>  	int ret;
>  
> -	if (oob)
> -		ops.ooblen = blocksize;
> -	else
> -		ops.len = blocksize;
> +	while (len > 0) {
> +		thislen = min_t(ssize_t, len, blocksize);
> +		thislen = ALIGN(thislen, mtd->writesize);
>  
> -	while (blocks) {
>  		ret = mtd->block_isbad(mtd, ofs);
>  		if (ret) {
>  			printk("Bad blocks %d at 0x%x\n",
>  			       (u32)(ofs >> this->erase_shift), (u32)ofs);
> -			ofs += blocksize;
> +			ofs += thislen;
>  			continue;
>  		}
>  
> -		if (oob)
> +		if (oob) {
>  			ops.oobbuf = buf;
> -		else
> +			ops.ooblen = thislen;
> +		} else {
>  			ops.datbuf = buf;
> +			ops.len = thislen;

thislen can be greater than len, in which case you'll be overflowing buf.

If you want to allow partial page reads, you need to allocate a temporary
buffer at some point.  If not (I don't see a huge need), the ALIGN()
should be an error check instead.

Does this code handle being given an offset that is not at a block (or
page) boundary?  It doesn't look like it (it will try to read across
block boundaries).

> @@ -265,9 +276,10 @@ static int onenand_block_test(u32 start, u32 size)
>  			goto next;
>  		}
>  
> -		if (memcmp(buf, verify_buf, blocksize))
> +		if (memcmp(buf, verify_buf, blocksize)) {
>  			printk("\nRead/Write test failed at 0x%x\n", (u32)ofs);
> -
> +			break;
> +		}
> @@ -322,6 +334,7 @@ static int onenand_dump(struct mtd_info *mtd, ulong off, int only_oob)
>  		p += 16;
>  	}
>  	puts("OOB:\n");
> +	p = oobbuf;
>  	i = mtd->oobsize >> 3;
>  	while (i--) {
>  		printf("\t%02x %02x %02x %02x %02x %02x %02x %02x\n",
> @@ -339,7 +352,7 @@ int do_onenand(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
>  	struct onenand_chip *this;
>  	int blocksize;
>  	ulong addr, ofs;
> -	size_t len, retlen = 0;
> +	ssize_t len, retlen = 0;
>  	int ret = 0;
>  	char *cmd, *s;
>  
> @@ -385,7 +398,8 @@ int do_onenand(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
>  			int erase;
>  
>  			erase = strcmp(cmd, "erase") == 0; /* 1 = erase, 0 = test */
> -			printf("\nOneNAND %s: ", erase ? "erase" : "test");
> +			printf("\nOneNAND %s %s: ", erase ? "erase" : "test",
> +				force ? "force" : "");

These seem to be unrelated changes.

-Scott