diff mbox series

[U-Boot,RFC,v2,1/2] usb: fastboot: Convert USB f_fastboot to shared fastboot

Message ID 1526573624-6433-2-git-send-email-alex.kiernan@gmail.com
State RFC
Delegated to: Lukasz Majewski
Headers show
Series Convert USB fastboot code to use shared protocol | expand

Commit Message

Alex Kiernan May 17, 2018, 4:13 p.m. UTC
Convert USB fastboot code to use shared fastboot protocol.

Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
---

Changes in v2:
- remove redundant version.h
- use new fastboot_get_bytes_remaining() function

 drivers/fastboot/Makefile       |   4 +-
 drivers/usb/gadget/f_fastboot.c | 318 +++++-----------------------------------
 2 files changed, 37 insertions(+), 285 deletions(-)

Comments

Sam Protsenko May 18, 2018, 6:32 p.m. UTC | #1
On 17 May 2018 at 19:13, Alex Kiernan <alex.kiernan@gmail.com> wrote:
> Convert USB fastboot code to use shared fastboot protocol.
>
> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
> ---
>
> Changes in v2:
> - remove redundant version.h
> - use new fastboot_get_bytes_remaining() function
>
>  drivers/fastboot/Makefile       |   4 +-
>  drivers/usb/gadget/f_fastboot.c | 318 +++++-----------------------------------
>  2 files changed, 37 insertions(+), 285 deletions(-)
>

Hi Alex,

Unfortunately this patch breaks regular fastboot (over USB). At least
on TI BeagleBoard X15. I tested your patch series using
https://github.com/akiernan/u-boot.git, on branch us-fastboot-udp-v6
(hope it contains v2 of this patch). I'm unable to do "fastboot flash"
or "fastboot oem format" commands after this patch. Below some debug
output that can help you debug this issue.

On host side I see:

    FAILED (remote: unrecognized command)

On device side next messages appear:

    ** Unrecognized filesystem type **
    buffer overflowstatus: -104 ep 'ep1in-bulk' trans: 0
    command es not recognized.

I'd appreciate if you can fix it in next series. Please add me to Cc:
so that I can test it on my boards and verify if it's work.

Also, as I understand, your github branch contains patches from
different patch series. If you can split them into corresponding
branches there, I can test your patch series separately and provide
tested tag.

Thanks.
Alex Kiernan May 18, 2018, 9:42 p.m. UTC | #2
Hi Sam
On Fri, May 18, 2018 at 7:32 PM Sam Protsenko <semen.protsenko@linaro.org>
wrote:

> On 17 May 2018 at 19:13, Alex Kiernan <alex.kiernan@gmail.com> wrote:
> > Convert USB fastboot code to use shared fastboot protocol.
> >
> > Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
> > ---
> >
> > Changes in v2:
> > - remove redundant version.h
> > - use new fastboot_get_bytes_remaining() function
> >
> >  drivers/fastboot/Makefile       |   4 +-
> >  drivers/usb/gadget/f_fastboot.c | 318
+++++-----------------------------------
> >  2 files changed, 37 insertions(+), 285 deletions(-)
> >

> Hi Alex,

> Unfortunately this patch breaks regular fastboot (over USB). At least
> on TI BeagleBoard X15. I tested your patch series using
> https://github.com/akiernan/u-boot.git, on branch us-fastboot-udp-v6
> (hope it contains v2 of this patch). I'm unable to do "fastboot flash"
> or "fastboot oem format" commands after this patch. Below some debug
> output that can help you debug this issue.


Do you remember if you did the 'oem format' after the 'flash' command? If
so, likely it's got out of sync and the 'oem format' was an innocent victim
after the 'flash' failed...

There's definitely a change in the data transfer which looks like it should
be fine, but I can imagine I've not got quite right.

> On host side I see:

>      FAILED (remote: unrecognized command)

> On device side next messages appear:

>      ** Unrecognized filesystem type **
>      buffer overflowstatus: -104 ep 'ep1in-bulk' trans: 0
>      command es not recognized.

> I'd appreciate if you can fix it in next series. Please add me to Cc:
> so that I can test it on my boards and verify if it's work.

> Also, as I understand, your github branch contains patches from
> different patch series. If you can split them into corresponding
> branches there, I can test your patch series separately and provide
> tested tag.


I've split off the last two patches, so there's just this series which adds
UDP fastboot in that branch now and has only simple refactorings in the USB
path. If you're able to test just that to see if it works?
Alex Kiernan May 19, 2018, 1:32 p.m. UTC | #3
On Fri, May 18, 2018 at 10:42 PM Alex Kiernan <alex.kiernan@gmail.com>
wrote:

> Hi Sam
> On Fri, May 18, 2018 at 7:32 PM Sam Protsenko <semen.protsenko@linaro.org>
> wrote:

> > On 17 May 2018 at 19:13, Alex Kiernan <alex.kiernan@gmail.com> wrote:
> > > Convert USB fastboot code to use shared fastboot protocol.
> > >
> > > Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
> > > ---
> > >
> > > Changes in v2:
> > > - remove redundant version.h
> > > - use new fastboot_get_bytes_remaining() function
> > >
> > >  drivers/fastboot/Makefile       |   4 +-
> > >  drivers/usb/gadget/f_fastboot.c | 318
> +++++-----------------------------------
> > >  2 files changed, 37 insertions(+), 285 deletions(-)
> > >

> > Hi Alex,

> > Unfortunately this patch breaks regular fastboot (over USB). At least
> > on TI BeagleBoard X15. I tested your patch series using
> > https://github.com/akiernan/u-boot.git, on branch us-fastboot-udp-v6
> > (hope it contains v2 of this patch). I'm unable to do "fastboot flash"
> > or "fastboot oem format" commands after this patch. Below some debug
> > output that can help you debug this issue.


> Do you remember if you did the 'oem format' after the 'flash' command? If
> so, likely it's got out of sync and the 'oem format' was an innocent
victim
> after the 'flash' failed...

> There's definitely a change in the data transfer which looks like it
should
> be fine, but I can imagine I've not got quite right.


I can see what I've done wrong... the transition into the USB download
phase will never actually execute as I've put it inside an impossible
conditional :(

I've pushed us-fastboot-udp-v6 which is everything apart from the USB
migration, the USB migration (including this fix) is on us-fastboot-usb-v3
diff mbox series

Patch

diff --git a/drivers/fastboot/Makefile b/drivers/fastboot/Makefile
index 8831096..a242156 100644
--- a/drivers/fastboot/Makefile
+++ b/drivers/fastboot/Makefile
@@ -1,7 +1,7 @@ 
 # SPDX-License-Identifier:      GPL-2.0+
 
 obj-y += fb_common.o
-obj-$(CONFIG_UDP_FUNCTION_FASTBOOT) += fb_getvar.o
-obj-$(CONFIG_UDP_FUNCTION_FASTBOOT) += fb_command.o
+obj-y += fb_getvar.o
+obj-y += fb_command.o
 obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fb_mmc.o
 obj-$(CONFIG_FASTBOOT_FLASH_NAND) += fb_nand.o
diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index 07d6a62..4a87dc5 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -18,14 +18,7 @@ 
 #include <linux/usb/gadget.h>
 #include <linux/usb/composite.h>
 #include <linux/compiler.h>
-#include <version.h>
 #include <g_dnl.h>
-#ifdef CONFIG_FASTBOOT_FLASH_MMC
-#include <fb_mmc.h>
-#endif
-#ifdef CONFIG_FASTBOOT_FLASH_NAND
-#include <fb_nand.h>
-#endif
 
 #define FASTBOOT_INTERFACE_CLASS	0xff
 #define FASTBOOT_INTERFACE_SUB_CLASS	0x42
@@ -56,8 +49,6 @@  static inline struct f_fastboot *func_to_fastboot(struct usb_function *f)
 }
 
 static struct f_fastboot *fastboot_func;
-static unsigned int download_size;
-static unsigned int download_bytes;
 
 static struct usb_endpoint_descriptor fs_ep_in = {
 	.bLength            = USB_DT_ENDPOINT_SIZE,
@@ -145,7 +136,6 @@  static struct usb_gadget_strings *fastboot_strings[] = {
 };
 
 static void rx_handler_command(struct usb_ep *ep, struct usb_request *req);
-static int strcmp_l1(const char *s1, const char *s2);
 
 static void fastboot_complete(struct usb_ep *ep, struct usb_request *req)
 {
@@ -355,85 +345,9 @@  static void compl_do_reset(struct usb_ep *ep, struct usb_request *req)
 	do_reset(NULL, 0, 0, NULL);
 }
 
-static void cb_reboot(struct usb_ep *ep, struct usb_request *req)
-{
-	char *cmd = req->buf;
-	if (!strcmp_l1("reboot-bootloader", cmd)) {
-		if (fastboot_set_reboot_flag()) {
-			fastboot_tx_write_str("FAILCannot set reboot flag");
-			return;
-		}
-	}
-	fastboot_func->in_req->complete = compl_do_reset;
-	fastboot_tx_write_str("OKAY");
-}
-
-static int strcmp_l1(const char *s1, const char *s2)
-{
-	if (!s1 || !s2)
-		return -1;
-	return strncmp(s1, s2, strlen(s1));
-}
-
-static void cb_getvar(struct usb_ep *ep, struct usb_request *req)
-{
-	char *cmd = req->buf;
-	char response[FASTBOOT_RESPONSE_LEN];
-	const char *s;
-	size_t chars_left;
-
-	strcpy(response, "OKAY");
-	chars_left = sizeof(response) - strlen(response) - 1;
-
-	strsep(&cmd, ":");
-	if (!cmd) {
-		pr_err("missing variable");
-		fastboot_tx_write_str("FAILmissing var");
-		return;
-	}
-
-	if (!strcmp_l1("version", cmd)) {
-		strncat(response, FASTBOOT_VERSION, chars_left);
-	} else if (!strcmp_l1("bootloader-version", cmd)) {
-		strncat(response, U_BOOT_VERSION, chars_left);
-	} else if (!strcmp_l1("downloadsize", cmd) ||
-		!strcmp_l1("max-download-size", cmd)) {
-		char str_num[12];
-
-		sprintf(str_num, "0x%08x", CONFIG_FASTBOOT_BUF_SIZE);
-		strncat(response, str_num, chars_left);
-	} else if (!strcmp_l1("serialno", cmd)) {
-		s = env_get("serial#");
-		if (s)
-			strncat(response, s, chars_left);
-		else
-			strcpy(response, "FAILValue not set");
-	} else {
-		char *envstr;
-
-		envstr = malloc(strlen("fastboot.") + strlen(cmd) + 1);
-		if (!envstr) {
-			fastboot_tx_write_str("FAILmalloc error");
-			return;
-		}
-
-		sprintf(envstr, "fastboot.%s", cmd);
-		s = env_get(envstr);
-		if (s) {
-			strncat(response, s, chars_left);
-		} else {
-			printf("WARNING: unknown variable: %s\n", cmd);
-			strcpy(response, "FAILVariable not implemented");
-		}
-
-		free(envstr);
-	}
-	fastboot_tx_write_str(response);
-}
-
 static unsigned int rx_bytes_expected(struct usb_ep *ep)
 {
-	int rx_remain = download_size - download_bytes;
+	int rx_remain = fastboot_get_bytes_remaining();
 	unsigned int rem;
 	unsigned int maxpacket = ep->maxpacket;
 
@@ -455,14 +369,12 @@  static unsigned int rx_bytes_expected(struct usb_ep *ep)
 	return rx_remain;
 }
 
-#define BYTES_PER_DOT	0x20000
 static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req)
 {
-	char response[FASTBOOT_RESPONSE_LEN];
-	unsigned int transfer_size = download_size - download_bytes;
+	char response[FASTBOOT_RESPONSE_LEN] = {0};
+	unsigned int transfer_size = fastboot_get_bytes_remaining();
 	const unsigned char *buffer = req->buf;
 	unsigned int buffer_size = req->actual;
-	unsigned int pre_dot_num, now_dot_num;
 
 	if (req->status != 0) {
 		printf("Bad status: %d\n", req->status);
@@ -472,33 +384,15 @@  static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req)
 	if (buffer_size < transfer_size)
 		transfer_size = buffer_size;
 
-	memcpy((void *)CONFIG_FASTBOOT_BUF_ADDR + download_bytes,
-	       buffer, transfer_size);
-
-	pre_dot_num = download_bytes / BYTES_PER_DOT;
-	download_bytes += transfer_size;
-	now_dot_num = download_bytes / BYTES_PER_DOT;
-
-	if (pre_dot_num != now_dot_num) {
-		putc('.');
-		if (!(now_dot_num % 74))
-			putc('\n');
-	}
-
-	/* Check if transfer is done */
-	if (download_bytes >= download_size) {
+	fastboot_download_data(buffer, transfer_size, response);
+	if (!strncmp("OKAY", response, 4) || !strncmp("FAIL", response, 4)) {
 		/*
-		 * Reset global transfer variable, keep download_bytes because
-		 * it will be used in the next possible flashing command
+		 * Reset global transfer variable
 		 */
-		download_size = 0;
 		req->complete = rx_handler_command;
 		req->length = EP_BUFFER_SIZE;
 
-		strcpy(response, "OKAY");
 		fastboot_tx_write_str(response);
-
-		printf("\ndownloading of %d bytes finished\n", download_bytes);
 	} else {
 		req->length = rx_bytes_expected(ep);
 	}
@@ -507,197 +401,55 @@  static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req)
 	usb_ep_queue(ep, req, 0);
 }
 
-static void cb_download(struct usb_ep *ep, struct usb_request *req)
-{
-	char *cmd = req->buf;
-	char response[FASTBOOT_RESPONSE_LEN];
-
-	strsep(&cmd, ":");
-	download_size = simple_strtoul(cmd, NULL, 16);
-	download_bytes = 0;
-
-	printf("Starting download of %d bytes\n", download_size);
-
-	if (0 == download_size) {
-		strcpy(response, "FAILdata invalid size");
-	} else if (download_size > CONFIG_FASTBOOT_BUF_SIZE) {
-		download_size = 0;
-		strcpy(response, "FAILdata too large");
-	} else {
-		sprintf(response, "DATA%08x", download_size);
-		req->complete = rx_handler_dl_image;
-		req->length = rx_bytes_expected(ep);
-	}
-	fastboot_tx_write_str(response);
-}
-
-static void do_bootm_on_complete(struct usb_ep *ep, struct usb_request *req)
-{
-	char boot_addr_start[12];
-	char *bootm_args[] = { "bootm", boot_addr_start, NULL };
-
-	puts("Booting kernel..\n");
-
-	sprintf(boot_addr_start, "0x%lx", (long)CONFIG_FASTBOOT_BUF_ADDR);
-	do_bootm(NULL, 0, 2, bootm_args);
-
-	/* This only happens if image is somehow faulty so we start over */
-	do_reset(NULL, 0, 0, NULL);
-}
-
-static void cb_boot(struct usb_ep *ep, struct usb_request *req)
-{
-	fastboot_func->in_req->complete = do_bootm_on_complete;
-	fastboot_tx_write_str("OKAY");
-}
-
 static void do_exit_on_complete(struct usb_ep *ep, struct usb_request *req)
 {
 	g_dnl_trigger_detach();
 }
 
-static void cb_continue(struct usb_ep *ep, struct usb_request *req)
+static void do_bootm_on_complete(struct usb_ep *ep, struct usb_request *req)
 {
-	fastboot_func->in_req->complete = do_exit_on_complete;
-	fastboot_tx_write_str("OKAY");
+	fastboot_boot();
+	do_exit_on_complete(ep, req);
 }
 
-#ifdef CONFIG_FASTBOOT_FLASH
-static void cb_flash(struct usb_ep *ep, struct usb_request *req)
+static void rx_handler_command(struct usb_ep *ep, struct usb_request *req)
 {
-	char *cmd = req->buf;
-	char response[FASTBOOT_RESPONSE_LEN];
+	char *cmdbuf = req->buf;
+	char response[FASTBOOT_RESPONSE_LEN] = {0};
+	int cmd = -1;
 
-	strsep(&cmd, ":");
-	if (!cmd) {
-		pr_err("missing partition name");
-		fastboot_tx_write_str("FAILmissing partition name");
+	if (req->status != 0 || req->length == 0)
 		return;
-	}
-
-	fastboot_fail("no flash device defined", response);
-#ifdef CONFIG_FASTBOOT_FLASH_MMC
-	fastboot_mmc_flash_write(cmd, (void *)CONFIG_FASTBOOT_BUF_ADDR,
-				 download_bytes, response);
-#endif
-#ifdef CONFIG_FASTBOOT_FLASH_NAND
-	fastboot_nand_flash_write(cmd, (void *)CONFIG_FASTBOOT_BUF_ADDR,
-				  download_bytes, response);
-#endif
-	fastboot_tx_write_str(response);
-}
-#endif
-
-static void cb_oem(struct usb_ep *ep, struct usb_request *req)
-{
-	char *cmd = req->buf;
-#ifdef CONFIG_FASTBOOT_FLASH_MMC
-	if (strncmp("format", cmd + 4, 6) == 0) {
-		char cmdbuf[32];
-                sprintf(cmdbuf, "gpt write mmc %x $partitions",
-			CONFIG_FASTBOOT_FLASH_MMC_DEV);
-                if (run_command(cmdbuf, 0))
-			fastboot_tx_write_str("FAIL");
-                else
-			fastboot_tx_write_str("OKAY");
-	} else
-#endif
-	if (strncmp("unlock", cmd + 4, 8) == 0) {
-		fastboot_tx_write_str("FAILnot implemented");
-	}
-	else {
-		fastboot_tx_write_str("FAILunknown oem command");
-	}
-}
-
-#ifdef CONFIG_FASTBOOT_FLASH
-static void cb_erase(struct usb_ep *ep, struct usb_request *req)
-{
-	char *cmd = req->buf;
-	char response[FASTBOOT_RESPONSE_LEN];
 
-	strsep(&cmd, ":");
-	if (!cmd) {
-		pr_err("missing partition name");
-		fastboot_tx_write_str("FAILmissing partition name");
-		return;
+	if (req->actual < req->length) {
+		cmdbuf[req->actual] = '\0';
+		cmd = fastboot_handle_command(cmdbuf, response);
+	} else {
+		pr_err("buffer overflow");
+		fastboot_fail("buffer overflow", response);
 	}
 
-	fastboot_fail("no flash device defined", response);
-#ifdef CONFIG_FASTBOOT_FLASH_MMC
-	fastboot_mmc_erase(cmd, response);
-#endif
-#ifdef CONFIG_FASTBOOT_FLASH_NAND
-	fastboot_nand_erase(cmd, response);
-#endif
 	fastboot_tx_write_str(response);
-}
-#endif
 
-struct cmd_dispatch_info {
-	char *cmd;
-	void (*cb)(struct usb_ep *ep, struct usb_request *req);
-};
-
-static const struct cmd_dispatch_info cmd_dispatch_info[] = {
-	{
-		.cmd = "reboot",
-		.cb = cb_reboot,
-	}, {
-		.cmd = "getvar:",
-		.cb = cb_getvar,
-	}, {
-		.cmd = "download:",
-		.cb = cb_download,
-	}, {
-		.cmd = "boot",
-		.cb = cb_boot,
-	}, {
-		.cmd = "continue",
-		.cb = cb_continue,
-	},
-#ifdef CONFIG_FASTBOOT_FLASH
-	{
-		.cmd = "flash",
-		.cb = cb_flash,
-	}, {
-		.cmd = "erase",
-		.cb = cb_erase,
-	},
-#endif
-	{
-		.cmd = "oem",
-		.cb = cb_oem,
-	},
-};
-
-static void rx_handler_command(struct usb_ep *ep, struct usb_request *req)
-{
-	char *cmdbuf = req->buf;
-	void (*func_cb)(struct usb_ep *ep, struct usb_request *req) = NULL;
-	int i;
+	if (!strncmp("OKAY", response, 4)) {
+		switch (cmd) {
+		case FASTBOOT_COMMAND_BOOT:
+			fastboot_func->in_req->complete = do_bootm_on_complete;
+			break;
 
-	if (req->status != 0 || req->length == 0)
-		return;
+		case FASTBOOT_COMMAND_CONTINUE:
+			fastboot_func->in_req->complete = do_exit_on_complete;
+			break;
 
-	for (i = 0; i < ARRAY_SIZE(cmd_dispatch_info); i++) {
-		if (!strcmp_l1(cmd_dispatch_info[i].cmd, cmdbuf)) {
-			func_cb = cmd_dispatch_info[i].cb;
+		case FASTBOOT_COMMAND_REBOOT:
+		case FASTBOOT_COMMAND_REBOOT_BOOTLOADER:
+			fastboot_func->in_req->complete = compl_do_reset;
 			break;
-		}
-	}
 
-	if (!func_cb) {
-		pr_err("unknown command: %.*s", req->actual, cmdbuf);
-		fastboot_tx_write_str("FAILunknown command");
-	} else {
-		if (req->actual < req->length) {
-			u8 *buf = (u8 *)req->buf;
-			buf[req->actual] = 0;
-			func_cb(ep, req);
-		} else {
-			pr_err("buffer overflow");
-			fastboot_tx_write_str("FAILbuffer overflow");
+		case FASTBOOT_COMMAND_DOWNLOAD:
+			req->complete = rx_handler_dl_image;
+			req->length = rx_bytes_expected(ep);
+			break;
 		}
 	}