[PATCH] ARM: Drop fixed 200 Hz timer requirement from Exynos platforms

[PATCH] ARM: Drop fixed 200 Hz timer requirement from Exynos platforms

[Krzysztof Kozlowski]

All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
200 Hz.  Unfortunately in case of multiplatform image this affects also
other platforms when Exynos is selected.

This looks like an very old legacy code, dating back to initial
upstreaming of S3C24xx.  Probably it was required for s3c24xx timer
driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
unused plat-samsung/time.c").

Since then, this fixed 200 Hz spread everywhere, including out-of-tree
Samsung kernels (SoC vendor's and Tizen's).  I believe this choice
was rather an effect of coincidence instead of conscious choice.  In
fact Exynos can work with different timers.

Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
other values.

Reported-by: Lee Jones <lee.jones@linaro.org>
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

---

Testing on other Exynos platforms would be appreciated.
---
 arch/arm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b5d529fdffab..0d10e45f9175 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1497,7 +1497,7 @@ source kernel/Kconfig.preempt
 config HZ_FIXED
 	int
 	default 200 if ARCH_EBSA110 || ARCH_S3C24XX || \
-		ARCH_S5PV210 || ARCH_EXYNOS4
+		ARCH_S5PV210
 	default 128 if SOC_AT91RM9200
 	default 0
 
-- 
2.7.4

[PATCH] ARM: Drop fixed 200 Hz timer requirement from Exynos platforms

[Krzysztof Kozlowski]

All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
200 Hz.  Unfortunately in case of multiplatform image this affects also
other platforms when Exynos is selected.

This looks like an very old legacy code, dating back to initial
upstreaming of S3C24xx.  Probably it was required for s3c24xx timer
driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
unused plat-samsung/time.c").

Since then, this fixed 200 Hz spread everywhere, including out-of-tree
Samsung kernels (SoC vendor's and Tizen's).  I believe this choice
was rather an effect of coincidence instead of conscious choice.  In
fact Exynos can work with different timers.

Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
other values.

Reported-by: Lee Jones <lee.jones@linaro.org>
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

---

Testing on other Exynos platforms would be appreciated.
---
 arch/arm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b5d529fdffab..0d10e45f9175 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1497,7 +1497,7 @@ source kernel/Kconfig.preempt
 config HZ_FIXED
 	int
 	default 200 if ARCH_EBSA110 || ARCH_S3C24XX || \
-		ARCH_S5PV210 || ARCH_EXYNOS4
+		ARCH_S5PV210
 	default 128 if SOC_AT91RM9200
 	default 0
 
-- 
2.7.4

Re: [PATCH] ARM: Drop fixed 200 Hz timer requirement from Exynos platforms

[Arnd Bergmann]

On Monday, November 14, 2016 8:27:05 PM CET Krzysztof Kozlowski wrote:
> @@ -1497,7 +1497,7 @@ source kernel/Kconfig.preempt
>  config HZ_FIXED
>         int
>         default 200 if ARCH_EBSA110 || ARCH_S3C24XX || \
> -               ARCH_S5PV210 || ARCH_EXYNOS4
> +               ARCH_S5PV210
>         default 128 if SOC_AT91RM9200
>         default 0

After further research, I've concluded that we should also drop the
settings for ARCH_S5PV210 and ARCH_S3C24XX here.

ARCH_S5PV210 behaves exactly like EXYNOS here, it has 32-bit timers
so there won't be any overflow with 100Hz.

For ARCH_S3C24XX, it the requirement was that HZ_100 could not
be used with the old arch/arm/plat-samsung/time.c code that would
overflow its 16-bit counter.
However, the new drivers/clocksource/samsung_pwm_timer.c configures
the clock divider to '50' instead of '6', so there is no longer
a 16-bit overflow before the 100Hz tick, it now overflows every
3.7ms for the typical 12MHz clock.

	Arnd

Re: [PATCH] ARM: Drop fixed 200 Hz timer requirement from Exynos platforms

[Arnd Bergmann]

On Monday, November 14, 2016 8:27:05 PM CET Krzysztof Kozlowski wrote:
> @@ -1497,7 +1497,7 @@ source kernel/Kconfig.preempt
>  config HZ_FIXED
>         int
>         default 200 if ARCH_EBSA110 || ARCH_S3C24XX || \
> -               ARCH_S5PV210 || ARCH_EXYNOS4
> +               ARCH_S5PV210
>         default 128 if SOC_AT91RM9200
>         default 0

After further research, I've concluded that we should also drop the
settings for ARCH_S5PV210 and ARCH_S3C24XX here.

ARCH_S5PV210 behaves exactly like EXYNOS here, it has 32-bit timers
so there won't be any overflow with 100Hz.

For ARCH_S3C24XX, it the requirement was that HZ_100 could not
be used with the old arch/arm/plat-samsung/time.c code that would
overflow its 16-bit counter.
However, the new drivers/clocksource/samsung_pwm_timer.c configures
the clock divider to '50' instead of '6', so there is no longer
a 16-bit overflow before the 100Hz tick, it now overflows every
3.7ms for the typical 12MHz clock.

	Arnd

[PATCH] ARM: Drop fixed 200 Hz timer requirement from Exynos platforms

[Arnd Bergmann]

On Monday, November 14, 2016 8:27:05 PM CET Krzysztof Kozlowski wrote:
> @@ -1497,7 +1497,7 @@ source kernel/Kconfig.preempt
>  config HZ_FIXED
>         int
>         default 200 if ARCH_EBSA110 || ARCH_S3C24XX || \
> -               ARCH_S5PV210 || ARCH_EXYNOS4
> +               ARCH_S5PV210
>         default 128 if SOC_AT91RM9200
>         default 0

After further research, I've concluded that we should also drop the
settings for ARCH_S5PV210 and ARCH_S3C24XX here.

ARCH_S5PV210 behaves exactly like EXYNOS here, it has 32-bit timers
so there won't be any overflow with 100Hz.

For ARCH_S3C24XX, it the requirement was that HZ_100 could not
be used with the old arch/arm/plat-samsung/time.c code that would
overflow its 16-bit counter.
However, the new drivers/clocksource/samsung_pwm_timer.c configures
the clock divider to '50' instead of '6', so there is no longer
a 16-bit overflow before the 100Hz tick, it now overflows every
3.7ms for the typical 12MHz clock.

	Arnd

Re: [PATCH] ARM: Drop fixed 200 Hz timer requirement from Exynos platforms

[Javier Martinez Canillas]

Hello Krzysztof,

On 11/14/2016 03:27 PM, Krzysztof Kozlowski wrote:
> All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
> 200 Hz.  Unfortunately in case of multiplatform image this affects also
> other platforms when Exynos is selected.
> 
> This looks like an very old legacy code, dating back to initial
> upstreaming of S3C24xx.  Probably it was required for s3c24xx timer
> driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
> unused plat-samsung/time.c").
> 
> Since then, this fixed 200 Hz spread everywhere, including out-of-tree
> Samsung kernels (SoC vendor's and Tizen's).  I believe this choice
> was rather an effect of coincidence instead of conscious choice.  In
> fact Exynos can work with different timers.
> 
> Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
> A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
> other values.
> 
> Reported-by: Lee Jones <lee.jones@linaro.org>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> ---
> 
> Testing on other Exynos platforms would be appreciated.

I tested this patch on an Exynos5800 Peach Pi Chromebook with different
HZ values (100/200/250/300/500/1000), and found no issues.

Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH] ARM: Drop fixed 200 Hz timer requirement from Exynos platforms

[Javier Martinez Canillas]

Hello Krzysztof,

On 11/14/2016 03:27 PM, Krzysztof Kozlowski wrote:
> All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
> 200 Hz.  Unfortunately in case of multiplatform image this affects also
> other platforms when Exynos is selected.
> 
> This looks like an very old legacy code, dating back to initial
> upstreaming of S3C24xx.  Probably it was required for s3c24xx timer
> driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
> unused plat-samsung/time.c").
> 
> Since then, this fixed 200 Hz spread everywhere, including out-of-tree
> Samsung kernels (SoC vendor's and Tizen's).  I believe this choice
> was rather an effect of coincidence instead of conscious choice.  In
> fact Exynos can work with different timers.
> 
> Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
> A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
> other values.
> 
> Reported-by: Lee Jones <lee.jones@linaro.org>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> ---
> 
> Testing on other Exynos platforms would be appreciated.

I tested this patch on an Exynos5800 Peach Pi Chromebook with different
HZ values (100/200/250/300/500/1000), and found no issues.

Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[PATCH] ARM: Drop fixed 200 Hz timer requirement from Exynos platforms

[Javier Martinez Canillas]

Hello Krzysztof,

On 11/14/2016 03:27 PM, Krzysztof Kozlowski wrote:
> All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
> 200 Hz.  Unfortunately in case of multiplatform image this affects also
> other platforms when Exynos is selected.
> 
> This looks like an very old legacy code, dating back to initial
> upstreaming of S3C24xx.  Probably it was required for s3c24xx timer
> driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
> unused plat-samsung/time.c").
> 
> Since then, this fixed 200 Hz spread everywhere, including out-of-tree
> Samsung kernels (SoC vendor's and Tizen's).  I believe this choice
> was rather an effect of coincidence instead of conscious choice.  In
> fact Exynos can work with different timers.
> 
> Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
> A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
> other values.
> 
> Reported-by: Lee Jones <lee.jones@linaro.org>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> ---
> 
> Testing on other Exynos platforms would be appreciated.

I tested this patch on an Exynos5800 Peach Pi Chromebook with different
HZ values (100/200/250/300/500/1000), and found no issues.

Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH] ARM: Drop fixed 200 Hz timer requirement from Exynos platforms

[Krzysztof Kozlowski]

On Thu, Nov 17, 2016 at 01:35:45PM +0100, Arnd Bergmann wrote:
> On Monday, November 14, 2016 8:27:05 PM CET Krzysztof Kozlowski wrote:
> > @@ -1497,7 +1497,7 @@ source kernel/Kconfig.preempt
> >  config HZ_FIXED
> >         int
> >         default 200 if ARCH_EBSA110 || ARCH_S3C24XX || \
> > -               ARCH_S5PV210 || ARCH_EXYNOS4
> > +               ARCH_S5PV210
> >         default 128 if SOC_AT91RM9200
> >         default 0
> 
> After further research, I've concluded that we should also drop the
> settings for ARCH_S5PV210 and ARCH_S3C24XX here.
> 
> ARCH_S5PV210 behaves exactly like EXYNOS here, it has 32-bit timers
> so there won't be any overflow with 100Hz.
> 
> For ARCH_S3C24XX, it the requirement was that HZ_100 could not
> be used with the old arch/arm/plat-samsung/time.c code that would
> overflow its 16-bit counter.
> However, the new drivers/clocksource/samsung_pwm_timer.c configures
> the clock divider to '50' instead of '6', so there is no longer
> a 16-bit overflow before the 100Hz tick, it now overflows every
> 3.7ms for the typical 12MHz clock.

I can send an updated version however testing would be nice... I know
Sylwester has a S3C6410 platform running, maybe S3C24xx as well.

Best regards,
Krzysztof

[PATCH] ARM: Drop fixed 200 Hz timer requirement from Exynos platforms

[Krzysztof Kozlowski]

On Thu, Nov 17, 2016 at 01:35:45PM +0100, Arnd Bergmann wrote:
> On Monday, November 14, 2016 8:27:05 PM CET Krzysztof Kozlowski wrote:
> > @@ -1497,7 +1497,7 @@ source kernel/Kconfig.preempt
> >  config HZ_FIXED
> >         int
> >         default 200 if ARCH_EBSA110 || ARCH_S3C24XX || \
> > -               ARCH_S5PV210 || ARCH_EXYNOS4
> > +               ARCH_S5PV210
> >         default 128 if SOC_AT91RM9200
> >         default 0
> 
> After further research, I've concluded that we should also drop the
> settings for ARCH_S5PV210 and ARCH_S3C24XX here.
> 
> ARCH_S5PV210 behaves exactly like EXYNOS here, it has 32-bit timers
> so there won't be any overflow with 100Hz.
> 
> For ARCH_S3C24XX, it the requirement was that HZ_100 could not
> be used with the old arch/arm/plat-samsung/time.c code that would
> overflow its 16-bit counter.
> However, the new drivers/clocksource/samsung_pwm_timer.c configures
> the clock divider to '50' instead of '6', so there is no longer
> a 16-bit overflow before the 100Hz tick, it now overflows every
> 3.7ms for the typical 12MHz clock.

I can send an updated version however testing would be nice... I know
Sylwester has a S3C6410 platform running, maybe S3C24xx as well.

Best regards,
Krzysztof