On Tue, May 8, 2018 at 10:21 AM Lukasz Majewski lukma@denx.de wrote:
Hi Alex,
On Tue, May 8, 2018 at 8:41 AM Lukasz Majewski lukma@denx.de wrote:
On Tue, 08 May 2018 05:15:13 +0000 Alex Kiernan alex.kiernan@gmail.com wrote:
On Wed, May 2, 2018 at 3:11 PM Lukasz Majewski lukma@denx.de wrote:
Those two functions can be used to provide easy bootcount management.
Signed-off-by: Lukasz Majewski lukma@denx.de
Reviewed-by: Tom Rini trini@konsulko.com Reviewed-by: Stefan Roese sr@denx.de
Changes in v5:
- Provide parenthesis for #if defined(FOO) && ...
Changes in v4:
- Remove enum bootcount_context and replace it with checking
gd_t->flags (The GD_FLG_SPL_INIT is only set in SPL, it is cleared in u-boot proper, so can be used as indication if we are in u-boot or SPL).
- Do not call bootcount_store() twice when it is not needed.
- Call env_set_ulong("bootcount", bootcount); only in NON SPL
context - Boards with TINY_PRINTF (in newest mainline) will build break as this
function
requires simple_itoa() from vsprintf.c (now not always build in SPL).
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 | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
diff --git a/include/bootcount.h b/include/bootcount.h index e3b3f7028e..a886c98f44 100644 --- a/include/bootcount.h +++ b/include/bootcount.h @@ -11,6 +11,8 @@ #include <asm/io.h> #include <asm/byteorder.h>
+#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 +42,49 @@ static inline u32 raw_bootcount_load(volatile u32
*addr)
return in_be32(addr);} #endif
+DECLARE_GLOBAL_DATA_PTR; +static inline int bootcount_error(void) +{
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 (!(gd->flags & GD_FLG_SPL_INIT))printf(" Using altbootcmd.");printf("\n");return 1;}return 0;+}
+static inline void bootcount_inc(void) +{
unsigned long bootcount = bootcount_load();if (gd->flags & GD_FLG_SPL_INIT) {bootcount_store(++bootcount);return;}+#ifndef CONFIG_SPL_BUILD
/* Only increment bootcount when no bootcount support inSPL */ +#ifndef CONFIG_SPL_BOOTCOUNT_LIMIT
bootcount_store(++bootcount);+#endif
env_set_ulong("bootcount", bootcount);+#endif /* !CONFIG_SPL_BUILD */ +}
I'm kinda confused by this code... isn't this equivalent.?
static inline void bootcount_inc(void) { unsigned long bootcount = bootcount_load();
bootcount_store(++bootcount);#ifndef CONFIG_SPL_BUILD env_set_ulong("bootcount", bootcount); #endif /* !CONFIG_SPL_BUILD */ }
Also I suspect bootcount_store() will fail at link time on boards where the bootcount is stored in ext4?
I've run this patch set several times with travis-CI. No errors were present (the travis-ci link is in the cover letter).
Maybe there are some out of tree boards, which use the bootcount in some "odd" way....
I'm only really aware of the ext4 stuff from when I picked through it to consolidate the bootcount into Kconfig. I don't think there would be any Travis failures for this case as you need to have bootcount in SPL which you can't have before this series.
Could you be more specific here?
This series adds support for bootcount in SPL. The travis-CI tests which I've performed were correct for the all boards which use it:
The bootcount_inc() is added in generic SPL code - this seems to work on all boards - boards which don't define CONFIG_SPL_BOOTCOUNT will just return from it.
However, I don't know how the code will behave if one would like to add ext4 support to it (including ext4 support to SPL).
I suppose that those boards will not enable SPL BOOTCOUNT in SPL and use the code as it is now - just in u-boot.
This patch series was designed with one main use case in mind - the "falcon" boot of Linux from SPL with bootcount support.
If I try building a board like this (choose ext4 for the bootcount) and it fails to link:
drivers/built-in.o: In function `bootcount_store': u-boot/drivers/bootcount/bootcount_ext.c:18: undefined reference to `fs_set_blk_dev' u-boot/drivers/bootcount/bootcount_ext.c:29: undefined reference to `fs_write' drivers/built-in.o: In function `bootcount_load': u-boot/drivers/bootcount/bootcount_ext.c:41: undefined reference to `fs_set_blk_dev' u-boot/drivers/bootcount/bootcount_ext.c:47: undefined reference to `fs_read'
But having just played around with Kconfig a bit for this case, I'm not sure there's anything that's actually any better than a link time error; anything we did in Kconfig would just end up modelling out that link error.
Could you share which particular board this breaks? I've tested it on Beagle Bone Black (and display5).
There isn't one (at least not one I'm aware of), but this gives another impossible combination which you can select, of which we've lots already.
I should've played around some more before commenting as I think what you've got is everything that's needed.
-- Alex Kiernan