Hi Steve,
On Thu, Feb 04, 2016 at 10:51:00AM -0800, Steve Rae wrote:
Hi Maxime,
On Thu, Feb 4, 2016 at 4:20 AM, Maxime Ripard < maxime.ripard@free-electrons.com> wrote:
Hi Steve,
On Wed, Feb 03, 2016 at 12:46:02PM -0800, Steve Rae wrote:
remove logging of the 'skipped' blocks
Signed-off-by: Steve Rae srae@broadcom.com
common/image-sparse.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/common/image-sparse.c b/common/image-sparse.c index f02aee4..594bf4e 100644 --- a/common/image-sparse.c +++ b/common/image-sparse.c @@ -275,7 +275,6 @@ int store_sparse_image(sparse_storage_t *storage,
void *storage_priv,
sparse_buffer_t *buffer; uint32_t start; uint32_t total_blocks = 0;
uint32_t skipped = 0; int i; debug("=== Storage ===\n");@@ -334,7 +333,6 @@ int store_sparse_image(sparse_storage_t *storage,
void *storage_priv,
storage,sparse_header);
total_blocks += blkcnt;This change (in the first patch), updates the "total_blocks" value, so that the "next" chunk has the proper "starting block" address (see these line 363...) 362 ret = storage->write(storage, storage_priv, 363 start + total_blocks, 364 buffer_blk_cnt, 365 buffer->data); Without this change, all the blocks written to the partition after the CHUNK_TYPE_DONT_CARE blocks are corrupted (they are not in the correct location). So, even though we are not actually writing any blocks to this space, the space must be maintained!
Ah, yeah, understood.
I'm guessing it was working in my case since I had no DONT_CARE chunks in the first sparse image sent, and then only DONT_CARE chunks for the space you already wrote, we got that covered by last_offset... :/
So, yeah, it's broken...
(Recently, I am now understanding that with NAND, there may be more complications; probably cannot just increment the "total_blocks" -- I suspect that it is required to actually determine if there are bad blocks in this space, and update the "total_blocks" value accordingly....)
Yes, if you try to write to a bad block on NAND, you're actually going to write to the next block, which will introduce some offset, or you'll going to write to a block that's already been written.
Maxime
skipped += blkcnt;continue; }@@ -375,8 +373,8 @@ int store_sparse_image(sparse_storage_t *storage,
void *storage_priv,
sparse_put_data_buffer(buffer); }
debug("Wrote %d blocks, skipped %d, expected to write %d blocks\n",total_blocks, skipped,
debug("Wrote %d blocks, expected to write %d blocks\n",total_blocks,What's the rationale between those two patches?
see inline comment above
Do we really want to treat the DONT_CARE chunks as if they were written?
I suspect that we do, and "sparse_header->total_blks" actually includes them in the count too... This "total_blocks" count is actually the number of blocks "processed" (which may or may not include actually writing to the partition). IMO - I think counting the "skipped blocks is unnecessary.
Ok, sounds good.
Thanks! Maxime