Hi Alex, Stefan,
> 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.+1 > > With your patch version, the bootcounter would get incremented > twice in this case. > > Thanks, > Stefan Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
pgpATZ9W4p7XB.pgp
Description: OpenPGP digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
