[U-Boot] [PATCH v4 4/5] usb/gadget: add the fastboot gadget

[Lukasz Majewski]

Hi Rob,

> On Fri, Apr 25, 2014 at 12:26 AM, Lukasz Majewski
> <l.majewski at samsung.com> wrote:
> > Hi Rob,
> >
> >> On Wed, Apr 23, 2014 at 6:02 AM, Lukasz Majewski
> >> <l.majewski at samsung.com> wrote:
> >> > Hi Rob,
> >> >
> >> >> From: Sebastian Siewior <bigeasy at linutronix.de>
> >> >>
> >> >> This patch contains an implementation of the fastboot protocol
> >> >> on the device side and documentation. This is based on USB
> >> >> download gadget infrastructure. The fastboot function
> >> >> implements the getvar, reboot, download and reboot commands.
> >> >> What is missing is the flash handling i.e. writting the image
> >> >> to media.
> >> >>
> >>
> >> [...]
> 
> >> >         Please consider using dfu_get_buf() from dfu.c to provide
> >> >         dynamically allocated and controlled buffer instead of
> >> > the CONFIG_USB_FASTBOOT_BUF_ADDR and _SIZE.
> >> >
> >> >         Another advantage of this code is the ability to set
> >> >         "dfu_bufsiz" env variable with size of the buffer.
> >>
> >> I considered this already. I certainly don't like reinventing
> >> things which was why I originally used loadaddr and added loadsize
> >> to provide a defined load buffer size.
> >>
> >> The problem is fastboot needs enough RAM to download an entire
> >> sparse filesystem. I have no idea what size exactly is typical or
> >> required, but it seems that we want to be able to use nearly all
> >> free RAM. We can talk all we want about how this is a crappy
> >> design, but it is what it is. This is how the protocol works.
> >
> > I understand you :-). The same situation was with DFU on the
> > beginning. Large buffer with starting address defined per board.
> >
> > Then, after some discussion, we come to conclusion that it would be
> > better to increase malloc pool and dynamically allocate buffer.
> >
> > Am I correct, that you don't know beforehand what would be the size
> > of downloaded file - maybe 5 MiB or maybe 512 MiB? Also from your
> > descriptor it seems like fastboot protocol don't want to impose any
> > restrictions about the size. Is it user's responsibility to send
> > data smaller than RAM size?
> 
> Correct. The client side will check the size which is one of the
> variables. I searched around some to try to get an idea of what the
> typical buffer size is without much luck.

Ok, I see.

> 
> > In the DFU/THOR we store data in buffer size packets (32 MiB). It
> > also has some drawbacks - with large raw data images we cannot
> > download the whole (e.g. rootfs) image and beforehand flashing
> > check integrity.
> >
> > One question - when your board has e.g. 768 MiB of "available" RAM,
> > then is the size of large rootfs restricted to this size?
> 
> Yes, but that is not the size of the rootfs partition. The downloaded
> files are sparse. I would guess only the minimal filesystem is laid
> down this way and most optional pieces are installed later.

Or it is resized when needed.

> 
> >>
> >> The problem with the DFU buffer is it is allocated from the malloc
> >> region. If we just increase the malloc region to be close to total
> >> RAM size then we will start to break other commands like tftp and
> >> fsload which typically just use the RAM u-boot is not using (i.e.
> >> all but the end of memory). The only platforms which have more
> >> than a few MB for malloc are the ones that enable DFU.
> >
> > Correct. On the other hand when we want to allocate too large buffer
> > we receive error from malloc and flashing is aborted. No harm is
> > done.
> 
> If increasing your malloc region breaks various load commands, then
> harm is done.

To be more precise - in our boards we have at least 1 GiB of RAM. The
"large" malloc'ed buffer for DFU has 32 MiB at our boards. The total
pool size is 80 MiB, which is less than 10% of total RAM. Hence I
don't have problems similar to yours.

My little request - please make those defines to be easily reusable at
other boards.

> 
> Rob


-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group