* [PATCH 0/2] cmd: bcb: extend BCB APIs to support Android boot flow @ 2023-11-09 0:36 Dmitrii Merkurev 2023-11-09 0:36 ` [PATCH 1/2] cmd: bcb: support various block device interfaces for BCB command Dmitrii Merkurev 2023-11-09 0:36 ` [PATCH 2/2] cmd: bcb: extend BCB C API to allow read/write the fields Dmitrii Merkurev 0 siblings, 2 replies; 8+ messages in thread From: Dmitrii Merkurev @ 2023-11-09 0:36 UTC (permalink / raw) To: u-boot Cc: rammuthiah, Dmitrii Merkurev, Eugeniu Rosca, Ying-Chun Liu, Simon Glass, Mattijs Korpershoek, Sean Anderson, Cody Schuffelen 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(-) -- 2.42.0.869.gea05f2083d-goog ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] cmd: bcb: support various block device interfaces for BCB command 2023-11-09 0:36 [PATCH 0/2] cmd: bcb: extend BCB APIs to support Android boot flow Dmitrii Merkurev @ 2023-11-09 0:36 ` Dmitrii Merkurev 2023-11-09 9:40 ` Mattijs Korpershoek 2023-11-09 0:36 ` [PATCH 2/2] cmd: bcb: extend BCB C API to allow read/write the fields Dmitrii Merkurev 1 sibling, 1 reply; 8+ messages in thread From: Dmitrii Merkurev @ 2023-11-09 0:36 UTC (permalink / raw) To: u-boot Cc: rammuthiah, Dmitrii Merkurev, Eugeniu Rosca, Ying-Chun Liu, Simon Glass, Mattijs Korpershoek, Sean Anderson, Cody Schuffelen 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; } -- 2.42.0.869.gea05f2083d-goog ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] cmd: bcb: support various block device interfaces for BCB command 2023-11-09 0:36 ` [PATCH 1/2] cmd: bcb: support various block device interfaces for BCB command Dmitrii Merkurev @ 2023-11-09 9:40 ` Mattijs Korpershoek 2023-11-09 10:06 ` Mattijs Korpershoek 0 siblings, 1 reply; 8+ messages in thread From: Mattijs Korpershoek @ 2023-11-09 9:40 UTC (permalink / raw) To: Dmitrii Merkurev, u-boot Cc: rammuthiah, Dmitrii Merkurev, Eugeniu Rosca, Ying-Chun Liu, Simon Glass, Sean Anderson, Cody Schuffelen 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; > 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"; 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 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] cmd: bcb: support various block device interfaces for BCB command 2023-11-09 9:40 ` Mattijs Korpershoek @ 2023-11-09 10:06 ` Mattijs Korpershoek 2023-11-10 6:05 ` Dmitrii Merkurev 0 siblings, 1 reply; 8+ messages in thread From: Mattijs Korpershoek @ 2023-11-09 10:06 UTC (permalink / raw) To: Dmitrii Merkurev, u-boot Cc: rammuthiah, Dmitrii Merkurev, Eugeniu Rosca, Ying-Chun Liu, Simon Glass, Sean Anderson, Cody Schuffelen 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; >> 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"; > > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] cmd: bcb: support various block device interfaces for BCB command 2023-11-09 10:06 ` Mattijs Korpershoek @ 2023-11-10 6:05 ` Dmitrii Merkurev 0 siblings, 0 replies; 8+ messages in thread From: Dmitrii Merkurev @ 2023-11-10 6:05 UTC (permalink / raw) To: Mattijs Korpershoek Cc: u-boot, rammuthiah, Eugeniu Rosca, Ying-Chun Liu, Simon Glass, Sean Anderson, Cody Schuffelen 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; > >> 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"; > > > > 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] cmd: bcb: extend BCB C API to allow read/write the fields 2023-11-09 0:36 [PATCH 0/2] cmd: bcb: extend BCB APIs to support Android boot flow Dmitrii Merkurev 2023-11-09 0:36 ` [PATCH 1/2] cmd: bcb: support various block device interfaces for BCB command Dmitrii Merkurev @ 2023-11-09 0:36 ` Dmitrii Merkurev 2023-11-09 10:04 ` Mattijs Korpershoek 1 sibling, 1 reply; 8+ messages in thread From: Dmitrii Merkurev @ 2023-11-09 0:36 UTC (permalink / raw) To: u-boot Cc: rammuthiah, Dmitrii Merkurev, Cody Schuffelen, Eugeniu Rosca, Ying-Chun Liu, Simon Glass, Mattijs Korpershoek, Sean Anderson 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__ */ -- 2.42.0.869.gea05f2083d-goog ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] cmd: bcb: extend BCB C API to allow read/write the fields 2023-11-09 0:36 ` [PATCH 2/2] cmd: bcb: extend BCB C API to allow read/write the fields Dmitrii Merkurev @ 2023-11-09 10:04 ` Mattijs Korpershoek 2023-11-10 6:06 ` Dmitrii Merkurev 0 siblings, 1 reply; 8+ messages in thread From: Mattijs Korpershoek @ 2023-11-09 10:04 UTC (permalink / raw) To: Dmitrii Merkurev, u-boot Cc: rammuthiah, Dmitrii Merkurev, Cody Schuffelen, Eugeniu Rosca, Ying-Chun Liu, Simon Glass, Sean Anderson 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] cmd: bcb: extend BCB C API to allow read/write the fields 2023-11-09 10:04 ` Mattijs Korpershoek @ 2023-11-10 6:06 ` Dmitrii Merkurev 0 siblings, 0 replies; 8+ messages in thread From: Dmitrii Merkurev @ 2023-11-10 6:06 UTC (permalink / raw) To: Mattijs Korpershoek Cc: u-boot, rammuthiah, Cody Schuffelen, Eugeniu Rosca, Ying-Chun Liu, Simon Glass, Sean Anderson 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-11-10 6:07 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-11-09 0:36 [PATCH 0/2] cmd: bcb: extend BCB APIs to support Android boot flow Dmitrii Merkurev 2023-11-09 0:36 ` [PATCH 1/2] cmd: bcb: support various block device interfaces for BCB command Dmitrii Merkurev 2023-11-09 9:40 ` Mattijs Korpershoek 2023-11-09 10:06 ` Mattijs Korpershoek 2023-11-10 6:05 ` Dmitrii Merkurev 2023-11-09 0:36 ` [PATCH 2/2] cmd: bcb: extend BCB C API to allow read/write the fields Dmitrii Merkurev 2023-11-09 10:04 ` Mattijs Korpershoek 2023-11-10 6:06 ` Dmitrii Merkurev
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox