On Thu, Sep 29, 2016 at 02:17:13AM +0800, fu.wei@xxxxxxxxxx wrote:
> From: Fu Wei <fu.wei@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Signed-off-by: Hanjun Guo <hanjun.guo@xxxxxxxxxx>
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
