[PATCH 0/2] cmd: bcb: extend BCB APIs to support Android boot flow
Following patches introduce various block interfaces support and extend API of BCB (bootloader control block) to reuse it for Android boot flow of Cuttlefish virtual device.
Signed-off-by: Dmitrii Merkurev dimorinny@google.com Cc: Eugeniu Rosca erosca@de.adit-jv.com Cc: Ying-Chun Liu (PaulLiu) paul.liu@linaro.org Cc: Simon Glass sjg@chromium.org Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Sean Anderson sean.anderson@seco.com Cc: Cody Schuffelen schuffelen@google.com
Dmitrii Merkurev (2): cmd: bcb: support various block device interfaces for BCB command cmd: bcb: extend BCB C API to allow read/write the fields
cmd/Kconfig | 1 - cmd/bcb.c | 206 +++++++++++++++++++++++------------ doc/android/bcb.rst | 34 +++--- drivers/fastboot/fb_common.c | 14 ++- include/bcb.h | 59 +++++++++- 5 files changed, 227 insertions(+), 87 deletions(-)
Currently BCB command-line, C APIs and implementation only support MMC interface. Extend it to allow various block device interfaces.
Signed-off-by: Dmitrii Merkurev dimorinny@google.com Cc: Eugeniu Rosca erosca@de.adit-jv.com Cc: Ying-Chun Liu (PaulLiu) paul.liu@linaro.org Cc: Simon Glass sjg@chromium.org Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Sean Anderson sean.anderson@seco.com Cc: Cody Schuffelen schuffelen@google.com --- cmd/Kconfig | 1 - cmd/bcb.c | 70 ++++++++++++++++++++++-------------- doc/android/bcb.rst | 34 +++++++++--------- drivers/fastboot/fb_common.c | 2 +- include/bcb.h | 5 +-- 5 files changed, 65 insertions(+), 47 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index df6d71c103..ce2435bbb8 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -981,7 +981,6 @@ config CMD_ADC
config CMD_BCB bool "bcb" - depends on MMC depends on PARTITIONS help Read/modify/write the fields of Bootloader Control Block, usually diff --git a/cmd/bcb.c b/cmd/bcb.c index 02d0c70d87..5d8c232663 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -25,6 +25,7 @@ enum bcb_cmd { BCB_CMD_STORE, };
+static enum uclass_id bcb_uclass_id = UCLASS_INVALID; static int bcb_dev = -1; static int bcb_part = -1; static struct bootloader_message bcb __aligned(ARCH_DMA_MINALIGN) = { { 0 } }; @@ -53,6 +54,9 @@ static int bcb_is_misused(int argc, char *const argv[])
switch (cmd) { case BCB_CMD_LOAD: + if (argc != 3 && argc != 4) + goto err; + break; case BCB_CMD_FIELD_SET: if (argc != 3) goto err; @@ -115,7 +119,7 @@ static int bcb_field_get(char *name, char **fieldp, int *sizep) return 0; }
-static int __bcb_load(int devnum, const char *partp) +static int __bcb_load(const char *iface, int devnum, const char *partp) { struct blk_desc *desc; struct disk_partition info; @@ -123,14 +127,14 @@ static int __bcb_load(int devnum, const char *partp) char *endp; int part, ret;
- desc = blk_get_devnum_by_uclass_id(UCLASS_MMC, devnum); + desc = blk_get_dev(iface, devnum); if (!desc) { ret = -ENODEV; goto err_read_fail; }
/* - * always select the USER mmc hwpart in case another + * always select the first hwpart in case another * blk operation selected a different hwpart */ ret = blk_dselect_hwpart(desc, 0); @@ -161,18 +165,20 @@ static int __bcb_load(int devnum, const char *partp) goto err_read_fail; }
+ bcb_uclass_id = desc->uclass_id; bcb_dev = desc->devnum; bcb_part = part; - debug("%s: Loaded from mmc %d:%d\n", __func__, bcb_dev, bcb_part); + debug("%s: Loaded from %s %d:%d\n", __func__, iface, bcb_dev, bcb_part);
return CMD_RET_SUCCESS; err_read_fail: - printf("Error: mmc %d:%s read failed (%d)\n", devnum, partp, ret); + printf("Error: %s %d:%s read failed (%d)\n", iface, devnum, partp, ret); goto err; err_too_small: - printf("Error: mmc %d:%s too small!", devnum, partp); + printf("Error: %s %d:%s too small!", iface, devnum, partp); goto err; err: + bcb_uclass_id = UCLASS_INVALID; bcb_dev = -1; bcb_part = -1;
@@ -182,15 +188,23 @@ err: static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]) { + int devnum; char *endp; - int devnum = simple_strtoul(argv[1], &endp, 0); + char *iface = "mcc"; + + if (argc == 4) { + iface = argv[1]; + argc--; + argv++; + }
+ devnum = simple_strtoul(argv[1], &endp, 0); if (*endp != '\0') { printf("Error: Device id '%s' not a number\n", argv[1]); return CMD_RET_FAILURE; }
- return __bcb_load(devnum, argv[2]); + return __bcb_load(iface, devnum, argv[2]); }
static int __bcb_set(char *fieldp, const char *valp) @@ -298,7 +312,7 @@ static int __bcb_store(void) u64 cnt; int ret;
- desc = blk_get_devnum_by_uclass_id(UCLASS_MMC, bcb_dev); + desc = blk_get_devnum_by_uclass_id(bcb_uclass_id, bcb_dev); if (!desc) { ret = -ENODEV; goto err; @@ -317,7 +331,7 @@ static int __bcb_store(void)
return CMD_RET_SUCCESS; err: - printf("Error: mmc %d:%d write failed (%d)\n", bcb_dev, bcb_part, ret); + printf("Error: %d %d:%d write failed (%d)\n", bcb_uclass_id, bcb_dev, bcb_part, ret);
return CMD_RET_FAILURE; } @@ -328,11 +342,11 @@ static int do_bcb_store(struct cmd_tbl *cmdtp, int flag, int argc, return __bcb_store(); }
-int bcb_write_reboot_reason(int devnum, char *partp, const char *reasonp) +int bcb_write_reboot_reason(const char *iface, int devnum, char *partp, const char *reasonp) { int ret;
- ret = __bcb_load(devnum, partp); + ret = __bcb_load(iface, devnum, partp); if (ret != CMD_RET_SUCCESS) return ret;
@@ -385,21 +399,23 @@ static int do_bcb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) U_BOOT_CMD( bcb, CONFIG_SYS_MAXARGS, 1, do_bcb, "Load/set/clear/test/dump/store Android BCB fields", - "load <dev> <part> - load BCB from mmc <dev>:<part>\n" - "bcb set <field> <val> - set BCB <field> to <val>\n" - "bcb clear [<field>] - clear BCB <field> or all fields\n" - "bcb test <field> <op> <val> - test BCB <field> against <val>\n" - "bcb dump <field> - dump BCB <field>\n" - "bcb store - store BCB back to mmc\n" + "load <interface> <dev> <part> - load BCB from <interface> <dev>:<part>\n" + "load <dev> <part> - load BCB from mmc <dev>:<part>\n" + "bcb set <field> <val> - set BCB <field> to <val>\n" + "bcb clear [<field>] - clear BCB <field> or all fields\n" + "bcb test <field> <op> <val> - test BCB <field> against <val>\n" + "bcb dump <field> - dump BCB <field>\n" + "bcb store - store BCB back to <interface>\n" "\n" "Legend:\n" - "<dev> - MMC device index containing the BCB partition\n" - "<part> - MMC partition index or name containing the BCB\n" - "<field> - one of {command,status,recovery,stage,reserved}\n" - "<op> - the binary operator used in 'bcb test':\n" - " '=' returns true if <val> matches the string stored in <field>\n" - " '~' returns true if <val> matches a subset of <field>'s string\n" - "<val> - string/text provided as input to bcb {set,test}\n" - " NOTE: any ':' character in <val> will be replaced by line feed\n" - " during 'bcb set' and used as separator by upper layers\n" + "<interface> - storage device interface (virtio, mmc, etc)\n" + "<dev> - storage device index containing the BCB partition\n" + "<part> - partition index or name containing the BCB\n" + "<field> - one of {command,status,recovery,stage,reserved}\n" + "<op> - the binary operator used in 'bcb test':\n" + " '=' returns true if <val> matches the string stored in <field>\n" + " '~' returns true if <val> matches a subset of <field>'s string\n" + "<val> - string/text provided as input to bcb {set,test}\n" + " NOTE: any ':' character in <val> will be replaced by line feed\n" + " during 'bcb set' and used as separator by upper layers\n" ); diff --git a/doc/android/bcb.rst b/doc/android/bcb.rst index 8861608300..2226517d39 100644 --- a/doc/android/bcb.rst +++ b/doc/android/bcb.rst @@ -41,23 +41,25 @@ requirements enumerated above. Below is the command's help message:: bcb - Load/set/clear/test/dump/store Android BCB fields
Usage: - bcb load <dev> <part> - load BCB from mmc <dev>:<part> - bcb set <field> <val> - set BCB <field> to <val> - bcb clear [<field>] - clear BCB <field> or all fields - bcb test <field> <op> <val> - test BCB <field> against <val> - bcb dump <field> - dump BCB <field> - bcb store - store BCB back to mmc + bcb load <interface> <dev> <part> - load BCB from <interface> <dev>:<part> + load <dev> <part> - load BCB from mmc <dev>:<part> + bcb set <field> <val> - set BCB <field> to <val> + bcb clear [<field>] - clear BCB <field> or all fields + bcb test <field> <op> <val> - test BCB <field> against <val> + bcb dump <field> - dump BCB <field> + bcb store - store BCB back to <interface>
Legend: - <dev> - MMC device index containing the BCB partition - <part> - MMC partition index or name containing the BCB - <field> - one of {command,status,recovery,stage,reserved} - <op> - the binary operator used in 'bcb test': - '=' returns true if <val> matches the string stored in <field> - '~' returns true if <val> matches a subset of <field>'s string - <val> - string/text provided as input to bcb {set,test} - NOTE: any ':' character in <val> will be replaced by line feed - during 'bcb set' and used as separator by upper layers + <interface> - storage device interface (virtio, mmc, etc) + <dev> - storage device index containing the BCB partition + <part> - partition index or name containing the BCB + <field> - one of {command,status,recovery,stage,reserved} + <op> - the binary operator used in 'bcb test': + '=' returns true if <val> matches the string stored in <field> + '~' returns true if <val> matches a subset of <field>'s string + <val> - string/text provided as input to bcb {set,test} + NOTE: any ':' character in <val> will be replaced by line feed + during 'bcb set' and used as separator by upper layers
'bcb'. Example of getting reboot reason @@ -91,7 +93,7 @@ The following Kconfig options must be enabled::
CONFIG_PARTITIONS=y CONFIG_MMC=y - CONFIG_BCB=y + CONFIG_CMD_BCB=y
.. [1] https://android.googlesource.com/platform/bootable/recovery .. [2] https://source.android.com/devices/bootloader diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c index 4e9d9b719c..2a6608b28c 100644 --- a/drivers/fastboot/fb_common.c +++ b/drivers/fastboot/fb_common.c @@ -105,7 +105,7 @@ int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason) if (reason >= FASTBOOT_REBOOT_REASONS_COUNT) return -EINVAL;
- return bcb_write_reboot_reason(mmc_dev, "misc", boot_cmds[reason]); + return bcb_write_reboot_reason("mmc", mmc_dev, "misc", boot_cmds[reason]); }
/** diff --git a/include/bcb.h b/include/bcb.h index 5edb17aa47..a6326523c4 100644 --- a/include/bcb.h +++ b/include/bcb.h @@ -9,10 +9,11 @@ #define __BCB_H__
#if IS_ENABLED(CONFIG_CMD_BCB) -int bcb_write_reboot_reason(int devnum, char *partp, const char *reasonp); +int bcb_write_reboot_reason(const char *iface, int devnum, char *partp, const char *reasonp); #else #include <linux/errno.h> -static inline int bcb_write_reboot_reason(int devnum, char *partp, const char *reasonp) +static inline int bcb_write_reboot_reason(const char *iface, int devnum, + char *partp, const char *reasonp) { return -EOPNOTSUPP; }
Hi Dmitrii,
Thank you for your patch.
Thank you as well for taking the time to upstream things from: https://android.googlesource.com/platform/external/u-boot/
On jeu., nov. 09, 2023 at 00:36, Dmitrii Merkurev dimorinny@google.com wrote:
Currently BCB command-line, C APIs and implementation only support MMC interface. Extend it to allow various block device interfaces.
Signed-off-by: Dmitrii Merkurev dimorinny@google.com Cc: Eugeniu Rosca erosca@de.adit-jv.com Cc: Ying-Chun Liu (PaulLiu) paul.liu@linaro.org Cc: Simon Glass sjg@chromium.org Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Sean Anderson sean.anderson@seco.com Cc: Cody Schuffelen schuffelen@google.com
This breaks vim3/vim3l android boot flow because both rely on the usage of the bcb command in their U-Boot environment:
Error: 1572575691 110528528: read failed (-19) Warning: BCB is corrupted or does not exist
The following diff fixes it:
diff --git a/include/configs/meson64_android.h b/include/configs/meson64_android.h index c0e977abb01f..442233e827d8 100644 --- a/include/configs/meson64_android.h +++ b/include/configs/meson64_android.h @@ -164,7 +164,7 @@ "fi; " \ "fi;" \ "if test "${run_fastboot}" -eq 0; then " \ - "if bcb load " __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \ + "if bcb load mmc " __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \ CONTROL_PARTITION "; then " \ "if bcb test command = bootonce-bootloader; then " \ "echo BCB: Bootloader boot...; " \ @@ -195,7 +195,7 @@ "echo Recovery button is pressed;" \ "setenv run_recovery 1;" \ "fi; " \ - "if bcb load " __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \ + "if bcb load mmc " __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \ CONTROL_PARTITION "; then " \ "if bcb test command = boot-recovery; then " \ "echo BCB: Recovery boot...; " \ diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h index 4e5aa74147d4..f571744adaf8 100644 --- a/include/configs/ti_omap5_common.h +++ b/include/configs/ti_omap5_common.h @@ -168,7 +168,7 @@ "mmc dev $mmcdev; " \ "mmc rescan; " \ AB_SELECT_SLOT \ - "if bcb load " __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \ + "if bcb load mmc " __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \ CONTROL_PARTITION "; then " \ "setenv ardaddr -; " \ "if bcb test command = bootonce-bootloader; then " \
Can you consider including this as part of this patch ?
cmd/Kconfig | 1 - cmd/bcb.c | 70 ++++++++++++++++++++++-------------- doc/android/bcb.rst | 34 +++++++++--------- drivers/fastboot/fb_common.c | 2 +- include/bcb.h | 5 +-- 5 files changed, 65 insertions(+), 47 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index df6d71c103..ce2435bbb8 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -981,7 +981,6 @@ config CMD_ADC
config CMD_BCB bool "bcb"
- depends on MMC depends on PARTITIONS help Read/modify/write the fields of Bootloader Control Block, usually
diff --git a/cmd/bcb.c b/cmd/bcb.c index 02d0c70d87..5d8c232663 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -25,6 +25,7 @@ enum bcb_cmd { BCB_CMD_STORE, };
+static enum uclass_id bcb_uclass_id = UCLASS_INVALID; static int bcb_dev = -1; static int bcb_part = -1; static struct bootloader_message bcb __aligned(ARCH_DMA_MINALIGN) = { { 0 } }; @@ -53,6 +54,9 @@ static int bcb_is_misused(int argc, char *const argv[])
switch (cmd) { case BCB_CMD_LOAD:
if (argc != 3 && argc != 4)goto err;break;@@ -115,7 +119,7 @@ static int bcb_field_get(char *name, char **fieldp, int *sizep) return 0; }
-static int __bcb_load(int devnum, const char *partp) +static int __bcb_load(const char *iface, int devnum, const char *partp) { struct blk_desc *desc; struct disk_partition info; @@ -123,14 +127,14 @@ static int __bcb_load(int devnum, const char *partp) char *endp; int part, ret;
- desc = blk_get_devnum_by_uclass_id(UCLASS_MMC, devnum);
desc = blk_get_dev(iface, devnum); if (!desc) { ret = -ENODEV; goto err_read_fail; }
/*
* always select the USER mmc hwpart in case another
* always select the first hwpart in case another*/ ret = blk_dselect_hwpart(desc, 0);
- blk operation selected a different hwpart
@@ -161,18 +165,20 @@ static int __bcb_load(int devnum, const char *partp) goto err_read_fail; }
- bcb_uclass_id = desc->uclass_id; bcb_dev = desc->devnum; bcb_part = part;
- debug("%s: Loaded from mmc %d:%d\n", __func__, bcb_dev, bcb_part);
debug("%s: Loaded from %s %d:%d\n", __func__, iface, bcb_dev, bcb_part);
return CMD_RET_SUCCESS;
err_read_fail:
- printf("Error: mmc %d:%s read failed (%d)\n", devnum, partp, ret);
- printf("Error: %s %d:%s read failed (%d)\n", iface, devnum, partp, ret); goto err;
err_too_small:
- printf("Error: mmc %d:%s too small!", devnum, partp);
- printf("Error: %s %d:%s too small!", iface, devnum, partp); goto err;
err:
- bcb_uclass_id = UCLASS_INVALID; bcb_dev = -1; bcb_part = -1;
@@ -182,15 +188,23 @@ err: static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]) {
- int devnum; char *endp;
- int devnum = simple_strtoul(argv[1], &endp, 0);
- char *iface = "mcc";
Should this be mmc instead of mcc ?
if (argc == 4) {
iface = argv[1];argc--;argv++;}
devnum = simple_strtoul(argv[1], &endp, 0); if (*endp != '\0') { printf("Error: Device id '%s' not a number\n", argv[1]); return CMD_RET_FAILURE; }
- return __bcb_load(devnum, argv[2]);
- return __bcb_load(iface, devnum, argv[2]);
}
static int __bcb_set(char *fieldp, const char *valp) @@ -298,7 +312,7 @@ static int __bcb_store(void) u64 cnt; int ret;
- desc = blk_get_devnum_by_uclass_id(UCLASS_MMC, bcb_dev);
- desc = blk_get_devnum_by_uclass_id(bcb_uclass_id, bcb_dev); if (!desc) { ret = -ENODEV; goto err;
@@ -317,7 +331,7 @@ static int __bcb_store(void)
return CMD_RET_SUCCESS; err:
- printf("Error: mmc %d:%d write failed (%d)\n", bcb_dev, bcb_part, ret);
printf("Error: %d %d:%d write failed (%d)\n", bcb_uclass_id, bcb_dev, bcb_part, ret);
return CMD_RET_FAILURE;
} @@ -328,11 +342,11 @@ static int do_bcb_store(struct cmd_tbl *cmdtp, int flag, int argc, return __bcb_store(); }
-int bcb_write_reboot_reason(int devnum, char *partp, const char *reasonp) +int bcb_write_reboot_reason(const char *iface, int devnum, char *partp, const char *reasonp) { int ret;
- ret = __bcb_load(devnum, partp);
- ret = __bcb_load(iface, devnum, partp); if (ret != CMD_RET_SUCCESS) return ret;
@@ -385,21 +399,23 @@ static int do_bcb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) U_BOOT_CMD( bcb, CONFIG_SYS_MAXARGS, 1, do_bcb, "Load/set/clear/test/dump/store Android BCB fields",
- "load <dev> <part> - load BCB from mmc <dev>:<part>\n"
- "bcb set <field> <val> - set BCB <field> to <val>\n"
- "bcb clear [<field>] - clear BCB <field> or all fields\n"
- "bcb test <field> <op> <val> - test BCB <field> against <val>\n"
- "bcb dump <field> - dump BCB <field>\n"
- "bcb store - store BCB back to mmc\n"
- "load <interface> <dev> <part> - load BCB from <interface> <dev>:<part>\n"
- "load <dev> <part> - load BCB from mmc <dev>:<part>\n"
- "bcb set <field> <val> - set BCB <field> to <val>\n"
- "bcb clear [<field>] - clear BCB <field> or all fields\n"
- "bcb test <field> <op> <val> - test BCB <field> against <val>\n"
- "bcb dump <field> - dump BCB <field>\n"
- "bcb store - store BCB back to <interface>\n" "\n" "Legend:\n"
- "<dev> - MMC device index containing the BCB partition\n"
- "<part> - MMC partition index or name containing the BCB\n"
- "<field> - one of {command,status,recovery,stage,reserved}\n"
- "<op> - the binary operator used in 'bcb test':\n"
- " '=' returns true if <val> matches the string stored in <field>\n"
- " '~' returns true if <val> matches a subset of <field>'s string\n"
- "<val> - string/text provided as input to bcb {set,test}\n"
- " NOTE: any ':' character in <val> will be replaced by line feed\n"
- " during 'bcb set' and used as separator by upper layers\n"
- "<interface> - storage device interface (virtio, mmc, etc)\n"
- "<dev> - storage device index containing the BCB partition\n"
- "<part> - partition index or name containing the BCB\n"
- "<field> - one of {command,status,recovery,stage,reserved}\n"
- "<op> - the binary operator used in 'bcb test':\n"
- " '=' returns true if <val> matches the string stored in <field>\n"
- " '~' returns true if <val> matches a subset of <field>'s string\n"
- "<val> - string/text provided as input to bcb {set,test}\n"
- " NOTE: any ':' character in <val> will be replaced by line feed\n"
- " during 'bcb set' and used as separator by upper layers\n"
); diff --git a/doc/android/bcb.rst b/doc/android/bcb.rst index 8861608300..2226517d39 100644 --- a/doc/android/bcb.rst +++ b/doc/android/bcb.rst @@ -41,23 +41,25 @@ requirements enumerated above. Below is the command's help message:: bcb - Load/set/clear/test/dump/store Android BCB fields
Usage:
- bcb load <dev> <part> - load BCB from mmc <dev>:<part>
- bcb set <field> <val> - set BCB <field> to <val>
- bcb clear [<field>] - clear BCB <field> or all fields
- bcb test <field> <op> <val> - test BCB <field> against <val>
- bcb dump <field> - dump BCB <field>
- bcb store - store BCB back to mmc
bcb load <interface> <dev> <part> - load BCB from <interface> <dev>:<part>
load <dev> <part> - load BCB from mmc <dev>:<part>
bcb set <field> <val> - set BCB <field> to <val>
bcb clear [<field>] - clear BCB <field> or all fields
bcb test <field> <op> <val> - test BCB <field> against <val>
bcb dump <field> - dump BCB <field>
bcb store - store BCB back to <interface>
Legend:
- <dev> - MMC device index containing the BCB partition
- <part> - MMC partition index or name containing the BCB
- <field> - one of {command,status,recovery,stage,reserved}
- <op> - the binary operator used in 'bcb test':
'=' returns true if <val> matches the string stored in <field>'~' returns true if <val> matches a subset of <field>'s string- <val> - string/text provided as input to bcb {set,test}
NOTE: any ':' character in <val> will be replaced by line feedduring 'bcb set' and used as separator by upper layers
- <interface> - storage device interface (virtio, mmc, etc)
- <dev> - storage device index containing the BCB partition
- <part> - partition index or name containing the BCB
- <field> - one of {command,status,recovery,stage,reserved}
- <op> - the binary operator used in 'bcb test':
'=' returns true if <val> matches the string stored in <field>'~' returns true if <val> matches a subset of <field>'s string- <val> - string/text provided as input to bcb {set,test}
NOTE: any ':' character in <val> will be replaced by line feedduring 'bcb set' and used as separator by upper layers'bcb'. Example of getting reboot reason @@ -91,7 +93,7 @@ The following Kconfig options must be enabled::
CONFIG_PARTITIONS=y CONFIG_MMC=y
- CONFIG_BCB=y
- CONFIG_CMD_BCB=y
.. [1] https://android.googlesource.com/platform/bootable/recovery .. [2] https://source.android.com/devices/bootloader diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c index 4e9d9b719c..2a6608b28c 100644 --- a/drivers/fastboot/fb_common.c +++ b/drivers/fastboot/fb_common.c @@ -105,7 +105,7 @@ int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason) if (reason >= FASTBOOT_REBOOT_REASONS_COUNT) return -EINVAL;
- return bcb_write_reboot_reason(mmc_dev, "misc", boot_cmds[reason]);
- return bcb_write_reboot_reason("mmc", mmc_dev, "misc", boot_cmds[reason]);
}
/** diff --git a/include/bcb.h b/include/bcb.h index 5edb17aa47..a6326523c4 100644 --- a/include/bcb.h +++ b/include/bcb.h @@ -9,10 +9,11 @@ #define __BCB_H__
#if IS_ENABLED(CONFIG_CMD_BCB) -int bcb_write_reboot_reason(int devnum, char *partp, const char *reasonp); +int bcb_write_reboot_reason(const char *iface, int devnum, char *partp, const char *reasonp); #else #include <linux/errno.h> -static inline int bcb_write_reboot_reason(int devnum, char *partp, const char *reasonp) +static inline int bcb_write_reboot_reason(const char *iface, int devnum,
char *partp, const char *reasonp){ return -EOPNOTSUPP; } -- 2.42.0.869.gea05f2083d-goog
On jeu., nov. 09, 2023 at 10:40, Mattijs Korpershoek mkorpershoek@baylibre.com wrote:
Hi Dmitrii,
Thank you for your patch.
Thank you as well for taking the time to upstream things from: https://android.googlesource.com/platform/external/u-boot/
On jeu., nov. 09, 2023 at 00:36, Dmitrii Merkurev dimorinny@google.com wrote:
Currently BCB command-line, C APIs and implementation only support MMC interface. Extend it to allow various block device interfaces.
Signed-off-by: Dmitrii Merkurev dimorinny@google.com Cc: Eugeniu Rosca erosca@de.adit-jv.com Cc: Ying-Chun Liu (PaulLiu) paul.liu@linaro.org Cc: Simon Glass sjg@chromium.org Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Sean Anderson sean.anderson@seco.com Cc: Cody Schuffelen schuffelen@google.com
This breaks vim3/vim3l android boot flow because both rely on the usage of the bcb command in their U-Boot environment:
Error: 1572575691 110528528: read failed (-19) Warning: BCB is corrupted or does not exist
The following diff fixes it:
diff --git a/include/configs/meson64_android.h b/include/configs/meson64_android.h index c0e977abb01f..442233e827d8 100644 --- a/include/configs/meson64_android.h +++ b/include/configs/meson64_android.h @@ -164,7 +164,7 @@ "fi; " \ "fi;" \ "if test "${run_fastboot}" -eq 0; then " \
"if bcb load " __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \
"if bcb load mmc " __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \ CONTROL_PARTITION "; then " \ "if bcb test command = bootonce-bootloader; then " \ "echo BCB: Bootloader boot...; " \@@ -195,7 +195,7 @@ "echo Recovery button is pressed;" \ "setenv run_recovery 1;" \ "fi; " \
"if bcb load " __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \
"if bcb load mmc " __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \ CONTROL_PARTITION "; then " \ "if bcb test command = boot-recovery; then " \ "echo BCB: Recovery boot...; " \diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h index 4e5aa74147d4..f571744adaf8 100644 --- a/include/configs/ti_omap5_common.h +++ b/include/configs/ti_omap5_common.h @@ -168,7 +168,7 @@ "mmc dev $mmcdev; " \ "mmc rescan; " \ AB_SELECT_SLOT \
"if bcb load " __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \
"if bcb load mmc " __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \ CONTROL_PARTITION "; then " \ "setenv ardaddr -; " \ "if bcb test command = bootonce-bootloader; then " \Can you consider including this as part of this patch ?
cmd/Kconfig | 1 - cmd/bcb.c | 70 ++++++++++++++++++++++-------------- doc/android/bcb.rst | 34 +++++++++--------- drivers/fastboot/fb_common.c | 2 +- include/bcb.h | 5 +-- 5 files changed, 65 insertions(+), 47 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index df6d71c103..ce2435bbb8 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -981,7 +981,6 @@ config CMD_ADC
config CMD_BCB bool "bcb"
- depends on MMC depends on PARTITIONS help Read/modify/write the fields of Bootloader Control Block, usually
diff --git a/cmd/bcb.c b/cmd/bcb.c index 02d0c70d87..5d8c232663 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -25,6 +25,7 @@ enum bcb_cmd { BCB_CMD_STORE, };
+static enum uclass_id bcb_uclass_id = UCLASS_INVALID; static int bcb_dev = -1; static int bcb_part = -1; static struct bootloader_message bcb __aligned(ARCH_DMA_MINALIGN) = { { 0 } }; @@ -53,6 +54,9 @@ static int bcb_is_misused(int argc, char *const argv[])
switch (cmd) { case BCB_CMD_LOAD:
if (argc != 3 && argc != 4)goto err;break;@@ -115,7 +119,7 @@ static int bcb_field_get(char *name, char **fieldp, int *sizep) return 0; }
-static int __bcb_load(int devnum, const char *partp) +static int __bcb_load(const char *iface, int devnum, const char *partp) { struct blk_desc *desc; struct disk_partition info; @@ -123,14 +127,14 @@ static int __bcb_load(int devnum, const char *partp) char *endp; int part, ret;
- desc = blk_get_devnum_by_uclass_id(UCLASS_MMC, devnum);
desc = blk_get_dev(iface, devnum); if (!desc) { ret = -ENODEV; goto err_read_fail; }
/*
* always select the USER mmc hwpart in case another
* always select the first hwpart in case another*/ ret = blk_dselect_hwpart(desc, 0);
- blk operation selected a different hwpart
@@ -161,18 +165,20 @@ static int __bcb_load(int devnum, const char *partp) goto err_read_fail; }
- bcb_uclass_id = desc->uclass_id; bcb_dev = desc->devnum; bcb_part = part;
- debug("%s: Loaded from mmc %d:%d\n", __func__, bcb_dev, bcb_part);
debug("%s: Loaded from %s %d:%d\n", __func__, iface, bcb_dev, bcb_part);
return CMD_RET_SUCCESS;
err_read_fail:
- printf("Error: mmc %d:%s read failed (%d)\n", devnum, partp, ret);
- printf("Error: %s %d:%s read failed (%d)\n", iface, devnum, partp, ret); goto err;
err_too_small:
- printf("Error: mmc %d:%s too small!", devnum, partp);
- printf("Error: %s %d:%s too small!", iface, devnum, partp); goto err;
err:
- bcb_uclass_id = UCLASS_INVALID; bcb_dev = -1; bcb_part = -1;
@@ -182,15 +188,23 @@ err: static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]) {
- int devnum; char *endp;
- int devnum = simple_strtoul(argv[1], &endp, 0);
- char *iface = "mcc";
Should this be mmc instead of mcc ?
Just to clarify: having "mmc" instead of "mcc" also fixes the bcb reading on vim3.
if (argc == 4) {
iface = argv[1];argc--;argv++;}
devnum = simple_strtoul(argv[1], &endp, 0); if (*endp != '\0') { printf("Error: Device id '%s' not a number\n", argv[1]); return CMD_RET_FAILURE; }
- return __bcb_load(devnum, argv[2]);
- return __bcb_load(iface, devnum, argv[2]);
}
static int __bcb_set(char *fieldp, const char *valp) @@ -298,7 +312,7 @@ static int __bcb_store(void) u64 cnt; int ret;
- desc = blk_get_devnum_by_uclass_id(UCLASS_MMC, bcb_dev);
- desc = blk_get_devnum_by_uclass_id(bcb_uclass_id, bcb_dev); if (!desc) { ret = -ENODEV; goto err;
@@ -317,7 +331,7 @@ static int __bcb_store(void)
return CMD_RET_SUCCESS; err:
- printf("Error: mmc %d:%d write failed (%d)\n", bcb_dev, bcb_part, ret);
printf("Error: %d %d:%d write failed (%d)\n", bcb_uclass_id, bcb_dev, bcb_part, ret);
return CMD_RET_FAILURE;
} @@ -328,11 +342,11 @@ static int do_bcb_store(struct cmd_tbl *cmdtp, int flag, int argc, return __bcb_store(); }
-int bcb_write_reboot_reason(int devnum, char *partp, const char *reasonp) +int bcb_write_reboot_reason(const char *iface, int devnum, char *partp, const char *reasonp) { int ret;
- ret = __bcb_load(devnum, partp);
- ret = __bcb_load(iface, devnum, partp); if (ret != CMD_RET_SUCCESS) return ret;
@@ -385,21 +399,23 @@ static int do_bcb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) U_BOOT_CMD( bcb, CONFIG_SYS_MAXARGS, 1, do_bcb, "Load/set/clear/test/dump/store Android BCB fields",
- "load <dev> <part> - load BCB from mmc <dev>:<part>\n"
- "bcb set <field> <val> - set BCB <field> to <val>\n"
- "bcb clear [<field>] - clear BCB <field> or all fields\n"
- "bcb test <field> <op> <val> - test BCB <field> against <val>\n"
- "bcb dump <field> - dump BCB <field>\n"
- "bcb store - store BCB back to mmc\n"
- "load <interface> <dev> <part> - load BCB from <interface> <dev>:<part>\n"
- "load <dev> <part> - load BCB from mmc <dev>:<part>\n"
- "bcb set <field> <val> - set BCB <field> to <val>\n"
- "bcb clear [<field>] - clear BCB <field> or all fields\n"
- "bcb test <field> <op> <val> - test BCB <field> against <val>\n"
- "bcb dump <field> - dump BCB <field>\n"
- "bcb store - store BCB back to <interface>\n" "\n" "Legend:\n"
- "<dev> - MMC device index containing the BCB partition\n"
- "<part> - MMC partition index or name containing the BCB\n"
- "<field> - one of {command,status,recovery,stage,reserved}\n"
- "<op> - the binary operator used in 'bcb test':\n"
- " '=' returns true if <val> matches the string stored in <field>\n"
- " '~' returns true if <val> matches a subset of <field>'s string\n"
- "<val> - string/text provided as input to bcb {set,test}\n"
- " NOTE: any ':' character in <val> will be replaced by line feed\n"
- " during 'bcb set' and used as separator by upper layers\n"
- "<interface> - storage device interface (virtio, mmc, etc)\n"
- "<dev> - storage device index containing the BCB partition\n"
- "<part> - partition index or name containing the BCB\n"
- "<field> - one of {command,status,recovery,stage,reserved}\n"
- "<op> - the binary operator used in 'bcb test':\n"
- " '=' returns true if <val> matches the string stored in <field>\n"
- " '~' returns true if <val> matches a subset of <field>'s string\n"
- "<val> - string/text provided as input to bcb {set,test}\n"
- " NOTE: any ':' character in <val> will be replaced by line feed\n"
- " during 'bcb set' and used as separator by upper layers\n"
); diff --git a/doc/android/bcb.rst b/doc/android/bcb.rst index 8861608300..2226517d39 100644 --- a/doc/android/bcb.rst +++ b/doc/android/bcb.rst @@ -41,23 +41,25 @@ requirements enumerated above. Below is the command's help message:: bcb - Load/set/clear/test/dump/store Android BCB fields
Usage:
- bcb load <dev> <part> - load BCB from mmc <dev>:<part>
- bcb set <field> <val> - set BCB <field> to <val>
- bcb clear [<field>] - clear BCB <field> or all fields
- bcb test <field> <op> <val> - test BCB <field> against <val>
- bcb dump <field> - dump BCB <field>
- bcb store - store BCB back to mmc
bcb load <interface> <dev> <part> - load BCB from <interface> <dev>:<part>
load <dev> <part> - load BCB from mmc <dev>:<part>
bcb set <field> <val> - set BCB <field> to <val>
bcb clear [<field>] - clear BCB <field> or all fields
bcb test <field> <op> <val> - test BCB <field> against <val>
bcb dump <field> - dump BCB <field>
bcb store - store BCB back to <interface>
Legend:
- <dev> - MMC device index containing the BCB partition
- <part> - MMC partition index or name containing the BCB
- <field> - one of {command,status,recovery,stage,reserved}
- <op> - the binary operator used in 'bcb test':
'=' returns true if <val> matches the string stored in <field>'~' returns true if <val> matches a subset of <field>'s string- <val> - string/text provided as input to bcb {set,test}
NOTE: any ':' character in <val> will be replaced by line feedduring 'bcb set' and used as separator by upper layers
- <interface> - storage device interface (virtio, mmc, etc)
- <dev> - storage device index containing the BCB partition
- <part> - partition index or name containing the BCB
- <field> - one of {command,status,recovery,stage,reserved}
- <op> - the binary operator used in 'bcb test':
'=' returns true if <val> matches the string stored in <field>'~' returns true if <val> matches a subset of <field>'s string- <val> - string/text provided as input to bcb {set,test}
NOTE: any ':' character in <val> will be replaced by line feedduring 'bcb set' and used as separator by upper layers'bcb'. Example of getting reboot reason @@ -91,7 +93,7 @@ The following Kconfig options must be enabled::
CONFIG_PARTITIONS=y CONFIG_MMC=y
- CONFIG_BCB=y
- CONFIG_CMD_BCB=y
.. [1] https://android.googlesource.com/platform/bootable/recovery .. [2] https://source.android.com/devices/bootloader diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c index 4e9d9b719c..2a6608b28c 100644 --- a/drivers/fastboot/fb_common.c +++ b/drivers/fastboot/fb_common.c @@ -105,7 +105,7 @@ int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason) if (reason >= FASTBOOT_REBOOT_REASONS_COUNT) return -EINVAL;
- return bcb_write_reboot_reason(mmc_dev, "misc", boot_cmds[reason]);
- return bcb_write_reboot_reason("mmc", mmc_dev, "misc", boot_cmds[reason]);
}
/** diff --git a/include/bcb.h b/include/bcb.h index 5edb17aa47..a6326523c4 100644 --- a/include/bcb.h +++ b/include/bcb.h @@ -9,10 +9,11 @@ #define __BCB_H__
#if IS_ENABLED(CONFIG_CMD_BCB) -int bcb_write_reboot_reason(int devnum, char *partp, const char *reasonp); +int bcb_write_reboot_reason(const char *iface, int devnum, char *partp, const char *reasonp); #else #include <linux/errno.h> -static inline int bcb_write_reboot_reason(int devnum, char *partp, const char *reasonp) +static inline int bcb_write_reboot_reason(const char *iface, int devnum,
char *partp, const char *reasonp){ return -EOPNOTSUPP; } -- 2.42.0.869.gea05f2083d-goog
Thank you Mattijs for taking time to verify it, uploaded v2 with "mcc" thing fixed
On Thu, Nov 9, 2023 at 2:06 AM Mattijs Korpershoek < mkorpershoek@baylibre.com> wrote:
On jeu., nov. 09, 2023 at 10:40, Mattijs Korpershoek < mkorpershoek@baylibre.com> wrote:
Hi Dmitrii,
Thank you for your patch.
Thank you as well for taking the time to upstream things from: https://android.googlesource.com/platform/external/u-boot/
On jeu., nov. 09, 2023 at 00:36, Dmitrii Merkurev dimorinny@google.com
wrote:
Currently BCB command-line, C APIs and implementation only support MMC interface. Extend it to allow various block device interfaces.
Signed-off-by: Dmitrii Merkurev dimorinny@google.com Cc: Eugeniu Rosca erosca@de.adit-jv.com Cc: Ying-Chun Liu (PaulLiu) paul.liu@linaro.org Cc: Simon Glass sjg@chromium.org Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Sean Anderson sean.anderson@seco.com Cc: Cody Schuffelen schuffelen@google.com
This breaks vim3/vim3l android boot flow because both rely on the usage of the bcb command in their U-Boot environment:
Error: 1572575691 110528528: read failed (-19) Warning: BCB is corrupted or does not exist
The following diff fixes it:
diff --git a/include/configs/meson64_android.h
b/include/configs/meson64_android.h
index c0e977abb01f..442233e827d8 100644 --- a/include/configs/meson64_android.h +++ b/include/configs/meson64_android.h @@ -164,7 +164,7 @@ "fi; " \ "fi;" \ "if test "${run_fastboot}" -eq 0; then " \
"if bcb load "__stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \
"if bcb load mmc "__stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \
CONTROL_PARTITION "; then " \ "if bcb test command =bootonce-bootloader; then " \
"echo BCB: Bootloader boot...; "\
@@ -195,7 +195,7 @@ "echo Recovery button is pressed;" \ "setenv run_recovery 1;" \ "fi; " \
"if bcb load "__stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \
"if bcb load mmc "__stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \
CONTROL_PARTITION "; then " \ "if bcb test command = boot-recovery; then " \ "echo BCB: Recovery boot...; " \diff --git a/include/configs/ti_omap5_common.h
b/include/configs/ti_omap5_common.h
index 4e5aa74147d4..f571744adaf8 100644 --- a/include/configs/ti_omap5_common.h +++ b/include/configs/ti_omap5_common.h @@ -168,7 +168,7 @@ "mmc dev $mmcdev; " \ "mmc rescan; " \ AB_SELECT_SLOT \
"if bcb load "__stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \
"if bcb load mmc "__stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) " " \
CONTROL_PARTITION "; then " \ "setenv ardaddr -; " \ "if bcb test command = bootonce-bootloader; then" \
Can you consider including this as part of this patch ?
cmd/Kconfig | 1 - cmd/bcb.c | 70 ++++++++++++++++++++++-------------- doc/android/bcb.rst | 34 +++++++++--------- drivers/fastboot/fb_common.c | 2 +- include/bcb.h | 5 +-- 5 files changed, 65 insertions(+), 47 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index df6d71c103..ce2435bbb8 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -981,7 +981,6 @@ config CMD_ADC
config CMD_BCB bool "bcb"
- depends on MMC depends on PARTITIONS help Read/modify/write the fields of Bootloader Control Block, usually
diff --git a/cmd/bcb.c b/cmd/bcb.c index 02d0c70d87..5d8c232663 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -25,6 +25,7 @@ enum bcb_cmd { BCB_CMD_STORE, };
+static enum uclass_id bcb_uclass_id = UCLASS_INVALID; static int bcb_dev = -1; static int bcb_part = -1; static struct bootloader_message bcb __aligned(ARCH_DMA_MINALIGN) = {
{ 0 } };
@@ -53,6 +54,9 @@ static int bcb_is_misused(int argc, char *const
argv[])
switch (cmd) { case BCB_CMD_LOAD:
if (argc != 3 && argc != 4)goto err;break;@@ -115,7 +119,7 @@ static int bcb_field_get(char *name, char **fieldp,
int *sizep)
return 0;}
-static int __bcb_load(int devnum, const char *partp) +static int __bcb_load(const char *iface, int devnum, const char *partp) { struct blk_desc *desc; struct disk_partition info; @@ -123,14 +127,14 @@ static int __bcb_load(int devnum, const char
*partp)
char *endp; int part, ret;
- desc = blk_get_devnum_by_uclass_id(UCLASS_MMC, devnum);
desc = blk_get_dev(iface, devnum); if (!desc) { ret = -ENODEV; goto err_read_fail; }
/*
* always select the USER mmc hwpart in case another
* always select the first hwpart in case another * blk operation selected a different hwpart */@@ -161,18 +165,20 @@ static int __bcb_load(int devnum, const char
*partp)
goto err_read_fail; }
- bcb_uclass_id = desc->uclass_id; bcb_dev = desc->devnum; bcb_part = part;
- debug("%s: Loaded from mmc %d:%d\n", __func__, bcb_dev, bcb_part);
- debug("%s: Loaded from %s %d:%d\n", __func__, iface, bcb_dev,
bcb_part);
return CMD_RET_SUCCESS;err_read_fail:
- printf("Error: mmc %d:%s read failed (%d)\n", devnum, partp, ret);
- printf("Error: %s %d:%s read failed (%d)\n", iface, devnum, partp,
ret);
goto err;err_too_small:
- printf("Error: mmc %d:%s too small!", devnum, partp);
- printf("Error: %s %d:%s too small!", iface, devnum, partp); goto err;
err:
- bcb_uclass_id = UCLASS_INVALID; bcb_dev = -1; bcb_part = -1;
@@ -182,15 +188,23 @@ err: static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]) {
- int devnum; char *endp;
- int devnum = simple_strtoul(argv[1], &endp, 0);
- char *iface = "mcc";
Should this be mmc instead of mcc ?
Just to clarify: having "mmc" instead of "mcc" also fixes the bcb reading on vim3.
if (argc == 4) {
iface = argv[1];argc--;argv++;}
devnum = simple_strtoul(argv[1], &endp, 0); if (*endp != '\0') { printf("Error: Device id '%s' not a number\n", argv[1]); return CMD_RET_FAILURE; }
- return __bcb_load(devnum, argv[2]);
- return __bcb_load(iface, devnum, argv[2]);
}
static int __bcb_set(char *fieldp, const char *valp) @@ -298,7 +312,7 @@ static int __bcb_store(void) u64 cnt; int ret;
- desc = blk_get_devnum_by_uclass_id(UCLASS_MMC, bcb_dev);
- desc = blk_get_devnum_by_uclass_id(bcb_uclass_id, bcb_dev); if (!desc) { ret = -ENODEV; goto err;
@@ -317,7 +331,7 @@ static int __bcb_store(void)
return CMD_RET_SUCCESS;err:
- printf("Error: mmc %d:%d write failed (%d)\n", bcb_dev, bcb_part,
ret);
- printf("Error: %d %d:%d write failed (%d)\n", bcb_uclass_id,
bcb_dev, bcb_part, ret);
return CMD_RET_FAILURE;} @@ -328,11 +342,11 @@ static int do_bcb_store(struct cmd_tbl *cmdtp,
int flag, int argc,
return __bcb_store();}
-int bcb_write_reboot_reason(int devnum, char *partp, const char
*reasonp)
+int bcb_write_reboot_reason(const char *iface, int devnum, char
*partp, const char *reasonp)
{ int ret;
- ret = __bcb_load(devnum, partp);
- ret = __bcb_load(iface, devnum, partp); if (ret != CMD_RET_SUCCESS) return ret;
@@ -385,21 +399,23 @@ static int do_bcb(struct cmd_tbl *cmdtp, int
flag, int argc, char *const argv[])
U_BOOT_CMD( bcb, CONFIG_SYS_MAXARGS, 1, do_bcb, "Load/set/clear/test/dump/store Android BCB fields",
- "load <dev> <part> - load BCB from mmc <dev>:<part>\n"
- "bcb set <field> <val> - set BCB <field> to <val>\n"
- "bcb clear [<field>] - clear BCB <field> or all fields\n"
- "bcb test <field> <op> <val> - test BCB <field> against <val>\n"
- "bcb dump <field> - dump BCB <field>\n"
- "bcb store - store BCB back to mmc\n"
- "load <interface> <dev> <part> - load BCB from <interface>
<dev>:<part>\n"
- "load <dev> <part> - load BCB from mmc
<dev>:<part>\n"
- "bcb set <field> <val> - set BCB <field> to <val>\n"
- "bcb clear [<field>] - clear BCB <field> or all
fields\n"
- "bcb test <field> <op> <val> - test BCB <field> against
<val>\n"
- "bcb dump <field> - dump BCB <field>\n"
- "bcb store - store BCB back to <interface>\n" "\n" "Legend:\n"
- "<dev> - MMC device index containing the BCB partition\n"
- "<part> - MMC partition index or name containing the BCB\n"
- "<field> - one of {command,status,recovery,stage,reserved}\n"
- "<op> - the binary operator used in 'bcb test':\n"
- " '=' returns true if <val> matches the string stored in
<field>\n"
- " '~' returns true if <val> matches a subset of <field>'s
string\n"
- "<val> - string/text provided as input to bcb {set,test}\n"
- " NOTE: any ':' character in <val> will be replaced by
line feed\n"
- " during 'bcb set' and used as separator by upper
layers\n"
- "<interface> - storage device interface (virtio, mmc, etc)\n"
- "<dev> - storage device index containing the BCB partition\n"
- "<part> - partition index or name containing the BCB\n"
- "<field> - one of {command,status,recovery,stage,reserved}\n"
- "<op> - the binary operator used in 'bcb test':\n"
- " '=' returns true if <val> matches the string stored
in <field>\n"
- " '~' returns true if <val> matches a subset of
<field>'s string\n"
- "<val> - string/text provided as input to bcb {set,test}\n"
- " NOTE: any ':' character in <val> will be replaced
by line feed\n"
- " during 'bcb set' and used as separator by upper
layers\n"
); diff --git a/doc/android/bcb.rst b/doc/android/bcb.rst index 8861608300..2226517d39 100644 --- a/doc/android/bcb.rst +++ b/doc/android/bcb.rst @@ -41,23 +41,25 @@ requirements enumerated above. Below is the
command's help message::
bcb - Load/set/clear/test/dump/store Android BCB fields Usage:
- bcb load <dev> <part> - load BCB from mmc <dev>:<part>
- bcb set <field> <val> - set BCB <field> to <val>
- bcb clear [<field>] - clear BCB <field> or all fields
- bcb test <field> <op> <val> - test BCB <field> against <val>
- bcb dump <field> - dump BCB <field>
- bcb store - store BCB back to mmc
- bcb load <interface> <dev> <part> - load BCB from <interface>
<dev>:<part>
load <dev> <part> - load BCB from mmc <dev>:<part>
bcb set <field> <val> - set BCB <field> to <val>
bcb clear [<field>] - clear BCB <field> or all fields
bcb test <field> <op> <val> - test BCB <field> against <val>
bcb dump <field> - dump BCB <field>
bcb store - store BCB back to <interface>
Legend:
- <dev> - MMC device index containing the BCB partition
- <part> - MMC partition index or name containing the BCB
- <field> - one of {command,status,recovery,stage,reserved}
- <op> - the binary operator used in 'bcb test':
'=' returns true if <val> matches the string stored in<field> >> - '~' returns true if <val> matches a subset of <field>'s string >> - <val> - string/text provided as input to bcb {set,test} >> - NOTE: any ':' character in <val> will be replaced by line feed >> - during 'bcb set' and used as separator by upper layers >> + <interface> - storage device interface (virtio, mmc, etc) >> + <dev> - storage device index containing the BCB partition >> + <part> - partition index or name containing the BCB >> + <field> - one of {command,status,recovery,stage,reserved} >> + <op> - the binary operator used in 'bcb test': >> + '=' returns true if <val> matches the string stored in <field> >> + '~' returns true if <val> matches a subset of <field>'s string >> + <val> - string/text provided as input to bcb {set,test} >> + NOTE: any ':' character in <val> will be replaced by line feed >> + during 'bcb set' and used as separator by upper layers >> >> >> 'bcb'. Example of getting reboot reason >> @@ -91,7 +93,7 @@ The following Kconfig options must be enabled:: >> >> CONFIG_PARTITIONS=y >> CONFIG_MMC=y >> - CONFIG_BCB=y >> + CONFIG_CMD_BCB=y >> >> .. [1] https://android.googlesource.com/platform/bootable/recovery >> .. [2] https://source.android.com/devices/bootloader >> diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c >> index 4e9d9b719c..2a6608b28c 100644 >> --- a/drivers/fastboot/fb_common.c >> +++ b/drivers/fastboot/fb_common.c >> @@ -105,7 +105,7 @@ int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason) >> if (reason >= FASTBOOT_REBOOT_REASONS_COUNT) >> return -EINVAL; >> >> - return bcb_write_reboot_reason(mmc_dev, "misc", boot_cmds[reason]); >> + return bcb_write_reboot_reason("mmc", mmc_dev, "misc", boot_cmds[reason]); >> } >> >> /** >> diff --git a/include/bcb.h b/include/bcb.h >> index 5edb17aa47..a6326523c4 100644 >> --- a/include/bcb.h >> +++ b/include/bcb.h >> @@ -9,10 +9,11 @@ >> #define __BCB_H__ >> >> #if IS_ENABLED(CONFIG_CMD_BCB) >> -int bcb_write_reboot_reason(int devnum, char *partp, const char *reasonp); >> +int bcb_write_reboot_reason(const char *iface, int devnum, char *partp, const char *reasonp); >> #else >> #include <linux/errno.h> >> -static inline int bcb_write_reboot_reason(int devnum, char *partp, const char *reasonp) >> +static inline int bcb_write_reboot_reason(const char *iface, int devnum, >> + char *partp, const char *reasonp) >> { >> return -EOPNOTSUPP; >> } >> -- >> 2.42.0.869.gea05f2083d-goog
Currently BCB C API only allows to modify 'command' BCB field. Extend it so that we can also read and modify all the available BCB fields (command, status, recovery, stage).
Co-developed-by: Cody Schuffelen schuffelen@google.com Signed-off-by: Cody Schuffelen schuffelen@google.com Signed-off-by: Dmitrii Merkurev dimorinny@google.com Cc: Eugeniu Rosca erosca@de.adit-jv.com Cc: Ying-Chun Liu (PaulLiu) paul.liu@linaro.org Cc: Simon Glass sjg@chromium.org Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Sean Anderson sean.anderson@seco.com Cc: Cody Schuffelen schuffelen@google.com --- cmd/bcb.c | 162 +++++++++++++++++++++++------------ drivers/fastboot/fb_common.c | 14 ++- include/bcb.h | 60 ++++++++++++- 3 files changed, 179 insertions(+), 57 deletions(-)
diff --git a/cmd/bcb.c b/cmd/bcb.c index 5d8c232663..7a77b7f7b0 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -25,10 +25,18 @@ enum bcb_cmd { BCB_CMD_STORE, };
-static enum uclass_id bcb_uclass_id = UCLASS_INVALID; -static int bcb_dev = -1; -static int bcb_part = -1; +static const char * const fields[] = { + "command", + "status", + "recovery", + "stage" +}; + static struct bootloader_message bcb __aligned(ARCH_DMA_MINALIGN) = { { 0 } }; +static struct disk_partition partition_data; + +static struct blk_desc *block; +static struct disk_partition *partition = &partition_data;
static int bcb_cmd_get(char *cmd) { @@ -82,7 +90,7 @@ static int bcb_is_misused(int argc, char *const argv[]) return -1; }
- if (cmd != BCB_CMD_LOAD && (bcb_dev < 0 || bcb_part < 0)) { + if (cmd != BCB_CMD_LOAD && !block) { printf("Error: Please, load BCB first!\n"); return -1; } @@ -94,7 +102,7 @@ err: return -1; }
-static int bcb_field_get(char *name, char **fieldp, int *sizep) +static int bcb_field_get(const char *name, char **fieldp, int *sizep) { if (!strcmp(name, "command")) { *fieldp = bcb.command; @@ -119,16 +127,21 @@ static int bcb_field_get(char *name, char **fieldp, int *sizep) return 0; }
-static int __bcb_load(const char *iface, int devnum, const char *partp) +static void __bcb_reset(void) +{ + block = NULL; + partition = &partition_data; + memset(&partition_data, 0, sizeof(struct disk_partition)); + memset(&bcb, 0, sizeof(struct bootloader_message)); +} + +static int __bcb_initialize(const char *iface, int devnum, const char *partp) { - struct blk_desc *desc; - struct disk_partition info; - u64 cnt; char *endp; int part, ret;
- desc = blk_get_dev(iface, devnum); - if (!desc) { + block = blk_get_dev(iface, devnum); + if (!block) { ret = -ENODEV; goto err_read_fail; } @@ -137,7 +150,7 @@ static int __bcb_load(const char *iface, int devnum, const char *partp) * always select the first hwpart in case another * blk operation selected a different hwpart */ - ret = blk_dselect_hwpart(desc, 0); + ret = blk_dselect_hwpart(block, 0); if (IS_ERR_VALUE(ret)) { ret = -ENODEV; goto err_read_fail; @@ -145,49 +158,63 @@ static int __bcb_load(const char *iface, int devnum, const char *partp)
part = simple_strtoul(partp, &endp, 0); if (*endp == '\0') { - ret = part_get_info(desc, part, &info); + ret = part_get_info(block, part, partition); if (ret) goto err_read_fail; } else { - part = part_get_info_by_name(desc, partp, &info); + part = part_get_info_by_name(block, partp, partition); if (part < 0) { ret = part; goto err_read_fail; } }
- cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), info.blksz); - if (cnt > info.size) + return CMD_RET_SUCCESS; + +err_read_fail: + printf("Error: %d %d:%s read failed (%d)\n", block->uclass_id, + block->devnum, partition->name, ret); + goto err; +err: + __bcb_reset(); + return CMD_RET_FAILURE; +} + +static int __bcb_load(void) +{ + u64 cnt; + int ret; + + cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), partition->blksz); + if (cnt > partition->size) goto err_too_small;
- if (blk_dread(desc, info.start, cnt, &bcb) != cnt) { + if (blk_dread(block, partition->start, cnt, &bcb) != cnt) { ret = -EIO; goto err_read_fail; }
- bcb_uclass_id = desc->uclass_id; - bcb_dev = desc->devnum; - bcb_part = part; - debug("%s: Loaded from %s %d:%d\n", __func__, iface, bcb_dev, bcb_part); + debug("%s: Loaded from %d %d:%s\n", __func__, block->uclass_id, + block->devnum, partition->name);
return CMD_RET_SUCCESS; err_read_fail: - printf("Error: %s %d:%s read failed (%d)\n", iface, devnum, partp, ret); + printf("Error: %d %d:%s read failed (%d)\n", block->uclass_id, + block->devnum, partition->name, ret); goto err; err_too_small: - printf("Error: %s %d:%s too small!", iface, devnum, partp); + printf("Error: %d %d:%s too small!", block->uclass_id, + block->devnum, partition->name); goto err; err: - bcb_uclass_id = UCLASS_INVALID; - bcb_dev = -1; - bcb_part = -1; - + __bcb_reset(); return CMD_RET_FAILURE; }
static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]) { + int ret; int devnum; char *endp; char *iface = "mcc"; @@ -204,10 +231,14 @@ static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc, return CMD_RET_FAILURE; }
- return __bcb_load(iface, devnum, argv[2]); + ret = __bcb_initialize(iface, devnum, argv[2]); + if (ret != CMD_RET_SUCCESS) + return ret; + + return __bcb_load(); }
-static int __bcb_set(char *fieldp, const char *valp) +static int __bcb_set(const char *fieldp, const char *valp) { int size, len; char *field, *str, *found, *tmp; @@ -307,31 +338,20 @@ static int do_bcb_dump(struct cmd_tbl *cmdtp, int flag, int argc,
static int __bcb_store(void) { - struct blk_desc *desc; - struct disk_partition info; u64 cnt; int ret;
- desc = blk_get_devnum_by_uclass_id(bcb_uclass_id, bcb_dev); - if (!desc) { - ret = -ENODEV; - goto err; - } - - ret = part_get_info(desc, bcb_part, &info); - if (ret) - goto err; - - cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), info.blksz); + cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), partition->blksz);
- if (blk_dwrite(desc, info.start, cnt, &bcb) != cnt) { + if (blk_dwrite(block, partition->start, cnt, &bcb) != cnt) { ret = -EIO; goto err; }
return CMD_RET_SUCCESS; err: - printf("Error: %d %d:%d write failed (%d)\n", bcb_uclass_id, bcb_dev, bcb_part, ret); + printf("Error: %d %d:%s write failed (%d)\n", block->uclass_id, + block->devnum, partition->name, ret);
return CMD_RET_FAILURE; } @@ -342,23 +362,59 @@ static int do_bcb_store(struct cmd_tbl *cmdtp, int flag, int argc, return __bcb_store(); }
-int bcb_write_reboot_reason(const char *iface, int devnum, char *partp, const char *reasonp) +int bcb_find_partition_and_load(const char *iface, int devnum, char *partp) { int ret;
- ret = __bcb_load(iface, devnum, partp); - if (ret != CMD_RET_SUCCESS) - return ret; + __bcb_reset();
- ret = __bcb_set("command", reasonp); + ret = __bcb_initialize(iface, devnum, partp); if (ret != CMD_RET_SUCCESS) return ret;
- ret = __bcb_store(); - if (ret != CMD_RET_SUCCESS) - return ret; + return __bcb_load(); +}
- return 0; +int bcb_load(struct blk_desc *block_description, struct disk_partition *disk_partition) +{ + __bcb_reset(); + + block = block_description; + partition = disk_partition; + + return __bcb_load(); +} + +int bcb_set(enum bcb_field field, const char *value) +{ + if (field > BCB_FIELD_STAGE) + return CMD_RET_FAILURE; + return __bcb_set(fields[field], value); +} + +int bcb_get(enum bcb_field field, char *value_out, size_t value_size) +{ + int size; + char *field_value; + + if (field > BCB_FIELD_STAGE) + return CMD_RET_FAILURE; + if (bcb_field_get(fields[field], &field_value, &size)) + return CMD_RET_FAILURE; + + strlcpy(value_out, field_value, value_size); + + return CMD_RET_SUCCESS; +} + +int bcb_store(void) +{ + return __bcb_store(); +} + +void bcb_reset(void) +{ + __bcb_reset(); }
static struct cmd_tbl cmd_bcb_sub[] = { diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c index 2a6608b28c..3576b06772 100644 --- a/drivers/fastboot/fb_common.c +++ b/drivers/fastboot/fb_common.c @@ -91,6 +91,7 @@ void fastboot_okay(const char *reason, char *response) */ int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason) { + int ret; static const char * const boot_cmds[] = { [FASTBOOT_REBOOT_REASON_BOOTLOADER] = "bootonce-bootloader", [FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot", @@ -105,7 +106,18 @@ int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason) if (reason >= FASTBOOT_REBOOT_REASONS_COUNT) return -EINVAL;
- return bcb_write_reboot_reason("mmc", mmc_dev, "misc", boot_cmds[reason]); + ret = bcb_find_partition_and_load("mmc", mmc_dev, "misc"); + if (ret) + goto out; + + ret = bcb_set(BCB_FIELD_COMMAND, boot_cmds[reason]); + if (ret) + goto out; + + ret = bcb_store(); +out: + bcb_reset(); + return ret; }
/** diff --git a/include/bcb.h b/include/bcb.h index a6326523c4..1941d8c28b 100644 --- a/include/bcb.h +++ b/include/bcb.h @@ -8,15 +8,69 @@ #ifndef __BCB_H__ #define __BCB_H__
+#include <part.h> + +enum bcb_field { + BCB_FIELD_COMMAND, + BCB_FIELD_STATUS, + BCB_FIELD_RECOVERY, + BCB_FIELD_STAGE +}; + #if IS_ENABLED(CONFIG_CMD_BCB) -int bcb_write_reboot_reason(const char *iface, int devnum, char *partp, const char *reasonp); + +int bcb_find_partition_and_load(const char *iface, + int devnum, char *partp); +int bcb_load(struct blk_desc *block_description, + struct disk_partition *disk_partition); +int bcb_set(enum bcb_field field, const char *value); + +/** + * bcb_get() - get the field value. + * @field: field to get + * @value_out: buffer to copy bcb field value to + * @value_size: buffer size to avoid overflow in case + * value_out is smaller then the field value + */ +int bcb_get(enum bcb_field field, char *value_out, size_t value_size); + +int bcb_store(void); +void bcb_reset(void); + #else + #include <linux/errno.h> -static inline int bcb_write_reboot_reason(const char *iface, int devnum, - char *partp, const char *reasonp) + +static inline int bcb_load(struct blk_desc *block_description, + struct disk_partition *disk_partition) +{ + return -EOPNOTSUPP; +} + +static inline int bcb_find_partition_and_load(const char *iface, + int devnum, char *partp) +{ + return -EOPNOTSUPP; +} + +static inline int bcb_set(enum bcb_field field, const char *value) +{ + return -EOPNOTSUPP; +} + +static inline int bcb_get(enum bcb_field field, char *value_out) { return -EOPNOTSUPP; } + +static inline int bcb_store(void) +{ + return -EOPNOTSUPP; +} + +static inline void bcb_reset(void) +{ +} #endif
#endif /* __BCB_H__ */
Hi Dmitrii,
Thank you for your patch.
On jeu., nov. 09, 2023 at 00:36, Dmitrii Merkurev dimorinny@google.com wrote:
Currently BCB C API only allows to modify 'command' BCB field. Extend it so that we can also read and modify all the available BCB fields (command, status, recovery, stage).
Co-developed-by: Cody Schuffelen schuffelen@google.com Signed-off-by: Cody Schuffelen schuffelen@google.com Signed-off-by: Dmitrii Merkurev dimorinny@google.com Cc: Eugeniu Rosca erosca@de.adit-jv.com Cc: Ying-Chun Liu (PaulLiu) paul.liu@linaro.org Cc: Simon Glass sjg@chromium.org Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Sean Anderson sean.anderson@seco.com Cc: Cody Schuffelen schuffelen@google.com
I could test this after applying the diffs from: https://lore.kernel.org/all/87a5rn9sdm.fsf@baylibre.com/
I tested with: $ fastboot reboot bootloader => bcb load mmc 2 misc => bcb dump command
I also tested => bcb set status hello => bcb dump status
Tested-by: Mattijs Korpershoek mkorpershoek@baylibre.com # on vim3
Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
Some small nitpicks below. The nitpick do not have to be adressed if you don't want to.
cmd/bcb.c | 162 +++++++++++++++++++++++------------ drivers/fastboot/fb_common.c | 14 ++- include/bcb.h | 60 ++++++++++++- 3 files changed, 179 insertions(+), 57 deletions(-)
diff --git a/cmd/bcb.c b/cmd/bcb.c index 5d8c232663..7a77b7f7b0 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -25,10 +25,18 @@ enum bcb_cmd { BCB_CMD_STORE, };
-static enum uclass_id bcb_uclass_id = UCLASS_INVALID; -static int bcb_dev = -1; -static int bcb_part = -1; +static const char * const fields[] = {
- "command",
- "status",
- "recovery",
- "stage"
+};
static struct bootloader_message bcb __aligned(ARCH_DMA_MINALIGN) = { { 0 } }; +static struct disk_partition partition_data;
+static struct blk_desc *block; +static struct disk_partition *partition = &partition_data;
static int bcb_cmd_get(char *cmd) { @@ -82,7 +90,7 @@ static int bcb_is_misused(int argc, char *const argv[]) return -1; }
- if (cmd != BCB_CMD_LOAD && (bcb_dev < 0 || bcb_part < 0)) {
- if (cmd != BCB_CMD_LOAD && !block) { printf("Error: Please, load BCB first!\n"); return -1; }
@@ -94,7 +102,7 @@ err: return -1; }
-static int bcb_field_get(char *name, char **fieldp, int *sizep) +static int bcb_field_get(const char *name, char **fieldp, int *sizep) { if (!strcmp(name, "command")) { *fieldp = bcb.command; @@ -119,16 +127,21 @@ static int bcb_field_get(char *name, char **fieldp, int *sizep) return 0; }
-static int __bcb_load(const char *iface, int devnum, const char *partp) +static void __bcb_reset(void) +{
- block = NULL;
- partition = &partition_data;
- memset(&partition_data, 0, sizeof(struct disk_partition));
- memset(&bcb, 0, sizeof(struct bootloader_message));
+}
+static int __bcb_initialize(const char *iface, int devnum, const char *partp) {
struct blk_desc *desc;
struct disk_partition info;
u64 cnt; char *endp; int part, ret;
desc = blk_get_dev(iface, devnum);
if (!desc) {
- block = blk_get_dev(iface, devnum);
- if (!block) { ret = -ENODEV; goto err_read_fail; }
@@ -137,7 +150,7 @@ static int __bcb_load(const char *iface, int devnum, const char *partp) * always select the first hwpart in case another * blk operation selected a different hwpart */
- ret = blk_dselect_hwpart(desc, 0);
- ret = blk_dselect_hwpart(block, 0); if (IS_ERR_VALUE(ret)) { ret = -ENODEV; goto err_read_fail;
@@ -145,49 +158,63 @@ static int __bcb_load(const char *iface, int devnum, const char *partp)
part = simple_strtoul(partp, &endp, 0); if (*endp == '\0') {
ret = part_get_info(desc, part, &info);
ret = part_get_info(block, part, partition);
part = part_get_info_by_name(desc, partp, &info);
part = part_get_info_by_name(block, partp, partition);
- cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), info.blksz);
- if (cnt > info.size)
- return CMD_RET_SUCCESS;
+err_read_fail:
- printf("Error: %d %d:%s read failed (%d)\n", block->uclass_id,
block->devnum, partition->name, ret);- goto err;
Nitpick: No need for this goto, we can just fall-through.
+err:
- __bcb_reset();
- return CMD_RET_FAILURE;
+}
+static int __bcb_load(void) +{
- u64 cnt;
- int ret;
- cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), partition->blksz);
- if (cnt > partition->size) goto err_too_small;
- if (blk_dread(desc, info.start, cnt, &bcb) != cnt) {
- if (blk_dread(block, partition->start, cnt, &bcb) != cnt) { ret = -EIO; goto err_read_fail; }
- bcb_uclass_id = desc->uclass_id;
- bcb_dev = desc->devnum;
- bcb_part = part;
- debug("%s: Loaded from %s %d:%d\n", __func__, iface, bcb_dev, bcb_part);
debug("%s: Loaded from %d %d:%s\n", __func__, block->uclass_id,
block->devnum, partition->name);return CMD_RET_SUCCESS;
err_read_fail:
- printf("Error: %s %d:%s read failed (%d)\n", iface, devnum, partp, ret);
- printf("Error: %d %d:%s read failed (%d)\n", block->uclass_id,
block->devnum, partition->name, ret);err_too_small:
- printf("Error: %s %d:%s too small!", iface, devnum, partp);
- printf("Error: %d %d:%s too small!", block->uclass_id,
block->devnum, partition->name);err:
- bcb_uclass_id = UCLASS_INVALID;
- bcb_dev = -1;
- bcb_part = -1;
- __bcb_reset();
Nitpick: __bcb_reset() introduction could have been done in a separate patch
return CMD_RET_FAILURE; }
static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]) {
- int ret; int devnum; char *endp; char *iface = "mcc";
@@ -204,10 +231,14 @@ static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc, return CMD_RET_FAILURE; }
- return __bcb_load(iface, devnum, argv[2]);
- ret = __bcb_initialize(iface, devnum, argv[2]);
- if (ret != CMD_RET_SUCCESS)
return ret;- return __bcb_load();
}
-static int __bcb_set(char *fieldp, const char *valp) +static int __bcb_set(const char *fieldp, const char *valp) { int size, len; char *field, *str, *found, *tmp; @@ -307,31 +338,20 @@ static int do_bcb_dump(struct cmd_tbl *cmdtp, int flag, int argc,
static int __bcb_store(void) {
struct blk_desc *desc;
struct disk_partition info; u64 cnt; int ret;
desc = blk_get_devnum_by_uclass_id(bcb_uclass_id, bcb_dev);
if (!desc) {
ret = -ENODEV;goto err;}
ret = part_get_info(desc, bcb_part, &info);
if (ret)
goto err;cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), info.blksz);
- cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), partition->blksz);
- if (blk_dwrite(desc, info.start, cnt, &bcb) != cnt) {
if (blk_dwrite(block, partition->start, cnt, &bcb) != cnt) { ret = -EIO; goto err; }
return CMD_RET_SUCCESS;
err:
- printf("Error: %d %d:%d write failed (%d)\n", bcb_uclass_id, bcb_dev, bcb_part, ret);
printf("Error: %d %d:%s write failed (%d)\n", block->uclass_id,
block->devnum, partition->name, ret);return CMD_RET_FAILURE;
} @@ -342,23 +362,59 @@ static int do_bcb_store(struct cmd_tbl *cmdtp, int flag, int argc, return __bcb_store(); }
-int bcb_write_reboot_reason(const char *iface, int devnum, char *partp, const char *reasonp) +int bcb_find_partition_and_load(const char *iface, int devnum, char *partp) { int ret;
- ret = __bcb_load(iface, devnum, partp);
- if (ret != CMD_RET_SUCCESS)
return ret;
- __bcb_reset();
- ret = __bcb_set("command", reasonp);
- ret = __bcb_initialize(iface, devnum, partp); if (ret != CMD_RET_SUCCESS) return ret;
- ret = __bcb_store();
- if (ret != CMD_RET_SUCCESS)
return ret;
- return __bcb_load();
+}
- return 0;
+int bcb_load(struct blk_desc *block_description, struct disk_partition *disk_partition) +{
- __bcb_reset();
- block = block_description;
- partition = disk_partition;
- return __bcb_load();
+}
+int bcb_set(enum bcb_field field, const char *value) +{
- if (field > BCB_FIELD_STAGE)
return CMD_RET_FAILURE;- return __bcb_set(fields[field], value);
+}
+int bcb_get(enum bcb_field field, char *value_out, size_t value_size) +{
- int size;
- char *field_value;
- if (field > BCB_FIELD_STAGE)
return CMD_RET_FAILURE;- if (bcb_field_get(fields[field], &field_value, &size))
return CMD_RET_FAILURE;- strlcpy(value_out, field_value, value_size);
- return CMD_RET_SUCCESS;
+}
+int bcb_store(void) +{
- return __bcb_store();
+}
+void bcb_reset(void) +{
- __bcb_reset();
}
static struct cmd_tbl cmd_bcb_sub[] = { diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c index 2a6608b28c..3576b06772 100644 --- a/drivers/fastboot/fb_common.c +++ b/drivers/fastboot/fb_common.c @@ -91,6 +91,7 @@ void fastboot_okay(const char *reason, char *response) */ int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason) {
- int ret; static const char * const boot_cmds[] = { [FASTBOOT_REBOOT_REASON_BOOTLOADER] = "bootonce-bootloader", [FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot",
@@ -105,7 +106,18 @@ int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason) if (reason >= FASTBOOT_REBOOT_REASONS_COUNT) return -EINVAL;
- return bcb_write_reboot_reason("mmc", mmc_dev, "misc", boot_cmds[reason]);
- ret = bcb_find_partition_and_load("mmc", mmc_dev, "misc");
- if (ret)
goto out;- ret = bcb_set(BCB_FIELD_COMMAND, boot_cmds[reason]);
- if (ret)
goto out;- ret = bcb_store();
+out:
- bcb_reset();
- return ret;
}
/** diff --git a/include/bcb.h b/include/bcb.h index a6326523c4..1941d8c28b 100644 --- a/include/bcb.h +++ b/include/bcb.h @@ -8,15 +8,69 @@ #ifndef __BCB_H__ #define __BCB_H__
+#include <part.h>
+enum bcb_field {
- BCB_FIELD_COMMAND,
- BCB_FIELD_STATUS,
- BCB_FIELD_RECOVERY,
- BCB_FIELD_STAGE
+};
#if IS_ENABLED(CONFIG_CMD_BCB) -int bcb_write_reboot_reason(const char *iface, int devnum, char *partp, const char *reasonp);
+int bcb_find_partition_and_load(const char *iface,
int devnum, char *partp);+int bcb_load(struct blk_desc *block_description,
struct disk_partition *disk_partition);+int bcb_set(enum bcb_field field, const char *value);
+/**
- bcb_get() - get the field value.
- @field: field to get
- @value_out: buffer to copy bcb field value to
- @value_size: buffer size to avoid overflow in case
value_out is smaller then the field value- */
+int bcb_get(enum bcb_field field, char *value_out, size_t value_size);
+int bcb_store(void); +void bcb_reset(void);
#else
#include <linux/errno.h> -static inline int bcb_write_reboot_reason(const char *iface, int devnum,
char *partp, const char *reasonp)
+static inline int bcb_load(struct blk_desc *block_description,
struct disk_partition *disk_partition)+{
- return -EOPNOTSUPP;
+}
+static inline int bcb_find_partition_and_load(const char *iface,
int devnum, char *partp)+{
- return -EOPNOTSUPP;
+}
+static inline int bcb_set(enum bcb_field field, const char *value) +{
- return -EOPNOTSUPP;
+}
+static inline int bcb_get(enum bcb_field field, char *value_out) { return -EOPNOTSUPP; }
+static inline int bcb_store(void) +{
- return -EOPNOTSUPP;
+}
+static inline void bcb_reset(void) +{ +} #endif
#endif /* __BCB_H__ */
2.42.0.869.gea05f2083d-goog
Uploaded v2 with fixes
On Thu, Nov 9, 2023 at 2:05 AM Mattijs Korpershoek < mkorpershoek@baylibre.com> wrote:
Hi Dmitrii,
Thank you for your patch.
On jeu., nov. 09, 2023 at 00:36, Dmitrii Merkurev dimorinny@google.com wrote:
Currently BCB C API only allows to modify 'command' BCB field. Extend it so that we can also read and modify all the available BCB fields (command, status, recovery, stage).
Co-developed-by: Cody Schuffelen schuffelen@google.com Signed-off-by: Cody Schuffelen schuffelen@google.com Signed-off-by: Dmitrii Merkurev dimorinny@google.com Cc: Eugeniu Rosca erosca@de.adit-jv.com Cc: Ying-Chun Liu (PaulLiu) paul.liu@linaro.org Cc: Simon Glass sjg@chromium.org Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Sean Anderson sean.anderson@seco.com Cc: Cody Schuffelen schuffelen@google.com
I could test this after applying the diffs from: https://lore.kernel.org/all/87a5rn9sdm.fsf@baylibre.com/
I tested with: $ fastboot reboot bootloader => bcb load mmc 2 misc => bcb dump command
I also tested => bcb set status hello => bcb dump status
Tested-by: Mattijs Korpershoek mkorpershoek@baylibre.com # on vim3
Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
Some small nitpicks below. The nitpick do not have to be adressed if you don't want to.
cmd/bcb.c | 162 +++++++++++++++++++++++------------ drivers/fastboot/fb_common.c | 14 ++- include/bcb.h | 60 ++++++++++++- 3 files changed, 179 insertions(+), 57 deletions(-)
diff --git a/cmd/bcb.c b/cmd/bcb.c index 5d8c232663..7a77b7f7b0 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -25,10 +25,18 @@ enum bcb_cmd { BCB_CMD_STORE, };
-static enum uclass_id bcb_uclass_id = UCLASS_INVALID; -static int bcb_dev = -1; -static int bcb_part = -1; +static const char * const fields[] = {
"command","status","recovery","stage"+};
static struct bootloader_message bcb __aligned(ARCH_DMA_MINALIGN) = { {
0 } };
+static struct disk_partition partition_data;
+static struct blk_desc *block; +static struct disk_partition *partition = &partition_data;
static int bcb_cmd_get(char *cmd) { @@ -82,7 +90,7 @@ static int bcb_is_misused(int argc, char *const argv[]) return -1; }
if (cmd != BCB_CMD_LOAD && (bcb_dev < 0 || bcb_part < 0)) {
if (cmd != BCB_CMD_LOAD && !block) { printf("Error: Please, load BCB first!\n"); return -1; }@@ -94,7 +102,7 @@ err: return -1; }
-static int bcb_field_get(char *name, char **fieldp, int *sizep) +static int bcb_field_get(const char *name, char **fieldp, int *sizep) { if (!strcmp(name, "command")) { *fieldp = bcb.command; @@ -119,16 +127,21 @@ static int bcb_field_get(char *name, char
**fieldp, int *sizep)
return 0;}
-static int __bcb_load(const char *iface, int devnum, const char *partp) +static void __bcb_reset(void) +{
block = NULL;partition = &partition_data;memset(&partition_data, 0, sizeof(struct disk_partition));memset(&bcb, 0, sizeof(struct bootloader_message));+}
+static int __bcb_initialize(const char *iface, int devnum, const char
*partp)
{
struct blk_desc *desc;struct disk_partition info;u64 cnt; char *endp; int part, ret;desc = blk_get_dev(iface, devnum);if (!desc) {
block = blk_get_dev(iface, devnum);if (!block) { ret = -ENODEV; goto err_read_fail; }@@ -137,7 +150,7 @@ static int __bcb_load(const char *iface, int devnum,
const char *partp)
* always select the first hwpart in case another * blk operation selected a different hwpart */
ret = blk_dselect_hwpart(desc, 0);
ret = blk_dselect_hwpart(block, 0); if (IS_ERR_VALUE(ret)) { ret = -ENODEV; goto err_read_fail;@@ -145,49 +158,63 @@ static int __bcb_load(const char *iface, int
devnum, const char *partp)
part = simple_strtoul(partp, &endp, 0); if (*endp == '\0') {
ret = part_get_info(desc, part, &info);
ret = part_get_info(block, part, partition); if (ret) goto err_read_fail; } else {
part = part_get_info_by_name(desc, partp, &info);
part = part_get_info_by_name(block, partp, partition); if (part < 0) { ret = part; goto err_read_fail; } }
cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), info.blksz);if (cnt > info.size)
return CMD_RET_SUCCESS;+err_read_fail:
printf("Error: %d %d:%s read failed (%d)\n", block->uclass_id,block->devnum, partition->name, ret);goto err;Nitpick: No need for this goto, we can just fall-through.
+err:
__bcb_reset();return CMD_RET_FAILURE;+}
+static int __bcb_load(void) +{
u64 cnt;int ret;cnt = DIV_ROUND_UP(sizeof(struct bootloader_message),partition->blksz);
if (cnt > partition->size) goto err_too_small;
if (blk_dread(desc, info.start, cnt, &bcb) != cnt) {
if (blk_dread(block, partition->start, cnt, &bcb) != cnt) { ret = -EIO; goto err_read_fail; }
bcb_uclass_id = desc->uclass_id;bcb_dev = desc->devnum;bcb_part = part;debug("%s: Loaded from %s %d:%d\n", __func__, iface, bcb_dev,bcb_part);
debug("%s: Loaded from %d %d:%s\n", __func__, block->uclass_id,block->devnum, partition->name); return CMD_RET_SUCCESS;err_read_fail:
printf("Error: %s %d:%s read failed (%d)\n", iface, devnum, partp,ret);
printf("Error: %d %d:%s read failed (%d)\n", block->uclass_id,block->devnum, partition->name, ret); goto err;err_too_small:
printf("Error: %s %d:%s too small!", iface, devnum, partp);
printf("Error: %d %d:%s too small!", block->uclass_id,block->devnum, partition->name); goto err;err:
bcb_uclass_id = UCLASS_INVALID;bcb_dev = -1;bcb_part = -1;
__bcb_reset();Nitpick: __bcb_reset() introduction could have been done in a separate patch
return CMD_RET_FAILURE;}
static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]) {
int ret; int devnum; char *endp; char *iface = "mcc";@@ -204,10 +231,14 @@ static int do_bcb_load(struct cmd_tbl *cmdtp, int
flag, int argc,
return CMD_RET_FAILURE; }
return __bcb_load(iface, devnum, argv[2]);
ret = __bcb_initialize(iface, devnum, argv[2]);if (ret != CMD_RET_SUCCESS)return ret;return __bcb_load();}
-static int __bcb_set(char *fieldp, const char *valp) +static int __bcb_set(const char *fieldp, const char *valp) { int size, len; char *field, *str, *found, *tmp; @@ -307,31 +338,20 @@ static int do_bcb_dump(struct cmd_tbl *cmdtp, int
flag, int argc,
static int __bcb_store(void) {
struct blk_desc *desc;struct disk_partition info; u64 cnt; int ret;desc = blk_get_devnum_by_uclass_id(bcb_uclass_id, bcb_dev);if (!desc) {ret = -ENODEV;goto err;}ret = part_get_info(desc, bcb_part, &info);if (ret)goto err;cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), info.blksz);
cnt = DIV_ROUND_UP(sizeof(struct bootloader_message),partition->blksz);
if (blk_dwrite(desc, info.start, cnt, &bcb) != cnt) {
if (blk_dwrite(block, partition->start, cnt, &bcb) != cnt) { ret = -EIO; goto err; } return CMD_RET_SUCCESS;err:
printf("Error: %d %d:%d write failed (%d)\n", bcb_uclass_id,bcb_dev, bcb_part, ret);
printf("Error: %d %d:%s write failed (%d)\n", block->uclass_id,block->devnum, partition->name, ret); return CMD_RET_FAILURE;} @@ -342,23 +362,59 @@ static int do_bcb_store(struct cmd_tbl *cmdtp, int
flag, int argc,
return __bcb_store();}
-int bcb_write_reboot_reason(const char *iface, int devnum, char *partp,
const char *reasonp)
+int bcb_find_partition_and_load(const char *iface, int devnum, char
*partp)
{ int ret;
ret = __bcb_load(iface, devnum, partp);if (ret != CMD_RET_SUCCESS)return ret;
__bcb_reset();
ret = __bcb_set("command", reasonp);
ret = __bcb_initialize(iface, devnum, partp); if (ret != CMD_RET_SUCCESS) return ret;
ret = __bcb_store();if (ret != CMD_RET_SUCCESS)return ret;
return __bcb_load();+}
return 0;+int bcb_load(struct blk_desc *block_description, struct disk_partition
*disk_partition)
+{
__bcb_reset();block = block_description;partition = disk_partition;return __bcb_load();+}
+int bcb_set(enum bcb_field field, const char *value) +{
if (field > BCB_FIELD_STAGE)return CMD_RET_FAILURE;return __bcb_set(fields[field], value);+}
+int bcb_get(enum bcb_field field, char *value_out, size_t value_size) +{
int size;char *field_value;if (field > BCB_FIELD_STAGE)return CMD_RET_FAILURE;if (bcb_field_get(fields[field], &field_value, &size))return CMD_RET_FAILURE;strlcpy(value_out, field_value, value_size);return CMD_RET_SUCCESS;+}
+int bcb_store(void) +{
return __bcb_store();+}
+void bcb_reset(void) +{
__bcb_reset();}
static struct cmd_tbl cmd_bcb_sub[] = { diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c index 2a6608b28c..3576b06772 100644 --- a/drivers/fastboot/fb_common.c +++ b/drivers/fastboot/fb_common.c @@ -91,6 +91,7 @@ void fastboot_okay(const char *reason, char *response) */ int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason) {
int ret; static const char * const boot_cmds[] = { [FASTBOOT_REBOOT_REASON_BOOTLOADER] ="bootonce-bootloader",
[FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot",@@ -105,7 +106,18 @@ int __weak fastboot_set_reboot_flag(enum
fastboot_reboot_reason reason)
if (reason >= FASTBOOT_REBOOT_REASONS_COUNT) return -EINVAL;
return bcb_write_reboot_reason("mmc", mmc_dev, "misc",boot_cmds[reason]);
ret = bcb_find_partition_and_load("mmc", mmc_dev, "misc");if (ret)goto out;ret = bcb_set(BCB_FIELD_COMMAND, boot_cmds[reason]);if (ret)goto out;ret = bcb_store();+out:
bcb_reset();return ret;}
/** diff --git a/include/bcb.h b/include/bcb.h index a6326523c4..1941d8c28b 100644 --- a/include/bcb.h +++ b/include/bcb.h @@ -8,15 +8,69 @@ #ifndef __BCB_H__ #define __BCB_H__
+#include <part.h>
+enum bcb_field {
BCB_FIELD_COMMAND,BCB_FIELD_STATUS,BCB_FIELD_RECOVERY,BCB_FIELD_STAGE+};
#if IS_ENABLED(CONFIG_CMD_BCB) -int bcb_write_reboot_reason(const char *iface, int devnum, char *partp,
const char *reasonp);
+int bcb_find_partition_and_load(const char *iface,
int devnum, char *partp);+int bcb_load(struct blk_desc *block_description,
struct disk_partition *disk_partition);+int bcb_set(enum bcb_field field, const char *value);
+/**
- bcb_get() - get the field value.
- @field: field to get
- @value_out: buffer to copy bcb field value to
- @value_size: buffer size to avoid overflow in case
value_out is smaller then the field value- */
+int bcb_get(enum bcb_field field, char *value_out, size_t value_size);
+int bcb_store(void); +void bcb_reset(void);
#else
#include <linux/errno.h> -static inline int bcb_write_reboot_reason(const char *iface, int devnum,
char *partp, const char *reasonp)
+static inline int bcb_load(struct blk_desc *block_description,
struct disk_partition *disk_partition)+{
return -EOPNOTSUPP;+}
+static inline int bcb_find_partition_and_load(const char *iface,
int devnum, char *partp)+{
return -EOPNOTSUPP;+}
+static inline int bcb_set(enum bcb_field field, const char *value) +{
return -EOPNOTSUPP;+}
+static inline int bcb_get(enum bcb_field field, char *value_out) { return -EOPNOTSUPP; }
+static inline int bcb_store(void) +{
return -EOPNOTSUPP;+}
+static inline void bcb_reset(void) +{ +} #endif
#endif /* __BCB_H__ */
2.42.0.869.gea05f2083d-goog
participants (2)
-
Dmitrii Merkurev -
Mattijs Korpershoek