diff mbox series

[U-Boot,RFC,v2,20/20] fastboot: net: Split fastboot protocol out from net

Message ID 1525077174-6211-21-git-send-email-alex.kiernan@gmail.com
State RFC
Delegated to: Lukasz Majewski
Headers show
Series Add fastboot UDP support | expand

Commit Message

Alex Kiernan April 30, 2018, 8:32 a.m. UTC
Separate the fastboot protocol handling from the fastboot UDP code in
preparation for reusing it in the USB code.

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

Changes in v2: None

 drivers/fastboot/Makefile    |   1 +
 drivers/fastboot/fb_packet.c | 249 +++++++++++++++++++++++++++++++++++++++++++
 include/fastboot.h           |   3 +
 net/fastboot.c               | 234 +---------------------------------------
 4 files changed, 258 insertions(+), 229 deletions(-)
 create mode 100644 drivers/fastboot/fb_packet.c

Comments

Joe Hershberger May 3, 2018, 9:29 p.m. UTC | #1
On Mon, Apr 30, 2018 at 3:32 AM, Alex Kiernan <alex.kiernan@gmail.com> wrote:
> Separate the fastboot protocol handling from the fastboot UDP code in
> preparation for reusing it in the USB code.
>
> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>

This is the last patch, yet you are preparing for something to follow,
so I'll review when you get to that point.

Cheers,
-Joe
Alex Kiernan May 4, 2018, 6:05 a.m. UTC | #2
On Thu, May 3, 2018 at 10:30 PM Joe Hershberger <joe.hershberger@ni.com>
wrote:

> On Mon, Apr 30, 2018 at 3:32 AM, Alex Kiernan <alex.kiernan@gmail.com>
wrote:
> > Separate the fastboot protocol handling from the fastboot UDP code in
> > preparation for reusing it in the USB code.
> >
> > Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>

> This is the last patch, yet you are preparing for something to follow,
> so I'll review when you get to that point.


Thanks for all your reviewing efforts so far.

Stopping here, was clearly the right choice as now I'm carving up the USB
code, it's clear that I left some of the UDP protocol handling in this
patch.
diff mbox series

Patch

diff --git a/drivers/fastboot/Makefile b/drivers/fastboot/Makefile
index 9af4073..c0106a7 100644
--- a/drivers/fastboot/Makefile
+++ b/drivers/fastboot/Makefile
@@ -2,6 +2,7 @@ 
 
 obj-y += fb_common.o
 obj-y += fb_getvar.o
+obj-y += fb_packet.o
 obj-$(CONFIG_FASTBOOT_FLASH) += image-sparse.o
 
 obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fb_mmc.o
diff --git a/drivers/fastboot/fb_packet.c b/drivers/fastboot/fb_packet.c
new file mode 100644
index 0000000..fbe5668
--- /dev/null
+++ b/drivers/fastboot/fb_packet.c
@@ -0,0 +1,249 @@ 
+// SPDX-License-Identifier: BSD-2-Clause
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ */
+
+#include <common.h>
+#include <fastboot.h>
+#include <fb_mmc.h>
+#include <part.h>
+#include <stdlib.h>
+#include <version.h>
+
+/* Parsed from first fastboot command packet */
+static char *cmd_string;
+static char *cmd_parameter;
+static int cmd = -1;
+
+/* Fastboot download parameters */
+static unsigned int bytes_received;
+static unsigned int bytes_expected;
+static unsigned int image_size;
+
+static void cb_okay(char *, char *, unsigned int, char *);
+static void cb_getvar(char *, char *, unsigned int, char *);
+static void cb_download(char *, char *, unsigned int, char *);
+#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
+static void cb_flash(char *, char *, unsigned int, char *);
+static void cb_erase(char *, char *, unsigned int, char *);
+#endif
+static void cb_continue(char *, char *, unsigned int, char *);
+static void cb_reboot_bootloader(char *, char *, unsigned int, char *);
+
+static void (*fb_net_dispatch[])(char *cmd_parameter,
+				 char *fastboot_data,
+				 unsigned int fastboot_data_len,
+				 char *response) = {
+	[FB_CMD_GETVAR] = cb_getvar,
+	[FB_CMD_DOWNLOAD] = cb_download,
+	[FB_CMD_VERIFY] = NULL,
+#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
+	[FB_CMD_FLASH] = cb_flash,
+	[FB_CMD_ERASE] = cb_erase,
+#else
+	[FB_CMD_FLASH] = NULL,
+	[FB_CMD_ERASE] = NULL,
+#endif
+	[FB_CMD_BOOT] = cb_okay,
+	[FB_CMD_CONTINUE] = cb_continue,
+	[FB_CMD_REBOOT] = cb_okay,
+	[FB_CMD_REBOOT_BOOTLOADER] = cb_reboot_bootloader,
+	[FB_CMD_POWERDOWN] = NULL,
+	[FB_CMD_SET_ACTIVE] = cb_okay,
+	[FB_CMD_UPLOAD] = NULL,
+};
+
+static void cleanup_command_data(void);
+
+int fastboot_packet_handle(char *fastboot_data,
+			   unsigned int fastboot_data_len, char *response)
+{
+	if (!cmd_string) {
+		/* Parse command and send ack */
+		cmd_parameter = fastboot_data;
+		cmd_string = strsep(&cmd_parameter, ":");
+		cmd_string = strdup(cmd_string);
+		cmd = fastboot_lookup_command(cmd_string);
+		if (cmd_parameter)
+			cmd_parameter = strdup(cmd_parameter);
+	} else {
+		if (cmd >= 0) {
+			void (*fb_call)(char *cmd_parameter,
+					char *fastboot_data,
+					unsigned int fastboot_data_len,
+					char *response);
+			fb_call = fb_net_dispatch[cmd];
+			if (fb_call) {
+				fb_call(cmd_parameter, fastboot_data,
+					fastboot_data_len, response);
+			} else {
+				pr_err("command %s not implemented.\n",
+				       cmd_string);
+				fastboot_fail("unrecognized command",
+					      response);
+			}
+		} else {
+			pr_err("command %s not recognized.\n",
+			       cmd_string);
+			fastboot_fail("unrecognized command", response);
+		}
+	}
+	return cmd;
+}
+
+void fastboot_after_response(char *response)
+{
+	/* OKAY and FAIL indicate command is complete */
+	if (!strncmp("OKAY", response, 4) || !strncmp("FAIL", response, 4))
+		cleanup_command_data();
+}
+
+static void cb_okay(char *cmd_parameter, char *fastboot_data,
+		    unsigned int fastboot_data_len, char *response)
+{
+	fastboot_okay(NULL, response);
+}
+
+static void cb_getvar(char *cmd_parameter, char *fastboot_data,
+		      unsigned int fastboot_data_len, char *response)
+{
+	fb_getvar(cmd_parameter, response);
+}
+
+/**
+ * Copies image data from fastboot_data to CONFIG_FASTBOOT_BUF_ADDR.
+ * Writes to response.
+ *
+ * @param fastboot_data        Pointer to received fastboot data
+ * @param fastboot_data_len    Length of received fastboot data
+ * @param repsonse             Pointer to fastboot response buffer
+ */
+static void cb_download(char *cmd_parameter, char *fastboot_data,
+			unsigned int fastboot_data_len, char *response)
+{
+	char *tmp;
+
+	if (bytes_expected == 0) {
+		if (!cmd_parameter) {
+			fastboot_fail("Expected command parameter", response);
+			return;
+		}
+		bytes_expected = simple_strtoul(cmd_parameter, &tmp, 16);
+		if (bytes_expected == 0) {
+			fastboot_fail("Expected nonzero image size", response);
+			return;
+		}
+	}
+	if (fastboot_data_len == 0 && bytes_received == 0) {
+		/* Nothing to download yet. Response is of the form:
+		 * [DATA|FAIL]$cmd_parameter
+		 *
+		 * where cmd_parameter is an 8 digit hexadecimal number
+		 */
+		if (bytes_expected > CONFIG_FASTBOOT_BUF_SIZE)
+			fastboot_fail(cmd_parameter, response);
+		else
+			fastboot_response("DATA", response, "%s",
+					  cmd_parameter);
+	} else if (fastboot_data_len == 0 &&
+		   (bytes_received >= bytes_expected)) {
+		/* Download complete. Respond with "OKAY" */
+		fastboot_okay(NULL, response);
+		image_size = bytes_received;
+		bytes_expected = 0;
+		bytes_received = 0;
+	} else {
+		if (fastboot_data_len == 0 ||
+		    (bytes_received + fastboot_data_len) > bytes_expected) {
+			fastboot_fail("Received invalid data length",
+				      response);
+			return;
+		}
+		/* Download data to CONFIG_FASTBOOT_BUF_ADDR */
+		memcpy((void *)CONFIG_FASTBOOT_BUF_ADDR + bytes_received,
+		       fastboot_data, fastboot_data_len);
+		bytes_received += fastboot_data_len;
+	}
+}
+
+#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
+/**
+ * Writes the previously downloaded image to the partition indicated by
+ * cmd_parameter. Writes to response.
+ *
+ * @param repsonse    Pointer to fastboot response buffer
+ */
+static void cb_flash(char *cmd_parameter, char *fastboot_data,
+		     unsigned int fastboot_data_len, char *response)
+{
+#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
+	fb_mmc_flash_write(cmd_parameter, (void *)CONFIG_FASTBOOT_BUF_ADDR,
+			   image_size, response);
+#endif
+#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
+	fb_nand_flash_write(cmd_parameter, (void *)CONFIG_FASTBOOT_BUF_ADDR,
+			    image_size, response);
+#endif
+}
+
+/**
+ * Erases the partition indicated by cmd_parameter (clear to 0x00s). Writes
+ * to response.
+ *
+ * @param repsonse    Pointer to fastboot response buffer
+ */
+static void cb_erase(char *cmd_parameter, char *fastboot_data,
+		     unsigned int fastboot_data_len, char *response)
+{
+#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
+	fb_mmc_erase(cmd_parameter, response);
+#endif
+#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
+	fb_nand_erase(cmd_parameter, response);
+#endif
+}
+#endif
+
+/**
+ * Continues normal boot process by exiting fastboot server. Writes
+ * to response.
+ *
+ * @param repsonse    Pointer to fastboot response buffer
+ */
+static void cb_continue(char *cmd_parameter, char *fastboot_data,
+			unsigned int fastboot_data_len, char *response)
+{
+	net_set_state(NETLOOP_SUCCESS);
+	fastboot_okay(NULL, response);
+}
+
+/**
+ * Sets reboot bootloader flag if requested. Writes to response.
+ *
+ * @param repsonse    Pointer to fastboot response buffer
+ */
+static void cb_reboot_bootloader(char *cmd_parameter, char *fastboot_data,
+				 unsigned int fastboot_data_len, char *response)
+{
+	if (fb_set_reboot_flag())
+		fastboot_fail("Cannot set reboot flag", response);
+	else
+		fastboot_okay(NULL, response);
+}
+
+/**
+ * Frees any resources allocated during current fastboot command.
+ */
+static void cleanup_command_data(void)
+{
+	/* cmd_parameter and cmd_string potentially point to memory allocated by
+	 * strdup
+	 */
+	if (cmd_parameter)
+		free(cmd_parameter);
+	if (cmd_string)
+		free(cmd_string);
+	cmd_parameter = NULL;
+	cmd_string = NULL;
+	cmd = -1;
+}
diff --git a/include/fastboot.h b/include/fastboot.h
index 64f9939..4a02554 100644
--- a/include/fastboot.h
+++ b/include/fastboot.h
@@ -77,4 +77,7 @@  int strcmp_l1(const char *s1, const char *s2);
 int fastboot_lookup_command(const char *cmd_string);
 int fb_set_reboot_flag(void);
 void fastboot_boot(void *addr);
+int fastboot_packet_handle(char *fastboot_data,
+			   unsigned int fastboot_data_len, char *response);
+void fastboot_after_response(char *response);
 #endif /* _FASTBOOT_H_ */
diff --git a/net/fastboot.c b/net/fastboot.c
index a07b1ad..97f75a5 100644
--- a/net/fastboot.c
+++ b/net/fastboot.c
@@ -5,12 +5,8 @@ 
 
 #include <common.h>
 #include <fastboot.h>
-#include <fb_mmc.h>
 #include <net.h>
 #include <net/fastboot.h>
-#include <part.h>
-#include <stdlib.h>
-#include <version.h>
 
 /* Fastboot port # defined in spec */
 #define WELL_KNOWN_PORT 5554
@@ -42,56 +38,13 @@  static const unsigned short fb_udp_version = 1;
 static uchar last_packet[PACKET_SIZE];
 static unsigned int last_packet_len;
 
-/* Parsed from first fastboot command packet */
-static char *cmd_string;
-static char *cmd_parameter;
-
-/* Fastboot download parameters */
-static unsigned int bytes_received;
-static unsigned int bytes_expected;
-static unsigned int image_size;
-
 static struct in_addr fastboot_remote_ip;
 /* The UDP port at their end */
 static int fastboot_remote_port;
 /* The UDP port at our end */
 static int fastboot_our_port;
 
-static void cb_okay(char *, char *, unsigned int, char *);
-static void cb_getvar(char *, char *, unsigned int, char *);
-static void cb_download(char *, char *, unsigned int, char *);
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
-static void cb_flash(char *, char *, unsigned int, char *);
-static void cb_erase(char *, char *, unsigned int, char *);
-#endif
-static void cb_continue(char *, char *, unsigned int, char *);
-static void cb_reboot_bootloader(char *, char *, unsigned int, char *);
-
-static void (*fb_net_dispatch[])(char *cmd_parameter,
-				 char *fastboot_data,
-				 unsigned int fastboot_data_len,
-				 char *response) = {
-	[FB_CMD_GETVAR] = cb_getvar,
-	[FB_CMD_DOWNLOAD] = cb_download,
-	[FB_CMD_VERIFY] = NULL,
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
-	[FB_CMD_FLASH] = cb_flash,
-	[FB_CMD_ERASE] = cb_erase,
-#else
-	[FB_CMD_FLASH] = NULL,
-	[FB_CMD_ERASE] = NULL,
-#endif
-	[FB_CMD_BOOT] = cb_okay,
-	[FB_CMD_CONTINUE] = cb_continue,
-	[FB_CMD_REBOOT] = cb_okay,
-	[FB_CMD_REBOOT_BOOTLOADER] = cb_reboot_bootloader,
-	[FB_CMD_POWERDOWN] = NULL,
-	[FB_CMD_SET_ACTIVE] = cb_okay,
-	[FB_CMD_UPLOAD] = NULL,
-};
-
 static void boot_downloaded_image(void);
-static void cleanup_command_data(void);
 
 void fastboot_send_info(const char *msg)
 {
@@ -185,36 +138,8 @@  static void fastboot_send(struct fastboot_header fb_header, char *fastboot_data,
 		packet += strlen(error_msg);
 		break;
 	case FASTBOOT_FASTBOOT:
-		if (!cmd_string) {
-			/* Parse command and send ack */
-			cmd_parameter = fastboot_data;
-			cmd_string = strsep(&cmd_parameter, ":");
-			cmd_string = strdup(cmd_string);
-			if (cmd_parameter)
-				cmd_parameter = strdup(cmd_parameter);
-		} else {
-			cmd = fastboot_lookup_command(cmd_string);
-			if (cmd >= 0) {
-				void (*fb_call)(char *cmd_parameter,
-						char *fastboot_data,
-						unsigned int fastboot_data_len,
-						char *response);
-				fb_call = fb_net_dispatch[cmd];
-				if (fb_call) {
-					fb_call(cmd_parameter, fastboot_data,
-						fastboot_data_len, response);
-				} else {
-					pr_err("command %s not implemented.\n",
-					       cmd_string);
-					fastboot_fail("unrecognized command",
-						      response);
-				}
-			} else {
-				pr_err("command %s not recognized.\n",
-				       cmd_string);
-				fastboot_fail("unrecognized command", response);
-			}
-		}
+		cmd = fastboot_packet_handle(fastboot_data, fastboot_data_len,
+					     response);
 		/* Sent some INFO packets, need to update sequence number in
 		 * header
 		 */
@@ -245,148 +170,15 @@  static void fastboot_send(struct fastboot_header fb_header, char *fastboot_data,
 	if (!strncmp("OKAY", response, 4)) {
 		if (cmd == FB_CMD_BOOT) {
 			boot_downloaded_image();
+		} else if (cmd == FB_CMD_CONTINUE) {
+			net_set_state(NETLOOP_SUCCESS);
 		} else if (cmd == FB_CMD_REBOOT ||
 			   cmd == FB_CMD_REBOOT_BOOTLOADER) {
 			do_reset(NULL, 0, 0, NULL);
 		}
 	}
 
-	/* OKAY and FAIL indicate command is complete */
-	if (!strncmp("OKAY", response, 4) || !strncmp("FAIL", response, 4))
-		cleanup_command_data();
-}
-
-static void cb_okay(char *cmd_parameter, char *fastboot_data,
-		    unsigned int fastboot_data_len, char *response)
-{
-	fastboot_okay(NULL, response);
-}
-
-static void cb_getvar(char *cmd_parameter, char *fastboot_data,
-		      unsigned int fastboot_data_len, char *response)
-{
-	fb_getvar(cmd_parameter, response);
-}
-
-/**
- * Copies image data from fastboot_data to CONFIG_FASTBOOT_BUF_ADDR.
- * Writes to response.
- *
- * @param fastboot_data        Pointer to received fastboot data
- * @param fastboot_data_len    Length of received fastboot data
- * @param repsonse             Pointer to fastboot response buffer
- */
-static void cb_download(char *cmd_parameter, char *fastboot_data,
-			unsigned int fastboot_data_len, char *response)
-{
-	char *tmp;
-
-	if (bytes_expected == 0) {
-		if (!cmd_parameter) {
-			fastboot_fail("Expected command parameter", response);
-			return;
-		}
-		bytes_expected = simple_strtoul(cmd_parameter, &tmp, 16);
-		if (bytes_expected == 0) {
-			fastboot_fail("Expected nonzero image size", response);
-			return;
-		}
-	}
-	if (fastboot_data_len == 0 && bytes_received == 0) {
-		/* Nothing to download yet. Response is of the form:
-		 * [DATA|FAIL]$cmd_parameter
-		 *
-		 * where cmd_parameter is an 8 digit hexadecimal number
-		 */
-		if (bytes_expected > CONFIG_FASTBOOT_BUF_SIZE)
-			fastboot_fail(cmd_parameter, response);
-		else
-			fastboot_response("DATA", response, "%s",
-					  cmd_parameter);
-	} else if (fastboot_data_len == 0 &&
-		   (bytes_received >= bytes_expected)) {
-		/* Download complete. Respond with "OKAY" */
-		fastboot_okay(NULL, response);
-		image_size = bytes_received;
-		bytes_expected = 0;
-		bytes_received = 0;
-	} else {
-		if (fastboot_data_len == 0 ||
-		    (bytes_received + fastboot_data_len) > bytes_expected) {
-			fastboot_fail("Received invalid data length",
-				      response);
-			return;
-		}
-		/* Download data to CONFIG_FASTBOOT_BUF_ADDR */
-		memcpy((void *)CONFIG_FASTBOOT_BUF_ADDR + bytes_received,
-		       fastboot_data, fastboot_data_len);
-		bytes_received += fastboot_data_len;
-	}
-}
-
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
-/**
- * Writes the previously downloaded image to the partition indicated by
- * cmd_parameter. Writes to response.
- *
- * @param repsonse    Pointer to fastboot response buffer
- */
-static void cb_flash(char *cmd_parameter, char *fastboot_data,
-		     unsigned int fastboot_data_len, char *response)
-{
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
-	fb_mmc_flash_write(cmd_parameter, (void *)CONFIG_FASTBOOT_BUF_ADDR,
-			   image_size, response);
-#endif
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
-	fb_nand_flash_write(cmd_parameter, (void *)CONFIG_FASTBOOT_BUF_ADDR,
-			    image_size, response);
-#endif
-}
-
-/**
- * Erases the partition indicated by cmd_parameter (clear to 0x00s). Writes
- * to response.
- *
- * @param repsonse    Pointer to fastboot response buffer
- */
-static void cb_erase(char *cmd_parameter, char *fastboot_data,
-		     unsigned int fastboot_data_len, char *response)
-{
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
-	fb_mmc_erase(cmd_parameter, response);
-#endif
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
-	fb_nand_erase(cmd_parameter, response);
-#endif
-}
-#endif
-
-/**
- * Continues normal boot process by exiting fastboot server. Writes
- * to response.
- *
- * @param repsonse    Pointer to fastboot response buffer
- */
-static void cb_continue(char *cmd_parameter, char *fastboot_data,
-			unsigned int fastboot_data_len, char *response)
-{
-	net_set_state(NETLOOP_SUCCESS);
-	fastboot_okay(NULL, response);
-}
-
-/**
- * Sets reboot bootloader flag if requested. Writes to response.
- *
- * @param repsonse    Pointer to fastboot response buffer
- */
-static void cb_reboot_bootloader(char *cmd_parameter, char *fastboot_data,
-				 unsigned int fastboot_data_len, char *response)
-{
-	if (fb_set_reboot_flag())
-		fastboot_fail("Cannot set reboot flag", response);
-	else
-		fastboot_okay(NULL, response);
+	fastboot_after_response(response);
 }
 
 /**
@@ -399,22 +191,6 @@  static void boot_downloaded_image(void)
 }
 
 /**
- * Frees any resources allocated during current fastboot command.
- */
-static void cleanup_command_data(void)
-{
-	/* cmd_parameter and cmd_string potentially point to memory allocated by
-	 * strdup
-	 */
-	if (cmd_parameter)
-		free(cmd_parameter);
-	if (cmd_string)
-		free(cmd_string);
-	cmd_parameter = NULL;
-	cmd_string = NULL;
-}
-
-/**
  * Incoming UDP packet handler.
  *
  * @param packet  Pointer to incoming UDP packet