On Tue, May 8, 2018 at 8:11 AM Stefan Roese <s...@denx.de> wrote:
> On 08.05.2018 08:58, Alex Kiernan wrote:
> <snip>
> >>> +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 */
> >> }
> >
> > I should've included my reasoning as I've got to be missing
something... if
> > GD_FLG_SPL_INIT is always set when we get here in SPL, then it's
equivalent
> > to the compile time guard. Which I think says I don't understand the
flow
> > to how we get here, otherwise we wouldn't need the runtime guard.
> When using with SPL and bootcounter support, this code will get
> called twice, first from the SPL, where the counter will get
> incremented. And second from main U-Boot, where we need to make
> sure, that the counter does not get incremented again, if SPL
> has already done so.
> With your patch version, the bootcounter would get incremented
> twice in this case.
Ahh... thank you. That was the important piece!
--
Alex Kiernan
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
