Re: Subject: [PATCH 1/1] ARM:lib32: add architected timer

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi,

On 03.03.23 18:05, Renaud Barbier wrote:
> In preparation for the introduction of the LS1021A support,
> add a specific timer support based on the LS1046A support so
> that delays can be used in the PBL.
> 
> Signed-off-by: Renaud Barbier <renaud.barbier@ametek.com>
> ---
>  arch/arm/lib32/Makefile                |  2 ++
>  arch/arm/lib32/arm_architected_timer.c | 17 +++++++++++++++++
>  include/clock.h                        |  2 ++
>  3 files changed, 21 insertions(+)
>  create mode 100644 arch/arm/lib32/arm_architected_timer.c
> 
> diff --git a/arch/arm/lib32/Makefile b/arch/arm/lib32/Makefile
> index 82507fffc0..d54fb7644c 100644
> --- a/arch/arm/lib32/Makefile
> +++ b/arch/arm/lib32/Makefile
> @@ -31,6 +31,8 @@ extra-y += barebox.lds
>  pbl-y	+= lib1funcs.o
>  pbl-y	+= ashldi3.o
>  pbl-y	+= div0.o
> +pbl-$(CONFIG_CPU_32v7)	+= arm_architected_timer.o
> +CFLAGS_arm_architected_timer.o := -march=armv7-a

Sorry, I should have been clearer before. Once you only build the
file for ARMv7, you don't need to mess with -march=armv7-a anymore,
see arch/arm/Makefile:
  
  arch-$(CONFIG_CPU_32v7) :=-D__LINUX_ARM_ARCH__=7 $(call cc-option,-march=armv7-a,-march=armv5t -Wa$(comma)-march=armv7-a)

>  obj-pbl-y	+= setjmp.o
>  
> diff --git a/arch/arm/lib32/arm_architected_timer.c b/arch/arm/lib32/arm_architected_timer.c
> new file mode 100644
> index 0000000000..83b49656cb
> --- /dev/null
> +++ b/arch/arm/lib32/arm_architected_timer.c
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <asm/system.h>
> +#include <clock.h>
> +#include <common.h>
> +
> +/* Unlike the ARMv8, the timer is not generic to ARM32 */
> +void arm_architected_timer_udelay(unsigned long us)
> +{
> +	unsigned long long ticks, cntfrq = get_cntfrq();
> +	unsigned long long start = get_cntpct();
> +
> +	ticks = us * cntfrq + 999999;
> +	do_div(ticks, 1000000);

There's DIV_ROUND_UP_ULL you could use instead of opencoding that.
I see that we round down in the udelay for ARM64.
Do you think we should rather be rounding up there too?

In any case, it's worth mentioning why you choose to round up here
in the commit message.

> +
> +	while ((long)(start + ticks - get_cntpct()) > 0);
> +}
> diff --git a/include/clock.h b/include/clock.h
> index e6197e7eb0..8e07c35d37 100644
> --- a/include/clock.h
> +++ b/include/clock.h
> @@ -35,6 +35,8 @@ uint32_t clocksource_hz2mult(uint32_t hz, uint32_t shift_constant);
>  int is_timeout(uint64_t start_ns, uint64_t time_offset_ns);
>  int is_timeout_non_interruptible(uint64_t start_ns, uint64_t time_offset_ns);
>  
> +void arm_architected_timer_udelay(unsigned long us);
> +
>  void ndelay(unsigned long nsecs);
>  void udelay(unsigned long usecs);
>  void mdelay(unsigned long msecs);

-- 
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 |