[U-Boot] [RFC] fastboot: sparse image handling and sessionId

[Steve Rae]

On Apr 15, 2016 21:14, "Steve Rae" <steve.rae at broadcom.com> wrote:
>
> Maxime,
>
> I suspect that the issue fixed
>   in commit c7529db    fastboot: sparse: fix block addressing for
> don't care chunk type
>
> complicated the assumptions which led to the implementation of the
"sessionId"
>   in commit 1f8690a    sparse: Implement several chunks flashing
>
>     The fastboot client will split the sparse images into several chunks
if the
>     image that it tries to flash is bigger than what the device can
handle.
>
>     In such a case, the bootloader is supposed to retain the last offset
to
>     which it wrote to, so that it can resume the writes at the right
offset
>     when flashing the next chunk.
>
>     Retain the last offset we used, and use the session ID to know if we
need
>     it or not.
>
>
> Testing of this scenario has revealed that "fastboot flash" works as
follows:
>
> When the image to download is larger than the
>     "target reported max download size of 40960000 bytes",
> the fastboot client (running on the host) essentially performs
> multiple, independent fastboot download/write steps.
>
> As seen in the logs below, each download/write step:
> (a) starts at the same address (the address of the first block of the
> partition...)
> (b) ends at the same address (start address plus number of blocks
"written"...)
> (c) and uses the CHUNK_TYPE_DONT_CARE 'chunk_sz' to manipulate the
> addresses to achieve (a) and (b)
>
> Here is an example of the first three fastboot download/writes for a
> large image to "file-system":
> - in the first, data is written to 0x24000-0x37848, then there is a
> CHUNK_TYPE_DONT_CARE which "moves" the address pointer to 0x424000
> - in the second, it starts with a CHUNK_TYPE_DONT_CARE which "moves"
> the address from 0x24000 to 0x37848, then writes from 0x37848-0x4b098,
> then there is a CHUNK_TYPE_DONT_CARE which "moves" the address pointer
> to 0x424000
> - in the third, it starts with a CHUNK_TYPE_DONT_CARE which "moves"
> the address from 0x24000 to 0x4b098, then writes from 0x4b098-0x5e8e0,
> then there is a CHUNK_TYPE_DONT_CARE which "moves" the address pointer
> to 0x424000
>
>
> Starting download of 40932412 bytes
> ..........................................................................
> downloading of 40932412 bytes finished
> Flashing sparse image at offset 147456
> Flashing Sparse Image
> - this is the starting                      address: 0x24000
> ++++ (writing to flash)
> - this is the CHUNK_TYPE_DONT_CARE (before) address: 0x37848
> - this is the CHUNK_TYPE_DONT_CARE (after)  address: 0x424000
> - this is the ending                        address: 0x424000
> ........ wrote 4194304 blocks to 'file-system'
>
> Starting download of 40954208 bytes
> ..........................................................................
> downloading of 40954208 bytes finished
> Flashing sparse image at offset 147456
> Flashing Sparse Image
> - this is the starting                      address: 0x24000
> - this is the CHUNK_TYPE_DONT_CARE (before) address: 0x24000
> - this is the CHUNK_TYPE_DONT_CARE (after)  address: 0x37848
> ++++ (writing to flash)
> - this is the CHUNK_TYPE_DONT_CARE (before) address: 0x4b098
> - this is the CHUNK_TYPE_DONT_CARE (after)  address: 0x424000
> - this is the ending                        address: 0x424000
> ........ wrote 4194304 blocks to 'file-system'
>
> Starting download of 40955896 bytes
> ..........................................................................
> downloading of 40955896 bytes finished
> Flashing sparse image at offset 147456
> Flashing Sparse Image
> - this is the starting                      address: 0x24000
> - this is the CHUNK_TYPE_DONT_CARE (before) address: 0x24000
> - this is the CHUNK_TYPE_DONT_CARE (after)  address: 0x4b098
> ++++ (writing to flash)
> - this is the CHUNK_TYPE_DONT_CARE (before) address: 0x5e8e0
> - this is the CHUNK_TYPE_DONT_CARE (after)  address: 0x424000
> - this is the ending                        address: 0x424000
> ........ wrote 4194304 blocks to 'file-system'
>
> [... snip ...]
>
>
> Therefore, I think that the "sessonId" that is implemented in U-Boot
> is actually incorrect. There is no need to keep track of the addresses
> as each "fastboot flash" download/write step is completely
> independent.
> And I actually need to remove it in order to get this working again:
>
> -       if (session_id > 0)
> -               start = last_offset;
> -       else
> -               start = storage->start;
> +       start = storage->start;
>
Maxime:
(nudge)
Any comments here?
Would you like me to submit a patch to fix this?
Thanks, Steve
>
> Additionally (unfortunately) as with the previous discussion that we
> had about this code:
>
> #ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
>                         total_blocks += blkcnt;
> #endif
>
> the NAND implementation requires a method to process "blkcnt" to
> ensure the blkcnt skips the bad blocks...
>
>
> Please advise, Steve