lkml.org 
[lkml]   [2016]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v14 5/9] clocksource/drivers/arm_arch_timer: Simplify ACPI support code.
    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.

    \
     
     \ /
      Last update: 2016-10-20 18:59    [W:4.120 / U:0.296 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site