[U-Boot] [RFC PATCH v2 12/20] fastboot: net: Convert command lookup to a table

[Joe Hershberger <joe.hershberger@ni.com>]

On Mon, Apr 30, 2018 at 3:32 AM, Alex Kiernan <alex.kiernan@gmail.com> wrote:
> Change command lookup to use a lookup table so it matches the existing
> USB fastboot code.
>
> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
> ---
>
> 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.

> +                       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?

> +                   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.
> @@ -237,8 +277,8 @@ static void fastboot_send(struct fastboot_header fb_header, char *fastboot_data,
>   * @param fastboot_data_len    Length of received fastboot data
>   * @param repsonse             Pointer to fastboot response buffer
>   */
> -static void fb_download(char *fastboot_data, unsigned int fastboot_data_len,
> -                       char *response)
> +static void cb_download(char *cmd_parameter, char *fastboot_data,
> +                       unsigned int fastboot_data_len, char *response)
>  {
>         char *tmp;
>
> @@ -292,7 +332,8 @@ static void fb_download(char *fastboot_data, unsigned int fastboot_data_len,
>   *
>   * @param repsonse    Pointer to fastboot response buffer
>   */
> -static void fb_flash(char *response)
> +static void cb_flash(char *cmd_parameter, char *fastboot_data,
> +                    unsigned int fastboot_data_len, char *response)
>  {
>         fb_mmc_flash_write(cmd_parameter, (void *)CONFIG_FASTBOOT_BUF_ADDR,
>                            image_size, response);
> @@ -304,7 +345,8 @@ static void fb_flash(char *response)
>   *
>   * @param repsonse    Pointer to fastboot response buffer
>   */
> -static void fb_erase(char *response)
> +static void cb_erase(char *cmd_parameter, char *fastboot_data,
> +                    unsigned int fastboot_data_len, char *response)
>  {
>         fb_mmc_erase(cmd_parameter, response);
>  }
> @@ -316,7 +358,8 @@ static void fb_erase(char *response)
>   *
>   * @param repsonse    Pointer to fastboot response buffer
>   */
> -static void fb_continue(char *response)
> +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);
> @@ -327,7 +370,8 @@ static void fb_continue(char *response)
>   *
>   * @param repsonse    Pointer to fastboot response buffer
>   */
> -static void fb_reboot(char *response)
> +static void cb_reboot(char *cmd_parameter, char *fastboot_data,
> +                     unsigned int fastboot_data_len, char *response)
>  {
>         fastboot_okay(NULL, response);
>         if (!strcmp("reboot-bootloader", cmd_string))
> --
> 2.7.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot