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

["Jiaxun Yang" <jiaxun.yang@flygoat.com>]

在2025年1月3日一月 上午2:36，Huacai Chen写道：
> 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.

That’s the one from the manual :-( I’d agree with you though.

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

Will do in v2.


Thanks

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

-- 
- Jiaxun