On Tue, May 8, 2018 at 8:41 AM Lukasz Majewski <lu...@denx.de> wrote:
> On Tue, 08 May 2018 05:15:13 +0000 > Alex Kiernan <alex.kier...@gmail.com> wrote: > > On Wed, May 2, 2018 at 3:11 PM Lukasz Majewski <lu...@denx.de> wrote: > > > > > Those two functions can be used to provide easy bootcount > > > management. > > > > > Signed-off-by: Lukasz Majewski <lu...@denx.de> > > > > > Reviewed-by: Tom Rini <tr...@konsulko.com> > > > Reviewed-by: Stefan Roese <s...@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 in > > > SPL */ +#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. 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. -- Alex Kiernan _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot