[RESEND 0/7] Refactor generic fastboot_set_reboot_flag implementation

[RESEND 0/7] Refactor generic fastboot_set_reboot_flag implementation

[Roman Kovalivskyi]

Current generic implementation of fastboot_set_reboot_flag is somewhat
messy and requires some additional configuration option to be enabled
besides CMD_BCB, so it reverts that implementtion in order to bring a
new cleaner one.

New function called bcb_set_reboot_reason should be exposed by BCB
commands, so that all of the details could be put there instead of
dealing with it in fastboot implementation directly. After that,
fastboot_set_reboot_flag could simply pass correct reboot reason
string to the BCB implementation. If CMD_BCB is disabled then the
whole operation would return error code, which is no different
behaviour than the current implementation.

Eugeniu Rosca (5):
  cmd: bcb: Extract '__bcb_load' from 'do_bcb_load' for internal needs
  cmd: bcb: Extract '__bcb_set' from 'do_bcb_set' for internal needs
  cmd: bcb: Extract '__bcb_store' from 'do_bcb_store' for internal needs
  cmd: bcb: Expose 'bcb_write_reboot_reason' to external callers
  cmd: bcb: Add support for processing const string literals in
    bcb_set()

Roman Kovalivskyi (2):
  Revert "fastboot: Add default fastboot_set_reboot_flag implementation"
  fastboot: Implement generic fastboot_set_reboot_flag

 cmd/bcb.c                      | 88 +++++++++++++++++++++++++++-------
 drivers/fastboot/Kconfig       | 12 -----
 drivers/fastboot/Makefile      |  1 -
 drivers/fastboot/fb_bcb_impl.c | 43 -----------------
 drivers/fastboot/fb_common.c   | 12 ++++-
 include/bcb.h                  | 20 ++++++++
 include/fastboot.h             |  9 ----
 7 files changed, 101 insertions(+), 84 deletions(-)
 delete mode 100644 drivers/fastboot/fb_bcb_impl.c
 create mode 100644 include/bcb.h

-- 
2.17.1

[RESEND 1/7] cmd: bcb: Extract '__bcb_load' from 'do_bcb_load' for internal needs

[Roman Kovalivskyi]

From: Eugeniu Rosca <erosca@de.adit-jv.com>

Enriching the functionality of U-Boot 'bcb' may assume using the
existing sub-commands as building blocks for the next ones.

A clean way to achive the above is to expose a number of static
routines, each mapped to an existing user command (e.g. load/set/store),
with a user/caller-friendly prototype (i.e. do not force the caller
to wrap an integer into a string).

This first patch makes '__bcb_load' available for internal needs.

No functional change, except for a tiny update in error handling.

Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Signed-off-by: Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
---
 cmd/bcb.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/cmd/bcb.c b/cmd/bcb.c
index e03218066bf2..2ed8b801a3e2 100644
--- a/cmd/bcb.c
+++ b/cmd/bcb.c
@@ -110,8 +110,7 @@ static int bcb_field_get(char *name, char **fieldp, int *sizep)
 	return 0;
 }
 
-static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc,
-		       char *const argv[])
+static int __bcb_load(int devnum, const char *partp)
 {
 	struct blk_desc *desc;
 	struct disk_partition info;
@@ -119,17 +118,19 @@ static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc,
 	char *endp;
 	int part, ret;
 
-	ret = blk_get_device_by_str("mmc", argv[1], &desc);
-	if (ret < 0)
+	desc = blk_get_devnum_by_type(IF_TYPE_MMC, devnum);
+	if (!desc) {
+		ret = -ENODEV;
 		goto err_read_fail;
+	}
 
-	part = simple_strtoul(argv[2], &endp, 0);
+	part = simple_strtoul(partp, &endp, 0);
 	if (*endp == '\0') {
 		ret = part_get_info(desc, part, &info);
 		if (ret)
 			goto err_read_fail;
 	} else {
-		part = part_get_info_by_name(desc, argv[2], &info);
+		part = part_get_info_by_name(desc, partp, &info);
 		if (part < 0) {
 			ret = part;
 			goto err_read_fail;
@@ -151,10 +152,10 @@ static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc,
 
 	return CMD_RET_SUCCESS;
 err_read_fail:
-	printf("Error: mmc %s:%s read failed (%d)\n", argv[1], argv[2], ret);
+	printf("Error: mmc %d:%s read failed (%d)\n", devnum, partp, ret);
 	goto err;
 err_too_small:
-	printf("Error: mmc %s:%s too small!", argv[1], argv[2]);
+	printf("Error: mmc %d:%s too small!", devnum, partp);
 	goto err;
 err:
 	bcb_dev = -1;
@@ -163,6 +164,20 @@ err:
 	return CMD_RET_FAILURE;
 }
 
+static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc,
+		       char * const argv[])
+{
+	char *endp;
+	int 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]);
+}
+
 static int do_bcb_set(struct cmd_tbl *cmdtp, int flag, int argc,
 		      char *const argv[])
 {
-- 
2.17.1

[RESEND 2/7] cmd: bcb: Extract '__bcb_set' from 'do_bcb_set' for internal needs

[Roman Kovalivskyi]

From: Eugeniu Rosca <erosca@de.adit-jv.com>

Enriching the functionality of U-Boot 'bcb' may assume using the
existing sub-commands as building blocks for the next ones.

A clean way to achive the above is to expose a number of static
routines, each mapped to an existing user command (e.g. load/set/store),
with a user/caller-friendly prototype (i.e. do not force the caller
to wrap an integer into a string).

This second patch makes '__bcb_set' available for internal needs.

No functional change intended.

Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Signed-off-by: Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
---
 cmd/bcb.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/cmd/bcb.c b/cmd/bcb.c
index 2ed8b801a3e2..113f04ffe6b2 100644
--- a/cmd/bcb.c
+++ b/cmd/bcb.c
@@ -178,22 +178,21 @@ static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc,
 	return __bcb_load(devnum, argv[2]);
 }
 
-static int do_bcb_set(struct cmd_tbl *cmdtp, int flag, int argc,
-		      char *const argv[])
+static int __bcb_set(char *fieldp, char *valp)
 {
 	int size, len;
 	char *field, *str, *found;
 
-	if (bcb_field_get(argv[1], &field, &size))
+	if (bcb_field_get(fieldp, &field, &size))
 		return CMD_RET_FAILURE;
 
-	len = strlen(argv[2]);
+	len = strlen(valp);
 	if (len >= size) {
 		printf("Error: sizeof('%s') = %d >= %d = sizeof(bcb.%s)\n",
-		       argv[2], len, size, argv[1]);
+		       valp, len, size, fieldp);
 		return CMD_RET_FAILURE;
 	}
-	str = argv[2];
+	str = valp;
 
 	field[0] = '\0';
 	while ((found = strsep(&str, ":"))) {
@@ -205,6 +204,12 @@ static int do_bcb_set(struct cmd_tbl *cmdtp, int flag, int argc,
 	return CMD_RET_SUCCESS;
 }
 
+static int do_bcb_set(struct cmd_tbl *cmdtp, int flag, int argc,
+		      char * const argv[])
+{
+	return __bcb_set(argv[1], argv[2]);
+}
+
 static int do_bcb_clear(struct cmd_tbl *cmdtp, int flag, int argc,
 			char *const argv[])
 {
-- 
2.17.1

[RESEND 3/7] cmd: bcb: Extract '__bcb_store' from 'do_bcb_store' for internal needs

[Roman Kovalivskyi]

From: Eugeniu Rosca <erosca@de.adit-jv.com>

Enriching the functionality of U-Boot 'bcb' may assume using the
existing sub-commands as building blocks for the next ones.

A clean way to achive the above is to expose a number of static
routines, each mapped to an existing user command (e.g. load/set/store),
with a user/caller-friendly prototype (i.e. do not force the caller
to wrap an integer into a string).

This third patch makes '__bcb_store' available for internal needs.

No functional change intended.

Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Signed-off-by: Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
---
 cmd/bcb.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/cmd/bcb.c b/cmd/bcb.c
index 113f04ffe6b2..b9cd20ea3d56 100644
--- a/cmd/bcb.c
+++ b/cmd/bcb.c
@@ -270,8 +270,7 @@ static int do_bcb_dump(struct cmd_tbl *cmdtp, int flag, int argc,
 	return CMD_RET_SUCCESS;
 }
 
-static int do_bcb_store(struct cmd_tbl *cmdtp, int flag, int argc,
-			char *const argv[])
+static int __bcb_store(void)
 {
 	struct blk_desc *desc;
 	struct disk_partition info;
@@ -302,6 +301,12 @@ err:
 	return CMD_RET_FAILURE;
 }
 
+static int do_bcb_store(struct cmd_tbl *cmdtp, int flag, int argc,
+			char * const argv[])
+{
+	return __bcb_store();
+}
+
 static struct cmd_tbl cmd_bcb_sub[] = {
 	U_BOOT_CMD_MKENT(load, CONFIG_SYS_MAXARGS, 1, do_bcb_load, "", ""),
 	U_BOOT_CMD_MKENT(set, CONFIG_SYS_MAXARGS, 1, do_bcb_set, "", ""),
-- 
2.17.1

[RESEND 4/7] cmd: bcb: Expose 'bcb_write_reboot_reason' to external callers

[Roman Kovalivskyi]

From: Eugeniu Rosca <erosca@de.adit-jv.com>

Fastboot is evolving and beginning with commit [1], the
upstream implementation expects bootloaders to offer support for:
 - reboot-recovery
 - reboot-fastboot

The most natural way to achieve the above is through a set of
pre-defined "reboot reason" strings, written into / read from
the BCB "command" field, e.g.:
 - bootonce-bootloader [2]
 - boot-fastboot [3]
 - boot-recovery [4]

Expose the first 'bcb' API meant to be called by e.g. fastboot stack,
to allow updating the BCB reboot reason via the BCB 'command' field.

[1] https://android.googlesource.com/platform/system/core/+/dea91b4b5354af2
    ("Add fastbootd.")
[2] https://android.googlesource.com/platform/bootable/recovery/+/cba7fa88d8b9
    ("Add 'reboot bootloader' to bootloader_message.")
[3] https://android.googlesource.com/platform/bootable/recovery/+/eee4e260f9f6
    ("recovery: Add "boot-fastboot" command to BCB.")
[4] https://android.googlesource.com/platform/system/core/+/5e98b633a748695f
    ("init: Write the reason in BCB on "reboot recovery"")

Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Signed-off-by: Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
---
 cmd/bcb.c     | 20 ++++++++++++++++++++
 include/bcb.h | 20 ++++++++++++++++++++
 2 files changed, 40 insertions(+)
 create mode 100644 include/bcb.h

diff --git a/cmd/bcb.c b/cmd/bcb.c
index b9cd20ea3d56..5da3526142ad 100644
--- a/cmd/bcb.c
+++ b/cmd/bcb.c
@@ -6,6 +6,7 @@
  */
 
 #include <android_bootloader_message.h>
+#include <bcb.h>
 #include <command.h>
 #include <common.h>
 #include <log.h>
@@ -307,6 +308,25 @@ 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, char *reasonp)
+{
+	int ret;
+
+	ret = __bcb_load(devnum, partp);
+	if (ret != CMD_RET_SUCCESS)
+		return ret;
+
+	ret = __bcb_set("command", reasonp);
+	if (ret != CMD_RET_SUCCESS)
+		return ret;
+
+	ret = __bcb_store();
+	if (ret != CMD_RET_SUCCESS)
+		return ret;
+
+	return 0;
+}
+
 static struct cmd_tbl cmd_bcb_sub[] = {
 	U_BOOT_CMD_MKENT(load, CONFIG_SYS_MAXARGS, 1, do_bcb_load, "", ""),
 	U_BOOT_CMD_MKENT(set, CONFIG_SYS_MAXARGS, 1, do_bcb_set, "", ""),
diff --git a/include/bcb.h b/include/bcb.h
new file mode 100644
index 000000000000..9b662d1203a6
--- /dev/null
+++ b/include/bcb.h
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020 Eugeniu Rosca <rosca.eugeniu@gmail.com>
+ *
+ * Android Bootloader Control Block Header
+ */
+
+#ifndef __BCB_H__
+#define __BCB_H__
+
+#if CONFIG_IS_ENABLED(CMD_BCB)
+int bcb_write_reboot_reason(int devnum, char *partp, char *reasonp);
+#else
+static inline int bcb_write_reboot_reason(int devnum, char *partp, char *reasonp)
+{
+	return -EOPNOTSUPP;
+}
+#endif
+
+#endif /* __BCB_H__ */
-- 
2.17.1

[RESEND 5/7] cmd: bcb: Add support for processing const string literals in bcb_set()

[Roman Kovalivskyi]

From: Eugeniu Rosca <erosca@de.adit-jv.com>

On request/suggestion from Simon Glass back in May 22 2019 [1], the
'strsep' mechanism implemented in bcb_set() was set to work directly
with user-provided argv strings, to avoid duplicating memory and for
the sake of simpler implementation.

However, since we recently exposed bcb_write_reboot_reason() API to be
called by U-Boot fastboot, the idea is to be able to pass const string
literals to this new BCB API, carrying the reboot reason.

Since 'strsep' (just like its older/superseded sibling 'strtok')
modifies the input string passed as parameter, BCB command in its
current state would attempt to perform in-place modifications in a
readonly string, which might lead to unexpected results.

Fix the above with the cost of one dynamic memory allocation ('strdup').
This will also ensure no compiler warnings when passing string literals
to bcb_write_reboot_reason().

[1] http://u-boot.10912.n7.nabble.com/PATCH-v2-0-2-Add-bcb-command-to-read-modify-write-Android-BCB-td369934i20.html#a370456

Cc: Simon Glass <sjg@chromium.org>
Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Signed-off-by: Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
---
 cmd/bcb.c     | 17 ++++++++++++-----
 include/bcb.h |  4 ++--
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/cmd/bcb.c b/cmd/bcb.c
index 5da3526142ad..6b6f1e9a2f10 100644
--- a/cmd/bcb.c
+++ b/cmd/bcb.c
@@ -11,6 +11,7 @@
 #include <common.h>
 #include <log.h>
 #include <part.h>
+#include <malloc.h>
 
 enum bcb_cmd {
 	BCB_CMD_LOAD,
@@ -179,10 +180,10 @@ static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc,
 	return __bcb_load(devnum, argv[2]);
 }
 
-static int __bcb_set(char *fieldp, char *valp)
+static int __bcb_set(char *fieldp, const char *valp)
 {
 	int size, len;
-	char *field, *str, *found;
+	char *field, *str, *found, *tmp;
 
 	if (bcb_field_get(fieldp, &field, &size))
 		return CMD_RET_FAILURE;
@@ -193,14 +194,20 @@ static int __bcb_set(char *fieldp, char *valp)
 		       valp, len, size, fieldp);
 		return CMD_RET_FAILURE;
 	}
-	str = valp;
+	str = strdup(valp);
+	if (!str) {
+		printf("Error: Out of memory while strdup\n");
+		return CMD_RET_FAILURE;
+	}
 
+	tmp = str;
 	field[0] = '\0';
-	while ((found = strsep(&str, ":"))) {
+	while ((found = strsep(&tmp, ":"))) {
 		if (field[0] != '\0')
 			strcat(field, "\n");
 		strcat(field, found);
 	}
+	free(str);
 
 	return CMD_RET_SUCCESS;
 }
@@ -308,7 +315,7 @@ 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, char *reasonp)
+int bcb_write_reboot_reason(int devnum, char *partp, const char *reasonp)
 {
 	int ret;
 
diff --git a/include/bcb.h b/include/bcb.h
index 9b662d1203a6..9ef4928642ca 100644
--- a/include/bcb.h
+++ b/include/bcb.h
@@ -9,9 +9,9 @@
 #define __BCB_H__
 
 #if CONFIG_IS_ENABLED(CMD_BCB)
-int bcb_write_reboot_reason(int devnum, char *partp, char *reasonp);
+int bcb_write_reboot_reason(int devnum, char *partp, const char *reasonp);
 #else
-static inline int bcb_write_reboot_reason(int devnum, char *partp, char *reasonp)
+static inline int bcb_write_reboot_reason(int devnum, char *partp, const char *reasonp)
 {
 	return -EOPNOTSUPP;
 }
-- 
2.17.1

[RESEND 6/7] Revert "fastboot: Add default fastboot_set_reboot_flag implementation"

[Roman Kovalivskyi]

This reverts commit 0ebf9842e56c5b8cb7cb1f990bb452cc14af6225.

Current generic implementation of fastboot_set_reboot_flag is somewhat
messy and requires some additional configuration option to be enabled
besides CMD_BCB, so it reverts that implementtion in order to bring a
new cleaner one.

Next commit introduces new generic implementation of
fastboot_set_reboot_flag.

Signed-off-by: Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
---
 drivers/fastboot/Kconfig       | 12 ----------
 drivers/fastboot/Makefile      |  1 -
 drivers/fastboot/fb_bcb_impl.c | 43 ----------------------------------
 include/fastboot.h             |  9 -------
 4 files changed, 65 deletions(-)
 delete mode 100644 drivers/fastboot/fb_bcb_impl.c

diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
index 4352ba67a713..d4436dfc9173 100644
--- a/drivers/fastboot/Kconfig
+++ b/drivers/fastboot/Kconfig
@@ -165,18 +165,6 @@ config FASTBOOT_CMD_OEM_FORMAT
 	  relies on the env variable partitions to contain the list of
 	  partitions as required by the gpt command.
 
-config FASTBOOT_USE_BCB_SET_REBOOT_FLAG
-	bool "Use BCB by fastboot to set boot reason"
-	depends on CMD_BCB && !ARCH_MESON && !ARCH_ROCKCHIP && !TARGET_KC1 && \
-	  !TARGET_SNIPER && !TARGET_AM57XX_EVM && !TARGET_DRA7XX_EVM
-	default y
-	help
-	  Fastboot could implement setting of reboot reason in a generic fashion
-	  via BCB commands. BCB commands are able to write reboot reason into
-	  command field of boot control block. In general case it is sufficient
-	  implementation if your platform supports BCB commands and doesn't
-	  require any specific reboot reason handling.
-
 endif # FASTBOOT
 
 endmenu
diff --git a/drivers/fastboot/Makefile b/drivers/fastboot/Makefile
index 2b2c390fe4de..048af5aa8234 100644
--- a/drivers/fastboot/Makefile
+++ b/drivers/fastboot/Makefile
@@ -5,4 +5,3 @@ obj-y += fb_getvar.o
 obj-y += fb_command.o
 obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fb_mmc.o
 obj-$(CONFIG_FASTBOOT_FLASH_NAND) += fb_nand.o
-obj-$(CONFIG_FASTBOOT_USE_BCB_SET_REBOOT_FLAG) += fb_bcb_impl.o
diff --git a/drivers/fastboot/fb_bcb_impl.c b/drivers/fastboot/fb_bcb_impl.c
deleted file mode 100644
index 89ec3601b6f6..000000000000
--- a/drivers/fastboot/fb_bcb_impl.c
+++ /dev/null
@@ -1,43 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Copyright 2020 GlobalLogic.
- * Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
- */
-
-#include <common.h>
-#include <fastboot.h>
-
-/**
- * fastboot_set_reboot_flag() - Set flag to indicate reboot-bootloader
- *
- * Set flag which indicates that we should reboot into the bootloader
- * following the reboot that fastboot executes after this function.
- *
- * This function should be overridden in your board file with one
- * which sets whatever flag your board specific Android bootloader flow
- * requires in order to re-enter the bootloader.
- */
-int fastboot_set_reboot_flag(enum fastboot_reboot_reason reason)
-{
-	char cmd[64];
-
-	if (reason >= FASTBOOT_REBOOT_REASONS_COUNT)
-		return -EINVAL;
-
-	snprintf(cmd, sizeof(cmd), "bcb load %d misc",
-		 CONFIG_FASTBOOT_FLASH_MMC_DEV);
-
-	if (run_command(cmd, 0))
-		return -ENODEV;
-
-	snprintf(cmd, sizeof(cmd), "bcb set command %s",
-		 fastboot_boot_cmds[reason]);
-
-	if (run_command(cmd, 0))
-		return -ENOEXEC;
-
-	if (run_command("bcb store", 0))
-		return -EIO;
-
-	return 0;
-}
diff --git a/include/fastboot.h b/include/fastboot.h
index 8e9ee80907df..b86b508e69fd 100644
--- a/include/fastboot.h
+++ b/include/fastboot.h
@@ -52,15 +52,6 @@ enum fastboot_reboot_reason {
 	FASTBOOT_REBOOT_REASONS_COUNT
 };
 
-/**
- * BCB boot commands
- */
-static const char * const fastboot_boot_cmds[] = {
-	[FASTBOOT_REBOOT_REASON_BOOTLOADER] = "bootonce-bootloader",
-	[FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot",
-	[FASTBOOT_REBOOT_REASON_RECOVERY] = "boot-recovery"
-};
-
 /**
  * fastboot_response() - Writes a response of the form "$tag$reason".
  *
-- 
2.17.1

[RESEND 7/7] fastboot: Implement generic fastboot_set_reboot_flag

[Roman Kovalivskyi]

It is possible to implement fastboot_set_reboot_flag in a generic way
if BCB commands are turned on for a target. Using
bcb_set_reboot_reason allows to do this by simply passing string with
correct reboot reason that should be handled during next boot process.

If BCB are turned off, then bcb_set_reboot_reason would simply return
error, so it won't introduce any new behaviour for such targets.

Signed-off-by: Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
---
 drivers/fastboot/fb_common.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c
index 736ce1cd024f..005dccf3c967 100644
--- a/drivers/fastboot/fb_common.c
+++ b/drivers/fastboot/fb_common.c
@@ -10,6 +10,7 @@
  * Rob Herring <robh@kernel.org>
  */
 
+#include <bcb.h>
 #include <common.h>
 #include <command.h>
 #include <env.h>
@@ -90,7 +91,16 @@ void fastboot_okay(const char *reason, char *response)
  */
 int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason)
 {
-	return -ENOSYS;
+	static const char * const boot_cmds[] = {
+		[FASTBOOT_REBOOT_REASON_BOOTLOADER] = "bootonce-bootloader",
+		[FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot",
+		[FASTBOOT_REBOOT_REASON_RECOVERY] = "boot-recovery"
+	};
+
+	if (reason >= FASTBOOT_REBOOT_REASONS_COUNT)
+		return -EINVAL;
+
+	return bcb_write_reboot_reason(CONFIG_FASTBOOT_FLASH_MMC_DEV, "misc", boot_cmds[reason]);
 }
 
 /**
-- 
2.17.1

[RESEND 0/7] Refactor generic fastboot_set_reboot_flag implementation

[Eugeniu Rosca]

Dear U-Boot maintainers,

On Fri, Oct 23, 2020 at 11:52:18AM +0300, Roman Kovalivskyi wrote:
> Current generic implementation of fastboot_set_reboot_flag is somewhat
> messy and requires some additional configuration option to be enabled
> besides CMD_BCB, so it reverts that implementtion in order to bring a
> new cleaner one.
> 
> New function called bcb_set_reboot_reason should be exposed by BCB
> commands, so that all of the details could be put there instead of
> dealing with it in fastboot implementation directly. After that,
> fastboot_set_reboot_flag could simply pass correct reboot reason
> string to the BCB implementation. If CMD_BCB is disabled then the
> whole operation would return error code, which is no different
> behaviour than the current implementation.
> 
> Eugeniu Rosca (5):
>   cmd: bcb: Extract '__bcb_load' from 'do_bcb_load' for internal needs
>   cmd: bcb: Extract '__bcb_set' from 'do_bcb_set' for internal needs
>   cmd: bcb: Extract '__bcb_store' from 'do_bcb_store' for internal needs
>   cmd: bcb: Expose 'bcb_write_reboot_reason' to external callers
>   cmd: bcb: Add support for processing const string literals in
>     bcb_set()
> 
> Roman Kovalivskyi (2):
>   Revert "fastboot: Add default fastboot_set_reboot_flag implementation"
>   fastboot: Implement generic fastboot_set_reboot_flag
> 
>  cmd/bcb.c                      | 88 +++++++++++++++++++++++++++-------
>  drivers/fastboot/Kconfig       | 12 -----
>  drivers/fastboot/Makefile      |  1 -
>  drivers/fastboot/fb_bcb_impl.c | 43 -----------------
>  drivers/fastboot/fb_common.c   | 12 ++++-
>  include/bcb.h                  | 20 ++++++++
>  include/fastboot.h             |  9 ----
>  7 files changed, 101 insertions(+), 84 deletions(-)
>  delete mode 100644 drivers/fastboot/fb_bcb_impl.c
>  create mode 100644 include/bcb.h

Any chance the patches will be applied?

-- 
Best regards,
Eugeniu Rosca

[RESEND 0/7] Refactor generic fastboot_set_reboot_flag implementation

[Tom Rini]

On Thu, Dec 10, 2020 at 08:30:36AM +0100, Eugeniu Rosca wrote:
> Dear U-Boot maintainers,
> 
> On Fri, Oct 23, 2020 at 11:52:18AM +0300, Roman Kovalivskyi wrote:
> > Current generic implementation of fastboot_set_reboot_flag is somewhat
> > messy and requires some additional configuration option to be enabled
> > besides CMD_BCB, so it reverts that implementtion in order to bring a
> > new cleaner one.
> > 
> > New function called bcb_set_reboot_reason should be exposed by BCB
> > commands, so that all of the details could be put there instead of
> > dealing with it in fastboot implementation directly. After that,
> > fastboot_set_reboot_flag could simply pass correct reboot reason
> > string to the BCB implementation. If CMD_BCB is disabled then the
> > whole operation would return error code, which is no different
> > behaviour than the current implementation.
> > 
> > Eugeniu Rosca (5):
> >   cmd: bcb: Extract '__bcb_load' from 'do_bcb_load' for internal needs
> >   cmd: bcb: Extract '__bcb_set' from 'do_bcb_set' for internal needs
> >   cmd: bcb: Extract '__bcb_store' from 'do_bcb_store' for internal needs
> >   cmd: bcb: Expose 'bcb_write_reboot_reason' to external callers
> >   cmd: bcb: Add support for processing const string literals in
> >     bcb_set()
> > 
> > Roman Kovalivskyi (2):
> >   Revert "fastboot: Add default fastboot_set_reboot_flag implementation"
> >   fastboot: Implement generic fastboot_set_reboot_flag
> > 
> >  cmd/bcb.c                      | 88 +++++++++++++++++++++++++++-------
> >  drivers/fastboot/Kconfig       | 12 -----
> >  drivers/fastboot/Makefile      |  1 -
> >  drivers/fastboot/fb_bcb_impl.c | 43 -----------------
> >  drivers/fastboot/fb_common.c   | 12 ++++-
> >  include/bcb.h                  | 20 ++++++++
> >  include/fastboot.h             |  9 ----
> >  7 files changed, 101 insertions(+), 84 deletions(-)
> >  delete mode 100644 drivers/fastboot/fb_bcb_impl.c
> >  create mode 100644 include/bcb.h
> 
> Any chance the patches will be applied?

I'll try and review at least the generic fastboot changes soon and if
there's no problems, move this in to -next.  Thanks.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201210/2e30749f/attachment.sig>

[RESEND 0/7] Refactor generic fastboot_set_reboot_flag implementation

[Eugeniu Rosca]

Dear Tom,

On Thu, Dec 10, 2020 at 11:54:44AM -0500, Tom Rini wrote:
> On Thu, Dec 10, 2020 at 08:30:36AM +0100, Eugeniu Rosca wrote:
> > Dear U-Boot maintainers,
> > 
> > On Fri, Oct 23, 2020 at 11:52:18AM +0300, Roman Kovalivskyi wrote:
> > > Current generic implementation of fastboot_set_reboot_flag is somewhat
> > > messy and requires some additional configuration option to be enabled
> > > besides CMD_BCB, so it reverts that implementtion in order to bring a
> > > new cleaner one.
> > > 
> > > New function called bcb_set_reboot_reason should be exposed by BCB
> > > commands, so that all of the details could be put there instead of
> > > dealing with it in fastboot implementation directly. After that,
> > > fastboot_set_reboot_flag could simply pass correct reboot reason
> > > string to the BCB implementation. If CMD_BCB is disabled then the
> > > whole operation would return error code, which is no different
> > > behaviour than the current implementation.
> > > 
> > > Eugeniu Rosca (5):
> > >   cmd: bcb: Extract '__bcb_load' from 'do_bcb_load' for internal needs
> > >   cmd: bcb: Extract '__bcb_set' from 'do_bcb_set' for internal needs
> > >   cmd: bcb: Extract '__bcb_store' from 'do_bcb_store' for internal needs
> > >   cmd: bcb: Expose 'bcb_write_reboot_reason' to external callers
> > >   cmd: bcb: Add support for processing const string literals in
> > >     bcb_set()
> > > 
> > > Roman Kovalivskyi (2):
> > >   Revert "fastboot: Add default fastboot_set_reboot_flag implementation"
> > >   fastboot: Implement generic fastboot_set_reboot_flag
> > > 
> > >  cmd/bcb.c                      | 88 +++++++++++++++++++++++++++-------
> > >  drivers/fastboot/Kconfig       | 12 -----
> > >  drivers/fastboot/Makefile      |  1 -
> > >  drivers/fastboot/fb_bcb_impl.c | 43 -----------------
> > >  drivers/fastboot/fb_common.c   | 12 ++++-
> > >  include/bcb.h                  | 20 ++++++++
> > >  include/fastboot.h             |  9 ----
> > >  7 files changed, 101 insertions(+), 84 deletions(-)
> > >  delete mode 100644 drivers/fastboot/fb_bcb_impl.c
> > >  create mode 100644 include/bcb.h
> > 
> > Any chance the patches will be applied?
> 
> I'll try and review at least the generic fastboot changes soon and if
> there's no problems, move this in to -next.  Thanks.

Any suggestions how to bring this series back on track?
Thanks in advance.

-- 
Best regards,
Eugeniu Rosca

[RESEND 6/7] Revert "fastboot: Add default fastboot_set_reboot_flag implementation"

[Lukasz Majewski]

Hi Roman,

> This reverts commit 0ebf9842e56c5b8cb7cb1f990bb452cc14af6225.
> 
> Current generic implementation of fastboot_set_reboot_flag is somewhat
> messy and requires some additional configuration option to be enabled
> besides CMD_BCB, so it reverts that implementtion in order to bring a
> new cleaner one.
> 
> Next commit introduces new generic implementation of
> fastboot_set_reboot_flag.
> 
> Signed-off-by: Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
> ---
>  drivers/fastboot/Kconfig       | 12 ----------
>  drivers/fastboot/Makefile      |  1 -
>  drivers/fastboot/fb_bcb_impl.c | 43
> ---------------------------------- include/fastboot.h             |
> 9 ------- 4 files changed, 65 deletions(-)
>  delete mode 100644 drivers/fastboot/fb_bcb_impl.c
> 
> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> index 4352ba67a713..d4436dfc9173 100644
> --- a/drivers/fastboot/Kconfig
> +++ b/drivers/fastboot/Kconfig
> @@ -165,18 +165,6 @@ config FASTBOOT_CMD_OEM_FORMAT
>  	  relies on the env variable partitions to contain the list
> of partitions as required by the gpt command.
>  
> -config FASTBOOT_USE_BCB_SET_REBOOT_FLAG
> -	bool "Use BCB by fastboot to set boot reason"
> -	depends on CMD_BCB && !ARCH_MESON && !ARCH_ROCKCHIP &&
> !TARGET_KC1 && \
> -	  !TARGET_SNIPER && !TARGET_AM57XX_EVM && !TARGET_DRA7XX_EVM
> -	default y
> -	help
> -	  Fastboot could implement setting of reboot reason in a
> generic fashion
> -	  via BCB commands. BCB commands are able to write reboot
> reason into
> -	  command field of boot control block. In general case it is
> sufficient
> -	  implementation if your platform supports BCB commands and
> doesn't
> -	  require any specific reboot reason handling.
> -
>  endif # FASTBOOT
>  
>  endmenu
> diff --git a/drivers/fastboot/Makefile b/drivers/fastboot/Makefile
> index 2b2c390fe4de..048af5aa8234 100644
> --- a/drivers/fastboot/Makefile
> +++ b/drivers/fastboot/Makefile
> @@ -5,4 +5,3 @@ obj-y += fb_getvar.o
>  obj-y += fb_command.o
>  obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fb_mmc.o
>  obj-$(CONFIG_FASTBOOT_FLASH_NAND) += fb_nand.o
> -obj-$(CONFIG_FASTBOOT_USE_BCB_SET_REBOOT_FLAG) += fb_bcb_impl.o
> diff --git a/drivers/fastboot/fb_bcb_impl.c
> b/drivers/fastboot/fb_bcb_impl.c deleted file mode 100644
> index 89ec3601b6f6..000000000000
> --- a/drivers/fastboot/fb_bcb_impl.c
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0+
> -/*
> - * Copyright 2020 GlobalLogic.
> - * Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
> - */
> -
> -#include <common.h>
> -#include <fastboot.h>
> -
> -/**
> - * fastboot_set_reboot_flag() - Set flag to indicate
> reboot-bootloader
> - *
> - * Set flag which indicates that we should reboot into the bootloader
> - * following the reboot that fastboot executes after this function.
> - *
> - * This function should be overridden in your board file with one
> - * which sets whatever flag your board specific Android bootloader
> flow
> - * requires in order to re-enter the bootloader.
> - */
> -int fastboot_set_reboot_flag(enum fastboot_reboot_reason reason)
> -{
> -	char cmd[64];
> -
> -	if (reason >= FASTBOOT_REBOOT_REASONS_COUNT)
> -		return -EINVAL;
> -
> -	snprintf(cmd, sizeof(cmd), "bcb load %d misc",
> -		 CONFIG_FASTBOOT_FLASH_MMC_DEV);
> -
> -	if (run_command(cmd, 0))
> -		return -ENODEV;
> -
> -	snprintf(cmd, sizeof(cmd), "bcb set command %s",
> -		 fastboot_boot_cmds[reason]);
> -
> -	if (run_command(cmd, 0))
> -		return -ENOEXEC;
> -
> -	if (run_command("bcb store", 0))
> -		return -EIO;
> -
> -	return 0;
> -}
> diff --git a/include/fastboot.h b/include/fastboot.h
> index 8e9ee80907df..b86b508e69fd 100644
> --- a/include/fastboot.h
> +++ b/include/fastboot.h
> @@ -52,15 +52,6 @@ enum fastboot_reboot_reason {
>  	FASTBOOT_REBOOT_REASONS_COUNT
>  };
>  
> -/**
> - * BCB boot commands
> - */
> -static const char * const fastboot_boot_cmds[] = {
> -	[FASTBOOT_REBOOT_REASON_BOOTLOADER] = "bootonce-bootloader",
> -	[FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot",
> -	[FASTBOOT_REBOOT_REASON_RECOVERY] = "boot-recovery"
> -};
> -
>  /**
>   * fastboot_response() - Writes a response of the form "$tag$reason".
>   *

If this patch is still needed - please rebase it on newest master
(after the incoming PR) as it causes build breaks.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210123/7db9611c/attachment.sig>

[RESEND 7/7] fastboot: Implement generic fastboot_set_reboot_flag

[Lukasz Majewski]

Hi Roman,

> It is possible to implement fastboot_set_reboot_flag in a generic way
> if BCB commands are turned on for a target. Using
> bcb_set_reboot_reason allows to do this by simply passing string with
> correct reboot reason that should be handled during next boot process.
> 
> If BCB are turned off, then bcb_set_reboot_reason would simply return
> error, so it won't introduce any new behaviour for such targets.
> 
> Signed-off-by: Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
> ---
>  drivers/fastboot/fb_common.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/fastboot/fb_common.c
> b/drivers/fastboot/fb_common.c index 736ce1cd024f..005dccf3c967 100644
> --- a/drivers/fastboot/fb_common.c
> +++ b/drivers/fastboot/fb_common.c
> @@ -10,6 +10,7 @@
>   * Rob Herring <robh@kernel.org>
>   */
>  
> +#include <bcb.h>
>  #include <common.h>
>  #include <command.h>
>  #include <env.h>
> @@ -90,7 +91,16 @@ void fastboot_okay(const char *reason, char
> *response) */
>  int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason
> reason) {
> -	return -ENOSYS;
> +	static const char * const boot_cmds[] = {
> +		[FASTBOOT_REBOOT_REASON_BOOTLOADER] =
> "bootonce-bootloader",
> +		[FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot",
> +		[FASTBOOT_REBOOT_REASON_RECOVERY] = "boot-recovery"
> +	};
> +
> +	if (reason >= FASTBOOT_REBOOT_REASONS_COUNT)
> +		return -EINVAL;
> +
> +	return
> bcb_write_reboot_reason(CONFIG_FASTBOOT_FLASH_MMC_DEV, "misc",
> boot_cmds[reason]); } 
>  /**

This patch causes build breaks when I run the CI on azzure. If it is
still needed, please rebase it and resend.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210123/1d033591/attachment.sig>

[RESEND 6/7] Revert "fastboot: Add default fastboot_set_reboot_flag implementation"

[Roman Kovalivskyi]

Hi Lukasz,

On 1/23/21 5:08 PM, Lukasz Majewski wrote:
> Hi Roman,
> 
>> This reverts commit 0ebf9842e56c5b8cb7cb1f990bb452cc14af6225.
>>
>> Current generic implementation of fastboot_set_reboot_flag is somewhat
>> messy and requires some additional configuration option to be enabled
>> besides CMD_BCB, so it reverts that implementtion in order to bring a
>> new cleaner one.
>>
>> Next commit introduces new generic implementation of
>> fastboot_set_reboot_flag.
>>
>> Signed-off-by: Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
>> ---
>>   drivers/fastboot/Kconfig       | 12 ----------
>>   drivers/fastboot/Makefile      |  1 -
>>   drivers/fastboot/fb_bcb_impl.c | 43
>> ---------------------------------- include/fastboot.h             |
>> 9 ------- 4 files changed, 65 deletions(-)
>>   delete mode 100644 drivers/fastboot/fb_bcb_impl.c
>>
>> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
>> index 4352ba67a713..d4436dfc9173 100644
>> --- a/drivers/fastboot/Kconfig
>> +++ b/drivers/fastboot/Kconfig
>> @@ -165,18 +165,6 @@ config FASTBOOT_CMD_OEM_FORMAT
>>   	  relies on the env variable partitions to contain the list
>> of partitions as required by the gpt command.
>>   
>> -config FASTBOOT_USE_BCB_SET_REBOOT_FLAG
>> -	bool "Use BCB by fastboot to set boot reason"
>> -	depends on CMD_BCB && !ARCH_MESON && !ARCH_ROCKCHIP &&
>> !TARGET_KC1 && \
>> -	  !TARGET_SNIPER && !TARGET_AM57XX_EVM && !TARGET_DRA7XX_EVM
>> -	default y
>> -	help
>> -	  Fastboot could implement setting of reboot reason in a
>> generic fashion
>> -	  via BCB commands. BCB commands are able to write reboot
>> reason into
>> -	  command field of boot control block. In general case it is
>> sufficient
>> -	  implementation if your platform supports BCB commands and
>> doesn't
>> -	  require any specific reboot reason handling.
>> -
>>   endif # FASTBOOT
>>   
>>   endmenu
>> diff --git a/drivers/fastboot/Makefile b/drivers/fastboot/Makefile
>> index 2b2c390fe4de..048af5aa8234 100644
>> --- a/drivers/fastboot/Makefile
>> +++ b/drivers/fastboot/Makefile
>> @@ -5,4 +5,3 @@ obj-y += fb_getvar.o
>>   obj-y += fb_command.o
>>   obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fb_mmc.o
>>   obj-$(CONFIG_FASTBOOT_FLASH_NAND) += fb_nand.o
>> -obj-$(CONFIG_FASTBOOT_USE_BCB_SET_REBOOT_FLAG) += fb_bcb_impl.o
>> diff --git a/drivers/fastboot/fb_bcb_impl.c
>> b/drivers/fastboot/fb_bcb_impl.c deleted file mode 100644
>> index 89ec3601b6f6..000000000000
>> --- a/drivers/fastboot/fb_bcb_impl.c
>> +++ /dev/null
>> @@ -1,43 +0,0 @@
>> -// SPDX-License-Identifier: GPL-2.0+
>> -/*
>> - * Copyright 2020 GlobalLogic.
>> - * Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
>> - */
>> -
>> -#include <common.h>
>> -#include <fastboot.h>
>> -
>> -/**
>> - * fastboot_set_reboot_flag() - Set flag to indicate
>> reboot-bootloader
>> - *
>> - * Set flag which indicates that we should reboot into the bootloader
>> - * following the reboot that fastboot executes after this function.
>> - *
>> - * This function should be overridden in your board file with one
>> - * which sets whatever flag your board specific Android bootloader
>> flow
>> - * requires in order to re-enter the bootloader.
>> - */
>> -int fastboot_set_reboot_flag(enum fastboot_reboot_reason reason)
>> -{
>> -	char cmd[64];
>> -
>> -	if (reason >= FASTBOOT_REBOOT_REASONS_COUNT)
>> -		return -EINVAL;
>> -
>> -	snprintf(cmd, sizeof(cmd), "bcb load %d misc",
>> -		 CONFIG_FASTBOOT_FLASH_MMC_DEV);
>> -
>> -	if (run_command(cmd, 0))
>> -		return -ENODEV;
>> -
>> -	snprintf(cmd, sizeof(cmd), "bcb set command %s",
>> -		 fastboot_boot_cmds[reason]);
>> -
>> -	if (run_command(cmd, 0))
>> -		return -ENOEXEC;
>> -
>> -	if (run_command("bcb store", 0))
>> -		return -EIO;
>> -
>> -	return 0;
>> -}
>> diff --git a/include/fastboot.h b/include/fastboot.h
>> index 8e9ee80907df..b86b508e69fd 100644
>> --- a/include/fastboot.h
>> +++ b/include/fastboot.h
>> @@ -52,15 +52,6 @@ enum fastboot_reboot_reason {
>>   	FASTBOOT_REBOOT_REASONS_COUNT
>>   };
>>   
>> -/**
>> - * BCB boot commands
>> - */
>> -static const char * const fastboot_boot_cmds[] = {
>> -	[FASTBOOT_REBOOT_REASON_BOOTLOADER] = "bootonce-bootloader",
>> -	[FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot",
>> -	[FASTBOOT_REBOOT_REASON_RECOVERY] = "boot-recovery"
>> -};
>> -
>>   /**
>>    * fastboot_response() - Writes a response of the form "$tag$reason".
>>    *
> 
> If this patch is still needed - please rebase it on newest master
> (after the incoming PR) as it causes build breaks.
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
> 

Could you please share some info on how do you build it (used defconfig, 
error messages, used compiler, etc.)? I cannot reproduce those issues on 
my side.

I've tried to rebase my patches onto the current master and rebase was 
done cleanly, there are no conflicts. I've tried building it with 
rcar3_ulcb_defconfig and build was successful.

Used defconfig: rcar3_ulcb_defconfig
Used arch: ARM
Used compiler: gcc 7.1 for aarch64-linux-gnu
Used CFLAGS: -fgnu89-inline

Best regards,
Roman Kovalivskyi

[RESEND 6/7] Revert "fastboot: Add default fastboot_set_reboot_flag implementation"

[Tom Rini]

On Mon, Jan 25, 2021 at 07:16:37PM +0200, Roman Kovalivskyi wrote:
> Hi Lukasz,
> 
> On 1/23/21 5:08 PM, Lukasz Majewski wrote:
> > Hi Roman,
> > 
> > > This reverts commit 0ebf9842e56c5b8cb7cb1f990bb452cc14af6225.
> > > 
> > > Current generic implementation of fastboot_set_reboot_flag is somewhat
> > > messy and requires some additional configuration option to be enabled
> > > besides CMD_BCB, so it reverts that implementtion in order to bring a
> > > new cleaner one.
> > > 
> > > Next commit introduces new generic implementation of
> > > fastboot_set_reboot_flag.
> > > 
> > > Signed-off-by: Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
> > > ---
> > >   drivers/fastboot/Kconfig       | 12 ----------
> > >   drivers/fastboot/Makefile      |  1 -
> > >   drivers/fastboot/fb_bcb_impl.c | 43
> > > ---------------------------------- include/fastboot.h             |
> > > 9 ------- 4 files changed, 65 deletions(-)
> > >   delete mode 100644 drivers/fastboot/fb_bcb_impl.c
> > > 
> > > diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> > > index 4352ba67a713..d4436dfc9173 100644
> > > --- a/drivers/fastboot/Kconfig
> > > +++ b/drivers/fastboot/Kconfig
> > > @@ -165,18 +165,6 @@ config FASTBOOT_CMD_OEM_FORMAT
> > >   	  relies on the env variable partitions to contain the list
> > > of partitions as required by the gpt command.
> > > -config FASTBOOT_USE_BCB_SET_REBOOT_FLAG
> > > -	bool "Use BCB by fastboot to set boot reason"
> > > -	depends on CMD_BCB && !ARCH_MESON && !ARCH_ROCKCHIP &&
> > > !TARGET_KC1 && \
> > > -	  !TARGET_SNIPER && !TARGET_AM57XX_EVM && !TARGET_DRA7XX_EVM
> > > -	default y
> > > -	help
> > > -	  Fastboot could implement setting of reboot reason in a
> > > generic fashion
> > > -	  via BCB commands. BCB commands are able to write reboot
> > > reason into
> > > -	  command field of boot control block. In general case it is
> > > sufficient
> > > -	  implementation if your platform supports BCB commands and
> > > doesn't
> > > -	  require any specific reboot reason handling.
> > > -
> > >   endif # FASTBOOT
> > >   endmenu
> > > diff --git a/drivers/fastboot/Makefile b/drivers/fastboot/Makefile
> > > index 2b2c390fe4de..048af5aa8234 100644
> > > --- a/drivers/fastboot/Makefile
> > > +++ b/drivers/fastboot/Makefile
> > > @@ -5,4 +5,3 @@ obj-y += fb_getvar.o
> > >   obj-y += fb_command.o
> > >   obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fb_mmc.o
> > >   obj-$(CONFIG_FASTBOOT_FLASH_NAND) += fb_nand.o
> > > -obj-$(CONFIG_FASTBOOT_USE_BCB_SET_REBOOT_FLAG) += fb_bcb_impl.o
> > > diff --git a/drivers/fastboot/fb_bcb_impl.c
> > > b/drivers/fastboot/fb_bcb_impl.c deleted file mode 100644
> > > index 89ec3601b6f6..000000000000
> > > --- a/drivers/fastboot/fb_bcb_impl.c
> > > +++ /dev/null
> > > @@ -1,43 +0,0 @@
> > > -// SPDX-License-Identifier: GPL-2.0+
> > > -/*
> > > - * Copyright 2020 GlobalLogic.
> > > - * Roman Kovalivskyi <roman.kovalivskyi@globallogic.com>
> > > - */
> > > -
> > > -#include <common.h>
> > > -#include <fastboot.h>
> > > -
> > > -/**
> > > - * fastboot_set_reboot_flag() - Set flag to indicate
> > > reboot-bootloader
> > > - *
> > > - * Set flag which indicates that we should reboot into the bootloader
> > > - * following the reboot that fastboot executes after this function.
> > > - *
> > > - * This function should be overridden in your board file with one
> > > - * which sets whatever flag your board specific Android bootloader
> > > flow
> > > - * requires in order to re-enter the bootloader.
> > > - */
> > > -int fastboot_set_reboot_flag(enum fastboot_reboot_reason reason)
> > > -{
> > > -	char cmd[64];
> > > -
> > > -	if (reason >= FASTBOOT_REBOOT_REASONS_COUNT)
> > > -		return -EINVAL;
> > > -
> > > -	snprintf(cmd, sizeof(cmd), "bcb load %d misc",
> > > -		 CONFIG_FASTBOOT_FLASH_MMC_DEV);
> > > -
> > > -	if (run_command(cmd, 0))
> > > -		return -ENODEV;
> > > -
> > > -	snprintf(cmd, sizeof(cmd), "bcb set command %s",
> > > -		 fastboot_boot_cmds[reason]);
> > > -
> > > -	if (run_command(cmd, 0))
> > > -		return -ENOEXEC;
> > > -
> > > -	if (run_command("bcb store", 0))
> > > -		return -EIO;
> > > -
> > > -	return 0;
> > > -}
> > > diff --git a/include/fastboot.h b/include/fastboot.h
> > > index 8e9ee80907df..b86b508e69fd 100644
> > > --- a/include/fastboot.h
> > > +++ b/include/fastboot.h
> > > @@ -52,15 +52,6 @@ enum fastboot_reboot_reason {
> > >   	FASTBOOT_REBOOT_REASONS_COUNT
> > >   };
> > > -/**
> > > - * BCB boot commands
> > > - */
> > > -static const char * const fastboot_boot_cmds[] = {
> > > -	[FASTBOOT_REBOOT_REASON_BOOTLOADER] = "bootonce-bootloader",
> > > -	[FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot",
> > > -	[FASTBOOT_REBOOT_REASON_RECOVERY] = "boot-recovery"
> > > -};
> > > -
> > >   /**
> > >    * fastboot_response() - Writes a response of the form "$tag$reason".
> > >    *
> > 
> > If this patch is still needed - please rebase it on newest master
> > (after the incoming PR) as it causes build breaks.
> > 
> > 
> > Best regards,
> > 
> > Lukasz Majewski
> > 
> > --
> > 
> > DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> > Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
> > 
> 
> Could you please share some info on how do you build it (used defconfig,
> error messages, used compiler, etc.)? I cannot reproduce those issues on my
> side.
> 
> I've tried to rebase my patches onto the current master and rebase was done
> cleanly, there are no conflicts. I've tried building it with
> rcar3_ulcb_defconfig and build was successful.
> 
> Used defconfig: rcar3_ulcb_defconfig
> Used arch: ARM
> Used compiler: gcc 7.1 for aarch64-linux-gnu
> Used CFLAGS: -fgnu89-inline

You likely want to submit a GitHub PR against
https://github.com/u-boot/u-boot so that you can trigger a world build
via Azure and see what fails to build from that.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210125/9321bb3d/attachment.sig>

[RESEND 6/7] Revert "fastboot: Add default fastboot_set_reboot_flag implementation"

[Marek Vasut]

On 1/25/21 6:16 PM, Roman Kovalivskyi wrote:

[...]

> Could you please share some info on how do you build it (used defconfig, 
> error messages, used compiler, etc.)? I cannot reproduce those issues on 
> my side.
> 
> I've tried to rebase my patches onto the current master and rebase was 
> done cleanly, there are no conflicts. I've tried building it with 
> rcar3_ulcb_defconfig and build was successful.
> 
> Used defconfig: rcar3_ulcb_defconfig
> Used arch: ARM
> Used compiler: gcc 7.1 for aarch64-linux-gnu
> Used CFLAGS: -fgnu89-inline

Mainline U-Boot for rcar3 has no USB gadget support though ?