Use Exynos-MCT timer as wakeup for Arm arch timer
Subsystems (2)
cover Use Exynos-MCT timer as wakeup for Arm arch timer
Hi all,
These two patches build on top of my other series for adding per-cpu wakeup timers [1] which is currently queued in -tip [2].
The basic idea is to prefer the Arm architected timer, even when CLOCK_EVT_FEAT_C3STOP is set, as long as we have another timer which can be used for the per-cpu wakeup. These patches use Exynos-MCT as an example, since that driver currently goes out of its way to prevent use of the Arm architected timer due to issues with a 32-bit SoC.
Cheers,
Will
[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=timers/core
--->8
Will Deacon (2):
clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64 clocksource/drivers/exynos_mct: Mark MCT device as
CLOCK_EVT_FEAT_PERCPU
drivers/clocksource/exynos_mct.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
Signature · 1 line
2.32.0.rc1.229.g3e70b5a671-goog
PATCH 1/2 clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64
All arm64 CPUs feature an architected timer, which offers a relatively low-latency interface to a per-cpu clocksource and timer. For the most part, using this interface is a no-brainer, with the exception of SoCs where it cannot be used to wake up from deep idle state (i.e. CLOCK_EVT_FEAT_C3STOP is set).
On the contrary, the Exynos MCT is extremely slow to access yet can be used as a wakeup source. In preparation for using the Exynos MCT as a potential wakeup timer for the Arm architected timer, reduce its ratings so that the architected timer is preferred.
This effectively reverts the decision made in 6282edb72bed ("clocksource/drivers/exynos_mct: Increase priority over ARM arch timer") for arm64, as the reasoning for the original change was to work around a 32-bit SoC design.
--- drivers/clocksource/exynos_mct.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
drivers/clocksource/exynos_mct.c +11 -2
@@ -51,6 +51,15 @@#define TICK_BASE_CNT 1 +#ifdef CONFIG_ARM +/* Use values higher than ARM arch timer. See 6282edb72bed. */ +#define MCT_CLKSOURCE_RATING 450 +#define MCT_CLKEVENTS_RATING 500 +#else +#define MCT_CLKSOURCE_RATING 350 +#define MCT_CLKEVENTS_RATING 350 +#endif +enum {MCT_INT_SPI ,MCT_INT_PPI
@@ -206,7 +215,7 @@ static void exynos4_frc_resume(struct clocksource *cs)static struct clocksource mct_frc = { .name ="mct-frc" , - .rating =450 ,/* use value higher than ARM arch timer */ + .rating =MCT_CLKSOURCE_RATING , .read = exynos4_frc_read, .mask =CLOCKSOURCE_MASK (32 ), .flags =CLOCK_SOURCE_IS_CONTINUOUS ,
@@ -457,7 +466,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 =500 ;/* use value higher than ARM arch timer */ + evt->rating =MCT_CLKEVENTS_RATING ,exynos4_mct_write (TICK_BASE_CNT , mevt->base +MCT_L_TCNTB_OFFSET );
Signature · 1 line
2.32.0.rc1.229.g3e70b5a671-goog
Re: [PATCH 1/2] clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64
Quoted 32 lines · Will Deacon
All arm64 CPUs feature an architected timer, which offers a relatively low-latency interface to a per-cpu clocksource and timer. For the most part, using this interface is a no-brainer, with the exception of SoCs where it cannot be used to wake up from deep idle state (i.e.
CLOCK_EVT_FEAT_C3STOPis set).On the contrary, the Exynos MCT is extremely slow to access yet can be used as a wakeup source. In preparation for using the Exynos MCT as a potential wakeup timer for the Arm architected timer, reduce its ratings so that the architected timer is preferred.
This effectively reverts the decision made in
6282edb72bed("clocksource/drivers/exynos_mct: Increase priority over ARM arch timer") for arm64, as the reasoning for the original change was to work around a 32-bit SoC design.Cc Marek Szyprowski <[email protected]>Cc Krzysztof Kozlowski <[email protected]>Cc Chanwoo Choi <[email protected]>Cc Thomas Gleixner <[email protected]>Signed-off-by Will Deacon <[email protected]>--- drivers/clocksource/exynos_mct.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)drivers/clocksource/exynos_mct.c +0
-0@@ -51,6 +51,15 @@#define TICK_BASE_CNT 1
drivers/clocksource/exynos_mct.c line 62
Quoted 10 lines
drivers/clocksource/exynos_mct.c +9
-0+#ifdef CONFIG_ARM +/* Use values higher than ARM arch timer. See 6282edb72bed. */ +#define MCT_CLKSOURCE_RATING 450 +#define MCT_CLKEVENTS_RATING 500 +#else +#define MCT_CLKSOURCE_RATING 350 +#define MCT_CLKEVENTS_RATING 350 +#endif +
Tested on Exynos5422 (32-bit ARM) and it seems to work. I don't have ARMv8 board to check it. We want Exynos MCT on ARMv8 as well, because unfortunately it shares the block with Arch timer. This was investigated by Marek:
https://lore.kernel.org/linux-samsung-soc/20181017123622eucas1p14654c89a8590fd094d638b60ab9af8f0~eZY3j27rs0422004220eucas1p1M@eucas1p1.samsung.com/
Best regards,
Krzysztof
Re: [PATCH 1/2] clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64
In reply to Will DeaconHi,
Quoted 56 lines · Will Deacon
All arm64 CPUs feature an architected timer, which offers a relatively low-latency interface to a per-cpu clocksource and timer. For the most part, using this interface is a no-brainer, with the exception of SoCs where it cannot be used to wake up from deep idle state (i.e.
CLOCK_EVT_FEAT_C3STOPis set).On the contrary, the Exynos MCT is extremely slow to access yet can be used as a wakeup source. In preparation for using the Exynos MCT as a potential wakeup timer for the Arm architected timer, reduce its ratings so that the architected timer is preferred.
This effectively reverts the decision made in
6282edb72bed("clocksource/drivers/exynos_mct: Increase priority over ARM arch timer") for arm64, as the reasoning for the original change was to work around a 32-bit SoC design.Cc Marek Szyprowski <[email protected]>Cc Krzysztof Kozlowski <[email protected]>Cc Chanwoo Choi <[email protected]>Cc Thomas Gleixner <[email protected]>Signed-off-by Will Deacon <[email protected]>--- drivers/clocksource/exynos_mct.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)drivers/clocksource/exynos_mct.c +10
-1@@ -51,6 +51,15 @@#define TICK_BASE_CNT 1 +#ifdef CONFIG_ARM +/* Use values higher than ARM arch timer. See 6282edb72bed. */ +#define MCT_CLKSOURCE_RATING 450 +#define MCT_CLKEVENTS_RATING 500 +#else +#define MCT_CLKSOURCE_RATING 350 +#define MCT_CLKEVENTS_RATING 350 +#endif +enum {MCT_INT_SPI ,MCT_INT_PPI @@ -206,7 +215,7 @@ static void exynos4_frc_resume(struct clocksource *cs)static struct clocksource mct_frc = { .name ="mct-frc" , - .rating =450 ,/* use value higher than ARM arch timer */ + .rating =MCT_CLKSOURCE_RATING , .read = exynos4_frc_read, .mask =CLOCKSOURCE_MASK (32 ), .flags =CLOCK_SOURCE_IS_CONTINUOUS ,
drivers/clocksource/exynos_mct.c static int exynos4_mct_starting_cpu(unsigned int cpu) · line 472
Quoted 8 lines
drivers/clocksource/exynos_mct.c +1
-1@@ -457,7 +466,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 =500 ;/* use value higher than ARM arch timer */ + evt->rating =MCT_CLKEVENTS_RATING ,exynos4_mct_write (TICK_BASE_CNT , mevt->base +MCT_L_TCNTB_OFFSET );
I'm not sure that exynos mct is working without problem such as the case of 6282edb72bed.
As described on On ,6282edb72bed the arch timer on exynos SoC depends on Exynos MCT device. the arch timer is not able to work without Exynos MCT because of using the common module.
Signature · 3 lines
Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH 1/2] clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64
Quoted 64 lines · Chanwoo Choi
Hi,
Quoted 61 lines · Will Deacon
All arm64 CPUs feature an architected timer, which offers a relatively low-latency interface to a per-cpu clocksource and timer. For the most part, using this interface is a no-brainer, with the exception of SoCs where it cannot be used to wake up from deep idle state (i.e.
CLOCK_EVT_FEAT_C3STOPis set).On the contrary, the Exynos MCT is extremely slow to access yet can be used as a wakeup source. In preparation for using the Exynos MCT as a potential wakeup timer for the Arm architected timer, reduce its ratings so that the architected timer is preferred.
This effectively reverts the decision made in
6282edb72bed("clocksource/drivers/exynos_mct: Increase priority over ARM arch timer") for arm64, as the reasoning for the original change was to work around a 32-bit SoC design.Cc Marek Szyprowski <[email protected]>Cc Krzysztof Kozlowski <[email protected]>Cc Chanwoo Choi <[email protected]>Cc Thomas Gleixner <[email protected]>Signed-off-by Will Deacon <[email protected]>--- drivers/clocksource/exynos_mct.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)drivers/clocksource/exynos_mct.c +11
-2@@ -51,6 +51,15 @@#define TICK_BASE_CNT 1 +#ifdef CONFIG_ARM +/* Use values higher than ARM arch timer. See 6282edb72bed. */ +#define MCT_CLKSOURCE_RATING 450 +#define MCT_CLKEVENTS_RATING 500 +#else +#define MCT_CLKSOURCE_RATING 350 +#define MCT_CLKEVENTS_RATING 350 +#endif +enum {MCT_INT_SPI ,MCT_INT_PPI @@ -206,7 +215,7 @@ static void exynos4_frc_resume(struct clocksource *cs)static struct clocksource mct_frc = { .name ="mct-frc" , - .rating =450 ,/* use value higher than ARM arch timer */ + .rating =MCT_CLKSOURCE_RATING , .read = exynos4_frc_read, .mask =CLOCKSOURCE_MASK (32 ), .flags =CLOCK_SOURCE_IS_CONTINUOUS ,@@ -457,7 +466,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 =500 ;/* use value higher than ARM arch timer */ + evt->rating = MCT_CLKEVENTS_RATING,
Quoted 9 lines
drivers/clocksource/exynos_mct.c static int exynos4_mct_starting_cpu(unsigned int cpu) · line 472
Quoted 3 lines
drivers/clocksource/exynos_mct.c +0
-0exynos4_mct_write (TICK_BASE_CNT , mevt->base +MCT_L_TCNTB_OFFSET );I'm not sure that exynos mct is working without problem such as the case of
6282edb72bed.
As described on On ,6282edb72bed the arch timer on exynos SoC depends on Exynos MCT device. the arch timer is not able to work without Exynos MCT because of using the common module.
Is it possible to change the DT to have a phandle to the exynos_mct, so it will be probed before the arch_arm_timer ?
Signature · 5 lines
<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: [PATCH 1/2] clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64
Quoted 75 lines · Daniel Lezcano
Quoted 64 lines · Chanwoo Choi
Hi,
Quoted 61 lines · Will Deacon
All arm64 CPUs feature an architected timer, which offers a relatively low-latency interface to a per-cpu clocksource and timer. For the most part, using this interface is a no-brainer, with the exception of SoCs where it cannot be used to wake up from deep idle state (i.e.
CLOCK_EVT_FEAT_C3STOPis set).On the contrary, the Exynos MCT is extremely slow to access yet can be used as a wakeup source. In preparation for using the Exynos MCT as a potential wakeup timer for the Arm architected timer, reduce its ratings so that the architected timer is preferred.
This effectively reverts the decision made in
6282edb72bed("clocksource/drivers/exynos_mct: Increase priority over ARM arch timer") for arm64, as the reasoning for the original change was to work around a 32-bit SoC design.Cc Marek Szyprowski <[email protected]>Cc Krzysztof Kozlowski <[email protected]>Cc Chanwoo Choi <[email protected]>Cc Thomas Gleixner <[email protected]>Signed-off-by Will Deacon <[email protected]>--- drivers/clocksource/exynos_mct.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)drivers/clocksource/exynos_mct.c +11
-2@@ -51,6 +51,15 @@#define TICK_BASE_CNT 1 +#ifdef CONFIG_ARM +/* Use values higher than ARM arch timer. See 6282edb72bed. */ +#define MCT_CLKSOURCE_RATING 450 +#define MCT_CLKEVENTS_RATING 500 +#else +#define MCT_CLKSOURCE_RATING 350 +#define MCT_CLKEVENTS_RATING 350 +#endif +enum {MCT_INT_SPI ,MCT_INT_PPI @@ -206,7 +215,7 @@ static void exynos4_frc_resume(struct clocksource *cs)static struct clocksource mct_frc = { .name ="mct-frc" , - .rating =450 ,/* use value higher than ARM arch timer */ + .rating =MCT_CLKSOURCE_RATING , .read = exynos4_frc_read, .mask =CLOCKSOURCE_MASK (32 ), .flags =CLOCK_SOURCE_IS_CONTINUOUS ,@@ -457,7 +466,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 =500 ;/* use value higher than ARM arch timer */ + evt->rating = MCT_CLKEVENTS_RATING,Quoted 9 lines
drivers/clocksource/exynos_mct.c static int exynos4_mct_starting_cpu(unsigned int cpu) · line 472
Quoted 3 lines
drivers/clocksource/exynos_mct.c +0
-0exynos4_mct_write (TICK_BASE_CNT , mevt->base +MCT_L_TCNTB_OFFSET );I'm not sure that exynos mct is working without problem such as the case of
6282edb72bed.
As described on On ,6282edb72bed the arch timer on exynos SoC depends on Exynos MCT device. the arch timer is not able to work without Exynos MCT because of using the common module.
Quoted 2 lines
Is it possible to change the DT to have a phandle to the
exynos_mct, so it will be probed before thearch_arm_timer?
I think that DT changes is not proper way to keep the order between exynos_mct and arch timer.
Signature · 3 lines
Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH 1/2] clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64
Quoted 49 lines · Chanwoo Choi
Quoted 48 lines · Daniel Lezcano
Quoted 46 lines · Chanwoo Choi
Quoted 31 lines · Will Deacon
drivers/clocksource/exynos_mct.c +10
-1@@ -51,6 +51,15 @@#define TICK_BASE_CNT 1 +#ifdef CONFIG_ARM +/* Use values higher than ARM arch timer. See 6282edb72bed. */ +#define MCT_CLKSOURCE_RATING 450 +#define MCT_CLKEVENTS_RATING 500 +#else +#define MCT_CLKSOURCE_RATING 350 +#define MCT_CLKEVENTS_RATING 350 +#endif +enum {MCT_INT_SPI ,MCT_INT_PPI @@ -206,7 +215,7 @@ static void exynos4_frc_resume(struct clocksource *cs)static struct clocksource mct_frc = { .name ="mct-frc" , - .rating =450 ,/* use value higher than ARM arch timer */ + .rating =MCT_CLKSOURCE_RATING , .read = exynos4_frc_read, .mask =CLOCKSOURCE_MASK (32 ), .flags =CLOCK_SOURCE_IS_CONTINUOUS ,drivers/clocksource/exynos_mct.c static int exynos4_mct_starting_cpu(unsigned int cpu) · line 472
Quoted 8 lines
drivers/clocksource/exynos_mct.c +1
-1@@ -457,7 +466,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 =500 ;/* use value higher than ARM arch timer */ + evt->rating =MCT_CLKEVENTS_RATING ,exynos4_mct_write (TICK_BASE_CNT , mevt->base +MCT_L_TCNTB_OFFSET );I'm not sure that exynos mct is working without problem such as the case of
6282edb72bed.
As described on On ,6282edb72bed the arch timer on exynos SoC depends on Exynos MCT device. the arch timer is not able to work without Exynos MCT because of using the common module.
Quoted 5 lines
Quoted 2 lines
Is it possible to change the DT to have a phandle to the
exynos_mct, so it will be probed before thearch_arm_timer?I think that DT changes is not proper way to keep the order between
exynos_mctand arch timer.
exynos4_mct_frc_start() is called unconditionally from probe via exynos4_clocksource_init() so as long as the mct probes first, then the arch timer should work, no? The rating shouldn't affect that.
Will
Re: [PATCH 1/2] clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64
Quoted 53 lines · Will Deacon
Quoted 52 lines · Chanwoo Choi
Quoted 50 lines · Daniel Lezcano
Quoted 37 lines · Chanwoo Choi
Quoted 36 lines · Will Deacon
drivers/clocksource/exynos_mct.c +11
-2@@ -51,6 +51,15 @@#define TICK_BASE_CNT 1 +#ifdef CONFIG_ARM +/* Use values higher than ARM arch timer. See 6282edb72bed. */ +#define MCT_CLKSOURCE_RATING 450 +#define MCT_CLKEVENTS_RATING 500 +#else +#define MCT_CLKSOURCE_RATING 350 +#define MCT_CLKEVENTS_RATING 350 +#endif +enum {MCT_INT_SPI ,MCT_INT_PPI @@ -206,7 +215,7 @@ static void exynos4_frc_resume(struct clocksource *cs)static struct clocksource mct_frc = { .name ="mct-frc" , - .rating =450 ,/* use value higher than ARM arch timer */ + .rating =MCT_CLKSOURCE_RATING , .read = exynos4_frc_read, .mask =CLOCKSOURCE_MASK (32 ), .flags =CLOCK_SOURCE_IS_CONTINUOUS ,@@ -457,7 +466,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 =500 ;/* use value higher than ARM arch timer */ + evt->rating = MCT_CLKEVENTS_RATING,Quoted 9 lines
drivers/clocksource/exynos_mct.c static int exynos4_mct_starting_cpu(unsigned int cpu) · line 472
Quoted 3 lines
drivers/clocksource/exynos_mct.c +0
-0exynos4_mct_write (TICK_BASE_CNT , mevt->base +MCT_L_TCNTB_OFFSET );I'm not sure that exynos mct is working without problem such as the case of
6282edb72bed.
As described on On ,6282edb72bed the arch timer on exynos SoC depends on Exynos MCT device. the arch timer is not able to work without Exynos MCT because of using the common module.Is it possible to change the DT to have a phandle to the
exynos_mct, so it will be probed before thearch_arm_timer?
Quoted 6 lines
Quoted 2 lines
I think that DT changes is not proper way to keep the order between
exynos_mctand arch timer.
exynos4_mct_frc_start()is called unconditionally from probe viaexynos4_clocksource_init()so as long as the mct probes first, then the arch timer should work, no? The rating shouldn't affect that.
How do you ensure the exynos mct is probed before the arch timer ?
The Makefile provides the right order, but the dependency is implicit.
Signature · 5 lines
<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: [PATCH 1/2] clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64
Quoted 57 lines · Daniel Lezcano
Quoted 56 lines · Will Deacon
Quoted 54 lines · Chanwoo Choi
Quoted 48 lines · Daniel Lezcano
Quoted 37 lines · Chanwoo Choi
Quoted 36 lines · Will Deacon
drivers/clocksource/exynos_mct.c +11
-2@@ -51,6 +51,15 @@#define TICK_BASE_CNT 1 +#ifdef CONFIG_ARM +/* Use values higher than ARM arch timer. See 6282edb72bed. */ +#define MCT_CLKSOURCE_RATING 450 +#define MCT_CLKEVENTS_RATING 500 +#else +#define MCT_CLKSOURCE_RATING 350 +#define MCT_CLKEVENTS_RATING 350 +#endif +enum {MCT_INT_SPI ,MCT_INT_PPI @@ -206,7 +215,7 @@ static void exynos4_frc_resume(struct clocksource *cs)static struct clocksource mct_frc = { .name ="mct-frc" , - .rating =450 ,/* use value higher than ARM arch timer */ + .rating =MCT_CLKSOURCE_RATING , .read = exynos4_frc_read, .mask =CLOCKSOURCE_MASK (32 ), .flags =CLOCK_SOURCE_IS_CONTINUOUS ,@@ -457,7 +466,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 =500 ;/* use value higher than ARM arch timer */ + evt->rating = MCT_CLKEVENTS_RATING,Quoted 9 lines
drivers/clocksource/exynos_mct.c static int exynos4_mct_starting_cpu(unsigned int cpu) · line 472
Quoted 3 lines
drivers/clocksource/exynos_mct.c +0
-0exynos4_mct_write (TICK_BASE_CNT , mevt->base +MCT_L_TCNTB_OFFSET );I'm not sure that exynos mct is working without problem such as the case of
6282edb72bed.
As described on On ,6282edb72bed the arch timer on exynos SoC depends on Exynos MCT device. the arch timer is not able to work without Exynos MCT because of using the common module.Quoted 2 lines
Is it possible to change the DT to have a phandle to the
exynos_mct, so it will be probed before thearch_arm_timer?I think that DT changes is not proper way to keep the order between
exynos_mctand arch timer.
Quoted 7 lines
Quoted 3 lines
exynos4_mct_frc_start()is called unconditionally from probe viaexynos4_clocksource_init()so as long as the mct probes first, then the arch timer should work, no? The rating shouldn't affect that.How do you ensure the exynos mct is probed before the arch timer ?
The
Makefileprovides the right order, but the dependency is implicit.
Currently, I think it's done by the order of the CPU hotplug notifiers ( see the hunk of 6282edb72bed which touches cpuhotplug.h).
Will
Re: [PATCH 1/2] clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64
On 21/06/2021 12:10, Will Deacon wrote:
[ ... ]
Quoted 10 lines
Quoted 7 lines
Quoted 3 lines
exynos4_mct_frc_start()is called unconditionally from probe viaexynos4_clocksource_init()so as long as the mct probes first, then the arch timer should work, no? The rating shouldn't affect that.How do you ensure the exynos mct is probed before the arch timer ?
The
Makefileprovides the right order, but the dependency is implicit.Currently, I think it's done by the order of the CPU hotplug notifiers ( see the hunk of
6282edb72bedwhich touchescpuhotplug.h).
Ah, right. Indeed whatever the DT order, the cpuhotplug order solves the dependency.
Chanwoo, are fine with this change ?
Signature · 5 lines
<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: [PATCH 1/2] clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64
Quoted 12 lines · Daniel Lezcano
On 21/06/2021 12:10, Will Deacon wrote:
[ ... ]
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
exynos4_mct_frc_start()is called unconditionally from probe viaexynos4_clocksource_init()so as long as the mct probes first, then the arch timer should work, no? The rating shouldn't affect that.How do you ensure the exynos mct is probed before the arch timer ?
The
Makefileprovides the right order, but the dependency is implicit.
Quoted 7 lines
Quoted 2 lines
Currently, I think it's done by the order of the CPU hotplug notifiers ( see the hunk of
6282edb72bedwhich touchescpuhotplug.h).Ah, right. Indeed whatever the DT order, the cpuhotplug order solves the dependency.
Chanwoo, are fine with this change ?
OK about the order.
Actually, I have not fully tested the arch timer on Exynos5433 64bit because of the dependency between arch timer and MCT as we knew.
If the Krzysztof and Marek have no any objection,
I have no any objection anymore. Thanks.
Signature · 3 lines
Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH 1/2] clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64
Quoted 21 lines · Chanwoo Choi
Quoted 12 lines · Daniel Lezcano
On 21/06/2021 12:10, Will Deacon wrote:
[ ... ]
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
exynos4_mct_frc_start()is called unconditionally from probe viaexynos4_clocksource_init()so as long as the mct probes first, then the arch timer should work, no? The rating shouldn't affect that.How do you ensure the exynos mct is probed before the arch timer ?
The
Makefileprovides the right order, but the dependency is implicit.Quoted 7 lines
Quoted 2 lines
Currently, I think it's done by the order of the CPU hotplug notifiers ( see the hunk of
6282edb72bedwhich touchescpuhotplug.h).Ah, right. Indeed whatever the DT order, the cpuhotplug order solves the dependency.
Chanwoo, are fine with this change ?
Quoted 8 lines
OK about the order.
Actually, I have not fully tested the arch timer on Exynos5433 64bit because of the dependency between arch timer and MCT as we knew.
If the Krzysztof and Marek have no any objection,
I have no any objection anymore. Thanks.
Shall I consider it as an Acked-by ?
Signature · 5 lines
<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: [PATCH 1/2] clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64
Quoted 24 lines · Daniel Lezcano
Quoted 23 lines · Chanwoo Choi
Quoted 12 lines · Daniel Lezcano
On 21/06/2021 12:10, Will Deacon wrote:
[ ... ]
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
exynos4_mct_frc_start()is called unconditionally from probe viaexynos4_clocksource_init()so as long as the mct probes first, then the arch timer should work, no? The rating shouldn't affect that.How do you ensure the exynos mct is probed before the arch timer ?
The
Makefileprovides the right order, but the dependency is implicit.Quoted 7 lines
Quoted 2 lines
Currently, I think it's done by the order of the CPU hotplug notifiers ( see the hunk of
6282edb72bedwhich touchescpuhotplug.h).Ah, right. Indeed whatever the DT order, the cpuhotplug order solves the dependency.
Chanwoo, are fine with this change ?
OK about the order.
Quoted 9 lines
Quoted 6 lines
Actually, I have not fully tested the arch timer on Exynos5433 64bit because of the dependency between arch timer and MCT as we knew.
If the Krzysztof and Marek have no any objection,
I have no any objection anymore. Thanks.Shall I consider it as an Acked-by ?
Unfortunately, it is not acked. Just no objection.
I'm not sure that all cases will be working when using arch timer because as I said, I have only used the exynos mct timer for all of cases.
Signature · 3 lines
Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH 1/2] clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64
In reply to Daniel LezcanoHi Daniel,
Quoted 51 lines · Daniel Lezcano
Quoted 50 lines · Will Deacon
Quoted 49 lines · Chanwoo Choi
Quoted 48 lines · Daniel Lezcano
Quoted 37 lines · Chanwoo Choi
Quoted 36 lines · Will Deacon
drivers/clocksource/exynos_mct.c +11
-2@@ -51,6 +51,15 @@#define TICK_BASE_CNT 1 +#ifdef CONFIG_ARM +/* Use values higher than ARM arch timer. See 6282edb72bed. */ +#define MCT_CLKSOURCE_RATING 450 +#define MCT_CLKEVENTS_RATING 500 +#else +#define MCT_CLKSOURCE_RATING 350 +#define MCT_CLKEVENTS_RATING 350 +#endif +enum {MCT_INT_SPI ,MCT_INT_PPI @@ -206,7 +215,7 @@ static void exynos4_frc_resume(struct clocksource *cs)static struct clocksource mct_frc = { .name ="mct-frc" , - .rating =450 ,/* use value higher than ARM arch timer */ + .rating =MCT_CLKSOURCE_RATING , .read = exynos4_frc_read, .mask =CLOCKSOURCE_MASK (32 ), .flags =CLOCK_SOURCE_IS_CONTINUOUS ,@@ -457,7 +466,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 =500 ;/* use value higher than ARM arch timer */ + evt->rating = MCT_CLKEVENTS_RATING,Quoted 8 lines
drivers/clocksource/exynos_mct.c static int exynos4_mct_starting_cpu(unsigned int cpu) · line 472
Quoted 3 lines
drivers/clocksource/exynos_mct.c +0
-0exynos4_mct_write (TICK_BASE_CNT , mevt->base +MCT_L_TCNTB_OFFSET );I'm not sure that exynos mct is working without problem such as the case of
6282edb72bed.
As described on On ,6282edb72bed the arch timer on exynos SoC depends on Exynos MCT device. the arch timer is not able to work without Exynos MCT because of using the common module.Is it possible to change the DT to have a phandle to the
exynos_mct, so it will be probed before thearch_arm_timer?
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
I think that DT changes is not proper way to keep the order between
exynos_mctand arch timer.
exynos4_mct_frc_start()is called unconditionally from probe viaexynos4_clocksource_init()so as long as the mct probes first, then the arch timer should work, no? The rating shouldn't affect that.How do you ensure the exynos mct is probed before the arch timer ?
The
Makefileprovides the right order, but the dependency is implicit.
In my test systems based on Exynos5433, the bootloader enabled MCT, so kernel can also start with arch_timer then switch to MCT. The probe order also depend on the order of DT nodes, so with the current dts MCT is probed first. This is not the best approach, but so far it works.
Best regards
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland
Re: [PATCH 1/2] clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64
On 21/06/2021 12:17, Marek Szyprowski wrote:
[ ... ]
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 9 lines
Quoted 7 lines
Quoted 5 lines
I'm not sure that exynos mct is working without problem such as the case of
6282edb72bed.
As described on On ,6282edb72bed the arch timer on exynos SoC depends on Exynos MCT device. the arch timer is not able to work without Exynos MCT because of using the common module.Is it possible to change the DT to have a phandle to the
exynos_mct, so it will be probed before thearch_arm_timer?I think that DT changes is not proper way to keep the order between
exynos_mctand arch timer.
exynos4_mct_frc_start()is called unconditionally from probe viaexynos4_clocksource_init()so as long as the mct probes first, then the arch timer should work, no? The rating shouldn't affect that.
Quoted 8 lines
Quoted 3 lines
How do you ensure the exynos mct is probed before the arch timer ?
The
Makefileprovides the right order, but the dependency is implicit.In my test systems based on Exynos5433, the bootloader enabled MCT, so kernel can also start with
arch_timerthen switch to MCT. The probe order also depend on the order of DT nodes, so with the current dts MCT is probed first. This is not the best approach, but so far it works.
Right, generally assuming the firmware is doing the right thing from an initialization POV is not a good idea :/
But in our case, as pointed by Will, the cpuhotplug ordering gives the right sequence. So it should be fine.
Signature · 5 lines
<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
[tip: timers/core] clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64
In reply to Will DeaconThe following commit has been merged into the timers/core branch of tip:
The following commit has been merged into the timers/core branch of tip:
Commit-ID: ae460fd9164b16654d8ec06cbc280b832f840eac Gitweb: https://git.kernel.org/tip/ae460fd9164b16654d8ec06cbc280b832f840eac Author: Will Deacon <[email protected]> AuthorDate: Tue, 08 Jun 2021 16:43:40 +01:00 Committer: Daniel Lezcano <[email protected]> CommitterDate: Fri, 13 Aug 2021 09:24:22 +02:00
clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64
All arm64 CPUs feature an architected timer, which offers a relatively low-latency interface to a per-cpu clocksource and timer. For the most part, using this interface is a no-brainer, with the exception of SoCs where it cannot be used to wake up from deep idle state (i.e. CLOCK_EVT_FEAT_C3STOP is set).
On the contrary, the Exynos MCT is extremely slow to access yet can be used as a wakeup source. In preparation for using the Exynos MCT as a potential wakeup timer for the Arm architected timer, reduce its ratings so that the architected timer is preferred.
This effectively reverts the decision made in 6282edb72bed ("clocksource/drivers/exynos_mct: Increase priority over ARM arch timer") for arm64, as the reasoning for the original change was to work around a 32-bit SoC design.
--- drivers/clocksource/exynos_mct.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
drivers/clocksource/exynos_mct.c +11 -2
@@ -51,6 +51,15 @@#define TICK_BASE_CNT 1 +#ifdef CONFIG_ARM +/* Use values higher than ARM arch timer. See 6282edb72bed. */ +#define MCT_CLKSOURCE_RATING 450 +#define MCT_CLKEVENTS_RATING 500 +#else +#define MCT_CLKSOURCE_RATING 350 +#define MCT_CLKEVENTS_RATING 350 +#endif +enum {MCT_INT_SPI ,MCT_INT_PPI
@@ -206,7 +215,7 @@ static void exynos4_frc_resume(struct clocksource *cs)static struct clocksource mct_frc = { .name ="mct-frc" , - .rating =450 ,/* use value higher than ARM arch timer */ + .rating =MCT_CLKSOURCE_RATING , .read = exynos4_frc_read, .mask =CLOCKSOURCE_MASK (32 ), .flags =CLOCK_SOURCE_IS_CONTINUOUS ,
@@ -457,7 +466,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 =500 ;/* use value higher than ARM arch timer */ + evt->rating =MCT_CLKEVENTS_RATING ,exynos4_mct_write (TICK_BASE_CNT , mevt->base +MCT_L_TCNTB_OFFSET );
PATCH 2/2 clocksource/drivers/exynos_mct: Mark MCT device as CLOCK_EVT_FEAT_PERCPU
In reply to Will DeaconThe "mct_tick" is a per-cpu clockevents device. Set the CLOCK_EVT_FEAT_PERCPU feature to prevent e.g. mct_tick0 being unsafely designated as the global broadcast timer and instead treat the device as a per-cpu wakeup timer.
--- drivers/clocksource/exynos_mct.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/clocksource/exynos_mct.c +2 -1
@@ -465,7 +465,8 @@ static int exynos4_mct_starting_cpu(unsigned int cpu) evt->set_state_oneshot = set_state_shutdown; 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->features =CLOCK_EVT_FEAT_PERIODIC |CLOCK_EVT_FEAT_ONESHOT | +CLOCK_EVT_FEAT_PERCPU ; evt->rating =MCT_CLKEVENTS_RATING ,exynos4_mct_write (TICK_BASE_CNT , mevt->base +MCT_L_TCNTB_OFFSET );
Signature · 1 line
2.32.0.rc1.229.g3e70b5a671-goog
Re: [PATCH 2/2] clocksource/drivers/exynos_mct: Mark MCT device as CLOCK_EVT_FEAT_PERCPU
Quoted 5 lines · Will Deacon
The "
mct_tick" is a per-cpu clockevents device. Set theCLOCK_EVT_FEAT_PERCPUfeature to prevent e.g.mct_tick0being unsafely designated as the global broadcast timer and instead treat the device as a per-cpu wakeup timer.
Quoted 7 lines
Cc Daniel Lezcano <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Krzysztof Kozlowski <[email protected]>Signed-off-by Will Deacon <[email protected]>--- drivers/clocksource/exynos_mct.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Best regards,
Krzysztof
Re: [PATCH 2/2] clocksource/drivers/exynos_mct: Mark MCT device as CLOCK_EVT_FEAT_PERCPU
In reply to Will DeaconQuoted 19 lines · Will Deacon
The "
mct_tick" is a per-cpu clockevents device. Set theCLOCK_EVT_FEAT_PERCPUfeature to prevent e.g.mct_tick0being unsafely designated as the global broadcast timer and instead treat the device as a per-cpu wakeup timer.Cc Daniel Lezcano <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Krzysztof Kozlowski <[email protected]>Signed-off-by Will Deacon <[email protected]>--- drivers/clocksource/exynos_mct.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/clocksource/exynos_mct.c +0
-0
drivers/clocksource/exynos_mct.c static int exynos4_mct_starting_cpu(unsigned int cpu) · line 472
Quoted 9 lines
drivers/clocksource/exynos_mct.c +2
-1@@ -465,7 +465,8 @@ static int exynos4_mct_starting_cpu(unsigned int cpu) evt->set_state_oneshot = set_state_shutdown; 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->features =CLOCK_EVT_FEAT_PERIODIC |CLOCK_EVT_FEAT_ONESHOT | +CLOCK_EVT_FEAT_PERCPU ; evt->rating =MCT_CLKEVENTS_RATING ,exynos4_mct_write (TICK_BASE_CNT , mevt->base +MCT_L_TCNTB_OFFSET );
Signature · 3 lines
Best Regards, Chanwoo Choi Samsung Electronics
[tip: timers/core] clocksource/drivers/exynos_mct: Mark MCT device as CLOCK_EVT_FEAT_PERCPU
In reply to Will DeaconThe following commit has been merged into the timers/core branch of tip:
The following commit has been merged into the timers/core branch of tip:
Commit-ID: 88183788eacb782eb6e1295f1934fb9531b503d6 Gitweb: https://git.kernel.org/tip/88183788eacb782eb6e1295f1934fb9531b503d6 Author: Will Deacon <[email protected]> AuthorDate: Tue, 08 Jun 2021 16:43:41 +01:00 Committer: Daniel Lezcano <[email protected]> CommitterDate: Fri, 13 Aug 2021 09:24:22 +02:00
clocksource/drivers/exynos_mct: Mark MCT device as CLOCK_EVT_FEAT_PERCPU
The "mct_tick" is a per-cpu clockevents device. Set the CLOCK_EVT_FEAT_PERCPU feature to prevent e.g. mct_tick0 being unsafely designated as the global broadcast timer and instead treat the device as a per-cpu wakeup timer.
--- drivers/clocksource/exynos_mct.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/clocksource/exynos_mct.c +2 -1
@@ -465,7 +465,8 @@ static int exynos4_mct_starting_cpu(unsigned int cpu) evt->set_state_oneshot = set_state_shutdown; 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->features =CLOCK_EVT_FEAT_PERIODIC |CLOCK_EVT_FEAT_ONESHOT | +CLOCK_EVT_FEAT_PERCPU ; evt->rating =MCT_CLKEVENTS_RATING ,exynos4_mct_write (TICK_BASE_CNT , mevt->base +MCT_L_TCNTB_OFFSET );