Re: [U-Boot] [PATCH v5 3/7] bootcount: Add function wrappers to handle bootcount increment and error checking

8 May 2018


      On Tue, May 8, 2018 at 8:11 AM Stefan Roese sr@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