Re: [PATCH v3] xen/arm: split the init_xen_time() in 2 parts

[Julien Grall <julien.grall@linaro.org>]

Hi Oleksandr,

Thank you for the quick change.

On 27/01/2015 18:59, Oleksandr Tyshchenko wrote:
> Create preinit_xen_time() and move to it minimum required
> subset of operations needed to properly initialized
> cpu_khz and boot_count vars. This is allow us to use udelay()
> immediately after the call.
>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr.tyshchenko@globallogic.com>
> CC: Julien Grall <julien.grall@linaro.org>

Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

> ---
> Changes in v2:
> 1. Move timer_ids out of the find_timer_node() and frop this func.
> 2. Use void as a return value for preinit_xen_time().
> 3. Move platform_init() before calling preinit_xen_time().
>
> Changes in v3:
> 1. Store the device node for timer outside the funcs.
>
> ---
>   xen/arch/arm/setup.c   |  6 ++++--
>   xen/arch/arm/time.c    | 57 ++++++++++++++++++++++++++++----------------------
>   xen/include/xen/time.h |  1 +
>   3 files changed, 37 insertions(+), 27 deletions(-)
>
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index f49569d..a916ca6 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -743,6 +743,10 @@ void __init start_xen(unsigned long boot_phys_offset,
>
>       init_IRQ();
>
> +    platform_init();
> +
> +    preinit_xen_time();
> +
>       dt_uart_init();
>       console_init_preirq();
>       console_init_ring();
> @@ -751,8 +755,6 @@ void __init start_xen(unsigned long boot_phys_offset,
>
>       processor_id();
>
> -    platform_init();
> -
>       smp_init_cpus();
>       cpus = smp_get_max_cpus();
>
> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
> index 0add494..04291f9 100644
> --- a/xen/arch/arm/time.c
> +++ b/xen/arch/arm/time.c
> @@ -61,56 +61,63 @@ unsigned int timer_get_irq(enum timer_ppi ppi)
>       return muldiv64(ns, 1000 * cpu_khz, SECONDS(1));
>   }
>
> -/* Set up the timer on the boot CPU */
> -int __init init_xen_time(void)
> +static __initdata struct dt_device_node *timer;
> +
> +/* Set up the timer on the boot CPU (early init function) */
> +void __init preinit_xen_time(void)
>   {
>       static const struct dt_device_match timer_ids[] __initconst =
>       {
>           DT_MATCH_TIMER,
>           { /* sentinel */ },
>       };
> -    struct dt_device_node *dev;
>       int res;
> -    unsigned int i;
>       u32 rate;
>
> -    dev = dt_find_matching_node(NULL, timer_ids);
> -    if ( !dev )
> +    timer = dt_find_matching_node(NULL, timer_ids);
> +    if ( !timer )
>           panic("Unable to find a compatible timer in the device tree");
>
> -    dt_device_set_used_by(dev, DOMID_XEN);
> +    dt_device_set_used_by(timer, DOMID_XEN);
> +
> +    res = platform_init_time();
> +    if ( res )
> +        panic("Timer: Cannot initialize platform timer");
> +
> +    res = dt_property_read_u32(timer, "clock-frequency", &rate);
> +    if ( res )
> +        cpu_khz = rate / 1000;
> +    else
> +        cpu_khz = READ_SYSREG32(CNTFRQ_EL0) / 1000;
> +
> +    boot_count = READ_SYSREG64(CNTPCT_EL0);
> +}
> +
> +/* Set up the timer on the boot CPU (late init function) */
> +int __init init_xen_time(void)
> +{
> +    int res;
> +    unsigned int i;
>
>       /* Retrieve all IRQs for the timer */
>       for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
>       {
> -        res = platform_get_irq(dev, i);
> +        res = platform_get_irq(timer, i);
>
>           if ( res < 0 )
>               panic("Timer: Unable to retrieve IRQ %u from the device tree", i);
>           timer_irq[i] = res;
>       }
>
> -    printk("Generic Timer IRQ: phys=%u hyp=%u virt=%u\n",
> -           timer_irq[TIMER_PHYS_NONSECURE_PPI],
> -           timer_irq[TIMER_HYP_PPI],
> -           timer_irq[TIMER_VIRT_PPI]);
> -
> -    res = platform_init_time();
> -    if ( res )
> -        panic("Timer: Cannot initialize platform timer");
> -
>       /* Check that this CPU supports the Generic Timer interface */
>       if ( !cpu_has_gentimer )
>           panic("CPU does not support the Generic Timer v1 interface");
>
> -    res = dt_property_read_u32(dev, "clock-frequency", &rate);
> -    if ( res )
> -        cpu_khz = rate / 1000;
> -    else
> -        cpu_khz = READ_SYSREG32(CNTFRQ_EL0) / 1000;
> -
> -    boot_count = READ_SYSREG64(CNTPCT_EL0);
> -    printk("Using generic timer at %lu KHz\n", cpu_khz);
> +    printk("Generic Timer IRQ: phys=%u hyp=%u virt=%u Freq: %lu KHz\n",
> +           timer_irq[TIMER_PHYS_NONSECURE_PPI],
> +           timer_irq[TIMER_HYP_PPI],
> +           timer_irq[TIMER_VIRT_PPI],
> +           cpu_khz);
>
>       return 0;
>   }
> diff --git a/xen/include/xen/time.h b/xen/include/xen/time.h
> index 709501f..bb6259d 100644
> --- a/xen/include/xen/time.h
> +++ b/xen/include/xen/time.h
> @@ -12,6 +12,7 @@
>   #include <public/xen.h>
>
>   extern int init_xen_time(void);
> +void preinit_xen_time(void);
>   extern void cstate_restore_tsc(void);
>
>   extern unsigned long cpu_khz;
>

-- 
Julien Grall