[v1] loongarch: time: Derive max_delta from prcfg1 tmrbits

[PATCH] loongarch: time: Derive max_delta from prcfg1 tmrbits

Posted by Jiaxun Yang 1 year, 1 month ago

As per arch spec, maximum timer bits is configurable and
should not be hardcoded in any way.

Use prcfg1 tmrbits to determine clockevent's max_delta
to be conformance.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
LA32R doesn't have prcfg, maybe we should handle this
during cpu-probe and save it in cpuinfo_loongarch,

What's your opinion, Huacai?
---
 arch/loongarch/include/asm/loongarch.h | 1 -
 arch/loongarch/kernel/time.c           | 7 ++++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/loongarch/include/asm/loongarch.h b/arch/loongarch/include/asm/loongarch.h
index 64ad277e096edd7d77af6f37e234d68e571764a4..a3cc4f8d4c4a0b0521ef2b76c87fa57eca417d75 100644
--- a/arch/loongarch/include/asm/loongarch.h
+++ b/arch/loongarch/include/asm/loongarch.h
@@ -466,7 +466,6 @@
 
 #define LOONGARCH_CSR_TCFG		0x41	/* Timer config */
 #define  CSR_TCFG_VAL_SHIFT		2
-#define	 CSR_TCFG_VAL_WIDTH		48
 #define  CSR_TCFG_VAL			(_ULCAST_(0x3fffffffffff) << CSR_TCFG_VAL_SHIFT)
 #define  CSR_TCFG_PERIOD_SHIFT		1
 #define  CSR_TCFG_PERIOD		(_ULCAST_(0x1) << CSR_TCFG_PERIOD_SHIFT)
diff --git a/arch/loongarch/kernel/time.c b/arch/loongarch/kernel/time.c
index a07d7eff4dc5fb0f41f5fad02698ff7a84c7bc8a..bde2297f139c2d5e26894a2ebc1748cc6fef3146 100644
--- a/arch/loongarch/kernel/time.c
+++ b/arch/loongarch/kernel/time.c
@@ -126,16 +126,21 @@ void sync_counter(void)
 
 int constant_clockevent_init(void)
 {
+	uint32_t tmrbits;
 	unsigned int cpu = smp_processor_id();
 #ifdef CONFIG_PREEMPT_RT
 	unsigned long min_delta = 100;
 #else
 	unsigned long min_delta = 1000;
 #endif
-	unsigned long max_delta = (1UL << 48) - 1;
+	unsigned long max_delta;
 	struct clock_event_device *cd;
 	static int irq = 0, timer_irq_installed = 0;
 
+	tmrbits = read_csr_prcfg1() & CSR_CONF1_TMRBITS;
+	tmrbits = tmrbits >> CSR_CONF1_TMRBITS_SHIFT;
+	max_delta = CLOCKSOURCE_MASK(tmrbits + 1);
+
 	if (!timer_irq_installed) {
 		irq = get_percpu_irq(INT_TI);
 		if (irq < 0)

---
base-commit: 8155b4ef3466f0e289e8fcc9e6e62f3f4dceeac2
change-id: 20241223-la-misc-d610d14f947e

Best regards,
-- 
Jiaxun Yang <jiaxun.yang@flygoat.com>

Re: [PATCH] loongarch: time: Derive max_delta from prcfg1 tmrbits

Posted by Huacai Chen 1 year, 1 month ago

Hi, Jiaxun,

On Thu, Jan 2, 2025 at 10:05 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>
> As per arch spec, maximum timer bits is configurable and
> should not be hardcoded in any way.
>
> Use prcfg1 tmrbits to determine clockevent's max_delta
> to be conformance.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> LA32R doesn't have prcfg, maybe we should handle this
> during cpu-probe and save it in cpuinfo_loongarch,
>
> What's your opinion, Huacai?
> ---
>  arch/loongarch/include/asm/loongarch.h | 1 -
>  arch/loongarch/kernel/time.c           | 7 ++++++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/loongarch.h b/arch/loongarch/include/asm/loongarch.h
> index 64ad277e096edd7d77af6f37e234d68e571764a4..a3cc4f8d4c4a0b0521ef2b76c87fa57eca417d75 100644
> --- a/arch/loongarch/include/asm/loongarch.h
> +++ b/arch/loongarch/include/asm/loongarch.h
> @@ -466,7 +466,6 @@
>
>  #define LOONGARCH_CSR_TCFG             0x41    /* Timer config */
>  #define  CSR_TCFG_VAL_SHIFT            2
> -#define         CSR_TCFG_VAL_WIDTH             48
>  #define  CSR_TCFG_VAL                  (_ULCAST_(0x3fffffffffff) << CSR_TCFG_VAL_SHIFT)
>  #define  CSR_TCFG_PERIOD_SHIFT         1
>  #define  CSR_TCFG_PERIOD               (_ULCAST_(0x1) << CSR_TCFG_PERIOD_SHIFT)
> diff --git a/arch/loongarch/kernel/time.c b/arch/loongarch/kernel/time.c
> index a07d7eff4dc5fb0f41f5fad02698ff7a84c7bc8a..bde2297f139c2d5e26894a2ebc1748cc6fef3146 100644
> --- a/arch/loongarch/kernel/time.c
> +++ b/arch/loongarch/kernel/time.c
> @@ -126,16 +126,21 @@ void sync_counter(void)
>
>  int constant_clockevent_init(void)
>  {
> +       uint32_t tmrbits;
tmr is a strange short name, you can use timer_bits or tbits for short.

>         unsigned int cpu = smp_processor_id();
>  #ifdef CONFIG_PREEMPT_RT
>         unsigned long min_delta = 100;
>  #else
>         unsigned long min_delta = 1000;
>  #endif
> -       unsigned long max_delta = (1UL << 48) - 1;
> +       unsigned long max_delta;
>         struct clock_event_device *cd;
>         static int irq = 0, timer_irq_installed = 0;
>
> +       tmrbits = read_csr_prcfg1() & CSR_CONF1_TMRBITS;
> +       tmrbits = tmrbits >> CSR_CONF1_TMRBITS_SHIFT;
> +       max_delta = CLOCKSOURCE_MASK(tmrbits + 1);
I prefer to evaluate the tbits and max_delta directly when declaring them.


Huacai
> +
>         if (!timer_irq_installed) {
>                 irq = get_percpu_irq(INT_TI);
>                 if (irq < 0)
>
> ---
> base-commit: 8155b4ef3466f0e289e8fcc9e6e62f3f4dceeac2
> change-id: 20241223-la-misc-d610d14f947e
>
> Best regards,
> --
> Jiaxun Yang <jiaxun.yang@flygoat.com>
>