[PATCH 0/2] cmd: bcb: extend BCB APIs to support Android boot flow

[PATCH 0/2] cmd: bcb: extend BCB APIs to support Android boot flow

[Dmitrii Merkurev]

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

[PATCH 0/2] cmd: bcb: extend BCB APIs to support Android boot flow

[Dmitrii Merkurev]

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>

---
Changes for v2:
- "mcc" to "mmc" fixes
- Clean some gotos

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                    | 205 +++++++++++++++++++++++------------
 doc/android/bcb.rst          |  34 +++---
 drivers/fastboot/fb_common.c |  14 ++-
 include/bcb.h                |  59 +++++++++-
 5 files changed, 225 insertions(+), 88 deletions(-)

-- 
2.43.0.rc0.421.g78406f8d94-goog

[PATCH v2 1/2] cmd: bcb: support various block device interfaces for BCB command

[Dmitrii Merkurev]

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..6594ac6439 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 = "mmc";
+
+	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.43.0.rc0.421.g78406f8d94-goog

[PATCH v2 2/2] cmd: bcb: extend BCB C API to allow read/write the fields

[Dmitrii Merkurev]

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>
Tested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> # on vim3
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
 cmd/bcb.c                    | 161 +++++++++++++++++++++++------------
 drivers/fastboot/fb_common.c |  14 ++-
 include/bcb.h                |  60 ++++++++++++-
 3 files changed, 177 insertions(+), 58 deletions(-)

diff --git a/cmd/bcb.c b/cmd/bcb.c
index 6594ac6439..f3b92564d1 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,60 @@ 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);
+	__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);
-	goto err;
+	printf("Error: %d %d:%s too small!", block->uclass_id,
+	       block->devnum, partition->name);
 err:
-	bcb_uclass_id = UCLASS_INVALID;
-	bcb_dev = -1;
-	bcb_part = -1;
-
+	__bcb_reset();
 	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 = "mmc";
@@ -204,10 +228,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 +335,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;
-	}
+	cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), partition->blksz);
 
-	ret = part_get_info(desc, bcb_part, &info);
-	if (ret)
-		goto err;
-
-	cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), info.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 +359,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.43.0.rc0.421.g78406f8d94-goog

Re: [PATCH v2 1/2] cmd: bcb: support various block device interfaces for BCB command

[Mattijs Korpershoek]

Hi Dmitrii,

Thank you for  your patch.

On ven., nov. 10, 2023 at 05:59, 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>

I confirm that I can reboot from U-Boot into other modes (like
fastbootd) using the default U-boot environment

Tested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> # on vim3

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.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..6594ac6439 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 = "mmc";
> +
> +	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.43.0.rc0.421.g78406f8d94-goog

Re: [PATCH 0/2] cmd: bcb: extend BCB APIs to support Android boot flow

[Mattijs Korpershoek]

Hi Dmitrii,

Next time, please add the version (v2) in the cover letter as you did in
the patches (subject should start with "[PATCH v2 0/2]")

No need to respin the series for this.

Thanks again for your contribution!

Cheers
Mattijs

On ven., nov. 10, 2023 at 05:59, Dmitrii Merkurev <dimorinny@google.com> wrote:

> 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>
>
> ---
> Changes for v2:
> - "mcc" to "mmc" fixes
> - Clean some gotos
>
> 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                    | 205 +++++++++++++++++++++++------------
>  doc/android/bcb.rst          |  34 +++---
>  drivers/fastboot/fb_common.c |  14 ++-
>  include/bcb.h                |  59 +++++++++-
>  5 files changed, 225 insertions(+), 88 deletions(-)
>
> -- 
> 2.43.0.rc0.421.g78406f8d94-goog

Re: [PATCH v2 1/2] cmd: bcb: support various block device interfaces for BCB command

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]

On Fri, Nov 10, 2023 at 05:59:54AM +0000, Dmitrii Merkurev 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>
> Tested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> # on vim3
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 2/2] cmd: bcb: extend BCB C API to allow read/write the fields

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 899 bytes --]

On Fri, Nov 10, 2023 at 05:59:55AM +0000, Dmitrii Merkurev 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>
> Tested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> # on vim3
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]