[PATCH 0/5] Exynos SoCs: enable support for ARM Architected Timers

[PATCH 0/5] Exynos SoCs: enable support for ARM Architected Timers

[Marek Szyprowski]

Dear All,

ARM Architected Timers are present in all CortexA7/A15 based Samsung
Exynos SoCs. So far they were not enabled, because there were some issues
related to their initialization. Samsung Exynos SoCs used custom timer
hardware - Exynos MultiCore Timer. It turned out that enabling MCT it is
also needed to get ARM Architected Timers working, because they both
share some common hardware blocks (global system counter).

This patchset enables support for ARM Architected Timer driver together
with a standard Exynos MultiCore Timer driver, which is kept as a default
timer source on ARM 32bit platforms. Support for ARM architected timers
is essential for enabling proper KVM support on those platforms.

Some of the MCT cleanup patches were earlier a part of the similar
patchset for ARM64-based Exynos5433 SoC.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland

Marek Szyprowski (5):
  clocksource: exynos_mct: Remove dead code
  clocksource: exynos_mct: Fix error path in timer resources
    initialization
  clocksource: exynos_mct: Increase priority over ARM arch timer
  ARM: dts: exynos: Add support ARM architected timers
  ARM: exynos: Enable support for ARM architected timers

 arch/arm/boot/dts/exynos3250.dtsi | 10 ++++++++++
 arch/arm/boot/dts/exynos5250.dtsi |  1 +
 arch/arm/boot/dts/exynos54xx.dtsi | 10 ++++++++++
 arch/arm/mach-exynos/Kconfig      |  1 +
 drivers/clocksource/exynos_mct.c  | 27 +++++++++++++++++----------
 include/linux/cpuhotplug.h        |  2 +-
 6 files changed, 40 insertions(+), 11 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/5] clocksource: exynos_mct: Remove dead code

[Marek Szyprowski]

Exynos Multi-Core Timer driver is used only on device-tree based
systems, so remove non-dt related code. In case of !CONFIG_OF
the code is anyway equal because of_irq_count() has a stub
returning 0. Device node pointer is always provided when driver
has been probed from device tree.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/clocksource/exynos_mct.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 7a244b681876..ef18bbf8d20c 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -507,13 +507,12 @@ static int __init exynos4_timer_resources(struct device_node *np, void __iomem *
 	int err, cpu;
 	struct clk *mct_clk, *tick_clk;
 
-	tick_clk = np ? of_clk_get_by_name(np, "fin_pll") :
-				clk_get(NULL, "fin_pll");
+	tick_clk = of_clk_get_by_name(np, "fin_pll");
 	if (IS_ERR(tick_clk))
 		panic("%s: unable to determine tick clock rate\n", __func__);
 	clk_rate = clk_get_rate(tick_clk);
 
-	mct_clk = np ? of_clk_get_by_name(np, "mct") : clk_get(NULL, "mct");
+	mct_clk = of_clk_get_by_name(np, "mct");
 	if (IS_ERR(mct_clk))
 		panic("%s: unable to retrieve mct clock instance\n", __func__);
 	clk_prepare_enable(mct_clk);
@@ -581,11 +580,7 @@ static int __init mct_init_dt(struct device_node *np, unsigned int int_type)
 	 * timer irqs are specified after the four global timer
 	 * irqs are specified.
 	 */
-#ifdef CONFIG_OF
 	nr_irqs = of_irq_count(np);
-#else
-	nr_irqs = 0;
-#endif
 	for (i = MCT_L0_IRQ; i < nr_irqs; i++)
 		mct_irqs[i] = irq_of_parse_and_map(np, i);
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 2/6] clocksource: exynos_mct: Fix error path in timer resources initialization

[Marek Szyprowski]

While freeing interrupt handlers in error path, don't assume that all
requested interrupts are per-processor interrupts and properly release
standard interrupts too.

Reported-by: Krzysztof Kozlowski <krzk@kernel.org>
Fixes: 56a94f13919c ("clocksource: exynos_mct: Avoid blocking calls in the cpu hotplug notifier")
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/clocksource/exynos_mct.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index ef18bbf8d20c..49413900b24c 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -561,7 +561,19 @@ static int __init exynos4_timer_resources(struct device_node *np, void __iomem *
 	return 0;
 
 out_irq:
-	free_percpu_irq(mct_irqs[MCT_L0_IRQ], &percpu_mct_tick);
+	if (mct_int_type == MCT_INT_PPI) {
+		free_percpu_irq(mct_irqs[MCT_L0_IRQ], &percpu_mct_tick);
+	} else {
+		for_each_possible_cpu(cpu) {
+			struct mct_clock_event_device *pcpu_mevt =
+				per_cpu_ptr(&percpu_mct_tick, cpu);
+
+			if (pcpu_mevt->evt.irq != -1) {
+				free_irq(pcpu_mevt->evt.irq, pcpu_mevt);
+				pcpu_mevt->evt.irq = -1;
+			}
+		}
+	}
 	return err;
 }
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/5] clocksource: exynos_mct: Increase priority over ARM arch timer

[Marek Szyprowski]

Exynos Multi-Core Timer driver (exynos_mct) must be started before ARM
Architected Timers (arch_timer), because they both share some common
hardware blocks (global system counter) and turning on MCT is needed
to get ARM Architected Timer working properly. Increase MCT timer rating
and hotplug priority over ARM Archictected timer driver to achieve that.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/clocksource/exynos_mct.c | 4 ++--
 include/linux/cpuhotplug.h       | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 49413900b24c..58090f57ada3 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -211,7 +211,7 @@ static void exynos4_frc_resume(struct clocksource *cs)
 
 static struct clocksource mct_frc = {
 	.name		= "mct-frc",
-	.rating		= 400,
+	.rating		= 450,
 	.read		= exynos4_frc_read,
 	.mask		= CLOCKSOURCE_MASK(32),
 	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
@@ -465,7 +465,7 @@ static int exynos4_mct_starting_cpu(unsigned int cpu)
 	evt->set_state_oneshot_stopped = set_state_shutdown;
 	evt->tick_resume = set_state_shutdown;
 	evt->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
-	evt->rating = 450;
+	evt->rating = 500;
 
 	exynos4_mct_write(TICK_BASE_CNT, mevt->base + MCT_L_TCNTB_OFFSET);
 
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index fd586d0301e7..bdd544f21102 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -115,10 +115,10 @@ enum cpuhp_state {
 	CPUHP_AP_PERF_ARM_ACPI_STARTING,
 	CPUHP_AP_PERF_ARM_STARTING,
 	CPUHP_AP_ARM_L2X0_STARTING,
+	CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
 	CPUHP_AP_ARM_ARCH_TIMER_STARTING,
 	CPUHP_AP_ARM_GLOBAL_TIMER_STARTING,
 	CPUHP_AP_JCORE_TIMER_STARTING,
-	CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
 	CPUHP_AP_ARM_TWD_STARTING,
 	CPUHP_AP_QCOM_TIMER_STARTING,
 	CPUHP_AP_ARMADA_TIMER_STARTING,
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] clocksource: exynos_mct: Increase priority over ARM arch timer

[Krzysztof Kozlowski]

On Fri, 15 Feb 2019 at 13:52, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Exynos Multi-Core Timer driver (exynos_mct) must be started before ARM
> Architected Timers (arch_timer), because they both share some common
> hardware blocks (global system counter) and turning on MCT is needed
> to get ARM Architected Timer working properly. Increase MCT timer rating
> and hotplug priority over ARM Archictected timer driver to achieve that.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/clocksource/exynos_mct.c | 4 ++--
>  include/linux/cpuhotplug.h       | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] clocksource: exynos_mct: Increase priority over ARM arch timer

[Daniel Lezcano]

On 15/02/2019 13:52, Marek Szyprowski wrote:
> Exynos Multi-Core Timer driver (exynos_mct) must be started before ARM
> Architected Timers (arch_timer), because they both share some common
> hardware blocks (global system counter) and turning on MCT is needed
> to get ARM Architected Timer working properly. Increase MCT timer rating
> and hotplug priority over ARM Archictected timer driver to achieve that.

This is a hack.

There are mechanisms to handle this kind of dependency.

eg. https://www.kernel.org/doc/html/v4.15/driver-api/device_link.html


> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/clocksource/exynos_mct.c | 4 ++--
>  include/linux/cpuhotplug.h       | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
> index 49413900b24c..58090f57ada3 100644
> --- a/drivers/clocksource/exynos_mct.c
> +++ b/drivers/clocksource/exynos_mct.c
> @@ -211,7 +211,7 @@ static void exynos4_frc_resume(struct clocksource *cs)
>  
>  static struct clocksource mct_frc = {
>  	.name		= "mct-frc",
> -	.rating		= 400,
> +	.rating		= 450,
>  	.read		= exynos4_frc_read,
>  	.mask		= CLOCKSOURCE_MASK(32),
>  	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
> @@ -465,7 +465,7 @@ static int exynos4_mct_starting_cpu(unsigned int cpu)
>  	evt->set_state_oneshot_stopped = set_state_shutdown;
>  	evt->tick_resume = set_state_shutdown;
>  	evt->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
> -	evt->rating = 450;
> +	evt->rating = 500;
>  
>  	exynos4_mct_write(TICK_BASE_CNT, mevt->base + MCT_L_TCNTB_OFFSET);
>  
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index fd586d0301e7..bdd544f21102 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -115,10 +115,10 @@ enum cpuhp_state {
>  	CPUHP_AP_PERF_ARM_ACPI_STARTING,
>  	CPUHP_AP_PERF_ARM_STARTING,
>  	CPUHP_AP_ARM_L2X0_STARTING,
> +	CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
>  	CPUHP_AP_ARM_ARCH_TIMER_STARTING,
>  	CPUHP_AP_ARM_GLOBAL_TIMER_STARTING,
>  	CPUHP_AP_JCORE_TIMER_STARTING,
> -	CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
>  	CPUHP_AP_ARM_TWD_STARTING,
>  	CPUHP_AP_QCOM_TIMER_STARTING,
>  	CPUHP_AP_ARMADA_TIMER_STARTING,
> 


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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] clocksource: exynos_mct: Increase priority over ARM arch timer

[Marek Szyprowski]

Hi Daniel,

On 2019-02-15 17:35, Daniel Lezcano wrote:
> On 15/02/2019 13:52, Marek Szyprowski wrote:
>> Exynos Multi-Core Timer driver (exynos_mct) must be started before ARM
>> Architected Timers (arch_timer), because they both share some common
>> hardware blocks (global system counter) and turning on MCT is needed
>> to get ARM Architected Timer working properly. Increase MCT timer rating
>> and hotplug priority over ARM Archictected timer driver to achieve that.
> This is a hack.
>
> There are mechanisms to handle this kind of dependency.
>
> eg. https://www.kernel.org/doc/html/v4.15/driver-api/device_link.html

Sorry, but this is not a hack, but proper way to handle this in
clocksource/timers framework. The ratings assigned to each driver and
cpu hotplug priority list are exactly for managing the requested order
of operations if more than one driver has been registered.

Device links operates on the 'kernel device' objects, which are created
and managed much later than clocksource/timers are initialized. I see no
use of device links in this context.

>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>  drivers/clocksource/exynos_mct.c | 4 ++--
>>  include/linux/cpuhotplug.h       | 2 +-
>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
>> index 49413900b24c..58090f57ada3 100644
>> --- a/drivers/clocksource/exynos_mct.c
>> +++ b/drivers/clocksource/exynos_mct.c
>> @@ -211,7 +211,7 @@ static void exynos4_frc_resume(struct clocksource *cs)
>>  
>>  static struct clocksource mct_frc = {
>>  	.name		= "mct-frc",
>> -	.rating		= 400,
>> +	.rating		= 450,
>>  	.read		= exynos4_frc_read,
>>  	.mask		= CLOCKSOURCE_MASK(32),
>>  	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
>> @@ -465,7 +465,7 @@ static int exynos4_mct_starting_cpu(unsigned int cpu)
>>  	evt->set_state_oneshot_stopped = set_state_shutdown;
>>  	evt->tick_resume = set_state_shutdown;
>>  	evt->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
>> -	evt->rating = 450;
>> +	evt->rating = 500;
>>  
>>  	exynos4_mct_write(TICK_BASE_CNT, mevt->base + MCT_L_TCNTB_OFFSET);
>>  
>> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
>> index fd586d0301e7..bdd544f21102 100644
>> --- a/include/linux/cpuhotplug.h
>> +++ b/include/linux/cpuhotplug.h
>> @@ -115,10 +115,10 @@ enum cpuhp_state {
>>  	CPUHP_AP_PERF_ARM_ACPI_STARTING,
>>  	CPUHP_AP_PERF_ARM_STARTING,
>>  	CPUHP_AP_ARM_L2X0_STARTING,
>> +	CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
>>  	CPUHP_AP_ARM_ARCH_TIMER_STARTING,
>>  	CPUHP_AP_ARM_GLOBAL_TIMER_STARTING,
>>  	CPUHP_AP_JCORE_TIMER_STARTING,
>> -	CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
>>  	CPUHP_AP_ARM_TWD_STARTING,
>>  	CPUHP_AP_QCOM_TIMER_STARTING,
>>  	CPUHP_AP_ARMADA_TIMER_STARTING,
>>
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] clocksource: exynos_mct: Increase priority over ARM arch timer

[Daniel Lezcano]

On 18/02/2019 08:41, Marek Szyprowski wrote:
> Hi Daniel,
> 
> On 2019-02-15 17:35, Daniel Lezcano wrote:
>> On 15/02/2019 13:52, Marek Szyprowski wrote:
>>> Exynos Multi-Core Timer driver (exynos_mct) must be started before ARM
>>> Architected Timers (arch_timer), because they both share some common
>>> hardware blocks (global system counter) and turning on MCT is needed
>>> to get ARM Architected Timer working properly. Increase MCT timer rating
>>> and hotplug priority over ARM Archictected timer driver to achieve that.
>> This is a hack.
>>
>> There are mechanisms to handle this kind of dependency.
>>
>> eg. https://www.kernel.org/doc/html/v4.15/driver-api/device_link.html
> 
> Sorry, but this is not a hack, but proper way to handle this in
> clocksource/timers framework. The ratings assigned to each driver and
> cpu hotplug priority list are exactly for managing the requested order
> of operations if more than one driver has been registered.

Actually, I'm reluctant with this change (even if it is correct) because
it introduces an implicit dependency in the cpu hotplug enumeration for
the timer subsystem while it may be more correct to have it explicitly
with the hardware description (or whatever).

However I don't see a proper way to describe this dependency without
touching the DT and add a phandle to the arch timer in the exynos_mct
timer description.

> Device links operates on the 'kernel device' objects, which are created
> and managed much later than clocksource/timers are initialized. I see no
> use of device links in this context.

Yes, that was just an example to show there are mechanisms to allow
proper ordering. As said before, not the best example for this situation.

>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> ---
>>>  drivers/clocksource/exynos_mct.c | 4 ++--
>>>  include/linux/cpuhotplug.h       | 2 +-
>>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
>>> index 49413900b24c..58090f57ada3 100644
>>> --- a/drivers/clocksource/exynos_mct.c
>>> +++ b/drivers/clocksource/exynos_mct.c
>>> @@ -211,7 +211,7 @@ static void exynos4_frc_resume(struct clocksource *cs)
>>>  
>>>  static struct clocksource mct_frc = {
>>>  	.name		= "mct-frc",
>>> -	.rating		= 400,
>>> +	.rating		= 450,
>>>  	.read		= exynos4_frc_read,
>>>  	.mask		= CLOCKSOURCE_MASK(32),
>>>  	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
>>> @@ -465,7 +465,7 @@ static int exynos4_mct_starting_cpu(unsigned int cpu)
>>>  	evt->set_state_oneshot_stopped = set_state_shutdown;
>>>  	evt->tick_resume = set_state_shutdown;
>>>  	evt->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
>>> -	evt->rating = 450;
>>> +	evt->rating = 500;
>>>  
>>>  	exynos4_mct_write(TICK_BASE_CNT, mevt->base + MCT_L_TCNTB_OFFSET);
>>>  
>>> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
>>> index fd586d0301e7..bdd544f21102 100644
>>> --- a/include/linux/cpuhotplug.h
>>> +++ b/include/linux/cpuhotplug.h
>>> @@ -115,10 +115,10 @@ enum cpuhp_state {
>>>  	CPUHP_AP_PERF_ARM_ACPI_STARTING,
>>>  	CPUHP_AP_PERF_ARM_STARTING,
>>>  	CPUHP_AP_ARM_L2X0_STARTING,
>>> +	CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
>>>  	CPUHP_AP_ARM_ARCH_TIMER_STARTING,
>>>  	CPUHP_AP_ARM_GLOBAL_TIMER_STARTING,
>>>  	CPUHP_AP_JCORE_TIMER_STARTING,
>>> -	CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
>>>  	CPUHP_AP_ARM_TWD_STARTING,
>>>  	CPUHP_AP_QCOM_TIMER_STARTING,
>>>  	CPUHP_AP_ARMADA_TIMER_STARTING,
>>>
>>
> Best regards
> 


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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] clocksource: exynos_mct: Increase priority over ARM arch timer

[Marek Szyprowski]

Hi Daniel,

On 2019-02-18 10:24, Daniel Lezcano wrote:
> On 18/02/2019 08:41, Marek Szyprowski wrote:
>> On 2019-02-15 17:35, Daniel Lezcano wrote:
>>> On 15/02/2019 13:52, Marek Szyprowski wrote:
>>>> Exynos Multi-Core Timer driver (exynos_mct) must be started before ARM
>>>> Architected Timers (arch_timer), because they both share some common
>>>> hardware blocks (global system counter) and turning on MCT is needed
>>>> to get ARM Architected Timer working properly. Increase MCT timer rating
>>>> and hotplug priority over ARM Archictected timer driver to achieve that.
>>> This is a hack.
>>>
>>> There are mechanisms to handle this kind of dependency.
>>>
>>> eg. https://www.kernel.org/doc/html/v4.15/driver-api/device_link.html
>> Sorry, but this is not a hack, but proper way to handle this in
>> clocksource/timers framework. The ratings assigned to each driver and
>> cpu hotplug priority list are exactly for managing the requested order
>> of operations if more than one driver has been registered.
> Actually, I'm reluctant with this change (even if it is correct) because
> it introduces an implicit dependency in the cpu hotplug enumeration for
> the timer subsystem while it may be more correct to have it explicitly
> with the hardware description (or whatever).

Maybe additional comment in the cpu hotplug list will convince you? For
the current code changing the order of enums it is more than enough.
There are no dynamic CPU hotplug priorities, so MCT driver cannot change
its behavior depending on any dt-property or so. Increasing MCT priority
has no side-effects in relation to other drivers (as they cannot
existing on the same board).

> However I don't see a proper way to describe this dependency without
> touching the DT and add a phandle to the arch timer in the exynos_mct
> timer description.
>
>> Device links operates on the 'kernel device' objects, which are created
>> and managed much later than clocksource/timers are initialized. I see no
>> use of device links in this context.
> Yes, that was just an example to show there are mechanisms to allow
> proper ordering. As said before, not the best example for this situation.
>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>> ---
>>>>  drivers/clocksource/exynos_mct.c | 4 ++--
>>>>  include/linux/cpuhotplug.h       | 2 +-
>>>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
>>>> index 49413900b24c..58090f57ada3 100644
>>>> --- a/drivers/clocksource/exynos_mct.c
>>>> +++ b/drivers/clocksource/exynos_mct.c
>>>> @@ -211,7 +211,7 @@ static void exynos4_frc_resume(struct clocksource *cs)
>>>>  
>>>>  static struct clocksource mct_frc = {
>>>>  	.name		= "mct-frc",
>>>> -	.rating		= 400,
>>>> +	.rating		= 450,
>>>>  	.read		= exynos4_frc_read,
>>>>  	.mask		= CLOCKSOURCE_MASK(32),
>>>>  	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
>>>> @@ -465,7 +465,7 @@ static int exynos4_mct_starting_cpu(unsigned int cpu)
>>>>  	evt->set_state_oneshot_stopped = set_state_shutdown;
>>>>  	evt->tick_resume = set_state_shutdown;
>>>>  	evt->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
>>>> -	evt->rating = 450;
>>>> +	evt->rating = 500;
>>>>  
>>>>  	exynos4_mct_write(TICK_BASE_CNT, mevt->base + MCT_L_TCNTB_OFFSET);
>>>>  
>>>> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
>>>> index fd586d0301e7..bdd544f21102 100644
>>>> --- a/include/linux/cpuhotplug.h
>>>> +++ b/include/linux/cpuhotplug.h
>>>> @@ -115,10 +115,10 @@ enum cpuhp_state {
>>>>  	CPUHP_AP_PERF_ARM_ACPI_STARTING,
>>>>  	CPUHP_AP_PERF_ARM_STARTING,
>>>>  	CPUHP_AP_ARM_L2X0_STARTING,
>>>> +	CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
>>>>  	CPUHP_AP_ARM_ARCH_TIMER_STARTING,
>>>>  	CPUHP_AP_ARM_GLOBAL_TIMER_STARTING,
>>>>  	CPUHP_AP_JCORE_TIMER_STARTING,
>>>> -	CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
>>>>  	CPUHP_AP_ARM_TWD_STARTING,
>>>>  	CPUHP_AP_QCOM_TIMER_STARTING,
>>>>  	CPUHP_AP_ARMADA_TIMER_STARTING,
>>>>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] clocksource: exynos_mct: Increase priority over ARM arch timer

[Daniel Lezcano]

On 18/02/2019 10:49, Marek Szyprowski wrote:
> Hi Daniel,
> 
> On 2019-02-18 10:24, Daniel Lezcano wrote:
>> On 18/02/2019 08:41, Marek Szyprowski wrote:
>>> On 2019-02-15 17:35, Daniel Lezcano wrote:
>>>> On 15/02/2019 13:52, Marek Szyprowski wrote:
>>>>> Exynos Multi-Core Timer driver (exynos_mct) must be started before ARM
>>>>> Architected Timers (arch_timer), because they both share some common
>>>>> hardware blocks (global system counter) and turning on MCT is needed
>>>>> to get ARM Architected Timer working properly. Increase MCT timer rating
>>>>> and hotplug priority over ARM Archictected timer driver to achieve that.
>>>> This is a hack.
>>>>
>>>> There are mechanisms to handle this kind of dependency.
>>>>
>>>> eg. https://www.kernel.org/doc/html/v4.15/driver-api/device_link.html
>>> Sorry, but this is not a hack, but proper way to handle this in
>>> clocksource/timers framework. The ratings assigned to each driver and
>>> cpu hotplug priority list are exactly for managing the requested order
>>> of operations if more than one driver has been registered.
>> Actually, I'm reluctant with this change (even if it is correct) because
>> it introduces an implicit dependency in the cpu hotplug enumeration for
>> the timer subsystem while it may be more correct to have it explicitly
>> with the hardware description (or whatever).
> 
> Maybe additional comment in the cpu hotplug list will convince you? For
> the current code changing the order of enums it is more than enough.
> There are no dynamic CPU hotplug priorities, so MCT driver cannot change
> its behavior depending on any dt-property or so. Increasing MCT priority
> has no side-effects in relation to other drivers (as they cannot
> existing on the same board).

It is Thomas's call, I will let him decide if it is ok .



>> However I don't see a proper way to describe this dependency without
>> touching the DT and add a phandle to the arch timer in the exynos_mct
>> timer description.
>>
>>> Device links operates on the 'kernel device' objects, which are created
>>> and managed much later than clocksource/timers are initialized. I see no
>>> use of device links in this context.
>> Yes, that was just an example to show there are mechanisms to allow
>> proper ordering. As said before, not the best example for this situation.
>>
>>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>>> ---
>>>>>  drivers/clocksource/exynos_mct.c | 4 ++--
>>>>>  include/linux/cpuhotplug.h       | 2 +-
>>>>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
>>>>> index 49413900b24c..58090f57ada3 100644
>>>>> --- a/drivers/clocksource/exynos_mct.c
>>>>> +++ b/drivers/clocksource/exynos_mct.c
>>>>> @@ -211,7 +211,7 @@ static void exynos4_frc_resume(struct clocksource *cs)
>>>>>  
>>>>>  static struct clocksource mct_frc = {
>>>>>  	.name		= "mct-frc",
>>>>> -	.rating		= 400,
>>>>> +	.rating		= 450,
>>>>>  	.read		= exynos4_frc_read,
>>>>>  	.mask		= CLOCKSOURCE_MASK(32),
>>>>>  	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
>>>>> @@ -465,7 +465,7 @@ static int exynos4_mct_starting_cpu(unsigned int cpu)
>>>>>  	evt->set_state_oneshot_stopped = set_state_shutdown;
>>>>>  	evt->tick_resume = set_state_shutdown;
>>>>>  	evt->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
>>>>> -	evt->rating = 450;
>>>>> +	evt->rating = 500;
>>>>>  
>>>>>  	exynos4_mct_write(TICK_BASE_CNT, mevt->base + MCT_L_TCNTB_OFFSET);
>>>>>  
>>>>> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
>>>>> index fd586d0301e7..bdd544f21102 100644
>>>>> --- a/include/linux/cpuhotplug.h
>>>>> +++ b/include/linux/cpuhotplug.h
>>>>> @@ -115,10 +115,10 @@ enum cpuhp_state {
>>>>>  	CPUHP_AP_PERF_ARM_ACPI_STARTING,
>>>>>  	CPUHP_AP_PERF_ARM_STARTING,
>>>>>  	CPUHP_AP_ARM_L2X0_STARTING,
>>>>> +	CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
>>>>>  	CPUHP_AP_ARM_ARCH_TIMER_STARTING,
>>>>>  	CPUHP_AP_ARM_GLOBAL_TIMER_STARTING,
>>>>>  	CPUHP_AP_JCORE_TIMER_STARTING,
>>>>> -	CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
>>>>>  	CPUHP_AP_ARM_TWD_STARTING,
>>>>>  	CPUHP_AP_QCOM_TIMER_STARTING,
>>>>>  	CPUHP_AP_ARMADA_TIMER_STARTING,
>>>>>
> Best regards
> 


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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] clocksource: exynos_mct: Increase priority over ARM arch timer

[Krzysztof Kozlowski]

On Fri, 15 Feb 2019 at 17:35, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 15/02/2019 13:52, Marek Szyprowski wrote:
> > Exynos Multi-Core Timer driver (exynos_mct) must be started before ARM
> > Architected Timers (arch_timer), because they both share some common
> > hardware blocks (global system counter) and turning on MCT is needed
> > to get ARM Architected Timer working properly. Increase MCT timer rating
> > and hotplug priority over ARM Archictected timer driver to achieve that.
>
> This is a hack.
>
> There are mechanisms to handle this kind of dependency.
>
> eg. https://www.kernel.org/doc/html/v4.15/driver-api/device_link.html

Hi Daniel,

Are you sure you have proper devices at this point so you can order
dependencies with device links?

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] clocksource: exynos_mct: Increase priority over ARM arch timer

[Daniel Lezcano]

On 18/02/2019 09:53, Krzysztof Kozlowski wrote:
> On Fri, 15 Feb 2019 at 17:35, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>
>> On 15/02/2019 13:52, Marek Szyprowski wrote:
>>> Exynos Multi-Core Timer driver (exynos_mct) must be started before ARM
>>> Architected Timers (arch_timer), because they both share some common
>>> hardware blocks (global system counter) and turning on MCT is needed
>>> to get ARM Architected Timer working properly. Increase MCT timer rating
>>> and hotplug priority over ARM Archictected timer driver to achieve that.
>>
>> This is a hack.
>>
>> There are mechanisms to handle this kind of dependency.
>>
>> eg. https://www.kernel.org/doc/html/v4.15/driver-api/device_link.html
> 
> Hi Daniel,
> 
> Are you sure you have proper devices at this point so you can order
> dependencies with device links?

No, it was an example to show there are mechanism to handle the
dependencies. Agree it is a confusing example.




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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/5] ARM: dts: exynos: Add support ARM architected timers

[Marek Szyprowski]

All CortexA7/A15 based Exynos SoCs have ARM architected timers, so enable
support for them directly in the base dtsi for Exynos3250, Exynos5250
and Exynos54xx SoCs. None of the known firmware properly configures arch
timer registers, so mark them as not-fw-configured and set frequency
to 24MHz, which is the only configuration supported by the remaining
drivers (clocks) so far.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos3250.dtsi | 10 ++++++++++
 arch/arm/boot/dts/exynos5250.dtsi |  1 +
 arch/arm/boot/dts/exynos54xx.dtsi | 10 ++++++++++
 3 files changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
index 608d17454179..3966e7960964 100644
--- a/arch/arm/boot/dts/exynos3250.dtsi
+++ b/arch/arm/boot/dts/exynos3250.dtsi
@@ -134,6 +134,16 @@
 			};
 		};
 
+		timer {
+			compatible = "arm,armv7-timer";
+			interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+				     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+				     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+				     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
+			clock-frequency = <24000000>;
+			arm,cpu-registers-not-fw-configured;
+		};
+
 		sysram@2020000 {
 			compatible = "mmio-sram";
 			reg = <0x02020000 0x40000>;
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 80986b97dfe5..95af4050f69d 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -239,6 +239,7 @@
 			 * so we need the value from DT.
 			 */
 			clock-frequency = <24000000>;
+			arm,cpu-registers-not-fw-configured;
 		};
 
 		mct@101c0000 {
diff --git a/arch/arm/boot/dts/exynos54xx.dtsi b/arch/arm/boot/dts/exynos54xx.dtsi
index de26e5ee0d2d..45e06717ee51 100644
--- a/arch/arm/boot/dts/exynos54xx.dtsi
+++ b/arch/arm/boot/dts/exynos54xx.dtsi
@@ -46,6 +46,16 @@
 			status = "disabled";
 		};
 
+		timer {
+			compatible = "arm,armv7-timer";
+			interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+				     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+				     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+				     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
+			arm,cpu-registers-not-fw-configured;
+			clock-frequency = <24000000>;
+		};
+
 		sysram@2020000 {
 			compatible = "mmio-sram";
 			reg = <0x02020000 0x54000>;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/5] ARM: dts: exynos: Add support ARM architected timers

[Chanwoo Choi]

Hi Marek,

On 19. 2. 15. 오후 9:52, Marek Szyprowski wrote:
> All CortexA7/A15 based Exynos SoCs have ARM architected timers, so enable
> support for them directly in the base dtsi for Exynos3250, Exynos5250
> and Exynos54xx SoCs. None of the known firmware properly configures arch
> timer registers, so mark them as not-fw-configured and set frequency
> to 24MHz, which is the only configuration supported by the remaining
> drivers (clocks) so far.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  arch/arm/boot/dts/exynos3250.dtsi | 10 ++++++++++
>  arch/arm/boot/dts/exynos5250.dtsi |  1 +
>  arch/arm/boot/dts/exynos54xx.dtsi | 10 ++++++++++
>  3 files changed, 21 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
> index 608d17454179..3966e7960964 100644
> --- a/arch/arm/boot/dts/exynos3250.dtsi
> +++ b/arch/arm/boot/dts/exynos3250.dtsi
> @@ -134,6 +134,16 @@
>  			};
>  		};
>  
> +		timer {
> +			compatible = "arm,armv7-timer";
> +			interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +				     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +				     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +				     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
> +			clock-frequency = <24000000>;
> +			arm,cpu-registers-not-fw-configured;
> +		};
> +
>  		sysram@2020000 {
>  			compatible = "mmio-sram";
>  			reg = <0x02020000 0x40000>;
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> index 80986b97dfe5..95af4050f69d 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -239,6 +239,7 @@
>  			 * so we need the value from DT.
>  			 */
>  			clock-frequency = <24000000>;
> +			arm,cpu-registers-not-fw-configured;
>  		};
>  
>  		mct@101c0000 {
> diff --git a/arch/arm/boot/dts/exynos54xx.dtsi b/arch/arm/boot/dts/exynos54xx.dtsi
> index de26e5ee0d2d..45e06717ee51 100644
> --- a/arch/arm/boot/dts/exynos54xx.dtsi
> +++ b/arch/arm/boot/dts/exynos54xx.dtsi
> @@ -46,6 +46,16 @@
>  			status = "disabled";
>  		};
>  
> +		timer {
> +			compatible = "arm,armv7-timer";
> +			interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +				     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +				     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +				     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
> +			arm,cpu-registers-not-fw-configured;
> +			clock-frequency = <24000000>;
> +		};
> +
>  		sysram@2020000 {
>  			compatible = "mmio-sram";
>  			reg = <0x02020000 0x54000>;
> 

The exynos54xx.dtsi affects the exynos5410 SoC.
When I checked the PPI port number on Exynos5410/Exynos5422 TRM,
they have the different PPI port number as following:

				|Exynos5410	|Exynos5422	|
----------------------------------------------------------------------------------------
ARCH_TIMER_PHYS_SECURE_PPI	|1 nCNTPSIRQ	|13 nCNTPSIRQ	|Secure physical timer
ARCH_TIMER_PHYS_NONSECURE_PPI	|2 nCNTPNSIRQ	|14 nCNTPNSIRQ	|Non-secure physical timer
ARCH_TIMER_VIRT_PPI		|4 nCNTVIRQ	|11 nCNTVIRQ	|Virtual timer
ARCH_TIMER_HYP_PPI		|5 nCNTHPIRQ	|10 nCNTHPIRQ	|Hypervisor timer
----------------------------------------------------------------------------------------


Exynos5410 and Exynos5422 have the different usage of bitfield
of  nCNTPSIRQ/nCNTPNSIRQ/nCNTVIRQ/nCNTHPIRQ as following:

				|Exynos5410	|Exynos5422
------------------------------------------------------------
nCNTPSIRQ[3:0]			| KFC		| EAGLE
nCNTPNSIRQ[3:0]			| KFC		| EAGLE
nCNTVIRQ[3:0]			| KFC		| EAGLE
nCNTHPIRQ[3:0]			| KFC		| EAGLE
------------------------------------------------------------
nCNTPSIRQ[7:4]			| EAGLE		| KFC
nCNTPNSIRQ[7:4]			| EAGLE		| KFC
nCNTVIRQ[7:4]			| EAGLE		| KFC
nCNTHPIRQ[7:4]			| EAGLE		| KFC


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/5] ARM: dts: exynos: Add support ARM architected timers

[Marek Szyprowski]

Hi Chanwoo,

On 2019-02-18 12:58, Chanwoo Choi wrote:
> On 19. 2. 15. 오후 9:52, Marek Szyprowski wrote:
>> All CortexA7/A15 based Exynos SoCs have ARM architected timers, so enable
>> support for them directly in the base dtsi for Exynos3250, Exynos5250
>> and Exynos54xx SoCs. None of the known firmware properly configures arch
>> timer registers, so mark them as not-fw-configured and set frequency
>> to 24MHz, which is the only configuration supported by the remaining
>> drivers (clocks) so far.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>  arch/arm/boot/dts/exynos3250.dtsi | 10 ++++++++++
>>  arch/arm/boot/dts/exynos5250.dtsi |  1 +
>>  arch/arm/boot/dts/exynos54xx.dtsi | 10 ++++++++++
>>  3 files changed, 21 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
>> index 608d17454179..3966e7960964 100644
>> --- a/arch/arm/boot/dts/exynos3250.dtsi
>> +++ b/arch/arm/boot/dts/exynos3250.dtsi
>> @@ -134,6 +134,16 @@
>>  			};
>>  		};
>>  
>> +		timer {
>> +			compatible = "arm,armv7-timer";
>> +			interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> +				     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> +				     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> +				     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
>> +			clock-frequency = <24000000>;
>> +			arm,cpu-registers-not-fw-configured;
>> +		};
>> +
>>  		sysram@2020000 {
>>  			compatible = "mmio-sram";
>>  			reg = <0x02020000 0x40000>;
>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
>> index 80986b97dfe5..95af4050f69d 100644
>> --- a/arch/arm/boot/dts/exynos5250.dtsi
>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>> @@ -239,6 +239,7 @@
>>  			 * so we need the value from DT.
>>  			 */
>>  			clock-frequency = <24000000>;
>> +			arm,cpu-registers-not-fw-configured;
>>  		};
>>  
>>  		mct@101c0000 {
>> diff --git a/arch/arm/boot/dts/exynos54xx.dtsi b/arch/arm/boot/dts/exynos54xx.dtsi
>> index de26e5ee0d2d..45e06717ee51 100644
>> --- a/arch/arm/boot/dts/exynos54xx.dtsi
>> +++ b/arch/arm/boot/dts/exynos54xx.dtsi
>> @@ -46,6 +46,16 @@
>>  			status = "disabled";
>>  		};
>>  
>> +		timer {
>> +			compatible = "arm,armv7-timer";
>> +			interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> +				     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> +				     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> +				     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
>> +			arm,cpu-registers-not-fw-configured;
>> +			clock-frequency = <24000000>;
>> +		};
>> +
>>  		sysram@2020000 {
>>  			compatible = "mmio-sram";
>>  			reg = <0x02020000 0x54000>;
>>
> The exynos54xx.dtsi affects the exynos5410 SoC.
> When I checked the PPI port number on Exynos5410/Exynos5422 TRM,
> they have the different PPI port number as following:
>
> 				|Exynos5410	|Exynos5422	|
> ----------------------------------------------------------------------------------------
> ARCH_TIMER_PHYS_SECURE_PPI	|1 nCNTPSIRQ	|13 nCNTPSIRQ	|Secure physical timer
> ARCH_TIMER_PHYS_NONSECURE_PPI	|2 nCNTPNSIRQ	|14 nCNTPNSIRQ	|Non-secure physical timer
> ARCH_TIMER_VIRT_PPI		|4 nCNTVIRQ	|11 nCNTVIRQ	|Virtual timer
> ARCH_TIMER_HYP_PPI		|5 nCNTHPIRQ	|10 nCNTHPIRQ	|Hypervisor timer
> ----------------------------------------------------------------------------------------

Sadly, Exynos5410 TRM seems to be wrong. All other Exynos5 TRMs (5250,
5260, 5420, 5422) defines 13,14,11,10 PPI for arch timer. I've checked
on the real hardware (Exynos5410-based Odroid XU board) and the values
taken from Exynos5422 TRM works fine, while the ones from 5410 TRM don't.

> Exynos5410 and Exynos5422 have the different usage of bitfield
> of  nCNTPSIRQ/nCNTPNSIRQ/nCNTVIRQ/nCNTHPIRQ as following:
>
> 				|Exynos5410	|Exynos5422
> ------------------------------------------------------------
> nCNTPSIRQ[3:0]			| KFC		| EAGLE
> nCNTPNSIRQ[3:0]			| KFC		| EAGLE
> nCNTVIRQ[3:0]			| KFC		| EAGLE
> nCNTHPIRQ[3:0]			| KFC		| EAGLE
> ------------------------------------------------------------
> nCNTPSIRQ[7:4]			| EAGLE		| KFC
> nCNTPNSIRQ[7:4]			| EAGLE		| KFC
> nCNTVIRQ[7:4]			| EAGLE		| KFC
> nCNTHPIRQ[7:4]			| EAGLE		| KFC

This is not an issue, this is a result of different order of bit/LITTLE
cores. Same for Exynos5420.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/5] ARM: dts: exynos: Add support ARM architected timers

[Chanwoo Choi]

Hi Marek,

On 19. 2. 18. 오후 9:24, Marek Szyprowski wrote:
> Hi Chanwoo,
> 
> On 2019-02-18 12:58, Chanwoo Choi wrote:
>> On 19. 2. 15. 오후 9:52, Marek Szyprowski wrote:
>>> All CortexA7/A15 based Exynos SoCs have ARM architected timers, so enable
>>> support for them directly in the base dtsi for Exynos3250, Exynos5250
>>> and Exynos54xx SoCs. None of the known firmware properly configures arch
>>> timer registers, so mark them as not-fw-configured and set frequency
>>> to 24MHz, which is the only configuration supported by the remaining
>>> drivers (clocks) so far.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> ---
>>>  arch/arm/boot/dts/exynos3250.dtsi | 10 ++++++++++
>>>  arch/arm/boot/dts/exynos5250.dtsi |  1 +
>>>  arch/arm/boot/dts/exynos54xx.dtsi | 10 ++++++++++
>>>  3 files changed, 21 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
>>> index 608d17454179..3966e7960964 100644
>>> --- a/arch/arm/boot/dts/exynos3250.dtsi
>>> +++ b/arch/arm/boot/dts/exynos3250.dtsi
>>> @@ -134,6 +134,16 @@
>>>  			};
>>>  		};
>>>  
>>> +		timer {
>>> +			compatible = "arm,armv7-timer";
>>> +			interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>>> +				     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>>> +				     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>>> +				     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
>>> +			clock-frequency = <24000000>;
>>> +			arm,cpu-registers-not-fw-configured;
>>> +		};
>>> +
>>>  		sysram@2020000 {
>>>  			compatible = "mmio-sram";
>>>  			reg = <0x02020000 0x40000>;
>>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
>>> index 80986b97dfe5..95af4050f69d 100644
>>> --- a/arch/arm/boot/dts/exynos5250.dtsi
>>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>>> @@ -239,6 +239,7 @@
>>>  			 * so we need the value from DT.
>>>  			 */
>>>  			clock-frequency = <24000000>;
>>> +			arm,cpu-registers-not-fw-configured;
>>>  		};
>>>  
>>>  		mct@101c0000 {
>>> diff --git a/arch/arm/boot/dts/exynos54xx.dtsi b/arch/arm/boot/dts/exynos54xx.dtsi
>>> index de26e5ee0d2d..45e06717ee51 100644
>>> --- a/arch/arm/boot/dts/exynos54xx.dtsi
>>> +++ b/arch/arm/boot/dts/exynos54xx.dtsi
>>> @@ -46,6 +46,16 @@
>>>  			status = "disabled";
>>>  		};
>>>  
>>> +		timer {
>>> +			compatible = "arm,armv7-timer";
>>> +			interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>>> +				     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>>> +				     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>>> +				     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
>>> +			arm,cpu-registers-not-fw-configured;
>>> +			clock-frequency = <24000000>;
>>> +		};
>>> +
>>>  		sysram@2020000 {
>>>  			compatible = "mmio-sram";
>>>  			reg = <0x02020000 0x54000>;
>>>
>> The exynos54xx.dtsi affects the exynos5410 SoC.
>> When I checked the PPI port number on Exynos5410/Exynos5422 TRM,
>> they have the different PPI port number as following:
>>
>> 				|Exynos5410	|Exynos5422	|
>> ----------------------------------------------------------------------------------------
>> ARCH_TIMER_PHYS_SECURE_PPI	|1 nCNTPSIRQ	|13 nCNTPSIRQ	|Secure physical timer
>> ARCH_TIMER_PHYS_NONSECURE_PPI	|2 nCNTPNSIRQ	|14 nCNTPNSIRQ	|Non-secure physical timer
>> ARCH_TIMER_VIRT_PPI		|4 nCNTVIRQ	|11 nCNTVIRQ	|Virtual timer
>> ARCH_TIMER_HYP_PPI		|5 nCNTHPIRQ	|10 nCNTHPIRQ	|Hypervisor timer
>> ----------------------------------------------------------------------------------------
> 
> Sadly, Exynos5410 TRM seems to be wrong. All other Exynos5 TRMs (5250,
> 5260, 5420, 5422) defines 13,14,11,10 PPI for arch timer. I've checked
> on the real hardware (Exynos5410-based Odroid XU board) and the values
> taken from Exynos5422 TRM works fine, while the ones from 5410 TRM don't.

Thanks for sharing the tested result.

Reveiwed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/5] ARM: exynos: Enable support for ARM architected timers

[Marek Szyprowski]

ARM architected timer can be used together with Exynos MultiCore Timer
driver, so enable support for it. Support for ARM architected timers is
essential for enabling proper KVM support.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/mach-exynos/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index b40963cf91c7..b5ec21c027a1 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -18,6 +18,7 @@ menuconfig ARCH_EXYNOS
 	select EXYNOS_SROM
 	select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS
 	select GPIOLIB
+	select HAVE_ARM_ARCH_TIMER
 	select HAVE_ARM_SCU if SMP
 	select HAVE_S3C2410_I2C if I2C
 	select HAVE_S3C2410_WATCHDOG if WATCHDOG
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/5] ARM: exynos: Enable support for ARM architected timers

[Chanwoo Choi]

Hi Marek,

On 19. 2. 15. 오후 9:52, Marek Szyprowski wrote:
> ARM architected timer can be used together with Exynos MultiCore Timer
> driver, so enable support for it. Support for ARM architected timers is
> essential for enabling proper KVM support.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  arch/arm/mach-exynos/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
> index b40963cf91c7..b5ec21c027a1 100644
> --- a/arch/arm/mach-exynos/Kconfig
> +++ b/arch/arm/mach-exynos/Kconfig
> @@ -18,6 +18,7 @@ menuconfig ARCH_EXYNOS
>  	select EXYNOS_SROM
>  	select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS
>  	select GPIOLIB
> +	select HAVE_ARM_ARCH_TIMER
>  	select HAVE_ARM_SCU if SMP
>  	select HAVE_S3C2410_I2C if I2C
>  	select HAVE_S3C2410_WATCHDOG if WATCHDOG
> 

Reveiwed-by: Chanwoo Choi <cw00.choi@samsung.com>


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/5] Exynos SoCs: enable support for ARM Architected Timers

[Daniel Lezcano]

Hi Marek,

I will pick patch 1 and 2 for 5.1

Thanks

  -- Daniel


On 15/02/2019 13:52, Marek Szyprowski wrote:
> Dear All,
> 
> ARM Architected Timers are present in all CortexA7/A15 based Samsung
> Exynos SoCs. So far they were not enabled, because there were some issues
> related to their initialization. Samsung Exynos SoCs used custom timer
> hardware - Exynos MultiCore Timer. It turned out that enabling MCT it is
> also needed to get ARM Architected Timers working, because they both
> share some common hardware blocks (global system counter).
> 
> This patchset enables support for ARM Architected Timer driver together
> with a standard Exynos MultiCore Timer driver, which is kept as a default
> timer source on ARM 32bit platforms. Support for ARM architected timers
> is essential for enabling proper KVM support on those platforms.
> 
> Some of the MCT cleanup patches were earlier a part of the similar
> patchset for ARM64-based Exynos5433 SoC.
> 
> Best regards
> Marek Szyprowski
> Samsung R&D Institute Poland
> 
> Marek Szyprowski (5):
>   clocksource: exynos_mct: Remove dead code
>   clocksource: exynos_mct: Fix error path in timer resources
>     initialization
>   clocksource: exynos_mct: Increase priority over ARM arch timer
>   ARM: dts: exynos: Add support ARM architected timers
>   ARM: exynos: Enable support for ARM architected timers
> 
>  arch/arm/boot/dts/exynos3250.dtsi | 10 ++++++++++
>  arch/arm/boot/dts/exynos5250.dtsi |  1 +
>  arch/arm/boot/dts/exynos54xx.dtsi | 10 ++++++++++
>  arch/arm/mach-exynos/Kconfig      |  1 +
>  drivers/clocksource/exynos_mct.c  | 27 +++++++++++++++++----------
>  include/linux/cpuhotplug.h        |  2 +-
>  6 files changed, 40 insertions(+), 11 deletions(-)
> 


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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel