Hi,
On Mon, Feb 08, 2016 at 10:04:03AM -0800, Steve Rae wrote:
> Hi Maxime,
>
> On Mon, Feb 8, 2016 at 12:19 AM, Maxime Ripard <
> maxime.rip...@free-electrons.com> wrote:
>
> > 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.rip...@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 <s...@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
> >
> >
> So, to handle MMC versus NAND, I propose that we follow the same method
> used throughout 'fastboot':
>
> +#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
> total_blocks += blkcnt;
> +#endif
> +#ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV
> + /* TBD */
> +#endifEventually, we should support both. But is it even broken now? It was working just fine last time I tried. The write function is supposed to return the adjusted number of blocks that the write actually used (bad blocks included). Am I missing something? Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
