Re: [PATCH] ARM: dts: Update arch timer node with clock frequency

[Olof Johansson <olof@lixom.net>]

On Wed, Oct 09, 2013 at 04:48:32PM -0500, Rob Herring wrote:
> On Tue, Oct 8, 2013 at 5:15 PM, Olof Johansson <olof@lixom.net> wrote:
> > [Adding Tony, who reported a mainline booting issue, and Sean who
> > helped me track this down]
> >
> > On Mon, Sep 23, 2013 at 7:15 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> >> On Sat, Sep 21, 2013 at 04:24:59PM +0100, Tomasz Figa wrote:
> >>> Hi Yuvaraj,
> >>>
> >>> On Wednesday 18 of September 2013 15:41:53 Yuvaraj Kumar C D wrote:
> >>> > Without the "clock-frequency" property in arch timer node, could able
> >>> > to see the below crash dump.
> >>> [snip]
> >>> > diff --git a/arch/arm/boot/dts/exynos5250.dtsi
> >>> > b/arch/arm/boot/dts/exynos5250.dtsi index 7d7cc77..668ce5d 100644
> >>> > --- a/arch/arm/boot/dts/exynos5250.dtsi
> >>> > +++ b/arch/arm/boot/dts/exynos5250.dtsi
> >>> > @@ -96,6 +96,7 @@
> >>> >                          <1 14 0xf08>,
> >>> >                          <1 11 0xf08>,
> >>> >                          <1 10 0xf08>;
> >>> > +           clock-frequency = <24000000>;
> >>>
> >>> Shouldn't it rather come from some clock provided by some clock controller
> >>> instead?
> >>>
> >>> The frequency would be then retrieved using clk_get_rate() on a clock
> >>> received by clk_get(), specified in device tree using generic clock
> >>> bindings.
> >>
> >> If the bootloader has initialised the generic timer correctly, the
> >> CNTFRQ register should contain the clock frequency, independent of any
> >> external clock.
> >
> > So, we just sat here to bisect a problem on the Samsung Chromebook
> > where we hit exactly this problem. The read-only firmware on the
> > device does not set CNTFRQ at boot, so this fails.
> >
> > Apparantly the u-boot that comes with Arndale sets it, so I haven't
> > seen this error on that platform.
> >
> >> Having the bootloader set CNTFRQ is by far the preferable solution, it
> >> is architected for this purpose.
> >
> > Unfortunately there is now real hardware out there that needs this due
> > to firmware bugs / missing features, so there's little other choice.
> > :(
> >
> > I'll pick this patch up in the fixes branch for 3.12, unless someone
> > complains loudly.
> 
> Perhaps the subject should say something about this only applying to exynos.

Definitely, and I'll add a comment as Mark requested.


-Olof