public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [RFC] fastboot: sparse image handling and sessionId
@ 2016-04-16  4:14 Steve Rae
  2016-05-02  5:57 ` Steve Rae
  2016-05-09 15:00 ` Maxime Ripard
  0 siblings, 2 replies; 5+ messages in thread
From: Steve Rae @ 2016-04-16  4:14 UTC (permalink / raw)
  To: u-boot

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;


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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [RFC] fastboot: sparse image handling and sessionId
  2016-04-16  4:14 [U-Boot] [RFC] fastboot: sparse image handling and sessionId Steve Rae
@ 2016-05-02  5:57 ` Steve Rae
  2016-05-06 14:30   ` Tom Rini
  2016-05-09 15:00 ` Maxime Ripard
  1 sibling, 1 reply; 5+ messages in thread
From: Steve Rae @ 2016-05-02  5:57 UTC (permalink / raw)
  To: u-boot

On Apr 15, 2016 21:14, "Steve Rae" <steve.rae@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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [RFC] fastboot: sparse image handling and sessionId
  2016-05-02  5:57 ` Steve Rae
@ 2016-05-06 14:30   ` Tom Rini
  0 siblings, 0 replies; 5+ messages in thread
From: Tom Rini @ 2016-05-06 14:30 UTC (permalink / raw)
  To: u-boot

On Sun, May 01, 2016 at 10:57:55PM -0700, Steve Rae wrote:
> On Apr 15, 2016 21:14, "Steve Rae" <steve.rae@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?

I think if you can fix this issue, please do.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160506/174966b8/attachment.sig>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [RFC] fastboot: sparse image handling and sessionId
  2016-04-16  4:14 [U-Boot] [RFC] fastboot: sparse image handling and sessionId Steve Rae
  2016-05-02  5:57 ` Steve Rae
@ 2016-05-09 15:00 ` Maxime Ripard
  2016-05-13 17:48   ` Steve Rae
  1 sibling, 1 reply; 5+ messages in thread
From: Maxime Ripard @ 2016-05-09 15:00 UTC (permalink / raw)
  To: u-boot

Hi Steve,

Sorry for the late reply.

On Fri, Apr 15, 2016 at 09:14:41PM -0700, Steve Rae 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.

There was at the time, since it wasn't exactly clear that the next
chunks was holding DONT_CARE data to move the device pointer properly.

Also, when it comes to the NAND, it will have quite an impact on the
flashing time if we have to rescan the whole partition before the area
that we want to flash, at each new chunk. Especially when you already
have that info from the previous flashing.

> 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;
> 
> 
> 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...

I don't really find what's specific about the NAND here. It was
supposed to be a generic abstraction to be able to add easily support
new storage backends, and supporting several of them at the same
time. Adding that kind of code removes that genericity.

You can add a new callback in the passed structure to adjust the block
numbers, the implementation for MMC being trivial.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160509/2a6bcd2a/attachment.sig>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [RFC] fastboot: sparse image handling and sessionId
  2016-05-09 15:00 ` Maxime Ripard
@ 2016-05-13 17:48   ` Steve Rae
  0 siblings, 0 replies; 5+ messages in thread
From: Steve Rae @ 2016-05-13 17:48 UTC (permalink / raw)
  To: u-boot

On Mon, May 9, 2016 at 8:00 AM, Maxime Ripard <
maxime.ripard@free-electrons.com> wrote:

> Hi Steve,
>
> Sorry for the late reply.
>
> On Fri, Apr 15, 2016 at 09:14:41PM -0700, Steve Rae 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.
>
> There was at the time, since it wasn't exactly clear that the next
> chunks was holding DONT_CARE data to move the device pointer properly.
>
> Also, when it comes to the NAND, it will have quite an impact on the
> flashing time if we have to rescan the whole partition before the area
> that we want to flash, at each new chunk. Especially when you already
> have that info from the previous flashing.
>

I hear what you are saying about the impact to rescan NAND....
I my experience, however, images have CHUNK_TYPE_DONT_CARE scattered
through the sparse image (not just at the beginning and the end)
Thus, the CHUNK_TYPE_DONT_CARE must be processed properly, otherwise the
stored image is corrupted.

Then, to implement an algorithm which would speed up this process by
retaining data from one "session" to the next would require significant
more thought....


> > 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;
> >
> >
> > 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...
>
> I don't really find what's specific about the NAND here. It was
> supposed to be a generic abstraction to be able to add easily support
> new storage backends, and supporting several of them at the same
> time. Adding that kind of code removes that genericity.
>
> You can add a new callback in the passed structure to adjust the block
> numbers, the implementation for MMC being trivial.
>

Will implement abstraction in my next patch (possibly next week)  --
thanks...


>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-05-13 17:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-16  4:14 [U-Boot] [RFC] fastboot: sparse image handling and sessionId Steve Rae
2016-05-02  5:57 ` Steve Rae
2016-05-06 14:30   ` Tom Rini
2016-05-09 15:00 ` Maxime Ripard
2016-05-13 17:48   ` Steve Rae

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox