[RESEND PATCH v3] clocksource: exynos_mct: Add the support for Exynos 64bit SoC

[RESEND PATCH v3] clocksource: exynos_mct: Add the support for Exynos 64bit SoC

[Chanwoo Choi]

This patch adds the support for Exynos 64bit SoC. The delay_timer is only used
for Exynos 32bit SoC.

Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
This patch set is tested on 64-bit Exynos SoC. I send only this patch from
following patchst[1].
[1] https://lkml.org/lkml/2014/12/2/134

Changes from v2:
- None
Changes from v1:
- Use CONFIG_ARM instead of CONFIG_ARM64

 drivers/clocksource/Kconfig      | 1 -
 drivers/clocksource/exynos_mct.c | 4 ++++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index fc01ec2..be38119 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -135,7 +135,6 @@ config CLKSRC_METAG_GENERIC
 
 config CLKSRC_EXYNOS_MCT
 	def_bool y if ARCH_EXYNOS
-	depends on !ARM64
 	help
 	  Support for Multi Core Timer controller on Exynos SoCs.
 
diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 9403061..b840ea1 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -223,6 +223,7 @@ static u64 notrace exynos4_read_sched_clock(void)
 	return exynos4_read_count_32();
 }
 
+#if defined(CONFIG_ARM)
 static struct delay_timer exynos4_delay_timer;
 
 static cycles_t exynos4_read_current_timer(void)
@@ -231,14 +232,17 @@ static cycles_t exynos4_read_current_timer(void)
 			 "cycles_t needs to move to 32-bit for ARM64 usage");
 	return exynos4_read_count_32();
 }
+#endif
 
 static void __init exynos4_clocksource_init(void)
 {
 	exynos4_mct_frc_start();
 
+#if defined(CONFIG_ARM)
 	exynos4_delay_timer.read_current_timer = &exynos4_read_current_timer;
 	exynos4_delay_timer.freq = clk_rate;
 	register_current_timer_delay(&exynos4_delay_timer);
+#endif
 
 	if (clocksource_register_hz(&mct_frc, clk_rate))
 		panic("%s: can't register clocksource\n", mct_frc.name);
-- 
1.8.5.5

Re: [RESEND PATCH v3] clocksource: exynos_mct: Add the support for Exynos 64bit SoC

[Kukjin Kim]

On 01/14/15 14:33, Chanwoo Choi wrote:

Hi,

+ Doug, Olof

> This patch adds the support for Exynos 64bit SoC. The delay_timer is only used
> for Exynos 32bit SoC.
> 
Yes, the Exynos MCT(Multi-Core Timer) is 64bit timer and it is available
on 64bit exynos SoC such as exynos7. But basically ARMv8 architecture is
including ARM ARCH timer (ARM Generic Timer) and exynos7 also has
implemented it and additionally its access is faster than using memory
mapped register called SFR for MCT...so Doug submitted patch to use MCT
on 32bit exynos SoCs before.

I know using MCT on 64bit exynos is usefulness for Power Management and
I need to talk to relevant guys in office again. If anything, I'll let
you know.

Thanks,
Kukjin

> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Kukjin Kim <kgene.kim@samsung.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
> This patch set is tested on 64-bit Exynos SoC. I send only this patch from
> following patchst[1].
> [1] https://lkml.org/lkml/2014/12/2/134
> 
> Changes from v2:
> - None
> Changes from v1:
> - Use CONFIG_ARM instead of CONFIG_ARM64
> 
>  drivers/clocksource/Kconfig      | 1 -
>  drivers/clocksource/exynos_mct.c | 4 ++++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index fc01ec2..be38119 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -135,7 +135,6 @@ config CLKSRC_METAG_GENERIC
>  
>  config CLKSRC_EXYNOS_MCT
>  	def_bool y if ARCH_EXYNOS
> -	depends on !ARM64
>  	help
>  	  Support for Multi Core Timer controller on Exynos SoCs.
>  
> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
> index 9403061..b840ea1 100644
> --- a/drivers/clocksource/exynos_mct.c
> +++ b/drivers/clocksource/exynos_mct.c
> @@ -223,6 +223,7 @@ static u64 notrace exynos4_read_sched_clock(void)
>  	return exynos4_read_count_32();
>  }
>  
> +#if defined(CONFIG_ARM)
>  static struct delay_timer exynos4_delay_timer;
>  
>  static cycles_t exynos4_read_current_timer(void)
> @@ -231,14 +232,17 @@ static cycles_t exynos4_read_current_timer(void)
>  			 "cycles_t needs to move to 32-bit for ARM64 usage");
>  	return exynos4_read_count_32();
>  }
> +#endif
>  
>  static void __init exynos4_clocksource_init(void)
>  {
>  	exynos4_mct_frc_start();
>  
> +#if defined(CONFIG_ARM)
>  	exynos4_delay_timer.read_current_timer = &exynos4_read_current_timer;
>  	exynos4_delay_timer.freq = clk_rate;
>  	register_current_timer_delay(&exynos4_delay_timer);
> +#endif
>  
>  	if (clocksource_register_hz(&mct_frc, clk_rate))
>  		panic("%s: can't register clocksource\n", mct_frc.name);

Re: [RESEND PATCH v3] clocksource: exynos_mct: Add the support for Exynos 64bit SoC

[Daniel Lezcano]

On 01/14/2015 04:51 PM, Kukjin Kim wrote:
> On 01/14/15 14:33, Chanwoo Choi wrote:
>
> Hi,
>
> + Doug, Olof
>
>> This patch adds the support for Exynos 64bit SoC. The delay_timer is only used
>> for Exynos 32bit SoC.
>>
> Yes, the Exynos MCT(Multi-Core Timer) is 64bit timer and it is available
> on 64bit exynos SoC such as exynos7. But basically ARMv8 architecture is
> including ARM ARCH timer (ARM Generic Timer) and exynos7 also has
> implemented it and additionally its access is faster than using memory
> mapped register called SFR for MCT...so Doug submitted patch to use MCT
> on 32bit exynos SoCs before.
>
> I know using MCT on 64bit exynos is usefulness for Power Management and
> I need to talk to relevant guys in office again. If anything, I'll let
> you know.

I will wait for your answer before digging more the patch.

Thanks
   -- Daniel

>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Kukjin Kim <kgene.kim@samsung.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>> This patch set is tested on 64-bit Exynos SoC. I send only this patch from
>> following patchst[1].
>> [1] https://lkml.org/lkml/2014/12/2/134
>>
>> Changes from v2:
>> - None
>> Changes from v1:
>> - Use CONFIG_ARM instead of CONFIG_ARM64
>>
>>   drivers/clocksource/Kconfig      | 1 -
>>   drivers/clocksource/exynos_mct.c | 4 ++++
>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index fc01ec2..be38119 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -135,7 +135,6 @@ config CLKSRC_METAG_GENERIC
>>
>>   config CLKSRC_EXYNOS_MCT
>>   	def_bool y if ARCH_EXYNOS
>> -	depends on !ARM64
>>   	help
>>   	  Support for Multi Core Timer controller on Exynos SoCs.
>>
>> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
>> index 9403061..b840ea1 100644
>> --- a/drivers/clocksource/exynos_mct.c
>> +++ b/drivers/clocksource/exynos_mct.c
>> @@ -223,6 +223,7 @@ static u64 notrace exynos4_read_sched_clock(void)
>>   	return exynos4_read_count_32();
>>   }
>>
>> +#if defined(CONFIG_ARM)
>>   static struct delay_timer exynos4_delay_timer;
>>
>>   static cycles_t exynos4_read_current_timer(void)
>> @@ -231,14 +232,17 @@ static cycles_t exynos4_read_current_timer(void)
>>   			 "cycles_t needs to move to 32-bit for ARM64 usage");
>>   	return exynos4_read_count_32();
>>   }
>> +#endif
>>
>>   static void __init exynos4_clocksource_init(void)
>>   {
>>   	exynos4_mct_frc_start();
>>
>> +#if defined(CONFIG_ARM)
>>   	exynos4_delay_timer.read_current_timer = &exynos4_read_current_timer;
>>   	exynos4_delay_timer.freq = clk_rate;
>>   	register_current_timer_delay(&exynos4_delay_timer);
>> +#endif
>>
>>   	if (clocksource_register_hz(&mct_frc, clk_rate))
>>   		panic("%s: can't register clocksource\n", mct_frc.name);


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [RESEND PATCH v3] clocksource: exynos_mct: Add the support for Exynos 64bit SoC

[Chanwoo Choi]

Hi Kukjin,

On 01/15/2015 01:02 AM, Daniel Lezcano wrote:
> On 01/14/2015 04:51 PM, Kukjin Kim wrote:
>> On 01/14/15 14:33, Chanwoo Choi wrote:
>>
>> Hi,
>>
>> + Doug, Olof
>>
>>> This patch adds the support for Exynos 64bit SoC. The delay_timer is only used
>>> for Exynos 32bit SoC.
>>>
>> Yes, the Exynos MCT(Multi-Core Timer) is 64bit timer and it is available
>> on 64bit exynos SoC such as exynos7. But basically ARMv8 architecture is
>> including ARM ARCH timer (ARM Generic Timer) and exynos7 also has
>> implemented it and additionally its access is faster than using memory
>> mapped register called SFR for MCT...so Doug submitted patch to use MCT
>> on 32bit exynos SoCs before.

I know arch_timer. As you comment, ARCH timer would be used for system timer for ARMv8.
But, Exynos5433/Exynos7 (ARMv8) include MCT (Multi-Core Timer) IP. I checked it on
Exynos5433/EXynos7 User-manaual and tested it.

I think that exynos_mct.c should support the Exynos 64-bit SoC
because Exynos5433/Exynos7 include already MCT (Multi-Core Timer) IP.

Also, I have a problem to verify ARCH timer on Exynos SoC. Exynos User-manual never includes
the detailed information about for ARCH timer(e.g, clock for ARCH timer). I knew that
I can get the document of ARCH timer for ARM official site but I think it is insufficient
to implement ARCH timer on Exynos SoC. 

Best Regards,
Chanwoo Choi

>>
>> I know using MCT on 64bit exynos is usefulness for Power Management and
>> I need to talk to relevant guys in office again. If anything, I'll let
>> you know.
> 
> I will wait for your answer before digging more the patch.
> 
> Thanks
>   -- Daniel
> 
>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Kukjin Kim <kgene.kim@samsung.com>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>> ---
>>> This patch set is tested on 64-bit Exynos SoC. I send only this patch from
>>> following patchst[1].
>>> [1] https://lkml.org/lkml/2014/12/2/134
>>>
>>> Changes from v2:
>>> - None
>>> Changes from v1:
>>> - Use CONFIG_ARM instead of CONFIG_ARM64
>>>
>>>   drivers/clocksource/Kconfig      | 1 -
>>>   drivers/clocksource/exynos_mct.c | 4 ++++
>>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>> index fc01ec2..be38119 100644
>>> --- a/drivers/clocksource/Kconfig
>>> +++ b/drivers/clocksource/Kconfig
>>> @@ -135,7 +135,6 @@ config CLKSRC_METAG_GENERIC
>>>
>>>   config CLKSRC_EXYNOS_MCT
>>>       def_bool y if ARCH_EXYNOS
>>> -    depends on !ARM64
>>>       help
>>>         Support for Multi Core Timer controller on Exynos SoCs.
>>>
>>> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
>>> index 9403061..b840ea1 100644
>>> --- a/drivers/clocksource/exynos_mct.c
>>> +++ b/drivers/clocksource/exynos_mct.c
>>> @@ -223,6 +223,7 @@ static u64 notrace exynos4_read_sched_clock(void)
>>>       return exynos4_read_count_32();
>>>   }
>>>
>>> +#if defined(CONFIG_ARM)
>>>   static struct delay_timer exynos4_delay_timer;
>>>
>>>   static cycles_t exynos4_read_current_timer(void)
>>> @@ -231,14 +232,17 @@ static cycles_t exynos4_read_current_timer(void)
>>>                "cycles_t needs to move to 32-bit for ARM64 usage");
>>>       return exynos4_read_count_32();
>>>   }
>>> +#endif
>>>
>>>   static void __init exynos4_clocksource_init(void)
>>>   {
>>>       exynos4_mct_frc_start();
>>>
>>> +#if defined(CONFIG_ARM)
>>>       exynos4_delay_timer.read_current_timer = &exynos4_read_current_timer;
>>>       exynos4_delay_timer.freq = clk_rate;
>>>       register_current_timer_delay(&exynos4_delay_timer);
>>> +#endif
>>>
>>>       if (clocksource_register_hz(&mct_frc, clk_rate))
>>>           panic("%s: can't register clocksource\n", mct_frc.name);
> 
> 

Re: [RESEND PATCH v3] clocksource: exynos_mct: Add the support for Exynos 64bit SoC

[Mark Rutland]

On Wed, Jan 14, 2015 at 11:57:00PM +0000, Chanwoo Choi wrote:
> Hi Kukjin,
> 
> On 01/15/2015 01:02 AM, Daniel Lezcano wrote:
> > On 01/14/2015 04:51 PM, Kukjin Kim wrote:
> >> On 01/14/15 14:33, Chanwoo Choi wrote:
> >>
> >> Hi,
> >>
> >> + Doug, Olof
> >>
> >>> This patch adds the support for Exynos 64bit SoC. The delay_timer is only used
> >>> for Exynos 32bit SoC.
> >>>
> >> Yes, the Exynos MCT(Multi-Core Timer) is 64bit timer and it is available
> >> on 64bit exynos SoC such as exynos7. But basically ARMv8 architecture is
> >> including ARM ARCH timer (ARM Generic Timer) and exynos7 also has
> >> implemented it and additionally its access is faster than using memory
> >> mapped register called SFR for MCT...so Doug submitted patch to use MCT
> >> on 32bit exynos SoCs before.
> 
> I know arch_timer. As you comment, ARCH timer would be used for system timer for ARMv8.
> But, Exynos5433/Exynos7 (ARMv8) include MCT (Multi-Core Timer) IP. I checked it on
> Exynos5433/EXynos7 User-manaual and tested it.
> 
> I think that exynos_mct.c should support the Exynos 64-bit SoC
> because Exynos5433/Exynos7 include already MCT (Multi-Core Timer) IP.
> 
> Also, I have a problem to verify ARCH timer on Exynos SoC. Exynos User-manual never includes
> the detailed information about for ARCH timer(e.g, clock for ARCH timer). I knew that
> I can get the document of ARCH timer for ARM official site but I think it is insufficient
> to implement ARCH timer on Exynos SoC. 

What do you mean by "insufficient to implement ARCH timer"?

The architected timer is mandatory in ARMv8, and required by the arm64
kernel.

Additional timers may be requried if you want to put all CPUs into low
power states where the timer logic may be disabled and/or lose state,
but regardless the architected timers are necessary.

Thanks,
Mark.

Re: [RESEND PATCH v3] clocksource: exynos_mct: Add the support for Exynos 64bit SoC

[Chanwoo Choi]

Hi Mark,

On Thu, Jan 15, 2015 at 8:29 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Jan 14, 2015 at 11:57:00PM +0000, Chanwoo Choi wrote:
>> Hi Kukjin,
>>
>> On 01/15/2015 01:02 AM, Daniel Lezcano wrote:
>> > On 01/14/2015 04:51 PM, Kukjin Kim wrote:
>> >> On 01/14/15 14:33, Chanwoo Choi wrote:
>> >>
>> >> Hi,
>> >>
>> >> + Doug, Olof
>> >>
>> >>> This patch adds the support for Exynos 64bit SoC. The delay_timer is only used
>> >>> for Exynos 32bit SoC.
>> >>>
>> >> Yes, the Exynos MCT(Multi-Core Timer) is 64bit timer and it is available
>> >> on 64bit exynos SoC such as exynos7. But basically ARMv8 architecture is
>> >> including ARM ARCH timer (ARM Generic Timer) and exynos7 also has
>> >> implemented it and additionally its access is faster than using memory
>> >> mapped register called SFR for MCT...so Doug submitted patch to use MCT
>> >> on 32bit exynos SoCs before.
>>
>> I know arch_timer. As you comment, ARCH timer would be used for system timer for ARMv8.
>> But, Exynos5433/Exynos7 (ARMv8) include MCT (Multi-Core Timer) IP. I checked it on
>> Exynos5433/EXynos7 User-manaual and tested it.
>>
>> I think that exynos_mct.c should support the Exynos 64-bit SoC
>> because Exynos5433/Exynos7 include already MCT (Multi-Core Timer) IP.
>>
>> Also, I have a problem to verify ARCH timer on Exynos SoC. Exynos User-manual never includes
>> the detailed information about for ARCH timer(e.g, clock for ARCH timer). I knew that
>> I can get the document of ARCH timer for ARM official site but I think it is insufficient
>> to implement ARCH timer on Exynos SoC.
>
> What do you mean by "insufficient to implement ARCH timer"?

As I knew, timer must need the source clock. The clock for ARCH timer
has dependency on Exynos SoC, But I cannot find

>
> The architected timer is mandatory in ARMv8, and required by the arm64
> kernel.
>
> Additional timers may be requried if you want to put all CPUs into low
> power states where the timer logic may be disabled and/or lose state,
> but regardless the architected timers are necessary.
>
> Thanks,
> Mark.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RESEND PATCH v3] clocksource: exynos_mct: Add the support for Exynos 64bit SoC

[Chanwoo Choi]

On Thu, Jan 15, 2015 at 9:46 PM, Chanwoo Choi <cwchoi00@gmail.com> wrote:
> Hi Mark,
>
> On Thu, Jan 15, 2015 at 8:29 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>> On Wed, Jan 14, 2015 at 11:57:00PM +0000, Chanwoo Choi wrote:
>>> Hi Kukjin,
>>>
>>> On 01/15/2015 01:02 AM, Daniel Lezcano wrote:
>>> > On 01/14/2015 04:51 PM, Kukjin Kim wrote:
>>> >> On 01/14/15 14:33, Chanwoo Choi wrote:
>>> >>
>>> >> Hi,
>>> >>
>>> >> + Doug, Olof
>>> >>
>>> >>> This patch adds the support for Exynos 64bit SoC. The delay_timer is only used
>>> >>> for Exynos 32bit SoC.
>>> >>>
>>> >> Yes, the Exynos MCT(Multi-Core Timer) is 64bit timer and it is available
>>> >> on 64bit exynos SoC such as exynos7. But basically ARMv8 architecture is
>>> >> including ARM ARCH timer (ARM Generic Timer) and exynos7 also has
>>> >> implemented it and additionally its access is faster than using memory
>>> >> mapped register called SFR for MCT...so Doug submitted patch to use MCT
>>> >> on 32bit exynos SoCs before.
>>>
>>> I know arch_timer. As you comment, ARCH timer would be used for system timer for ARMv8.
>>> But, Exynos5433/Exynos7 (ARMv8) include MCT (Multi-Core Timer) IP. I checked it on
>>> Exynos5433/EXynos7 User-manaual and tested it.
>>>
>>> I think that exynos_mct.c should support the Exynos 64-bit SoC
>>> because Exynos5433/Exynos7 include already MCT (Multi-Core Timer) IP.
>>>
>>> Also, I have a problem to verify ARCH timer on Exynos SoC. Exynos User-manual never includes
>>> the detailed information about for ARCH timer(e.g, clock for ARCH timer). I knew that
>>> I can get the document of ARCH timer for ARM official site but I think it is insufficient
>>> to implement ARCH timer on Exynos SoC.
>>
>> What do you mean by "insufficient to implement ARCH timer"?
>
> As I knew, timer must need the source clock. The clock for ARCH timer
> has dependency on Exynos SoC, But I cannot find

I'm so sorry about this mistake. I pressed the send button before
completing reply.

As I knew, timer must need the source clock. The clock for ARCH timer
has dependency on Exynos SoC, But I cannot find the clock information
for ARCH timer on Exynos SoC user-manual.

When I tried to use ARCH timer on Exynos3250, It is not working. We
can check this ARCH timer issue of Exynos3250
on following patch[1]:
[1] http://www.spinics.net/lists/arm-kernel/msg322943.html

>
>>
>> The architected timer is mandatory in ARMv8, and required by the arm64
>> kernel.
>>
>> Additional timers may be requried if you want to put all CPUs into low
>> power states where the timer logic may be disabled and/or lose state,
>> but regardless the architected timers are necessary.

I agree that ARCH timer is mandatory.

I just think that existing exynos-mct.c driver should support the Exynos5/7 SoC
because released Exynos5/7 SoC includes already MCT IP for system timer.

Best Regards,
Chanwoo Choi

Re: [RESEND PATCH v3] clocksource: exynos_mct: Add the support for Exynos 64bit SoC

[Mark Rutland]

On Thu, Jan 15, 2015 at 12:52:38PM +0000, Chanwoo Choi wrote:
> On Thu, Jan 15, 2015 at 9:46 PM, Chanwoo Choi <cwchoi00@gmail.com> wrote:
> > Hi Mark,
> >
> > On Thu, Jan 15, 2015 at 8:29 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> >> On Wed, Jan 14, 2015 at 11:57:00PM +0000, Chanwoo Choi wrote:
> >>> Hi Kukjin,
> >>>
> >>> On 01/15/2015 01:02 AM, Daniel Lezcano wrote:
> >>> > On 01/14/2015 04:51 PM, Kukjin Kim wrote:
> >>> >> On 01/14/15 14:33, Chanwoo Choi wrote:
> >>> >>
> >>> >> Hi,
> >>> >>
> >>> >> + Doug, Olof
> >>> >>
> >>> >>> This patch adds the support for Exynos 64bit SoC. The delay_timer is only used
> >>> >>> for Exynos 32bit SoC.
> >>> >>>
> >>> >> Yes, the Exynos MCT(Multi-Core Timer) is 64bit timer and it is available
> >>> >> on 64bit exynos SoC such as exynos7. But basically ARMv8 architecture is
> >>> >> including ARM ARCH timer (ARM Generic Timer) and exynos7 also has
> >>> >> implemented it and additionally its access is faster than using memory
> >>> >> mapped register called SFR for MCT...so Doug submitted patch to use MCT
> >>> >> on 32bit exynos SoCs before.
> >>>
> >>> I know arch_timer. As you comment, ARCH timer would be used for system timer for ARMv8.
> >>> But, Exynos5433/Exynos7 (ARMv8) include MCT (Multi-Core Timer) IP. I checked it on
> >>> Exynos5433/EXynos7 User-manaual and tested it.
> >>>
> >>> I think that exynos_mct.c should support the Exynos 64-bit SoC
> >>> because Exynos5433/Exynos7 include already MCT (Multi-Core Timer) IP.
> >>>
> >>> Also, I have a problem to verify ARCH timer on Exynos SoC. Exynos User-manual never includes
> >>> the detailed information about for ARCH timer(e.g, clock for ARCH timer). I knew that
> >>> I can get the document of ARCH timer for ARM official site but I think it is insufficient
> >>> to implement ARCH timer on Exynos SoC.
> >>
> >> What do you mean by "insufficient to implement ARCH timer"?
> >
> > As I knew, timer must need the source clock. The clock for ARCH timer
> > has dependency on Exynos SoC, But I cannot find
> 
> I'm so sorry about this mistake. I pressed the send button before
> completing reply.
> 
> As I knew, timer must need the source clock. The clock for ARCH timer
> has dependency on Exynos SoC, But I cannot find the clock information
> for ARCH timer on Exynos SoC user-manual.
> 
> When I tried to use ARCH timer on Exynos3250, It is not working. We
> can check this ARCH timer issue of Exynos3250
> on following patch[1]:
> [1] http://www.spinics.net/lists/arm-kernel/msg322943.html

Hmm. That is annoying. Your boot code should have been initialising this
already.

> >> The architected timer is mandatory in ARMv8, and required by the arm64
> >> kernel.
> >>
> >> Additional timers may be requried if you want to put all CPUs into low
> >> power states where the timer logic may be disabled and/or lose state,
> >> but regardless the architected timers are necessary.
> 
> I agree that ARCH timer is mandatory.
> 
> I just think that existing exynos-mct.c driver should support the Exynos5/7 SoC
> because released Exynos5/7 SoC includes already MCT IP for system timer.

I'm not opposed to the MCT. My only concern is that a configured and
enabled architected timer is mandated by the boot protocol, and is a
prerequisite for a functioning kernel. Your initial response made it
sound like you expected the MCT alone to be sufficient.

Thanks,
Mark.

Re: [RESEND PATCH v3] clocksource: exynos_mct: Add the support for Exynos 64bit SoC

[Chanwoo Choi]

On 01/15/2015 10:34 PM, Mark Rutland wrote:
> On Thu, Jan 15, 2015 at 12:52:38PM +0000, Chanwoo Choi wrote:
>> On Thu, Jan 15, 2015 at 9:46 PM, Chanwoo Choi <cwchoi00@gmail.com> wrote:
>>> Hi Mark,
>>>
>>> On Thu, Jan 15, 2015 at 8:29 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>>>> On Wed, Jan 14, 2015 at 11:57:00PM +0000, Chanwoo Choi wrote:
>>>>> Hi Kukjin,
>>>>>
>>>>> On 01/15/2015 01:02 AM, Daniel Lezcano wrote:
>>>>>> On 01/14/2015 04:51 PM, Kukjin Kim wrote:
>>>>>>> On 01/14/15 14:33, Chanwoo Choi wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> + Doug, Olof
>>>>>>>
>>>>>>>> This patch adds the support for Exynos 64bit SoC. The delay_timer is only used
>>>>>>>> for Exynos 32bit SoC.
>>>>>>>>
>>>>>>> Yes, the Exynos MCT(Multi-Core Timer) is 64bit timer and it is available
>>>>>>> on 64bit exynos SoC such as exynos7. But basically ARMv8 architecture is
>>>>>>> including ARM ARCH timer (ARM Generic Timer) and exynos7 also has
>>>>>>> implemented it and additionally its access is faster than using memory
>>>>>>> mapped register called SFR for MCT...so Doug submitted patch to use MCT
>>>>>>> on 32bit exynos SoCs before.
>>>>>
>>>>> I know arch_timer. As you comment, ARCH timer would be used for system timer for ARMv8.
>>>>> But, Exynos5433/Exynos7 (ARMv8) include MCT (Multi-Core Timer) IP. I checked it on
>>>>> Exynos5433/EXynos7 User-manaual and tested it.
>>>>>
>>>>> I think that exynos_mct.c should support the Exynos 64-bit SoC
>>>>> because Exynos5433/Exynos7 include already MCT (Multi-Core Timer) IP.
>>>>>
>>>>> Also, I have a problem to verify ARCH timer on Exynos SoC. Exynos User-manual never includes
>>>>> the detailed information about for ARCH timer(e.g, clock for ARCH timer). I knew that
>>>>> I can get the document of ARCH timer for ARM official site but I think it is insufficient
>>>>> to implement ARCH timer on Exynos SoC.
>>>>
>>>> What do you mean by "insufficient to implement ARCH timer"?
>>>
>>> As I knew, timer must need the source clock. The clock for ARCH timer
>>> has dependency on Exynos SoC, But I cannot find
>>
>> I'm so sorry about this mistake. I pressed the send button before
>> completing reply.
>>
>> As I knew, timer must need the source clock. The clock for ARCH timer
>> has dependency on Exynos SoC, But I cannot find the clock information
>> for ARCH timer on Exynos SoC user-manual.
>>
>> When I tried to use ARCH timer on Exynos3250, It is not working. We
>> can check this ARCH timer issue of Exynos3250
>> on following patch[1]:
>> [1] http://www.spinics.net/lists/arm-kernel/msg322943.html
> 
> Hmm. That is annoying. Your boot code should have been initialising this
> already.

Right, 
I want to resolve the issue for Exynos3250 but I don't have any information.
Maybe this issue should be fixed by the architector of Exynos SoC or any samsung
developer who can contact the information of ARCH timer.

> 
>>>> The architected timer is mandatory in ARMv8, and required by the arm64
>>>> kernel.
>>>>
>>>> Additional timers may be requried if you want to put all CPUs into low
>>>> power states where the timer logic may be disabled and/or lose state,
>>>> but regardless the architected timers are necessary.
>>
>> I agree that ARCH timer is mandatory.
>>
>> I just think that existing exynos-mct.c driver should support the Exynos5/7 SoC
>> because released Exynos5/7 SoC includes already MCT IP for system timer.
> 
> I'm not opposed to the MCT. My only concern is that a configured and
> enabled architected timer is mandated by the boot protocol, and is a
> prerequisite for a functioning kernel. 

Thanks for your opinion.
As I replyed on previous mail, I agree that ARCH timer is necessary.

Your initial response made it
> sound like you expected the MCT alone to be sufficient.

I didn't mean it.

Best Regards,
Chanwoo Choi

Re: [RESEND PATCH v3] clocksource: exynos_mct: Add the support for Exynos 64bit SoC

[Chanwoo Choi]

Dear Daniel and Kukjin,

On 01/15/2015 01:02 AM, Daniel Lezcano wrote:
> On 01/14/2015 04:51 PM, Kukjin Kim wrote:
>> On 01/14/15 14:33, Chanwoo Choi wrote:
>>
>> Hi,
>>
>> + Doug, Olof
>>
>>> This patch adds the support for Exynos 64bit SoC. The delay_timer is only used
>>> for Exynos 32bit SoC.
>>>
>> Yes, the Exynos MCT(Multi-Core Timer) is 64bit timer and it is available
>> on 64bit exynos SoC such as exynos7. But basically ARMv8 architecture is
>> including ARM ARCH timer (ARM Generic Timer) and exynos7 also has
>> implemented it and additionally its access is faster than using memory
>> mapped register called SFR for MCT...so Doug submitted patch to use MCT
>> on 32bit exynos SoCs before.
>>
>> I know using MCT on 64bit exynos is usefulness for Power Management and
>> I need to talk to relevant guys in office again. If anything, I'll let
>> you know.
> 
> I will wait for your answer before digging more the patch.
> 
> Thanks
>   -- Daniel
> 
>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Kukjin Kim <kgene.kim@samsung.com>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>> ---
>>> This patch set is tested on 64-bit Exynos SoC. I send only this patch from
>>> following patchst[1].
>>> [1] https://lkml.org/lkml/2014/12/2/134
>>>
>>> Changes from v2:
>>> - None
>>> Changes from v1:
>>> - Use CONFIG_ARM instead of CONFIG_ARM64
>>>
>>>   drivers/clocksource/Kconfig      | 1 -
>>>   drivers/clocksource/exynos_mct.c | 4 ++++
>>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>> index fc01ec2..be38119 100644
>>> --- a/drivers/clocksource/Kconfig
>>> +++ b/drivers/clocksource/Kconfig
>>> @@ -135,7 +135,6 @@ config CLKSRC_METAG_GENERIC
>>>
>>>   config CLKSRC_EXYNOS_MCT
>>>       def_bool y if ARCH_EXYNOS
>>> -    depends on !ARM64
>>>       help
>>>         Support for Multi Core Timer controller on Exynos SoCs.
>>>
>>> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
>>> index 9403061..b840ea1 100644
>>> --- a/drivers/clocksource/exynos_mct.c
>>> +++ b/drivers/clocksource/exynos_mct.c
>>> @@ -223,6 +223,7 @@ static u64 notrace exynos4_read_sched_clock(void)
>>>       return exynos4_read_count_32();
>>>   }
>>>
>>> +#if defined(CONFIG_ARM)
>>>   static struct delay_timer exynos4_delay_timer;
>>>
>>>   static cycles_t exynos4_read_current_timer(void)
>>> @@ -231,14 +232,17 @@ static cycles_t exynos4_read_current_timer(void)
>>>                "cycles_t needs to move to 32-bit for ARM64 usage");
>>>       return exynos4_read_count_32();
>>>   }
>>> +#endif
>>>
>>>   static void __init exynos4_clocksource_init(void)
>>>   {
>>>       exynos4_mct_frc_start();
>>>
>>> +#if defined(CONFIG_ARM)
>>>       exynos4_delay_timer.read_current_timer = &exynos4_read_current_timer;
>>>       exynos4_delay_timer.freq = clk_rate;
>>>       register_current_timer_delay(&exynos4_delay_timer);
>>> +#endif
>>>
>>>       if (clocksource_register_hz(&mct_frc, clk_rate))
>>>           panic("%s: can't register clocksource\n", mct_frc.name);
> 
> 

Do you have any comment about this patch?

Best Regards,
Chanwoo Choi

Re: [RESEND PATCH v3] clocksource: exynos_mct: Add the support for Exynos 64bit SoC

[Daniel Lezcano]

On 01/19/2015 01:54 AM, Chanwoo Choi wrote:
> Dear Daniel and Kukjin,
>
> On 01/15/2015 01:02 AM, Daniel Lezcano wrote:
>> On 01/14/2015 04:51 PM, Kukjin Kim wrote:
>>> On 01/14/15 14:33, Chanwoo Choi wrote:
>>>
>>> Hi,
>>>
>>> + Doug, Olof
>>>
>>>> This patch adds the support for Exynos 64bit SoC. The delay_timer is only used
>>>> for Exynos 32bit SoC.
>>>>
>>> Yes, the Exynos MCT(Multi-Core Timer) is 64bit timer and it is available
>>> on 64bit exynos SoC such as exynos7. But basically ARMv8 architecture is
>>> including ARM ARCH timer (ARM Generic Timer) and exynos7 also has
>>> implemented it and additionally its access is faster than using memory
>>> mapped register called SFR for MCT...so Doug submitted patch to use MCT
>>> on 32bit exynos SoCs before.
>>>
>>> I know using MCT on 64bit exynos is usefulness for Power Management and
>>> I need to talk to relevant guys in office again. If anything, I'll let
>>> you know.
>>
>> I will wait for your answer before digging more the patch.

Hi Chanwoo,

[ ... ]

> Do you have any comment about this patch?

Yes, a similar patch has been posted for the tegra2 timer to run on 
arm64. The patch in question put macros #ifdef CONFIG_ARM64 to disable 
some parts of the code. That ended, the tegra2 timer was not needed for 
the moment because of the arch timer present, so it has been disabled 
from the compilation until a proper fix without macros could be proposed.

It is happening exactly the same with this patch. As Kukjin pointed it, 
the exynos_mct may not be needed (at least until a backup timer is 
needed at PM time).

I suggest you look at a nicer way to fix that instead of introducing 
macros (which is by the way valid but not recommended by the CodingStyle 
rules) and perhaps sync with Paul and Thierry [cc'ed] to find a common 
solution.

   -- Daniel


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [RESEND PATCH v3] clocksource: exynos_mct: Add the support for Exynos 64bit SoC

[Chanwoo Choi]

Hi Daniel,

On 01/19/2015 06:27 PM, Daniel Lezcano wrote:
> On 01/19/2015 01:54 AM, Chanwoo Choi wrote:
>> Dear Daniel and Kukjin,
>>
>> On 01/15/2015 01:02 AM, Daniel Lezcano wrote:
>>> On 01/14/2015 04:51 PM, Kukjin Kim wrote:
>>>> On 01/14/15 14:33, Chanwoo Choi wrote:
>>>>
>>>> Hi,
>>>>
>>>> + Doug, Olof
>>>>
>>>>> This patch adds the support for Exynos 64bit SoC. The delay_timer is only used
>>>>> for Exynos 32bit SoC.
>>>>>
>>>> Yes, the Exynos MCT(Multi-Core Timer) is 64bit timer and it is available
>>>> on 64bit exynos SoC such as exynos7. But basically ARMv8 architecture is
>>>> including ARM ARCH timer (ARM Generic Timer) and exynos7 also has
>>>> implemented it and additionally its access is faster than using memory
>>>> mapped register called SFR for MCT...so Doug submitted patch to use MCT
>>>> on 32bit exynos SoCs before.
>>>>
>>>> I know using MCT on 64bit exynos is usefulness for Power Management and
>>>> I need to talk to relevant guys in office again. If anything, I'll let
>>>> you know.
>>>
>>> I will wait for your answer before digging more the patch.
> 
> Hi Chanwoo,
> 
> [ ... ]
> 
>> Do you have any comment about this patch?
> 
> Yes, a similar patch has been posted for the tegra2 timer to run on arm64. The patch in question put macros #ifdef CONFIG_ARM64 to disable some parts of the code. That ended, the tegra2 timer was not needed for the moment because of the arch timer present, so it has been disabled from the compilation until a proper fix without macros could be proposed.
> 
> It is happening exactly the same with this patch. As Kukjin pointed it, the exynos_mct may not be needed (at least until a backup timer is needed at PM time).
> 
> I suggest you look at a nicer way to fix that instead of introducing macros (which is by the way valid but not recommended by the CodingStyle rules) and perhaps sync with Paul and Thierry [cc'ed] to find a common solution.

OK, I understand your opinion. I'll try to consider it without adding '#ifdef'.

Best Regards,
Chanwoo Choi