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

From: Sebastian Siewior <bigeasy@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.

v3 (Rob Herring):
This is based on http://patchwork.ozlabs.org/patch/126798/ with the
following changes:
- Rebase to current mainline and updates for current gadget API
- Use SPDX identifiers for licenses
- Traced the history and added missing copyright to cmd_fastboot.c
- Use load_addr/load_size for transfer buffer
- Allow vendor strings to be optional
- Set vendor/product ID from config defines
- Allow Ctrl-C to exit fastboot mode
v4:
- Major re-write to use the USB download gadget. Consolidated function
code to a single file.
- Moved globals into single struct.
- Use puts and putc as appropriate.
- Added CONFIG_USB_FASTBOOT_BUF_ADDR and CONFIG_USB_FASTBOOT_BUF_SIZE to
set the fastboot transfer buffer.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 common/Makefile                      |   2 +
 common/cmd_fastboot.c                |  36 +++
 doc/README.android-fastboot          |  91 ++++++
 doc/README.android-fastboot-protocol | 170 ++++++++++++
 drivers/usb/gadget/Makefile          |   1 +
 drivers/usb/gadget/f_fastboot.c      | 518 +++++++++++++++++++++++++++++++++++
 drivers/usb/gadget/g_dnl.c           |   6 +
 include/fastboot.h                   |  22 ++
 8 files changed, 846 insertions(+)
 create mode 100644 common/cmd_fastboot.c
 create mode 100644 doc/README.android-fastboot
 create mode 100644 doc/README.android-fastboot-protocol
 create mode 100644 drivers/usb/gadget/f_fastboot.c
 create mode 100644 include/fastboot.h

Hi Rob,

> From: Sebastian Siewior <bigeasy@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.
> 
> v3 (Rob Herring):
> This is based on http://patchwork.ozlabs.org/patch/126798/ with the
> following changes:
> - Rebase to current mainline and updates for current gadget API
> - Use SPDX identifiers for licenses
> - Traced the history and added missing copyright to cmd_fastboot.c
> - Use load_addr/load_size for transfer buffer
> - Allow vendor strings to be optional
> - Set vendor/product ID from config defines
> - Allow Ctrl-C to exit fastboot mode
> v4:
> - Major re-write to use the USB download gadget. Consolidated function
> code to a single file.
> - Moved globals into single struct.
> - Use puts and putc as appropriate.
> - Added CONFIG_USB_FASTBOOT_BUF_ADDR and CONFIG_USB_FASTBOOT_BUF_SIZE
> to set the fastboot transfer buffer.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  common/Makefile                      |   2 +
>  common/cmd_fastboot.c                |  36 +++
>  doc/README.android-fastboot          |  91 ++++++
>  doc/README.android-fastboot-protocol | 170 ++++++++++++
>  drivers/usb/gadget/Makefile          |   1 +
>  drivers/usb/gadget/f_fastboot.c      | 518
> +++++++++++++++++++++++++++++++++++
> drivers/usb/gadget/g_dnl.c           |   6 +
> include/fastboot.h                   |  22 ++ 8 files changed, 846
> insertions(+) create mode 100644 common/cmd_fastboot.c
>  create mode 100644 doc/README.android-fastboot
>  create mode 100644 doc/README.android-fastboot-protocol
>  create mode 100644 drivers/usb/gadget/f_fastboot.c
>  create mode 100644 include/fastboot.h
> 
> diff --git a/common/Makefile b/common/Makefile
> index da208f3..fe1d8b9 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -167,6 +167,8 @@ obj-y += cmd_usb.o
>  obj-y += usb.o usb_hub.o
>  obj-$(CONFIG_USB_STORAGE) += usb_storage.o
>  endif
> +obj-$(CONFIG_CMD_FASTBOOT) += cmd_fastboot.o
> +
>  obj-$(CONFIG_CMD_USB_MASS_STORAGE) += cmd_usb_mass_storage.o
>  obj-$(CONFIG_CMD_THOR_DOWNLOAD) += cmd_thordown.o
>  obj-$(CONFIG_CMD_XIMG) += cmd_ximg.o
> diff --git a/common/cmd_fastboot.c b/common/cmd_fastboot.c
> new file mode 100644
> index 0000000..ce7e198
> --- /dev/null
> +++ b/common/cmd_fastboot.c
> @@ -0,0 +1,36 @@
> +/*
> + * Copyright 2008 - 2009 Windriver, <www.windriver.com>
> + * Author: Tom Rix <Tom.Rix@windriver.com>
> + *
> + * (C) Copyright 2014 Linaro, Ltd.
> + * Rob Herring <robh@kernel.org>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +#include <common.h>
> +#include <command.h>
> +#include <g_dnl.h>
> +
> +static int do_fastboot(cmd_tbl_t *cmdtp, int flag, int argc, char
> *const argv[]) +{
> +	int ret;
> +
> +	ret = g_dnl_register("fastboot");
> +	if (ret)
> +		return ret;
> +
> +	while (1) {
> +		if (ctrlc())
> +			break;
> +		usb_gadget_handle_interrupts();
> +	}
> +
> +	g_dnl_unregister();
> +	return CMD_RET_SUCCESS;
> +}
> +
> +U_BOOT_CMD(
> +	fastboot,	1,	1,	do_fastboot,
> +	"fastboot - enter USB Fastboot protocol",
> +	""
> +);
> diff --git a/doc/README.android-fastboot b/doc/README.android-fastboot
> new file mode 100644
> index 0000000..f1d128c
> --- /dev/null
> +++ b/doc/README.android-fastboot
> @@ -0,0 +1,91 @@
> +Android Fastboot
> +~~~~~~~~~~~~~~~~
> +
> +Overview
> +========
> +The protocol that is used over USB is described in
> +README.android-fastboot-protocol in same directory.
> +
> +The current implementation does not yet support the flash and erase
> +commands.
> +
> +Client installation
> +===================
> +The counterpart to this gadget is the fastboot client which can
> +be found in Android's platform/system/core repository in the fastboot
> +folder. It runs on Windows, Linux and even OSX. Linux user are lucky
> since +they only need libusb.
> +Windows users need to bring some time until they have Android SDK
> (currently +http://dl.google.com/android/installer_r12-windows.exe)
> installed. You +need to install ADB package which contains the
> required glue libraries for +accessing USB. Also you need "Google USB
> driver package" and "SDK platform +tools". Once installed the usb
> driver is placed in your SDK folder under +extras\google\usb_driver.
> The android_winusb.inf needs a line like +
> +   %SingleBootLoaderInterface% = USB_Install, USB\VID_0451&PID_D022
> +
> +either in the [Google.NTx86] section for 32bit Windows or
> [Google.NTamd64] +for 64bit Windows. VID and PID should match
> whatever the fastboot is +advertising.
> +
> +Board specific
> +==============
> +The fastboot gadget relies on the USB download gadget, so the
> following +options must be configured:
> +
> +CONFIG_USBDOWNLOAD_GADGET
> +CONFIG_G_DNL_VENDOR_NUM
> +CONFIG_G_DNL_PRODUCT_NUM
> +CONFIG_G_DNL_MANUFACTURER
> +
> +The fastboot function is enabled by defining CONFIG_CMD_FASTBOOT and
> +CONFIG_ANDROID_BOOT_IMAGE.
> +
> +The fastboot protocol requires a large memory buffer for downloads.
> This +buffer should be as large as possible for a platform. The
> location of the +buffer and size are set with
> CONFIG_USB_FASTBOOT_BUF_ADDR and +CONFIG_USB_FASTBOOT_BUF_SIZE.
> +
> +In Action
> +=========
> +Enter into fastboot by executing the fastboot command in u-boot and
> you +should see:
> +|GADGET DRIVER: usb_dnl_fastboot
> +
> +On the client side you can fetch the bootloader version for instance:
> +|>fastboot getvar bootloader-version
> +|bootloader-version: U-Boot 2014.04-00005-gd24cabc
> +|finished. total time: 0.000s
> +
> +or initiate a reboot:
> +|>fastboot reboot
> +
> +and once the client comes back, the board should reset.
> +
> +You can also specify a kernel image to boot. You have to either
> specify +the an image in Android format _or_ pass a binary kernel and
> let the +fastboot client wrap the Android suite around it. On OMAP
> for instance you +take zImage kernel and pass it to the fastboot
> client: +
> +|>fastboot -b 0x80000000 -c "console=ttyO2 earlyprintk root=/dev/ram0
> +|	mem=128M" boot zImage
> +|creating boot image...
> +|creating boot image - 1847296 bytes
> +|downloading 'boot.img'...
> +|OKAY [  2.766s]
> +|booting...
> +|OKAY [ -0.000s]
> +|finished. total time: 2.766s
> +
> +and on the gadget side you should see:
> +|Starting download of 1847296 bytes
> +|........................................................
> +|downloading of 1847296 bytes finished
> +|Booting kernel..
> +|## Booting Android Image at 0x81000000 ...
> +|Kernel load addr 0x80008000 size 1801 KiB
> +|Kernel command line: console=ttyO2 earlyprintk root=/dev/ram0
> mem=128M +|   Loading Kernel Image ... OK
> +|OK
> +|
> +|Starting kernel ...
> diff --git a/doc/README.android-fastboot-protocol
> b/doc/README.android-fastboot-protocol new file mode 100644
> index 0000000..e9e7166
> --- /dev/null
> +++ b/doc/README.android-fastboot-protocol
> @@ -0,0 +1,170 @@
> +FastBoot  Version  0.4
> +----------------------
> +
> +The fastboot protocol is a mechanism for communicating with
> bootloaders +over USB.  It is designed to be very straightforward to
> implement, to +allow it to be used across a wide range of devices and
> from hosts running +Linux, Windows, or OSX.
> +
> +
> +Basic Requirements
> +------------------
> +
> +* Two bulk endpoints (in, out) are required
> +* Max packet size must be 64 bytes for full-speed and 512 bytes for
> +  high-speed USB
> +* The protocol is entirely host-driven and synchronous (unlike the
> +  multi-channel, bi-directional, asynchronous ADB protocol)
> +
> +
> +Transport and Framing
> +---------------------
> +
> +1. Host sends a command, which is an ascii string in a single
> +   packet no greater than 64 bytes.
> +
> +2. Client response with a single packet no greater than 64 bytes.
> +   The first four bytes of the response are "OKAY", "FAIL", "DATA",
> +   or "INFO".  Additional bytes may contain an (ascii) informative
> +   message.
> +
> +   a. INFO -> the remaining 60 bytes are an informative message
> +      (providing progress or diagnostic messages).  They should
> +      be displayed and then step #2 repeats
> +
> +   b. FAIL -> the requested command failed.  The remaining 60 bytes
> +      of the response (if present) provide a textual failure message
> +      to present to the user.  Stop.
> +
> +   c. OKAY -> the requested command completed successfully.  Go to #5
> +
> +   d. DATA -> the requested command is ready for the data phase.
> +      A DATA response packet will be 12 bytes long, in the form of
> +      DATA00000000 where the 8 digit hexidecimal number represents
> +      the total data size to transfer.
> +
> +3. Data phase.  Depending on the command, the host or client will
> +   send the indicated amount of data.  Short packets are always
> +   acceptable and zero-length packets are ignored.  This phase
> continues
> +   until the client has sent or received the number of bytes
> indicated
> +   in the "DATA" response above.
> +
> +4. Client responds with a single packet no greater than 64 bytes.
> +   The first four bytes of the response are "OKAY", "FAIL", or
> "INFO".
> +   Similar to #2:
> +
> +   a. INFO -> display the remaining 60 bytes and return to #4
> +
> +   b. FAIL -> display the remaining 60 bytes (if present) as a
> failure
> +      reason and consider the command failed.  Stop.
> +
> +   c. OKAY -> success.  Go to #5
> +
> +5. Success.  Stop.
> +
> +
> +Example Session
> +---------------
> +
> +Host:    "getvar:version"        request version variable
> +
> +Client:  "OKAY0.4"               return version "0.4"
> +
> +Host:    "getvar:nonexistant"    request some undefined variable
> +
> +Client:  "OKAY"                  return value ""
> +
> +Host:    "download:00001234"     request to send 0x1234 bytes of data
> +
> +Client:  "DATA00001234"          ready to accept data
> +
> +Host:    < 0x1234 bytes >        send data
> +
> +Client:  "OKAY"                  success
> +
> +Host:    "flash:bootloader"      request to flash the data to the
> bootloader +
> +Client:  "INFOerasing flash"     indicate status / progress
> +         "INFOwriting flash"
> +         "OKAY"                  indicate success
> +
> +Host:    "powerdown"             send a command
> +
> +Client:  "FAILunknown command"   indicate failure
> +
> +
> +Command Reference
> +-----------------
> +
> +* Command parameters are indicated by printf-style escape sequences.
> +
> +* Commands are ascii strings and sent without the quotes (which are
> +  for illustration only here) and without a trailing 0 byte.
> +
> +* Commands that begin with a lowercase letter are reserved for this
> +  specification.  OEM-specific commands should not begin with a
> +  lowercase letter, to prevent incompatibilities with future specs.
> +
> + "getvar:%s"           Read a config/version variable from the
> bootloader.
> +                       The variable contents will be returned after
> the
> +                       OKAY response.
> +
> + "download:%08x"       Write data to memory which will be later used
> +                       by "boot", "ramdisk", "flash", etc.  The
> client
> +                       will reply with "DATA%08x" if it has enough
> +                       space in RAM or "FAIL" if not.  The size of
> +                       the download is remembered.
> +
> +  "verify:%08x"        Send a digital signature to verify the
> downloaded
> +                       data.  Required if the bootloader is "secure"
> +                       otherwise "flash" and "boot" will be ignored.
> +
> +  "flash:%s"           Write the previously downloaded image to the
> +                       named partition (if possible).
> +
> +  "erase:%s"           Erase the indicated partition (clear to 0xFFs)
> +
> +  "boot"               The previously downloaded data is a boot.img
> +                       and should be booted according to the normal
> +                       procedure for a boot.img
> +
> +  "continue"           Continue booting as normal (if possible)
> +
> +  "reboot"             Reboot the device.
> +
> +  "reboot-bootloader"  Reboot back into the bootloader.
> +                       Useful for upgrade processes that require
> upgrading
> +                       the bootloader and then upgrading other
> partitions
> +                       using the new bootloader.
> +
> +  "powerdown"          Power off the device.
> +
> +
> +
> +Client Variables
> +----------------
> +
> +The "getvar:%s" command is used to read client variables which
> +represent various information about the device and the software
> +on it.
> +
> +The various currently defined names are:
> +
> +  version             Version of FastBoot protocol supported.
> +                      It should be "0.3" for this document.
> +
> +  version-bootloader  Version string for the Bootloader.
> +
> +  version-baseband    Version string of the Baseband Software
> +
> +  product             Name of the product
> +
> +  serialno            Product serial number
> +
> +  secure              If the value is "yes", this is a secure
> +                      bootloader requiring a signature before
> +                      it will install or boot images.
> +
> +Names starting with a lowercase character are reserved by this
> +specification.  OEM-specific names should not start with lowercase
> +characters.
> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> index 804a2bd..3375f68 100644
> --- a/drivers/usb/gadget/Makefile
> +++ b/drivers/usb/gadget/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_THOR_FUNCTION) += f_thor.o
>  obj-$(CONFIG_USBDOWNLOAD_GADGET) += g_dnl.o
>  obj-$(CONFIG_DFU_FUNCTION) += f_dfu.o
>  obj-$(CONFIG_USB_GADGET_MASS_STORAGE) += f_mass_storage.o
> +obj-$(CONFIG_CMD_FASTBOOT) += f_fastboot.o
>  endif
>  ifdef CONFIG_USB_ETHER
>  obj-y += ether.o
> diff --git a/drivers/usb/gadget/f_fastboot.c
> b/drivers/usb/gadget/f_fastboot.c new file mode 100644
> index 0000000..514279f
> --- /dev/null
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -0,0 +1,518 @@
> +/*
> + * (C) Copyright 2008 - 2009
> + * Windriver, <www.windriver.com>
> + * Tom Rix <Tom.Rix@windriver.com>
> + *
> + * Copyright 2011 Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> + *
> + * Copyright 2014 Linaro, Ltd.
> + * Rob Herring <robh@kernel.org>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +#include <common.h>
> +#include <errno.h>
> +#include <malloc.h>
> +#include <linux/usb/ch9.h>
> +#include <linux/usb/gadget.h>
> +#include <linux/usb/composite.h>
> +#include <linux/compiler.h>
> +#include <version.h>
> +#include <g_dnl.h>
> +#include <fastboot.h>
> +
> +#define FASTBOOT_VERSION		"0.4"
> +
> +#define FASTBOOT_INTERFACE_CLASS	0xff
> +#define FASTBOOT_INTERFACE_SUB_CLASS	0x42
> +#define FASTBOOT_INTERFACE_PROTOCOL	0x03
> +
> +#define RX_ENDPOINT_MAXIMUM_PACKET_SIZE_2_0  (0x0200)
> +#define RX_ENDPOINT_MAXIMUM_PACKET_SIZE_1_1  (0x0040)
> +#define TX_ENDPOINT_MAXIMUM_PACKET_SIZE      (0x0040)
> +
> +/* The 64 defined bytes plus \0 */
> +#define RESPONSE_LEN	(64 + 1)
> +
> +#define EP_BUFFER_SIZE			4096
> +
> +struct f_fastboot {
> +	struct usb_function usb_function;
> +
> +	/* IN/OUT EP's and correspoinding requests */
> +	struct usb_ep *in_ep, *out_ep;
> +	struct usb_request *in_req, *out_req;
> +};
> +
> +static inline struct f_fastboot *func_to_fastboot(struct
> usb_function *f) +{
> +	return container_of(f, struct f_fastboot, usb_function);
> +}
> +
> +static struct f_fastboot *fastboot_func;
> +static unsigned int download_size;
> +static unsigned int download_bytes;
> +
> +static struct usb_endpoint_descriptor fs_ep_in = {
> +	.bLength            = USB_DT_ENDPOINT_SIZE,
> +	.bDescriptorType    = USB_DT_ENDPOINT,
> +	.bEndpointAddress   = USB_DIR_IN,
> +	.bmAttributes       = USB_ENDPOINT_XFER_BULK,
> +	.wMaxPacketSize     = TX_ENDPOINT_MAXIMUM_PACKET_SIZE,
> +	.bInterval          = 0x00,
> +};
> +
> +static struct usb_endpoint_descriptor fs_ep_out = {
> +	.bLength		= USB_DT_ENDPOINT_SIZE,
> +	.bDescriptorType	= USB_DT_ENDPOINT,
> +	.bEndpointAddress	= USB_DIR_OUT,
> +	.bmAttributes		= USB_ENDPOINT_XFER_BULK,
> +	.wMaxPacketSize		=
> RX_ENDPOINT_MAXIMUM_PACKET_SIZE_1_1,
> +	.bInterval		= 0x00,
> +};
> +
> +static struct usb_endpoint_descriptor hs_ep_out = {
> +	.bLength		= USB_DT_ENDPOINT_SIZE,
> +	.bDescriptorType	= USB_DT_ENDPOINT,
> +	.bEndpointAddress	= USB_DIR_OUT,
> +	.bmAttributes		= USB_ENDPOINT_XFER_BULK,
> +	.wMaxPacketSize		=
> RX_ENDPOINT_MAXIMUM_PACKET_SIZE_2_0,
> +	.bInterval		= 0x00,
> +};
> +
> +static struct usb_interface_descriptor interface_desc = {
> +	.bLength		= USB_DT_INTERFACE_SIZE,
> +	.bDescriptorType	= USB_DT_INTERFACE,
> +	.bInterfaceNumber	= 0x00,
> +	.bAlternateSetting	= 0x00,
> +	.bNumEndpoints		= 0x02,
> +	.bInterfaceClass	= FASTBOOT_INTERFACE_CLASS,
> +	.bInterfaceSubClass	= FASTBOOT_INTERFACE_SUB_CLASS,
> +	.bInterfaceProtocol	= FASTBOOT_INTERFACE_PROTOCOL,
> +};
> +
> +static struct usb_descriptor_header *fb_runtime_descs[] = {
> +	(struct usb_descriptor_header *)&interface_desc,
> +	(struct usb_descriptor_header *)&fs_ep_in,
> +	(struct usb_descriptor_header *)&hs_ep_out,
> +	NULL,
> +};
> +
> +/*
> + * static strings, in UTF-8
> + */
> +static const char fastboot_name[] = "Android Fastboot";
> +
> +static struct usb_string fastboot_string_defs[] = {
> +	[0].s = fastboot_name,
> +	{  }			/* end of list */
> +};
> +
> +static struct usb_gadget_strings stringtab_fastboot = {
> +	.language	= 0x0409,	/* en-us */
> +	.strings	= fastboot_string_defs,
> +};
> +
> +static struct usb_gadget_strings *fastboot_strings[] = {
> +	&stringtab_fastboot,
> +	NULL,
> +};
> +
> +static void rx_handler_command(struct usb_ep *ep, struct usb_request
> *req); +
> +static void fastboot_complete(struct usb_ep *ep, struct usb_request
> *req) +{
> +	int status = req->status;
> +	if (!status)
> +		return;
> +	printf("status: %d ep '%s' trans: %d\n", status, ep->name,
> req->actual); +}
> +
> +static int fastboot_bind(struct usb_configuration *c, struct
> usb_function *f) +{
> +	int status, id;
> +	struct usb_gadget *gadget = c->cdev->gadget;
> +	struct f_fastboot *f_fb = func_to_fastboot(f);
> +
> +	/* DYNAMIC interface numbers assignments */
> +	status = usb_interface_id(c, f);
> +	if (status < 0)
> +		goto err;
> +
> +	interface_desc.bInterfaceNumber = status;
> +
> +	id = usb_string_id(c->cdev);
> +	if (id < 0)
> +		goto err;
> +	fastboot_string_defs[0].id = id;
> +	interface_desc.iInterface = id;
> +
> +	f_fb->in_ep = usb_ep_autoconfig(gadget, &fs_ep_in);
> +	if (!f_fb->in_ep)
> +		goto err;
> +	f_fb->in_ep->driver_data = c->cdev;
> +
> +	f_fb->out_ep = usb_ep_autoconfig(gadget, &fs_ep_out);
> +	if (!f_fb->out_ep)
> +		goto err;
> +	f_fb->out_ep->driver_data = c->cdev;
> +
> +	hs_ep_out.bEndpointAddress = fs_ep_out.bEndpointAddress;
> +
> +	return 0;
> +err:
> +	return -1;

		Maybe -ENODEV or -EINVAL?

> +}
> +
> +static void fastboot_unbind(struct usb_configuration *c, struct
> usb_function *f) +{
> +	memset(fastboot_func, 0, sizeof(*fastboot_func));
> +}
> +
> +static void fastboot_disable(struct usb_function *f)
> +{
> +	struct f_fastboot *f_fb = func_to_fastboot(f);
> +
> +	usb_ep_disable(f_fb->out_ep);
> +	usb_ep_disable(f_fb->in_ep);
> +
> +	if (f_fb->out_req) {
> +		free(f_fb->out_req->buf);
> +		usb_ep_free_request(f_fb->out_ep, f_fb->out_req);
> +		f_fb->out_req = NULL;
> +	}
> +	if (f_fb->in_req) {
> +		free(f_fb->in_req->buf);
> +		usb_ep_free_request(f_fb->in_ep, f_fb->in_req);
> +		f_fb->in_req = NULL;
> +	}
> +}
> +
> +static struct usb_request *fastboot_start_ep(struct usb_ep *ep)
> +{
> +	struct usb_request *req;
> +
> +	req = usb_ep_alloc_request(ep, 0);
> +	if (!req)
> +		return NULL;
> +
> +	req->length = EP_BUFFER_SIZE;
> +	req->buf = memalign(CONFIG_SYS_CACHELINE_SIZE,
> EP_BUFFER_SIZE);
> +	if (!req->buf) {
> +		usb_ep_free_request(ep, req);
> +		return NULL;
> +	}
> +
> +	memset(req->buf, 0, req->length);
> +	return req;
> +}
> +
> +static int fastboot_set_alt(struct usb_function *f,
> +			    unsigned interface, unsigned alt)
> +{
> +	int ret;
> +	struct usb_composite_dev *cdev = f->config->cdev;
> +	struct usb_gadget *gadget = cdev->gadget;
> +	struct f_fastboot *f_fb = func_to_fastboot(f);
> +
> +	debug("%s: func: %s intf: %d alt: %d\n",
> +	      __func__, f->name, interface, alt);
> +
> +	/* make sure we don't enable the ep twice */
> +	if (gadget->speed == USB_SPEED_HIGH)
> +		ret = usb_ep_enable(f_fb->out_ep, &hs_ep_out);
> +	else
> +		ret = usb_ep_enable(f_fb->out_ep, &fs_ep_out);
> +	if (ret) {
> +		puts("failed to enable out ep\n");
> +		goto err;
> +	}
> +
> +	f_fb->out_req = fastboot_start_ep(f_fb->out_ep);
> +	if (!f_fb->out_req) {
> +		puts("failed to alloc out req\n");
> +		goto err;
> +	}
> +
> +	ret = usb_ep_enable(f_fb->in_ep, &fs_ep_in);
> +	if (ret) {
> +		puts("failed to enable in ep\n");
> +		goto err;
> +	}
> +	f_fb->out_req->complete = rx_handler_command;
> +
> +	f_fb->in_req = fastboot_start_ep(f_fb->in_ep);
> +	if (!f_fb->in_req) {
> +		puts("failed alloc req in\n");
> +		goto err;
> +	}
> +	f_fb->in_req->complete = fastboot_complete;
> +
> +	ret = usb_ep_queue(f_fb->out_ep, f_fb->out_req, 0);
> +	if (ret)
> +		goto err;
> +
> +	return 0;
> +err:
> +	fastboot_disable(f);
> +	return -1;

	Here,I would also encourage to use appropriate error code from
	<errno.h>.

> +}
> +
> +int fastboot_add(struct usb_configuration *c)
> +{
> +	struct f_fastboot *f_fb = fastboot_func;
> +	int status;
> +
> +	debug("%s: cdev: 0x%p\n", __func__, c->cdev);
> +
> +	if (!f_fb) {
> +		f_fb = memalign(CONFIG_SYS_CACHELINE_SIZE,
> sizeof(*f_fb));
> +		if (!f_fb)
> +			return -ENOMEM;
> +
> +		fastboot_func = f_fb;
> +		memset(f_fb, 0, sizeof(*f_fb));
> +	}
> +
> +	f_fb->usb_function.name = "f_fastboot";
> +	f_fb->usb_function.hs_descriptors = fb_runtime_descs;
> +	f_fb->usb_function.bind = fastboot_bind;
> +	f_fb->usb_function.unbind = fastboot_unbind;
> +	f_fb->usb_function.set_alt = fastboot_set_alt;
> +	f_fb->usb_function.disable = fastboot_disable;
> +	f_fb->usb_function.strings = fastboot_strings;
> +
> +	status = usb_add_function(c, &f_fb->usb_function);
> +	if (status) {
> +		free(f_fb);
> +		fastboot_func = f_fb;
> +	}
> +
> +	return status;
> +}
> +
> +int fastboot_tx_write(const char *buffer, unsigned int buffer_size)
> +{
> +	struct usb_request *in_req = fastboot_func->in_req;
> +	int ret;
> +
> +	if (in_req->complete == NULL)
> +		in_req->complete = fastboot_complete;
> +
> +	memcpy(in_req->buf, buffer, buffer_size);
> +	in_req->length = buffer_size;
> +	ret = usb_ep_queue(fastboot_func->in_ep, in_req, 0);
> +	if (ret)
> +		printf("Error %d on queue\n", ret);
> +	return 0;
> +}
> +
> +static int fastboot_tx_write_str(const char *buffer)
> +{
> +	return fastboot_tx_write(buffer, strlen(buffer));
> +}
> +
> +static void compl_do_reset(struct usb_ep *ep, struct usb_request
> *req) +{
> +	do_reset(NULL, 0, 0, NULL);
> +}
> +
> +static void cb_reboot(struct usb_ep *ep, struct usb_request *req)
> +{
> +	fastboot_func->in_req->complete = compl_do_reset;
> +	fastboot_tx_write_str("OKAY");
> +}
> +
> +static int strcmp_l1(const char *s1, const char *s2)
> +{
> +	if (!s1 || !s2)
> +		return -1;
> +	return strncmp(s1, s2, strlen(s1));
> +}
> +
> +static void cb_getvar(struct usb_ep *ep, struct usb_request *req)
> +{
> +	char *cmd = req->buf;
> +	char response[RESPONSE_LEN];
> +	const char *s;
> +
> +	strcpy(response, "OKAY");
> +	strsep(&cmd, ":");
> +	if (!cmd) {
> +		fastboot_tx_write_str("FAILmissing var");
> +		return;
> +	}
> +
> +	if (!strcmp_l1("version", cmd)) {
> +		strncat(response, FASTBOOT_VERSION,
> sizeof(response));
> +	} else if (!strcmp_l1("bootloader-version", cmd)) {
> +		strncat(response, U_BOOT_VERSION, sizeof(response));
> +	} else if (!strcmp_l1("downloadsize", cmd)) {
> +		char str_num[12];
> +
> +		sprintf(str_num, "%08x",
> CONFIG_USB_FASTBOOT_BUF_SIZE);
> +		strncat(response, str_num, sizeof(response));
> +	} else if (!strcmp_l1("serialno", cmd)) {
> +		s = getenv("serial#");
> +		if (s)
> +			strncat(response, s, sizeof(response));
> +		else
> +			strcpy(response, "FAILValue not set");
> +	} else {
> +		strcpy(response, "FAILVariable not implemented");
> +	}
> +	fastboot_tx_write_str(response);
> +}
> +
> +static unsigned int rx_bytes_expected(void)
> +{
> +	int rx_remain = download_size - download_bytes;
> +	if (rx_remain < 0)
> +		return 0;
> +	if (rx_remain > EP_BUFFER_SIZE)
> +		return EP_BUFFER_SIZE;
> +	return rx_remain;
> +}
> +
> +#define BYTES_PER_DOT	32768
> +static void rx_handler_dl_image(struct usb_ep *ep, struct
> usb_request *req) +{
> +	char response[RESPONSE_LEN];
> +	unsigned int transfer_size = download_size - download_bytes;
> +	const unsigned char *buffer = req->buf;
> +	unsigned int buffer_size = req->actual;
> +
> +	if (req->status != 0) {
> +		printf("Bad status: %d\n", req->status);
> +		return;
> +	}
> +
> +	if (buffer_size < transfer_size)
> +		transfer_size = buffer_size;
> +
> +	memcpy((void *)CONFIG_USB_FASTBOOT_BUF_ADDR + download_bytes,

	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.

> +	       buffer, transfer_size);
> +
> +	download_bytes += transfer_size;
> +
> +	/* Check if transfer is done */
> +	if (download_bytes >= download_size) {
> +		/*
> +		 * Reset global transfer variable, keep
> download_bytes because
> +		 * it will be used in the next possible flashing
> command
> +		 */
> +		download_size = 0;
> +		req->complete = rx_handler_command;
> +		req->length = EP_BUFFER_SIZE;
> +
> +		sprintf(response, "OKAY");
> +		fastboot_tx_write_str(response);
> +
> +		printf("\ndownloading of %d bytes finished\n",
> download_bytes);
> +	} else {
> +		req->length = rx_bytes_expected();
> +	}
> +
> +	if (download_bytes && !(download_bytes % BYTES_PER_DOT)) {
> +		putc('.');
> +		if (!(download_bytes % (74 * BYTES_PER_DOT)))
> +			putc('\n');
> +	}
> +	req->actual = 0;
> +	usb_ep_queue(ep, req, 0);
> +}
> +
> +static void cb_download(struct usb_ep *ep, struct usb_request *req)
> +{
> +	char *cmd = req->buf;
> +	char response[RESPONSE_LEN];
> +
> +	strsep(&cmd, ":");
> +	download_size = simple_strtoul(cmd, NULL, 16);
> +	download_bytes = 0;
> +
> +	printf("Starting download of %d bytes\n", download_size);
> +
> +	if (0 == download_size) {
> +		sprintf(response, "FAILdata invalid size");
> +	} else if (download_size > CONFIG_USB_FASTBOOT_BUF_SIZE) {
> +		download_size = 0;
> +		sprintf(response, "FAILdata too large");
> +	} else {
> +		sprintf(response, "DATA%08x", download_size);
> +		req->complete = rx_handler_dl_image;
> +		req->length = rx_bytes_expected();
> +	}
> +	fastboot_tx_write_str(response);
> +}
> +
> +static void do_bootm_on_complete(struct usb_ep *ep, struct
> usb_request *req) +{
> +	char boot_addr_start[12];
> +	char *bootm_args[] = { "bootm", boot_addr_start, NULL };
> +
> +	req->complete = NULL;
> +	g_dnl_unregister();
> +	puts("Booting kernel..\n");
> +
> +	sprintf(boot_addr_start, "0x%lx", load_addr);
> +	do_bootm(NULL, 0, 2, bootm_args);
> +
> +	/* This only happens if image is somehow faulty so we start
> over */
> +	do_reset(NULL, 0, 0, NULL);
> +}
> +
> +static void cb_boot(struct usb_ep *ep, struct usb_request *req)
> +{
> +	fastboot_func->in_req->complete = do_bootm_on_complete;
> +	fastboot_tx_write_str("OKAY");
> +}
> +
> +struct cmd_dispatch_info {
> +	char *cmd;
> +	void (*cb)(struct usb_ep *ep, struct usb_request *req);
> +};
> +
> +static const struct cmd_dispatch_info cmd_dispatch_info[] = {
> +	{
> +		.cmd = "reboot",
> +		.cb = cb_reboot,
> +	}, {
> +		.cmd = "getvar:",
> +		.cb = cb_getvar,
> +	}, {
> +		.cmd = "download:",
> +		.cb = cb_download,
> +	}, {
> +		.cmd = "boot",
> +		.cb = cb_boot,
> +	},
> +};
> +
> +static void rx_handler_command(struct usb_ep *ep, struct usb_request
> *req) +{
> +	char response[RESPONSE_LEN];
> +	char *cmdbuf = req->buf;
> +	void (*func_cb)(struct usb_ep *ep, struct usb_request *req)
> = NULL;
> +	int i;
> +
> +	sprintf(response, "FAIL");
> +
> +	for (i = 0; i < ARRAY_SIZE(cmd_dispatch_info); i++) {
> +		if (!strcmp_l1(cmd_dispatch_info[i].cmd, cmdbuf)) {
> +			func_cb = cmd_dispatch_info[i].cb;
> +			break;
> +		}
> +	}
> +
> +	if (!func_cb)
> +		fastboot_tx_write_str("FAILunknown command");
> +	else
> +		func_cb(ep, req);
> +
> +	if (req->status == 0) {
> +		*cmdbuf = '\0';
> +		req->actual = 0;
> +		usb_ep_queue(ep, req, 0);
> +	}
> +}
> diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
> index dd95afe..dbea322 100644
> --- a/drivers/usb/gadget/g_dnl.c
> +++ b/drivers/usb/gadget/g_dnl.c
> @@ -17,6 +17,7 @@
>  #include <usb_mass_storage.h>
>  #include <dfu.h>
>  #include <thor.h>
> +#include <fastboot.h>
>  
>  #include "gadget_chips.h"
>  #include "composite.c"
> @@ -117,6 +118,8 @@ static int g_dnl_do_config(struct
> usb_configuration *c) ret = fsg_add(c);
>  	else if (!strcmp(s, "usb_dnl_thor"))
>  		ret = thor_add(c);
> +	else if (!strcmp(s, "usb_dnl_fastboot"))
> +		ret = fastboot_add(c);
>  
>  	return ret;
>  }
> @@ -242,6 +245,9 @@ int g_dnl_register(const char *type)
>  	} else if (!strcmp(type, "thor")) {
>  		strcpy(name, shortname);
>  		strcat(name, type);
> +	} else if (!strcmp(type, "fastboot")) {
> +		strcpy(name, shortname);
> +		strcat(name, type);
>  	} else {
>  		printf("%s: unknown command: %s\n", __func__, type);
>  		return -EINVAL;
> diff --git a/include/fastboot.h b/include/fastboot.h
> new file mode 100644
> index 0000000..3c26bd6
> --- /dev/null
> +++ b/include/fastboot.h
> @@ -0,0 +1,22 @@
> +/*
> + * (C) Copyright 2014 Linaro, Ltd.
> + * Rob Herring <robh@kernel.org>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#ifndef FASTBOOT_H
> +#define FASTBOOT_H
> +
> +#include <linux/usb/composite.h>
> +
> +#ifdef CONFIG_CMD_FASTBOOT
> +int fastboot_add(struct usb_configuration *c);
> +#else
> +static int fastboot_add(struct usb_configuration *c)
> +{
> +	return 0;
> +}
> +#endif
> +
> +#endif

One more remark regarding your patches.

There is an ongoing work done by Mateusz Zalega (CC'ed). 

Patch series
"[PATCH v4 00/13] DFU, MMC, Gadget, Goni, misc."
embracing below patch:
http://patchwork.ozlabs.org/patch/339263/

is changing the g_dnl interface.

I'm not the USB maintainer :-) (Marek Vasut is), but for me it seems
that above patches are more likely to be earlier accepted to u-boot-usb
tree. 

Marek, is this a correct assumption? 

If Marek don't mind, I'd like to ask you to rebase yours patches on top
of Mateusz work.

On Wednesday, April 23, 2014 at 01:02:13 PM, Lukasz Majewski wrote:
[...]

> > +#include <linux/usb/composite.h>
> > +
> > +#ifdef CONFIG_CMD_FASTBOOT
> > +int fastboot_add(struct usb_configuration *c);
> > +#else
> > +static int fastboot_add(struct usb_configuration *c)
> > +{
> > +	return 0;
> > +}
> > +#endif
> > +
> > +#endif
> 
> One more remark regarding your patches.
> 
> There is an ongoing work done by Mateusz Zalega (CC'ed).
> 
> Patch series
> "[PATCH v4 00/13] DFU, MMC, Gadget, Goni, misc."
> embracing below patch:
> http://patchwork.ozlabs.org/patch/339263/
> 
> is changing the g_dnl interface.
> 
> I'm not the USB maintainer :-) (Marek Vasut is), but for me it seems
> that above patches are more likely to be earlier accepted to u-boot-usb
> tree.
> 
> Marek, is this a correct assumption?

Depends on Mateusz, surely we will see V5 of his set.

> If Marek don't mind, I'd like to ask you to rebase yours patches on top
> of Mateusz work.

btw Please learn to trim the message to only the relevant bits.

Best regards,
Marek Vasut

On Wed, Apr 23, 2014 at 6:02 AM, Lukasz Majewski <l.majewski@samsung.com> wrote:
> Hi Rob,
>
>> From: Sebastian Siewior <bigeasy@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.
>>

[...]

>> +     f_fb->out_ep = usb_ep_autoconfig(gadget, &fs_ep_out);
>> +     if (!f_fb->out_ep)
>> +             goto err;
>> +     f_fb->out_ep->driver_data = c->cdev;
>> +
>> +     hs_ep_out.bEndpointAddress = fs_ep_out.bEndpointAddress;
>> +
>> +     return 0;
>> +err:
>> +     return -1;
>
>                 Maybe -ENODEV or -EINVAL?

Okay. Actually, we can remove the goto and just return the original error codes.


>> +     if (buffer_size < transfer_size)
>> +             transfer_size = buffer_size;
>> +
>> +     memcpy((void *)CONFIG_USB_FASTBOOT_BUF_ADDR + download_bytes,
>
>         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.

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.

Rob

Hi Rob,

> On Wed, Apr 23, 2014 at 6:02 AM, Lukasz Majewski
> <l.majewski@samsung.com> wrote:
> > Hi Rob,
> >
> >> From: Sebastian Siewior <bigeasy@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.
> >>
> 
> [...]
> 
> >> +     f_fb->out_ep = usb_ep_autoconfig(gadget, &fs_ep_out);
> >> +     if (!f_fb->out_ep)
> >> +             goto err;
> >> +     f_fb->out_ep->driver_data = c->cdev;
> >> +
> >> +     hs_ep_out.bEndpointAddress = fs_ep_out.bEndpointAddress;
> >> +
> >> +     return 0;
> >> +err:
> >> +     return -1;
> >
> >                 Maybe -ENODEV or -EINVAL?
> 
> Okay. Actually, we can remove the goto and just return the original
> error codes.
> 
> 
> >> +     if (buffer_size < transfer_size)
> >> +             transfer_size = buffer_size;
> >> +
> >> +     memcpy((void *)CONFIG_USB_FASTBOOT_BUF_ADDR + download_bytes,
> >
> >         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?

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?


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

> 
> Rob

On Fri, Apr 25, 2014 at 12:26 AM, Lukasz Majewski
<l.majewski@samsung.com> wrote:
> Hi Rob,
>
>> On Wed, Apr 23, 2014 at 6:02 AM, Lukasz Majewski
>> <l.majewski@samsung.com> wrote:
>> > Hi Rob,
>> >
>> >> From: Sebastian Siewior <bigeasy@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.

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

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

Rob

Hi Rob,

> On Fri, Apr 25, 2014 at 12:26 AM, Lukasz Majewski
> <l.majewski@samsung.com> wrote:
> > Hi Rob,
> >
> >> On Wed, Apr 23, 2014 at 6:02 AM, Lukasz Majewski
> >> <l.majewski@samsung.com> wrote:
> >> > Hi Rob,
> >> >
> >> >> From: Sebastian Siewior <bigeasy@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