RE: [PATCH v2 3/3] clocksource/drivers/exynos_mct: bump up number of local timer

From: Alim Akhtar
Date: Sun Feb 20 2022 - 12:00:09 EST




>-----Original Message-----
>From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@xxxxxxxxxxxxx]
>Sent: Sunday, February 20, 2022 9:03 PM
>To: Alim Akhtar <alim.akhtar@xxxxxxxxxxx>; linux-arm-
>kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
>Cc: linux-samsung-soc@xxxxxxxxxxxxxxx; daniel.lezcano@xxxxxxxxxx;
>tglx@xxxxxxxxxxxxx; pankaj.dubey@xxxxxxxxxxx;
>m.szyprowski@xxxxxxxxxxx
>Subject: Re: [PATCH v2 3/3] clocksource/drivers/exynos_mct: bump up
>number of local timer
>
>On 20/02/2022 14:38, Alim Akhtar wrote:
>> As per the dt binding, maximum number of local timer can be up to 16.
>> Increase the array size of the _name_ variable which holds the number
>> of local timer name per CPU to reflect the binding. While at it,
>> change the magic number to a meaningful macro.
>
>This still does not make sense. Let's say you have 16 local timers, so why the
>name of clock event device should be maximum 16? How are these related?
>
As you rightly commented on v1 and it is mainly for "mct_tick%d" with number of local timers and
local timer is per cpu. With increase in cpu number, name[10] restrict the scalability.
So either we can change to 11, or use some reference (as local timer number in this patch).
Let me know your thought on this.

>>
>> Signed-off-by: Alim Akhtar <alim.akhtar@xxxxxxxxxxx>
>> ---
>> drivers/clocksource/exynos_mct.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clocksource/exynos_mct.c
>> b/drivers/clocksource/exynos_mct.c
>> index 0c7931f7f99a..8d63a9376701 100644
>> --- a/drivers/clocksource/exynos_mct.c
>> +++ b/drivers/clocksource/exynos_mct.c
>> @@ -66,6 +66,8 @@
>> #define MCT_L0_IRQ 4
>> /* Max number of IRQ as per DT binding document */
>> #define MCT_NR_IRQS 20
>> +/* Max number of local timers */
>> +#define MCT_NR_LOCAL_TIMERS 16
>>
>> enum {
>> MCT_INT_SPI,
>> @@ -80,7 +82,7 @@ static int mct_irqs[MCT_NR_IRQS]; struct
>> mct_clock_event_device {
>> struct clock_event_device evt;
>> unsigned long base;
>> - char name[10];
>> + char name[MCT_NR_LOCAL_TIMERS];
>> };
>>
>> static void exynos4_mct_write(unsigned int value, unsigned long
>> offset)
>
>
>Best regards,
>Krzysztof