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

[Stefan Roese]

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.

Thanks,
Stefan