* [U-Boot] [PATCH v3 0/7] Provide SPL support for bootcount (in the case of using falcon boot mode)
@ 2018-04-29 13:36 Lukasz Majewski
2018-04-29 13:36 ` [U-Boot] [PATCH v3 1/7] bootcount: spl: Enable bootcount support in SPL Lukasz Majewski
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: Lukasz Majewski @ 2018-04-29 13:36 UTC (permalink / raw)
To: u-boot
This patch series provides support for controlling bootcount limits in SPL.
Moreover, the common code has been identified and reused in the
common/autoboot.c file.
It also enables this feature on display5 board to present usage patterns.
This patch has been applied on top of u-boot/master:
SHA1 : ec37f05ec0a999e0bd79f87354716df6f9bc074d
Test HW: Beagle Bone Black (am335x) , Display5 (imx6q)
Travis-Ci: https://travis-ci.org/lmajewski/u-boot-dfu/builds/372217887
Lukasz Majewski (7):
bootcount: spl: Enable bootcount support in SPL
bootcount: Add include guards into bootcount.h file
bootcount: Add function wrappers to handle bootcount increment and
error checking
bootcount: Rewrite autoboot to use wrapper functions from bootcount.h
bootcount: spl: Extend SPL to support bootcount incrementation
bootcount: display5: spl: Extend DISPLAY5 board SPL to support
bootcount checking
bootcount: display5: config: Enable boot count feature in the display5
board
board/liebherr/display5/spl.c | 3 ++-
common/autoboot.c | 23 ++++---------------
common/spl/Kconfig | 9 ++++++++
common/spl/spl.c | 3 +++
configs/display5_defconfig | 4 ++++
drivers/Makefile | 1 +
include/bootcount.h | 53 +++++++++++++++++++++++++++++++++++++++++++
7 files changed, 77 insertions(+), 19 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v3 1/7] bootcount: spl: Enable bootcount support in SPL
2018-04-29 13:36 [U-Boot] [PATCH v3 0/7] Provide SPL support for bootcount (in the case of using falcon boot mode) Lukasz Majewski
@ 2018-04-29 13:36 ` Lukasz Majewski
2018-04-30 14:17 ` Tom Rini
2018-04-29 13:36 ` [U-Boot] [PATCH v3 2/7] bootcount: Add include guards into bootcount.h file Lukasz Majewski
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Lukasz Majewski @ 2018-04-29 13:36 UTC (permalink / raw)
To: u-boot
New, SPL related config option - CONFIG_SPL_BOOTCOUNT_LIMIT has been
added to allow drivers/bootcount code re-usage in SPL.
This code is necessary to use and setup bootcount in SPL in the case of
falcon boot mode.
Signed-off-by: Lukasz Majewski <lukma@denx.de>
Reviewed-by: Stefan Roese <sr@denx.de>
---
Changes in v3:
- None
Changes in v2:
- None
common/spl/Kconfig | 9 +++++++++
drivers/Makefile | 1 +
2 files changed, 10 insertions(+)
diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 4d27565566..2a61d2364b 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -54,6 +54,15 @@ config SPL_BOOTROM_SUPPORT
BOOT_DEVICE_BOOTROM (or fall-through to the next boot device in the
boot device list, if not implemented for a given board)
+config SPL_BOOTCOUNT_LIMIT
+ bool "Support bootcount in SPL"
+ depends on SPL_ENV_SUPPORT
+ help
+ On some boards, which use 'falcon' mode, it is necessary to check
+ and increment the number of boot attempts. Such boards do not
+ use proper U-Boot for normal boot flow and hence needs those
+ adjustments to be done in the SPL.
+
config SPL_RAW_IMAGE_SUPPORT
bool "Support SPL loading and booting of RAW images"
default n if (ARCH_MX6 && (SPL_MMC_SUPPORT || SPL_SATA_SUPPORT))
diff --git a/drivers/Makefile b/drivers/Makefile
index 6846d181aa..061331eadd 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_$(SPL_TPL_)TIMER) += timer/
ifndef CONFIG_TPL_BUILD
ifdef CONFIG_SPL_BUILD
+obj-$(CONFIG_SPL_BOOTCOUNT_LIMIT) += bootcount/
obj-$(CONFIG_SPL_CPU_SUPPORT) += cpu/
obj-$(CONFIG_SPL_CRYPTO_SUPPORT) += crypto/
obj-$(CONFIG_SPL_GPIO_SUPPORT) += gpio/
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v3 2/7] bootcount: Add include guards into bootcount.h file
2018-04-29 13:36 [U-Boot] [PATCH v3 0/7] Provide SPL support for bootcount (in the case of using falcon boot mode) Lukasz Majewski
2018-04-29 13:36 ` [U-Boot] [PATCH v3 1/7] bootcount: spl: Enable bootcount support in SPL Lukasz Majewski
@ 2018-04-29 13:36 ` Lukasz Majewski
2018-04-30 14:17 ` Tom Rini
2018-04-29 13:36 ` [U-Boot] [PATCH v3 3/7] bootcount: Add function wrappers to handle bootcount increment and error checking Lukasz Majewski
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Lukasz Majewski @ 2018-04-29 13:36 UTC (permalink / raw)
To: u-boot
This patch adds missing include guards for bootcount.h file.
Signed-off-by: Lukasz Majewski <lukma@denx.de>
Reviewed-by: Stefan Roese <sr@denx.de>
---
Changes in v3:
- None
Changes in v2:
- New patch
include/bootcount.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/bootcount.h b/include/bootcount.h
index 06fb4d3578..e3b3f7028e 100644
--- a/include/bootcount.h
+++ b/include/bootcount.h
@@ -4,6 +4,8 @@
*
* SPDX-License-Identifier: GPL-2.0+
*/
+#ifndef _BOOTCOUNT_H__
+#define _BOOTCOUNT_H__
#include <common.h>
#include <asm/io.h>
@@ -38,3 +40,4 @@ static inline u32 raw_bootcount_load(volatile u32 *addr)
return in_be32(addr);
}
#endif
+#endif /* _BOOTCOUNT_H__ */
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v3 3/7] bootcount: Add function wrappers to handle bootcount increment and error checking
2018-04-29 13:36 [U-Boot] [PATCH v3 0/7] Provide SPL support for bootcount (in the case of using falcon boot mode) Lukasz Majewski
2018-04-29 13:36 ` [U-Boot] [PATCH v3 1/7] bootcount: spl: Enable bootcount support in SPL Lukasz Majewski
2018-04-29 13:36 ` [U-Boot] [PATCH v3 2/7] bootcount: Add include guards into bootcount.h file Lukasz Majewski
@ 2018-04-29 13:36 ` Lukasz Majewski
2018-04-30 8:07 ` Stefan Roese
2018-04-29 13:36 ` [U-Boot] [PATCH v3 4/7] bootcount: Rewrite autoboot to use wrapper functions from bootcount.h Lukasz Majewski
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Lukasz Majewski @ 2018-04-29 13:36 UTC (permalink / raw)
To: u-boot
Those two functions can be used to provide easy bootcount management.
Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
Changes in v3:
- Unify those functions to also work with common/autoboot.c code
- Add enum bootcount_context to distinguish between u-boot proper and SPL
Changes in v2:
- None
include/bootcount.h | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/include/bootcount.h b/include/bootcount.h
index e3b3f7028e..16fc657b2a 100644
--- a/include/bootcount.h
+++ b/include/bootcount.h
@@ -11,6 +11,13 @@
#include <asm/io.h>
#include <asm/byteorder.h>
+enum bootcount_context {
+ SPL = 1,
+ UBOOT,
+};
+
+#if defined CONFIG_SPL_BOOTCOUNT_LIMIT || defined CONFIG_BOOTCOUNT_LIMIT
+
#if !defined(CONFIG_SYS_BOOTCOUNT_LE) && !defined(CONFIG_SYS_BOOTCOUNT_BE)
# if __BYTE_ORDER == __LITTLE_ENDIAN
# define CONFIG_SYS_BOOTCOUNT_LE
@@ -40,4 +47,47 @@ static inline u32 raw_bootcount_load(volatile u32 *addr)
return in_be32(addr);
}
#endif
+
+static inline int bootcount_error(enum bootcount_context bc)
+{
+ unsigned long bootcount = bootcount_load();
+ unsigned long bootlimit = env_get_ulong("bootlimit", 10, 0);
+
+ if (bootlimit && bootcount > bootlimit) {
+ printf("Warning: Bootlimit (%lu) exceeded.", bootlimit);
+ if (bc == UBOOT)
+ printf(" Using altbootcmd.");
+ printf("\n");
+
+ return 1;
+ }
+
+ return 0;
+}
+
+static inline void bootcount_inc(enum bootcount_context bc)
+{
+ unsigned long bootcount = bootcount_load();
+
+ if (bc == SPL) {
+ bootcount_store(++bootcount);
+ return;
+ }
+
+ /* Only increment bootcount when no bootcount support in SPL */
+#ifndef CONFIG_SPL_BOOTCOUNT_LIMIT
+ bootcount++;
+#endif
+ bootcount_store(bootcount);
+ env_set_ulong("bootcount", bootcount);
+}
+
+#if defined CONFIG_SPL_BUILD && !defined CONFIG_SPL_BOOTCOUNT_LIMIT
+void bootcount_store(ulong a) {};
+ulong bootcount_load(void) { return 0; }
+#endif /* CONFIG_SPL_BUILD && !CONFIG_SPL_BOOTCOUNT_LIMIT */
+#else
+static inline int bootcount_error(enum bootcount_context bc) { return 0; }
+static inline void bootcount_inc(enum bootcount_context bc) {}
+#endif /* CONFIG_SPL_BOOTCOUNT_LIMIT || CONFIG_BOOTCOUNT_LIMIT */
#endif /* _BOOTCOUNT_H__ */
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v3 4/7] bootcount: Rewrite autoboot to use wrapper functions from bootcount.h
2018-04-29 13:36 [U-Boot] [PATCH v3 0/7] Provide SPL support for bootcount (in the case of using falcon boot mode) Lukasz Majewski
` (2 preceding siblings ...)
2018-04-29 13:36 ` [U-Boot] [PATCH v3 3/7] bootcount: Add function wrappers to handle bootcount increment and error checking Lukasz Majewski
@ 2018-04-29 13:36 ` Lukasz Majewski
2018-04-30 8:08 ` Stefan Roese
2018-04-30 14:17 ` Tom Rini
2018-04-29 13:36 ` [U-Boot] [PATCH v3 5/7] bootcount: spl: Extend SPL to support bootcount incrementation Lukasz Majewski
` (2 subsequent siblings)
6 siblings, 2 replies; 17+ messages in thread
From: Lukasz Majewski @ 2018-04-29 13:36 UTC (permalink / raw)
To: u-boot
The code has been refactored to use common wrappers from bootcount.h
header.
Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
Changes in v3:
- New patch
Changes in v2:
- None
common/autoboot.c | 23 +++++------------------
1 file changed, 5 insertions(+), 18 deletions(-)
diff --git a/common/autoboot.c b/common/autoboot.c
index 2eef7a04cc..bfe4bdcf50 100644
--- a/common/autoboot.c
+++ b/common/autoboot.c
@@ -14,6 +14,7 @@
#include <menu.h>
#include <post.h>
#include <u-boot/sha256.h>
+#include <bootcount.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -291,18 +292,8 @@ const char *bootdelay_process(void)
{
char *s;
int bootdelay;
-#ifdef CONFIG_BOOTCOUNT_LIMIT
- unsigned long bootcount = 0;
- unsigned long bootlimit = 0;
-#endif /* CONFIG_BOOTCOUNT_LIMIT */
-
-#ifdef CONFIG_BOOTCOUNT_LIMIT
- bootcount = bootcount_load();
- bootcount++;
- bootcount_store(bootcount);
- env_set_ulong("bootcount", bootcount);
- bootlimit = env_get_ulong("bootlimit", 10, 0);
-#endif /* CONFIG_BOOTCOUNT_LIMIT */
+
+ bootcount_inc(UBOOT);
s = env_get("bootdelay");
bootdelay = s ? (int)simple_strtol(s, NULL, 10) : CONFIG_BOOTDELAY;
@@ -324,13 +315,9 @@ const char *bootdelay_process(void)
s = env_get("failbootcmd");
} else
#endif /* CONFIG_POST */
-#ifdef CONFIG_BOOTCOUNT_LIMIT
- if (bootlimit && (bootcount > bootlimit)) {
- printf("Warning: Bootlimit (%u) exceeded. Using altbootcmd.\n",
- (unsigned)bootlimit);
+ if (bootcount_error(UBOOT))
s = env_get("altbootcmd");
- } else
-#endif /* CONFIG_BOOTCOUNT_LIMIT */
+ else
s = env_get("bootcmd");
process_fdt_options(gd->fdt_blob);
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v3 5/7] bootcount: spl: Extend SPL to support bootcount incrementation
2018-04-29 13:36 [U-Boot] [PATCH v3 0/7] Provide SPL support for bootcount (in the case of using falcon boot mode) Lukasz Majewski
` (3 preceding siblings ...)
2018-04-29 13:36 ` [U-Boot] [PATCH v3 4/7] bootcount: Rewrite autoboot to use wrapper functions from bootcount.h Lukasz Majewski
@ 2018-04-29 13:36 ` Lukasz Majewski
2018-04-30 8:10 ` Stefan Roese
2018-04-30 14:17 ` Tom Rini
2018-04-29 13:36 ` [U-Boot] [PATCH v3 6/7] bootcount: display5: spl: Extend DISPLAY5 board SPL to support bootcount checking Lukasz Majewski
2018-04-29 13:36 ` [U-Boot] [PATCH v3 7/7] bootcount: display5: config: Enable boot count feature in the display5 board Lukasz Majewski
6 siblings, 2 replies; 17+ messages in thread
From: Lukasz Majewski @ 2018-04-29 13:36 UTC (permalink / raw)
To: u-boot
This patch adds support for incrementation of the bootcount in SPL.
Such feature is necessary when we do want to use this feature with
'falcon' boot mode (which loads OS directly in SPL).
Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
Changes in v3:
- Remove not needed #ifdefs
- Add enum bootcount_context parameter to bootcount_inc() function
Changes in v2:
- New patch - as suggested by Stefan Roese - bootcount_inc() is called
in common SPL code (./common/spl/spl.c), so other boards can also
reuse it without modification
common/spl/spl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/common/spl/spl.c b/common/spl/spl.c
index 61d3071324..2d10c84296 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -20,6 +20,7 @@
#include <dm/root.h>
#include <linux/compiler.h>
#include <fdt_support.h>
+#include <bootcount.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -417,6 +418,8 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
spl_board_init();
#endif
+ bootcount_inc(SPL);
+
memset(&spl_image, '\0', sizeof(spl_image));
#ifdef CONFIG_SYS_SPL_ARGS_ADDR
spl_image.arg = (void *)CONFIG_SYS_SPL_ARGS_ADDR;
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v3 6/7] bootcount: display5: spl: Extend DISPLAY5 board SPL to support bootcount checking
2018-04-29 13:36 [U-Boot] [PATCH v3 0/7] Provide SPL support for bootcount (in the case of using falcon boot mode) Lukasz Majewski
` (4 preceding siblings ...)
2018-04-29 13:36 ` [U-Boot] [PATCH v3 5/7] bootcount: spl: Extend SPL to support bootcount incrementation Lukasz Majewski
@ 2018-04-29 13:36 ` Lukasz Majewski
2018-04-29 13:36 ` [U-Boot] [PATCH v3 7/7] bootcount: display5: config: Enable boot count feature in the display5 board Lukasz Majewski
6 siblings, 0 replies; 17+ messages in thread
From: Lukasz Majewski @ 2018-04-29 13:36 UTC (permalink / raw)
To: u-boot
This patch is necessary for providing basic bootcount checking in the case
of using "falcon" boot mode in that board.
It forces u-boot proper boot, when we exceed the number of errors.
Signed-off-by: Lukasz Majewski <lukma@denx.de>
Reviewed-by: Stefan Roese <sr@denx.de>
---
Changes in v3:
- The bootcount_error now accepts enum bootcount_error input parameter
Changes in v2:
- Remove bootcount_init() from SPL specific board code
board/liebherr/display5/spl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/board/liebherr/display5/spl.c b/board/liebherr/display5/spl.c
index 437963e225..147ea62683 100644
--- a/board/liebherr/display5/spl.c
+++ b/board/liebherr/display5/spl.c
@@ -20,6 +20,7 @@
#include <environment.h>
#include <fsl_esdhc.h>
#include <netdev.h>
+#include <bootcount.h>
#include "common.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -214,7 +215,7 @@ void board_boot_order(u32 *spl_boot_list)
env_load();
s = env_get("BOOT_FROM");
- if (s && strcmp(s, "ACTIVE") == 0) {
+ if (s && !bootcount_error(SPL) && strcmp(s, "ACTIVE") == 0) {
spl_boot_list[0] = BOOT_DEVICE_MMC1;
spl_boot_list[1] = spl_boot_device();
}
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v3 7/7] bootcount: display5: config: Enable boot count feature in the display5 board
2018-04-29 13:36 [U-Boot] [PATCH v3 0/7] Provide SPL support for bootcount (in the case of using falcon boot mode) Lukasz Majewski
` (5 preceding siblings ...)
2018-04-29 13:36 ` [U-Boot] [PATCH v3 6/7] bootcount: display5: spl: Extend DISPLAY5 board SPL to support bootcount checking Lukasz Majewski
@ 2018-04-29 13:36 ` Lukasz Majewski
6 siblings, 0 replies; 17+ messages in thread
From: Lukasz Majewski @ 2018-04-29 13:36 UTC (permalink / raw)
To: u-boot
The boot count is enabled in both SPL and proper u-boot.
Signed-off-by: Lukasz Majewski <lukma@denx.de>
Reviewed-by: Stefan Roese <sr@denx.de>
---
Changes in v3:
- None
Changes in v2:
- None
configs/display5_defconfig | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/configs/display5_defconfig b/configs/display5_defconfig
index a5f1cf1e92..6a2408260f 100644
--- a/configs/display5_defconfig
+++ b/configs/display5_defconfig
@@ -16,6 +16,7 @@ CONFIG_SPL_LOAD_FIT=y
CONFIG_OF_BOARD_SETUP=y
CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=arch/arm/mach-imx/spl_sd.cfg,MX6Q"
CONFIG_SUPPORT_RAW_INITRD=y
+CONFIG_SPL_BOOTCOUNT_LIMIT=y
# CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR is not set
CONFIG_SPL_DMA_SUPPORT=y
CONFIG_SPL_ENV_SUPPORT=y
@@ -53,6 +54,9 @@ CONFIG_EFI_PARTITION=y
CONFIG_OF_CONTROL=y
CONFIG_ENV_IS_IN_SPI_FLASH=y
CONFIG_FSL_ESDHC=y
+CONFIG_BOOTCOUNT_LIMIT=y
+CONFIG_SYS_BOOTCOUNT_SINGLEWORD=y
+CONFIG_SYS_BOOTCOUNT_ADDR=0x020CC068
CONFIG_SPI_FLASH=y
CONFIG_SPI_FLASH_BAR=y
CONFIG_SPI_FLASH_SPANSION=y
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v3 3/7] bootcount: Add function wrappers to handle bootcount increment and error checking
2018-04-29 13:36 ` [U-Boot] [PATCH v3 3/7] bootcount: Add function wrappers to handle bootcount increment and error checking Lukasz Majewski
@ 2018-04-30 8:07 ` Stefan Roese
2018-04-30 8:39 ` Lukasz Majewski
0 siblings, 1 reply; 17+ messages in thread
From: Stefan Roese @ 2018-04-30 8:07 UTC (permalink / raw)
To: u-boot
Hi Lukasz,
sorry, I still have some comments to (hopefully) make this
implementation a bit more "elegant". Please see below.
On 29.04.2018 15:36, Lukasz Majewski wrote:
> Those two functions can be used to provide easy bootcount management.
>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
>
> ---
>
> Changes in v3:
> - Unify those functions to also work with common/autoboot.c code
> - Add enum bootcount_context to distinguish between u-boot proper and SPL
>
> Changes in v2:
> - None
>
> include/bootcount.h | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/include/bootcount.h b/include/bootcount.h
> index e3b3f7028e..16fc657b2a 100644
> --- a/include/bootcount.h
> +++ b/include/bootcount.h
> @@ -11,6 +11,13 @@
> #include <asm/io.h>
> #include <asm/byteorder.h>
>
> +enum bootcount_context {
> + SPL = 1,
> + UBOOT,
> +};
Perhaps better some bootcount specific values / enums, like:
enum bootcount_context {
BOOTCOUNT_STATE_SPL = 1,
BOOTCOUNT_STATE_UBOOT,
};
Or even more generic, as this "boot-state" does not have to be bootcount
specific:
enum u_boot_context {
U_BOOT_STATE_SPL = 1,
U_BOOT_STATE_U_BOOT,
};
Or do we already have something like this, perhaps in "gd", where
its marked, in which state we are currently running? If not, it
could be added there and could be used from the bootcounter code
and other interfaces / drivers as well. The parts pre-relocation and
post-relocation fall also into this area (for the pre- / post-reloc
we definitely have a variable / flag in gd somewhere). So perhaps:
enum u_boot_context {
U_BOOT_STATE_SPL = 1,
U_BOOT_STATE_U_BOOT_PRE_RELOC,
U_BOOT_STATE_U_BOOT_POST_RELOC,
};
> +#if defined CONFIG_SPL_BOOTCOUNT_LIMIT || defined CONFIG_BOOTCOUNT_LIMIT
> +
> #if !defined(CONFIG_SYS_BOOTCOUNT_LE) && !defined(CONFIG_SYS_BOOTCOUNT_BE)
> # if __BYTE_ORDER == __LITTLE_ENDIAN
> # define CONFIG_SYS_BOOTCOUNT_LE
> @@ -40,4 +47,47 @@ static inline u32 raw_bootcount_load(volatile u32 *addr)
> return in_be32(addr);
> }
> #endif
> +
> +static inline int bootcount_error(enum bootcount_context bc)
> +{
> + unsigned long bootcount = bootcount_load();
> + unsigned long bootlimit = env_get_ulong("bootlimit", 10, 0);
> +
> + if (bootlimit && bootcount > bootlimit) {
> + printf("Warning: Bootlimit (%lu) exceeded.", bootlimit);
> + if (bc == UBOOT)
> + printf(" Using altbootcmd.");
> + printf("\n");
> +
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +static inline void bootcount_inc(enum bootcount_context bc)
> +{
> + unsigned long bootcount = bootcount_load();
> +
> + if (bc == SPL) {
> + bootcount_store(++bootcount);
> + return;
> + }
> +
> + /* Only increment bootcount when no bootcount support in SPL */
> +#ifndef CONFIG_SPL_BOOTCOUNT_LIMIT
> + bootcount++;
> +#endif
> + bootcount_store(bootcount);
> + env_set_ulong("bootcount", bootcount);
> +}
Why not use the same logic / code as above (the store does not need
to happen twice):
/* Only increment bootcount when no bootcount support in SPL */
#ifndef CONFIG_SPL_BOOTCOUNT_LIMIT
bootcount_store(++bootcount);
#endif
env_set_ulong("bootcount", bootcount);
?
What do you think?
Thanks,
Stefan
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v3 4/7] bootcount: Rewrite autoboot to use wrapper functions from bootcount.h
2018-04-29 13:36 ` [U-Boot] [PATCH v3 4/7] bootcount: Rewrite autoboot to use wrapper functions from bootcount.h Lukasz Majewski
@ 2018-04-30 8:08 ` Stefan Roese
2018-04-30 14:17 ` Tom Rini
1 sibling, 0 replies; 17+ messages in thread
From: Stefan Roese @ 2018-04-30 8:08 UTC (permalink / raw)
To: u-boot
Hi Lukasz,
On 29.04.2018 15:36, Lukasz Majewski wrote:
> The code has been refactored to use common wrappers from bootcount.h
> header.
>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
>
> ---
>
> Changes in v3:
> - New patch
>
> Changes in v2:
> - None
>
> common/autoboot.c | 23 +++++------------------
> 1 file changed, 5 insertions(+), 18 deletions(-)
>
> diff --git a/common/autoboot.c b/common/autoboot.c
> index 2eef7a04cc..bfe4bdcf50 100644
> --- a/common/autoboot.c
> +++ b/common/autoboot.c
> @@ -14,6 +14,7 @@
> #include <menu.h>
> #include <post.h>
> #include <u-boot/sha256.h>
> +#include <bootcount.h>
>
> DECLARE_GLOBAL_DATA_PTR;
>
> @@ -291,18 +292,8 @@ const char *bootdelay_process(void)
> {
> char *s;
> int bootdelay;
> -#ifdef CONFIG_BOOTCOUNT_LIMIT
> - unsigned long bootcount = 0;
> - unsigned long bootlimit = 0;
> -#endif /* CONFIG_BOOTCOUNT_LIMIT */
> -
> -#ifdef CONFIG_BOOTCOUNT_LIMIT
> - bootcount = bootcount_load();
> - bootcount++;
> - bootcount_store(bootcount);
> - env_set_ulong("bootcount", bootcount);
> - bootlimit = env_get_ulong("bootlimit", 10, 0);
> -#endif /* CONFIG_BOOTCOUNT_LIMIT */
> +
> + bootcount_inc(UBOOT);
>
> s = env_get("bootdelay");
> bootdelay = s ? (int)simple_strtol(s, NULL, 10) : CONFIG_BOOTDELAY;
> @@ -324,13 +315,9 @@ const char *bootdelay_process(void)
> s = env_get("failbootcmd");
> } else
> #endif /* CONFIG_POST */
> -#ifdef CONFIG_BOOTCOUNT_LIMIT
> - if (bootlimit && (bootcount > bootlimit)) {
> - printf("Warning: Bootlimit (%u) exceeded. Using altbootcmd.\n",
> - (unsigned)bootlimit);
> + if (bootcount_error(UBOOT))
> s = env_get("altbootcmd");
> - } else
> -#endif /* CONFIG_BOOTCOUNT_LIMIT */
> + else
> s = env_get("bootcmd");
>
> process_fdt_options(gd->fdt_blob);
Now that look much simpler / cleaner. Thanks for working on this.
Reviewed-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v3 5/7] bootcount: spl: Extend SPL to support bootcount incrementation
2018-04-29 13:36 ` [U-Boot] [PATCH v3 5/7] bootcount: spl: Extend SPL to support bootcount incrementation Lukasz Majewski
@ 2018-04-30 8:10 ` Stefan Roese
2018-04-30 14:17 ` Tom Rini
1 sibling, 0 replies; 17+ messages in thread
From: Stefan Roese @ 2018-04-30 8:10 UTC (permalink / raw)
To: u-boot
Hi Lukasz,
On 29.04.2018 15:36, Lukasz Majewski wrote:
> This patch adds support for incrementation of the bootcount in SPL.
> Such feature is necessary when we do want to use this feature with
> 'falcon' boot mode (which loads OS directly in SPL).
>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
>
> ---
>
> Changes in v3:
> - Remove not needed #ifdefs
> - Add enum bootcount_context parameter to bootcount_inc() function
>
> Changes in v2:
> - New patch - as suggested by Stefan Roese - bootcount_inc() is called
> in common SPL code (./common/spl/spl.c), so other boards can also
> reuse it without modification
>
> common/spl/spl.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index 61d3071324..2d10c84296 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -20,6 +20,7 @@
> #include <dm/root.h>
> #include <linux/compiler.h>
> #include <fdt_support.h>
> +#include <bootcount.h>
>
> DECLARE_GLOBAL_DATA_PTR;
>
> @@ -417,6 +418,8 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
> spl_board_init();
> #endif
>
> + bootcount_inc(SPL);
> +
Thanks. With the suggested addition of automatic runtime detection
of the boot-stage (SPL vs U-Boot etc), this SPL parameter can be
dropped.
Other that this:
Reviewed-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v3 3/7] bootcount: Add function wrappers to handle bootcount increment and error checking
2018-04-30 8:07 ` Stefan Roese
@ 2018-04-30 8:39 ` Lukasz Majewski
0 siblings, 0 replies; 17+ messages in thread
From: Lukasz Majewski @ 2018-04-30 8:39 UTC (permalink / raw)
To: u-boot
Hi Stefan,
> Hi Lukasz,
>
> sorry, I still have some comments to (hopefully) make this
> implementation a bit more "elegant". Please see below.
>
> On 29.04.2018 15:36, Lukasz Majewski wrote:
> > Those two functions can be used to provide easy bootcount
> > management.
> >
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> >
> > ---
> >
> > Changes in v3:
> > - Unify those functions to also work with common/autoboot.c code
> > - Add enum bootcount_context to distinguish between u-boot proper
> > and SPL
> >
> > Changes in v2:
> > - None
> >
> > include/bootcount.h | 50
> > ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed,
> > 50 insertions(+)
> >
> > diff --git a/include/bootcount.h b/include/bootcount.h
> > index e3b3f7028e..16fc657b2a 100644
> > --- a/include/bootcount.h
> > +++ b/include/bootcount.h
> > @@ -11,6 +11,13 @@
> > #include <asm/io.h>
> > #include <asm/byteorder.h>
> >
> > +enum bootcount_context {
> > + SPL = 1,
> > + UBOOT,
> > +};
>
> Perhaps better some bootcount specific values / enums, like:
>
> enum bootcount_context {
> BOOTCOUNT_STATE_SPL = 1,
> BOOTCOUNT_STATE_UBOOT,
> };
>
> Or even more generic, as this "boot-state" does not have to be
> bootcount specific:
>
> enum u_boot_context {
> U_BOOT_STATE_SPL = 1,
> U_BOOT_STATE_U_BOOT,
> };
>
> Or do we already have something like this, perhaps in "gd", where
> its marked, in which state we are currently running? If not, it
> could be added there and could be used from the bootcounter code
> and other interfaces / drivers as well. The parts pre-relocation and
> post-relocation fall also into this area (for the pre- / post-reloc
> we definitely have a variable / flag in gd somewhere). So perhaps:
>
> enum u_boot_context {
> U_BOOT_STATE_SPL = 1,
> U_BOOT_STATE_U_BOOT_PRE_RELOC,
> U_BOOT_STATE_U_BOOT_POST_RELOC,
> };
I will check if we do have such flags already defined. "gd" approach
looks better (more generic for sure) than the one from v3.
>
> > +#if defined CONFIG_SPL_BOOTCOUNT_LIMIT || defined
> > CONFIG_BOOTCOUNT_LIMIT +
> > #if !defined(CONFIG_SYS_BOOTCOUNT_LE)
> > && !defined(CONFIG_SYS_BOOTCOUNT_BE) # if __BYTE_ORDER ==
> > __LITTLE_ENDIAN # define CONFIG_SYS_BOOTCOUNT_LE
> > @@ -40,4 +47,47 @@ static inline u32 raw_bootcount_load(volatile
> > u32 *addr) return in_be32(addr);
> > }
> > #endif
> > +
> > +static inline int bootcount_error(enum bootcount_context bc)
> > +{
> > + unsigned long bootcount = bootcount_load();
> > + unsigned long bootlimit = env_get_ulong("bootlimit", 10,
> > 0); +
> > + if (bootlimit && bootcount > bootlimit) {
> > + printf("Warning: Bootlimit (%lu) exceeded.",
> > bootlimit);
> > + if (bc == UBOOT)
> > + printf(" Using altbootcmd.");
> > + printf("\n");
> > +
> > + return 1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static inline void bootcount_inc(enum bootcount_context bc)
> > +{
> > + unsigned long bootcount = bootcount_load();
> > +
> > + if (bc == SPL) {
> > + bootcount_store(++bootcount);
> > + return;
> > + }
> > +
> > + /* Only increment bootcount when no bootcount support in
> > SPL */ +#ifndef CONFIG_SPL_BOOTCOUNT_LIMIT
> > + bootcount++;
> > +#endif
> > + bootcount_store(bootcount);
> > + env_set_ulong("bootcount", bootcount);
> > +}
>
> Why not use the same logic / code as above (the store does not need
> to happen twice):
>
> /* Only increment bootcount when no bootcount support in SPL
> */ #ifndef CONFIG_SPL_BOOTCOUNT_LIMIT
> bootcount_store(++bootcount);
> #endif
>
> env_set_ulong("bootcount", bootcount);
>
> ?
>
> What do you think?
Good point - I will add it in v4.
Thanks for input.
>
> Thanks,
> Stefan
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-10 Fax: (+49)-8142-66989-80 Email: wd 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: <http://lists.denx.de/pipermail/u-boot/attachments/20180430/b5de1d9d/attachment.sig>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v3 1/7] bootcount: spl: Enable bootcount support in SPL
2018-04-29 13:36 ` [U-Boot] [PATCH v3 1/7] bootcount: spl: Enable bootcount support in SPL Lukasz Majewski
@ 2018-04-30 14:17 ` Tom Rini
0 siblings, 0 replies; 17+ messages in thread
From: Tom Rini @ 2018-04-30 14:17 UTC (permalink / raw)
To: u-boot
On Sun, Apr 29, 2018 at 03:36:27PM +0200, Lukasz Majewski wrote:
> New, SPL related config option - CONFIG_SPL_BOOTCOUNT_LIMIT has been
> added to allow drivers/bootcount code re-usage in SPL.
>
> This code is necessary to use and setup bootcount in SPL in the case of
> falcon boot mode.
>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> Reviewed-by: Stefan Roese <sr@denx.de>
>
>
Reviewed-by: Tom Rini <trini@konsulko.com>
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180430/6039d08a/attachment.sig>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v3 2/7] bootcount: Add include guards into bootcount.h file
2018-04-29 13:36 ` [U-Boot] [PATCH v3 2/7] bootcount: Add include guards into bootcount.h file Lukasz Majewski
@ 2018-04-30 14:17 ` Tom Rini
0 siblings, 0 replies; 17+ messages in thread
From: Tom Rini @ 2018-04-30 14:17 UTC (permalink / raw)
To: u-boot
On Sun, Apr 29, 2018 at 03:36:28PM +0200, Lukasz Majewski wrote:
> This patch adds missing include guards for bootcount.h file.
>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> Reviewed-by: Stefan Roese <sr@denx.de>
>
Reviewed-by: Tom Rini <trini@konsulko.com>
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180430/cfaac831/attachment.sig>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v3 4/7] bootcount: Rewrite autoboot to use wrapper functions from bootcount.h
2018-04-29 13:36 ` [U-Boot] [PATCH v3 4/7] bootcount: Rewrite autoboot to use wrapper functions from bootcount.h Lukasz Majewski
2018-04-30 8:08 ` Stefan Roese
@ 2018-04-30 14:17 ` Tom Rini
1 sibling, 0 replies; 17+ messages in thread
From: Tom Rini @ 2018-04-30 14:17 UTC (permalink / raw)
To: u-boot
On Sun, Apr 29, 2018 at 03:36:30PM +0200, Lukasz Majewski wrote:
> The code has been refactored to use common wrappers from bootcount.h
> header.
>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
>
Reviewed-by: Tom Rini <trini@konsulko.com>
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180430/e226aa93/attachment.sig>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v3 5/7] bootcount: spl: Extend SPL to support bootcount incrementation
2018-04-29 13:36 ` [U-Boot] [PATCH v3 5/7] bootcount: spl: Extend SPL to support bootcount incrementation Lukasz Majewski
2018-04-30 8:10 ` Stefan Roese
@ 2018-04-30 14:17 ` Tom Rini
1 sibling, 0 replies; 17+ messages in thread
From: Tom Rini @ 2018-04-30 14:17 UTC (permalink / raw)
To: u-boot
On Sun, Apr 29, 2018 at 03:36:31PM +0200, Lukasz Majewski wrote:
> This patch adds support for incrementation of the bootcount in SPL.
> Such feature is necessary when we do want to use this feature with
> 'falcon' boot mode (which loads OS directly in SPL).
>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
>
Reviewed-by: Tom Rini <trini@konsulko.com>
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180430/9aa6daeb/attachment.sig>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v3 6/7] bootcount: display5: spl: Extend DISPLAY5 board SPL to support bootcount checking
2018-05-02 7:08 [U-Boot] [PATCH v3 0/7] Provide SPL support for bootcount (in the case of using falcon boot mode) Lukasz Majewski
@ 2018-05-02 7:08 ` Lukasz Majewski
0 siblings, 0 replies; 17+ messages in thread
From: Lukasz Majewski @ 2018-05-02 7:08 UTC (permalink / raw)
To: u-boot
This patch is necessary for providing basic bootcount checking in the case
of using "falcon" boot mode in that board.
It forces u-boot proper boot, when we exceed the number of errors.
Signed-off-by: Lukasz Majewski <lukma@denx.de>
Reviewed-by: Stefan Roese <sr@denx.de>
---
Changes in v4:
- Use global data pointer (gd) instead of bootcount specific enum (SPL)
Changes in v3:
- The bootcount_error now accepts enum bootcount_error input parameter
Changes in v2:
- Remove bootcount_init() from SPL specific board code
board/liebherr/display5/spl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/board/liebherr/display5/spl.c b/board/liebherr/display5/spl.c
index 437963e225..7712e5bc3f 100644
--- a/board/liebherr/display5/spl.c
+++ b/board/liebherr/display5/spl.c
@@ -20,6 +20,7 @@
#include <environment.h>
#include <fsl_esdhc.h>
#include <netdev.h>
+#include <bootcount.h>
#include "common.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -214,7 +215,7 @@ void board_boot_order(u32 *spl_boot_list)
env_load();
s = env_get("BOOT_FROM");
- if (s && strcmp(s, "ACTIVE") == 0) {
+ if (s && !bootcount_error() && strcmp(s, "ACTIVE") == 0) {
spl_boot_list[0] = BOOT_DEVICE_MMC1;
spl_boot_list[1] = spl_boot_device();
}
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-05-02 7:08 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-29 13:36 [U-Boot] [PATCH v3 0/7] Provide SPL support for bootcount (in the case of using falcon boot mode) Lukasz Majewski
2018-04-29 13:36 ` [U-Boot] [PATCH v3 1/7] bootcount: spl: Enable bootcount support in SPL Lukasz Majewski
2018-04-30 14:17 ` Tom Rini
2018-04-29 13:36 ` [U-Boot] [PATCH v3 2/7] bootcount: Add include guards into bootcount.h file Lukasz Majewski
2018-04-30 14:17 ` Tom Rini
2018-04-29 13:36 ` [U-Boot] [PATCH v3 3/7] bootcount: Add function wrappers to handle bootcount increment and error checking Lukasz Majewski
2018-04-30 8:07 ` Stefan Roese
2018-04-30 8:39 ` Lukasz Majewski
2018-04-29 13:36 ` [U-Boot] [PATCH v3 4/7] bootcount: Rewrite autoboot to use wrapper functions from bootcount.h Lukasz Majewski
2018-04-30 8:08 ` Stefan Roese
2018-04-30 14:17 ` Tom Rini
2018-04-29 13:36 ` [U-Boot] [PATCH v3 5/7] bootcount: spl: Extend SPL to support bootcount incrementation Lukasz Majewski
2018-04-30 8:10 ` Stefan Roese
2018-04-30 14:17 ` Tom Rini
2018-04-29 13:36 ` [U-Boot] [PATCH v3 6/7] bootcount: display5: spl: Extend DISPLAY5 board SPL to support bootcount checking Lukasz Majewski
2018-04-29 13:36 ` [U-Boot] [PATCH v3 7/7] bootcount: display5: config: Enable boot count feature in the display5 board Lukasz Majewski
2018-05-02 7:08 [U-Boot] [PATCH v3 0/7] Provide SPL support for bootcount (in the case of using falcon boot mode) Lukasz Majewski
2018-05-02 7:08 ` [U-Boot] [PATCH v3 6/7] bootcount: display5: spl: Extend DISPLAY5 board SPL to support bootcount checking Lukasz Majewski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.