[PATCH v1 2/5] fastboot: implement "getvar all"
Svyatoslav Ryhel
clamor95 at gmail.com
Tue Nov 14 10:38:43 CET 2023
14 листопада 2023 р. 11:32:35 GMT+02:00, Mattijs Korpershoek <mkorpershoek at baylibre.com> написав(-ла):
>Hi Svyatoslav,
>
>Thank you for your patch.
>
>On mar., nov. 07, 2023 at 14:42, Svyatoslav Ryhel <clamor95 at gmail.com> wrote:
>
>> From: Ion Agorria <ion at 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 <ion at agorria.com>
>> Signed-off-by: Svyatoslav Ryhel <clamor95 at gmail.com>
>
>Some small comments below.
>
>With those addressed, please add:
>
>Reviewed-by: Mattijs Korpershoek <mkorpershoek at 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
>
About missing lines, I have intentionally not added them since it does not make reading code harder while having a line-space-line IMHO makes it more unpleasant to read. I would like to leave it as is if possible. Everything else will be adjusted. Thanks!
>> + 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
More information about the U-Boot
mailing list