[PATCH 1/2] lib: time: Change behaviour of CONFIG_TIMER_EARLY

[Sean Anderson]

On 3/11/22 12:11 PM, Johannes Krottmayer wrote:
> If CONFIG_TIMER_EARLY is selected and the timer driver implements
> timer_early_get_count() and timer_early_get_rate(), check first
> if the virtual root driver is running, when it is initialized yet
> use the virtual timer driver instead. When the virtual root driver
> doesn't exists fallback to the timer_early_get_count() and
> timer_early_get_rate().
> 
> Signed-off-by: Johannes Krottmayer <krjdev at gmail.com>
> Cc: Wolfgang Denk <wd at denx.de>
> Cc: Thomas Chou <thomas at wytron.com.tw>
> Cc: Rick Chen <rick at andestech.com>
> Cc: Leo <ycliang at andestech.com>
> Cc: Bin Meng <bmeng.cn at gmail.com>
> Cc: Sean Anderson <seanga2 at gmail.com>
> Cc: Anup Patel <anup at brainfault.org>
> Cc: Thomas Chou <thomas at wytron.com.tw>
> ---
>   lib/time.c | 42 +++++++++++++++++++++++++++++++++---------
>   1 file changed, 33 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/time.c b/lib/time.c
> index 96074b84af..b5c9760042 100644
> --- a/lib/time.c
> +++ b/lib/time.c
> @@ -8,6 +8,7 @@
>   #include <clock_legacy.h>
>   #include <bootstage.h>
>   #include <dm.h>
> +#include <dm/root.h>
>   #include <errno.h>
>   #include <init.h>
>   #include <spl.h>
> @@ -66,16 +67,28 @@ extern unsigned long __weak timer_read_counter(void);
>   #if CONFIG_IS_ENABLED(TIMER)
>   ulong notrace get_tbclk(void)
>   {
> -	if (!gd->timer) {
> +	int ret;
> +
>   #ifdef CONFIG_TIMER_EARLY
> +
> +	/*
> +	 * Check if the virtual root driver does not yet exist,
> +	 * if not fallback to timer_early_get_rate().
> +	 */
> +	if (gd->dm_root == NULL)
>   		return timer_early_get_rate();
> -#else
> -		int ret;
>   
> +#endif /* CONFIG_TIMER_EARLY */
> +
> +	/*
> +	 * Check if the virtual timer driver does not yet exist,
> +	 * if not initialize the driver.
> +	 */
> +	if (!gd->timer) {
>   		ret = dm_timer_init();
> +
>   		if (ret)
> -			return ret;
> -#endif
> +			panic("Could not initialize timer (err %d)\n", ret);
>   	}
>   
>   	return timer_get_rate(gd->timer);
> @@ -86,19 +99,30 @@ uint64_t notrace get_ticks(void)
>   	u64 count;
>   	int ret;
>   
> -	if (!gd->timer) {
>   #ifdef CONFIG_TIMER_EARLY
> +
> +	/*
> +	 * Check if the virtual root driver does not yet exist,
> +	 * if not fallback to timer_early_get_rate().
> +	 */
> +	if (gd->dm_root == NULL)

if (!gd->dm_root)

>   		return timer_early_get_count();
> -#else
> -		int ret;
>   
> +#endif /* CONFIG_TIMER_EARLY */
> +
> +	/*
> +	 * Check if the virtual timer driver does not yet exist,
> +	 * if not initialize the driver.
> +	 */
> +	if (!gd->timer) {
>   		ret = dm_timer_init();
> +

Please don't insert empty lines.

>   		if (ret)
>   			panic("Could not initialize timer (err %d)\n", ret);
> -#endif
>   	}
>   
>   	ret = timer_get_count(gd->timer, &count);
> +

ditto

>   	if (ret) {
>   		if (spl_phase() > PHASE_TPL)
>   			panic("Could not read count from timer (err %d)\n",
> 

OK, so as I understand it, the current logic is something like

	if (!gd->timer) {
		if (CONFIG_TIMER_EARLY)
			return early_timer();
		else
			init_dm_timer();
	}
	return dm_timer();

and this patch would change it to be

	if (CONFIG_TIMER_EARLY && !gd->dm_root)
		return early_timer();
	if (!gd->timer)
		init_dm_timer();
	return dm_timer();

which has the consequence (with your next patch) that timer initialization
is always done by the first timer API call, and not by initf_dm. However,
I'm not sure why you want to change this.

--Sean