[BUG] [PATCH v2 1/2] clocksource: arm_global_timer: implement rate compensation whenever source clock changes

From: Johan Jonker
Date: Mon Oct 17 2022 - 04:03:43 EST

Hi Andrea,

Your patch contribution causes a kernel panic on MK808 with Rockchip rk3066a SoC.
Would you like to contribute to fix this issue?
The psv variable divides with 0 and more things might be wrong.
A revert makes it work again.


Johan

=====

[ 13.087809] ------------[ cut here ]------------
[ 13.110701] kernel BUG at drivers/cpufreq/cpufreq.c:1480!
[ 13.134378] Internal error: Oops - BUG: 0 [#1] SMP ARM
[ 13.157793] Modules linked in:
[ 13.178760] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 6.0.0-next-20221013 #1
[ 13.205990] Hardware name: Rockchip (Device Tree)
[ 13.228793] PC is at cpufreq_online+0x8fc/0xa1c
[ 13.251421] LR is at __wake_up_common_lock+0x98/0xc4
[ 13.274536] pc : [<c06883d8>] lr : [<c0171c50>] psr: a0000013
[ 13.299046] sp : f0821c50 ip : f0821b70 fp : f0821c9c
[ 13.322445] r10: 00000000 r9 : 00000000 r8 : c0f9b800
[ 13.345887] r7 : 000493e0 r6 : c115ef18 r5 : c1005058 r4 : c2055e00
[ 13.370714] r3 : b4aeb193 r2 : b4aeb193 r1 : 60000013 r0 : fffffff0
[ 13.395373] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[ 13.420948] Control: 10c5387d Table: 6000404a DAC: 00000051
[ 13.444914] Register r0 information: non-paged memory
[ 13.468230] Register r1 information: non-paged memory
[ 13.491289] Register r2 information: non-paged memory
[ 13.514135] Register r3 information: non-paged memory
[ 13.536767] Register r4 information: slab kmalloc-512 start c2055e00 pointer offset 0 size 512
[ 13.563712] Register r5 information: non-slab/vmalloc memory
[ 13.587158] Register r6 information: non-slab/vmalloc memory
[ 13.610385] Register r7 information: non-paged memory
[ 13.632699] Register r8 information: non-slab/vmalloc memory
[ 13.655529] Register r9 information: NULL pointer
[ 13.677204] Register r10 information: NULL pointer
[ 13.698865] Register r11 information: 2-page vmalloc region starting at 0xf0820000 allocated at kernel_clone+0xc0/0x3b0
[ 13.727678] Register r12 information: 2-page vmalloc region starting at 0xf0820000 allocated at kernel_clone+0xc0/0x3b0
[ 13.756320] Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
[ 13.779523] Stack: (0xf0821c50 to 0xf0822000)
[ 13.800858] 1c40: 00000000 00000001 ef7c5050 00000001
[ 13.826719] 1c60: c10da900 00000002 c115ef18 c2055f20 c0942224 00000000 ef7c5050 c1004ec8
[ 13.852544] 1c80: c20d7b40 c10da884 c1005058 c1004f14 f0821cb4 f0821ca0 c0688590 c0687ae8
[ 13.878330] 1ca0: c10da570 c10cf8a8 f0821ce4 f0821cb8 c0595080 c0688524 c14f7958 c1645434
[ 13.904232] 1cc0: 00000000 b4aeb193 00000060 c10da900 c115ef18 c10da434 f0821d0c f0821ce8
[ 13.930231] 1ce0: c06859e0 c0594fdc c10da8ec ef7c5050 c20d7b40 00000002 c10da8ec ef7c5050
[ 13.956321] 1d00: f0821d8c f0821d10 c068b9a4 c068589c c0b161cc c1004ec8 00000000 c0b7b718
[ 13.982392] 1d20: c14e3400 00000000 c20d7b48 00000001 c033f608 c20da5d8 c20da688 c160c688
[ 14.008446] 1d40: c0b8bd28 00000001 c0c47d54 c111b000 f0821d84 00000000 00000000 b4aeb193
[ 14.034637] 1d60: c14e3410 c14e3410 00000000 c10da898 00000000 c20d7ab8 c0c47d54 c111b000
[ 14.060896] 1d80: f0821dac f0821d90 c0599424 c068b68c c14e3410 00000000 c10da898 00000000
[ 14.087313] 1da0: f0821dcc f0821db0 c0596cd0 c05993c4 c14e3410 c10da898 c14e3410 00000000
[ 14.113601] 1dc0: f0821de4 f0821dd0 c0596f8c c0596c04 c115dd68 c14e3410 f0821e04 f0821de8
[ 14.139950] 1de0: c0596fe8 c0596ec0 00000000 c14e3410 c10da898 c059741c f0821e2c f0821e08
[ 14.166351] 1e00: c0597490 c0596fb4 c1546ab4 c1004ec8 c10da898 c059741c c20d7ab8 c0c47d54
[ 14.192790] 1e20: f0821e5c f0821e30 c0594c2c c0597428 f0821e68 c14f7858 c1546ab4 b4aeb193
[ 14.219273] 1e40: c10da898 c10cf808 c20d7a80 00000000 f0821e6c f0821e60 c0596528 c0594bb4
[ 14.245875] 1e60: f0821e9c f0821e70 c0595e60 c0596508 c0b7b7c8 00000000 f0821e9c c10da898
[ 14.272550] 1e80: c1004ec8 c0c28e84 00000000 c1456854 f0821eb4 f0821ea0 c0597c80 c0595cc0
[ 14.299270] 1ea0: c110cec0 c1004ec8 f0821ec4 f0821eb8 c0599130 c0597c08 f0821ed4 f0821ec8
[ 14.326125] 1ec0: c0c28ea8 c0599110 f0821f4c f0821ed8 c0102180 c0c28e90 c03318e4 c0b14f30
[ 14.352912] 1ee0: c0b14f10 c0b14f00 c0b2bc78 c1004ec8 00000000 c0b855d0 00000006 00000006
[ 14.379805] 1f00: 00000000 c0b76eb0 c0c0055c c0bebf78 c0953a9c c154665b 00000000 b4aeb193
[ 14.406665] 1f20: c0181cb8 b4aeb193 000000c6 c0c82160 00000007 c0c47d34 c1546600 000000c6
[ 14.433661] 1f40: f0821f94 f0821f50 c0c01a48 c0102134 00000006 00000006 00000000 c0c0055c
[ 14.460621] 1f60: c0c0055c c0bebf78 c1588000 00004ec0 c096ea48 00000000 00000000 00000000
[ 14.487681] 1f80: 00000000 00000000 f0821fac f0821f98 c096ea6c c0c01884 00000000 c096ea48
[ 14.514695] 1fa0: 00000000 f0821fb0 c0100148 c096ea54 00000000 00000000 00000000 00000000
[ 14.541771] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 14.568615] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[ 14.595194] Backtrace:
[ 14.615207] cpufreq_online from cpufreq_add_dev+0x78/0x84
[ 14.638819] r10:c1004f14 r9:c1005058 r8:c10da884 r7:c20d7b40 r6:c1004ec8 r5:ef7c5050
[ 14.665198] r4:00000000
[ 14.685514] cpufreq_add_dev from subsys_interface_register+0xb0/0xfc
[ 14.710558] r5:c10cf8a8 r4:c10da570
[ 14.732076] subsys_interface_register from cpufreq_register_driver+0x150/0x2d8
[ 14.758094] r6:c10da434 r5:c115ef18 r4:c10da900
[ 14.780943] cpufreq_register_driver from dt_cpufreq_probe+0x324/0x49c
[ 14.806283] r6:ef7c5050 r5:c10da8ec r4:00000002
[ 14.829299] dt_cpufreq_probe from platform_probe+0x6c/0xc8
[ 14.853499] r10:c111b000 r9:c0c47d54 r8:c20d7ab8 r7:00000000 r6:c10da898 r5:00000000
[ 14.880388] r4:c14e3410
[ 14.901252] platform_probe from really_probe+0xd8/0x2bc
[ 14.925473] r7:00000000 r6:c10da898 r5:00000000 r4:c14e3410
[ 14.949898] really_probe from __driver_probe_device+0xd8/0xf4
[ 14.974546] r7:00000000 r6:c14e3410 r5:c10da898 r4:c14e3410
[ 14.998855] __driver_probe_device from driver_probe_device+0x40/0xd0
[ 15.024426] r5:c14e3410 r4:c115dd68
[ 15.046857] driver_probe_device from __driver_attach+0x74/0x120
[ 15.072380] r7:c059741c r6:c10da898 r5:c14e3410 r4:00000000
[ 15.097388] __driver_attach from bus_for_each_dev+0x84/0xc4
[ 15.122444] r9:c0c47d54 r8:c20d7ab8 r7:c059741c r6:c10da898 r5:c1004ec8 r4:c1546ab4
[ 15.150131] bus_for_each_dev from driver_attach+0x2c/0x30
[ 15.175532] r7:00000000 r6:c20d7a80 r5:c10cf808 r4:c10da898
[ 15.201134] driver_attach from bus_add_driver+0x1ac/0x1f8
[ 15.226772] bus_add_driver from driver_register+0x84/0x11c
[ 15.252559] r8:c1456854 r7:00000000 r6:c0c28e84 r5:c1004ec8 r4:c10da898
[ 15.279483] driver_register from __platform_driver_register+0x2c/0x34
[ 15.306523] r5:c1004ec8 r4:c110cec0
[ 15.329801] __platform_driver_register from dt_cpufreq_platdrv_init+0x24/0x28
[ 15.357500] dt_cpufreq_platdrv_init from do_one_initcall+0x58/0x20c
[ 15.384159] do_one_initcall from kernel_init_freeable+0x1d0/0x22c
[ 15.410635] r8:000000c6 r7:c1546600 r6:c0c47d34 r5:00000007 r4:c0c82160
[ 15.437567] kernel_init_freeable from kernel_init+0x24/0x144
[ 15.463573] r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c096ea48
[ 15.491869] r4:00004ec0
[ 15.514022] kernel_init from ret_from_fork+0x14/0x2c
[ 15.539079] Exception stack(0xf0821fb0 to 0xf0821ff8)
[ 15.563943] 1fa0: 00000000 00000000 00000000 00000000
[ 15.592459] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 15.620836] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 15.647222] r5:c096ea48 r4:00000000
[ 15.670196] Code: e1a00004 ebfff74d e3500000 0a00001a (e7f001f2)
[ 15.696086] ---[ end trace 0000000000000000 ]---
[ 15.720522] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[ 15.748404] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---


On 4/6/21 15:00, Andrea Merello wrote:
> This patch adds rate change notification support for the parent clock;
> should that clock change, then we try to adjust the our prescaler in order
> to compensate (i.e. we adjust to still get the same timer frequency).
>
> This is loosely based on what it's done in timer-cadence-ttc. timer-sun51,
> mips-gic-timer and smp_twd.c also seem to look at their parent clock rate
> and to perform some kind of adjustment whenever needed.
>
> In this particular case we have only one single counter and prescaler for
> all clocksource, clockevent and timer_delay, and we just update it for all
> (i.e. we don't let it go and call clockevents_update_freq() to notify to
> the kernel that our rate has changed).
>
> Note that, there is apparently no other way to fixup things, because once
> we call register_current_timer_delay(), specifying the timer rate, it seems
> that that rate is not supposed to change ever.
>
> In order for this mechanism to work, we have to make assumptions about how
> much the initial clock is supposed to eventually decrease from the initial
> one, and set our initial prescaler to a value that we can eventually
> decrease enough to compensate. We provide an option in KConfig for this.
>
> In case we end up in a situation in which we are not able to compensate the
> parent clock change, we fail returning NOTIFY_BAD.
>
> This fixes a real-world problem with Zynq arch not being able to use this
> driver and CPU_FREQ at the same time (because ARM global timer is fed by
> the CPU clock, which may keep changing when CPU_FREQ is enabled).
>
> Signed-off-by: Andrea Merello <andrea.merello@xxxxxxxxx>
> Cc: Patrice Chotard <patrice.chotard@xxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Cc: Michal Simek <michal.simek@xxxxxxxxxx>
> Cc: Sören Brinkmann <soren.brinkmann@xxxxxxxxxx>
> ---
> drivers/clocksource/Kconfig | 13 +++
> drivers/clocksource/arm_global_timer.c | 122 +++++++++++++++++++++++--
> 2 files changed, 125 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 39aa21d01e05..19fc5f8883e0 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -358,6 +358,19 @@ config ARM_GLOBAL_TIMER
> help
> This option enables support for the ARM global timer unit.
>
> +config ARM_GT_INITIAL_PRESCALER_VAL
> + int "ARM global timer initial prescaler value"
> + default 1
> + depends on ARM_GLOBAL_TIMER
> + help
> + When the ARM global timer initializes, its current rate is declared
> + to the kernel and maintained forever. Should it's parent clock
> + change, the driver tries to fix the timer's internal prescaler.
> + On some machs (i.e. Zynq) the initial prescaler value thus poses
> + bounds about how much the parent clock is allowed to decrease or
> + increase wrt the initial clock value.
> + This affects CPU_FREQ max delta from the initial frequency.
> +
> config ARM_TIMER_SP804
> bool "Support for Dual Timer SP804 module" if COMPILE_TEST
> depends on GENERIC_SCHED_CLOCK && CLKDEV_LOOKUP
> diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c
> index 88b2d38a7a61..60a8047fd32e 100644
> --- a/drivers/clocksource/arm_global_timer.c
> +++ b/drivers/clocksource/arm_global_timer.c
> @@ -31,6 +31,10 @@
> #define GT_CONTROL_COMP_ENABLE BIT(1) /* banked */
> #define GT_CONTROL_IRQ_ENABLE BIT(2) /* banked */
> #define GT_CONTROL_AUTO_INC BIT(3) /* banked */
> +#define GT_CONTROL_PRESCALER_SHIFT 8
> +#define GT_CONTROL_PRESCALER_MAX 0xF
> +#define GT_CONTROL_PRESCALER_MASK (GT_CONTROL_PRESCALER_MAX << \
> + GT_CONTROL_PRESCALER_SHIFT)
>
> #define GT_INT_STATUS 0x0c
> #define GT_INT_STATUS_EVENT_FLAG BIT(0)
> @@ -39,6 +43,7 @@
> #define GT_COMP1 0x14
> #define GT_AUTO_INC 0x18
>
> +#define MAX_F_ERR 50
> /*
> * We are expecting to be clocked by the ARM peripheral clock.
> *
> @@ -46,7 +51,8 @@
> * the units for all operations.
> */
> static void __iomem *gt_base;
> -static unsigned long gt_clk_rate;
> +struct notifier_block gt_clk_rate_change_nb;
> +static u32 gt_psv_new, gt_psv_bck, gt_target_rate;
> static int gt_ppi;
> static struct clock_event_device __percpu *gt_evt;
>
> @@ -96,7 +102,10 @@ static void gt_compare_set(unsigned long delta, int periodic)
> unsigned long ctrl;
>
> counter += delta;
> - ctrl = GT_CONTROL_TIMER_ENABLE;
> + ctrl = readl(gt_base + GT_CONTROL);
> + ctrl &= ~(GT_CONTROL_COMP_ENABLE | GT_CONTROL_IRQ_ENABLE |
> + GT_CONTROL_AUTO_INC | GT_CONTROL_AUTO_INC);
> + ctrl |= GT_CONTROL_TIMER_ENABLE;
> writel_relaxed(ctrl, gt_base + GT_CONTROL);
> writel_relaxed(lower_32_bits(counter), gt_base + GT_COMP0);
> writel_relaxed(upper_32_bits(counter), gt_base + GT_COMP1);
> @@ -123,7 +132,7 @@ static int gt_clockevent_shutdown(struct clock_event_device *evt)
>
> static int gt_clockevent_set_periodic(struct clock_event_device *evt)
> {
> - gt_compare_set(DIV_ROUND_CLOSEST(gt_clk_rate, HZ), 1);
> + gt_compare_set(DIV_ROUND_CLOSEST(gt_target_rate, HZ), 1);
> return 0;
> }
>
> @@ -177,7 +186,7 @@ static int gt_starting_cpu(unsigned int cpu)
> clk->cpumask = cpumask_of(cpu);
> clk->rating = 300;
> clk->irq = gt_ppi;
> - clockevents_config_and_register(clk, gt_clk_rate,
> + clockevents_config_and_register(clk, gt_target_rate,
> 1, 0xffffffff);
> enable_percpu_irq(clk->irq, IRQ_TYPE_NONE);
> return 0;
> @@ -232,9 +241,28 @@ static struct delay_timer gt_delay_timer = {
> .read_current_timer = gt_read_long,
> };
>
> +static void gt_write_presc(u32 psv)
> +{
> + u32 reg;
> +
> + reg = readl(gt_base + GT_CONTROL);
> + reg &= ~GT_CONTROL_PRESCALER_MASK;
> + reg |= psv << GT_CONTROL_PRESCALER_SHIFT;
> + writel(reg, gt_base + GT_CONTROL);
> +}
> +
> +static u32 gt_read_presc(void)
> +{
> + u32 reg;
> +
> + reg = readl(gt_base + GT_CONTROL);
> + reg &= GT_CONTROL_PRESCALER_MASK;
> + return reg >> GT_CONTROL_PRESCALER_SHIFT;
> +}
> +
> static void __init gt_delay_timer_init(void)
> {
> - gt_delay_timer.freq = gt_clk_rate;
> + gt_delay_timer.freq = gt_target_rate;
> register_current_timer_delay(&gt_delay_timer);
> }
>
> @@ -243,18 +271,81 @@ static int __init gt_clocksource_init(void)
> writel(0, gt_base + GT_CONTROL);
> writel(0, gt_base + GT_COUNTER0);
> writel(0, gt_base + GT_COUNTER1);
> - /* enables timer on all the cores */
> - writel(GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL);
> + /* set prescaler and enable timer on all the cores */
> + writel(((CONFIG_ARM_GT_INITIAL_PRESCALER_VAL - 1) <<
> + GT_CONTROL_PRESCALER_SHIFT)
> + | GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL);
>
> #ifdef CONFIG_CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
> - sched_clock_register(gt_sched_clock_read, 64, gt_clk_rate);
> + sched_clock_register(gt_sched_clock_read, 64, gt_target_rate);
> #endif
> - return clocksource_register_hz(&gt_clocksource, gt_clk_rate);
> + return clocksource_register_hz(&gt_clocksource, gt_target_rate);
> +}
> +
> +static int gt_clk_rate_change_cb(struct notifier_block *nb,
> + unsigned long event, void *data)
> +{
> + struct clk_notifier_data *ndata = data;
> +
> + switch (event) {
> + case PRE_RATE_CHANGE:
> + {
> + int psv;
> +

> + psv = DIV_ROUND_CLOSEST(ndata->new_rate,
> + gt_target_rate);
> +

> + if (abs(gt_target_rate - (ndata->new_rate / psv)) > MAX_F_ERR)
> + return NOTIFY_BAD;

This psv variable can become a 0 divider.

> +
> + psv--;
> +
> + /* prescaler within legal range? */
> + if (psv < 0 || psv > GT_CONTROL_PRESCALER_MAX)
> + return NOTIFY_BAD;
> +
> + /*
> + * store timer clock ctrl register so we can restore it in case
> + * of an abort.
> + */
> + gt_psv_bck = gt_read_presc();
> + gt_psv_new = psv;
> + /* scale down: adjust divider in post-change notification */
> + if (ndata->new_rate < ndata->old_rate)
> + return NOTIFY_DONE;
> +
> + /* scale up: adjust divider now - before frequency change */
> + gt_write_presc(psv);
> + break;
> + }
> + case POST_RATE_CHANGE:
> + /* scale up: pre-change notification did the adjustment */
> + if (ndata->new_rate > ndata->old_rate)
> + return NOTIFY_OK;
> +
> + /* scale down: adjust divider now - after frequency change */
> + gt_write_presc(gt_psv_new);
> + break;
> +
> + case ABORT_RATE_CHANGE:
> + /* we have to undo the adjustment in case we scale up */
> + if (ndata->new_rate < ndata->old_rate)
> + return NOTIFY_OK;
> +
> + /* restore original register value */
> + gt_write_presc(gt_psv_bck);
> + break;
> + default:
> + return NOTIFY_DONE;
> + }
> +
> + return NOTIFY_DONE;
> }
>
> static int __init global_timer_of_register(struct device_node *np)
> {
> struct clk *gt_clk;
> + static unsigned long gt_clk_rate;
> int err = 0;
>
> /*
> @@ -292,11 +383,20 @@ static int __init global_timer_of_register(struct device_node *np)
> }
>
> gt_clk_rate = clk_get_rate(gt_clk);
> + gt_target_rate = gt_clk_rate / CONFIG_ARM_GT_INITIAL_PRESCALER_VAL;
> + gt_clk_rate_change_nb.notifier_call =
> + gt_clk_rate_change_cb;
> + err = clk_notifier_register(gt_clk, &gt_clk_rate_change_nb);
> + if (err) {
> + pr_warn("Unable to register clock notifier\n");
> + goto out_clk;
> + }
> +
> gt_evt = alloc_percpu(struct clock_event_device);
> if (!gt_evt) {
> pr_warn("global-timer: can't allocate memory\n");
> err = -ENOMEM;
> - goto out_clk;
> + goto out_clk_nb;
> }
>
> err = request_percpu_irq(gt_ppi, gt_clockevent_interrupt,
> @@ -326,6 +426,8 @@ static int __init global_timer_of_register(struct device_node *np)
> free_percpu_irq(gt_ppi, gt_evt);
> out_free:
> free_percpu(gt_evt);
> +out_clk_nb:
> + clk_notifier_unregister(gt_clk, &gt_clk_rate_change_nb);
> out_clk:
> clk_disable_unprepare(gt_clk);
> out_unmap:From d283521df900be5205ab6d7f2e62ae11770e8f56 Mon Sep 17 00:00:00 2001
From: Johan Jonker <jbx6244@xxxxxxxxx>
Date: Thu, 13 Oct 2022 20:34:30 +0200
Subject: [PATCH v2 1/5] Revert "clocksource/drivers/arm_global_timer: Remove
duplicated argument in arm_global_timer"

This reverts commit f94bc2667fb204d7c131ac39d9ea342bd16116dc.
---
drivers/clocksource/arm_global_timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c
index 44a61dc6f932..68b1d144a412 100644
--- a/drivers/clocksource/arm_global_timer.c
+++ b/drivers/clocksource/arm_global_timer.c
@@ -104,7 +104,7 @@ static void gt_compare_set(unsigned long delta, int periodic)
counter += delta;
ctrl = readl(gt_base + GT_CONTROL);
ctrl &= ~(GT_CONTROL_COMP_ENABLE | GT_CONTROL_IRQ_ENABLE |
- GT_CONTROL_AUTO_INC);
+ GT_CONTROL_AUTO_INC | GT_CONTROL_AUTO_INC);
ctrl |= GT_CONTROL_TIMER_ENABLE;
writel_relaxed(ctrl, gt_base + GT_CONTROL);
writel_relaxed(lower_32_bits(counter), gt_base + GT_COMP0);
--
2.20.1

From 1c1ed65928038321fa92e88d67329e255ec77144 Mon Sep 17 00:00:00 2001
From: Johan Jonker <jbx6244@xxxxxxxxx>
Date: Thu, 13 Oct 2022 20:36:35 +0200
Subject: [PATCH v2 2/5] Revert "clocksource/drivers/arm_global_timer: Make
symbol 'gt_clk_rate_change_nb' static"

This reverts commit be534f8ee137b95046d7c53c8200ffdcf05781a7.
---
drivers/clocksource/arm_global_timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c
index 68b1d144a412..60a8047fd32e 100644
--- a/drivers/clocksource/arm_global_timer.c
+++ b/drivers/clocksource/arm_global_timer.c
@@ -51,7 +51,7 @@
* the units for all operations.
*/
static void __iomem *gt_base;
-static struct notifier_block gt_clk_rate_change_nb;
+struct notifier_block gt_clk_rate_change_nb;
static u32 gt_psv_new, gt_psv_bck, gt_target_rate;
static int gt_ppi;
static struct clock_event_device __percpu *gt_evt;
--
2.20.1

From 742c4cc1853e2529d7b0d020c0387038f979bf07 Mon Sep 17 00:00:00 2001
From: Johan Jonker <jbx6244@xxxxxxxxx>
Date: Thu, 13 Oct 2022 20:45:37 +0200
Subject: [PATCH v2 3/5] Revert "clocksource/drivers/arm_global_timer:
Implement rate compensation whenever source clock changes"

This reverts commit 171b45a4a70eef2fd36bb794ce4f5a48c440361e.
---
drivers/clocksource/Kconfig | 14 ---
drivers/clocksource/arm_global_timer.c | 122 ++-----------------------
2 files changed, 10 insertions(+), 126 deletions(-)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 4469e7f555e9..7b00efa59ffb 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -375,20 +375,6 @@ config ARM_GLOBAL_TIMER
help
This option enables support for the ARM global timer unit.

-config ARM_GT_INITIAL_PRESCALER_VAL
- int "ARM global timer initial prescaler value"
- default 2 if ARCH_ZYNQ
- default 1
- depends on ARM_GLOBAL_TIMER
- help
- When the ARM global timer initializes, its current rate is declared
- to the kernel and maintained forever. Should its parent clock
- change, the driver tries to fix the timer's internal prescaler.
- On some machs (i.e. Zynq) the initial prescaler value thus poses
- bounds about how much the parent clock is allowed to decrease or
- increase wrt the initial clock value.
- This affects CPU_FREQ max delta from the initial frequency.
-
config ARM_TIMER_SP804
bool "Support for Dual Timer SP804 module" if COMPILE_TEST
depends on GENERIC_SCHED_CLOCK && HAVE_CLK
diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c
index 60a8047fd32e..88b2d38a7a61 100644
--- a/drivers/clocksource/arm_global_timer.c
+++ b/drivers/clocksource/arm_global_timer.c
@@ -31,10 +31,6 @@
#define GT_CONTROL_COMP_ENABLE BIT(1) /* banked */
#define GT_CONTROL_IRQ_ENABLE BIT(2) /* banked */
#define GT_CONTROL_AUTO_INC BIT(3) /* banked */
-#define GT_CONTROL_PRESCALER_SHIFT 8
-#define GT_CONTROL_PRESCALER_MAX 0xF
-#define GT_CONTROL_PRESCALER_MASK (GT_CONTROL_PRESCALER_MAX << \
- GT_CONTROL_PRESCALER_SHIFT)

#define GT_INT_STATUS 0x0c
#define GT_INT_STATUS_EVENT_FLAG BIT(0)
@@ -43,7 +39,6 @@
#define GT_COMP1 0x14
#define GT_AUTO_INC 0x18

-#define MAX_F_ERR 50
/*
* We are expecting to be clocked by the ARM peripheral clock.
*
@@ -51,8 +46,7 @@
* the units for all operations.
*/
static void __iomem *gt_base;
-struct notifier_block gt_clk_rate_change_nb;
-static u32 gt_psv_new, gt_psv_bck, gt_target_rate;
+static unsigned long gt_clk_rate;
static int gt_ppi;
static struct clock_event_device __percpu *gt_evt;

@@ -102,10 +96,7 @@ static void gt_compare_set(unsigned long delta, int periodic)
unsigned long ctrl;

counter += delta;
- ctrl = readl(gt_base + GT_CONTROL);
- ctrl &= ~(GT_CONTROL_COMP_ENABLE | GT_CONTROL_IRQ_ENABLE |
- GT_CONTROL_AUTO_INC | GT_CONTROL_AUTO_INC);
- ctrl |= GT_CONTROL_TIMER_ENABLE;
+ ctrl = GT_CONTROL_TIMER_ENABLE;
writel_relaxed(ctrl, gt_base + GT_CONTROL);
writel_relaxed(lower_32_bits(counter), gt_base + GT_COMP0);
writel_relaxed(upper_32_bits(counter), gt_base + GT_COMP1);
@@ -132,7 +123,7 @@ static int gt_clockevent_shutdown(struct clock_event_device *evt)

static int gt_clockevent_set_periodic(struct clock_event_device *evt)
{
- gt_compare_set(DIV_ROUND_CLOSEST(gt_target_rate, HZ), 1);
+ gt_compare_set(DIV_ROUND_CLOSEST(gt_clk_rate, HZ), 1);
return 0;
}

@@ -186,7 +177,7 @@ static int gt_starting_cpu(unsigned int cpu)
clk->cpumask = cpumask_of(cpu);
clk->rating = 300;
clk->irq = gt_ppi;
- clockevents_config_and_register(clk, gt_target_rate,
+ clockevents_config_and_register(clk, gt_clk_rate,
1, 0xffffffff);
enable_percpu_irq(clk->irq, IRQ_TYPE_NONE);
return 0;
@@ -241,28 +232,9 @@ static struct delay_timer gt_delay_timer = {
.read_current_timer = gt_read_long,
};

-static void gt_write_presc(u32 psv)
-{
- u32 reg;
-
- reg = readl(gt_base + GT_CONTROL);
- reg &= ~GT_CONTROL_PRESCALER_MASK;
- reg |= psv << GT_CONTROL_PRESCALER_SHIFT;
- writel(reg, gt_base + GT_CONTROL);
-}
-
-static u32 gt_read_presc(void)
-{
- u32 reg;
-
- reg = readl(gt_base + GT_CONTROL);
- reg &= GT_CONTROL_PRESCALER_MASK;
- return reg >> GT_CONTROL_PRESCALER_SHIFT;
-}
-
static void __init gt_delay_timer_init(void)
{
- gt_delay_timer.freq = gt_target_rate;
+ gt_delay_timer.freq = gt_clk_rate;
register_current_timer_delay(&gt_delay_timer);
}

@@ -271,81 +243,18 @@ static int __init gt_clocksource_init(void)
writel(0, gt_base + GT_CONTROL);
writel(0, gt_base + GT_COUNTER0);
writel(0, gt_base + GT_COUNTER1);
- /* set prescaler and enable timer on all the cores */
- writel(((CONFIG_ARM_GT_INITIAL_PRESCALER_VAL - 1) <<
- GT_CONTROL_PRESCALER_SHIFT)
- | GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL);
+ /* enables timer on all the cores */
+ writel(GT_CONTROL_TIMER_ENABLE, gt_base + GT_CONTROL);

#ifdef CONFIG_CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
- sched_clock_register(gt_sched_clock_read, 64, gt_target_rate);
+ sched_clock_register(gt_sched_clock_read, 64, gt_clk_rate);
#endif
- return clocksource_register_hz(&gt_clocksource, gt_target_rate);
-}
-
-static int gt_clk_rate_change_cb(struct notifier_block *nb,
- unsigned long event, void *data)
-{
- struct clk_notifier_data *ndata = data;
-
- switch (event) {
- case PRE_RATE_CHANGE:
- {
- int psv;
-
- psv = DIV_ROUND_CLOSEST(ndata->new_rate,
- gt_target_rate);
-
- if (abs(gt_target_rate - (ndata->new_rate / psv)) > MAX_F_ERR)
- return NOTIFY_BAD;
-
- psv--;
-
- /* prescaler within legal range? */
- if (psv < 0 || psv > GT_CONTROL_PRESCALER_MAX)
- return NOTIFY_BAD;
-
- /*
- * store timer clock ctrl register so we can restore it in case
- * of an abort.
- */
- gt_psv_bck = gt_read_presc();
- gt_psv_new = psv;
- /* scale down: adjust divider in post-change notification */
- if (ndata->new_rate < ndata->old_rate)
- return NOTIFY_DONE;
-
- /* scale up: adjust divider now - before frequency change */
- gt_write_presc(psv);
- break;
- }
- case POST_RATE_CHANGE:
- /* scale up: pre-change notification did the adjustment */
- if (ndata->new_rate > ndata->old_rate)
- return NOTIFY_OK;
-
- /* scale down: adjust divider now - after frequency change */
- gt_write_presc(gt_psv_new);
- break;
-
- case ABORT_RATE_CHANGE:
- /* we have to undo the adjustment in case we scale up */
- if (ndata->new_rate < ndata->old_rate)
- return NOTIFY_OK;
-
- /* restore original register value */
- gt_write_presc(gt_psv_bck);
- break;
- default:
- return NOTIFY_DONE;
- }
-
- return NOTIFY_DONE;
+ return clocksource_register_hz(&gt_clocksource, gt_clk_rate);
}

static int __init global_timer_of_register(struct device_node *np)
{
struct clk *gt_clk;
- static unsigned long gt_clk_rate;
int err = 0;

/*
@@ -383,20 +292,11 @@ static int __init global_timer_of_register(struct device_node *np)
}

gt_clk_rate = clk_get_rate(gt_clk);
- gt_target_rate = gt_clk_rate / CONFIG_ARM_GT_INITIAL_PRESCALER_VAL;
- gt_clk_rate_change_nb.notifier_call =
- gt_clk_rate_change_cb;
- err = clk_notifier_register(gt_clk, &gt_clk_rate_change_nb);
- if (err) {
- pr_warn("Unable to register clock notifier\n");
- goto out_clk;
- }
-
gt_evt = alloc_percpu(struct clock_event_device);
if (!gt_evt) {
pr_warn("global-timer: can't allocate memory\n");
err = -ENOMEM;
- goto out_clk_nb;
+ goto out_clk;
}

err = request_percpu_irq(gt_ppi, gt_clockevent_interrupt,
@@ -426,8 +326,6 @@ static int __init global_timer_of_register(struct device_node *np)
free_percpu_irq(gt_ppi, gt_evt);
out_free:
free_percpu(gt_evt);
-out_clk_nb:
- clk_notifier_unregister(gt_clk, &gt_clk_rate_change_nb);
out_clk:
clk_disable_unprepare(gt_clk);
out_unmap:
--
2.20.1