RE: [PATCH] clocksource/drivers/exynos_mct: Remove mct interrupt index enum

["Alim Akhtar" <alim.akhtar@samsung.com>]

>-----Original Message-----
>From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com]
>Sent: Sunday, February 20, 2022 3:32 PM
>To: Alim Akhtar <alim.akhtar@samsung.com>; linux-arm-
>kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>Cc: linux-samsung-soc@vger.kernel.org; daniel.lezcano@linaro.org;
>tglx@linutronix.de; pankaj.dubey@samsung.com;
>m.szyprowski@samsung.com
>Subject: Re: [PATCH] clocksource/drivers/exynos_mct: Remove mct interrupt
>index enum
>
>On 19/02/2022 19:10, Alim Akhtar wrote:
>> MCT driver define an enum which list global and local timer's irq
>> index. Most of them are not used but MCT_G0_IRQ and MCT_L0_IRQ and
>> these two are at a fixed offset/index.
>> Get rid of this enum and use a #define for the used irq index.
>>
>> While at it, bump-up maximum number of MCT IRQ to match the binding
>> documentation. And also change the name variable to be more generic.
>>
>> No functional changes expected.
>
>There is a functional change - you increase MCT_NR_IRQS from 12 to 20 which
>affects size of mct_irqs. Can you increase it in separate commit?
>
Yes, my thought was it is going to increase the mct_irqs array size and it will not affect any 
of the current SoC's mct functionality.
Anyway, I will separate it out as you suggested.

>>
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> ---
>>  drivers/clocksource/exynos_mct.c | 25 ++++++++-----------------
>>  1 file changed, 8 insertions(+), 17 deletions(-)
>>
>> - currently tested on exynos7 platform, appreciate testing on
>> exynos-{3,4,5} platforms
>>
>> diff --git a/drivers/clocksource/exynos_mct.c
>> b/drivers/clocksource/exynos_mct.c
>> index 6db3d5511b0f..4aea9cd3f7ba 100644
>> --- a/drivers/clocksource/exynos_mct.c
>> +++ b/drivers/clocksource/exynos_mct.c
>> @@ -60,27 +60,18 @@
>>  #define MCT_CLKEVENTS_RATING		350
>>  #endif
>>
>> +/* There are four Global timers starting with 0 offset */
>> +#define MCT_G0_IRQ	0
>> +/* Local timers count starts after global timer count */
>> +#define MCT_L0_IRQ	4
>> +/* Max number of MCT IRQ as per binding document */
>> +#define MCT_NR_IRQS	20
>> +
>>  enum {
>>  	MCT_INT_SPI,
>>  	MCT_INT_PPI
>>  };
>>
>> -enum {
>> -	MCT_G0_IRQ,
>> -	MCT_G1_IRQ,
>> -	MCT_G2_IRQ,
>> -	MCT_G3_IRQ,
>> -	MCT_L0_IRQ,
>> -	MCT_L1_IRQ,
>> -	MCT_L2_IRQ,
>> -	MCT_L3_IRQ,
>> -	MCT_L4_IRQ,
>> -	MCT_L5_IRQ,
>> -	MCT_L6_IRQ,
>> -	MCT_L7_IRQ,
>> -	MCT_NR_IRQS,
>> -};
>> -
>>  static void __iomem *reg_base;
>>  static unsigned long clk_rate;
>>  static unsigned int mct_int_type;
>> @@ -89,7 +80,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_IRQS - 1];
>
>This does not look related MCT_NR_IRQS and using here MCT_NR_IRQS
>confuses. This is a "mct_tick%d" with number of local timers, so maybe make
>it just 11?
>
Yes, it is for local timer, let me add separate macro for this, which matches the current dt binding.
As per binding max 16 local timers can be supported. 
And it will make code scalable, which is the positive side effect of this change.

>>  };
>>
>>  static void exynos4_mct_write(unsigned int value, unsigned long
>> offset)
>
>
>Best regards,
>Krzysztof