Skip to content

Commit

Permalink
meta-lxatac-bsp: barebox: backport fastboot improvements from master
Browse files Browse the repository at this point in the history
This fixes linux-automation#129 (The TAC gets stuck in bootloader when connected via USB
to some PCs) by not stopping the boot process on fastboot getvar requests.

The issue was that fwupd automatically sends getvar requests to fastboot
devices, most likely to check for updates on modems.

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
  • Loading branch information
hnez committed Oct 1, 2024
1 parent 41a76e4 commit a45f20d
Show file tree
Hide file tree
Showing 5 changed files with 188 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
Date: Fri, 9 Aug 2024 16:19:57 +0200
Subject: [PATCH] fastboot: print all variables only on getvar:all and not its
prefixes

strcmp_l1 compares up to the length of the first arguments, i.e. it does
a prefix check. For this, the prefix, which is usually a string literal,
needs to be the first argument.

The check for getvar:all doesn't follow this with the result that all of

fastboot getvar 'all'
fastboot getvar 'al'
fastboot getvar 'a'
fastboot getvar ''

behave the same. This undocumented quirk is most likely unintended, so
let's replace this with an actual equality check.

Note that strcmp_l1 also does a NULL-ness check. This is safe to remove,
as explained in the follow-up commit.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Link: https://lore.barebox.org/20240809141959.313914-1-a.fatoum@pengutronix.de
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
common/fastboot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/fastboot.c b/common/fastboot.c
index 532286703089..e85cc6d8aaf8 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -312,7 +312,7 @@ static void cb_getvar(struct fastboot *fb, const char *cmd)

pr_debug("getvar: \"%s\"\n", cmd);

- if (!strcmp_l1(cmd, "all")) {
+ if (!strcmp(cmd, "all")) {
list_for_each_entry(var, &fb->variables, list)
fastboot_tx_print(fb, FASTBOOT_MSG_INFO, "%s: %s",
var->name, var->value);
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
Date: Fri, 9 Aug 2024 16:19:58 +0200
Subject: [PATCH] fastboot: retire strcmp_l1 in favor of str_has_prefix

strcmp_l1 is basically str_has_prefix with inverted arguments and a
NULL check.

We don't need the NULL check as cmdbuf and cmd->cmd should always be
non-NULL: The former is the address of an array populated by fastboot
network or USB code and the latter is a pointer to a string literal.

So let's codify the assumption that cmdbuf is not NULL into the
prototype and replace strcmp_l1 with str_has_prefix. This has the added
benefit that str_has_prefix returns the length of the prefix, which
saves us a repeated call to strlen(cmd->cmd);

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Link: https://lore.barebox.org/20240809141959.313914-2-a.fatoum@pengutronix.de
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
common/fastboot.c | 14 +++++---------
include/fastboot.h | 3 ++-
2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/common/fastboot.c b/common/fastboot.c
index e85cc6d8aaf8..dc66d7123b02 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -287,13 +287,6 @@ static void cb_reboot(struct fastboot *fb, const char *cmd)
restart_machine();
}

-static int strcmp_l1(const char *s1, const char *s2)
-{
- if (!s1 || !s2)
- return -1;
- return strncmp(s1, s2, strlen(s1));
-}
-
static void cb_getvar(struct fastboot *fb, const char *cmd)
{
struct fb_variable *var;
@@ -815,10 +808,13 @@ static void fb_run_command(struct fastboot *fb, const char *cmdbuf,
console_countdown_abort("fastboot");

for (i = 0; i < num_commands; i++) {
+ size_t cmdlen;
+
cmd = &cmds[i];

- if (!strcmp_l1(cmd->cmd, cmdbuf)) {
- cmd->cb(fb, cmdbuf + strlen(cmd->cmd));
+ cmdlen = str_has_prefix(cmdbuf, cmd->cmd);
+ if (cmdlen) {
+ cmd->cb(fb, cmdbuf + cmdlen);

return;
}
diff --git a/include/fastboot.h b/include/fastboot.h
index cd415847e348..4b2fdf3190b2 100644
--- a/include/fastboot.h
+++ b/include/fastboot.h
@@ -87,7 +87,8 @@ int fastboot_tx_print(struct fastboot *fb, enum fastboot_msg_type type,
const char *fmt, ...);
void fastboot_start_download_generic(struct fastboot *fb);
void fastboot_download_finished(struct fastboot *fb);
-void fastboot_exec_cmd(struct fastboot *fb, const char *cmdbuf);
+void fastboot_exec_cmd(struct fastboot *fb, const char *cmdbuf)
+ __attribute__((nonnull));
void fastboot_abort(struct fastboot *fb);

#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
Date: Fri, 9 Aug 2024 16:19:59 +0200
Subject: [PATCH] fastboot: avoid console_countdown_abort for getvar request

We currently abort boot countdown on any fastboot communication at all
as we have the expectation that this is what the user wants.

This doesn't hold true on systems with fwupd: The fastboot plugin probes
connected devices:

getvar:product
getvar:version
getvar:version-bootloader
getvar:serialno
getvar:secure

to determine whether an update is in-order. The first getvar will
automatically abort barebox boot up, which is likely not what the user
intended.

Therefore, let's abort console countdown only for non-getvar: requests.

Reported-by: Jonas Martin <j.martin@pengutronix.de>
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Link: https://lore.barebox.org/20240809141959.313914-3-a.fatoum@pengutronix.de
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
common/fastboot.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/common/fastboot.c b/common/fastboot.c
index dc66d7123b02..66b59ab9b0d7 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -794,6 +794,11 @@ static void cb_erase(struct fastboot *fb, const char *cmd)
fastboot_tx_print(fb, FASTBOOT_MSG_OKAY, "");
}

+static bool fastboot_cmd_should_abort(const char *cmdbuf)
+{
+ return !strstarts(cmdbuf, "getvar:");
+}
+
struct cmd_dispatch_info {
char *cmd;
void (*cb)(struct fastboot *fb, const char *opt);
@@ -805,7 +810,8 @@ static void fb_run_command(struct fastboot *fb, const char *cmdbuf,
const struct cmd_dispatch_info *cmd;
int i;

- console_countdown_abort("fastboot");
+ if (fastboot_cmd_should_abort(cmdbuf))
+ console_countdown_abort("fastboot");

for (i = 0; i < num_commands; i++) {
size_t cmdlen;
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
From: =?UTF-8?q?Leonard=20G=C3=B6hrs?= <l.goehrs@pengutronix.de>
Date: Wed, 18 Sep 2024 14:45:25 +0200
Subject: [PATCH] Release 2024.09.0/customers/lxa/tac/20240918-1
Date: Thu, 19 Sep 2024 14:21:41 +0200
Subject: [PATCH] Release 2024.09.0/customers/lxa/tac/20240919-1

---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 29d94792ffae..2b7e0604f838 100644
index 29d94792ffae..fb62e525f9c5 100644
--- a/Makefile
+++ b/Makefile
@@ -2,7 +2,7 @@
VERSION = 2024
PATCHLEVEL = 09
SUBLEVEL = 0
-EXTRAVERSION =
+EXTRAVERSION =-20240918-1
+EXTRAVERSION =-20240919-1
NAME = None

# *DOCUMENTATION*
18 changes: 13 additions & 5 deletions meta-lxatac-bsp/recipes-bsp/barebox/files/patches/series.inc
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# umpf-base: v2024.09.0
# umpf-name: 2024.09.0/customers/lxa/tac
# umpf-version: 2024.09.0/customers/lxa/tac/20240918-1
# umpf-version: 2024.09.0/customers/lxa/tac/20240919-1
# umpf-topic: v2024.09.0/topic/tlv
# umpf-hashinfo: 328855e48bdb9912ed99b13d817f79a90347d494
# umpf-topic-range: 7ae6bbdfa69855ee2f46fabda6622875f408fb1c..147c498e71cb5d60b4868c2f3eaf2d5e6ca12540
Expand All @@ -25,12 +25,20 @@ SRC_URI += "\
file://patches/0102-ARM-stm32mp-add-Linux-Automation-TAC-board.patch \
file://patches/0103-ARM-stm32mp-add-Linux-Automation-TAC-Generation-3.patch \
"
# umpf-release: 2024.09.0/customers/lxa/tac/20240918-1
# umpf-topic-range: 430d11d834bc265f559d962a2d22b274791d3b51..8b188e344eedfdf3f7fc7947f6a99c785ce69bf1
# umpf-topic: v2024.09.0/topic/fastboot
# umpf-hashinfo: 061a6052949159ef03e8d138707ec21d3fb342f8
# umpf-topic-range: 430d11d834bc265f559d962a2d22b274791d3b51..fced5cf3f44adffde53ee59c3685352576181d44
SRC_URI += "\
file://patches/0201-Release-2024.09.0-customers-lxa-tac-20240918-1.patch \
file://patches/0201-fastboot-print-all-variables-only-on-getvar-all-and-.patch \
file://patches/0202-fastboot-retire-strcmp_l1-in-favor-of-str_has_prefix.patch \
file://patches/0203-fastboot-avoid-console_countdown_abort-for-getvar-re.patch \
"
# umpf-release: 2024.09.0/customers/lxa/tac/20240919-1
# umpf-topic-range: fced5cf3f44adffde53ee59c3685352576181d44..fa8d7445ec861a2282579a096ea1d82c0672551d
SRC_URI += "\
file://patches/0301-Release-2024.09.0-customers-lxa-tac-20240919-1.patch \
"
UMPF_BASE = "2024.09.0"
UMPF_VERSION = "20240918-1"
UMPF_VERSION = "20240919-1"
UMPF_PV = "${UMPF_BASE}-${UMPF_VERSION}"
# umpf-end

0 comments on commit a45f20d

Please sign in to comment.