* [RFC PATCH v1] xen/arm: split the init_xen_time() in 2 parts
@ 2015-01-23 15:49 Oleksandr Tyshchenko
2015-01-26 13:32 ` Julien Grall
0 siblings, 1 reply; 3+ messages in thread
From: Oleksandr Tyshchenko @ 2015-01-23 15:49 UTC (permalink / raw)
To: xen-devel; +Cc: julien.grall, tim, ian.campbell, stefano.stabellini
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>
---
xen/arch/arm/setup.c | 2 ++
xen/arch/arm/time.c | 71 ++++++++++++++++++++++++++++++++------------------
xen/include/xen/time.h | 1 +
3 files changed, 48 insertions(+), 26 deletions(-)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index f49569d..dbd7d5a 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -743,6 +743,8 @@ void __init start_xen(unsigned long boot_phys_offset,
init_IRQ();
+ preinit_xen_time();
+
dt_uart_init();
console_init_preirq();
console_init_ring();
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 455f217..b2560db 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -61,25 +61,56 @@ 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 __init struct dt_device_node *find_timer_node(void)
+{
+ static const struct dt_device_match timer_ids[] __initconst =
+ {
+ DT_MATCH_TIMER,
+ { /* sentinel */ },
+ };
+
+ return dt_find_matching_node(NULL, timer_ids);
+}
+
+/* Set up the timer on the boot CPU (early init function) */
+int __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);
+ dev = find_timer_node();
if ( !dev )
panic("Unable to find a compatible timer in the device tree");
dt_device_set_used_by(dev, DOMID_XEN);
+ res = platform_init_time();
+ if ( res )
+ panic("Timer: Cannot initialize platform timer");
+
+ 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);
+
+ return 0;
+}
+
+/* Set up the timer on the boot CPU (late init function) */
+int __init init_xen_time(void)
+{
+ struct dt_device_node *dev;
+ int res;
+ unsigned int i;
+
+ dev = find_timer_node();
+ if ( !dev )
+ panic("Unable to find a compatible timer in the device tree");
+
/* Retrieve all IRQs for the timer */
for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
{
@@ -90,27 +121,15 @@ int __init init_xen_time(void)
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..fc49f71 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);
+extern int preinit_xen_time(void);
extern void cstate_restore_tsc(void);
extern unsigned long cpu_khz;
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC PATCH v1] xen/arm: split the init_xen_time() in 2 parts
2015-01-23 15:49 [RFC PATCH v1] xen/arm: split the init_xen_time() in 2 parts Oleksandr Tyshchenko
@ 2015-01-26 13:32 ` Julien Grall
2015-01-26 13:54 ` Oleksandr Tyshchenko
0 siblings, 1 reply; 3+ messages in thread
From: Julien Grall @ 2015-01-26 13:32 UTC (permalink / raw)
To: Oleksandr Tyshchenko, xen-devel; +Cc: tim, ian.campbell, stefano.stabellini
Hi Oleksandr,
Thank you for the patch. See few comments below.
On 23/01/15 15:49, 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>
> ---
> xen/arch/arm/setup.c | 2 ++
> xen/arch/arm/time.c | 71 ++++++++++++++++++++++++++++++++------------------
> xen/include/xen/time.h | 1 +
> 3 files changed, 48 insertions(+), 26 deletions(-)
>
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index f49569d..dbd7d5a 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -743,6 +743,8 @@ void __init start_xen(unsigned long boot_phys_offset,
>
> init_IRQ();
>
> + preinit_xen_time();
> +
> dt_uart_init();
> console_init_preirq();
> console_init_ring();
> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
> index 455f217..b2560db 100644
> --- a/xen/arch/arm/time.c
> +++ b/xen/arch/arm/time.c
> @@ -61,25 +61,56 @@ 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 __init struct dt_device_node *find_timer_node(void)
> +{
> + static const struct dt_device_match timer_ids[] __initconst =
> + {
> + DT_MATCH_TIMER,
> + { /* sentinel */ },
> + };
> +
> + return dt_find_matching_node(NULL, timer_ids);
> +}
If you store the device tree pointer into a static variable, you won't
able to look at twice the node (in preinit_xen_time and init_xen_time).
> +
> +/* Set up the timer on the boot CPU (early init function) */
> +int __init preinit_xen_time(void)
You always return 0 and never check the return. Please use 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);
> + dev = find_timer_node();
> if ( !dev )
> panic("Unable to find a compatible timer in the device tree");
>
> dt_device_set_used_by(dev, DOMID_XEN);
>
> + res = platform_init_time();
If you move platform_init_time in preinit_xen_time, you also have to
move platform_init before calling preinit_xen_time.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC PATCH v1] xen/arm: split the init_xen_time() in 2 parts
2015-01-26 13:32 ` Julien Grall
@ 2015-01-26 13:54 ` Oleksandr Tyshchenko
0 siblings, 0 replies; 3+ messages in thread
From: Oleksandr Tyshchenko @ 2015-01-26 13:54 UTC (permalink / raw)
To: Julien Grall
Cc: Stefano Stabellini, Tim Deegan, Ian Campbell,
xen-devel@lists.xen.org
On Mon, Jan 26, 2015 at 3:32 PM, Julien Grall <julien.grall@linaro.org> wrote:
> Hi Oleksandr,
Hi Julien
Thank you for your comments.
>
> Thank you for the patch. See few comments below.
>
> On 23/01/15 15:49, 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>
>> ---
>> xen/arch/arm/setup.c | 2 ++
>> xen/arch/arm/time.c | 71 ++++++++++++++++++++++++++++++++------------------
>> xen/include/xen/time.h | 1 +
>> 3 files changed, 48 insertions(+), 26 deletions(-)
>>
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index f49569d..dbd7d5a 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -743,6 +743,8 @@ void __init start_xen(unsigned long boot_phys_offset,
>>
>> init_IRQ();
>>
>> + preinit_xen_time();
>> +
>> dt_uart_init();
>> console_init_preirq();
>> console_init_ring();
>> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
>> index 455f217..b2560db 100644
>> --- a/xen/arch/arm/time.c
>> +++ b/xen/arch/arm/time.c
>> @@ -61,25 +61,56 @@ 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 __init struct dt_device_node *find_timer_node(void)
>> +{
>> + static const struct dt_device_match timer_ids[] __initconst =
>> + {
>> + DT_MATCH_TIMER,
>> + { /* sentinel */ },
>> + };
>> +
>> + return dt_find_matching_node(NULL, timer_ids);
>> +}
>
> If you store the device tree pointer into a static variable, you won't
> able to look at twice the node (in preinit_xen_time and init_xen_time).
Agree.
>
>> +
>> +/* Set up the timer on the boot CPU (early init function) */
>> +int __init preinit_xen_time(void)
>
> You always return 0 and never check the return. Please use void.
ok
>
>> {
>> - 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);
>> + dev = find_timer_node();
>> if ( !dev )
>> panic("Unable to find a compatible timer in the device tree");
>>
>> dt_device_set_used_by(dev, DOMID_XEN);
>>
>> + res = platform_init_time();
>
> If you move platform_init_time in preinit_xen_time, you also have to
> move platform_init before calling preinit_xen_time.
ok
>
> Regards,
>
> --
> Julien Grall
--
Oleksandr Tyshchenko | Embedded Dev
GlobalLogic
www.globallogic.com
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-01-26 13:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-23 15:49 [RFC PATCH v1] xen/arm: split the init_xen_time() in 2 parts Oleksandr Tyshchenko
2015-01-26 13:32 ` Julien Grall
2015-01-26 13:54 ` Oleksandr Tyshchenko
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.