| Message ID | 1355153064-6008-1-git-send-email-panto@antoniou-consulting.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Scott Wood |
| Headers | show |
On 12/10/2012 09:24:24 AM, Pantelis Antoniou wrote: > When accessing nand any bad blocks encountered are skipped, with no > indication about the amount of bad blocks encountered. > While this is normally fine, when you have to write a large amount > of data in chunks, you need to account for the skipped amount due > to the presence of the bad blocks. > > nand_extend_skip_bad() returns the offset where the next access > should occur. s/extend/extent/ > > Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com> > --- > drivers/mtd/nand/nand_util.c | 50 > ++++++++++++++++++++++++++++++++++++++++++++ > include/nand.h | 2 ++ > 2 files changed, 52 insertions(+) > > diff --git a/drivers/mtd/nand/nand_util.c > b/drivers/mtd/nand/nand_util.c > index 2ba0c5e..a25a4cb 100644 > --- a/drivers/mtd/nand/nand_util.c > +++ b/drivers/mtd/nand/nand_util.c > @@ -684,6 +684,56 @@ int nand_read_skip_bad(nand_info_t *nand, loff_t > offset, size_t *length, > return 0; > } > > +/** > + * nand_extent_skip_bad: > + * > + * Find the extent of a chunk, return the offset where it ends > + * Blocks that are marked bad are skipped and the next block is > examined > + * instead as long as the extend is short enough to fit even after > skipping the > + * bad blocks. > + * > + * @param nand NAND device > + * @param offset offset in flash > + * @param length extend length > + * @return next offset in case of success (loff_t)-1 on error > + */ Would it be better to return this information from existing read/write functions -- either instead of or in addition to exporting this functionality? > +loff_t nand_extent_skip_bad(nand_info_t *nand, loff_t offset, size_t > length) > +{ > + size_t block_len, block_off; > + loff_t block_start; > + > + if ((offset & (nand->writesize - 1)) != 0) { > + printf ("%s: Attempt to check extend non page aligned > data\n", > + __func__); > + return (loff_t)-1; > + } > + > + while (length > 0) { > + > + if (offset >= nand->size) { > + printf("%s: offset >= nand->size (%llx >= > %llx)\n", > + __func__, offset, nand->size); > + return (loff_t)-1; > + } > + > + block_start = offset & ~(loff_t)(nand->erasesize - 1); > + block_off = offset & (nand->erasesize - 1); > + block_len = nand->erasesize - block_off; > + if (block_len > length) /* left over */ > + block_len = length; > + > + if (!nand_block_isbad(nand, block_start)) > + length -= block_len; > + else > + debug("%s: bad block at %llx (left %x)\n", > + __func__, block_start, length); > + > + offset += block_len; > + } > + > + return offset; > +} This seems duplicative of check_skip_len(). -Scott
Hi Scott, On Dec 11, 2012, at 12:53 AM, Scott Wood wrote: > On 12/10/2012 09:24:24 AM, Pantelis Antoniou wrote: >> When accessing nand any bad blocks encountered are skipped, with no >> indication about the amount of bad blocks encountered. >> While this is normally fine, when you have to write a large amount >> of data in chunks, you need to account for the skipped amount due >> to the presence of the bad blocks. >> nand_extend_skip_bad() returns the offset where the next access >> should occur. > > s/extend/extent/ > Yeah. >> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com> >> --- >> drivers/mtd/nand/nand_util.c | 50 ++++++++++++++++++++++++++++++++++++++++++++ >> include/nand.h | 2 ++ >> 2 files changed, 52 insertions(+) >> diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c >> index 2ba0c5e..a25a4cb 100644 >> --- a/drivers/mtd/nand/nand_util.c >> +++ b/drivers/mtd/nand/nand_util.c >> @@ -684,6 +684,56 @@ int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length, >> return 0; >> } >> +/** >> + * nand_extent_skip_bad: >> + * >> + * Find the extent of a chunk, return the offset where it ends >> + * Blocks that are marked bad are skipped and the next block is examined >> + * instead as long as the extend is short enough to fit even after skipping the >> + * bad blocks. >> + * >> + * @param nand NAND device >> + * @param offset offset in flash >> + * @param length extend length >> + * @return next offset in case of success (loff_t)-1 on error >> + */ > > Would it be better to return this information from existing read/write functions -- either instead of or in addition to exporting this functionality? > Yes it would. However that would require modifying all callers, which would be a hard sell when there's only one user of it. >> +loff_t nand_extent_skip_bad(nand_info_t *nand, loff_t offset, size_t length) >> +{ >> + size_t block_len, block_off; >> + loff_t block_start; >> + >> + if ((offset & (nand->writesize - 1)) != 0) { >> + printf ("%s: Attempt to check extend non page aligned data\n", >> + __func__); >> + return (loff_t)-1; >> + } >> + >> + while (length > 0) { >> + >> + if (offset >= nand->size) { >> + printf("%s: offset >= nand->size (%llx >= %llx)\n", >> + __func__, offset, nand->size); >> + return (loff_t)-1; >> + } >> + >> + block_start = offset & ~(loff_t)(nand->erasesize - 1); >> + block_off = offset & (nand->erasesize - 1); >> + block_len = nand->erasesize - block_off; >> + if (block_len > length) /* left over */ >> + block_len = length; >> + >> + if (!nand_block_isbad(nand, block_start)) >> + length -= block_len; >> + else >> + debug("%s: bad block at %llx (left %x)\n", >> + __func__, block_start, length); >> + >> + offset += block_len; >> + } >> + >> + return offset; >> +} > > This seems duplicative of check_skip_len(). > It is. check_skip_len doesn't return the information I need. I could modify check_skip_len with an extra parameter if that's going to be OK with you. > -Scott Regards -- Pantelis
On 12/11/2012 03:40:53 AM, Pantelis Antoniou wrote: > Hi Scott, > > On Dec 11, 2012, at 12:53 AM, Scott Wood wrote: > > >> +/** > >> + * nand_extent_skip_bad: > >> + * > >> + * Find the extent of a chunk, return the offset where it ends > >> + * Blocks that are marked bad are skipped and the next block is > examined > >> + * instead as long as the extend is short enough to fit even > after skipping the > >> + * bad blocks. > >> + * > >> + * @param nand NAND device > >> + * @param offset offset in flash > >> + * @param length extend length > >> + * @return next offset in case of success (loff_t)-1 on error > >> + */ > > > > Would it be better to return this information from existing > read/write functions -- either instead of or in addition to exporting > this functionality? > > > > Yes it would. However that would require modifying all callers, which > would be a hard sell when there's only one user of it. There aren't that many callers, and it's all common code (so no issue with testing on obscure hardware). > > This seems duplicative of check_skip_len(). > > > > It is. check_skip_len doesn't return the information I need. I could > modify check_skip_len with > an extra parameter if that's going to be OK with you. Yes, please modify check_skip_len() instead. -Scott
Hi Scott, On Dec 11, 2012, at 7:13 PM, Scott Wood wrote: > On 12/11/2012 03:40:53 AM, Pantelis Antoniou wrote: >> Hi Scott, >> On Dec 11, 2012, at 12:53 AM, Scott Wood wrote: >> >> +/** >> >> + * nand_extent_skip_bad: >> >> + * >> >> + * Find the extent of a chunk, return the offset where it ends >> >> + * Blocks that are marked bad are skipped and the next block is examined >> >> + * instead as long as the extend is short enough to fit even after skipping the >> >> + * bad blocks. >> >> + * >> >> + * @param nand NAND device >> >> + * @param offset offset in flash >> >> + * @param length extend length >> >> + * @return next offset in case of success (loff_t)-1 on error >> >> + */ >> > >> > Would it be better to return this information from existing read/write functions -- either instead of or in addition to exporting this functionality? >> > >> Yes it would. However that would require modifying all callers, which would be a hard sell when there's only one user of it. > > There aren't that many callers, and it's all common code (so no issue with testing on obscure hardware). > >> > This seems duplicative of check_skip_len(). >> > >> It is. check_skip_len doesn't return the information I need. I could modify check_skip_len with >> an extra parameter if that's going to be OK with you. > > Yes, please modify check_skip_len() instead. > > -Scott Nice, hope I'll get around doing it today or tomorrow. Regards -- Pantelis
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index 2ba0c5e..a25a4cb 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -684,6 +684,56 @@ int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length, return 0; } +/** + * nand_extent_skip_bad: + * + * Find the extent of a chunk, return the offset where it ends + * Blocks that are marked bad are skipped and the next block is examined + * instead as long as the extend is short enough to fit even after skipping the + * bad blocks. + * + * @param nand NAND device + * @param offset offset in flash + * @param length extend length + * @return next offset in case of success (loff_t)-1 on error + */ +loff_t nand_extent_skip_bad(nand_info_t *nand, loff_t offset, size_t length) +{ + size_t block_len, block_off; + loff_t block_start; + + if ((offset & (nand->writesize - 1)) != 0) { + printf ("%s: Attempt to check extend non page aligned data\n", + __func__); + return (loff_t)-1; + } + + while (length > 0) { + + if (offset >= nand->size) { + printf("%s: offset >= nand->size (%llx >= %llx)\n", + __func__, offset, nand->size); + return (loff_t)-1; + } + + block_start = offset & ~(loff_t)(nand->erasesize - 1); + block_off = offset & (nand->erasesize - 1); + block_len = nand->erasesize - block_off; + if (block_len > length) /* left over */ + block_len = length; + + if (!nand_block_isbad(nand, block_start)) + length -= block_len; + else + debug("%s: bad block at %llx (left %x)\n", + __func__, block_start, length); + + offset += block_len; + } + + return offset; +} + #ifdef CONFIG_CMD_NAND_TORTURE /** diff --git a/include/nand.h b/include/nand.h index dded4e2..710c11a 100644 --- a/include/nand.h +++ b/include/nand.h @@ -168,3 +168,5 @@ __attribute__((noreturn)) void nand_boot(void); #define ENV_OFFSET_SIZE 8 int get_nand_env_oob(nand_info_t *nand, unsigned long *result); #endif + +loff_t nand_extent_skip_bad(nand_info_t *nand, loff_t offset, size_t length);
When accessing nand any bad blocks encountered are skipped, with no indication about the amount of bad blocks encountered. While this is normally fine, when you have to write a large amount of data in chunks, you need to account for the skipped amount due to the presence of the bad blocks. nand_extend_skip_bad() returns the offset where the next access should occur. Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com> --- drivers/mtd/nand/nand_util.c | 50 ++++++++++++++++++++++++++++++++++++++++++++ include/nand.h | 2 ++ 2 files changed, 52 insertions(+)