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