From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH RFC 16/35] ARM64 / ACPI: Parse GTDT to initialize timer Date: Wed, 04 Feb 2015 21:51:27 +0000 Message-ID: <54D2945F.8070300@linaro.org> References: <1423058539-26403-1-git-send-email-parth.dixit@linaro.org> <1423058539-26403-17-git-send-email-parth.dixit@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1423058539-26403-17-git-send-email-parth.dixit@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: parth.dixit@linaro.org, xen-devel@lists.xen.org Cc: ian.campbell@citrix.com, Naresh Bhat , tim@xen.org, stefano.stabellini@citrix.com, jbeulich@suse.com, christoffer.dall@linaro.org List-Id: xen-devel@lists.xenproject.org Hi Parth, On 04/02/2015 14:02, parth.dixit@linaro.org wrote: > From: Naresh Bhat > > Parse GTDT (Generic Timer Descriptor Table) to initialize timer. > Using the information presented by GTDT to initialize the arch > timer (not momery-mapped). memory-mapped > Signed-off-by: Naresh Bhat > Signed-off-by: Parth Dixit > --- > xen/arch/arm/setup.c | 6 +++++ > xen/arch/arm/time.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++ > xen/include/xen/time.h | 1 + > 3 files changed, 73 insertions(+) > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index 569b2da..af9f429 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -775,7 +775,12 @@ void __init start_xen(unsigned long boot_phys_offset, > smp_init_cpus(); > cpus = smp_get_max_cpus(); > > +/* Comment for now take it after GIC initialization */ > +#if defined(CONFIG_ACPI) && defined(CONFIG_ARM_64) > + init_xen_acpi_time(); > +#else > init_xen_time(); > +#endif This is clearly wrong. A same Xen binary should run on both ACPI and DT when ACPI is enabled. Also, I would take care of the different between ACPI and DT in init_xen_time, rather than ifdefining here. It will make the code more cleaner. > gic_init(); > > @@ -789,6 +794,7 @@ void __init start_xen(unsigned long boot_phys_offset, > xsm_dt_init(); > > init_maintenance_interrupt(); > + Spurious change. > init_timer_interrupt(); > > timer_init(); > diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c > index 0add494..0d4c26d 100644 > --- a/xen/arch/arm/time.c > +++ b/xen/arch/arm/time.c > @@ -29,6 +29,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -239,6 +240,71 @@ struct tm wallclock_time(uint64_t *ns) > return (struct tm) { 0 }; > } > > +#if defined(CONFIG_ARM_64) && defined(CONFIG_ACPI) #ifdef CONFIG_ACPI > +#define ACPI_GTDT_INTR_MASK 0x3 > + > +static int GTDT_INTRL_TAB[] ={ DT_IRQ_TYPE_LEVEL_HIGH, DT_IRQ_TYPE_EDGE_RISING, > + DT_IRQ_TYPE_LEVEL_LOW ,DT_IRQ_TYPE_EDGE_FALLING}; > + > +/* Initialize per-processor generic timer */ > +static int __init arch_timer_acpi_init(struct acpi_table_header *table) > +{ > + int res; > + int type; > + struct acpi_table_gtdt *gtdt; > + > + gtdt = (struct acpi_table_gtdt *)table; > + > + /* Initialize all the generic timer IRQ variable from GTDT table */ > + > + type = GTDT_INTRL_TAB[gtdt->non_secure_el1_flags & ACPI_GTDT_INTR_MASK]; > + acpi_set_irq(gtdt->non_secure_el1_interrupt, type); > + timer_irq[TIMER_PHYS_NONSECURE_PPI] = gtdt->non_secure_el1_interrupt; > + > + type = GTDT_INTRL_TAB[gtdt->secure_el1_flags & ACPI_GTDT_INTR_MASK]; > + acpi_set_irq(gtdt->secure_el1_interrupt, type); > + timer_irq[TIMER_PHYS_SECURE_PPI] = gtdt->secure_el1_interrupt; > + > + type = GTDT_INTRL_TAB[gtdt->non_secure_el2_flags & ACPI_GTDT_INTR_MASK]; > + acpi_set_irq(gtdt->non_secure_el2_interrupt, type); > + timer_irq[TIMER_HYP_PPI] = gtdt->non_secure_el2_interrupt; > + > + type = GTDT_INTRL_TAB[gtdt->virtual_timer_flags & ACPI_GTDT_INTR_MASK]; > + acpi_set_irq(gtdt->virtual_timer_interrupt, type); > + timer_irq[TIMER_VIRT_PPI] = gtdt->virtual_timer_interrupt; > + > + printk("Generic Timer IRQ from ACPI GTDT: 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(); The platform code is DT-centrict. Although, most of the code below is similar to the Timer DT initialization. Please factor it. > + if ( res ) > + printk("Timer: Cannot initialize platform timer"); > + > + /* Check that this CPU supports the Generic Timer interface */ > + if ( !cpu_has_gentimer ) > + printk("CPU does not support the Generic Timer v1 interface"); > + > + cpu_khz = READ_SYSREG32(CNTFRQ_EL0) / 1000; > + > + boot_count = READ_SYSREG64(CNTPCT_EL0); > + > + printk("Using generic timer at %lu KHz\n", cpu_khz); > + > + return 0; > +} > + > +int __init init_xen_acpi_time(void) > +{ > + /* Initialize all the generic timers presented in GTDT */ > + acpi_table_parse(ACPI_SIG_GTDT, arch_timer_acpi_init); > + return 0; > +} > + > +#endif > + > /* > * Local variables: > * mode: C > diff --git a/xen/include/xen/time.h b/xen/include/xen/time.h > index 709501f..4598a0c 100644 > --- a/xen/include/xen/time.h > +++ b/xen/include/xen/time.h > @@ -11,6 +11,7 @@ > #include > #include > > +extern int init_xen_acpi_time(void); This is ARM specific, please move the prototype in asm-arm/time.h. > extern int init_xen_time(void); > extern void cstate_restore_tsc(void); > > Regards, -- Julien Grall