2022-10-30 18:50:05

by Johan Jonker

[permalink] [raw]
Subject: [PATCH v1 1/2] arm: rockchip: Kconfig: remove select ARM_GLOBAL_TIMER

The clocksource and the sched_clock provided by the arm_global_timer
on Rockchip rk3066a/rk3188 are quite unstable because their rates
depend on the cpu frequency.

Recent changes to the arm_global_timer driver makes it impossible to use.

On the other side, the arm_global_timer has a higher rating than the
rockchip_timer, it will be selected by default by the time framework
while we want to use the stable rockchip clocksource.

Let's disable the arm_global_timer in order to have the ROCKCHIP_TIMER
(rk3188) or DW_APB_TIMER (rk3066a) selected by default.

Signed-off-by: Johan Jonker <[email protected]>
---

ARM: dts: rockchip: disable arm-global-timer for rk3188
https://lore.kernel.org/linux-rockchip/[email protected]/

clocksource: arm_global_timer: implement rate compensation whenever source clock changes
https://lore.kernel.org/all/[email protected]/
---
arch/arm/mach-rockchip/Kconfig | 2 --
1 file changed, 2 deletions(-)

diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index b7855cc66..0432a4430 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -15,8 +15,6 @@ config ARCH_ROCKCHIP
select DW_APB_TIMER_OF
select REGULATOR if PM
select ROCKCHIP_TIMER
- select ARM_GLOBAL_TIMER
- select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
select ZONE_DMA if ARM_LPAE
select PM
help
--
2.20.1



2022-10-30 18:53:11

by Johan Jonker

[permalink] [raw]
Subject: [PATCH v1 2/2] ARM: dts: rockchip: rk3066a: disable arm_global_timer

The clocksource and the sched_clock provided by the arm_global_timer
on Rockchip rk3066a/rk3188 are quite unstable because their rates
depend on the cpu frequency.

Recent changes to the arm_global_timer driver makes it impossible to use.

On the other side, the arm_global_timer has a higher rating than the
rockchip_timer, it will be selected by default by the time framework
while we want to use the stable rockchip clocksource.

Let's disable the arm_global_timer in order to have the DW_APB_TIMER
(rk3066a) selected by default.

Signed-off-by: Johan Jonker <[email protected]>
---

ARM: dts: rockchip: disable arm-global-timer for rk3188
https://lore.kernel.org/linux-rockchip/[email protected]/

clocksource: arm_global_timer: implement rate compensation whenever source clock changes
https://lore.kernel.org/all/[email protected]/
---
arch/arm/boot/dts/rk3066a.dtsi | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/rk3066a.dtsi b/arch/arm/boot/dts/rk3066a.dtsi
index de9915d94..cf8045625 100644
--- a/arch/arm/boot/dts/rk3066a.dtsi
+++ b/arch/arm/boot/dts/rk3066a.dtsi
@@ -653,6 +653,10 @@
};
};

+&global_timer {
+ status = "disabled";
+};
+
&gpu {
compatible = "rockchip,rk3066-mali", "arm,mali-400";
interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
--
2.20.1


2022-10-30 20:00:44

by Johan Jonker

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] arm: rockchip: Kconfig: remove select ARM_GLOBAL_TIMER



On 10/30/22 20:30, Heiko Stuebner wrote:
> Hi Johan,
>
> Am Sonntag, 30. Oktober 2022, 19:36:01 CET schrieb Johan Jonker:
>> The clocksource and the sched_clock provided by the arm_global_timer
>> on Rockchip rk3066a/rk3188 are quite unstable because their rates
>> depend on the cpu frequency.
>>
>> Recent changes to the arm_global_timer driver makes it impossible to use.
>>
>> On the other side, the arm_global_timer has a higher rating than the
>> rockchip_timer, it will be selected by default by the time framework
>> while we want to use the stable rockchip clocksource.
>>
>> Let's disable the arm_global_timer in order to have the ROCKCHIP_TIMER
>> (rk3188) or DW_APB_TIMER (rk3066a) selected by default.
>
> Doing that change won't help you with the issue you see.
> These days kernel images are supposed to run on as many
> platforms as possible with the _same_ kernel image.
>
> So the muliplatform image build from the kernel's defconfig will
> include most of the time other platforms that will pull in the
> global-timer again.
>
>

How about "imply" instead of "select"?
It's no longer useful. One can get rid of if one wants a really lean kernel.

> I do see that you disable the global-timer
> in the rk3066 dtsi in patch2, though this would leave the rk3188
> still using the global-timer. Why not do this in the rk3xxx.dtsi?
>
> I.e. keep the global-timer node, but add a disabled property
> together with a comment above the status=disabled, describing
> the issue. [This should keep people from re-adding the global-timer
> in the future :-) ]
>
> Thanks
> Heiko
>
>> Signed-off-by: Johan Jonker <[email protected]>
>> ---
>>
>> ARM: dts: rockchip: disable arm-global-timer for rk3188
>> https://lore.kernel.org/linux-rockchip/[email protected]/
>>
>> clocksource: arm_global_timer: implement rate compensation whenever source clock changes
>> https://lore.kernel.org/all/[email protected]/
>> ---
>> arch/arm/mach-rockchip/Kconfig | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
>> index b7855cc66..0432a4430 100644
>> --- a/arch/arm/mach-rockchip/Kconfig
>> +++ b/arch/arm/mach-rockchip/Kconfig
>> @@ -15,8 +15,6 @@ config ARCH_ROCKCHIP
>> select DW_APB_TIMER_OF
>> select REGULATOR if PM
>> select ROCKCHIP_TIMER
>> - select ARM_GLOBAL_TIMER
>> - select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
>> select ZONE_DMA if ARM_LPAE
>> select PM
>> help
>> --
>> 2.20.1
>>
>>
>
>
>
>

2022-10-30 20:01:48

by Heiko Stübner

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] arm: rockchip: Kconfig: remove select ARM_GLOBAL_TIMER

Hi Johan,

Am Sonntag, 30. Oktober 2022, 19:36:01 CET schrieb Johan Jonker:
> The clocksource and the sched_clock provided by the arm_global_timer
> on Rockchip rk3066a/rk3188 are quite unstable because their rates
> depend on the cpu frequency.
>
> Recent changes to the arm_global_timer driver makes it impossible to use.
>
> On the other side, the arm_global_timer has a higher rating than the
> rockchip_timer, it will be selected by default by the time framework
> while we want to use the stable rockchip clocksource.
>
> Let's disable the arm_global_timer in order to have the ROCKCHIP_TIMER
> (rk3188) or DW_APB_TIMER (rk3066a) selected by default.

Doing that change won't help you with the issue you see.
These days kernel images are supposed to run on as many
platforms as possible with the _same_ kernel image.

So the muliplatform image build from the kernel's defconfig will
include most of the time other platforms that will pull in the
global-timer again.


I do see that you disable the global-timer
in the rk3066 dtsi in patch2, though this would leave the rk3188
still using the global-timer. Why not do this in the rk3xxx.dtsi?

I.e. keep the global-timer node, but add a disabled property
together with a comment above the status=disabled, describing
the issue. [This should keep people from re-adding the global-timer
in the future :-) ]

Thanks
Heiko

> Signed-off-by: Johan Jonker <[email protected]>
> ---
>
> ARM: dts: rockchip: disable arm-global-timer for rk3188
> https://lore.kernel.org/linux-rockchip/[email protected]/
>
> clocksource: arm_global_timer: implement rate compensation whenever source clock changes
> https://lore.kernel.org/all/[email protected]/
> ---
> arch/arm/mach-rockchip/Kconfig | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index b7855cc66..0432a4430 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -15,8 +15,6 @@ config ARCH_ROCKCHIP
> select DW_APB_TIMER_OF
> select REGULATOR if PM
> select ROCKCHIP_TIMER
> - select ARM_GLOBAL_TIMER
> - select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
> select ZONE_DMA if ARM_LPAE
> select PM
> help
> --
> 2.20.1
>
>