Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

From: Heiko Stübner
Date: Wed Nov 26 2014 - 07:24:37 EST


Hi Daniel,

Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
> Hi Doug, Olof,
>
> IIUC, it sounds like this patch is needed from some other patches in
> arm-soc. Olof was proposing to take this patch through its tree to
> facilitate the integration.
>
> Olof, is it this patch you were worried about ?

I think this is one of two patches in question.

"clocksource: arch_timer: Fix code to use physical timers when requested" [0]
would be the second one.

And the patch for arm-soc that Olof means would be "ARM: dts: rk3288: add
arm,cpu-registers-not-fw-configured" [1].


Heiko

[0]
https://git.linaro.org/people/daniel.lezcano/linux.git/commit/04f71c2cae54dc26b2a236c787ea8d56c174150b
[1] https://lkml.org/lkml/2014/11/25/975

>
> Thanks
>
> -- Daniel
>
> On 11/20/2014 12:01 AM, Doug Anderson wrote:
> > Daniel,
> >
> > On Wed, Oct 8, 2014 at 12:33 AM, Sonny Rao <sonnyrao@xxxxxxxxxxxx> wrote:
> >> From: Doug Anderson <dianders@xxxxxxxxxxxx>
> >>
> >> Some 32-bit (ARMv7) systems are architected like this:
> >>
> >> * The firmware doesn't know and doesn't care about hypervisor mode and
> >>
> >> we don't want to add the complexity of hypervisor there.
> >>
> >> * The firmware isn't involved in SMP bringup or resume.
> >>
> >> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
> >>
> >> between the virtual and physical counters. Each core gets a
> >> different random offset.
> >>
> >> * The device boots in "Secure SVC" mode.
> >>
> >> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
> >>
> >> CNTHCTL.PL1PCTEN (both default to 1 at reset)
> >>
> >> On systems like the above, it doesn't make sense to use the virtual
> >> counter. There's nobody managing the offset and each time a core goes
> >> down and comes back up it will get reinitialized to some other random
> >> value.
> >>
> >> This adds an optional property which can inform the kernel of this
> >> situation, and firmware is free to remove the property if it is going
> >> to initialize the CNTVOFF registers when each CPU comes out of reset.
> >>
> >> Currently, the best course of action in this case is to use the
> >> physical timer, which is why it is important that CNTHCTL hasn't been
> >> changed from its reset value and it's a reasonable assumption given
> >> that the firmware has never entered HYP mode.
> >>
> >> Note that it's been said that on ARMv8 systems the firmware and
> >> kernel really can't be architected as described above. That means
> >> using the physical timer like this really only makes sense for ARMv7
> >> systems.
> >>
> >> Signed-off-by: Doug Anderson <dianders@xxxxxxxxxxxx>
> >> Signed-off-by: Sonny Rao <sonnyrao@xxxxxxxxxxxx>
> >> Reviewed-by: Mark Rutland <mark.rutland@xxxxxxx>
> >> ---
> >> Changes in v2:
> >> - Add "#ifdef CONFIG_ARM" as per Will Deacon
> >>
> >> Changes in v3:
> >> - change property name to arm,cntvoff-not-fw-configured and specify
> >>
> >> that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
> >> of 1 as per Mark Rutland
> >>
> >> Changes in v4:
> >> - change property name to arm,cpu-registers-not-fw-configured and
> >>
> >> specify that all cpu registers must have architected reset values
> >> per Mark Rutland
> >>
> >> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
> >>
> >> Arnd Bergmann
> >>
> >> ---
> >>
> >> Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
> >> drivers/clocksource/arm_arch_timer.c | 8 ++++++++
> >> 2 files changed, 16 insertions(+)
> >
> > Do you know what the status of this patch is? This patch and Sonny's
> > patch at <https://patchwork.kernel.org/patch/5051901/> are needed on
> > Rockchip rk3288 for some specific things:
> >
> > 1. To make SMP happy with coreboot.
> >
> > 2. To (I assume) make SMP happy after S2R (no matter which firmware is
> > used since I don't think anyone has PSCI for rk3288).
> >
> > ...we still need a DTS entry atop these patches, but that's trivial to
> > add once these land.
> >
> > Thanks!

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/