On Mon, Feb 26, 2018 at 03:53:18PM +0100, Lukasz Majewski wrote:
Hi Tom,
On Mon, Feb 26, 2018 at 01:22:44PM +0100, Lukasz Majewski wrote:
If the CONFIG_SPL_BOOTCOUNT_LIMIT is defined, the bootcount variable is already incremented after each boot attempt.
For that reason we shall not increment it again in u-boot.
Signed-off-by: Lukasz Majewski lukma@denx.de
common/autoboot.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/common/autoboot.c b/common/autoboot.c index 2eef7a04cc..87fca2ea92 100644 --- a/common/autoboot.c +++ b/common/autoboot.c @@ -298,7 +298,9 @@ const char *bootdelay_process(void)
#ifdef CONFIG_BOOTCOUNT_LIMIT bootcount = bootcount_load(); +#ifndef CONFIG_SPL_BOOTCOUNT_LIMIT bootcount++; +#endif bootcount_store(bootcount); env_set_ulong("bootcount", bootcount); bootlimit = env_get_ulong("bootlimit", 10, 0);
Shouldn't we make the whole bit of code here depend on !CONFIG_SPL_BOOTCOUNT_LIMIT ?
Maybe I will try to present the scenario:
We do use SPL to boot from either eMMC (if in falcon) or from SPI-NOR (if not in falcon)
With bootcount: -- We may want to boot into eMMC directly, so we need to increment the bootcount (no matter where it is located).
-- We intentionally (by setting other env variable) or not (by entering X resets) abort falcon mode boot from SPL and boot to u-boot (which is in spi-nor). In this case we do need all the bootcount limit functionality (i.e. altbootcmd) excluding increment bootcount (as it was done in SPL).
+#ifndef CONFIG_SPL_BOOTCOUNT_LIMIT is only required to achieve this goal without making any more code change...
And you have one build that produces the binaries used in both cases, OK.
Or for that matter, re-work the first patch to instead be: bool SUPPORT_BOOTCOUNT "Support bootcount in some way" choice "Bootcount location" bool SPL_BOOTCOUNT_LIMIT "Count boots from POV of SPL" bool BOOTCOUNT_LIMIT "Count boots from POV of full U-Boot" endchoice
But then SPL_BOOTCOUNT_LIMIT would select BOOTCOUNT_LIMIT to fulfil the above scenario.
? Looking back at my old series from 2014 to do this, I just had a big warning saying you need to enable bootcount for SPL or for full U-Boot so you would never set BOOTCOUNT_LIMIT if doing SPL bootcount instead.
Such functionality can be achieved with SPL_BOOTCOUNT_LIMIT depending on BOOTCOUNT_LIMIT.
If SPL_BOOTCOUNT_LIMIT selected, then "[PATCH v1 3/5] bootcount: u-boot: Do not increment bootcount if already done in SPL" does its job.
OK, I expect that once this code comes in, it might get complicated a bit further by other use cases. But I don't have a specific example I'm going to implement currently, so I'm OK moving forward here and expanding it as other cases come up. Thanks!