Re: [PATCH 5/5] xen/arm: Add OMAP5 architected timer initialization codes.

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

On 08/07/2013 11:25 AM, Andrii Anisov wrote:
> 
>     >> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
>     >> index 4ed7882..6ca030a 100644
>     >> --- a/xen/arch/arm/time.c
>     >> +++ b/xen/arch/arm/time.c
>     >> @@ -104,6 +104,7 @@ int __init init_xen_time(void)
>     >>     struct dt_device_node *dev;
>     >>     int res;
>     >>     unsigned int i;
>     >> +    const __be32 *rate;
>     >>
>     >>     dev = dt_find_compatible_node(NULL, NULL, "arm,armv7-timer");
>     >>     if ( !dev )
>     >> @@ -134,7 +135,11 @@ int __init init_xen_time(void)
>     >>     if ( !cpu_has_gentimer )
>     >>         panic("CPU does not support the Generic Timer v1
>     interface.\n");
>     >>
>     >> -    cpu_khz = READ_SYSREG32(CNTFRQ_EL0) / 1000;
>     >> +    rate = dt_get_property(dev, "clock-frequency", NULL);
>     >
>     > You need to check if the length match the size of rate.
>     > Perhaps, you can add an helper dt_property_read_u32 (see
>     > linux/include/linux/of.h).
> 
>     OK.
> 
> 
> I would propose another approach here: just let cpu_khz be set up by
> architecture code.

Linux uses the same approach as Chen.
>From linux/Documentation/devicetree/bindings/arm/arch_timer.txt:

** Timer node properties:
- clock-frequency : The frequency of the main counter, in Hz. Optional.

As Xen uses the arch timer, which is common for all ARMv7 platform, why
does the platform code need to set up cpu_khz?

-- 
Julien