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

Those two functions can be used to provide easy bootcount management.

Signed-off-by: Lukasz Majewski <lukma@denx.de>

---

Changes in v2:
- None

 include/bootcount.h | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

On 14.03.2018 18:24, Lukasz Majewski wrote:
> Those two functions can be used to provide easy bootcount management.
> 
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> 
> ---
> 
> Changes in v2:
> - None
> 
>   include/bootcount.h | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
> 
> diff --git a/include/bootcount.h b/include/bootcount.h
> index e3b3f7028e..0ec9af6e2b 100644
> --- a/include/bootcount.h
> +++ b/include/bootcount.h
> @@ -40,4 +40,29 @@ static inline u32 raw_bootcount_load(volatile u32 *addr)
>   	return in_be32(addr);
>   }
>   #endif
> +
> +#ifdef CONFIG_SPL_BOOTCOUNT_LIMIT
> +static inline bool bootcount_error(void)
> +{
> +	unsigned long bootcount = bootcount_load();
> +	unsigned long bootlimit = env_get_ulong("bootlimit", 10, 0);
> +
> +	if (bootlimit && (bootcount > bootlimit)) {
> +		printf("Warning: Bootlimit (%lu) exceeded.\n", bootlimit);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +static inline void bootcount_inc(void)
> +{
> +	unsigned long bootcount = bootcount_load();
> +
> +	bootcount_store(++bootcount);
> +}

Why not:

static inline void bootcount_inc(void)
{
	bootcount_store(bootcount_load() + 1);
}

instead ?

> +#else
> +static inline bool bootcount_error(void) { return false; }
> +static inline void bootcount_inc(void) {}
> +#endif /* CONFIG_SPL_BOOTCOUNT_LIMIT */
>   #endif /* _BOOTCOUNT_H__ */

And can't you use these helper functions in proper U-Boot as well
(common/autoboot.c). Looking at this old code there, there is also
room for improvement - perhaps as a new patch in this series. ;)

Thanks,
Stefan

Hi Stefan,

> On 14.03.2018 18:24, Lukasz Majewski wrote:
> > Those two functions can be used to provide easy bootcount
> > management.
> > 
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > 
> > ---
> > 
> > Changes in v2:
> > - None
> > 
> >   include/bootcount.h | 25 +++++++++++++++++++++++++
> >   1 file changed, 25 insertions(+)
> > 
> > diff --git a/include/bootcount.h b/include/bootcount.h
> > index e3b3f7028e..0ec9af6e2b 100644
> > --- a/include/bootcount.h
> > +++ b/include/bootcount.h
> > @@ -40,4 +40,29 @@ static inline u32 raw_bootcount_load(volatile
> > u32 *addr) return in_be32(addr);
> >   }
> >   #endif
> > +
> > +#ifdef CONFIG_SPL_BOOTCOUNT_LIMIT
> > +static inline bool bootcount_error(void)
> > +{
> > +	unsigned long bootcount = bootcount_load();
> > +	unsigned long bootlimit = env_get_ulong("bootlimit", 10,
> > 0); +
> > +	if (bootlimit && (bootcount > bootlimit)) {
> > +		printf("Warning: Bootlimit (%lu) exceeded.\n",
> > bootlimit);
> > +		return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +static inline void bootcount_inc(void)
> > +{
> > +	unsigned long bootcount = bootcount_load();
> > +
> > +	bootcount_store(++bootcount);
> > +}  
> 
> Why not:
> 
> static inline void bootcount_inc(void)
> {
> 	bootcount_store(bootcount_load() + 1);
> }
> 
> instead ?

+1

> 
> > +#else
> > +static inline bool bootcount_error(void) { return false; }
> > +static inline void bootcount_inc(void) {}
> > +#endif /* CONFIG_SPL_BOOTCOUNT_LIMIT */
> >   #endif /* _BOOTCOUNT_H__ */  
> 
> And can't you use these helper functions in proper U-Boot as well
> (common/autoboot.c). Looking at this old code there, there is also
> room for improvement
> - perhaps as a new patch in this series. ;)

It would be best to have it on top of this series - as other patches
depends on it....

But, OK I will check if this can be done.

Thanks for hint.

> 
> 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@denx.de