[U-Boot] [PATCH 0/2] Add bootcount support to SPL

[U-Boot] [PATCH 0/2] Add bootcount support to SPL

[Tom Rini]

The following patches depend on the series I posted earlier that adds
environment support to SPL as well as the series that converts the
davinci bootcount driver to support am335x as well.  The most obvious
issue to me as that as Scott pointed out with the last series, this
would be clearer Kconfig where we wouldn't need to introduce
CONFIG_SPL_BOOTCOUNT_SUPPORT and just re-use CONFIG_BOOTCOUNT_SUPPORT.
Other than that, I think things look good.  It may be possible to
re-jigger bootcount drivers to allow counting for both SPL and full
U-Boot to happen, if that's strongly desired but the way I see this
being used is to fall-back to a failsafe full U-Boot to do what's needed
there.

-- 
Tom

[U-Boot] [PATCH 1/2] SPL: Add CONFIG_SPL_BOOTCOUNT_SUPPORT

[Tom Rini]

Add a new symbol, CONFIG_SPL_BOOTCOUNT_SUPPORT, to make use of the
existing BOOTCOUNT_SUPPORT within SPL.  It is strongly discouraged to
use bootcount in both SPL and full U-Boot, as they use the same counter.

Signed-off-by: Tom Rini <trini@ti.com>
---
 README            |    6 ++++++
 common/spl/spl.c  |   27 +++++++++++++++++++++++++++
 doc/README.falcon |    8 +++++++-
 include/spl.h     |    1 +
 spl/Makefile      |    1 +
 5 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/README b/README
index de17f59..5351d24 100644
--- a/README
+++ b/README
@@ -3035,6 +3035,12 @@ FIT uImage format:
 		Enable booting directly to an OS from SPL.
 		See also: doc/README.falcon
 
+		CONFIG_SPL_BOOTCOUNT_LIMIT
+		Optional part of CONFIG_SPL_OS_BOOT and requires
+		CONFIG_SPL_ENV_SUPPORT.  Adds the bootcount facility to
+		SPL.  It is strongly encouraged to not use bootcount in
+		both SPL and full U-Boot.
+
 		CONFIG_SPL_DISPLAY_PRINT
 		For ARM, enable an optional function to print more information
 		about the running system.
diff --git a/common/spl/spl.c b/common/spl/spl.c
index da31457..ffa49d9 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -38,6 +38,10 @@ static bd_t bdata __attribute__ ((section(".data")));
  *
  * Please implement your own board specific funcion to do this.
  *
+ * If both CONFIG_SPL_BOOTCOUNT_LIMIT and CONFIG_SPL_ENV_SUPPORT are
+ * set this function is responsible for calling
+ * spl_bootcount_limit_exceeded();
+ *
  * RETURN
  * 0 to not start u-boot
  * positive if u-boot should start
@@ -49,6 +53,29 @@ __weak int spl_start_uboot(void)
 	puts("SPL: Direct Linux boot not active!\n");
 	return 1;
 }
+
+#if defined(CONFIG_SPL_BOOTCOUNT_LIMIT) && defined(CONFIG_SPL_ENV_SUPPORT)
+/*
+ * Determine if we have exceeded our bootlimit.
+ *
+ * @return 1 if we have exceeded our limit, 0 otherwise.
+ */
+int spl_bootcount_limit_exceeded(void)
+{
+	unsigned long bootcount = 0;
+	unsigned long bootlimit = 0;
+
+	bootcount = bootcount_load();
+	bootcount++;
+	bootcount_store (bootcount);
+	setenv_ulong("bootcount", bootcount);
+	bootlimit = getenv_ulong("bootlimit", 10, 0);
+	printf("bootcount: %ld\nbootlimit: %ld\n", bootcount, bootlimit);
+	if (bootlimit)
+		return bootcount > bootlimit;
+	return 0;
+}
+#endif /* CONFIG_BOOTCOUNT_LIMIT && CONFIG_SPL_ENV_SUPPORT */
 #endif
 
 /*
diff --git a/doc/README.falcon b/doc/README.falcon
index 82a254b..c34c171 100644
--- a/doc/README.falcon
+++ b/doc/README.falcon
@@ -70,6 +70,8 @@ CONFIG_CMD_SPL_WRITE_SIZE 	Size of the parameters area to be copied
 
 CONFIG_SPL_OS_BOOT	Activate Falcon Mode.
 
+CONFIG_SPL_BOOTCOUNT_LIMIT	Use bootcount support to fall back to U-Boot
+
 Function that a board must implement
 ------------------------------------
 
@@ -78,7 +80,8 @@ void spl_board_prepare_for_linux(void) : optional
 
 spl_start_uboot() : required
 		Returns "0" if SPL should start the kernel, "1" if U-Boot
-		must be started.
+		must be started.  Must call spl_bootcount_limit_exceeded if
+		CONFIG_SPL_BOOTCOUNT_LIMIT is to be supported
 
 Environment variables
 ---------------------
@@ -89,6 +92,9 @@ mode.  In this case the following variables may be supported:
 boot_os : 		Set to yes/Yes/true/True/1 to enable booting to OS,
 			any other value to fall back to U-Boot (including
 			unset)
+bootlimit :		As part of CONFIG_SPL_BOOTCOUNT_LIMIT used to set the
+			maximum number of times to try and boot the OS before
+			falling back to U-Boot.
 falcon_args_file :	Filename to load as the 'args' portion of falcon mode
 			rather than the hard-coded value.
 falcon_image_file :	Filename to load as the OS image portion of falcon
diff --git a/include/spl.h b/include/spl.h
index 2bd6e16..ca654b3 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -37,6 +37,7 @@ void spl_parse_image_header(const struct image_header *header);
 void spl_board_prepare_for_linux(void);
 void __noreturn jump_to_image_linux(void *arg);
 int spl_start_uboot(void);
+int spl_bootcount_limit_exceeded(void);
 void spl_display_print(void);
 
 /* NAND SPL functions */
diff --git a/spl/Makefile b/spl/Makefile
index fa642ec..2a34df8 100644
--- a/spl/Makefile
+++ b/spl/Makefile
@@ -99,6 +99,7 @@ LIBS-$(CONFIG_SPL_ETH_SUPPORT) += drivers/net/phy/libphy.o
 LIBS-$(CONFIG_SPL_USBETH_SUPPORT) += drivers/net/phy/libphy.o
 LIBS-$(CONFIG_SPL_MUSB_NEW_SUPPORT) += drivers/usb/musb-new/libusb_musb-new.o
 LIBS-$(CONFIG_SPL_USBETH_SUPPORT) += drivers/usb/gadget/libusb_gadget.o
+LIBS-$(CONFIG_SPL_BOOTCOUNT_LIMIT) += drivers/bootcount/libbootcount.o
 LIBS-$(CONFIG_SPL_WATCHDOG_SUPPORT) += drivers/watchdog/libwatchdog.o
 
 ifneq ($(CONFIG_OMAP_COMMON),)
-- 
1.7.9.5

[U-Boot] [PATCH 1/2] SPL: Add CONFIG_SPL_BOOTCOUNT_SUPPORT

[Stefan Roese]

Hi Tom,

On 27.09.2013 21:26, Tom Rini wrote:
> Add a new symbol, CONFIG_SPL_BOOTCOUNT_SUPPORT, to make use of the
> existing BOOTCOUNT_SUPPORT within SPL.  It is strongly discouraged to
> use bootcount in both SPL and full U-Boot, as they use the same counter.

I just noticed that I missed sending the SPL bootcount implementation I
did a few weeks ago to the list. I'll send it shortly. The main
differences I see right now are:

- My implementation requires CONFIG_BOOTCOUNT_LIMIT and
  CONFIG_SPL_BOOTCOUNT_SUPPORT to be configured

- The check is not added to the board-specific code, but the
  medium code (here spl_nor.c). This might be consolidated even
  more by moving it to a non-boot-medium specific location (spl.c)?

Please take a look at it and let me know if which way to go.

Thanks,
Stefan

[U-Boot] [PATCH 1/2] SPL: Add CONFIG_SPL_BOOTCOUNT_SUPPORT

[Tom Rini]

On Mon, Sep 30, 2013 at 08:37:56AM +0200, Stefan Roese wrote:
> Hi Tom,
> 
> On 27.09.2013 21:26, Tom Rini wrote:
> > Add a new symbol, CONFIG_SPL_BOOTCOUNT_SUPPORT, to make use of the
> > existing BOOTCOUNT_SUPPORT within SPL.  It is strongly discouraged to
> > use bootcount in both SPL and full U-Boot, as they use the same counter.
> 
> I just noticed that I missed sending the SPL bootcount implementation I
> did a few weeks ago to the list. I'll send it shortly. The main
> differences I see right now are:
> 
> - My implementation requires CONFIG_BOOTCOUNT_LIMIT and
>   CONFIG_SPL_BOOTCOUNT_SUPPORT to be configured

OK, I see where you went with this, and I like this idea better.  I
shall incorporate that in v2.

> - The check is not added to the board-specific code, but the
>   medium code (here spl_nor.c). This might be consolidated even
>   more by moving it to a non-boot-medium specific location (spl.c)?

This I think takes things the wrong way.  When it's part of
spl_start_uboot we cover all boot method cases (MMC and NAND, in my
case).  Perhaps we should switch to a useful, but still weak default
function?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20131002/6ef38eed/attachment.pgp>

[U-Boot] [PATCH 2/2] am335x: Switch to CONFIG_SPL_BOOTCOUNT_SUPPORT

[Tom Rini]

Signed-off-by: Tom Rini <trini@ti.com>
---
 board/ti/am335x/board.c            |    4 ++++
 include/configs/ti_am335x_common.h |    2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c
index 0cfd048..0d6912b 100644
--- a/board/ti/am335x/board.c
+++ b/board/ti/am335x/board.c
@@ -239,6 +239,10 @@ int spl_start_uboot(void)
 #ifdef CONFIG_SPL_ENV_SUPPORT
 	env_init();
 	env_relocate_spec();
+#if defined(CONFIG_SPL_BOOTCOUNT_LIMIT) && defined(CONFIG_SPL_ENV_SUPPORT)
+	if (spl_bootcount_limit_exceeded())
+		return 1;
+#endif
 	if (getenv_yesno("boot_os") != 1)
 		return 1;
 #endif
diff --git a/include/configs/ti_am335x_common.h b/include/configs/ti_am335x_common.h
index a80bbf9..59c758c 100644
--- a/include/configs/ti_am335x_common.h
+++ b/include/configs/ti_am335x_common.h
@@ -40,7 +40,7 @@
 #define CONFIG_DRIVER_TI_CPSW		/* Driver for IP block */
 #define CONFIG_MII			/* Required in net/eth.c */
 
-#define CONFIG_BOOTCOUNT_LIMIT
+#define CONFIG_SPL_BOOTCOUNT_LIMIT
 #define CONFIG_SYS_BOOTCOUNT_ADDR	0x44E3E000
 
 /*
-- 
1.7.9.5

[U-Boot] [PATCH 0/2] Add bootcount support to SPL

[Otavio Salvador]

On Fri, Sep 27, 2013 at 4:26 PM, Tom Rini <trini@ti.com> wrote:
> The following patches depend on the series I posted earlier that adds
> environment support to SPL as well as the series that converts the
> davinci bootcount driver to support am335x as well.  The most obvious
> issue to me as that as Scott pointed out with the last series, this
> would be clearer Kconfig where we wouldn't need to introduce
> CONFIG_SPL_BOOTCOUNT_SUPPORT and just re-use CONFIG_BOOTCOUNT_SUPPORT.
> Other than that, I think things look good.  It may be possible to
> re-jigger bootcount drivers to allow counting for both SPL and full
> U-Boot to happen, if that's strongly desired but the way I see this
> being used is to fall-back to a failsafe full U-Boot to do what's needed
> there.

I think we need to count both; the SPL part solves the U-Boot failsafe
but the full U-Boot is needed to failsafe the OS.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

[U-Boot] [PATCH 0/2] Add bootcount support to SPL

[Tom Rini]

On Fri, Sep 27, 2013 at 10:20:52PM -0300, Otavio Salvador wrote:
> On Fri, Sep 27, 2013 at 4:26 PM, Tom Rini <trini@ti.com> wrote:
> > The following patches depend on the series I posted earlier that adds
> > environment support to SPL as well as the series that converts the
> > davinci bootcount driver to support am335x as well.  The most obvious
> > issue to me as that as Scott pointed out with the last series, this
> > would be clearer Kconfig where we wouldn't need to introduce
> > CONFIG_SPL_BOOTCOUNT_SUPPORT and just re-use CONFIG_BOOTCOUNT_SUPPORT.
> > Other than that, I think things look good.  It may be possible to
> > re-jigger bootcount drivers to allow counting for both SPL and full
> > U-Boot to happen, if that's strongly desired but the way I see this
> > being used is to fall-back to a failsafe full U-Boot to do what's needed
> > there.
> 
> I think we need to count both; the SPL part solves the U-Boot failsafe
> but the full U-Boot is needed to failsafe the OS.

It's _possible_ we could split the counter up further, 16bit magic, 8
bit SPL count, 8 bit U-Boot count.  But I'm not sure if it's useful.
Looking at Stefan's patch, we count in SPL, and use altbootcmd in full
U-Boot to know we have failure rather than just booting fully via
U-Boot.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20131002/a321b41c/attachment.pgp>