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

login
register
mail settings
Submitter Tom Rini
Date Sept. 27, 2013, 7:26 p.m.
Message ID <1380309969-5618-2-git-send-email-trini@ti.com>
Download mbox | patch
Permalink /patch/278671/
State New
Delegated to: Tom Rini
Headers show

Comments

Tom Rini - Sept. 27, 2013, 7:26 p.m.
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(-)
Stefan Roese - Sept. 30, 2013, 6:37 a.m.
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
Tom Rini - Oct. 2, 2013, 1:14 p.m.
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?

Patch

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),)