Re: [PATCH v2 2/3] ARM: lib32: pbl: implement get_time_ns and is_timeout

On 2026-08-11 12:21, Ahmad Fatoum wrote:
> Hi,
> 
> On 8/11/26 12:13 PM, Stefan Kerkmann wrote:
> > ARMv7-A cores can implement the optional 64bit wide generic timer
> > CNTPCT[1]. If it is present we can use it to implement the get_time_ns
> > and is_timeout helpers which in turn enable the whole read_poll_timeout
> > class of functions in the PBL. As it is guaranteed that the timer won't
> > wrap for 40 years no overflow handling is necessary.
> > 
> > [1]: See "ARM Architecture Reference Manual ARMv7-A and ARMv7-R edition
> > (rev C.d)", Chapter B8 "The Generic Timer"
> > 
> > Signed-off-by: Stefan Kerkmann <[email protected]>
> > ---
> >  arch/arm/lib32/arm_architected_timer.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/lib32/arm_architected_timer.c b/arch/arm/lib32/arm_architected_timer.c
> > index 54eca13f8b..75f74cbc15 100644
> > --- a/arch/arm/lib32/arm_architected_timer.c
> > +++ b/arch/arm/lib32/arm_architected_timer.c
> > @@ -10,8 +10,21 @@ void arm_architected_timer_udelay(unsigned long us)
> >  	unsigned long long ticks, cntfrq = get_cntfrq();
> >  	unsigned long long start = get_cntpct();
> >  
> > -	ticks = DIV_ROUND_DOWN_ULL((us * cntfrq), 1000000);
> > +	ticks = DIV_ROUND_DOWN_ULL((us * cntfrq), MSECOND);
> >  
> >  	while ((long)(start + ticks - get_cntpct()) > 0)
> >  		;
> >  }
> > +
> > +uint64_t get_time_ns(void)
> > +{
> > +	return get_cntpct() * SECOND / get_cntfrq();
> 
> I think you need to use one of the 64-bit division macros here.

Also the corresponding barebox proper code doesn't depend on the timer
being initialized to 0. I don't know if we can guarantee this for the
architected timer, but otherwise get_cntpct() * SECOND could easily
overflow.

> 
> > +int is_timeout(uint64_t start, uint64_t time_offset_ns)
> > +{
> > +	if ((int64_t)(start + time_offset_ns - get_time_ns()) < 0)
> > +		return 1;
> > +	else
> > +		return 0;
> 
> That's a convoluted way to write
> 
>   return get_time_ns() >= start + time_offset_ns;
> 
> Why did you structure it this way with the cast?
> 
> (A 64-bit nanosecond timestamp overflows after hundreds of years).

It's a direct copy of the non-pbl is_timeout implementation. It's
agnostic to the timer initialization value which is not necessarily 0.

Sascha

--
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |