From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukasz Majewski Date: Mon, 26 Feb 2018 17:27:09 +0100 Subject: [U-Boot] [PATCH v1 4/5] bootcount: display5: spl: Extend SPL to support bootcount checking In-Reply-To: References: <20180226122246.10595-1-lukma@denx.de> <20180226122246.10595-5-lukma@denx.de> <6ee70cb8-3aeb-5470-2445-3e6374a3dc70@denx.de> <20180226162225.475e8617@jawa> <20180226163147.756745a7@jawa> Message-ID: <20180226172709.4e61a74c@jawa> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Stefan, > Hi Lukasz, > > On 26.02.2018 16:31, Lukasz Majewski wrote: > > On Mon, 26 Feb 2018 16:22:25 +0100 > > Lukasz Majewski wrote: > > > >> Hi Stefan, > >> > >>> Hi Lukasz, > >>> > >>> On 26.02.2018 13:22, Lukasz Majewski wrote: > >>>> This patch is necessary for providing basic bootcount checking in > >>>> the case of using "falcon" boot mode. > >>>> > >>>> It forces u-boot proper boot, when we exceed the number of > >>>> errors. > >>>> > >>>> Signed-off-by: Lukasz Majewski > >>>> --- > >>>> > >>>> board/liebherr/display5/spl.c | 6 +++++- > >>>> 1 file changed, 5 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/board/liebherr/display5/spl.c > >>>> b/board/liebherr/display5/spl.c index 0a36e656c0..5812f71dfc > >>>> 100644 --- a/board/liebherr/display5/spl.c > >>>> +++ b/board/liebherr/display5/spl.c > >>>> @@ -20,6 +20,7 @@ > >>>> #include > >>>> #include > >>>> #include > >>>> +#include > >>>> #include "common.h" > >>>> > >>>> DECLARE_GLOBAL_DATA_PTR; > >>>> @@ -194,6 +195,9 @@ void board_init_f(ulong dummy) > >>>> /* Clear the BSS. */ > >>>> memset(__bss_start, 0, __bss_end - __bss_start); > >>>> > >>>> + /* Increment bootcount */ > >>>> + bootcount_inc(); > >>>> + > >>> > >>> Wouldn't it be better, to move this incrementing into some common > >>> code, so that other platform who might use the bootcounter in SPL > >>> in the future could also use it without code duplication? > >>> > >>> What do you think? > >> > >> To provide this functionality I need to add bootcount_inc() and > >> check bootcount_error(). Both are static inlines from #include > >> > >> > >> However, maybe it would be better to put this code to generic SPL. > >> I will try to do it for v2. > > > > The problem here is that we need very early code to do this > > (increment bootcount). Now it is done in SPL's board_init_f. > > It is the first C function, called from crt0.S asm file. > > And why does it need to be done "very early" in the SPL? In U-Boot > proper, its not done very early but only after all is initialized - > so quite late in the boot process. Why should this be different for > SPL? Why not increment the bootcounter somewhere in board_init_r() > (common/spl/spl.c)? I think I got the point. Even if we break "early" in SPL, we will not be able to recover as SPL requires u-boot proper to take any action. > > > I'm not sure if we have such early code common for all > > archs/platforms. > > > > If we put this bootcount_inc() to latter SPL code, we risk hanging > > in SPL with WDT reset and not incremented bootcount... > > So you have a very fast watchdog on your system? No. I do use 60s. > > > Also the bootcount shall be incremented in the same function where > > we enable and initialize WDT. > > Hmmm. This is also different from U-Boot proper, AFAIK. Yes. This is different. We increment bootcount when we do have everything setup (just before we count down with "bootdelay"). WDT is setup earlier. > Where is your > watchdog now initialized in SPL? WDT is setup in board_init_f. Just after providing clocks. > You should be okay, if the watchdog > is enabled late in the SPL boot stage. Or am I missing something > here? No. I think that it would be safe to move the WDT and bootcount latter in SPL. > > 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: wd at denx.de -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 488 bytes Desc: OpenPGP digital signature URL: