On Thu, Sep 29, 2016 at 02:17:13AM +0800, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
The patch update arm_arch_timer driver to use the function provided by the new GTDT driver of ACPI. By this way, arm_arch_timer.c can be simplified, and separate all the ACPI GTDT knowledge from this timer driver.
Signed-off-by: Fu Wei fu.wei@linaro.org Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
This generally looks fine, but:
- arch_timer_ppi[PHYS_SECURE_PPI] = acpi_gtdt_map_ppi(PHYS_SECURE_PPI);
As mentioned on the prior patch, I think we shouldn't bother parsing the secure interrupt, given the problem with the GSIV, and the fact that we shouldn't need it in Linux.
- arch_timer_ppi[PHYS_NONSECURE_PPI] = acpi_gtdt_map_ppi(PHYS_NONSECURE_PPI);
- arch_timer_ppi[VIRT_PPI] = acpi_gtdt_map_ppi(VIRT_PPI);
- arch_timer_ppi[HYP_PPI] = acpi_gtdt_map_ppi(HYP_PPI);
- /* Always-on capability */
- arch_timer_c3stop = acpi_gtdt_c3stop();
... I think we should check the flag on the relevant interrupt, though that's worth clarifying.
- /* Always-on capability */
- arch_timer_c3stop = !(gtdt->non_secure_el1_flags & ACPI_GTDT_ALWAYS_ON);
- if (timer_count < 0)
pr_err("Failed to get platform timer info.\n");
Why don't we log this in the code that would try to initialise the MMIO timer? We can still fail after this.
Thanks, Mark.