All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexandre Belloni <alexandre.belloni@bootlin.com>
To: Ronald Wahl <rwahl@gmx.de>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>,
	Claudiu Beznea <claudiu.beznea@tuxon.dev>,
	linux-kernel@vger.kernel.org,
	Ronald Wahl <ronald.wahl@raritan.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [RESEND][PATCH] clocksource/drivers/timer-atmel-tcb: Fix initialization on SAM9 hardware
Date: Fri, 13 Oct 2023 12:23:11 +0200	[thread overview]
Message-ID: <20231013102311929c3fab@mail.local> (raw)
In-Reply-To: <20231007161803.31342-1-rwahl@gmx.de>

On 07/10/2023 18:17:13+0200, Ronald Wahl wrote:
> From: Ronald Wahl <ronald.wahl@raritan.com>
> 
> 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 <ronald.wahl@raritan.com>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Alexandre Belloni <alexandre.belloni@bootlin.com>
To: Ronald Wahl <rwahl@gmx.de>
Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Claudiu Beznea <claudiu.beznea@tuxon.dev>,
	Ronald Wahl <ronald.wahl@raritan.com>
Subject: Re: [RESEND][PATCH] clocksource/drivers/timer-atmel-tcb: Fix initialization on SAM9 hardware
Date: Fri, 13 Oct 2023 12:23:11 +0200	[thread overview]
Message-ID: <20231013102311929c3fab@mail.local> (raw)
In-Reply-To: <20231007161803.31342-1-rwahl@gmx.de>

On 07/10/2023 18:17:13+0200, Ronald Wahl wrote:
> From: Ronald Wahl <ronald.wahl@raritan.com>
> 
> 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 <ronald.wahl@raritan.com>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  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

  reply	other threads:[~2023-10-13 10:23 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-07 16:17 [RESEND][PATCH] clocksource/drivers/timer-atmel-tcb: Fix initialization on SAM9 hardware Ronald Wahl
2023-10-07 16:17 ` Ronald Wahl
2023-10-13 10:23 ` Alexandre Belloni [this message]
2023-10-13 10:23   ` Alexandre Belloni
2023-10-27 18:23 ` [tip: timers/core] " tip-bot2 for Ronald Wahl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231013102311929c3fab@mail.local \
    --to=alexandre.belloni@bootlin.com \
    --cc=claudiu.beznea@tuxon.dev \
    --cc=daniel.lezcano@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ronald.wahl@raritan.com \
    --cc=rwahl@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.