2023-10-07 16:19:48

by Ronald Wahl

[permalink] [raw]
Subject: [RESEND][PATCH] clocksource/drivers/timer-atmel-tcb: Fix initialization on SAM9 hardware

From: Ronald Wahl <[email protected]>

On SAM9 hardware two cascaded 16 bit timers are used to form a 32 bit
high resolution timer that is used as scheduler clock when the kernel
has been configured that way (CONFIG_ATMEL_CLOCKSOURCE_TCB).

The driver initially triggers a reset-to-zero of the two timers but this
reset is only performed on the next rising clock. For the first timer
this is ok - it will be in the next 60ns (16MHz clock). For the chained
second timer this will only happen after the first timer overflows, i.e.
after 2^16 clocks (~4ms with a 16MHz clock). So with other words the
scheduler clock resets to 0 after the first 2^16 clock cycles.

It looks like that the scheduler does not like this and behaves wrongly
over its lifetime, e.g. some tasks are scheduled with a long delay. Why
that is and if there are additional requirements for this behaviour has
not been further analysed.

There is a simple fix for resetting the second timer as well when the
first timer is reset and this is to set the ATMEL_TC_ASWTRG_SET bit in
the Channel Mode register (CMR) of the first timer. This will also rise
the TIOA line (clock input of the second timer) when a software trigger
respective SYNC is issued.

Signed-off-by: Ronald Wahl <[email protected]>
---
drivers/clocksource/timer-atmel-tcb.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/clocksource/timer-atmel-tcb.c b/drivers/clocksource/timer-atmel-tcb.c
index 27af17c99590..2a90c92a9182 100644
--- a/drivers/clocksource/timer-atmel-tcb.c
+++ b/drivers/clocksource/timer-atmel-tcb.c
@@ -315,6 +315,7 @@ static void __init tcb_setup_dual_chan(struct atmel_tc *tc, int mck_divisor_idx)
writel(mck_divisor_idx /* likely divide-by-8 */
| ATMEL_TC_WAVE
| ATMEL_TC_WAVESEL_UP /* free-run */
+ | ATMEL_TC_ASWTRG_SET /* TIOA0 rises at software trigger */
| ATMEL_TC_ACPA_SET /* TIOA0 rises at 0 */
| ATMEL_TC_ACPC_CLEAR, /* (duty cycle 50%) */
tcaddr + ATMEL_TC_REG(0, CMR));
--
2.41.0


2023-10-13 10:23:26

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [RESEND][PATCH] clocksource/drivers/timer-atmel-tcb: Fix initialization on SAM9 hardware

On 07/10/2023 18:17:13+0200, Ronald Wahl wrote:
> From: Ronald Wahl <[email protected]>
>
> On SAM9 hardware two cascaded 16 bit timers are used to form a 32 bit
> high resolution timer that is used as scheduler clock when the kernel
> has been configured that way (CONFIG_ATMEL_CLOCKSOURCE_TCB).
>
> The driver initially triggers a reset-to-zero of the two timers but this
> reset is only performed on the next rising clock. For the first timer
> this is ok - it will be in the next 60ns (16MHz clock). For the chained
> second timer this will only happen after the first timer overflows, i.e.
> after 2^16 clocks (~4ms with a 16MHz clock). So with other words the
> scheduler clock resets to 0 after the first 2^16 clock cycles.
>
> It looks like that the scheduler does not like this and behaves wrongly
> over its lifetime, e.g. some tasks are scheduled with a long delay. Why
> that is and if there are additional requirements for this behaviour has
> not been further analysed.
>
> There is a simple fix for resetting the second timer as well when the
> first timer is reset and this is to set the ATMEL_TC_ASWTRG_SET bit in
> the Channel Mode register (CMR) of the first timer. This will also rise
> the TIOA line (clock input of the second timer) when a software trigger
> respective SYNC is issued.
>
> Signed-off-by: Ronald Wahl <[email protected]>
Acked-by: Alexandre Belloni <[email protected]>

> ---
> drivers/clocksource/timer-atmel-tcb.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/clocksource/timer-atmel-tcb.c b/drivers/clocksource/timer-atmel-tcb.c
> index 27af17c99590..2a90c92a9182 100644
> --- a/drivers/clocksource/timer-atmel-tcb.c
> +++ b/drivers/clocksource/timer-atmel-tcb.c
> @@ -315,6 +315,7 @@ static void __init tcb_setup_dual_chan(struct atmel_tc *tc, int mck_divisor_idx)
> writel(mck_divisor_idx /* likely divide-by-8 */
> | ATMEL_TC_WAVE
> | ATMEL_TC_WAVESEL_UP /* free-run */
> + | ATMEL_TC_ASWTRG_SET /* TIOA0 rises at software trigger */
> | ATMEL_TC_ACPA_SET /* TIOA0 rises at 0 */
> | ATMEL_TC_ACPC_CLEAR, /* (duty cycle 50%) */
> tcaddr + ATMEL_TC_REG(0, CMR));
> --
> 2.41.0
>

--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Subject: [tip: timers/core] clocksource/drivers/timer-atmel-tcb: Fix initialization on SAM9 hardware

The following commit has been merged into the timers/core branch of tip:

Commit-ID: 6d3bc4c02d59996d1d3180d8ed409a9d7d5900e0
Gitweb: https://git.kernel.org/tip/6d3bc4c02d59996d1d3180d8ed409a9d7d5900e0
Author: Ronald Wahl <[email protected]>
AuthorDate: Sat, 07 Oct 2023 18:17:13 +02:00
Committer: Daniel Lezcano <[email protected]>
CommitterDate: Fri, 13 Oct 2023 12:56:50 +02:00

clocksource/drivers/timer-atmel-tcb: Fix initialization on SAM9 hardware

On SAM9 hardware two cascaded 16 bit timers are used to form a 32 bit
high resolution timer that is used as scheduler clock when the kernel
has been configured that way (CONFIG_ATMEL_CLOCKSOURCE_TCB).

The driver initially triggers a reset-to-zero of the two timers but this
reset is only performed on the next rising clock. For the first timer
this is ok - it will be in the next 60ns (16MHz clock). For the chained
second timer this will only happen after the first timer overflows, i.e.
after 2^16 clocks (~4ms with a 16MHz clock). So with other words the
scheduler clock resets to 0 after the first 2^16 clock cycles.

It looks like that the scheduler does not like this and behaves wrongly
over its lifetime, e.g. some tasks are scheduled with a long delay. Why
that is and if there are additional requirements for this behaviour has
not been further analysed.

There is a simple fix for resetting the second timer as well when the
first timer is reset and this is to set the ATMEL_TC_ASWTRG_SET bit in
the Channel Mode register (CMR) of the first timer. This will also rise
the TIOA line (clock input of the second timer) when a software trigger
respective SYNC is issued.

Signed-off-by: Ronald Wahl <[email protected]>
Acked-by: Alexandre Belloni <[email protected]>
Signed-off-by: Daniel Lezcano <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/clocksource/timer-atmel-tcb.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/clocksource/timer-atmel-tcb.c b/drivers/clocksource/timer-atmel-tcb.c
index 27af17c..2a90c92 100644
--- a/drivers/clocksource/timer-atmel-tcb.c
+++ b/drivers/clocksource/timer-atmel-tcb.c
@@ -315,6 +315,7 @@ static void __init tcb_setup_dual_chan(struct atmel_tc *tc, int mck_divisor_idx)
writel(mck_divisor_idx /* likely divide-by-8 */
| ATMEL_TC_WAVE
| ATMEL_TC_WAVESEL_UP /* free-run */
+ | ATMEL_TC_ASWTRG_SET /* TIOA0 rises at software trigger */
| ATMEL_TC_ACPA_SET /* TIOA0 rises at 0 */
| ATMEL_TC_ACPC_CLEAR, /* (duty cycle 50%) */
tcaddr + ATMEL_TC_REG(0, CMR));