Skip to contentSkip to conversation

imx: add USB Serial Download Protocol (SDP) support

PATCH New Stefan Agner 6 participants 30 messages Aug 4, 2017 – Aug 16, 2017

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover imx: add USB Serial Download Protocol (SDP) support

From: Stefan Agner <[email protected]>

This series adds NXP's Serial Download Protocol (SDP) support via USB for SPL/U-Boot. It allows to download U-Boot via USB from a (recovered) SPL using the same tools used to download SPL itself (specifically imx_usb, but also sb_loader seems to work).

The idea has been brought up when the first targets started to make use of SPL for DDR initialization, see:
https://lists.denx.de/pipermail/u-boot/2015-July/220330.html

The initial SDP implementation (patch 2) requires the payload to have the imx specific headers (hence the move of the imx header file in patch 1).

Patch 3 extends image header support beyond the SDP specification, specifically implements also support for U-Boot headers. This allows to use the same SPL/U-Boot binaries for recovery as used on the regular boot device (SD/eMMC). For that to work also the host side imx_usb tools needed an extension, currently available here:

https://github.com/toradex/imx_loader/tree/imx_usb_batch_mode_refactored

The full patchset allows to download SPL and U-Boot over USB to a target in recovery mode using the same usb_imx utility:

The usb_imx utility also has a batch mode which allows to download multiple artifacts with a single invocation. The details are outlined in the imx_usb commit message:
https://github.com/toradex/imx_loader/commit/5434415d921f1cc4d22332d9558bed6d42db9f60

In case this patchset gets accepted in U-Boot, I plan to push the imx_usb changes upstream as well.

Stefan Agner (7):
imx: move imximage header to common location
usb: gadget: add SDP driver
usb: gadget: sdp: extend images compatible for jumps
cmd: add sdp command
spl: add serial download protocol (SDP) support apalis/colibri_imx6: use independent USB PID for SPL apalis/colibri_imx6: enable SDP by default

 board/toradex/apalis_imx6/apalis_imx6.c   |  13 +
 board/toradex/colibri_imx6/colibri_imx6.c |  13 +
 cmd/Kconfig                               |   7 +
 cmd/Makefile                              |   1 +
 cmd/usb_gadget_sdp.c                      |  53 +++
 common/spl/Kconfig                        |   6 +
 common/spl/Makefile                       |   1 +
 common/spl/spl_sdp.c                      |  38 ++
 configs/apalis_imx6_defconfig             |   4 +
 configs/colibri_imx6_defconfig            |   4 +
 drivers/usb/gadget/Kconfig                |   7 +
 drivers/usb/gadget/Makefile               |   2 +
 drivers/usb/gadget/f_sdp.c                | 739 ++++++++++++++++++++++++++++++
 {tools => include}/imximage.h             |   0
 include/sdp.h                             |  16 +
 15 files changed, 904 insertions(+)
 create mode 100644 cmd/usb_gadget_sdp.c
 create mode 100644 common/spl/spl_sdp.c
 create mode 100644 drivers/usb/gadget/f_sdp.c
 rename {tools => include}/imximage.h (100%)
 create mode 100644 include/sdp.h
Signature · 1 line
2.13.3
#1 lore raw

PATCH 1/7 imx: move imximage header to common location

From: Stefan Agner <[email protected]>

Move the imximage.h header file to a common location so we can make use of it from U-Boot too.

---

 {tools => include}/imximage.h | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename {tools => include}/imximage.h (100%)
include/imximage.h renamed from tools/imximage.h +0 -0
similarity index 100%
rename from tools/imximage.h
rename to include/imximage.h
Signature · 1 line
2.13.3
#2 lore raw

PATCH 1/7 imx: move imximage header to common location

include/imximage.h

Quoted 17 lines · Stefan Agner

From: Stefan Agner <[email protected]>

Move the imximage.h header file to a common location so we can make use of it from U-Boot too.

---

 {tools => include}/imximage.h | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename {tools => include}/imximage.h (100%)
include/imximage.h renamed from tools/imximage.h +0 -0
similarity index 100%
rename from tools/imximage.h
rename to include/imximage.h
Signature · 9 lines
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
#3 lore raw

PATCH 2/7 usb: gadget: add SDP driver

In reply to Stefan Agner

From: Stefan Agner <[email protected]>

Add SDP (Serial Downloader Protocol) implementation for U-Boot. The protocol is used in NXP SoC's boot ROM and allows to download program images. Beside that, it can also be used to read/write registers and download complete Device Configuration Data (DCD) sets. This basic implementation supports downloading images with the imx header format and reading registers.

---

 drivers/usb/gadget/Kconfig  |   7 +
 drivers/usb/gadget/Makefile |   1 +
 drivers/usb/gadget/f_sdp.c  | 723 ++++++++++++++++++++++++++++++++++++++++++++
 include/sdp.h               |  16 +
 4 files changed, 747 insertions(+)
 create mode 100644 drivers/usb/gadget/f_sdp.c
 create mode 100644 include/sdp.h
drivers/usb/gadget/Kconfig +7 -0
index 261ed128ac..225b66bc95 100644
@@ -103,6 +103,13 @@ config USB_GADGET_DOWNLOAD
 
 if USB_GADGET_DOWNLOAD
 
+config USB_FUNCTION_SDP
+	bool "Enable USB SDP (Serial Download Protocol)"
+	help
+	  Enable Serial Download Protocol (SDP) device support in U-Boot. This
+	  allows to download images into memory and execute (jump to) them
+	  using the same protocol as implemented by the i.MX family's boot ROM.
+
 config G_DNL_MANUFACTURER
 	string "Vendor name of USB device"
 
drivers/usb/gadget/Makefile +1 -0
index 5e316a7cff..6a007d1bcb 100644
@@ -28,6 +28,7 @@ obj-$(CONFIG_USB_FUNCTION_THOR) += f_thor.o
 obj-$(CONFIG_USB_FUNCTION_DFU) += f_dfu.o
 obj-$(CONFIG_USB_FUNCTION_MASS_STORAGE) += f_mass_storage.o
 obj-$(CONFIG_USB_FUNCTION_FASTBOOT) += f_fastboot.o
+obj-$(CONFIG_USB_FUNCTION_SDP) += f_sdp.o
 endif
 endif
 ifdef CONFIG_USB_ETHER
drivers/usb/gadget/f_sdp.c new file +723 -0

723 lines not on this page · raw message

include/sdp.h new file +16 -0
new file mode 100644
index 0000000000..03c4a23434
@@ -0,0 +1,16 @@
+/*
+ * sdp.h - Serial Download Protocol
+ *
+ * Copyright (C) 2016 Toradex
+ * Author: Stefan Agner <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __SDP_H_
+#define __SDP_H_
+
+int sdp_init(void);
+int sdp_handle(void);
+
+#endif /* __SDP_H_ */
Signature · 1 line
2.13.3
#4 lore raw

PATCH 2/7 usb: gadget: add SDP driver

Hi,

Quoted 458 lines · On Fri, 4 Aug 2017 16:38:08 -0700 Stefan Agner wrote:

From: Stefan Agner <[email protected]>

Add SDP (Serial Downloader Protocol) implementation for U-Boot. The protocol is used in NXP SoC's boot ROM and allows to download program images. Beside that, it can also be used to read/write registers and download complete Device Configuration Data (DCD) sets. This basic implementation supports downloading images with the imx header format and reading registers.

---

 drivers/usb/gadget/Kconfig  |   7 +
 drivers/usb/gadget/Makefile |   1 +
 drivers/usb/gadget/f_sdp.c  | 723 ++++++++++++++++++++++++++++++++++++++++++++
 include/sdp.h               |  16 +
 4 files changed, 747 insertions(+)
 create mode 100644 drivers/usb/gadget/f_sdp.c
 create mode 100644 include/sdp.h
drivers/usb/gadget/Kconfig +7 -0
index 261ed128ac..225b66bc95 100644
@@ -103,6 +103,13 @@ config USB_GADGET_DOWNLOAD
 
 if USB_GADGET_DOWNLOAD
 
+config USB_FUNCTION_SDP
+	bool "Enable USB SDP (Serial Download Protocol)"
+	help
+	  Enable Serial Download Protocol (SDP) device support in U-Boot. This
+	  allows to download images into memory and execute (jump to) them
+	  using the same protocol as implemented by the i.MX family's boot ROM.
+
 config G_DNL_MANUFACTURER
 	string "Vendor name of USB device"
 
drivers/usb/gadget/Makefile +1 -0
index 5e316a7cff..6a007d1bcb 100644
@@ -28,6 +28,7 @@ obj-$(CONFIG_USB_FUNCTION_THOR) += f_thor.o
 obj-$(CONFIG_USB_FUNCTION_DFU) += f_dfu.o
 obj-$(CONFIG_USB_FUNCTION_MASS_STORAGE) += f_mass_storage.o
 obj-$(CONFIG_USB_FUNCTION_FASTBOOT) += f_fastboot.o
+obj-$(CONFIG_USB_FUNCTION_SDP) += f_sdp.o
 endif
 endif
 ifdef CONFIG_USB_ETHER
drivers/usb/gadget/f_sdp.c new file +401 -0
new file mode 100644
index 0000000000..eb89695aaf
@@ -0,0 +1,723 @@
+/*
+ * f_sdp.c -- USB HID Serial Download Protocol
+ *
+ * Copyright (C) 2016 Toradex
+ * Author: Stefan Agner <[email protected]>
+ *
+ * This file implements the Serial Download Protocol (SDP) as specified in
+ * the i.MX 6 Reference Manual. The SDP is a USB HID based protocol and
+ * allows to download images directly to memory. The implementation
+ * works with the imx_loader (imx_usb) USB client software on host side.
+ *
+ * Not all commands are implemented, e.g. WRITE_REGISTER, DCD_WRITE and
+ * SKIP_DCD_HEADER are only stubs.
+ *
+ * Parts of the implementation are based on f_dfu and f_thor.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <errno.h>
+#include <common.h>
+#include <console.h>
+#include <malloc.h>
+
+#include <linux/usb/ch9.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/composite.h>
+
+#include <asm/io.h>
+#include <g_dnl.h>
+#include <sdp.h>
+#include <imximage.h>
+
+#define HID_REPORT_ID_MASK	0x000000ff
+
+/*
+ * HID class requests
+ */
+#define HID_REQ_GET_REPORT		0x01
+#define HID_REQ_GET_IDLE		0x02
+#define HID_REQ_GET_PROTOCOL		0x03
+#define HID_REQ_SET_REPORT		0x09
+#define HID_REQ_SET_IDLE		0x0A
+#define HID_REQ_SET_PROTOCOL		0x0B
+
+#define HID_USAGE_PAGE_LEN		76
+
+struct hid_report {
+	u8 usage_page[HID_USAGE_PAGE_LEN];
+} __packed;
+
+#define SDP_READ_REGISTER	0x0101
+#define SDP_WRITE_REGISTER	0x0202
+#define SDP_WRITE_FILE		0x0404
+#define SDP_ERROR_STATUS	0x0505
+#define SDP_DCD_WRITE		0x0a0a
+#define SDP_JUMP_ADDRESS	0x0b0b
+#define SDP_SKIP_DCD_HEADER	0x0c0c
+
+#define SDP_WRITE_FILE_COMPLETE		0x88888888
+#define SDP_WRITE_REGISTER_COMPLETE	0x128A8A12
+#define SDP_SKIP_DCD_HEADER_COMPLETE	0x900DD009
+#define SDP_ERROR_IMXHEADER		0x000a0533
+
+#define SDP_COMMAND_LEN		16
+
+struct sdp_command {
+	u16 cmd;
+	u32 addr;
+	u8 format;
+	u32 cnt;
+	u32 data;
+	u8 rsvd;
+} __packed;
+
+enum sdp_state {
+	SDP_STATE_IDLE,
+	SDP_STATE_RX_DCD_DATA,
+	SDP_STATE_RX_FILE_DATA,
+	SDP_STATE_TX_SEC_CONF,
+	SDP_STATE_TX_SEC_CONF_BUSY,
+	SDP_STATE_TX_REGISTER,
+	SDP_STATE_TX_REGISTER_BUSY,
+	SDP_STATE_TX_STATUS,
+	SDP_STATE_TX_STATUS_BUSY,
+	SDP_STATE_JUMP,
+};
+
+struct f_sdp {
+	struct usb_function		usb_function;
+
+	struct usb_descriptor_header	**function;
+
+	u8				altsetting;
+	enum sdp_state			state;
+	enum sdp_state			next_state;
+	u32				dnl_address;
+	u32				dnl_bytes_remaining;
+	u32				jmp_address;
+	bool				always_send_status;
+	u32				error_status;
+
+	/* EP0 request */
+	struct usb_request		*req;
+
+	/* EP1 IN */
+	struct usb_ep			*in_ep;
+	struct usb_request		*in_req;
+
+	bool				configuration_done;
+};
+
+static struct f_sdp *sdp_func;
+
+static inline struct f_sdp *func_to_sdp(struct usb_function *f)
+{
+	return container_of(f, struct f_sdp, usb_function);
+}
+
+static struct usb_interface_descriptor sdp_intf_runtime = {
+	.bLength =		sizeof(sdp_intf_runtime),
+	.bDescriptorType =	USB_DT_INTERFACE,
+	.bAlternateSetting =	0,
+	.bNumEndpoints =	1,
+	.bInterfaceClass =	USB_CLASS_HID,
+	.bInterfaceSubClass =	0,
+	.bInterfaceProtocol =	0,
+	/* .iInterface = DYNAMIC */
+};
+
+/* HID configuration */
+static struct usb_class_hid_descriptor sdp_hid_desc = {
+	.bLength =		sizeof(sdp_hid_desc),
+	.bDescriptorType =	USB_DT_CS_DEVICE,
+
+	.bcdCDC =		__constant_cpu_to_le16(0x0110),
+	.bCountryCode =		0,
+	.bNumDescriptors =	1,
+
+	.bDescriptorType0	= USB_DT_HID_REPORT,
+	.wDescriptorLength0	= HID_USAGE_PAGE_LEN,
+};
+
+static struct usb_endpoint_descriptor in_desc = {
+	.bLength =		USB_DT_ENDPOINT_SIZE,
+	.bDescriptorType =	USB_DT_ENDPOINT, /*USB_DT_CS_ENDPOINT*/
+
+	.bEndpointAddress =	1 | USB_DIR_IN,
+	.bmAttributes =	USB_ENDPOINT_XFER_INT,
+	.wMaxPacketSize =	64,
+	.bInterval =		1,
+};
+
+static struct usb_descriptor_header *sdp_runtime_descs[] = {
+	(struct usb_descriptor_header *)&sdp_intf_runtime,
+	(struct usb_descriptor_header *)&sdp_hid_desc,
+	(struct usb_descriptor_header *)&in_desc,
+	NULL,
+};
+
+/* This is synchronized with what the SoC implementation reports */
+static struct hid_report sdp_hid_report = {
+	.usage_page = {
+		0x06, 0x00, 0xff, /* Usage Page */
+		0x09, 0x01, /* Usage (Poiter?) */
+		0xa1, 0x01, /* Collection */
+
+		0x85, 0x01, /* Report ID */
+		0x19, 0x01, /* Usage Minimum */
+		0x29, 0x01, /* Usage Maximum */
+		0x15, 0x00, /* Local Minimum */
+		0x26, 0xFF, 0x00, /* Local Maximum? */
+		0x75, 0x08, /* Report Size */
+		0x95, 0x10, /* Report Count */
+		0x91, 0x02, /* Output Data */
+
+		0x85, 0x02, /* Report ID */
+		0x19, 0x01, /* Usage Minimum */
+		0x29, 0x01, /* Usage Maximum */
+		0x15, 0x00, /* Local Minimum */
+		0x26, 0xFF, 0x00, /* Local Maximum? */
+		0x75, 0x80, /* Report Size 128 */
+		0x95, 0x40, /* Report Count */
+		0x91, 0x02, /* Output Data */
+
+		0x85, 0x03, /* Report ID */
+		0x19, 0x01, /* Usage Minimum */
+		0x29, 0x01, /* Usage Maximum */
+		0x15, 0x00, /* Local Minimum */
+		0x26, 0xFF, 0x00, /* Local Maximum? */
+		0x75, 0x08, /* Report Size 8 */
+		0x95, 0x04, /* Report Count */
+		0x81, 0x02, /* Input Data */
+
+		0x85, 0x04, /* Report ID */
+		0x19, 0x01, /* Usage Minimum */
+		0x29, 0x01, /* Usage Maximum */
+		0x15, 0x00, /* Local Minimum */
+		0x26, 0xFF, 0x00, /* Local Maximum? */
+		0x75, 0x08, /* Report Size 8 */
+		0x95, 0x40, /* Report Count */
+		0x81, 0x02, /* Input Data */
+		0xc0
+	},
+};
+
+static const char sdp_name[] = "Serial Downloader Protocol";
+
+/*
+ * static strings, in UTF-8
+ */
+static struct usb_string strings_sdp_generic[] = {
+	[0].s = sdp_name,
+	{  }			/* end of list */
+};
+
+static struct usb_gadget_strings stringtab_sdp_generic = {
+	.language	= 0x0409,	/* en-us */
+	.strings	= strings_sdp_generic,
+};
+
+static struct usb_gadget_strings *sdp_generic_strings[] = {
+	&stringtab_sdp_generic,
+	NULL,
+};
+
+static void sdp_rx_command_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_sdp *sdp = req->context;
+	int status = req->status;
+	u8 *data = req->buf;
+	u8 report = data[0];
+
+	if (status != 0) {
+		error("Status: %d", status);
+		return;
+	}
+
+	if (report != 1) {
+		error("Unexpected report %d", report);
+		return;
+	}
+
+	struct sdp_command *cmd = req->buf + 1;
+
+	debug("%s: command: %04x, addr: %08x, cnt: %u\n",
+	      __func__, be16_to_cpu(cmd->cmd),
+	      be32_to_cpu(cmd->addr), be32_to_cpu(cmd->cnt));
+
+	switch (be16_to_cpu(cmd->cmd)) {
+	case SDP_READ_REGISTER:
+		sdp->always_send_status = false;
+		sdp->error_status = 0x0;
+
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->dnl_address = be32_to_cpu(cmd->addr);
+		sdp->dnl_bytes_remaining = be32_to_cpu(cmd->cnt);
+		sdp->next_state = SDP_STATE_TX_REGISTER;
+		printf("Reading %d registers at 0x%08x... ",
+		       sdp->dnl_bytes_remaining, sdp->dnl_address);
+		break;
+	case SDP_WRITE_FILE:
+		sdp->always_send_status = true;
+		sdp->error_status = SDP_WRITE_FILE_COMPLETE;
+
+		sdp->state = SDP_STATE_RX_FILE_DATA;
+		sdp->dnl_address = be32_to_cpu(cmd->addr);
+		sdp->dnl_bytes_remaining = be32_to_cpu(cmd->cnt);
+		sdp->next_state = SDP_STATE_IDLE;
+
+		printf("Downloading file of size %d to 0x%08x... ",
+		       sdp->dnl_bytes_remaining, sdp->dnl_address);
+
+		break;
+	case SDP_ERROR_STATUS:
+		sdp->always_send_status = true;
+		sdp->error_status = 0;
+
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->next_state = SDP_STATE_IDLE;
+		break;
+	case SDP_DCD_WRITE:
+		sdp->always_send_status = true;
+		sdp->error_status = SDP_WRITE_REGISTER_COMPLETE;
+
+		sdp->state = SDP_STATE_RX_DCD_DATA;
+		sdp->dnl_bytes_remaining = be32_to_cpu(cmd->cnt);
+		sdp->next_state = SDP_STATE_IDLE;
+		break;
+	case SDP_JUMP_ADDRESS:
+		sdp->always_send_status = false;
+		sdp->error_status = 0;
+
+		sdp->jmp_address = be32_to_cpu(cmd->addr);
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->next_state = SDP_STATE_JUMP;
+		break;
+	case SDP_SKIP_DCD_HEADER:
+		sdp->always_send_status = true;
+		sdp->error_status = SDP_SKIP_DCD_HEADER_COMPLETE;
+
+		/* Ignore command, DCD not supported anyway */
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->next_state = SDP_STATE_IDLE;
+		break;
+	default:
+		error("Unknown command: %08x\n", be16_to_cpu(cmd->cmd));
+	}
+}
+
+static void sdp_rx_data_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_sdp *sdp = req->context;
+	int status = req->status;
+	u8 *data = req->buf;
+	u8 report = data[0];
+	int datalen = req->length - 1;
+
+	if (status != 0) {
+		error("Status: %d", status);
+		return;
+	}
+
+	if (report != 2) {
+		error("Unexpected report %d", report);
+		return;
+	}
+
+	if (sdp->dnl_bytes_remaining < datalen) {
+		/*
+		 * Some USB stacks require to send a complete buffer as
+		 * specified in the HID descriptor. This leads to longer
+		 * transfers than the file length, no problem for us.
+		 */
+		sdp->dnl_bytes_remaining = 0;
+	} else {
+		sdp->dnl_bytes_remaining -= datalen;
+	}
+
+	if (sdp->state == SDP_STATE_RX_FILE_DATA) {
+		memcpy((void *)sdp->dnl_address, req->buf + 1, datalen);
+		sdp->dnl_address += datalen;
+	}
+
+	if (sdp->dnl_bytes_remaining)
+		return;
+
+	printf("done\n");
+
+	switch (sdp->state) {
+	case SDP_STATE_RX_FILE_DATA:
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		break;
+	case SDP_STATE_RX_DCD_DATA:
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		break;
+	default:
+		error("Invalid state: %d", sdp->state);
+	}
+}
+
+
+
+static void sdp_tx_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_sdp *sdp = req->context;
+	int status = req->status;
+
+	if (status != 0) {
+		error("Status: %d", status);
+		return;
+	}
+
+	switch (sdp->state) {
+	case SDP_STATE_TX_SEC_CONF_BUSY:
+		/* Not all commands require status report */
+		if (sdp->always_send_status || sdp->error_status)
+			sdp->state = SDP_STATE_TX_STATUS;
+		else
+			sdp->state = sdp->next_state;
+
+		break;
+	case SDP_STATE_TX_STATUS_BUSY:
+		sdp->state = sdp->next_state;
+		break;
+	case SDP_STATE_TX_REGISTER_BUSY:
+		if (sdp->dnl_bytes_remaining)
+			sdp->state = SDP_STATE_TX_REGISTER;
+		else
+			sdp->state = SDP_STATE_IDLE;
+		break;
+	default:
+		error("Wrong State: %d", sdp->state);
+		sdp->state = SDP_STATE_IDLE;
+		break;
+	}
+	debug("%s complete --> %d, %d/%d\n", ep->name,
+	      status, req->actual, req->length);
+}
+
+static int sdp_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)

drivers/usb/gadget/f_sdp.c line 411

Quoted 10 lines
drivers/usb/gadget/f_sdp.c new file +10 -0
+{
+	struct usb_gadget *gadget = f->config->cdev->gadget;
+	struct usb_request *req = f->config->cdev->req;
+	struct f_sdp *sdp = f->config->cdev->req->context;
+	u16 len = le16_to_cpu(ctrl->wLength);
+	u16 w_value = le16_to_cpu(ctrl->wValue);
+	int value = 0;
+	u8 req_type = ctrl->bRequestType & USB_TYPE_MASK;
+
+	debug("w_value: 0x%x len: 0x%x\n", w_value, len);

s/0x%x/0x%04x/g ?

drivers/usb/gadget/f_sdp.c line 412

Quoted 1 line
drivers/usb/gadget/f_sdp.c new file +1 -0
+	debug("req_type: 0x%x ctrl->bRequest: 0x%x sdp->state: %d\n",

s/0x%x/0x%02x/g ?

[...]

drivers/usb/gadget/f_sdp.c line 614

Quoted 7 lines
drivers/usb/gadget/f_sdp.c new file +7 -0
+static u32 sdp_jump_imxheader(void *address)
+{
+	flash_header_v2_t *headerv2 = address;
+	ulong (*entry)(void);
+
+	if (headerv2->header.tag != IVT_HEADER_TAG) {
+		printf("Header Tag is not a IMX image\n");

<nit>
s/a IMX/an IMX/'

drivers/usb/gadget/f_sdp.c line 622

Quoted 8 lines
drivers/usb/gadget/f_sdp.c new file +8 -0
+		return SDP_ERROR_IMXHEADER;
+	}
+
+	printf("Jumping to 0x%08x\n", headerv2->entry);
+	entry = (void *)headerv2->entry;
+	entry();
+
+	/* The image probably never returns hence we wont reach that point */

s/wont/won't/
</nit>

[...]

drivers/usb/gadget/f_sdp.c line 717

Quoted 12 lines
drivers/usb/gadget/f_sdp.c new file +11 -0
+int sdp_add(struct usb_configuration *c)
+{
+	int id;
+
+	id = usb_string_id(c->cdev);
+	if (id < 0)
+		return id;
+	strings_sdp_generic[0].id = id;
+	sdp_intf_runtime.iInterface = id;
+
+	debug("%s: cdev: 0x%p gadget:0x%p gadget->ep0: 0x%p\n", __func__,

The string printed with '%p' already contains a '0x' prefix.

Lothar Waßmann

Signature · 9 lines
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
#5 lore raw

PATCH 2/7 usb: gadget: add SDP driver

In reply to Stefan Agner

Hi Stefan,

Quoted 10 lines

From: Stefan Agner <[email protected]>

Add SDP (Serial Downloader Protocol) implementation for U-Boot. The protocol is used in NXP SoC's boot ROM and allows to download program images. Beside that, it can also be used to read/write registers and download complete Device Configuration Data (DCD) sets. This basic implementation supports downloading images with the imx header format and reading registers.

Just some minor comments (despite comments from Lothar).

Quoted 46 lines

---

 drivers/usb/gadget/Kconfig  |   7 +
 drivers/usb/gadget/Makefile |   1 +
 drivers/usb/gadget/f_sdp.c  | 723 ++++++++++++++++++++++++++++++++++++++++++++
 include/sdp.h               |  16 +
 4 files changed, 747 insertions(+)
 create mode 100644 drivers/usb/gadget/f_sdp.c
 create mode 100644 include/sdp.h
drivers/usb/gadget/Kconfig +7 -0
index 261ed128ac..225b66bc95 100644
@@ -103,6 +103,13 @@ config USB_GADGET_DOWNLOAD
 
 if USB_GADGET_DOWNLOAD
 
+config USB_FUNCTION_SDP
+	bool "Enable USB SDP (Serial Download Protocol)"
+	help
+	  Enable Serial Download Protocol (SDP) device support in U-Boot. This
+	  allows to download images into memory and execute (jump to) them
+	  using the same protocol as implemented by the i.MX family's boot ROM.
+
 config G_DNL_MANUFACTURER
 	string "Vendor name of USB device"
 
drivers/usb/gadget/Makefile +1 -0
index 5e316a7cff..6a007d1bcb 100644
@@ -28,6 +28,7 @@ obj-$(CONFIG_USB_FUNCTION_THOR) += f_thor.o
 obj-$(CONFIG_USB_FUNCTION_DFU) += f_dfu.o
 obj-$(CONFIG_USB_FUNCTION_MASS_STORAGE) += f_mass_storage.o
 obj-$(CONFIG_USB_FUNCTION_FASTBOOT) += f_fastboot.o
+obj-$(CONFIG_USB_FUNCTION_SDP) += f_sdp.o
 endif
 endif
 ifdef CONFIG_USB_ETHER
drivers/usb/gadget/f_sdp.c new file +0 -0
new file mode 100644
index 0000000000..eb89695aaf

drivers/usb/gadget/f_sdp.c line 4

Quoted 4 lines
drivers/usb/gadget/f_sdp.c new file +4 -0
@@ -0,0 +1,723 @@
+/*
+ * f_sdp.c -- USB HID Serial Download Protocol
+ *
+ * Copyright (C) 2016 Toradex
		    ^^^^ - minor -> If you are going to prepare v2, please update the 

date.

Quoted 151 lines
drivers/usb/gadget/f_sdp.c new file +151 -0
+ * Author: Stefan Agner <[email protected]>
+ *
+ * This file implements the Serial Download Protocol (SDP) as specified in
+ * the i.MX 6 Reference Manual. The SDP is a USB HID based protocol and
+ * allows to download images directly to memory. The implementation
+ * works with the imx_loader (imx_usb) USB client software on host side.
+ *
+ * Not all commands are implemented, e.g. WRITE_REGISTER, DCD_WRITE and
+ * SKIP_DCD_HEADER are only stubs.
+ *
+ * Parts of the implementation are based on f_dfu and f_thor.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <errno.h>
+#include <common.h>
+#include <console.h>
+#include <malloc.h>
+
+#include <linux/usb/ch9.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/composite.h>
+
+#include <asm/io.h>
+#include <g_dnl.h>
+#include <sdp.h>
+#include <imximage.h>
+
+#define HID_REPORT_ID_MASK	0x000000ff
+
+/*
+ * HID class requests
+ */
+#define HID_REQ_GET_REPORT		0x01
+#define HID_REQ_GET_IDLE		0x02
+#define HID_REQ_GET_PROTOCOL		0x03
+#define HID_REQ_SET_REPORT		0x09
+#define HID_REQ_SET_IDLE		0x0A
+#define HID_REQ_SET_PROTOCOL		0x0B
+
+#define HID_USAGE_PAGE_LEN		76
+
+struct hid_report {
+	u8 usage_page[HID_USAGE_PAGE_LEN];
+} __packed;
+
+#define SDP_READ_REGISTER	0x0101
+#define SDP_WRITE_REGISTER	0x0202
+#define SDP_WRITE_FILE		0x0404
+#define SDP_ERROR_STATUS	0x0505
+#define SDP_DCD_WRITE		0x0a0a
+#define SDP_JUMP_ADDRESS	0x0b0b
+#define SDP_SKIP_DCD_HEADER	0x0c0c
+
+#define SDP_WRITE_FILE_COMPLETE		0x88888888
+#define SDP_WRITE_REGISTER_COMPLETE	0x128A8A12
+#define SDP_SKIP_DCD_HEADER_COMPLETE	0x900DD009
+#define SDP_ERROR_IMXHEADER		0x000a0533
+
+#define SDP_COMMAND_LEN		16
+
+struct sdp_command {
+	u16 cmd;
+	u32 addr;
+	u8 format;
+	u32 cnt;
+	u32 data;
+	u8 rsvd;
+} __packed;
+
+enum sdp_state {
+	SDP_STATE_IDLE,
+	SDP_STATE_RX_DCD_DATA,
+	SDP_STATE_RX_FILE_DATA,
+	SDP_STATE_TX_SEC_CONF,
+	SDP_STATE_TX_SEC_CONF_BUSY,
+	SDP_STATE_TX_REGISTER,
+	SDP_STATE_TX_REGISTER_BUSY,
+	SDP_STATE_TX_STATUS,
+	SDP_STATE_TX_STATUS_BUSY,
+	SDP_STATE_JUMP,
+};
+
+struct f_sdp {
+	struct usb_function		usb_function;
+
+	struct usb_descriptor_header	**function;
+
+	u8				altsetting;
+	enum sdp_state			state;
+	enum sdp_state			next_state;
+	u32				dnl_address;
+	u32				dnl_bytes_remaining;
+	u32				jmp_address;
+	bool				always_send_status;
+	u32				error_status;
+
+	/* EP0 request */
+	struct usb_request		*req;
+
+	/* EP1 IN */
+	struct usb_ep			*in_ep;
+	struct usb_request		*in_req;
+
+	bool				configuration_done;
+};
+
+static struct f_sdp *sdp_func;
+
+static inline struct f_sdp *func_to_sdp(struct usb_function *f)
+{
+	return container_of(f, struct f_sdp, usb_function);
+}
+
+static struct usb_interface_descriptor sdp_intf_runtime = {
+	.bLength =		sizeof(sdp_intf_runtime),
+	.bDescriptorType =	USB_DT_INTERFACE,
+	.bAlternateSetting =	0,
+	.bNumEndpoints =	1,
+	.bInterfaceClass =	USB_CLASS_HID,
+	.bInterfaceSubClass =	0,
+	.bInterfaceProtocol =	0,
+	/* .iInterface = DYNAMIC */
+};
+
+/* HID configuration */
+static struct usb_class_hid_descriptor sdp_hid_desc = {
+	.bLength =		sizeof(sdp_hid_desc),
+	.bDescriptorType =	USB_DT_CS_DEVICE,
+
+	.bcdCDC =		__constant_cpu_to_le16(0x0110),
+	.bCountryCode =		0,
+	.bNumDescriptors =	1,
+
+	.bDescriptorType0	= USB_DT_HID_REPORT,
+	.wDescriptorLength0	= HID_USAGE_PAGE_LEN,
+};
+
+static struct usb_endpoint_descriptor in_desc = {
+	.bLength =		USB_DT_ENDPOINT_SIZE,
+	.bDescriptorType =	USB_DT_ENDPOINT, /*USB_DT_CS_ENDPOINT*/
+
+	.bEndpointAddress =	1 | USB_DIR_IN,
+	.bmAttributes =	USB_ENDPOINT_XFER_INT,
+	.wMaxPacketSize =	64,
+	.bInterval =		1,
+};
+
+static struct usb_descriptor_header *sdp_runtime_descs[] = {
+	(struct usb_descriptor_header *)&sdp_intf_runtime,

drivers/usb/gadget/f_sdp.c line 165

Quoted 10 lines
drivers/usb/gadget/f_sdp.c new file +10 -0
+	(struct usb_descriptor_header *)&sdp_hid_desc,
+	(struct usb_descriptor_header *)&in_desc,
+	NULL,
+};
+
+/* This is synchronized with what the SoC implementation reports */
+static struct hid_report sdp_hid_report = {
+	.usage_page = {
+		0x06, 0x00, 0xff, /* Usage Page */
+		0x09, 0x01, /* Usage (Poiter?) */
				      ^^^^^^^ Pointer?
Quoted 187 lines
drivers/usb/gadget/f_sdp.c new file +187 -0
+		0xa1, 0x01, /* Collection */
+
+		0x85, 0x01, /* Report ID */
+		0x19, 0x01, /* Usage Minimum */
+		0x29, 0x01, /* Usage Maximum */
+		0x15, 0x00, /* Local Minimum */
+		0x26, 0xFF, 0x00, /* Local Maximum? */
+		0x75, 0x08, /* Report Size */
+		0x95, 0x10, /* Report Count */
+		0x91, 0x02, /* Output Data */
+
+		0x85, 0x02, /* Report ID */
+		0x19, 0x01, /* Usage Minimum */
+		0x29, 0x01, /* Usage Maximum */
+		0x15, 0x00, /* Local Minimum */
+		0x26, 0xFF, 0x00, /* Local Maximum? */
+		0x75, 0x80, /* Report Size 128 */
+		0x95, 0x40, /* Report Count */
+		0x91, 0x02, /* Output Data */
+
+		0x85, 0x03, /* Report ID */
+		0x19, 0x01, /* Usage Minimum */
+		0x29, 0x01, /* Usage Maximum */
+		0x15, 0x00, /* Local Minimum */
+		0x26, 0xFF, 0x00, /* Local Maximum? */
+		0x75, 0x08, /* Report Size 8 */
+		0x95, 0x04, /* Report Count */
+		0x81, 0x02, /* Input Data */
+
+		0x85, 0x04, /* Report ID */
+		0x19, 0x01, /* Usage Minimum */
+		0x29, 0x01, /* Usage Maximum */
+		0x15, 0x00, /* Local Minimum */
+		0x26, 0xFF, 0x00, /* Local Maximum? */
+		0x75, 0x08, /* Report Size 8 */
+		0x95, 0x40, /* Report Count */
+		0x81, 0x02, /* Input Data */
+		0xc0
+	},
+};
+
+static const char sdp_name[] = "Serial Downloader Protocol";
+
+/*
+ * static strings, in UTF-8
+ */
+static struct usb_string strings_sdp_generic[] = {
+	[0].s = sdp_name,
+	{  }			/* end of list */
+};
+
+static struct usb_gadget_strings stringtab_sdp_generic = {
+	.language	= 0x0409,	/* en-us */
+	.strings	= strings_sdp_generic,
+};
+
+static struct usb_gadget_strings *sdp_generic_strings[] = {
+	&stringtab_sdp_generic,
+	NULL,
+};
+
+static void sdp_rx_command_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_sdp *sdp = req->context;
+	int status = req->status;
+	u8 *data = req->buf;
+	u8 report = data[0];
+
+	if (status != 0) {
+		error("Status: %d", status);
+		return;
+	}
+
+	if (report != 1) {
+		error("Unexpected report %d", report);
+		return;
+	}
+
+	struct sdp_command *cmd = req->buf + 1;
+
+	debug("%s: command: %04x, addr: %08x, cnt: %u\n",
+	      __func__, be16_to_cpu(cmd->cmd),
+	      be32_to_cpu(cmd->addr), be32_to_cpu(cmd->cnt));
+
+	switch (be16_to_cpu(cmd->cmd)) {
+	case SDP_READ_REGISTER:
+		sdp->always_send_status = false;
+		sdp->error_status = 0x0;
+
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->dnl_address = be32_to_cpu(cmd->addr);
+		sdp->dnl_bytes_remaining = be32_to_cpu(cmd->cnt);
+		sdp->next_state = SDP_STATE_TX_REGISTER;
+		printf("Reading %d registers at 0x%08x... ",
+		       sdp->dnl_bytes_remaining, sdp->dnl_address);
+		break;
+	case SDP_WRITE_FILE:
+		sdp->always_send_status = true;
+		sdp->error_status = SDP_WRITE_FILE_COMPLETE;
+
+		sdp->state = SDP_STATE_RX_FILE_DATA;
+		sdp->dnl_address = be32_to_cpu(cmd->addr);
+		sdp->dnl_bytes_remaining = be32_to_cpu(cmd->cnt);
+		sdp->next_state = SDP_STATE_IDLE;
+
+		printf("Downloading file of size %d to 0x%08x... ",
+		       sdp->dnl_bytes_remaining, sdp->dnl_address);
+
+		break;
+	case SDP_ERROR_STATUS:
+		sdp->always_send_status = true;
+		sdp->error_status = 0;
+
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->next_state = SDP_STATE_IDLE;
+		break;
+	case SDP_DCD_WRITE:
+		sdp->always_send_status = true;
+		sdp->error_status = SDP_WRITE_REGISTER_COMPLETE;
+
+		sdp->state = SDP_STATE_RX_DCD_DATA;
+		sdp->dnl_bytes_remaining = be32_to_cpu(cmd->cnt);
+		sdp->next_state = SDP_STATE_IDLE;
+		break;
+	case SDP_JUMP_ADDRESS:
+		sdp->always_send_status = false;
+		sdp->error_status = 0;
+
+		sdp->jmp_address = be32_to_cpu(cmd->addr);
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->next_state = SDP_STATE_JUMP;
+		break;
+	case SDP_SKIP_DCD_HEADER:
+		sdp->always_send_status = true;
+		sdp->error_status = SDP_SKIP_DCD_HEADER_COMPLETE;
+
+		/* Ignore command, DCD not supported anyway */
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->next_state = SDP_STATE_IDLE;
+		break;
+	default:
+		error("Unknown command: %08x\n", be16_to_cpu(cmd->cmd));
+	}
+}
+
+static void sdp_rx_data_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_sdp *sdp = req->context;
+	int status = req->status;
+	u8 *data = req->buf;
+	u8 report = data[0];
+	int datalen = req->length - 1;
+
+	if (status != 0) {
+		error("Status: %d", status);
+		return;
+	}
+
+	if (report != 2) {
+		error("Unexpected report %d", report);
+		return;
+	}
+
+	if (sdp->dnl_bytes_remaining < datalen) {
+		/*
+		 * Some USB stacks require to send a complete buffer as
+		 * specified in the HID descriptor. This leads to longer
+		 * transfers than the file length, no problem for us.
+		 */
+		sdp->dnl_bytes_remaining = 0;
+	} else {
+		sdp->dnl_bytes_remaining -= datalen;
+	}
+
+	if (sdp->state == SDP_STATE_RX_FILE_DATA) {
+		memcpy((void *)sdp->dnl_address, req->buf + 1, datalen);
+		sdp->dnl_address += datalen;
+	}
+
+	if (sdp->dnl_bytes_remaining)
+		return;
+
+	printf("done\n");
+
+	switch (sdp->state) {
+	case SDP_STATE_RX_FILE_DATA:
+		sdp->state = SDP_STATE_TX_SEC_CONF;

drivers/usb/gadget/f_sdp.c line 362

Quoted 10 lines
drivers/usb/gadget/f_sdp.c new file +10 -0
+		break;
+	case SDP_STATE_RX_DCD_DATA:
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		break;
+	default:
+		error("Invalid state: %d", sdp->state);
+	}
+}
+
+

Please remove blank lines.

Quoted 384 lines
drivers/usb/gadget/f_sdp.c new file +361 -0
+
+static void sdp_tx_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_sdp *sdp = req->context;
+	int status = req->status;
+
+	if (status != 0) {
+		error("Status: %d", status);
+		return;
+	}
+
+	switch (sdp->state) {
+	case SDP_STATE_TX_SEC_CONF_BUSY:
+		/* Not all commands require status report */
+		if (sdp->always_send_status || sdp->error_status)
+			sdp->state = SDP_STATE_TX_STATUS;
+		else
+			sdp->state = sdp->next_state;
+
+		break;
+	case SDP_STATE_TX_STATUS_BUSY:
+		sdp->state = sdp->next_state;
+		break;
+	case SDP_STATE_TX_REGISTER_BUSY:
+		if (sdp->dnl_bytes_remaining)
+			sdp->state = SDP_STATE_TX_REGISTER;
+		else
+			sdp->state = SDP_STATE_IDLE;
+		break;
+	default:
+		error("Wrong State: %d", sdp->state);
+		sdp->state = SDP_STATE_IDLE;
+		break;
+	}
+	debug("%s complete --> %d, %d/%d\n", ep->name,
+	      status, req->actual, req->length);
+}
+
+static int sdp_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
+{
+	struct usb_gadget *gadget = f->config->cdev->gadget;
+	struct usb_request *req = f->config->cdev->req;
+	struct f_sdp *sdp = f->config->cdev->req->context;
+	u16 len = le16_to_cpu(ctrl->wLength);
+	u16 w_value = le16_to_cpu(ctrl->wValue);
+	int value = 0;
+	u8 req_type = ctrl->bRequestType & USB_TYPE_MASK;
+
+	debug("w_value: 0x%x len: 0x%x\n", w_value, len);
+	debug("req_type: 0x%x ctrl->bRequest: 0x%x sdp->state: %d\n",
+	      req_type, ctrl->bRequest, sdp->state);
+
+	if (req_type == USB_TYPE_STANDARD) {
+		if (ctrl->bRequest == USB_REQ_GET_DESCRIPTOR) {
+			/* Send HID report descriptor */
+			value = min(len, (u16) sizeof(sdp_hid_report));
+			memcpy(req->buf, &sdp_hid_report, value);
+			sdp->configuration_done = true;
+		}
+	}
+
+	if (req_type == USB_TYPE_CLASS) {
+		int report = w_value & HID_REPORT_ID_MASK;
+
+		/* HID (SDP) request */
+		switch (ctrl->bRequest) {
+		case HID_REQ_SET_REPORT:
+			switch (report) {
+			case 1:
+				value = SDP_COMMAND_LEN + 1;
+				req->complete = sdp_rx_command_complete;
+				break;
+			case 2:
+				value = len;
+				req->complete = sdp_rx_data_complete;
+				break;
+			}
+		}
+	}
+
+	if (value >= 0) {
+		req->length = value;
+		req->zero = value < len;
+		value = usb_ep_queue(gadget->ep0, req, 0);
+		if (value < 0) {
+			debug("ep_queue --> %d\n", value);
+			req->status = 0;
+		}
+	}
+
+	return value;
+}
+
+static int sdp_bind(struct usb_configuration *c, struct usb_function *f)
+{
+	struct usb_gadget *gadget = c->cdev->gadget;
+	struct usb_composite_dev *cdev = c->cdev;
+	struct f_sdp *sdp = func_to_sdp(f);
+	int rv = 0, id;
+
+	id = usb_interface_id(c, f);
+	if (id < 0)
+		return id;
+	sdp_intf_runtime.bInterfaceNumber = id;
+
+	struct usb_ep *ep;
+
+	/* allocate instance-specific endpoints */
+	ep = usb_ep_autoconfig(gadget, &in_desc);
+	if (!ep) {
+		rv = -ENODEV;
+		goto error;
+	}
+
+	sdp->in_ep = ep; /* Store IN EP for enabling @ setup */
+
+	cdev->req->context = sdp;
+
+error:
+	return rv;
+}
+
+static void sdp_unbind(struct usb_configuration *c, struct usb_function *f)
+{
+	free(sdp_func);
+	sdp_func = NULL;
+}
+
+static struct usb_request *alloc_ep_req(struct usb_ep *ep, unsigned length)
+{
+	struct usb_request *req;
+
+	req = usb_ep_alloc_request(ep, 0);
+	if (!req)
+		return req;
+
+	req->length = length;
+	req->buf = memalign(CONFIG_SYS_CACHELINE_SIZE, length);
+	if (!req->buf) {
+		usb_ep_free_request(ep, req);
+		req = NULL;
+	}
+
+	return req;
+}
+
+
+static struct usb_request *sdp_start_ep(struct usb_ep *ep)
+{
+	struct usb_request *req;
+
+	req = alloc_ep_req(ep, 64);
+	debug("%s: ep:%p req:%p\n", __func__, ep, req);
+
+	if (!req)
+		return NULL;
+
+	memset(req->buf, 0, req->length);
+	req->complete = sdp_tx_complete;
+
+	return req;
+}
+static int sdp_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
+{
+	struct f_sdp *sdp = func_to_sdp(f);
+	struct usb_composite_dev *cdev = f->config->cdev;
+	int result;
+
+	debug("%s: intf: %d alt: %d\n", __func__, intf, alt);
+
+	result = usb_ep_enable(sdp->in_ep, &in_desc);
+	if (result)
+		return result;
+	sdp->in_req = sdp_start_ep(sdp->in_ep);
+	sdp->in_req->context = sdp;
+
+	sdp->in_ep->driver_data = cdev; /* claim */
+
+	sdp->altsetting = alt;
+	sdp->state = SDP_STATE_IDLE;
+
+	return 0;
+}
+
+static int sdp_get_alt(struct usb_function *f, unsigned intf)
+{
+	struct f_sdp *sdp = func_to_sdp(f);
+
+	return sdp->altsetting;
+}
+
+static void sdp_disable(struct usb_function *f)
+{
+	struct f_sdp *sdp = func_to_sdp(f);
+
+	usb_ep_disable(sdp->in_ep);
+
+	if (sdp->in_req) {
+		free(sdp->in_req);
+		sdp->in_req = NULL;
+	}
+}
+
+static int sdp_bind_config(struct usb_configuration *c)
+{
+	int status;
+
+	if (!sdp_func) {
+		sdp_func = memalign(CONFIG_SYS_CACHELINE_SIZE, sizeof(*sdp_func));
+		if (!sdp_func)
+			return -ENOMEM;
+	}
+
+	memset(sdp_func, 0, sizeof(*sdp_func));
+
+	sdp_func->usb_function.name = "sdp";
+	sdp_func->usb_function.hs_descriptors = sdp_runtime_descs;
+	sdp_func->usb_function.descriptors = sdp_runtime_descs;
+	sdp_func->usb_function.bind = sdp_bind;
+	sdp_func->usb_function.unbind = sdp_unbind;
+	sdp_func->usb_function.set_alt = sdp_set_alt;
+	sdp_func->usb_function.get_alt = sdp_get_alt;
+	sdp_func->usb_function.disable = sdp_disable;
+	sdp_func->usb_function.strings = sdp_generic_strings;
+	sdp_func->usb_function.setup = sdp_setup;
+
+	status = usb_add_function(c, &sdp_func->usb_function);
+
+	return status;
+}
+
+int sdp_init(void)
+{
+	printf("SDP: initialize...\n");
+	while (!sdp_func->configuration_done) {
+		if (ctrlc()) {
+			puts("\rCTRL+C - Operation aborted.\n");
+			return 0;
+		}
+		usb_gadget_handle_interrupts(0);
+	}
+
+	return 0;
+}
+
+static u32 sdp_jump_imxheader(void *address)
+{
+	flash_header_v2_t *headerv2 = address;
+	ulong (*entry)(void);
+
+	if (headerv2->header.tag != IVT_HEADER_TAG) {
+		printf("Header Tag is not a IMX image\n");
+		return SDP_ERROR_IMXHEADER;
+	}
+
+	printf("Jumping to 0x%08x\n", headerv2->entry);
+	entry = (void *)headerv2->entry;
+	entry();
+
+	/* The image probably never returns hence we wont reach that point */
+	return 0;
+}
+
+static void sdp_handle_in_ep(void)
+{
+	u8 *data = sdp_func->in_req->buf;
+	u32 status;
+	int datalen;
+
+	switch (sdp_func->state) {
+	case SDP_STATE_TX_SEC_CONF:
+		debug("Report 3: HAB security\n");
+		data[0] = 3;
+
+		data[1] = 0x56;
+		data[2] = 0x78;
+		data[3] = 0x78;
+		data[4] = 0x56;
+
+		sdp_func->in_req->length = 5;
+		usb_ep_queue(sdp_func->in_ep, sdp_func->in_req, 0);
+		sdp_func->state = SDP_STATE_TX_SEC_CONF_BUSY;
+		break;
+
+	case SDP_STATE_TX_STATUS:
+		debug("Report 4: Status\n");
+		data[0] = 4;
+
+		memcpy(&data[1], &sdp_func->error_status, 4);
+		sdp_func->in_req->length = 65;
+		usb_ep_queue(sdp_func->in_ep, sdp_func->in_req, 0);
+		sdp_func->state = SDP_STATE_TX_STATUS_BUSY;
+		break;
+	case SDP_STATE_TX_REGISTER:
+		debug("Report 4: Register Values\n");
+		data[0] = 4;
+
+		datalen = sdp_func->dnl_bytes_remaining;
+
+		if (datalen > 64)
+			datalen = 64;
+
+		memcpy(&data[1], (void *)sdp_func->dnl_address, datalen);
+		sdp_func->in_req->length = 65;
+
+		sdp_func->dnl_bytes_remaining -= datalen;
+		sdp_func->dnl_address += datalen;
+
+		usb_ep_queue(sdp_func->in_ep, sdp_func->in_req, 0);
+		sdp_func->state = SDP_STATE_TX_REGISTER_BUSY;
+		break;
+	case SDP_STATE_JUMP:
+		printf("Checking imxheader at 0x%08x\n", f_sdp->jmp_address);
+		status = sdp_jump_imxheader((void *)f_sdp->jmp_address);
+
+		sdp_func->next_state = SDP_STATE_IDLE;
+		sdp_func->error_status = status;
+
+		/* Only send Report 4 if there was an error */
+		if (status)
+			sdp_func->state = SDP_STATE_TX_STATUS;
+		else
+			sdp_func->state = SDP_STATE_IDLE;
+		break;
+	default:
+		break;
+	};
+}
+
+int sdp_handle(void)
+{
+	printf("SDP: handle requests...\n");
+	while (1) {
+		if (ctrlc()) {
+			puts("\rCTRL+C - Operation aborted.\n");
+			return 0;
+		}
+
+		usb_gadget_handle_interrupts(0);
+
+		sdp_handle_in_ep();
+	}
+}
+
+int sdp_add(struct usb_configuration *c)
+{
+	int id;
+
+	id = usb_string_id(c->cdev);
+	if (id < 0)
+		return id;
+	strings_sdp_generic[0].id = id;
+	sdp_intf_runtime.iInterface = id;
+
+	debug("%s: cdev: 0x%p gadget:0x%p gadget->ep0: 0x%p\n", __func__,
+	      c->cdev, c->cdev->gadget, c->cdev->gadget->ep0);
+
+	return sdp_bind_config(c);
+}
+
+DECLARE_GADGET_BIND_CALLBACK(usb_dnl_sdp, sdp_add);
include/sdp.h new file +16 -0
new file mode 100644
index 0000000000..03c4a23434
@@ -0,0 +1,16 @@
+/*
+ * sdp.h - Serial Download Protocol
+ *
+ * Copyright (C) 2016 Toradex
+ * Author: Stefan Agner <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __SDP_H_
+#define __SDP_H_
+
+int sdp_init(void);
+int sdp_handle(void);
+
+#endif /* __SDP_H_ */
Signature · 9 lines
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
#6 lore raw

PATCH 2/7 usb: gadget: add SDP driver

In reply to Stefan Agner

Hi Stefan,

Quoted 104 lines · Stefan Agner

From: Stefan Agner <[email protected]>

Add SDP (Serial Downloader Protocol) implementation for U-Boot. The protocol is used in NXP SoC's boot ROM and allows to download program images. Beside that, it can also be used to read/write registers and download complete Device Configuration Data (DCD) sets. This basic implementation supports downloading images with the imx header format and reading registers.

---

 drivers/usb/gadget/Kconfig  |   7 +
 drivers/usb/gadget/Makefile |   1 +
 drivers/usb/gadget/f_sdp.c  | 723 ++++++++++++++++++++++++++++++++++++++++++++
 include/sdp.h               |  16 +
 4 files changed, 747 insertions(+)
 create mode 100644 drivers/usb/gadget/f_sdp.c
 create mode 100644 include/sdp.h
drivers/usb/gadget/Kconfig +7 -0
index 261ed128ac..225b66bc95 100644
@@ -103,6 +103,13 @@ config USB_GADGET_DOWNLOAD
 
 if USB_GADGET_DOWNLOAD
 
+config USB_FUNCTION_SDP
+	bool "Enable USB SDP (Serial Download Protocol)"
+	help
+	  Enable Serial Download Protocol (SDP) device support in U-Boot. This
+	  allows to download images into memory and execute (jump to) them
+	  using the same protocol as implemented by the i.MX family's boot ROM.
+
 config G_DNL_MANUFACTURER
 	string "Vendor name of USB device"
 
drivers/usb/gadget/Makefile +1 -0
index 5e316a7cff..6a007d1bcb 100644
@@ -28,6 +28,7 @@ obj-$(CONFIG_USB_FUNCTION_THOR) += f_thor.o
 obj-$(CONFIG_USB_FUNCTION_DFU) += f_dfu.o
 obj-$(CONFIG_USB_FUNCTION_MASS_STORAGE) += f_mass_storage.o
 obj-$(CONFIG_USB_FUNCTION_FASTBOOT) += f_fastboot.o
+obj-$(CONFIG_USB_FUNCTION_SDP) += f_sdp.o
 endif
 endif
 ifdef CONFIG_USB_ETHER
drivers/usb/gadget/f_sdp.c new file +48 -0
new file mode 100644
index 0000000000..eb89695aaf
@@ -0,0 +1,723 @@
+/*
+ * f_sdp.c -- USB HID Serial Download Protocol
+ *
+ * Copyright (C) 2016 Toradex
+ * Author: Stefan Agner <[email protected]>
+ *
+ * This file implements the Serial Download Protocol (SDP) as specified in
+ * the i.MX 6 Reference Manual. The SDP is a USB HID based protocol and
+ * allows to download images directly to memory. The implementation
+ * works with the imx_loader (imx_usb) USB client software on host side.
+ *
+ * Not all commands are implemented, e.g. WRITE_REGISTER, DCD_WRITE and
+ * SKIP_DCD_HEADER are only stubs.
+ *
+ * Parts of the implementation are based on f_dfu and f_thor.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <errno.h>
+#include <common.h>
+#include <console.h>
+#include <malloc.h>
+
+#include <linux/usb/ch9.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/composite.h>
+
+#include <asm/io.h>
+#include <g_dnl.h>
+#include <sdp.h>
+#include <imximage.h>
+
+#define HID_REPORT_ID_MASK	0x000000ff
+
+/*
+ * HID class requests
+ */
+#define HID_REQ_GET_REPORT		0x01
+#define HID_REQ_GET_IDLE		0x02
+#define HID_REQ_GET_PROTOCOL		0x03
+#define HID_REQ_SET_REPORT		0x09
+#define HID_REQ_SET_IDLE		0x0A
+#define HID_REQ_SET_PROTOCOL		0x0B
+
+#define HID_USAGE_PAGE_LEN		76
+
+struct hid_report {

drivers/usb/gadget/f_sdp.c line 58

Quoted 10 lines
drivers/usb/gadget/f_sdp.c new file +10 -0
+	u8 usage_page[HID_USAGE_PAGE_LEN];
+} __packed;
+
+#define SDP_READ_REGISTER	0x0101
+#define SDP_WRITE_REGISTER	0x0202
+#define SDP_WRITE_FILE		0x0404
+#define SDP_ERROR_STATUS	0x0505
+#define SDP_DCD_WRITE		0x0a0a
+#define SDP_JUMP_ADDRESS	0x0b0b
+#define SDP_SKIP_DCD_HEADER	0x0c0c

It looks like that I am again out of sync with documentation. Where is defined SDP_SKIP_DCD_HEADER ? It is undefined for MX6Q/D, Solo and DL.

Quoted 221 lines
drivers/usb/gadget/f_sdp.c new file +221 -0
+
+#define SDP_WRITE_FILE_COMPLETE		0x88888888
+#define SDP_WRITE_REGISTER_COMPLETE	0x128A8A12
+#define SDP_SKIP_DCD_HEADER_COMPLETE	0x900DD009
+#define SDP_ERROR_IMXHEADER		0x000a0533
+
+#define SDP_COMMAND_LEN		16
+
+struct sdp_command {
+	u16 cmd;
+	u32 addr;
+	u8 format;
+	u32 cnt;
+	u32 data;
+	u8 rsvd;
+} __packed;
+
+enum sdp_state {
+	SDP_STATE_IDLE,
+	SDP_STATE_RX_DCD_DATA,
+	SDP_STATE_RX_FILE_DATA,
+	SDP_STATE_TX_SEC_CONF,
+	SDP_STATE_TX_SEC_CONF_BUSY,
+	SDP_STATE_TX_REGISTER,
+	SDP_STATE_TX_REGISTER_BUSY,
+	SDP_STATE_TX_STATUS,
+	SDP_STATE_TX_STATUS_BUSY,
+	SDP_STATE_JUMP,
+};
+
+struct f_sdp {
+	struct usb_function		usb_function;
+
+	struct usb_descriptor_header	**function;
+
+	u8				altsetting;
+	enum sdp_state			state;
+	enum sdp_state			next_state;
+	u32				dnl_address;
+	u32				dnl_bytes_remaining;
+	u32				jmp_address;
+	bool				always_send_status;
+	u32				error_status;
+
+	/* EP0 request */
+	struct usb_request		*req;
+
+	/* EP1 IN */
+	struct usb_ep			*in_ep;
+	struct usb_request		*in_req;
+
+	bool				configuration_done;
+};
+
+static struct f_sdp *sdp_func;
+
+static inline struct f_sdp *func_to_sdp(struct usb_function *f)
+{
+	return container_of(f, struct f_sdp, usb_function);
+}
+
+static struct usb_interface_descriptor sdp_intf_runtime = {
+	.bLength =		sizeof(sdp_intf_runtime),
+	.bDescriptorType =	USB_DT_INTERFACE,
+	.bAlternateSetting =	0,
+	.bNumEndpoints =	1,
+	.bInterfaceClass =	USB_CLASS_HID,
+	.bInterfaceSubClass =	0,
+	.bInterfaceProtocol =	0,
+	/* .iInterface = DYNAMIC */
+};
+
+/* HID configuration */
+static struct usb_class_hid_descriptor sdp_hid_desc = {
+	.bLength =		sizeof(sdp_hid_desc),
+	.bDescriptorType =	USB_DT_CS_DEVICE,
+
+	.bcdCDC =		__constant_cpu_to_le16(0x0110),
+	.bCountryCode =		0,
+	.bNumDescriptors =	1,
+
+	.bDescriptorType0	= USB_DT_HID_REPORT,
+	.wDescriptorLength0	= HID_USAGE_PAGE_LEN,
+};
+
+static struct usb_endpoint_descriptor in_desc = {
+	.bLength =		USB_DT_ENDPOINT_SIZE,
+	.bDescriptorType =	USB_DT_ENDPOINT, /*USB_DT_CS_ENDPOINT*/
+
+	.bEndpointAddress =	1 | USB_DIR_IN,
+	.bmAttributes =	USB_ENDPOINT_XFER_INT,
+	.wMaxPacketSize =	64,
+	.bInterval =		1,
+};
+
+static struct usb_descriptor_header *sdp_runtime_descs[] = {
+	(struct usb_descriptor_header *)&sdp_intf_runtime,
+	(struct usb_descriptor_header *)&sdp_hid_desc,
+	(struct usb_descriptor_header *)&in_desc,
+	NULL,
+};
+
+/* This is synchronized with what the SoC implementation reports */
+static struct hid_report sdp_hid_report = {
+	.usage_page = {
+		0x06, 0x00, 0xff, /* Usage Page */
+		0x09, 0x01, /* Usage (Poiter?) */
+		0xa1, 0x01, /* Collection */
+
+		0x85, 0x01, /* Report ID */
+		0x19, 0x01, /* Usage Minimum */
+		0x29, 0x01, /* Usage Maximum */
+		0x15, 0x00, /* Local Minimum */
+		0x26, 0xFF, 0x00, /* Local Maximum? */
+		0x75, 0x08, /* Report Size */
+		0x95, 0x10, /* Report Count */
+		0x91, 0x02, /* Output Data */
+
+		0x85, 0x02, /* Report ID */
+		0x19, 0x01, /* Usage Minimum */
+		0x29, 0x01, /* Usage Maximum */
+		0x15, 0x00, /* Local Minimum */
+		0x26, 0xFF, 0x00, /* Local Maximum? */
+		0x75, 0x80, /* Report Size 128 */
+		0x95, 0x40, /* Report Count */
+		0x91, 0x02, /* Output Data */
+
+		0x85, 0x03, /* Report ID */
+		0x19, 0x01, /* Usage Minimum */
+		0x29, 0x01, /* Usage Maximum */
+		0x15, 0x00, /* Local Minimum */
+		0x26, 0xFF, 0x00, /* Local Maximum? */
+		0x75, 0x08, /* Report Size 8 */
+		0x95, 0x04, /* Report Count */
+		0x81, 0x02, /* Input Data */
+
+		0x85, 0x04, /* Report ID */
+		0x19, 0x01, /* Usage Minimum */
+		0x29, 0x01, /* Usage Maximum */
+		0x15, 0x00, /* Local Minimum */
+		0x26, 0xFF, 0x00, /* Local Maximum? */
+		0x75, 0x08, /* Report Size 8 */
+		0x95, 0x40, /* Report Count */
+		0x81, 0x02, /* Input Data */
+		0xc0
+	},
+};
+
+static const char sdp_name[] = "Serial Downloader Protocol";
+
+/*
+ * static strings, in UTF-8
+ */
+static struct usb_string strings_sdp_generic[] = {
+	[0].s = sdp_name,
+	{  }			/* end of list */
+};
+
+static struct usb_gadget_strings stringtab_sdp_generic = {
+	.language	= 0x0409,	/* en-us */
+	.strings	= strings_sdp_generic,
+};
+
+static struct usb_gadget_strings *sdp_generic_strings[] = {
+	&stringtab_sdp_generic,
+	NULL,
+};
+
+static void sdp_rx_command_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_sdp *sdp = req->context;
+	int status = req->status;
+	u8 *data = req->buf;
+	u8 report = data[0];
+
+	if (status != 0) {
+		error("Status: %d", status);
+		return;
+	}
+
+	if (report != 1) {
+		error("Unexpected report %d", report);
+		return;
+	}
+
+	struct sdp_command *cmd = req->buf + 1;
+
+	debug("%s: command: %04x, addr: %08x, cnt: %u\n",
+	      __func__, be16_to_cpu(cmd->cmd),
+	      be32_to_cpu(cmd->addr), be32_to_cpu(cmd->cnt));
+
+	switch (be16_to_cpu(cmd->cmd)) {
+	case SDP_READ_REGISTER:
+		sdp->always_send_status = false;
+		sdp->error_status = 0x0;
+
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->dnl_address = be32_to_cpu(cmd->addr);
+		sdp->dnl_bytes_remaining = be32_to_cpu(cmd->cnt);
+		sdp->next_state = SDP_STATE_TX_REGISTER;
+		printf("Reading %d registers at 0x%08x... ",
+		       sdp->dnl_bytes_remaining, sdp->dnl_address);
+		break;
+	case SDP_WRITE_FILE:
+		sdp->always_send_status = true;
+		sdp->error_status = SDP_WRITE_FILE_COMPLETE;
+
+		sdp->state = SDP_STATE_RX_FILE_DATA;
+		sdp->dnl_address = be32_to_cpu(cmd->addr);
+		sdp->dnl_bytes_remaining = be32_to_cpu(cmd->cnt);
+		sdp->next_state = SDP_STATE_IDLE;
+
+		printf("Downloading file of size %d to 0x%08x... ",
+		       sdp->dnl_bytes_remaining, sdp->dnl_address);
+
+		break;
+	case SDP_ERROR_STATUS:
+		sdp->always_send_status = true;
+		sdp->error_status = 0;
+
+		sdp->state = SDP_STATE_TX_SEC_CONF;

drivers/usb/gadget/f_sdp.c line 289

Quoted 10 lines
drivers/usb/gadget/f_sdp.c new file +10 -0
+		sdp->next_state = SDP_STATE_IDLE;
+		break;
+	case SDP_DCD_WRITE:
+		sdp->always_send_status = true;
+		sdp->error_status = SDP_WRITE_REGISTER_COMPLETE;
+
+		sdp->state = SDP_STATE_RX_DCD_DATA;
+		sdp->dnl_bytes_remaining = be32_to_cpu(cmd->cnt);
+		sdp->next_state = SDP_STATE_IDLE;
+		break;

It is fine, but I am just missing if this is a use case. DCD is interpreted by boot ROM, and we are here already over in SPL.

drivers/usb/gadget/f_sdp.c line 302

Quoted 13 lines
drivers/usb/gadget/f_sdp.c new file +13 -0
+	case SDP_JUMP_ADDRESS:
+		sdp->always_send_status = false;
+		sdp->error_status = 0;
+
+		sdp->jmp_address = be32_to_cpu(cmd->addr);
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->next_state = SDP_STATE_JUMP;
+		break;
+	case SDP_SKIP_DCD_HEADER:
+		sdp->always_send_status = true;
+		sdp->error_status = SDP_SKIP_DCD_HEADER_COMPLETE;
+
+		/* Ignore command, DCD not supported anyway */

Right - we load a file, we do not need a DCD.

Quoted 328 lines
drivers/usb/gadget/f_sdp.c new file +328 -0
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->next_state = SDP_STATE_IDLE;
+		break;
+	default:
+		error("Unknown command: %08x\n", be16_to_cpu(cmd->cmd));
+	}
+}
+
+static void sdp_rx_data_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_sdp *sdp = req->context;
+	int status = req->status;
+	u8 *data = req->buf;
+	u8 report = data[0];
+	int datalen = req->length - 1;
+
+	if (status != 0) {
+		error("Status: %d", status);
+		return;
+	}
+
+	if (report != 2) {
+		error("Unexpected report %d", report);
+		return;
+	}
+
+	if (sdp->dnl_bytes_remaining < datalen) {
+		/*
+		 * Some USB stacks require to send a complete buffer as
+		 * specified in the HID descriptor. This leads to longer
+		 * transfers than the file length, no problem for us.
+		 */
+		sdp->dnl_bytes_remaining = 0;
+	} else {
+		sdp->dnl_bytes_remaining -= datalen;
+	}
+
+	if (sdp->state == SDP_STATE_RX_FILE_DATA) {
+		memcpy((void *)sdp->dnl_address, req->buf + 1, datalen);
+		sdp->dnl_address += datalen;
+	}
+
+	if (sdp->dnl_bytes_remaining)
+		return;
+
+	printf("done\n");
+
+	switch (sdp->state) {
+	case SDP_STATE_RX_FILE_DATA:
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		break;
+	case SDP_STATE_RX_DCD_DATA:
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		break;
+	default:
+		error("Invalid state: %d", sdp->state);
+	}
+}
+
+
+
+static void sdp_tx_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_sdp *sdp = req->context;
+	int status = req->status;
+
+	if (status != 0) {
+		error("Status: %d", status);
+		return;
+	}
+
+	switch (sdp->state) {
+	case SDP_STATE_TX_SEC_CONF_BUSY:
+		/* Not all commands require status report */
+		if (sdp->always_send_status || sdp->error_status)
+			sdp->state = SDP_STATE_TX_STATUS;
+		else
+			sdp->state = sdp->next_state;
+
+		break;
+	case SDP_STATE_TX_STATUS_BUSY:
+		sdp->state = sdp->next_state;
+		break;
+	case SDP_STATE_TX_REGISTER_BUSY:
+		if (sdp->dnl_bytes_remaining)
+			sdp->state = SDP_STATE_TX_REGISTER;
+		else
+			sdp->state = SDP_STATE_IDLE;
+		break;
+	default:
+		error("Wrong State: %d", sdp->state);
+		sdp->state = SDP_STATE_IDLE;
+		break;
+	}
+	debug("%s complete --> %d, %d/%d\n", ep->name,
+	      status, req->actual, req->length);
+}
+
+static int sdp_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
+{
+	struct usb_gadget *gadget = f->config->cdev->gadget;
+	struct usb_request *req = f->config->cdev->req;
+	struct f_sdp *sdp = f->config->cdev->req->context;
+	u16 len = le16_to_cpu(ctrl->wLength);
+	u16 w_value = le16_to_cpu(ctrl->wValue);
+	int value = 0;
+	u8 req_type = ctrl->bRequestType & USB_TYPE_MASK;
+
+	debug("w_value: 0x%x len: 0x%x\n", w_value, len);
+	debug("req_type: 0x%x ctrl->bRequest: 0x%x sdp->state: %d\n",
+	      req_type, ctrl->bRequest, sdp->state);
+
+	if (req_type == USB_TYPE_STANDARD) {
+		if (ctrl->bRequest == USB_REQ_GET_DESCRIPTOR) {
+			/* Send HID report descriptor */
+			value = min(len, (u16) sizeof(sdp_hid_report));
+			memcpy(req->buf, &sdp_hid_report, value);
+			sdp->configuration_done = true;
+		}
+	}
+
+	if (req_type == USB_TYPE_CLASS) {
+		int report = w_value & HID_REPORT_ID_MASK;
+
+		/* HID (SDP) request */
+		switch (ctrl->bRequest) {
+		case HID_REQ_SET_REPORT:
+			switch (report) {
+			case 1:
+				value = SDP_COMMAND_LEN + 1;
+				req->complete = sdp_rx_command_complete;
+				break;
+			case 2:
+				value = len;
+				req->complete = sdp_rx_data_complete;
+				break;
+			}
+		}
+	}
+
+	if (value >= 0) {
+		req->length = value;
+		req->zero = value < len;
+		value = usb_ep_queue(gadget->ep0, req, 0);
+		if (value < 0) {
+			debug("ep_queue --> %d\n", value);
+			req->status = 0;
+		}
+	}
+
+	return value;
+}
+
+static int sdp_bind(struct usb_configuration *c, struct usb_function *f)
+{
+	struct usb_gadget *gadget = c->cdev->gadget;
+	struct usb_composite_dev *cdev = c->cdev;
+	struct f_sdp *sdp = func_to_sdp(f);
+	int rv = 0, id;
+
+	id = usb_interface_id(c, f);
+	if (id < 0)
+		return id;
+	sdp_intf_runtime.bInterfaceNumber = id;
+
+	struct usb_ep *ep;
+
+	/* allocate instance-specific endpoints */
+	ep = usb_ep_autoconfig(gadget, &in_desc);
+	if (!ep) {
+		rv = -ENODEV;
+		goto error;
+	}
+
+	sdp->in_ep = ep; /* Store IN EP for enabling @ setup */
+
+	cdev->req->context = sdp;
+
+error:
+	return rv;
+}
+
+static void sdp_unbind(struct usb_configuration *c, struct usb_function *f)
+{
+	free(sdp_func);
+	sdp_func = NULL;
+}
+
+static struct usb_request *alloc_ep_req(struct usb_ep *ep, unsigned length)
+{
+	struct usb_request *req;
+
+	req = usb_ep_alloc_request(ep, 0);
+	if (!req)
+		return req;
+
+	req->length = length;
+	req->buf = memalign(CONFIG_SYS_CACHELINE_SIZE, length);
+	if (!req->buf) {
+		usb_ep_free_request(ep, req);
+		req = NULL;
+	}
+
+	return req;
+}
+
+
+static struct usb_request *sdp_start_ep(struct usb_ep *ep)
+{
+	struct usb_request *req;
+
+	req = alloc_ep_req(ep, 64);
+	debug("%s: ep:%p req:%p\n", __func__, ep, req);
+
+	if (!req)
+		return NULL;
+
+	memset(req->buf, 0, req->length);
+	req->complete = sdp_tx_complete;
+
+	return req;
+}
+static int sdp_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
+{
+	struct f_sdp *sdp = func_to_sdp(f);
+	struct usb_composite_dev *cdev = f->config->cdev;
+	int result;
+
+	debug("%s: intf: %d alt: %d\n", __func__, intf, alt);
+
+	result = usb_ep_enable(sdp->in_ep, &in_desc);
+	if (result)
+		return result;
+	sdp->in_req = sdp_start_ep(sdp->in_ep);
+	sdp->in_req->context = sdp;
+
+	sdp->in_ep->driver_data = cdev; /* claim */
+
+	sdp->altsetting = alt;
+	sdp->state = SDP_STATE_IDLE;
+
+	return 0;
+}
+
+static int sdp_get_alt(struct usb_function *f, unsigned intf)
+{
+	struct f_sdp *sdp = func_to_sdp(f);
+
+	return sdp->altsetting;
+}
+
+static void sdp_disable(struct usb_function *f)
+{
+	struct f_sdp *sdp = func_to_sdp(f);
+
+	usb_ep_disable(sdp->in_ep);
+
+	if (sdp->in_req) {
+		free(sdp->in_req);
+		sdp->in_req = NULL;
+	}
+}
+
+static int sdp_bind_config(struct usb_configuration *c)
+{
+	int status;
+
+	if (!sdp_func) {
+		sdp_func = memalign(CONFIG_SYS_CACHELINE_SIZE, sizeof(*sdp_func));
+		if (!sdp_func)
+			return -ENOMEM;
+	}
+
+	memset(sdp_func, 0, sizeof(*sdp_func));
+
+	sdp_func->usb_function.name = "sdp";
+	sdp_func->usb_function.hs_descriptors = sdp_runtime_descs;
+	sdp_func->usb_function.descriptors = sdp_runtime_descs;
+	sdp_func->usb_function.bind = sdp_bind;
+	sdp_func->usb_function.unbind = sdp_unbind;
+	sdp_func->usb_function.set_alt = sdp_set_alt;
+	sdp_func->usb_function.get_alt = sdp_get_alt;
+	sdp_func->usb_function.disable = sdp_disable;
+	sdp_func->usb_function.strings = sdp_generic_strings;
+	sdp_func->usb_function.setup = sdp_setup;
+
+	status = usb_add_function(c, &sdp_func->usb_function);
+
+	return status;
+}
+
+int sdp_init(void)
+{
+	printf("SDP: initialize...\n");
+	while (!sdp_func->configuration_done) {
+		if (ctrlc()) {
+			puts("\rCTRL+C - Operation aborted.\n");
+			return 0;
+		}
+		usb_gadget_handle_interrupts(0);
+	}
+
+	return 0;
+}
+
+static u32 sdp_jump_imxheader(void *address)
+{
+	flash_header_v2_t *headerv2 = address;
+	ulong (*entry)(void);
+
+	if (headerv2->header.tag != IVT_HEADER_TAG) {
+		printf("Header Tag is not a IMX image\n");
+		return SDP_ERROR_IMXHEADER;
+	}
+
+	printf("Jumping to 0x%08x\n", headerv2->entry);
+	entry = (void *)headerv2->entry;
+	entry();
+
+	/* The image probably never returns hence we wont reach that point */
+	return 0;
+}
+
+static void sdp_handle_in_ep(void)
+{
+	u8 *data = sdp_func->in_req->buf;
+	u32 status;
+	int datalen;

drivers/usb/gadget/f_sdp.c line 640

Quoted 10 lines
drivers/usb/gadget/f_sdp.c new file +10 -0
+
+	switch (sdp_func->state) {
+	case SDP_STATE_TX_SEC_CONF:
+		debug("Report 3: HAB security\n");
+		data[0] = 3;
+
+		data[1] = 0x56;
+		data[2] = 0x78;
+		data[3] = 0x78;
+		data[4] = 0x56;

I am quite lost here - can you explain what are these magic numbers, and maybe add a comment for it (or self explaining defines) ?

Quoted 96 lines
drivers/usb/gadget/f_sdp.c new file +83 -0
+
+		sdp_func->in_req->length = 5;
+		usb_ep_queue(sdp_func->in_ep, sdp_func->in_req, 0);
+		sdp_func->state = SDP_STATE_TX_SEC_CONF_BUSY;
+		break;
+
+	case SDP_STATE_TX_STATUS:
+		debug("Report 4: Status\n");
+		data[0] = 4;
+
+		memcpy(&data[1], &sdp_func->error_status, 4);
+		sdp_func->in_req->length = 65;
+		usb_ep_queue(sdp_func->in_ep, sdp_func->in_req, 0);
+		sdp_func->state = SDP_STATE_TX_STATUS_BUSY;
+		break;
+	case SDP_STATE_TX_REGISTER:
+		debug("Report 4: Register Values\n");
+		data[0] = 4;
+
+		datalen = sdp_func->dnl_bytes_remaining;
+
+		if (datalen > 64)
+			datalen = 64;
+
+		memcpy(&data[1], (void *)sdp_func->dnl_address, datalen);
+		sdp_func->in_req->length = 65;
+
+		sdp_func->dnl_bytes_remaining -= datalen;
+		sdp_func->dnl_address += datalen;
+
+		usb_ep_queue(sdp_func->in_ep, sdp_func->in_req, 0);
+		sdp_func->state = SDP_STATE_TX_REGISTER_BUSY;
+		break;
+	case SDP_STATE_JUMP:
+		printf("Checking imxheader at 0x%08x\n", f_sdp->jmp_address);
+		status = sdp_jump_imxheader((void *)f_sdp->jmp_address);
+
+		sdp_func->next_state = SDP_STATE_IDLE;
+		sdp_func->error_status = status;
+
+		/* Only send Report 4 if there was an error */
+		if (status)
+			sdp_func->state = SDP_STATE_TX_STATUS;
+		else
+			sdp_func->state = SDP_STATE_IDLE;
+		break;
+	default:
+		break;
+	};
+}
+
+int sdp_handle(void)
+{
+	printf("SDP: handle requests...\n");
+	while (1) {
+		if (ctrlc()) {
+			puts("\rCTRL+C - Operation aborted.\n");
+			return 0;
+		}
+
+		usb_gadget_handle_interrupts(0);
+
+		sdp_handle_in_ep();
+	}
+}
+
+int sdp_add(struct usb_configuration *c)
+{
+	int id;
+
+	id = usb_string_id(c->cdev);
+	if (id < 0)
+		return id;
+	strings_sdp_generic[0].id = id;
+	sdp_intf_runtime.iInterface = id;
+
+	debug("%s: cdev: 0x%p gadget:0x%p gadget->ep0: 0x%p\n", __func__,
+	      c->cdev, c->cdev->gadget, c->cdev->gadget->ep0);
+
+	return sdp_bind_config(c);
+}
+
+DECLARE_GADGET_BIND_CALLBACK(usb_dnl_sdp, sdp_add);
include/sdp.h new file +6 -0
new file mode 100644
index 0000000000..03c4a23434
@@ -0,0 +1,16 @@
+/*
+ * sdp.h - Serial Download Protocol
+ *
+ * Copyright (C) 2016 Toradex
+ * Author: Stefan Agner <[email protected]>
+ *

include/sdp.h line 16

Quoted 10 lines
include/sdp.h new file +10 -0
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __SDP_H_
+#define __SDP_H_
+
+int sdp_init(void);
+int sdp_handle(void);
+
+#endif /* __SDP_H_ */

Best regards,
Stefano

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#7 lore raw

PATCH 2/7 usb: gadget: add SDP driver

Quoted 112 lines · Stefano Babic

Hi Stefan,

Quoted 109 lines · Stefan Agner

From: Stefan Agner <[email protected]>

Add SDP (Serial Downloader Protocol) implementation for U-Boot. The protocol is used in NXP SoC's boot ROM and allows to download program images. Beside that, it can also be used to read/write registers and download complete Device Configuration Data (DCD) sets. This basic implementation supports downloading images with the imx header format and reading registers.

---

 drivers/usb/gadget/Kconfig  |   7 +
 drivers/usb/gadget/Makefile |   1 +
 drivers/usb/gadget/f_sdp.c  | 723 ++++++++++++++++++++++++++++++++++++++++++++
 include/sdp.h               |  16 +
 4 files changed, 747 insertions(+)
 create mode 100644 drivers/usb/gadget/f_sdp.c
 create mode 100644 include/sdp.h
drivers/usb/gadget/Kconfig +7 -0
index 261ed128ac..225b66bc95 100644
@@ -103,6 +103,13 @@ config USB_GADGET_DOWNLOAD
 
 if USB_GADGET_DOWNLOAD
 
+config USB_FUNCTION_SDP
+	bool "Enable USB SDP (Serial Download Protocol)"
+	help
+	  Enable Serial Download Protocol (SDP) device support in U-Boot. This
+	  allows to download images into memory and execute (jump to) them
+	  using the same protocol as implemented by the i.MX family's boot ROM.
+
 config G_DNL_MANUFACTURER
 	string "Vendor name of USB device"
 
drivers/usb/gadget/Makefile +1 -0
index 5e316a7cff..6a007d1bcb 100644
@@ -28,6 +28,7 @@ obj-$(CONFIG_USB_FUNCTION_THOR) += f_thor.o
 obj-$(CONFIG_USB_FUNCTION_DFU) += f_dfu.o
 obj-$(CONFIG_USB_FUNCTION_MASS_STORAGE) += f_mass_storage.o
 obj-$(CONFIG_USB_FUNCTION_FASTBOOT) += f_fastboot.o
+obj-$(CONFIG_USB_FUNCTION_SDP) += f_sdp.o
 endif
 endif
 ifdef CONFIG_USB_ETHER
drivers/usb/gadget/f_sdp.c new file +53 -0
new file mode 100644
index 0000000000..eb89695aaf
@@ -0,0 +1,723 @@
+/*
+ * f_sdp.c -- USB HID Serial Download Protocol
+ *
+ * Copyright (C) 2016 Toradex
+ * Author: Stefan Agner <[email protected]>
+ *
+ * This file implements the Serial Download Protocol (SDP) as specified in
+ * the i.MX 6 Reference Manual. The SDP is a USB HID based protocol and
+ * allows to download images directly to memory. The implementation
+ * works with the imx_loader (imx_usb) USB client software on host side.
+ *
+ * Not all commands are implemented, e.g. WRITE_REGISTER, DCD_WRITE and
+ * SKIP_DCD_HEADER are only stubs.
+ *
+ * Parts of the implementation are based on f_dfu and f_thor.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <errno.h>
+#include <common.h>
+#include <console.h>
+#include <malloc.h>
+
+#include <linux/usb/ch9.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/composite.h>
+
+#include <asm/io.h>
+#include <g_dnl.h>
+#include <sdp.h>
+#include <imximage.h>
+
+#define HID_REPORT_ID_MASK	0x000000ff
+
+/*
+ * HID class requests
+ */
+#define HID_REQ_GET_REPORT		0x01
+#define HID_REQ_GET_IDLE		0x02
+#define HID_REQ_GET_PROTOCOL		0x03
+#define HID_REQ_SET_REPORT		0x09
+#define HID_REQ_SET_IDLE		0x0A
+#define HID_REQ_SET_PROTOCOL		0x0B
+
+#define HID_USAGE_PAGE_LEN		76
+
+struct hid_report {
+	u8 usage_page[HID_USAGE_PAGE_LEN];
+} __packed;
+
+#define SDP_READ_REGISTER	0x0101
+#define SDP_WRITE_REGISTER	0x0202
Quoted 9 lines

drivers/usb/gadget/f_sdp.c line 58

Quoted 5 lines
drivers/usb/gadget/f_sdp.c new file +5 -0
+#define SDP_WRITE_FILE		0x0404
+#define SDP_ERROR_STATUS	0x0505
+#define SDP_DCD_WRITE		0x0a0a
+#define SDP_JUMP_ADDRESS	0x0b0b
+#define SDP_SKIP_DCD_HEADER	0x0c0c

It looks like that I am again out of sync with documentation. Where is defined SDP_SKIP_DCD_HEADER ? It is undefined for MX6Q/D, Solo and DL.

This is only available in newer SoC's e.g. i.MX 7.

It allows to skip the DCD header in a downloaded image. Since the DCD header is anyway ignored by this SDP implementation, the command is kind of useless. I still think it is a good idea to have the command type define for completeness... And I think also some SDP host side implementation might issue the command...

Quoted 226 lines
Quoted 226 lines
drivers/usb/gadget/f_sdp.c new file +226 -0
+
+#define SDP_WRITE_FILE_COMPLETE		0x88888888
+#define SDP_WRITE_REGISTER_COMPLETE	0x128A8A12
+#define SDP_SKIP_DCD_HEADER_COMPLETE	0x900DD009
+#define SDP_ERROR_IMXHEADER		0x000a0533
+
+#define SDP_COMMAND_LEN		16
+
+struct sdp_command {
+	u16 cmd;
+	u32 addr;
+	u8 format;
+	u32 cnt;
+	u32 data;
+	u8 rsvd;
+} __packed;
+
+enum sdp_state {
+	SDP_STATE_IDLE,
+	SDP_STATE_RX_DCD_DATA,
+	SDP_STATE_RX_FILE_DATA,
+	SDP_STATE_TX_SEC_CONF,
+	SDP_STATE_TX_SEC_CONF_BUSY,
+	SDP_STATE_TX_REGISTER,
+	SDP_STATE_TX_REGISTER_BUSY,
+	SDP_STATE_TX_STATUS,
+	SDP_STATE_TX_STATUS_BUSY,
+	SDP_STATE_JUMP,
+};
+
+struct f_sdp {
+	struct usb_function		usb_function;
+
+	struct usb_descriptor_header	**function;
+
+	u8				altsetting;
+	enum sdp_state			state;
+	enum sdp_state			next_state;
+	u32				dnl_address;
+	u32				dnl_bytes_remaining;
+	u32				jmp_address;
+	bool				always_send_status;
+	u32				error_status;
+
+	/* EP0 request */
+	struct usb_request		*req;
+
+	/* EP1 IN */
+	struct usb_ep			*in_ep;
+	struct usb_request		*in_req;
+
+	bool				configuration_done;
+};
+
+static struct f_sdp *sdp_func;
+
+static inline struct f_sdp *func_to_sdp(struct usb_function *f)
+{
+	return container_of(f, struct f_sdp, usb_function);
+}
+
+static struct usb_interface_descriptor sdp_intf_runtime = {
+	.bLength =		sizeof(sdp_intf_runtime),
+	.bDescriptorType =	USB_DT_INTERFACE,
+	.bAlternateSetting =	0,
+	.bNumEndpoints =	1,
+	.bInterfaceClass =	USB_CLASS_HID,
+	.bInterfaceSubClass =	0,
+	.bInterfaceProtocol =	0,
+	/* .iInterface = DYNAMIC */
+};
+
+/* HID configuration */
+static struct usb_class_hid_descriptor sdp_hid_desc = {
+	.bLength =		sizeof(sdp_hid_desc),
+	.bDescriptorType =	USB_DT_CS_DEVICE,
+
+	.bcdCDC =		__constant_cpu_to_le16(0x0110),
+	.bCountryCode =		0,
+	.bNumDescriptors =	1,
+
+	.bDescriptorType0	= USB_DT_HID_REPORT,
+	.wDescriptorLength0	= HID_USAGE_PAGE_LEN,
+};
+
+static struct usb_endpoint_descriptor in_desc = {
+	.bLength =		USB_DT_ENDPOINT_SIZE,
+	.bDescriptorType =	USB_DT_ENDPOINT, /*USB_DT_CS_ENDPOINT*/
+
+	.bEndpointAddress =	1 | USB_DIR_IN,
+	.bmAttributes =	USB_ENDPOINT_XFER_INT,
+	.wMaxPacketSize =	64,
+	.bInterval =		1,
+};
+
+static struct usb_descriptor_header *sdp_runtime_descs[] = {
+	(struct usb_descriptor_header *)&sdp_intf_runtime,
+	(struct usb_descriptor_header *)&sdp_hid_desc,
+	(struct usb_descriptor_header *)&in_desc,
+	NULL,
+};
+
+/* This is synchronized with what the SoC implementation reports */
+static struct hid_report sdp_hid_report = {
+	.usage_page = {
+		0x06, 0x00, 0xff, /* Usage Page */
+		0x09, 0x01, /* Usage (Poiter?) */
+		0xa1, 0x01, /* Collection */
+
+		0x85, 0x01, /* Report ID */
+		0x19, 0x01, /* Usage Minimum */
+		0x29, 0x01, /* Usage Maximum */
+		0x15, 0x00, /* Local Minimum */
+		0x26, 0xFF, 0x00, /* Local Maximum? */
+		0x75, 0x08, /* Report Size */
+		0x95, 0x10, /* Report Count */
+		0x91, 0x02, /* Output Data */
+
+		0x85, 0x02, /* Report ID */
+		0x19, 0x01, /* Usage Minimum */
+		0x29, 0x01, /* Usage Maximum */
+		0x15, 0x00, /* Local Minimum */
+		0x26, 0xFF, 0x00, /* Local Maximum? */
+		0x75, 0x80, /* Report Size 128 */
+		0x95, 0x40, /* Report Count */
+		0x91, 0x02, /* Output Data */
+
+		0x85, 0x03, /* Report ID */
+		0x19, 0x01, /* Usage Minimum */
+		0x29, 0x01, /* Usage Maximum */
+		0x15, 0x00, /* Local Minimum */
+		0x26, 0xFF, 0x00, /* Local Maximum? */
+		0x75, 0x08, /* Report Size 8 */
+		0x95, 0x04, /* Report Count */
+		0x81, 0x02, /* Input Data */
+
+		0x85, 0x04, /* Report ID */
+		0x19, 0x01, /* Usage Minimum */
+		0x29, 0x01, /* Usage Maximum */
+		0x15, 0x00, /* Local Minimum */
+		0x26, 0xFF, 0x00, /* Local Maximum? */
+		0x75, 0x08, /* Report Size 8 */
+		0x95, 0x40, /* Report Count */
+		0x81, 0x02, /* Input Data */
+		0xc0
+	},
+};
+
+static const char sdp_name[] = "Serial Downloader Protocol";
+
+/*
+ * static strings, in UTF-8
+ */
+static struct usb_string strings_sdp_generic[] = {
+	[0].s = sdp_name,
+	{  }			/* end of list */
+};
+
+static struct usb_gadget_strings stringtab_sdp_generic = {
+	.language	= 0x0409,	/* en-us */
+	.strings	= strings_sdp_generic,
+};
+
+static struct usb_gadget_strings *sdp_generic_strings[] = {
+	&stringtab_sdp_generic,
+	NULL,
+};
+
+static void sdp_rx_command_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_sdp *sdp = req->context;
+	int status = req->status;
+	u8 *data = req->buf;
+	u8 report = data[0];
+
+	if (status != 0) {
+		error("Status: %d", status);
+		return;
+	}
+
+	if (report != 1) {
+		error("Unexpected report %d", report);
+		return;
+	}
+
+	struct sdp_command *cmd = req->buf + 1;
+
+	debug("%s: command: %04x, addr: %08x, cnt: %u\n",
+	      __func__, be16_to_cpu(cmd->cmd),
+	      be32_to_cpu(cmd->addr), be32_to_cpu(cmd->cnt));
+
+	switch (be16_to_cpu(cmd->cmd)) {
+	case SDP_READ_REGISTER:
+		sdp->always_send_status = false;
+		sdp->error_status = 0x0;
+
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->dnl_address = be32_to_cpu(cmd->addr);
+		sdp->dnl_bytes_remaining = be32_to_cpu(cmd->cnt);
+		sdp->next_state = SDP_STATE_TX_REGISTER;
+		printf("Reading %d registers at 0x%08x... ",
+		       sdp->dnl_bytes_remaining, sdp->dnl_address);
+		break;
+	case SDP_WRITE_FILE:
+		sdp->always_send_status = true;
+		sdp->error_status = SDP_WRITE_FILE_COMPLETE;
+
+		sdp->state = SDP_STATE_RX_FILE_DATA;
+		sdp->dnl_address = be32_to_cpu(cmd->addr);
+		sdp->dnl_bytes_remaining = be32_to_cpu(cmd->cnt);
+		sdp->next_state = SDP_STATE_IDLE;
+
+		printf("Downloading file of size %d to 0x%08x... ",
+		       sdp->dnl_bytes_remaining, sdp->dnl_address);
+
+		break;
+	case SDP_ERROR_STATUS:
+		sdp->always_send_status = true;
+		sdp->error_status = 0;
+
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->next_state = SDP_STATE_IDLE;
+		break;
+	case SDP_DCD_WRITE:
+		sdp->always_send_status = true;
+		sdp->error_status = SDP_WRITE_REGISTER_COMPLETE;
Quoted 9 lines

drivers/usb/gadget/f_sdp.c line 289

Quoted 5 lines
drivers/usb/gadget/f_sdp.c new file +5 -0
+
+		sdp->state = SDP_STATE_RX_DCD_DATA;
+		sdp->dnl_bytes_remaining = be32_to_cpu(cmd->cnt);
+		sdp->next_state = SDP_STATE_IDLE;
+		break;

It is fine, but I am just missing if this is a use case. DCD is interpreted by boot ROM, and we are here already over in SPL.

I also don't have a use case currently, but it is rather cheap so why don't?

Note that the SDP_READ_REGISTER command is actually used by the sb_loader to do some verification whether the image got correctly downloaded... But I don't think it required DCD_WRITE for something, but I would have to retest.

Quoted 349 lines

drivers/usb/gadget/f_sdp.c line 302

Quoted 13 lines
drivers/usb/gadget/f_sdp.c new file +13 -0
+	case SDP_JUMP_ADDRESS:
+		sdp->always_send_status = false;
+		sdp->error_status = 0;
+
+		sdp->jmp_address = be32_to_cpu(cmd->addr);
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->next_state = SDP_STATE_JUMP;
+		break;
+	case SDP_SKIP_DCD_HEADER:
+		sdp->always_send_status = true;
+		sdp->error_status = SDP_SKIP_DCD_HEADER_COMPLETE;
+
+		/* Ignore command, DCD not supported anyway */

Right - we load a file, we do not need a DCD.

Quoted 333 lines
drivers/usb/gadget/f_sdp.c new file +333 -0
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->next_state = SDP_STATE_IDLE;
+		break;
+	default:
+		error("Unknown command: %08x\n", be16_to_cpu(cmd->cmd));
+	}
+}
+
+static void sdp_rx_data_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_sdp *sdp = req->context;
+	int status = req->status;
+	u8 *data = req->buf;
+	u8 report = data[0];
+	int datalen = req->length - 1;
+
+	if (status != 0) {
+		error("Status: %d", status);
+		return;
+	}
+
+	if (report != 2) {
+		error("Unexpected report %d", report);
+		return;
+	}
+
+	if (sdp->dnl_bytes_remaining < datalen) {
+		/*
+		 * Some USB stacks require to send a complete buffer as
+		 * specified in the HID descriptor. This leads to longer
+		 * transfers than the file length, no problem for us.
+		 */
+		sdp->dnl_bytes_remaining = 0;
+	} else {
+		sdp->dnl_bytes_remaining -= datalen;
+	}
+
+	if (sdp->state == SDP_STATE_RX_FILE_DATA) {
+		memcpy((void *)sdp->dnl_address, req->buf + 1, datalen);
+		sdp->dnl_address += datalen;
+	}
+
+	if (sdp->dnl_bytes_remaining)
+		return;
+
+	printf("done\n");
+
+	switch (sdp->state) {
+	case SDP_STATE_RX_FILE_DATA:
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		break;
+	case SDP_STATE_RX_DCD_DATA:
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		break;
+	default:
+		error("Invalid state: %d", sdp->state);
+	}
+}
+
+
+
+static void sdp_tx_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_sdp *sdp = req->context;
+	int status = req->status;
+
+	if (status != 0) {
+		error("Status: %d", status);
+		return;
+	}
+
+	switch (sdp->state) {
+	case SDP_STATE_TX_SEC_CONF_BUSY:
+		/* Not all commands require status report */
+		if (sdp->always_send_status || sdp->error_status)
+			sdp->state = SDP_STATE_TX_STATUS;
+		else
+			sdp->state = sdp->next_state;
+
+		break;
+	case SDP_STATE_TX_STATUS_BUSY:
+		sdp->state = sdp->next_state;
+		break;
+	case SDP_STATE_TX_REGISTER_BUSY:
+		if (sdp->dnl_bytes_remaining)
+			sdp->state = SDP_STATE_TX_REGISTER;
+		else
+			sdp->state = SDP_STATE_IDLE;
+		break;
+	default:
+		error("Wrong State: %d", sdp->state);
+		sdp->state = SDP_STATE_IDLE;
+		break;
+	}
+	debug("%s complete --> %d, %d/%d\n", ep->name,
+	      status, req->actual, req->length);
+}
+
+static int sdp_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
+{
+	struct usb_gadget *gadget = f->config->cdev->gadget;
+	struct usb_request *req = f->config->cdev->req;
+	struct f_sdp *sdp = f->config->cdev->req->context;
+	u16 len = le16_to_cpu(ctrl->wLength);
+	u16 w_value = le16_to_cpu(ctrl->wValue);
+	int value = 0;
+	u8 req_type = ctrl->bRequestType & USB_TYPE_MASK;
+
+	debug("w_value: 0x%x len: 0x%x\n", w_value, len);
+	debug("req_type: 0x%x ctrl->bRequest: 0x%x sdp->state: %d\n",
+	      req_type, ctrl->bRequest, sdp->state);
+
+	if (req_type == USB_TYPE_STANDARD) {
+		if (ctrl->bRequest == USB_REQ_GET_DESCRIPTOR) {
+			/* Send HID report descriptor */
+			value = min(len, (u16) sizeof(sdp_hid_report));
+			memcpy(req->buf, &sdp_hid_report, value);
+			sdp->configuration_done = true;
+		}
+	}
+
+	if (req_type == USB_TYPE_CLASS) {
+		int report = w_value & HID_REPORT_ID_MASK;
+
+		/* HID (SDP) request */
+		switch (ctrl->bRequest) {
+		case HID_REQ_SET_REPORT:
+			switch (report) {
+			case 1:
+				value = SDP_COMMAND_LEN + 1;
+				req->complete = sdp_rx_command_complete;
+				break;
+			case 2:
+				value = len;
+				req->complete = sdp_rx_data_complete;
+				break;
+			}
+		}
+	}
+
+	if (value >= 0) {
+		req->length = value;
+		req->zero = value < len;
+		value = usb_ep_queue(gadget->ep0, req, 0);
+		if (value < 0) {
+			debug("ep_queue --> %d\n", value);
+			req->status = 0;
+		}
+	}
+
+	return value;
+}
+
+static int sdp_bind(struct usb_configuration *c, struct usb_function *f)
+{
+	struct usb_gadget *gadget = c->cdev->gadget;
+	struct usb_composite_dev *cdev = c->cdev;
+	struct f_sdp *sdp = func_to_sdp(f);
+	int rv = 0, id;
+
+	id = usb_interface_id(c, f);
+	if (id < 0)
+		return id;
+	sdp_intf_runtime.bInterfaceNumber = id;
+
+	struct usb_ep *ep;
+
+	/* allocate instance-specific endpoints */
+	ep = usb_ep_autoconfig(gadget, &in_desc);
+	if (!ep) {
+		rv = -ENODEV;
+		goto error;
+	}
+
+	sdp->in_ep = ep; /* Store IN EP for enabling @ setup */
+
+	cdev->req->context = sdp;
+
+error:
+	return rv;
+}
+
+static void sdp_unbind(struct usb_configuration *c, struct usb_function *f)
+{
+	free(sdp_func);
+	sdp_func = NULL;
+}
+
+static struct usb_request *alloc_ep_req(struct usb_ep *ep, unsigned length)
+{
+	struct usb_request *req;
+
+	req = usb_ep_alloc_request(ep, 0);
+	if (!req)
+		return req;
+
+	req->length = length;
+	req->buf = memalign(CONFIG_SYS_CACHELINE_SIZE, length);
+	if (!req->buf) {
+		usb_ep_free_request(ep, req);
+		req = NULL;
+	}
+
+	return req;
+}
+
+
+static struct usb_request *sdp_start_ep(struct usb_ep *ep)
+{
+	struct usb_request *req;
+
+	req = alloc_ep_req(ep, 64);
+	debug("%s: ep:%p req:%p\n", __func__, ep, req);
+
+	if (!req)
+		return NULL;
+
+	memset(req->buf, 0, req->length);
+	req->complete = sdp_tx_complete;
+
+	return req;
+}
+static int sdp_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
+{
+	struct f_sdp *sdp = func_to_sdp(f);
+	struct usb_composite_dev *cdev = f->config->cdev;
+	int result;
+
+	debug("%s: intf: %d alt: %d\n", __func__, intf, alt);
+
+	result = usb_ep_enable(sdp->in_ep, &in_desc);
+	if (result)
+		return result;
+	sdp->in_req = sdp_start_ep(sdp->in_ep);
+	sdp->in_req->context = sdp;
+
+	sdp->in_ep->driver_data = cdev; /* claim */
+
+	sdp->altsetting = alt;
+	sdp->state = SDP_STATE_IDLE;
+
+	return 0;
+}
+
+static int sdp_get_alt(struct usb_function *f, unsigned intf)
+{
+	struct f_sdp *sdp = func_to_sdp(f);
+
+	return sdp->altsetting;
+}
+
+static void sdp_disable(struct usb_function *f)
+{
+	struct f_sdp *sdp = func_to_sdp(f);
+
+	usb_ep_disable(sdp->in_ep);
+
+	if (sdp->in_req) {
+		free(sdp->in_req);
+		sdp->in_req = NULL;
+	}
+}
+
+static int sdp_bind_config(struct usb_configuration *c)
+{
+	int status;
+
+	if (!sdp_func) {
+		sdp_func = memalign(CONFIG_SYS_CACHELINE_SIZE, sizeof(*sdp_func));
+		if (!sdp_func)
+			return -ENOMEM;
+	}
+
+	memset(sdp_func, 0, sizeof(*sdp_func));
+
+	sdp_func->usb_function.name = "sdp";
+	sdp_func->usb_function.hs_descriptors = sdp_runtime_descs;
+	sdp_func->usb_function.descriptors = sdp_runtime_descs;
+	sdp_func->usb_function.bind = sdp_bind;
+	sdp_func->usb_function.unbind = sdp_unbind;
+	sdp_func->usb_function.set_alt = sdp_set_alt;
+	sdp_func->usb_function.get_alt = sdp_get_alt;
+	sdp_func->usb_function.disable = sdp_disable;
+	sdp_func->usb_function.strings = sdp_generic_strings;
+	sdp_func->usb_function.setup = sdp_setup;
+
+	status = usb_add_function(c, &sdp_func->usb_function);
+
+	return status;
+}
+
+int sdp_init(void)
+{
+	printf("SDP: initialize...\n");
+	while (!sdp_func->configuration_done) {
+		if (ctrlc()) {
+			puts("\rCTRL+C - Operation aborted.\n");
+			return 0;
+		}
+		usb_gadget_handle_interrupts(0);
+	}
+
+	return 0;
+}
+
+static u32 sdp_jump_imxheader(void *address)
+{
+	flash_header_v2_t *headerv2 = address;
+	ulong (*entry)(void);
+
+	if (headerv2->header.tag != IVT_HEADER_TAG) {
+		printf("Header Tag is not a IMX image\n");
+		return SDP_ERROR_IMXHEADER;
+	}
+
+	printf("Jumping to 0x%08x\n", headerv2->entry);
+	entry = (void *)headerv2->entry;
+	entry();
+
+	/* The image probably never returns hence we wont reach that point */
+	return 0;
+}
+
+static void sdp_handle_in_ep(void)
+{
+	u8 *data = sdp_func->in_req->buf;
+	u32 status;
+	int datalen;
+
+	switch (sdp_func->state) {
+	case SDP_STATE_TX_SEC_CONF:
+		debug("Report 3: HAB security\n");
+		data[0] = 3;
Quoted 9 lines

drivers/usb/gadget/f_sdp.c line 640

Quoted 5 lines
drivers/usb/gadget/f_sdp.c new file +5 -0
+
+		data[1] = 0x56;
+		data[2] = 0x78;
+		data[3] = 0x78;
+		data[4] = 0x56;

I am quite lost here - can you explain what are these magic numbers, and maybe add a comment for it (or self explaining defines) ?

Yeah protocol specific magic number:
HAB security
configuration. Device
sends 0x12343412 in
closed mode and
0x56787856 in open
mode.

We always assume open. Not sure what kind of implication that can have, I think imx_usb basically just prints out what the device says.

Will create proper defines.

--
Stefan

Quoted 109 lines
Quoted 96 lines
drivers/usb/gadget/f_sdp.c new file +83 -0
+
+		sdp_func->in_req->length = 5;
+		usb_ep_queue(sdp_func->in_ep, sdp_func->in_req, 0);
+		sdp_func->state = SDP_STATE_TX_SEC_CONF_BUSY;
+		break;
+
+	case SDP_STATE_TX_STATUS:
+		debug("Report 4: Status\n");
+		data[0] = 4;
+
+		memcpy(&data[1], &sdp_func->error_status, 4);
+		sdp_func->in_req->length = 65;
+		usb_ep_queue(sdp_func->in_ep, sdp_func->in_req, 0);
+		sdp_func->state = SDP_STATE_TX_STATUS_BUSY;
+		break;
+	case SDP_STATE_TX_REGISTER:
+		debug("Report 4: Register Values\n");
+		data[0] = 4;
+
+		datalen = sdp_func->dnl_bytes_remaining;
+
+		if (datalen > 64)
+			datalen = 64;
+
+		memcpy(&data[1], (void *)sdp_func->dnl_address, datalen);
+		sdp_func->in_req->length = 65;
+
+		sdp_func->dnl_bytes_remaining -= datalen;
+		sdp_func->dnl_address += datalen;
+
+		usb_ep_queue(sdp_func->in_ep, sdp_func->in_req, 0);
+		sdp_func->state = SDP_STATE_TX_REGISTER_BUSY;
+		break;
+	case SDP_STATE_JUMP:
+		printf("Checking imxheader at 0x%08x\n", f_sdp->jmp_address);
+		status = sdp_jump_imxheader((void *)f_sdp->jmp_address);
+
+		sdp_func->next_state = SDP_STATE_IDLE;
+		sdp_func->error_status = status;
+
+		/* Only send Report 4 if there was an error */
+		if (status)
+			sdp_func->state = SDP_STATE_TX_STATUS;
+		else
+			sdp_func->state = SDP_STATE_IDLE;
+		break;
+	default:
+		break;
+	};
+}
+
+int sdp_handle(void)
+{
+	printf("SDP: handle requests...\n");
+	while (1) {
+		if (ctrlc()) {
+			puts("\rCTRL+C - Operation aborted.\n");
+			return 0;
+		}
+
+		usb_gadget_handle_interrupts(0);
+
+		sdp_handle_in_ep();
+	}
+}
+
+int sdp_add(struct usb_configuration *c)
+{
+	int id;
+
+	id = usb_string_id(c->cdev);
+	if (id < 0)
+		return id;
+	strings_sdp_generic[0].id = id;
+	sdp_intf_runtime.iInterface = id;
+
+	debug("%s: cdev: 0x%p gadget:0x%p gadget->ep0: 0x%p\n", __func__,
+	      c->cdev, c->cdev->gadget, c->cdev->gadget->ep0);
+
+	return sdp_bind_config(c);
+}
+
+DECLARE_GADGET_BIND_CALLBACK(usb_dnl_sdp, sdp_add);
include/sdp.h new file +6 -0
new file mode 100644
index 0000000000..03c4a23434
@@ -0,0 +1,16 @@
+/*
+ * sdp.h - Serial Download Protocol
+ *
+ * Copyright (C) 2016 Toradex
+ * Author: Stefan Agner <[email protected]>
+ *

include/sdp.h line 16

Quoted 10 lines
include/sdp.h new file +10 -0
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __SDP_H_
+#define __SDP_H_
+
+int sdp_init(void);
+int sdp_handle(void);
+
+#endif /* __SDP_H_ */

Best regards,
Stefano

#8 lore raw

PATCH 2/7 usb: gadget: add SDP driver

Hi Stefan,

Quoted 6 lines · Stefan Agner
Quoted 4 lines

It looks like that I am again out of sync with documentation. Where is defined SDP_SKIP_DCD_HEADER ? It is undefined for MX6Q/D, Solo and DL.

This is only available in newer SoC's e.g. i.MX 7.

ok

Quoted 5 lines

It allows to skip the DCD header in a downloaded image. Since the DCD header is anyway ignored by this SDP implementation, the command is kind of useless. I still think it is a good idea to have the command type define for completeness...

I agree with you - I just wanted to understand.

Quoted 2 lines

And I think also some SDP host side
implementation might issue the command...

That's ok, thanks for clarification.

Quoted 7 lines
Quoted 4 lines

It is fine, but I am just missing if this is a use case. DCD is interpreted by boot ROM, and we are here already over in SPL.

I also don't have a use case currently, but it is rather cheap so why don't?

Yes, that's fine.

Quoted 374 lines

Note that the SDP_READ_REGISTER command is actually used by the sb_loader to do some verification whether the image got correctly downloaded... But I don't think it required DCD_WRITE for something, but I would have to retest.

Quoted 349 lines

drivers/usb/gadget/f_sdp.c line 302

Quoted 13 lines
drivers/usb/gadget/f_sdp.c new file +13 -0
+	case SDP_JUMP_ADDRESS:
+		sdp->always_send_status = false;
+		sdp->error_status = 0;
+
+		sdp->jmp_address = be32_to_cpu(cmd->addr);
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->next_state = SDP_STATE_JUMP;
+		break;
+	case SDP_SKIP_DCD_HEADER:
+		sdp->always_send_status = true;
+		sdp->error_status = SDP_SKIP_DCD_HEADER_COMPLETE;
+
+		/* Ignore command, DCD not supported anyway */

Right - we load a file, we do not need a DCD.

Quoted 333 lines
drivers/usb/gadget/f_sdp.c new file +333 -0
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		sdp->next_state = SDP_STATE_IDLE;
+		break;
+	default:
+		error("Unknown command: %08x\n", be16_to_cpu(cmd->cmd));
+	}
+}
+
+static void sdp_rx_data_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_sdp *sdp = req->context;
+	int status = req->status;
+	u8 *data = req->buf;
+	u8 report = data[0];
+	int datalen = req->length - 1;
+
+	if (status != 0) {
+		error("Status: %d", status);
+		return;
+	}
+
+	if (report != 2) {
+		error("Unexpected report %d", report);
+		return;
+	}
+
+	if (sdp->dnl_bytes_remaining < datalen) {
+		/*
+		 * Some USB stacks require to send a complete buffer as
+		 * specified in the HID descriptor. This leads to longer
+		 * transfers than the file length, no problem for us.
+		 */
+		sdp->dnl_bytes_remaining = 0;
+	} else {
+		sdp->dnl_bytes_remaining -= datalen;
+	}
+
+	if (sdp->state == SDP_STATE_RX_FILE_DATA) {
+		memcpy((void *)sdp->dnl_address, req->buf + 1, datalen);
+		sdp->dnl_address += datalen;
+	}
+
+	if (sdp->dnl_bytes_remaining)
+		return;
+
+	printf("done\n");
+
+	switch (sdp->state) {
+	case SDP_STATE_RX_FILE_DATA:
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		break;
+	case SDP_STATE_RX_DCD_DATA:
+		sdp->state = SDP_STATE_TX_SEC_CONF;
+		break;
+	default:
+		error("Invalid state: %d", sdp->state);
+	}
+}
+
+
+
+static void sdp_tx_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_sdp *sdp = req->context;
+	int status = req->status;
+
+	if (status != 0) {
+		error("Status: %d", status);
+		return;
+	}
+
+	switch (sdp->state) {
+	case SDP_STATE_TX_SEC_CONF_BUSY:
+		/* Not all commands require status report */
+		if (sdp->always_send_status || sdp->error_status)
+			sdp->state = SDP_STATE_TX_STATUS;
+		else
+			sdp->state = sdp->next_state;
+
+		break;
+	case SDP_STATE_TX_STATUS_BUSY:
+		sdp->state = sdp->next_state;
+		break;
+	case SDP_STATE_TX_REGISTER_BUSY:
+		if (sdp->dnl_bytes_remaining)
+			sdp->state = SDP_STATE_TX_REGISTER;
+		else
+			sdp->state = SDP_STATE_IDLE;
+		break;
+	default:
+		error("Wrong State: %d", sdp->state);
+		sdp->state = SDP_STATE_IDLE;
+		break;
+	}
+	debug("%s complete --> %d, %d/%d\n", ep->name,
+	      status, req->actual, req->length);
+}
+
+static int sdp_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
+{
+	struct usb_gadget *gadget = f->config->cdev->gadget;
+	struct usb_request *req = f->config->cdev->req;
+	struct f_sdp *sdp = f->config->cdev->req->context;
+	u16 len = le16_to_cpu(ctrl->wLength);
+	u16 w_value = le16_to_cpu(ctrl->wValue);
+	int value = 0;
+	u8 req_type = ctrl->bRequestType & USB_TYPE_MASK;
+
+	debug("w_value: 0x%x len: 0x%x\n", w_value, len);
+	debug("req_type: 0x%x ctrl->bRequest: 0x%x sdp->state: %d\n",
+	      req_type, ctrl->bRequest, sdp->state);
+
+	if (req_type == USB_TYPE_STANDARD) {
+		if (ctrl->bRequest == USB_REQ_GET_DESCRIPTOR) {
+			/* Send HID report descriptor */
+			value = min(len, (u16) sizeof(sdp_hid_report));
+			memcpy(req->buf, &sdp_hid_report, value);
+			sdp->configuration_done = true;
+		}
+	}
+
+	if (req_type == USB_TYPE_CLASS) {
+		int report = w_value & HID_REPORT_ID_MASK;
+
+		/* HID (SDP) request */
+		switch (ctrl->bRequest) {
+		case HID_REQ_SET_REPORT:
+			switch (report) {
+			case 1:
+				value = SDP_COMMAND_LEN + 1;
+				req->complete = sdp_rx_command_complete;
+				break;
+			case 2:
+				value = len;
+				req->complete = sdp_rx_data_complete;
+				break;
+			}
+		}
+	}
+
+	if (value >= 0) {
+		req->length = value;
+		req->zero = value < len;
+		value = usb_ep_queue(gadget->ep0, req, 0);
+		if (value < 0) {
+			debug("ep_queue --> %d\n", value);
+			req->status = 0;
+		}
+	}
+
+	return value;
+}
+
+static int sdp_bind(struct usb_configuration *c, struct usb_function *f)
+{
+	struct usb_gadget *gadget = c->cdev->gadget;
+	struct usb_composite_dev *cdev = c->cdev;
+	struct f_sdp *sdp = func_to_sdp(f);
+	int rv = 0, id;
+
+	id = usb_interface_id(c, f);
+	if (id < 0)
+		return id;
+	sdp_intf_runtime.bInterfaceNumber = id;
+
+	struct usb_ep *ep;
+
+	/* allocate instance-specific endpoints */
+	ep = usb_ep_autoconfig(gadget, &in_desc);
+	if (!ep) {
+		rv = -ENODEV;
+		goto error;
+	}
+
+	sdp->in_ep = ep; /* Store IN EP for enabling @ setup */
+
+	cdev->req->context = sdp;
+
+error:
+	return rv;
+}
+
+static void sdp_unbind(struct usb_configuration *c, struct usb_function *f)
+{
+	free(sdp_func);
+	sdp_func = NULL;
+}
+
+static struct usb_request *alloc_ep_req(struct usb_ep *ep, unsigned length)
+{
+	struct usb_request *req;
+
+	req = usb_ep_alloc_request(ep, 0);
+	if (!req)
+		return req;
+
+	req->length = length;
+	req->buf = memalign(CONFIG_SYS_CACHELINE_SIZE, length);
+	if (!req->buf) {
+		usb_ep_free_request(ep, req);
+		req = NULL;
+	}
+
+	return req;
+}
+
+
+static struct usb_request *sdp_start_ep(struct usb_ep *ep)
+{
+	struct usb_request *req;
+
+	req = alloc_ep_req(ep, 64);
+	debug("%s: ep:%p req:%p\n", __func__, ep, req);
+
+	if (!req)
+		return NULL;
+
+	memset(req->buf, 0, req->length);
+	req->complete = sdp_tx_complete;
+
+	return req;
+}
+static int sdp_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
+{
+	struct f_sdp *sdp = func_to_sdp(f);
+	struct usb_composite_dev *cdev = f->config->cdev;
+	int result;
+
+	debug("%s: intf: %d alt: %d\n", __func__, intf, alt);
+
+	result = usb_ep_enable(sdp->in_ep, &in_desc);
+	if (result)
+		return result;
+	sdp->in_req = sdp_start_ep(sdp->in_ep);
+	sdp->in_req->context = sdp;
+
+	sdp->in_ep->driver_data = cdev; /* claim */
+
+	sdp->altsetting = alt;
+	sdp->state = SDP_STATE_IDLE;
+
+	return 0;
+}
+
+static int sdp_get_alt(struct usb_function *f, unsigned intf)
+{
+	struct f_sdp *sdp = func_to_sdp(f);
+
+	return sdp->altsetting;
+}
+
+static void sdp_disable(struct usb_function *f)
+{
+	struct f_sdp *sdp = func_to_sdp(f);
+
+	usb_ep_disable(sdp->in_ep);
+
+	if (sdp->in_req) {
+		free(sdp->in_req);
+		sdp->in_req = NULL;
+	}
+}
+
+static int sdp_bind_config(struct usb_configuration *c)
+{
+	int status;
+
+	if (!sdp_func) {
+		sdp_func = memalign(CONFIG_SYS_CACHELINE_SIZE, sizeof(*sdp_func));
+		if (!sdp_func)
+			return -ENOMEM;
+	}
+
+	memset(sdp_func, 0, sizeof(*sdp_func));
+
+	sdp_func->usb_function.name = "sdp";
+	sdp_func->usb_function.hs_descriptors = sdp_runtime_descs;
+	sdp_func->usb_function.descriptors = sdp_runtime_descs;
+	sdp_func->usb_function.bind = sdp_bind;
+	sdp_func->usb_function.unbind = sdp_unbind;
+	sdp_func->usb_function.set_alt = sdp_set_alt;
+	sdp_func->usb_function.get_alt = sdp_get_alt;
+	sdp_func->usb_function.disable = sdp_disable;
+	sdp_func->usb_function.strings = sdp_generic_strings;
+	sdp_func->usb_function.setup = sdp_setup;
+
+	status = usb_add_function(c, &sdp_func->usb_function);
+
+	return status;
+}
+
+int sdp_init(void)
+{
+	printf("SDP: initialize...\n");
+	while (!sdp_func->configuration_done) {
+		if (ctrlc()) {
+			puts("\rCTRL+C - Operation aborted.\n");
+			return 0;
+		}
+		usb_gadget_handle_interrupts(0);
+	}
+
+	return 0;
+}
+
+static u32 sdp_jump_imxheader(void *address)
+{
+	flash_header_v2_t *headerv2 = address;
+	ulong (*entry)(void);
+
+	if (headerv2->header.tag != IVT_HEADER_TAG) {
+		printf("Header Tag is not a IMX image\n");
+		return SDP_ERROR_IMXHEADER;
+	}
+
+	printf("Jumping to 0x%08x\n", headerv2->entry);
+	entry = (void *)headerv2->entry;
+	entry();
+
+	/* The image probably never returns hence we wont reach that point */
+	return 0;
+}
+
+static void sdp_handle_in_ep(void)
+{
+	u8 *data = sdp_func->in_req->buf;
+	u32 status;
+	int datalen;
+
+	switch (sdp_func->state) {
+	case SDP_STATE_TX_SEC_CONF:
+		debug("Report 3: HAB security\n");
+		data[0] = 3;
Quoted 9 lines

drivers/usb/gadget/f_sdp.c line 640

Quoted 5 lines
drivers/usb/gadget/f_sdp.c new file +5 -0
+
+		data[1] = 0x56;
+		data[2] = 0x78;
+		data[3] = 0x78;
+		data[4] = 0x56;

I am quite lost here - can you explain what are these magic numbers, and maybe add a comment for it (or self explaining defines) ?

Yeah protocol specific magic number:
HAB security
configuration. Device
sends 0x12343412 in
closed mode and
0x56787856 in open
mode.

Quoted 4 lines

We always assume open. Not sure what kind of implication that can have, I think imx_usb basically just prints out what the device says.

Will create proper defines.

Thanks.

Best regards,
Stefano

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#9 lore raw

PATCH 3/7 usb: gadget: sdp: extend images compatible for jumps

In reply to Stefan Agner

From: Stefan Agner <[email protected]>

Support U-Boot images in SPL so that u-boot.img files can be directly downloaded and executed. Furthermore support U-Boot scripts download and execution in full U-Boot so that custom recovery actions can be downloaded from the host in a third step.

---

 drivers/usb/gadget/f_sdp.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)
drivers/usb/gadget/f_sdp.c +18 -2
index eb89695aaf..9a752843f0 100644
@@ -29,6 +29,8 @@
 #include <asm/io.h>
 #include <g_dnl.h>
 #include <sdp.h>
+#include <spl.h>
+#include <image.h>
 #include <imximage.h>
 
 #define HID_REPORT_ID_MASK	0x000000ff
@@ -672,8 +674,22 @@ static void sdp_handle_in_ep(void)
 		sdp_func->state = SDP_STATE_TX_REGISTER_BUSY;
 		break;
 	case SDP_STATE_JUMP:
-		printf("Checking imxheader at 0x%08x\n", f_sdp->jmp_address);
-		status = sdp_jump_imxheader((void *)f_sdp->jmp_address);
+		printf("Jumping to header at 0x%08x\n", sdp_func->jmp_address);
+		status = sdp_jump_imxheader((void *)sdp_func->jmp_address);
+
+		/* If imx header fails, try some U-Boot specific headers */
+		if (status) {
+#ifdef CONFIG_SPL_BUILD
+			/* In SPL, allow jumps to U-Boot images */
+			struct spl_image_info spl_image = {};
+			spl_parse_image_header(&spl_image,
+				(struct image_header *)sdp_func->jmp_address);
+			jump_to_image_no_args(&spl_image);
+#else
+			/* In U-Boot, allow jumps to scripts */
+			source(sdp_func->jmp_address, "script at 1");
+#endif
+		}
 
 		sdp_func->next_state = SDP_STATE_IDLE;
 		sdp_func->error_status = status;
Signature · 1 line
2.13.3
#10 lore raw

PATCH 3/7 usb: gadget: sdp: extend images compatible for jumps

Hi Stefan,

Quoted 9 lines

From: Stefan Agner <[email protected]>

Support U-Boot images in SPL so that u-boot.img files can be directly downloaded and executed. Furthermore support U-Boot scripts download and execution in full U-Boot so that custom recovery actions can be downloaded from the host in a third step.

Quoted 44 lines

---

 drivers/usb/gadget/f_sdp.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)
drivers/usb/gadget/f_sdp.c +18 -2
index eb89695aaf..9a752843f0 100644
@@ -29,6 +29,8 @@
 #include <asm/io.h>
 #include <g_dnl.h>
 #include <sdp.h>
+#include <spl.h>
+#include <image.h>
 #include <imximage.h>
 
 #define HID_REPORT_ID_MASK	0x000000ff
@@ -672,8 +674,22 @@ static void sdp_handle_in_ep(void)
 		sdp_func->state = SDP_STATE_TX_REGISTER_BUSY;
 		break;
 	case SDP_STATE_JUMP:
-		printf("Checking imxheader at 0x%08x\n", f_sdp->jmp_address);
-		status = sdp_jump_imxheader((void *)f_sdp->jmp_address);
+		printf("Jumping to header at 0x%08x\n", sdp_func->jmp_address);
+		status = sdp_jump_imxheader((void *)sdp_func->jmp_address);
+
+		/* If imx header fails, try some U-Boot specific headers */
+		if (status) {
+#ifdef CONFIG_SPL_BUILD
+			/* In SPL, allow jumps to U-Boot images */
+			struct spl_image_info spl_image = {};
+			spl_parse_image_header(&spl_image,
+				(struct image_header *)sdp_func->jmp_address);
+			jump_to_image_no_args(&spl_image);
+#else
+			/* In U-Boot, allow jumps to scripts */
+			source(sdp_func->jmp_address, "script at 1");
+#endif
+		}
 
 		sdp_func->next_state = SDP_STATE_IDLE;
 		sdp_func->error_status = status;
Signature · 9 lines
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
#11 lore raw

PATCH 3/7 usb: gadget: sdp: extend images compatible for jumps

In reply to Stefan Agner
Quoted 43 lines · Stefan Agner

From: Stefan Agner <[email protected]>

Support U-Boot images in SPL so that u-boot.img files can be directly downloaded and executed. Furthermore support U-Boot scripts download and execution in full U-Boot so that custom recovery actions can be downloaded from the host in a third step.

---

 drivers/usb/gadget/f_sdp.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)
drivers/usb/gadget/f_sdp.c +11 -2
index eb89695aaf..9a752843f0 100644
@@ -29,6 +29,8 @@
 #include <asm/io.h>
 #include <g_dnl.h>
 #include <sdp.h>
+#include <spl.h>
+#include <image.h>
 #include <imximage.h>
 
 #define HID_REPORT_ID_MASK	0x000000ff
@@ -672,8 +674,22 @@ static void sdp_handle_in_ep(void)
 		sdp_func->state = SDP_STATE_TX_REGISTER_BUSY;
 		break;
 	case SDP_STATE_JUMP:
-		printf("Checking imxheader at 0x%08x\n", f_sdp->jmp_address);
-		status = sdp_jump_imxheader((void *)f_sdp->jmp_address);
+		printf("Jumping to header at 0x%08x\n", sdp_func->jmp_address);
+		status = sdp_jump_imxheader((void *)sdp_func->jmp_address);
+
+		/* If imx header fails, try some U-Boot specific headers */
+		if (status) {
+#ifdef CONFIG_SPL_BUILD
+			/* In SPL, allow jumps to U-Boot images */
+			struct spl_image_info spl_image = {};
+			spl_parse_image_header(&spl_image,

drivers/usb/gadget/f_sdp.c static void sdp_handle_in_ep(void) · line 695

Quoted 10 lines
drivers/usb/gadget/f_sdp.c +7 -0
+				(struct image_header *)sdp_func->jmp_address);
+			jump_to_image_no_args(&spl_image);
+#else
+			/* In U-Boot, allow jumps to scripts */
+			source(sdp_func->jmp_address, "script at 1");
+#endif
+		}
 
 		sdp_func->next_state = SDP_STATE_IDLE;
 		sdp_func->error_status = status;

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#12 lore raw

PATCH 4/7 cmd: add sdp command

In reply to Stefan Agner

From: Stefan Agner <[email protected]>

Add a new command to start USB Serial Download Protocol (SDP) state machine.

---

 cmd/Kconfig          |  7 +++++++
 cmd/Makefile         |  1 +
 cmd/usb_gadget_sdp.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+)
 create mode 100644 cmd/usb_gadget_sdp.c
cmd/Kconfig +7 -0
index f18efc1e88..87333b3a97 100644
@@ -665,6 +665,13 @@ config CMD_DFU
 	  Enables the command "dfu" which is used to have U-Boot create a DFU
 	  class device via USB.
 
+config CMD_USB_SDP
+	bool "sdp"
+	select USB_FUNCTION_SDP
+	help
+	  Enables the command "sdp" which is used to have U-Boot emulating the
+	  Serial Download Protocol (SDP) via USB.
+
 config CMD_USB_MASS_STORAGE
 	bool "UMS usb mass storage"
 	help
cmd/Makefile +1 -0
index bd231f24d8..e0b5940ba6 100644
@@ -131,6 +131,7 @@ obj-$(CONFIG_CMD_FASTBOOT) += fastboot.o
 obj-$(CONFIG_CMD_FS_UUID) += fs_uuid.o
 
 obj-$(CONFIG_CMD_USB_MASS_STORAGE) += usb_mass_storage.o
+obj-$(CONFIG_CMD_USB_SDP) += usb_gadget_sdp.o
 obj-$(CONFIG_CMD_THOR_DOWNLOAD) += thordown.o
 obj-$(CONFIG_CMD_XIMG) += ximg.o
 obj-$(CONFIG_YAFFS2) += yaffs2.o
cmd/usb_gadget_sdp.c new file +53 -0

53 lines not on this page · raw message

Signature · 1 line
2.13.3
#13 lore raw

PATCH 4/7 cmd: add sdp command

Quoted 94 lines · Stefan Agner

From: Stefan Agner <[email protected]>

Add a new command to start USB Serial Download Protocol (SDP) state machine.

---

 cmd/Kconfig          |  7 +++++++
 cmd/Makefile         |  1 +
 cmd/usb_gadget_sdp.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+)
 create mode 100644 cmd/usb_gadget_sdp.c
cmd/Kconfig +7 -0
index f18efc1e88..87333b3a97 100644
@@ -665,6 +665,13 @@ config CMD_DFU
 	  Enables the command "dfu" which is used to have U-Boot create a DFU
 	  class device via USB.
 
+config CMD_USB_SDP
+	bool "sdp"
+	select USB_FUNCTION_SDP
+	help
+	  Enables the command "sdp" which is used to have U-Boot emulating the
+	  Serial Download Protocol (SDP) via USB.
+
 config CMD_USB_MASS_STORAGE
 	bool "UMS usb mass storage"
 	help
cmd/Makefile +1 -0
index bd231f24d8..e0b5940ba6 100644
@@ -131,6 +131,7 @@ obj-$(CONFIG_CMD_FASTBOOT) += fastboot.o
 obj-$(CONFIG_CMD_FS_UUID) += fs_uuid.o
 
 obj-$(CONFIG_CMD_USB_MASS_STORAGE) += usb_mass_storage.o
+obj-$(CONFIG_CMD_USB_SDP) += usb_gadget_sdp.o
 obj-$(CONFIG_CMD_THOR_DOWNLOAD) += thordown.o
 obj-$(CONFIG_CMD_XIMG) += ximg.o
 obj-$(CONFIG_YAFFS2) += yaffs2.o
cmd/usb_gadget_sdp.c new file +43 -0
new file mode 100644
index 0000000000..09ddb4f3aa
@@ -0,0 +1,53 @@
+/*
+ * cmd_sdp.c -- sdp command
+ *
+ * Copyright (C) 2016 Toradex
+ * Author: Stefan Agner <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <g_dnl.h>
+#include <sdp.h>
+#include <usb.h>
+
+static int do_sdp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	int ret = CMD_RET_SUCCESS;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	char *usb_controller = argv[1];
+	int controller_index = simple_strtoul(usb_controller, NULL, 0);
+	board_usb_init(controller_index, USB_INIT_DEVICE);
+
+	g_dnl_clear_detach();
+	g_dnl_register("usb_dnl_sdp");
+
+	ret = sdp_init();
+	if (ret) {
+		error("SDP init failed: %d", ret);
+		ret = CMD_RET_FAILURE;
+		goto exit;
+	}
+
+	ret = sdp_handle();
+	if (ret) {
+		error("SDP failed: %d", ret);
+		ret = CMD_RET_FAILURE;
+		goto exit;
+	}
+
+exit:

cmd/usb_gadget_sdp.c line 53

Quoted 10 lines
cmd/usb_gadget_sdp.c new file +10 -0
+	g_dnl_unregister();
+
+	return ret;
+}
+
+U_BOOT_CMD(sdp, 2, 1, do_sdp,
+	"Serial Downloader Protocol",
+	"<USB_controller>\n"
+	"  - serial downloader protocol via <USB_controller>\n"
+);
Signature · 9 lines
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
#14 lore raw

PATCH 4/7 cmd: add sdp command

In reply to Stefan Agner

Hi Stefan,

Quoted 94 lines · Stefan Agner

From: Stefan Agner <[email protected]>

Add a new command to start USB Serial Download Protocol (SDP) state machine.

---

 cmd/Kconfig          |  7 +++++++
 cmd/Makefile         |  1 +
 cmd/usb_gadget_sdp.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+)
 create mode 100644 cmd/usb_gadget_sdp.c
cmd/Kconfig +7 -0
index f18efc1e88..87333b3a97 100644
@@ -665,6 +665,13 @@ config CMD_DFU
 	  Enables the command "dfu" which is used to have U-Boot create a DFU
 	  class device via USB.
 
+config CMD_USB_SDP
+	bool "sdp"
+	select USB_FUNCTION_SDP
+	help
+	  Enables the command "sdp" which is used to have U-Boot emulating the
+	  Serial Download Protocol (SDP) via USB.
+
 config CMD_USB_MASS_STORAGE
 	bool "UMS usb mass storage"
 	help
cmd/Makefile +1 -0
index bd231f24d8..e0b5940ba6 100644
@@ -131,6 +131,7 @@ obj-$(CONFIG_CMD_FASTBOOT) += fastboot.o
 obj-$(CONFIG_CMD_FS_UUID) += fs_uuid.o
 
 obj-$(CONFIG_CMD_USB_MASS_STORAGE) += usb_mass_storage.o
+obj-$(CONFIG_CMD_USB_SDP) += usb_gadget_sdp.o
 obj-$(CONFIG_CMD_THOR_DOWNLOAD) += thordown.o
 obj-$(CONFIG_CMD_XIMG) += ximg.o
 obj-$(CONFIG_YAFFS2) += yaffs2.o
cmd/usb_gadget_sdp.c new file +43 -0
new file mode 100644
index 0000000000..09ddb4f3aa
@@ -0,0 +1,53 @@
+/*
+ * cmd_sdp.c -- sdp command
+ *
+ * Copyright (C) 2016 Toradex
+ * Author: Stefan Agner <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <g_dnl.h>
+#include <sdp.h>
+#include <usb.h>
+
+static int do_sdp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	int ret = CMD_RET_SUCCESS;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	char *usb_controller = argv[1];
+	int controller_index = simple_strtoul(usb_controller, NULL, 0);
+	board_usb_init(controller_index, USB_INIT_DEVICE);
+
+	g_dnl_clear_detach();
+	g_dnl_register("usb_dnl_sdp");
+
+	ret = sdp_init();
+	if (ret) {
+		error("SDP init failed: %d", ret);
+		ret = CMD_RET_FAILURE;
+		goto exit;
+	}
+
+	ret = sdp_handle();
+	if (ret) {
+		error("SDP failed: %d", ret);
+		ret = CMD_RET_FAILURE;
+		goto exit;
+	}
+
+exit:

cmd/usb_gadget_sdp.c line 53

Quoted 10 lines
cmd/usb_gadget_sdp.c new file +10 -0
+	g_dnl_unregister();
+
+	return ret;
+}
+
+U_BOOT_CMD(sdp, 2, 1, do_sdp,
+	"Serial Downloader Protocol",
+	"<USB_controller>\n"
+	"  - serial downloader protocol via <USB_controller>\n"
+);

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#15 lore raw

PATCH 5/7 spl: add serial download protocol (SDP) support

In reply to Stefan Agner

From: Stefan Agner <[email protected]>

Add USB serial download protocol support to SPL. If the SoC started in recovery mode the SPL will immediately switch to SDP and wait for further downloads/commands from the host side.

---

 common/spl/Kconfig          |  6 ++++++
 common/spl/Makefile         |  1 +
 common/spl/spl_sdp.c        | 38 ++++++++++++++++++++++++++++++++++++++
 drivers/usb/gadget/Makefile |  1 +
 4 files changed, 46 insertions(+)
 create mode 100644 common/spl/spl_sdp.c
common/spl/Kconfig +6 -0
index 4de81392b0..95378b98a0 100644
@@ -646,6 +646,12 @@ config SPL_DFU_RAM
 
 endchoice
 
+config SPL_USB_SDP_SUPPORT
+	bool "Support SDP (Serial Download Protocol)"
+	help
+	  Enable Serial Download Protocol (SDP) device support in SPL. This
+	  allows to download images into memory and execute (jump to) them
+	  using the same protocol as implemented by the i.MX family's boot ROM.
 endif
 
 config SPL_WATCHDOG_SUPPORT
common/spl/Makefile +1 -0
index 47a64dd7d0..a979560acf 100644
@@ -29,4 +29,5 @@ obj-$(CONFIG_SPL_SATA_SUPPORT) += spl_sata.o
 obj-$(CONFIG_SPL_DFU_SUPPORT) += spl_dfu.o
 obj-$(CONFIG_SPL_SPI_LOAD) += spl_spi.o
 obj-$(CONFIG_SPL_RAM_SUPPORT) += spl_ram.o
+obj-$(CONFIG_SPL_USB_SDP_SUPPORT) += spl_sdp.o
 endif
common/spl/spl_sdp.c new file +38 -0
new file mode 100644
index 0000000000..faa74b8bec
@@ -0,0 +1,38 @@
+/*
+ * (C) Copyright 2016 Toradex
+ * Author: Stefan Agner <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <spl.h>
+#include <usb.h>
+#include <g_dnl.h>
+#include <sdp.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int spl_sdp_load_image(struct spl_image_info *spl_image,
+			      struct spl_boot_device *bootdev)
+{
+	int ret;
+
+	g_dnl_clear_detach();
+	g_dnl_register("usb_dnl_sdp");
+
+	ret = sdp_init();
+	if (ret) {
+		error("SDP init failed: %d", ret);
+		return -ENODEV;
+	}
+
+	ret = sdp_handle();
+	if (ret) {
+		error("SDP failed: %d", ret);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+SPL_LOAD_IMAGE_METHOD("USB SDP", 0, BOOT_DEVICE_UART, spl_sdp_load_image);
drivers/usb/gadget/Makefile +1 -0
index 6a007d1bcb..7258099c1c 100644
@@ -11,6 +11,7 @@ obj-$(CONFIG_USB_ETHER) += epautoconf.o config.o usbstring.o
 ifdef CONFIG_SPL_BUILD
 obj-$(CONFIG_SPL_USB_GADGET_SUPPORT) += g_dnl.o
 obj-$(CONFIG_SPL_DFU_SUPPORT) += f_dfu.o
+obj-$(CONFIG_SPL_USB_SDP_SUPPORT) += f_sdp.o
 endif
 
 # new USB gadget layer dependencies
Signature · 1 line
2.13.3
#16 lore raw

PATCH 5/7 spl: add serial download protocol (SDP) support

Hi,

Quoted 70 lines · On Fri, 4 Aug 2017 16:38:11 -0700 Stefan Agner wrote:

From: Stefan Agner <[email protected]>

Add USB serial download protocol support to SPL. If the SoC started in recovery mode the SPL will immediately switch to SDP and wait for further downloads/commands from the host side.

---

 common/spl/Kconfig          |  6 ++++++
 common/spl/Makefile         |  1 +
 common/spl/spl_sdp.c        | 38 ++++++++++++++++++++++++++++++++++++++
 drivers/usb/gadget/Makefile |  1 +
 4 files changed, 46 insertions(+)
 create mode 100644 common/spl/spl_sdp.c
common/spl/Kconfig +6 -0
index 4de81392b0..95378b98a0 100644
@@ -646,6 +646,12 @@ config SPL_DFU_RAM
 
 endchoice
 
+config SPL_USB_SDP_SUPPORT
+	bool "Support SDP (Serial Download Protocol)"
+	help
+	  Enable Serial Download Protocol (SDP) device support in SPL. This
+	  allows to download images into memory and execute (jump to) them
+	  using the same protocol as implemented by the i.MX family's boot ROM.
 endif
 
 config SPL_WATCHDOG_SUPPORT
common/spl/Makefile +1 -0
index 47a64dd7d0..a979560acf 100644
@@ -29,4 +29,5 @@ obj-$(CONFIG_SPL_SATA_SUPPORT) += spl_sata.o
 obj-$(CONFIG_SPL_DFU_SUPPORT) += spl_dfu.o
 obj-$(CONFIG_SPL_SPI_LOAD) += spl_spi.o
 obj-$(CONFIG_SPL_RAM_SUPPORT) += spl_ram.o
+obj-$(CONFIG_SPL_USB_SDP_SUPPORT) += spl_sdp.o
 endif
common/spl/spl_sdp.c new file +20 -0
new file mode 100644
index 0000000000..faa74b8bec
@@ -0,0 +1,38 @@
+/*
+ * (C) Copyright 2016 Toradex
+ * Author: Stefan Agner <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <spl.h>
+#include <usb.h>
+#include <g_dnl.h>
+#include <sdp.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int spl_sdp_load_image(struct spl_image_info *spl_image,
+			      struct spl_boot_device *bootdev)
+{
+	int ret;
+

common/spl/spl_sdp.c line 30

Quoted 10 lines
common/spl/spl_sdp.c new file +10 -0
+	g_dnl_clear_detach();
+	g_dnl_register("usb_dnl_sdp");
+
+	ret = sdp_init();
+	if (ret) {
+		error("SDP init failed: %d", ret);
+		return -ENODEV;
+	}
+
+	ret = sdp_handle();

This function (and sdp_init() above) always return 0. Do you expect to be changed in future? Otherwise the return type of those functions should be changed to void.

Lothar Waßmann

#17 lore raw

PATCH 5/7 spl: add serial download protocol (SDP) support

Quoted 78 lines · Lothar Waßmann

Hi,

Quoted 75 lines · On Fri, 4 Aug 2017 16:38:11 -0700 Stefan Agner wrote:

From: Stefan Agner <[email protected]>

Add USB serial download protocol support to SPL. If the SoC started in recovery mode the SPL will immediately switch to SDP and wait for further downloads/commands from the host side.

---

 common/spl/Kconfig          |  6 ++++++
 common/spl/Makefile         |  1 +
 common/spl/spl_sdp.c        | 38 ++++++++++++++++++++++++++++++++++++++
 drivers/usb/gadget/Makefile |  1 +
 4 files changed, 46 insertions(+)
 create mode 100644 common/spl/spl_sdp.c
common/spl/Kconfig +6 -0
index 4de81392b0..95378b98a0 100644
@@ -646,6 +646,12 @@ config SPL_DFU_RAM
 
 endchoice
 
+config SPL_USB_SDP_SUPPORT
+	bool "Support SDP (Serial Download Protocol)"
+	help
+	  Enable Serial Download Protocol (SDP) device support in SPL. This
+	  allows to download images into memory and execute (jump to) them
+	  using the same protocol as implemented by the i.MX family's boot ROM.
 endif
 
 config SPL_WATCHDOG_SUPPORT
common/spl/Makefile +1 -0
index 47a64dd7d0..a979560acf 100644
@@ -29,4 +29,5 @@ obj-$(CONFIG_SPL_SATA_SUPPORT) += spl_sata.o
 obj-$(CONFIG_SPL_DFU_SUPPORT) += spl_dfu.o
 obj-$(CONFIG_SPL_SPI_LOAD) += spl_spi.o
 obj-$(CONFIG_SPL_RAM_SUPPORT) += spl_ram.o
+obj-$(CONFIG_SPL_USB_SDP_SUPPORT) += spl_sdp.o
 endif
common/spl/spl_sdp.c new file +26 -0
new file mode 100644
index 0000000000..faa74b8bec
@@ -0,0 +1,38 @@
+/*
+ * (C) Copyright 2016 Toradex
+ * Author: Stefan Agner <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <spl.h>
+#include <usb.h>
+#include <g_dnl.h>
+#include <sdp.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int spl_sdp_load_image(struct spl_image_info *spl_image,
+			      struct spl_boot_device *bootdev)
+{
+	int ret;
+
+	g_dnl_clear_detach();
+	g_dnl_register("usb_dnl_sdp");
+
+	ret = sdp_init();
+	if (ret) {
+		error("SDP init failed: %d", ret);
Quoted 9 lines

common/spl/spl_sdp.c line 30

Quoted 5 lines
common/spl/spl_sdp.c new file +4 -0
+		return -ENODEV;
+	}
+
+	ret = sdp_handle();

This function (and sdp_init() above) always return 0. Do you expect to be changed in future? Otherwise the return type of those functions should be changed to void.

I followed f_thor's approach here which use int in both functions, but yeah, in the SDP case there is currently no error condition.

Not sure there will be one added in the future, I guess we could just make it void for now.

--
Stefan

#18 lore raw

PATCH 5/7 spl: add serial download protocol (SDP) support

In reply to Stefan Agner

Stefano,

One question below:

Quoted 77 lines · Stefan Agner

From: Stefan Agner <[email protected]>

Add USB serial download protocol support to SPL. If the SoC started in recovery mode the SPL will immediately switch to SDP and wait for further downloads/commands from the host side.

---

 common/spl/Kconfig          |  6 ++++++
 common/spl/Makefile         |  1 +
 common/spl/spl_sdp.c        | 38 ++++++++++++++++++++++++++++++++++++++
 drivers/usb/gadget/Makefile |  1 +
 4 files changed, 46 insertions(+)
 create mode 100644 common/spl/spl_sdp.c
common/spl/Kconfig +6 -0
index 4de81392b0..95378b98a0 100644
@@ -646,6 +646,12 @@ config SPL_DFU_RAM
 
 endchoice
 
+config SPL_USB_SDP_SUPPORT
+	bool "Support SDP (Serial Download Protocol)"
+	help
+	  Enable Serial Download Protocol (SDP) device support in SPL. This
+	  allows to download images into memory and execute (jump to) them
+	  using the same protocol as implemented by the i.MX family's boot ROM.
 endif
 
 config SPL_WATCHDOG_SUPPORT
common/spl/Makefile +1 -0
index 47a64dd7d0..a979560acf 100644
@@ -29,4 +29,5 @@ obj-$(CONFIG_SPL_SATA_SUPPORT) += spl_sata.o
 obj-$(CONFIG_SPL_DFU_SUPPORT) += spl_dfu.o
 obj-$(CONFIG_SPL_SPI_LOAD) += spl_spi.o
 obj-$(CONFIG_SPL_RAM_SUPPORT) += spl_ram.o
+obj-$(CONFIG_SPL_USB_SDP_SUPPORT) += spl_sdp.o
 endif
common/spl/spl_sdp.c new file +28 -0
new file mode 100644
index 0000000000..faa74b8bec
@@ -0,0 +1,38 @@
+/*
+ * (C) Copyright 2016 Toradex
+ * Author: Stefan Agner <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <spl.h>
+#include <usb.h>
+#include <g_dnl.h>
+#include <sdp.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int spl_sdp_load_image(struct spl_image_info *spl_image,
+			      struct spl_boot_device *bootdev)
+{
+	int ret;
+
+	g_dnl_clear_detach();
+	g_dnl_register("usb_dnl_sdp");
+
+	ret = sdp_init();
+	if (ret) {
+		error("SDP init failed: %d", ret);
+		return -ENODEV;
+	}

common/spl/spl_sdp.c line 38

Quoted 10 lines
common/spl/spl_sdp.c new file +10 -0
+
+	ret = sdp_handle();
+	if (ret) {
+		error("SDP failed: %d", ret);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+SPL_LOAD_IMAGE_METHOD("USB SDP", 0, BOOT_DEVICE_UART, spl_sdp_load_image);

We currently use BOOT_DEVICE_UART when serial downloader boot mode is detected. This can be either USB or UART...

In fact, USB is probably much more often used since only 6UL/ULL have UART serial downloader support afact...

There is BOOT_DEVICE_BOARD which is used by e.g. Sunxi in case the boot ROM mechanism is used, what do you think, should be change that?

Ideally we should be able to discriminate between USB and UART. But I don't think its possible... (the USBPH0_PWD method likely does not work since even in the UART case the boot ROM will initialize the USB PHY first, at least according to the flow diagram in ULL's Chapter 9...)

The i.MX 7 which has the new Boot Information block actually only support USB serial downloader...

Thoughts?

--
Stefan

Quoted 12 lines
drivers/usb/gadget/Makefile +1 -0
index 6a007d1bcb..7258099c1c 100644
@@ -11,6 +11,7 @@ obj-$(CONFIG_USB_ETHER) += epautoconf.o config.o usbstring.o
 ifdef CONFIG_SPL_BUILD
 obj-$(CONFIG_SPL_USB_GADGET_SUPPORT) += g_dnl.o
 obj-$(CONFIG_SPL_DFU_SUPPORT) += f_dfu.o
+obj-$(CONFIG_SPL_USB_SDP_SUPPORT) += f_sdp.o
 endif
 
 # new USB gadget layer dependencies
#19 lore raw

PATCH 5/7 spl: add serial download protocol (SDP) support

Hi Stefan,

Quoted 88 lines · Stefan Agner

Stefano,

One question below:

Quoted 83 lines · Stefan Agner

From: Stefan Agner <[email protected]>

Add USB serial download protocol support to SPL. If the SoC started in recovery mode the SPL will immediately switch to SDP and wait for further downloads/commands from the host side.

---

 common/spl/Kconfig          |  6 ++++++
 common/spl/Makefile         |  1 +
 common/spl/spl_sdp.c        | 38 ++++++++++++++++++++++++++++++++++++++
 drivers/usb/gadget/Makefile |  1 +
 4 files changed, 46 insertions(+)
 create mode 100644 common/spl/spl_sdp.c
common/spl/Kconfig +6 -0
index 4de81392b0..95378b98a0 100644
@@ -646,6 +646,12 @@ config SPL_DFU_RAM
 
 endchoice
 
+config SPL_USB_SDP_SUPPORT
+	bool "Support SDP (Serial Download Protocol)"
+	help
+	  Enable Serial Download Protocol (SDP) device support in SPL. This
+	  allows to download images into memory and execute (jump to) them
+	  using the same protocol as implemented by the i.MX family's boot ROM.
 endif
 
 config SPL_WATCHDOG_SUPPORT
common/spl/Makefile +1 -0
index 47a64dd7d0..a979560acf 100644
@@ -29,4 +29,5 @@ obj-$(CONFIG_SPL_SATA_SUPPORT) += spl_sata.o
 obj-$(CONFIG_SPL_DFU_SUPPORT) += spl_dfu.o
 obj-$(CONFIG_SPL_SPI_LOAD) += spl_spi.o
 obj-$(CONFIG_SPL_RAM_SUPPORT) += spl_ram.o
+obj-$(CONFIG_SPL_USB_SDP_SUPPORT) += spl_sdp.o
 endif
common/spl/spl_sdp.c new file +34 -0
new file mode 100644
index 0000000000..faa74b8bec
@@ -0,0 +1,38 @@
+/*
+ * (C) Copyright 2016 Toradex
+ * Author: Stefan Agner <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <spl.h>
+#include <usb.h>
+#include <g_dnl.h>
+#include <sdp.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int spl_sdp_load_image(struct spl_image_info *spl_image,
+			      struct spl_boot_device *bootdev)
+{
+	int ret;
+
+	g_dnl_clear_detach();
+	g_dnl_register("usb_dnl_sdp");
+
+	ret = sdp_init();
+	if (ret) {
+		error("SDP init failed: %d", ret);
+		return -ENODEV;
+	}
+
+	ret = sdp_handle();
+	if (ret) {
+		error("SDP failed: %d", ret);
+		return -ENODEV;
+	}
Quoted 9 lines

common/spl/spl_sdp.c line 38

Quoted 4 lines
common/spl/spl_sdp.c new file +4 -0
+
+	return 0;
+}
+SPL_LOAD_IMAGE_METHOD("USB SDP", 0, BOOT_DEVICE_UART, spl_sdp_load_image);

We currently use BOOT_DEVICE_UART when serial downloader boot mode is detected. This can be either USB or UART...

Right

Quoted 2 lines

In fact, USB is probably much more often used since only 6UL/ULL have UART serial downloader support afact...

The misunderstanding is originated from NXP's documentation. The official name is "Serial Downloader" and this can be easy confused as "UART" downloader. Even if documentation is correct, it can be bad interpreted.

To be correct, we have a "serial protocol" over a device that can be either USB or UART.

Quoted 3 lines

There is BOOT_DEVICE_BOARD which is used by e.g. Sunxi in case the boot ROM mechanism is used, what do you think, should be change that?

Good point. To be consistent (thanks for hint, I was not aware of this in sunxi) we should change it, yes.

Quoted 5 lines

Ideally we should be able to discriminate between USB and UART. But I don't think its possible... (the USBPH0_PWD method likely does not work since even in the UART case the boot ROM will initialize the USB PHY first, at least according to the flow diagram in ULL's Chapter 9...)

I agree with you - we should get rid of undocumented feature inside ROM, whose behaviour could be changed with SOC revision number.

The use case is fixed - if a board loads from USB, it will always load from USB. A runtime detection is difficult and even overkilling for this application.

Quoted 5 lines

The i.MX 7 which has the new Boot Information block actually only support USB serial downloader...

Thoughts?

As far as I can see and apart of names, we should reach to have this mainlined as USB downloader. It covers quite all cases.

Best regards,
Stefano

Quoted 17 lines

--
Stefan

Quoted 12 lines
drivers/usb/gadget/Makefile +1 -0
index 6a007d1bcb..7258099c1c 100644
@@ -11,6 +11,7 @@ obj-$(CONFIG_USB_ETHER) += epautoconf.o config.o usbstring.o
 ifdef CONFIG_SPL_BUILD
 obj-$(CONFIG_SPL_USB_GADGET_SUPPORT) += g_dnl.o
 obj-$(CONFIG_SPL_DFU_SUPPORT) += f_dfu.o
+obj-$(CONFIG_SPL_USB_SDP_SUPPORT) += f_sdp.o
 endif
 
 # new USB gadget layer dependencies
Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#20 lore raw

PATCH 5/7 spl: add serial download protocol (SDP) support

In reply to Stefan Agner

Hi Stefan,

Quoted 93 lines · Stefan Agner

From: Stefan Agner <[email protected]>

Add USB serial download protocol support to SPL. If the SoC started in recovery mode the SPL will immediately switch to SDP and wait for further downloads/commands from the host side.

---

 common/spl/Kconfig          |  6 ++++++
 common/spl/Makefile         |  1 +
 common/spl/spl_sdp.c        | 38 ++++++++++++++++++++++++++++++++++++++
 drivers/usb/gadget/Makefile |  1 +
 4 files changed, 46 insertions(+)
 create mode 100644 common/spl/spl_sdp.c
common/spl/Kconfig +6 -0
index 4de81392b0..95378b98a0 100644
@@ -646,6 +646,12 @@ config SPL_DFU_RAM
 
 endchoice
 
+config SPL_USB_SDP_SUPPORT
+	bool "Support SDP (Serial Download Protocol)"
+	help
+	  Enable Serial Download Protocol (SDP) device support in SPL. This
+	  allows to download images into memory and execute (jump to) them
+	  using the same protocol as implemented by the i.MX family's boot ROM.
 endif
 
 config SPL_WATCHDOG_SUPPORT
common/spl/Makefile +1 -0
index 47a64dd7d0..a979560acf 100644
@@ -29,4 +29,5 @@ obj-$(CONFIG_SPL_SATA_SUPPORT) += spl_sata.o
 obj-$(CONFIG_SPL_DFU_SUPPORT) += spl_dfu.o
 obj-$(CONFIG_SPL_SPI_LOAD) += spl_spi.o
 obj-$(CONFIG_SPL_RAM_SUPPORT) += spl_ram.o
+obj-$(CONFIG_SPL_USB_SDP_SUPPORT) += spl_sdp.o
 endif
common/spl/spl_sdp.c new file +38 -0
new file mode 100644
index 0000000000..faa74b8bec
@@ -0,0 +1,38 @@
+/*
+ * (C) Copyright 2016 Toradex
+ * Author: Stefan Agner <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <spl.h>
+#include <usb.h>
+#include <g_dnl.h>
+#include <sdp.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int spl_sdp_load_image(struct spl_image_info *spl_image,
+			      struct spl_boot_device *bootdev)
+{
+	int ret;
+
+	g_dnl_clear_detach();
+	g_dnl_register("usb_dnl_sdp");
+
+	ret = sdp_init();
+	if (ret) {
+		error("SDP init failed: %d", ret);
+		return -ENODEV;
+	}
+
+	ret = sdp_handle();
+	if (ret) {
+		error("SDP failed: %d", ret);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+SPL_LOAD_IMAGE_METHOD("USB SDP", 0, BOOT_DEVICE_UART, spl_sdp_load_image);
drivers/usb/gadget/Makefile +0 -0
index 6a007d1bcb..7258099c1c 100644

drivers/usb/gadget/Makefile obj-$(CONFIG_USB_ETHER) += epautoconf.o config.o usbstring.o · line 17

Quoted 7 lines
drivers/usb/gadget/Makefile +1 -0
@@ -11,6 +11,7 @@ obj-$(CONFIG_USB_ETHER) += epautoconf.o config.o usbstring.o
 ifdef CONFIG_SPL_BUILD
 obj-$(CONFIG_SPL_USB_GADGET_SUPPORT) += g_dnl.o
 obj-$(CONFIG_SPL_DFU_SUPPORT) += f_dfu.o
+obj-$(CONFIG_SPL_USB_SDP_SUPPORT) += f_sdp.o
 endif
 
 # new USB gadget layer dependencies

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#21 lore raw

PATCH 6/7 apalis/colibri_imx6: use independent USB PID for SPL

In reply to Stefan Agner

From: Stefan Agner <[email protected]>

Use a completely independent USB Product ID for SPL. This allows to differentiate a SDP running in SPL and SDP running in a U-Boot which could not read the config block successfully.

---

 board/toradex/apalis_imx6/apalis_imx6.c   | 13 +++++++++++++
 board/toradex/colibri_imx6/colibri_imx6.c | 13 +++++++++++++
 2 files changed, 26 insertions(+)
board/toradex/apalis_imx6/apalis_imx6.c +13 -0
index 8e5613cb12..edaca5d346 100644
@@ -28,6 +28,7 @@
 #include <dm/platform_data/serial_mxc.h>
 #include <dm/platdata.h>
 #include <fsl_esdhc.h>
+#include <g_dnl.h>
 #include <i2c.h>
 #include <imx_thermal.h>
 #include <linux/errno.h>
@@ -1233,6 +1234,18 @@ void reset_cpu(ulong addr)
 {
 }
 
+#ifdef CONFIG_SPL_USB_GADGET_SUPPORT
+int g_dnl_bind_fixup(struct usb_device_descriptor *dev, const char *name)
+{
+	unsigned short usb_pid;
+
+	usb_pid = TORADEX_USB_PRODUCT_NUM_OFFSET + 0xfff;
+	put_unaligned(usb_pid, &dev->idProduct);
+
+	return 0;
+}
+#endif
+
 #endif
 
 static struct mxc_serial_platdata mxc_serial_plat = {
board/toradex/colibri_imx6/colibri_imx6.c +13 -0
index cbf7aa952a..0cc958a0a8 100644
@@ -28,6 +28,7 @@
 #include <dm/platform_data/serial_mxc.h>
 #include <dm/platdata.h>
 #include <fsl_esdhc.h>
+#include <g_dnl.h>
 #include <i2c.h>
 #include <imx_thermal.h>
 #include <linux/errno.h>
@@ -1118,6 +1119,18 @@ void reset_cpu(ulong addr)
 {
 }
 
+#ifdef CONFIG_SPL_USB_GADGET_SUPPORT
+int g_dnl_bind_fixup(struct usb_device_descriptor *dev, const char *name)
+{
+	unsigned short usb_pid;
+
+	usb_pid = TORADEX_USB_PRODUCT_NUM_OFFSET + 0xfff;
+	put_unaligned(usb_pid, &dev->idProduct);
+
+	return 0;
+}
+#endif
+
 #endif
 
 static struct mxc_serial_platdata mxc_serial_plat = {
Signature · 1 line
2.13.3
#22 lore raw

PATCH 7/7 apalis/colibri_imx6: enable SDP by default

In reply to Stefan Agner

From: Stefan Agner <[email protected]>

Enable Serial Download Protocol (SDP) in SPL and U-Boot. This is useful to make use of imx_usb to download the complete U-Boot (u-boot.img) after SPL has been downloaded. The U-Boot command sdp allows to enumerate as SDP capable device again, e.g. to download a Linux kernel and/or U-Boot script.

---

 configs/apalis_imx6_defconfig  | 4 ++++
 configs/colibri_imx6_defconfig | 4 ++++
 2 files changed, 8 insertions(+)
configs/apalis_imx6_defconfig +4 -0
index 4d88e70200..2a271f002c 100644
@@ -17,6 +17,9 @@ CONFIG_BOARD_EARLY_INIT_F=y
 CONFIG_SPL=y
 CONFIG_SPL_DMA_SUPPORT=y
 CONFIG_SPL_I2C_SUPPORT=y
+CONFIG_SPL_USB_HOST_SUPPORT=y
+CONFIG_SPL_USB_GADGET_SUPPORT=y
+CONFIG_SPL_USB_SDP_SUPPORT=y
 CONFIG_HUSH_PARSER=y
 CONFIG_SYS_PROMPT="Apalis iMX6 # "
 CONFIG_CMD_BOOTZ=y
@@ -31,6 +34,7 @@ CONFIG_CMD_MMC=y
 CONFIG_CMD_I2C=y
 CONFIG_CMD_USB=y
 CONFIG_CMD_DFU=y
+CONFIG_CMD_USB_SDP=y
 CONFIG_CMD_USB_MASS_STORAGE=y
 # CONFIG_CMD_FPGA is not set
 CONFIG_CMD_GPIO=y
configs/colibri_imx6_defconfig +4 -0
index a23975f858..d5d3e7a143 100644
@@ -17,6 +17,9 @@ CONFIG_BOARD_EARLY_INIT_F=y
 CONFIG_SPL=y
 CONFIG_SPL_DMA_SUPPORT=y
 CONFIG_SPL_I2C_SUPPORT=y
+CONFIG_SPL_USB_HOST_SUPPORT=y
+CONFIG_SPL_USB_GADGET_SUPPORT=y
+CONFIG_SPL_USB_SDP_SUPPORT=y
 CONFIG_HUSH_PARSER=y
 CONFIG_SYS_PROMPT="Colibri iMX6 # "
 CONFIG_CMD_BOOTZ=y
@@ -31,6 +34,7 @@ CONFIG_CMD_MMC=y
 CONFIG_CMD_I2C=y
 CONFIG_CMD_USB=y
 CONFIG_CMD_DFU=y
+CONFIG_CMD_USB_SDP=y
 CONFIG_CMD_USB_MASS_STORAGE=y
 # CONFIG_CMD_FPGA is not set
 CONFIG_CMD_GPIO=y
Signature · 1 line
2.13.3
#23 lore raw

cover imx: add USB Serial Download Protocol (SDP) support

In reply to Stefan Agner

Hi Stefan,

Quoted 7 lines · Stefan Agner

From: Stefan Agner <[email protected]>

This series adds NXP's Serial Download Protocol (SDP) support via USB for SPL/U-Boot. It allows to download U-Boot via USB from a (recovered) SPL using the same tools used to download SPL itself (specifically imx_usb, but also sb_loader seems to work).

Nice!

Quoted 4 lines

The idea has been brought up when the first targets started to make use of SPL for DDR initialization, see:
https://lists.denx.de/pipermail/u-boot/2015-July/220330.html

There have been lots of discussions surrounding the use of SDP, and this seems to be a nice alternative to using the i.MX "plugin" mode that I explored a while back:

https://lists.denx.de/pipermail/u-boot/2017-July/thread.html#298266

Quoted 10 lines

The initial SDP implementation (patch 2) requires the payload to have the imx specific headers (hence the move of the imx header file in patch 1).

Patch 3 extends image header support beyond the SDP specification, specifically implements also support for U-Boot headers. This allows to use the same SPL/U-Boot binaries for recovery as used on the regular boot device (SD/eMMC). For that to work also the host side imx_usb tools needed an extension, currently available here:

Quoted 4 lines

https://github.com/toradex/imx_loader/tree/imx_usb_batch_mode_refactored

The full patchset allows to download SPL and U-Boot over USB to a target in recovery mode using the same usb_imx utility:

imx_usb?

Quoted 8 lines

The usb_imx utility also has a batch mode which allows to download multiple artifacts with a single invocation. The details are outlined in the imx_usb commit message:
https://github.com/toradex/imx_loader/commit/5434415d921f1cc4d22332d9558bed6d42db9f60

In case this patchset gets accepted in U-Boot, I plan to push the imx_usb changes upstream as well.

Do you have numbers for how much code/data size this adds to SPL?

I believe some i.MX users have struggled to stay within the size of internal RAM.

Regards,

Eric

#24 lore raw

cover imx: add USB Serial Download Protocol (SDP) support

Hi Eric,

Quoted 18 lines · Eric Nelson

Hi Stefan,

Quoted 7 lines · Stefan Agner

From: Stefan Agner <[email protected]>

This series adds NXP's Serial Download Protocol (SDP) support via USB for SPL/U-Boot. It allows to download U-Boot via USB from a (recovered) SPL using the same tools used to download SPL itself (specifically imx_usb, but also sb_loader seems to work).

Nice!

Quoted 4 lines

The idea has been brought up when the first targets started to make use of SPL for DDR initialization, see:
https://lists.denx.de/pipermail/u-boot/2015-July/220330.html

Quoted 6 lines

There have been lots of discussions surrounding the use of SDP, and this seems to be a nice alternative to using the i.MX "plugin" mode that I explored a while back:

https://lists.denx.de/pipermail/u-boot/2017-July/thread.html#298266

Hm, wasn't aware of that particular effort, thanks for pointing to it. From a quick glance, it did not work out so far, is this correct?

I looked into plugin mode also a little bit, but I did not continue to look into it after reading this sentence in the i.MX 6 RM:

8.7 Plugin Image
The boot ROM detects the image type using the plugin flag of the boot data structure (see
Boot Data Structure). If the plugin flag is 1, then the ROM uses the image as a plugin
function. The function must initialize the boot device and copy the program image to the
final location. At the end the plugin function must return with the program image
parameters. (See High level boot sequence for details about boot flow).

So if SPL should work as a plugin as NXP defines it, SPL is supposed to load the image from somewhere. The boot ROM then only takes care about image verification and further boot steps, but it's the plugins job to get the image from somewhere and store it in RAM.

Afact this is not really helpful in our case. We would want the boot ROM to go through the boot sequence (again).

Unless returning an error/invalid image causes the boot ROM to go through boot sequence again?

The nice thing with our own SDP implementation is we can reuse it even from within U-Boot again, e.g. to download a kernel/initramfs....

Quoted 10 lines
Quoted 10 lines

The initial SDP implementation (patch 2) requires the payload to have the imx specific headers (hence the move of the imx header file in patch 1).

Patch 3 extends image header support beyond the SDP specification, specifically implements also support for U-Boot headers. This allows to use the same SPL/U-Boot binaries for recovery as used on the regular boot device (SD/eMMC). For that to work also the host side imx_usb tools needed an extension, currently available here:

Quoted 7 lines
Quoted 4 lines

https://github.com/toradex/imx_loader/tree/imx_usb_batch_mode_refactored

The full patchset allows to download SPL and U-Boot over USB to a target in recovery mode using the same usb_imx utility:

imx_usb?

Yeah right, sorry.

Quoted 11 lines
Quoted 8 lines

The usb_imx utility also has a batch mode which allows to download multiple artifacts with a single invocation. The details are outlined in the imx_usb commit message:
https://github.com/toradex/imx_loader/commit/5434415d921f1cc4d22332d9558bed6d42db9f60

In case this patchset gets accepted in U-Boot, I plan to push the imx_usb changes upstream as well.

Do you have numbers for how much code/data size this adds to SPL?

Besides the protocol implementation also general USB (gadget) support is required, hence it adds quite a bit.

Without USB Gadget/SDP support (Apalis iMX6 SPL):

$ arm-linux-gnueabihf-size spl/u-boot-spl
   text    data     bss     dec     hex filename
  24552    3808      92   28452    6f24 spl/u-boot-spl

With USB Gadget/SDP support:

$ arm-linux-gnueabihf-size spl/u-boot-spl
   text    data     bss     dec     hex filename
  42549    4679    1984   49212    c03c spl/u-boot-spl
Quoted 2 lines

I believe some i.MX users have struggled to stay within the size of internal RAM.

Hm, I think the limit is somewhere around 64kiB? In our case we are still well below...

--
Stefan

#25 lore raw

cover imx: add USB Serial Download Protocol (SDP) support

Hi Stefan,

Quoted 46 lines · Stefan Agner

Hi Eric,

Quoted 18 lines · Eric Nelson

Hi Stefan,

Quoted 7 lines · Stefan Agner

From: Stefan Agner <[email protected]>

This series adds NXP's Serial Download Protocol (SDP) support via USB for SPL/U-Boot. It allows to download U-Boot via USB from a (recovered) SPL using the same tools used to download SPL itself (specifically imx_usb, but also sb_loader seems to work).

Nice!

Quoted 4 lines

The idea has been brought up when the first targets started to make use of SPL for DDR initialization, see:
https://lists.denx.de/pipermail/u-boot/2015-July/220330.html

Quoted 6 lines

There have been lots of discussions surrounding the use of SDP, and this seems to be a nice alternative to using the i.MX "plugin" mode that I explored a while back:

https://lists.denx.de/pipermail/u-boot/2017-July/thread.html#298266

Hm, wasn't aware of that particular effort, thanks for pointing to it. From a quick glance, it did not work out so far, is this correct?

I looked into plugin mode also a little bit, but I did not continue to look into it after reading this sentence in the i.MX 6 RM:

8.7 Plugin Image
The boot ROM detects the image type using the plugin flag of the boot data structure (see
Boot Data Structure). If the plugin flag is 1, then the ROM uses the image as a plugin
function. The function must initialize the boot device and copy the program image to the
final location. At the end the plugin function must return with the program image
parameters. (See High level boot sequence for details about boot flow).

Quoted 5 lines

So if SPL should work as a plugin as NXP defines it, SPL is supposed to load the image from somewhere. The boot ROM then only takes care about image verification and further boot steps, but it's the plugins job to get the image from somewhere and store it in RAM.

Right, and this is also the weak point. There are also some cases (at least, in some projects of mine) where this conflicts with the setup of the RAM controller. We need to set the RAM controller to load the code or to fight with the limitation of the IRAM.

Quoted 2 lines

Afact this is not really helpful in our case. We would want the boot ROM to go through the boot sequence (again).

Agree. This makes the whole boot process easier to understand - and not to mention the fact that code of BootROM is not officially published, and we cannot know what it exactly does.

Quoted 7 lines

Unless returning an error/invalid image causes the boot ROM to go through boot sequence again?

The nice thing with our own SDP implementation is we can reuse it even from within U-Boot again, e.g. to download a kernel/initramfs....

Right - I think SDP is a nice solution, and thanks for your effort !

Quoted 13 lines
Quoted 13 lines
Quoted 13 lines

The initial SDP implementation (patch 2) requires the payload to have the imx specific headers (hence the move of the imx header file in patch 1).

Patch 3 extends image header support beyond the SDP specification, specifically implements also support for U-Boot headers. This allows to use the same SPL/U-Boot binaries for recovery as used on the regular boot device (SD/eMMC). For that to work also the host side imx_usb tools needed an extension, currently available here:

https://github.com/toradex/imx_loader/tree/imx_usb_batch_mode_refactored

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

The full patchset allows to download SPL and U-Boot over USB to a target in recovery mode using the same usb_imx utility:

imx_usb?

Yeah right, sorry.

But what about to merge into the official imx_usb repo ?

Quoted 24 lines
Quoted 6 lines
Quoted 6 lines

The usb_imx utility also has a batch mode which allows to download multiple artifacts with a single invocation. The details are outlined in the imx_usb commit message:
https://github.com/toradex/imx_loader/commit/5434415d921f1cc4d22332d9558bed6d42db9f60

Quoted 6 lines
Quoted 3 lines

In case this patchset gets accepted in U-Boot, I plan to push the imx_usb changes upstream as well.

Do you have numbers for how much code/data size this adds to SPL?

Besides the protocol implementation also general USB (gadget) support is required, hence it adds quite a bit.

Without USB Gadget/SDP support (Apalis iMX6 SPL):

$ arm-linux-gnueabihf-size spl/u-boot-spl
   text    data     bss     dec     hex filename
  24552    3808      92   28452    6f24 spl/u-boot-spl
Quoted 5 lines

With USB Gadget/SDP support:

$ arm-linux-gnueabihf-size spl/u-boot-spl
   text    data     bss     dec     hex filename
  42549    4679    1984   49212    c03c spl/u-boot-spl

mmmhhh...ok, I see.

Quoted 7 lines
Quoted 4 lines

I believe some i.MX users have struggled to stay within the size of internal RAM.

Hm, I think the limit is somewhere around 64kiB? In our case we are still well below...

I will try to build. The issue is with MX6 Solo (and some lower level MX6) because it has 64KB IRAM and due to BootROM Stack the maximum size for SPL is lower (maybe 48K ?). As far as I see, apalis is MX6Q or MX6D, in both cases no problem. If the size is too big for Solo, we will have to limit SDP's usage to the SOCs with more IRAM (Dual / Quad/ QP..)

Best regards,
Stefano

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#26 lore raw

cover imx: add USB Serial Download Protocol (SDP) support

Hi Stefano

Quoted 118 lines · Stefano Babic

Hi Stefan,

Quoted 56 lines · Stefan Agner

Hi Eric,

Quoted 21 lines · Eric Nelson

Hi Stefan,

Quoted 9 lines · Stefan Agner

From: Stefan Agner <[email protected]>

This series adds NXP's Serial Download Protocol (SDP) support via
USB for SPL/U-Boot. It allows to download U-Boot via USB from a (recovered) SPL using the same tools used to download SPL itself
(specifically imx_usb, but also sb_loader seems to work).

Nice!

Quoted 5 lines

The idea has been brought up when the first targets started to make
use of SPL for DDR initialization, see:
https://lists.denx.de/pipermail/u-boot/2015-July/220330.html

Quoted 8 lines

There have been lots of discussions surrounding the use of SDP, and this seems to be a nice alternative to using the i.MX "plugin"
mode that I explored a while back:

https://lists.denx.de/pipermail/u-boot/2017-July/thread.html#2982
66

Hm, wasn't aware of that particular effort, thanks for pointing to it.
From a quick glance, it did not work out so far, is this correct?

I looked into plugin mode also a little bit, but I did not continue to
look into it after reading this sentence in the i.MX 6 RM:

8.7 Plugin Image
The boot ROM detects the image type using the plugin flag of the boot
data structure (see
Boot Data Structure). If the plugin flag is 1, then the ROM uses the
image as a plugin
function. The function must initialize the boot device and copy the program image to the
final location. At the end the plugin function must return with the program image
parameters. (See High level boot sequence for details about boot flow).

Quoted 8 lines

So if SPL should work as a plugin as NXP defines it, SPL is supposed to
load the image from somewhere. The boot ROM then only takes care about
image verification and further boot steps, but it's the plugins job to
get the image from somewhere and store it in RAM.

Right, and this is also the weak point. There are also some cases (at least, in some projects of mine) where this conflicts with the setup of
the RAM controller. We need to set the RAM controller to load the code
or to fight with the limitation of the IRAM.

Quoted 3 lines

Afact this is not really helpful in our case. We would want the boot ROM
to go through the boot sequence (again).

Agree. This makes the whole boot process easier to understand - and not
to mention the fact that code of BootROM is not officially published, and we cannot know what it exactly does.

Quoted 8 lines

Unless returning an error/invalid image causes the boot ROM to go through boot sequence again?

The nice thing with our own SDP implementation is we can reuse it even
from within U-Boot again, e.g. to download a kernel/initramfs....

Right - I think SDP is a nice solution, and thanks for your effort !

Quoted 23 lines
Quoted 23 lines
Quoted 22 lines

The initial SDP implementation (patch 2) requires the payload to
have the imx specific headers (hence the move of the imx header file in patch 1).

Patch 3 extends image header support beyond the SDP specification,
specifically implements also support for U-Boot headers. This allows to use the same SPL/U-Boot binaries for recovery as used on
the regular boot device (SD/eMMC). For that to work also the host
side imx_usb tools needed an extension, currently available here:

https://github.com/toradex/imx_loader/tree/imx_usb_batch_mode_r
efactored

The full patchset allows to download SPL and U-Boot over USB to a
target in recovery mode using the same usb_imx utility:

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

imx_usb?

Yeah right, sorry.

But what about to merge into the official imx_usb repo ?

Yes, of course as Stefan mentioned before that is planned once this made it into U-Boot proper.

Quoted 41 lines
Quoted 27 lines
Quoted 7 lines
Quoted 7 lines

The usb_imx utility also has a batch mode which allows to download
multiple artifacts with a single invocation. The details are outlined in the imx_usb commit message:
https://github.com/toradex/imx_loader/commit/5434415d921f1cc4d2
2332d9558bed6d42db9f60

Quoted 7 lines
Quoted 4 lines

In case this patchset gets accepted in U-Boot, I plan to push the
imx_usb changes upstream as well.

Do you have numbers for how much code/data size this adds to SPL?

Besides the protocol implementation also general USB (gadget) support is
required, hence it adds quite a bit.

Without USB Gadget/SDP support (Apalis iMX6 SPL):

$ arm-linux-gnueabihf-size spl/u-boot-spl
   text    data     bss     dec     hex filename
  24552    3808      92   28452    6f24 spl/u-boot-spl
Quoted 5 lines

With USB Gadget/SDP support:

$ arm-linux-gnueabihf-size spl/u-boot-spl
   text    data     bss     dec     hex filename
  42549    4679    1984   49212    c03c spl/u-boot-spl

mmmhhh...ok, I see.

Quoted 5 lines
Quoted 4 lines

I believe some i.MX users have struggled to stay within the size of internal RAM.

Quoted 8 lines
Quoted 2 lines

Hm, I think the limit is somewhere around 64kiB? In our case we are still well below...

I will try to build. The issue is with MX6 Solo (and some lower level MX6) because it has 64KB IRAM and due to BootROM Stack the maximum size
for SPL is lower (maybe 48K ?). As far as I see, apalis is MX6Q or MX6D,

We of course also have our Colibri iMX6 with both Solo and DualLite options:

https://www.toradex.com/computer-on-modules/colibri-arm-family/nxp-free
scale-imx6#module_features

Quoted 3 lines

in both cases no problem. If the size is too big for Solo, we will have
to limit SDP's usage to the SOCs with more IRAM (Dual / Quad/ QP..)

No, it really works fine for us across all i.MX 6 variants.

Quoted 2 lines

Best regards,
Stefano

Cheers

Marcel

#27 lore raw

cover imx: add USB Serial Download Protocol (SDP) support

In reply to Stefano Babic
Quoted 60 lines · Stefano Babic

Hi Stefan,

Quoted 46 lines · Stefan Agner

Hi Eric,

Quoted 18 lines · Eric Nelson

Hi Stefan,

Quoted 7 lines · Stefan Agner

From: Stefan Agner <[email protected]>

This series adds NXP's Serial Download Protocol (SDP) support via USB for SPL/U-Boot. It allows to download U-Boot via USB from a (recovered) SPL using the same tools used to download SPL itself (specifically imx_usb, but also sb_loader seems to work).

Nice!

Quoted 4 lines

The idea has been brought up when the first targets started to make use of SPL for DDR initialization, see:
https://lists.denx.de/pipermail/u-boot/2015-July/220330.html

Quoted 6 lines

There have been lots of discussions surrounding the use of SDP, and this seems to be a nice alternative to using the i.MX "plugin" mode that I explored a while back:

https://lists.denx.de/pipermail/u-boot/2017-July/thread.html#298266

Hm, wasn't aware of that particular effort, thanks for pointing to it. From a quick glance, it did not work out so far, is this correct?

I looked into plugin mode also a little bit, but I did not continue to look into it after reading this sentence in the i.MX 6 RM:

8.7 Plugin Image
The boot ROM detects the image type using the plugin flag of the boot data structure (see
Boot Data Structure). If the plugin flag is 1, then the ROM uses the image as a plugin
function. The function must initialize the boot device and copy the program image to the
final location. At the end the plugin function must return with the program image
parameters. (See High level boot sequence for details about boot flow).

Quoted 5 lines

So if SPL should work as a plugin as NXP defines it, SPL is supposed to load the image from somewhere. The boot ROM then only takes care about image verification and further boot steps, but it's the plugins job to get the image from somewhere and store it in RAM.

Right, and this is also the weak point. There are also some cases (at least, in some projects of mine) where this conflicts with the setup of the RAM controller. We need to set the RAM controller to load the code or to fight with the limitation of the IRAM.

Quoted 7 lines
Quoted 2 lines

Afact this is not really helpful in our case. We would want the boot ROM to go through the boot sequence (again).

Agree. This makes the whole boot process easier to understand - and not to mention the fact that code of BootROM is not officially published, and we cannot know what it exactly does.

Yeah, I really wish NXP would add some functionality and document the boot ROM clearly.

One issue I have currently is when the SPL gets downloaded via USB serial download when the SoC entered serial download due to unbootable device, spl_boot_mode cannot detect that.

Downstream uses "circumstantial evidence" to get this info:
http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/tree/arch/arm/include/asm/arch-mx7/imx-regs.h?h=imx_v2016.03_4.1.15_2.0.0_ga#n1204

And I just learned that i.MX 7 Boot ROM actually provides a structure with boot info now, see get_boot_device in arch/arm/mach-imx/mx7/soc.c.

If anybody has a reliable/nicer mechanism than the is_boot_from_usb from downstream for i.MX 6 that I am all ears.

Quoted 26 lines
Quoted 7 lines

Unless returning an error/invalid image causes the boot ROM to go through boot sequence again?

The nice thing with our own SDP implementation is we can reuse it even from within U-Boot again, e.g. to download a kernel/initramfs....

Right - I think SDP is a nice solution, and thanks for your effort !

Quoted 16 lines
Quoted 16 lines
Quoted 15 lines

The initial SDP implementation (patch 2) requires the payload to have the imx specific headers (hence the move of the imx header file in patch 1).

Patch 3 extends image header support beyond the SDP specification, specifically implements also support for U-Boot headers. This allows to use the same SPL/U-Boot binaries for recovery as used on the regular boot device (SD/eMMC). For that to work also the host side imx_usb tools needed an extension, currently available here:

https://github.com/toradex/imx_loader/tree/imx_usb_batch_mode_refactored

The full patchset allows to download SPL and U-Boot over USB to a target in recovery mode using the same usb_imx utility:

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

imx_usb?

Yeah right, sorry.

But what about to merge into the official imx_usb repo ?

FWIW, many preparation patches actually already made it upstream:
https://github.com/boundarydevices/imx_usb_loader/pull/48

The pending patches are the changes which only make sense in case upstream U-Boot gains support for this (e.g. U-Boot header support for imx_usb).

It probably makes sense for reviewers/testers to look at those patches too.

Especially the batch mode:
https://github.com/toradex/imx_loader/commit/5434415d921f1cc4d22332d9558bed6d42db9f60

I was going to create the pull request after the U-Boot part gets merged, but if preferred, I can create a pull request immediately so people can review it over at Github?

Quoted 38 lines
Quoted 24 lines
Quoted 6 lines
Quoted 6 lines

The usb_imx utility also has a batch mode which allows to download multiple artifacts with a single invocation. The details are outlined in the imx_usb commit message:
https://github.com/toradex/imx_loader/commit/5434415d921f1cc4d22332d9558bed6d42db9f60

Quoted 6 lines
Quoted 3 lines

In case this patchset gets accepted in U-Boot, I plan to push the imx_usb changes upstream as well.

Do you have numbers for how much code/data size this adds to SPL?

Besides the protocol implementation also general USB (gadget) support is required, hence it adds quite a bit.

Without USB Gadget/SDP support (Apalis iMX6 SPL):

$ arm-linux-gnueabihf-size spl/u-boot-spl
   text    data     bss     dec     hex filename
  24552    3808      92   28452    6f24 spl/u-boot-spl
Quoted 5 lines

With USB Gadget/SDP support:

$ arm-linux-gnueabihf-size spl/u-boot-spl
   text    data     bss     dec     hex filename
  42549    4679    1984   49212    c03c spl/u-boot-spl

mmmhhh...ok, I see.

Quoted 5 lines
Quoted 4 lines

I believe some i.MX users have struggled to stay within the size of internal RAM.

Quoted 8 lines
Quoted 2 lines

Hm, I think the limit is somewhere around 64kiB? In our case we are still well below...

I will try to build. The issue is with MX6 Solo (and some lower level MX6) because it has 64KB IRAM and due to BootROM Stack the maximum size for SPL is lower (maybe 48K ?). As far as I see, apalis is MX6Q or MX6D, in both cases no problem. If the size is too big for Solo, we will have to limit SDP's usage to the SOCs with more IRAM (Dual / Quad/ QP..)

As Marcel mentioned, we successfully run it on Solo/DualLite too.

Boot ROM stack? Since it is not running as plugin, can we not just trash that?

--
Stefan

#28 lore raw

cover imx: add USB Serial Download Protocol (SDP) support

In reply to Stefan Agner

Hi Stefan,

Quoted 21 lines · Stefan Agner

Hi Eric,

Quoted 18 lines · Eric Nelson

Hi Stefan,

Quoted 7 lines · Stefan Agner

From: Stefan Agner <[email protected]>

This series adds NXP's Serial Download Protocol (SDP) support via USB for SPL/U-Boot. It allows to download U-Boot via USB from a (recovered) SPL using the same tools used to download SPL itself (specifically imx_usb, but also sb_loader seems to work).

Nice!

Quoted 4 lines

The idea has been brought up when the first targets started to make use of SPL for DDR initialization, see:
https://lists.denx.de/pipermail/u-boot/2015-July/220330.html

Quoted 9 lines
Quoted 6 lines

There have been lots of discussions surrounding the use of SDP, and this seems to be a nice alternative to using the i.MX "plugin" mode that I explored a while back:

https://lists.denx.de/pipermail/u-boot/2017-July/thread.html#298266

Hm, wasn't aware of that particular effort, thanks for pointing to it.

Quoted 1 line

From a quick glance, it did not work out so far, is this correct?

That's right. I believe that the trouble is in the return-to-ROM process. I hacked together a form of "setjmp/longjmp" to try and get it to work, but wasn't successful and the documentation for the entry points had me confused.

Quoted 4 lines

I looked into plugin mode also a little bit, but I did not continue to look into it after reading this sentence in the i.MX 6 RM:

Quoted 11 lines

8.7 Plugin Image
The boot ROM detects the image type using the plugin flag of the boot data structure (see
Boot Data Structure). If the plugin flag is 1, then the ROM uses the image as a plugin
function. The function must initialize the boot device and copy the program image to the
final location. At the end the plugin function must return with the program image
parameters. (See High level boot sequence for details about boot flow).

Hmm. That doesn't seem to match the existing code in the NXP U-Boot.

Quoted 6 lines

So if SPL should work as a plugin as NXP defines it, SPL is supposed to load the image from somewhere. The boot ROM then only takes care about image verification and further boot steps, but it's the plugins job to get the image from somewhere and store it in RAM.

I think the documentation is just misleading, as NXP is using SPL to load the payload in stages. The ROM is loading the portion that goes into RAM after executing the plugin to initialize the DDR controller (and PMIC if needed).

Quoted 3 lines

Afact this is not really helpful in our case. We would want the boot ROM to go through the boot sequence (again).

Not the full boot sequence. We'd just want it to load the rest of the image into external RAM.

Quoted 3 lines

Unless returning an error/invalid image causes the boot ROM to go through boot sequence again?

I'm not clear on how errors are handled.

Quoted 3 lines

The nice thing with our own SDP implementation is we can reuse it even from within U-Boot again, e.g. to download a kernel/initramfs....

There are lots of nice things about having SDP implemented in open-source "C" code and I'm inclined to give up on worrying about "plugin" mode, at least for now.

My primary rationale for trying to get plugins to work was to prevent the need to have a "full" U-Boot for initial programming of eMMC.

There is one other use case for plugins though, and that's the "resume from LPSR" on i.MX7. In this case, RAM already has a running kernel loaded, but the LPDDR is in self-refresh and something needs to detect that during boot and return after restoring the DDR registers.

Quoted 47 lines
Quoted 10 lines
Quoted 10 lines

The initial SDP implementation (patch 2) requires the payload to have the imx specific headers (hence the move of the imx header file in patch 1).

Patch 3 extends image header support beyond the SDP specification, specifically implements also support for U-Boot headers. This allows to use the same SPL/U-Boot binaries for recovery as used on the regular boot device (SD/eMMC). For that to work also the host side imx_usb tools needed an extension, currently available here:

Quoted 7 lines
Quoted 4 lines

https://github.com/toradex/imx_loader/tree/imx_usb_batch_mode_refactored

The full patchset allows to download SPL and U-Boot over USB to a target in recovery mode using the same usb_imx utility:

imx_usb?

Yeah right, sorry.

Quoted 11 lines
Quoted 8 lines

The usb_imx utility also has a batch mode which allows to download multiple artifacts with a single invocation. The details are outlined in the imx_usb commit message:
https://github.com/toradex/imx_loader/commit/5434415d921f1cc4d22332d9558bed6d42db9f60

In case this patchset gets accepted in U-Boot, I plan to push the imx_usb changes upstream as well.

Do you have numbers for how much code/data size this adds to SPL?

Besides the protocol implementation also general USB (gadget) support is required, hence it adds quite a bit.

Without USB Gadget/SDP support (Apalis iMX6 SPL):

$ arm-linux-gnueabihf-size spl/u-boot-spl
    text    data     bss     dec     hex filename
   24552    3808      92   28452    6f24 spl/u-boot-spl

With USB Gadget/SDP support:

$ arm-linux-gnueabihf-size spl/u-boot-spl
    text    data     bss     dec     hex filename
   42549    4679    1984   49212    c03c spl/u-boot-spl
Quoted 6 lines
Quoted 2 lines

I believe some i.MX users have struggled to stay within the size of internal RAM.

Hm, I think the limit is somewhere around 64kiB? In our case we are still well below...

I think these numbers will be a problem for some use cases.

Someone on the list (Tim Harvey perhaps?) has been struggling to keep a unified U-Boot under the IRAM limit and would need to choose between supporting lots of different hardware and boot options and using SPL with SDP.

Regards,

Eric

#29 lore raw

cover imx: add USB Serial Download Protocol (SDP) support

In reply to Stefan Agner

Hi Stefan,

Quoted 6 lines

From: Stefan Agner <[email protected]>

This series adds NXP's Serial Download Protocol (SDP) support via USB for SPL/U-Boot. It allows to download U-Boot via USB from a (recovered) SPL using the same tools used to download SPL itself (specifically imx_usb, but also sb_loader seems to work).

If I might ask - could you prepare some ./doc/README.sdp entry for v2?

I mean some kind of howto for using this feature?

Nothing long - just the most important commands (imx_usb?) including the list of supported (and not) features.

Quoted 20 lines

The idea has been brought up when the first targets started to make use of SPL for DDR initialization, see:
https://lists.denx.de/pipermail/u-boot/2015-July/220330.html

The initial SDP implementation (patch 2) requires the payload to have the imx specific headers (hence the move of the imx header file in patch 1).

Patch 3 extends image header support beyond the SDP specification, specifically implements also support for U-Boot headers. This allows to use the same SPL/U-Boot binaries for recovery as used on the regular boot device (SD/eMMC). For that to work also the host side imx_usb tools needed an extension, currently available here:

https://github.com/toradex/imx_loader/tree/imx_usb_batch_mode_refactored

The full patchset allows to download SPL and U-Boot over USB to a target in recovery mode using the same usb_imx utility:

Quoted 7 lines

The usb_imx utility also has a batch mode which allows to download multiple artifacts with a single invocation. The details are outlined in the imx_usb commit message:
https://github.com/toradex/imx_loader/commit/5434415d921f1cc4d22332d9558bed6d42db9f60

In case this patchset gets accepted in U-Boot, I plan to push the imx_usb changes upstream as well.

Thanks for the patch set. It would definitely facilitate work with IMX SoCs.

Quoted 33 lines

Stefan Agner (7):
imx: move imximage header to common location
usb: gadget: add SDP driver
usb: gadget: sdp: extend images compatible for jumps
cmd: add sdp command
spl: add serial download protocol (SDP) support apalis/colibri_imx6: use independent USB PID for SPL apalis/colibri_imx6: enable SDP by default

 board/toradex/apalis_imx6/apalis_imx6.c   |  13 +
 board/toradex/colibri_imx6/colibri_imx6.c |  13 +
 cmd/Kconfig                               |   7 +
 cmd/Makefile                              |   1 +
 cmd/usb_gadget_sdp.c                      |  53 +++
 common/spl/Kconfig                        |   6 +
 common/spl/Makefile                       |   1 +
 common/spl/spl_sdp.c                      |  38 ++
 configs/apalis_imx6_defconfig             |   4 +
 configs/colibri_imx6_defconfig            |   4 +
 drivers/usb/gadget/Kconfig                |   7 +
 drivers/usb/gadget/Makefile               |   2 +
 drivers/usb/gadget/f_sdp.c                | 739 ++++++++++++++++++++++++++++++
 {tools => include}/imximage.h             |   0
 include/sdp.h                             |  16 +
 15 files changed, 904 insertions(+)
 create mode 100644 cmd/usb_gadget_sdp.c
 create mode 100644 common/spl/spl_sdp.c
 create mode 100644 drivers/usb/gadget/f_sdp.c
 rename {tools => include}/imximage.h (100%)
 create mode 100644 include/sdp.h
Signature · 9 lines
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de