Hi Svyatoslav,
Thank you for your patch.
On mar., nov. 07, 2023 at 14:42, Svyatoslav Ryhel <clamo...@gmail.com> wrote:
> From: Ion Agorria <i...@agorria.com>
>
> This commit implements "fastboot getvar all" listing
> by iterating the existing dispatchers that don't require
> parameters (as we pass NULL), uses fastboot multiresponse.
>
> Signed-off-by: Ion Agorria <i...@agorria.com>
> Signed-off-by: Svyatoslav Ryhel <clamo...@gmail.com>
Some small comments below.
With those addressed, please add:
Reviewed-by: Mattijs Korpershoek <mkorpersh...@baylibre.com>
> ---
> doc/android/fastboot-protocol.rst | 3 ++
> drivers/fastboot/fb_command.c | 3 ++
> drivers/fastboot/fb_getvar.c | 75 +++++++++++++++++++++++++------
> include/fastboot-internal.h | 7 +++
> 4 files changed, 75 insertions(+), 13 deletions(-)
>
> diff --git a/doc/android/fastboot-protocol.rst
> b/doc/android/fastboot-protocol.rst
> index e8cbd7f24e..8bd6d7168f 100644
> --- a/doc/android/fastboot-protocol.rst
> +++ b/doc/android/fastboot-protocol.rst
> @@ -173,6 +173,9 @@ The various currently defined names are::
> bootloader requiring a signature before
> it will install or boot images.
>
> + all Provides all info from commands above as
> + they were called one by one
> +
> Names starting with a lowercase character are reserved by this
> specification. OEM-specific names should not start with lowercase
> characters.
> diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c
> index ab72d8c781..6f621df074 100644
> --- a/drivers/fastboot/fb_command.c
> +++ b/drivers/fastboot/fb_command.c
> @@ -156,6 +156,9 @@ int fastboot_handle_command(char *cmd_string, char
> *response)
> void fastboot_multiresponse(int cmd, char *response)
> {
> switch (cmd) {
> + case FASTBOOT_COMMAND_GETVAR:
> + fastboot_getvar_all(response);
> + break;
> default:
> fastboot_fail("Unknown multiresponse command", response);
> break;
> diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
> index 8cb8ffa2c6..fc5e1dac87 100644
> --- a/drivers/fastboot/fb_getvar.c
> +++ b/drivers/fastboot/fb_getvar.c
> @@ -29,53 +29,67 @@ static void getvar_is_userspace(char *var_parameter, char
> *response);
>
> static const struct {
> const char *variable;
> + bool list;
> void (*dispatch)(char *var_parameter, char *response);
> } getvar_dispatch[] = {
> {
> .variable = "version",
> - .dispatch = getvar_version
> + .dispatch = getvar_version,
> + .list = true,
> }, {
> .variable = "version-bootloader",
> - .dispatch = getvar_version_bootloader
> + .dispatch = getvar_version_bootloader,
> + .list = true
> }, {
> .variable = "downloadsize",
> - .dispatch = getvar_downloadsize
> + .dispatch = getvar_downloadsize,
> + .list = true
> }, {
> .variable = "max-download-size",
> - .dispatch = getvar_downloadsize
> + .dispatch = getvar_downloadsize,
> + .list = true
> }, {
> .variable = "serialno",
> - .dispatch = getvar_serialno
> + .dispatch = getvar_serialno,
> + .list = true
> }, {
> .variable = "version-baseband",
> - .dispatch = getvar_version_baseband
> + .dispatch = getvar_version_baseband,
> + .list = true
> }, {
> .variable = "product",
> - .dispatch = getvar_product
> + .dispatch = getvar_product,
> + .list = true
> }, {
> .variable = "platform",
> - .dispatch = getvar_platform
> + .dispatch = getvar_platform,
> + .list = true
> }, {
> .variable = "current-slot",
> - .dispatch = getvar_current_slot
> + .dispatch = getvar_current_slot,
> + .list = true
> #if IS_ENABLED(CONFIG_FASTBOOT_FLASH)
> }, {
> .variable = "has-slot",
> - .dispatch = getvar_has_slot
> + .dispatch = getvar_has_slot,
> + .list = false
> #endif
> #if IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC)
> }, {
> .variable = "partition-type",
> - .dispatch = getvar_partition_type
> + .dispatch = getvar_partition_type,
> + .list = false
> #endif
> #if IS_ENABLED(CONFIG_FASTBOOT_FLASH)
> }, {
> .variable = "partition-size",
> - .dispatch = getvar_partition_size
> + .dispatch = getvar_partition_size,
> + .list = false
> #endif
> }, {
> .variable = "is-userspace",
> - .dispatch = getvar_is_userspace
> + .dispatch = getvar_is_userspace,
> + .list = true
> }
> };
>
> @@ -237,6 +251,38 @@ static void getvar_is_userspace(char *var_parameter,
> char *response)
> fastboot_okay("no", response);
> }
>
> +int current_all_dispatch;
static please, as this is only used in drivers/fastboot/fb_getvar.c
> +void fastboot_getvar_all(char *response)
> +{
> + /*
> + * Find a dispatch getvar that can be listed and send
> + * it as INFO until we reach the end.
> + */
> + while (current_all_dispatch < ARRAY_SIZE(getvar_dispatch)) {
> + if (!getvar_dispatch[current_all_dispatch].list) {
> + current_all_dispatch++;
> + continue;
> + }
> +
> + char envstr[FASTBOOT_RESPONSE_LEN] = { 0 };
> + getvar_dispatch[current_all_dispatch].dispatch(NULL, envstr);
./scripts/checkpatch.pl --strict --u-boot complains a missing line here
> +
> + char *envstr_start = envstr;
./scripts/checkpatch.pl --strict --u-boot complains a missing line here
> + if (!strncmp("OKAY", envstr, 4) || !strncmp("FAIL", envstr, 4))
> + envstr_start += 4;
> +
> + fastboot_response("INFO", response, "%s: %s",
> +
> getvar_dispatch[current_all_dispatch].variable,
> + envstr_start);
> +
> + current_all_dispatch++;
> + return;
> + }
> +
> + fastboot_response("OKAY", response, NULL);
> + current_all_dispatch = 0;
> +}
> +
> /**
> * fastboot_getvar() - Writes variable indicated by cmd_parameter to
> response.
> *
> @@ -254,6 +300,9 @@ void fastboot_getvar(char *cmd_parameter, char *response)
> {
> if (!cmd_parameter) {
> fastboot_fail("missing var", response);
> + } else if (!strncmp("all", cmd_parameter, 3) && strlen(cmd_parameter)
> == 3) {
> + current_all_dispatch = 0;
> + fastboot_response("MORE", response, NULL);
> } else {
> #define FASTBOOT_ENV_PREFIX "fastboot."
> int i;
> diff --git a/include/fastboot-internal.h b/include/fastboot-internal.h
> index bf2f2b3c89..610d4f9141 100644
> --- a/include/fastboot-internal.h
> +++ b/include/fastboot-internal.h
> @@ -18,6 +18,13 @@ extern u32 fastboot_buf_size;
> */
> extern void (*fastboot_progress_callback)(const char *msg);
>
> +/**
> + * fastboot_getvar_all() - Writes current variable being listed from "all"
> to response.
> + *
> + * @response: Pointer to fastboot response buffer
> + */
> +void fastboot_getvar_all(char *response);
> +
> /**
> * fastboot_getvar() - Writes variable indicated by cmd_parameter to
> response.
> *
> --
> 2.40.1
