From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Kiernan Date: Fri, 04 May 2018 09:14:07 +0000 Subject: [U-Boot] [RFC PATCH v2 12/20] fastboot: net: Convert command lookup to a table In-Reply-To: References: <1525077174-6211-1-git-send-email-alex.kiernan@gmail.com> <1525077174-6211-13-git-send-email-alex.kiernan@gmail.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Thu, May 3, 2018 at 10:08 PM Joe Hershberger wrote: > On Mon, Apr 30, 2018 at 3:32 AM, Alex Kiernan wrote: > > Change command lookup to use a lookup table so it matches the existing > > USB fastboot code. > > > > Signed-off-by: Alex Kiernan > > --- > > > > Changes in v2: None > > > > drivers/fastboot/fb_common.c | 30 ++++++++++++ > > include/fastboot.h | 21 +++++++++ > > net/fastboot.c | 108 ++++++++++++++++++++++++++++++------------- > > 3 files changed, 127 insertions(+), 32 deletions(-) > > > > diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c > > index 3b68f25..8b3627b 100644 > > --- a/drivers/fastboot/fb_common.c > > +++ b/drivers/fastboot/fb_common.c > > @@ -72,3 +72,33 @@ int strcmp_l1(const char *s1, const char *s2) > > return -1; > > return strncmp(s1, s2, strlen(s1)); > > } > > + > > +static const char *const fb_commands[] = { > > + [FB_CMD_GETVAR] = "getvar", > > + [FB_CMD_DOWNLOAD] = "download", > > + [FB_CMD_VERIFY] = "verify", > > + [FB_CMD_FLASH] = "flash", > > + [FB_CMD_ERASE] = "erase", > > + [FB_CMD_BOOT] = "boot", > > + [FB_CMD_CONTINUE] = "continue", > > + [FB_CMD_REBOOT] = "reboot", > > + [FB_CMD_REBOOT_BOOTLOADER] = "reboot-bootloader", > > + [FB_CMD_POWERDOWN] = "powerdown", > > + [FB_CMD_SET_ACTIVE] = "set_active", > > + [FB_CMD_UPLOAD] = "upload", > > +}; > > + > > +int fastboot_lookup_command(const char *cmd_string) > > +{ > > + int i; > > + > > + for (i = 0; i < FB_CMD_COUNT; i++) { > > + int len = strlen(fb_commands[i]); > > + > > + if (!strncmp(fb_commands[i], cmd_string, len) && > Why not use your new strcmp_l1()? > > + (cmd_string[len] == '\0' || cmd_string[len] == ':')) > At this point the ':' is already deleted. It's a bit of my prep for picking the USB code leaking in. But I'm probably going to refactor this change out of existence now I'm actually looking at the USB code. > > + return i; > > + } > > + > > + return -1; > > +} > > diff --git a/include/fastboot.h b/include/fastboot.h > > index fb58358..de07220 100644 > > --- a/include/fastboot.h > > +++ b/include/fastboot.h > > @@ -18,6 +18,26 @@ > > /* The 64 defined bytes plus \0 */ > > #define FASTBOOT_RESPONSE_LEN (64 + 1) > > > > +/** > > + * All known commands to fastboot > > + */ > > +enum { > > + FB_CMD_GETVAR = 0, > > + FB_CMD_DOWNLOAD, > > + FB_CMD_VERIFY, > > + FB_CMD_FLASH, > > + FB_CMD_ERASE, > > + FB_CMD_BOOT, > > + FB_CMD_CONTINUE, > > + FB_CMD_REBOOT, > > + FB_CMD_REBOOT_BOOTLOADER, > > + FB_CMD_POWERDOWN, > > + FB_CMD_SET_ACTIVE, > > + FB_CMD_UPLOAD, > > + > > + FB_CMD_COUNT > > +}; > > + > > void fastboot_response(const char *tag, char *response, > > const char *format, ...) > > __attribute__ ((format (__printf__, 3, 4))); > > @@ -54,4 +74,5 @@ void timed_send_info(ulong *start, const char *msg); > > */ > > int strcmp_l1(const char *s1, const char *s2); > > > > +int fastboot_lookup_command(const char *cmd_string); > > #endif /* _FASTBOOT_H_ */ > > diff --git a/net/fastboot.c b/net/fastboot.c > > index ed13890..155049a 100644 > > --- a/net/fastboot.c > > +++ b/net/fastboot.c > > @@ -57,13 +57,39 @@ static int fastboot_remote_port; > > /* The UDP port at our end */ > > static int fastboot_our_port; > > > > -static void fb_download(char *, unsigned int, char *); > > +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_MMC) > > -static void fb_flash(char *); > > -static void fb_erase(char *); > > +static void cb_flash(char *, char *, unsigned int, char *); > > +static void cb_erase(char *, char *, unsigned int, char *); > > #endif > > -static void fb_continue(char *); > > -static void fb_reboot(char *); > > +static void cb_continue(char *, char *, unsigned int, char *); > > +static void cb_reboot(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_MMC) > > + [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_reboot, > > + [FB_CMD_REBOOT_BOOTLOADER] = cb_reboot, > > + [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); > > > > @@ -165,28 +191,30 @@ static void fastboot_send(struct fastboot_header fb_header, char *fastboot_data, > > cmd_string = strdup(cmd_string); > > if (cmd_parameter) > > cmd_parameter = strdup(cmd_parameter); > > - } else if (!strcmp("getvar", cmd_string)) { > > - fb_getvar(cmd_parameter, response); > > - } else if (!strcmp("download", cmd_string)) { > > - fb_download(fastboot_data, fastboot_data_len, response); > > -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC) > > - } else if (!strcmp("flash", cmd_string)) { > > - fb_flash(response); > > - } else if (!strcmp("erase", cmd_string)) { > > - fb_erase(response); > > -#endif > > - } else if (!strcmp("boot", cmd_string)) { > > - fastboot_okay(NULL, response); > > - } else if (!strcmp("continue", cmd_string)) { > > - fb_continue(response); > > - } else if (!strncmp("reboot", cmd_string, 6)) { > > - fb_reboot(response); > > - } else if (!strcmp("set_active", cmd_string)) { > > - /* A/B not implemented, for now do nothing */ > > - fastboot_okay(NULL, response); > > } else { > > - pr_err("command %s not implemented.\n", cmd_string); > > - fastboot_fail("unrecognized command", response); > > + int i; > > + > > + i = fastboot_lookup_command(cmd_string); > > + if (i >= 0) { > > + void (*fb_call)(char *cmd_parameter, > > + char *fastboot_data, > > + unsigned int fastboot_data_len, > > + char *response); > > + fb_call = fb_net_dispatch[i]; > > + 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); > > + } > > } > > /* Sent some INFO packets, need to update sequence number in > > * header > > @@ -229,6 +257,18 @@ static void fastboot_send(struct fastboot_header fb_header, char *fastboot_data, > > cleanup_command_data(); > > } > > > > +static void cb_okay(char *cmd_parameter, char *fastboot_data, > What does "cb_" stand for? Stolen from the USB fastboot code. I guess it's callback, but its not really that... I'll change it. -- Alex Kiernan