[U-Boot] [PATCH] OneNAND partial read/write support
Now OneNAND handles block operation only. With this patch OneNAND handles all read/write size.
Signed-off-by: Kyungmin Park kyungmin.park@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))) { @@ -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) { 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; + }
ops.retlen = 0; ret = mtd->read_oob(mtd, ofs, &ops); if (ret) { printk("Read failed 0x%x, %d\n", (u32)ofs, ret); - ofs += blocksize; + ofs += thislen; continue; } - ofs += blocksize; - buf += blocksize; - blocks--; + ofs += thislen; + buf += thislen; + len -= thislen; *retlen += ops.retlen; }
return 0; }
-static int onenand_block_write(loff_t to, size_t len, - size_t *retlen, const u_char * buf) +static int onenand_block_write(loff_t to, ssize_t len, + ssize_t *retlen, const u_char * buf) { struct onenand_chip *this = mtd->priv; - int blocks = len >> this->erase_shift; int blocksize = (1 << this->erase_shift); + struct mtd_oob_ops ops = { + .retlen = 0, + .oobbuf = NULL, + }; loff_t ofs; - size_t _retlen = 0; + ssize_t thislen; int ret;
if (to == next_ofs) { @@ -135,27 +139,34 @@ static int onenand_block_write(loff_t to, size_t len, } ofs = to;
- while (blocks) { + while (len > 0) { + thislen = min_t(ssize_t, len, blocksize); + thislen = ALIGN(thislen, mtd->writesize); + ret = mtd->block_isbad(mtd, ofs); if (ret) { printk("Bad blocks %d at 0x%x\n", (u32)(ofs >> this->erase_shift), (u32)ofs); - skip_ofs += blocksize; + skip_ofs += thislen; goto next; }
- ret = mtd->write(mtd, ofs, blocksize, &_retlen, buf); + + ops.datbuf = (u_char *) buf; + ops.len = thislen; + ops.retlen = 0; + ret = mtd->write_oob(mtd, ofs, &ops); if (ret) { printk("Write failed 0x%x, %d", (u32)ofs, ret); - skip_ofs += blocksize; + skip_ofs += thislen; goto next; }
- buf += blocksize; - blocks--; - *retlen += _retlen; + buf += thislen; + len -= thislen; + *retlen += ops.retlen; next: - ofs += blocksize; + ofs += thislen; }
return 0; @@ -234,7 +245,7 @@ static int onenand_block_test(u32 start, u32 size) end_block = mtd->size >> this->erase_shift;
blocks = start_block; - ofs = start; + ofs = start_block << this->erase_shift; while (blocks < end_block) { printf("\rTesting block %d at 0x%x", (u32)(ofs >> this->erase_shift), (u32)ofs);
@@ -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; + } next: ofs += blocksize; blocks++; @@ -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" : "");
/* skip first two or three arguments, look for offset and size */ if (arg_off_size(argc - o, argv + o, &ofs, &len) != 0)
Hi, Kyungmin Park! Thank you for patch! But I have a question - how should I apply it? Sorry, I'm not very skilled in UNIX/Linux. I've saved all code from:
diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c index 9090940..2b8f01b 100644
to:
if (arg_off_size(argc - o, argv + o, &ofs, &len) != 0)
into "1.patch" file. Tryed to patch -p1 < /1.patch from U-Boot directory, and get:
patching file common/cmd_onenand.c patch: **** malformed patch at line 6: return (*p != '\0' && *endptr== '\0') ? 1 : 0;
I use U-Boot2009.08.rc2.
What is wrong? Or maybe you could send me your full /common/cmd_onenand.c ?
On Monday 12 October 2009 11:27:10 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@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))) { @@ -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){ 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;}ops.retlen = 0; ret = mtd->read_oob(mtd, ofs, &ops); if (ret) { printk("Read failed 0x%x, %d\n", (u32)ofs, ret);
ofs += blocksize;
ofs += thislen; continue;
ofs += blocksize;buf += blocksize;blocks--;
ofs += thislen;buf += thislen;len -= thislen;*retlen += ops.retlen; }
return 0;
}
-static int onenand_block_write(loff_t to, size_t len,
size_t *retlen, const u_char * buf)+static int onenand_block_write(loff_t to, ssize_t len,
ssize_t *retlen, const u_char * buf){ struct onenand_chip *this = mtd->priv;
- int blocks = len >> this->erase_shift; int blocksize = (1 << this->erase_shift);
- struct mtd_oob_ops ops = {
.retlen = 0,.oobbuf = NULL,- }; loff_t ofs;
- size_t _retlen = 0;
ssize_t thislen; int ret;
if (to == next_ofs) {
@@ -135,27 +139,34 @@ static int onenand_block_write(loff_t to, size_t len, } ofs = to;
- while (blocks) {
- while (len > 0) {
thislen = min_t(ssize_t, len, blocksize);thislen = ALIGN(thislen, mtd->writesize);- ret = mtd->block_isbad(mtd, ofs); if (ret) { printk("Bad blocks %d at 0x%x\n", (u32)(ofs >> this->erase_shift), (u32)ofs);
skip_ofs += blocksize;
skip_ofs += thislen; goto next;
ret = mtd->write(mtd, ofs, blocksize, &_retlen, buf);
ops.datbuf = (u_char *) buf;ops.len = thislen;ops.retlen = 0;ret = mtd->write_oob(mtd, ofs, &ops);
skip_ofs += blocksize;
skip_ofs += thislen; goto next;
buf += blocksize;blocks--;*retlen += _retlen;
buf += thislen;len -= thislen;*retlen += ops.retlen;next:
ofs += blocksize;
ofs += thislen;}
return 0;
@@ -234,7 +245,7 @@ static int onenand_block_test(u32 start, u32 size) end_block = mtd->size >> this->erase_shift;
blocks = start_block;
- ofs = start;
- ofs = start_block << this->erase_shift; while (blocks < end_block) { printf("\rTesting block %d at 0x%x", (u32)(ofs >> this->erase_shift),
(u32)ofs);
@@ -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;}next: ofs += blocksize; blocks++; @@ -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" : ""); /* skip first two or three arguments, look for offset and size */ if (arg_off_size(argc - o, argv + o, &ofs, &len) != 0)
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi,
If you use the gmail, Select the raw message, and copy and paste Others are similar. Don't copy html format to your text file.
Thank you, Kyungmin Park
On Mon, Oct 12, 2009 at 5:51 PM, Tuma chernigovskiy@spb.gs.ru wrote:
Hi, Kyungmin Park! Thank you for patch! But I have a question - how should I apply it? Sorry, I'm not very skilled in UNIX/Linux. I've saved all code from:
diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c index 9090940..2b8f01b 100644
to:
if (arg_off_size(argc - o, argv + o, &ofs, &len) != 0)
into "1.patch" file. Tryed to patch -p1 < /1.patch from U-Boot directory, and get:
patching file common/cmd_onenand.c patch: **** malformed patch at line 6: return (*p != '\0' && *endptr== '\0') ? 1 : 0;
I use U-Boot2009.08.rc2.
What is wrong? Or maybe you could send me your full /common/cmd_onenand.c ?
On Monday 12 October 2009 11:27:10 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@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))) { @@ -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)
{ 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;
- }
ops.retlen = 0; ret = mtd->read_oob(mtd, ofs, &ops); if (ret) { printk("Read failed 0x%x, %d\n", (u32)ofs, ret);
- ofs += blocksize;
- ofs += thislen;
continue; }
- ofs += blocksize;
- buf += blocksize;
- blocks--;
- ofs += thislen;
- buf += thislen;
- len -= thislen;
*retlen += ops.retlen; }
return 0; }
-static int onenand_block_write(loff_t to, size_t len,
- size_t *retlen, const u_char * buf)
+static int onenand_block_write(loff_t to, ssize_t len,
- ssize_t *retlen, const u_char * buf)
{ struct onenand_chip *this = mtd->priv;
- int blocks = len >> this->erase_shift;
int blocksize = (1 << this->erase_shift);
- struct mtd_oob_ops ops = {
- .retlen = 0,
- .oobbuf = NULL,
- };
loff_t ofs;
- size_t _retlen = 0;
- ssize_t thislen;
int ret;
if (to == next_ofs) { @@ -135,27 +139,34 @@ static int onenand_block_write(loff_t to, size_t len, } ofs = to;
- while (blocks) {
- while (len > 0) {
- thislen = min_t(ssize_t, len, blocksize);
- thislen = ALIGN(thislen, mtd->writesize);
ret = mtd->block_isbad(mtd, ofs); if (ret) { printk("Bad blocks %d at 0x%x\n", (u32)(ofs >> this->erase_shift), (u32)ofs);
- skip_ofs += blocksize;
- skip_ofs += thislen;
goto next; }
- ret = mtd->write(mtd, ofs, blocksize, &_retlen, buf);
- ops.datbuf = (u_char *) buf;
- ops.len = thislen;
- ops.retlen = 0;
- ret = mtd->write_oob(mtd, ofs, &ops);
if (ret) { printk("Write failed 0x%x, %d", (u32)ofs, ret);
- skip_ofs += blocksize;
- skip_ofs += thislen;
goto next; }
- buf += blocksize;
- blocks--;
- *retlen += _retlen;
- buf += thislen;
- len -= thislen;
- *retlen += ops.retlen;
next:
- ofs += blocksize;
- ofs += thislen;
}
return 0; @@ -234,7 +245,7 @@ static int onenand_block_test(u32 start, u32 size) end_block = mtd->size >> this->erase_shift;
blocks = start_block;
- ofs = start;
- ofs = start_block << this->erase_shift;
while (blocks < end_block) { printf("\rTesting block %d at 0x%x", (u32)(ofs >> this->erase_shift), (u32)ofs);
@@ -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;
- }
next: ofs += blocksize; blocks++; @@ -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" : "");
/* skip first two or three arguments, look for offset and size */ if (arg_off_size(argc - o, argv + o, &ofs, &len) != 0) _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
-- Software Developer General Satellite Corp. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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@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
On Wed, Oct 21, 2009 at 7:48 AM, Scott Wood scottwood@freescale.com wrote:
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@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?
I don't know why these are implemented. it's existing one.
@@ -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.
No, len is unsigned int, but thislen is int. and can't overflow since. it's size down with blocksize at min macro.
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).
Basically it reads/writes data based on block. but last or first partial read/writes support.
@@ -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.
Right, but it's for exact display for user.
Thank you, Kyungmin Park
Kyungmin Park wrote:
On Wed, Oct 21, 2009 at 7:48 AM, Scott Wood scottwood@freescale.com wrote:
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@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?
I don't know why these are implemented. it's existing one.
I was asking about the change you were making... I guess it's because of the *off + *size check, though unsigned overflow should work as well.
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;elseops.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.
No, len is unsigned int, but thislen is int. and can't overflow since. it's size down with blocksize at min macro.
I don't see what signedness has to do with it. You cap thislen at the lesser of len or blocksize, but then you round thislen *up* to the page size. Thus thislen can be greater than len.
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).
Basically it reads/writes data based on block. but last or first partial read/writes support.
Partial as in page-aligned but block-unaligned, or partial as in can start/stop on any byte?
Either way, it doesn't seem to handle the partial first block properly. It will see that len is greater than the blocksize and not pay any attention to the alignment of ofs.
@@ -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.
Right, but it's for exact display for user.
Ideally, you should put minor unrelated changes in a separate patch from more siginficant changes. At least note them in the changelog.
-Scott
participants (3)
-
Kyungmin Park -
Scott Wood -
Tuma