On 08/27/2015 08:28 PM, Hanjun Guo wrote:
On 08/27/2015 08:08 PM, Thomas Gleixner wrote:
On Thu, 27 Aug 2015, Hanjun Guo wrote:
On 08/26/2015 03:17 AM, Thomas Gleixner wrote:
On Wed, 26 Aug 2015, Fu Wei wrote:
> /* Initialize per-processor generic timer */ > -static int __init arch_timer_acpi_init(struct acpi_table_header > *table) > +void __init arch_timer_acpi_init(void) > {
And how is that supposed to work when we have next generation CPUs which implement a different timer? You break multisystem kernels that way.
Sorry, I think I missed some context here that I don't understand why the code here will break multisystem kernels? I'm trying to understand the problem here and update the code :)
yes, you are right, If there is a next generation CPUs which implement a different timer, (maybe) this driver can not work. we may need a new timer driver.
But, (1) for now, aarch64 core always has the arch timer(this timer is part of aarch64 architecture). and the existing code make ARM64 kernel "select ARM_ARCH_TIMER " (2) GTDT is designed for generic timer, so in this call " arch_timer_acpi_init" we parse the gtdt info. (3) once we have a ARM64 CPUs which implement a different timer, we may need to select a right timer in the config stage. and this timer may not be described in GTDT. So we can implement another arch_timer_acpi_init by that time in new timer driver.. if the new time still uses GTDT(or new version GTDT), we may need to update gtdt.c for new timer by that time.
That's simply wrong. You want to build kernels which run on both cpus and the selection of the timer happens at runtime depending on the ACPI info. We do the same thing with device tree.
I think the code can do that if I understand correctly. The code for now is that we only support arch timer on ARM64, and this patch set is adding SBSA watchdog timer support which need same function in arch timer, so we move that function to common place.
We will load the driver (arch timer, memory mapped timer) when the ACPI table defines them, which when new timer is coming, that will defined in the ACPI table and load the driver as needed.
Please correct me if I misse something, thanks.
arch_timer_acpi_init() is called from the architecture boot code. So how is that supposed to work with different timers?
Are you going to have bla_timer_acpi_init() and foo_timer_acpi_init() calls as well?
Why not having a something like DT has: DECLARE_....
and the arch_timer_acpi_init() using that to figure out which of the timers to initialize.
Ah, ok, I can fully understand you now, thanks for your patience.
Yes, I agree with you, so this is not a problem for this patch, but for the code implementation of previous code. Actually we are on the road to do as you suggested, we introduced something like #define ACPI_DECLARE(table, name, table_id, subtable, data, fn) [1] in the GICv3 and GIC self probe patch set, and I said that infrastructure can be used as clock declare too, we just trying to not add such dependence on that patch set (it's still on discussion),
If that is ok with you, we will introduce similar DECLARE_ thing for clock declare.
Or we can drop this patch from this patch set, and clean up this patch when the ACPI_DECLARE() infrastructure is ready for upstream.
Thanks Hanjun