Re: [PATCH v3 04/12] dt-bindings: timer: arm,arch_timer: Add optional clock and reset

[Mark Rutland <mark.rutland@arm.com>]

Hi Geert,

On Tue, May 03, 2022 at 04:22:35PM +0200, Geert Uytterhoeven wrote:
> On Tue, May 3, 2022 at 3:12 PM Marc Zyngier <maz@kernel.org> wrote:
> > On 2022-05-03 12:55, Phil Edworthy wrote:
> > > Some SoCs use a gated clock for the timer and the means to reset the
> > > timer.
> > > Hence add these as optional.
> >
> > The architecture is crystal clear on the subject: the counter
> > is in an always-on domain. Why should this be visible to SW?
> > Also, reseting the counter breaks the guaranteed monotonicity
> > we rely on.
> 
> The DT bindings do state:
> 
>   always-on:
>     type: boolean
>     description: If present, the timer is powered through an always-on power
>       domain, therefore it never loses context.
> 
> and (surprisingly?) the absence of this property seems to be the
> norm...

That's the *timer* (i.e. the comparator logic within each CPU which
fires an interrupt), not the *counter* (i.e. the incrementing value fed
by a clock). What this is trying to say is whether that can be relied
upon to cause a wakeup while the CPU is in a low-power state, or whether
it cannot (and hence SW needs to use another timer for the wakeup).

It's legitimate for each timer to not be in an always-on power domain
because it is part of each CPU, whereas the counter is global to the
system.

We can clear up the wording here since it's apparently confusing.

> And:
> 
>   arm,no-tick-in-suspend:
>     type: boolean
>     description: The main counter does not tick when the system is in
>       low-power system suspend on some SoCs. This behavior does not match the
>       Architecture Reference Manual's specification that the system
> counter "must
>       be implemented in an always-on power domain."

This is admittedly a workaround for an integration bug, but it's quite
different and only affects the time jump that can be observed when going
into suspend an exiting from it. Whenever software is running the
counter is incrementing.

> So there's already precedent for clocks that can be disabled.

There's precedent for the clock being disabled in a specific deep sleep
state, not when SW is actively running.

> > Worse case, this belongs to the boot firmware, not the kernel,
> > and I don't think this should be described in the DT.
> 
> "DT describes hardware, not software policy"?

It's still describing the HW. There's plenty of other always-on stuff
that we don't describe because for all intents and purposes it is always
on.

Note that this being always-on isn't just a Linux thing; that affects
plenty of other SW which may run and it's an *architectural property*
that's apparently being violated.

Thanks,
Mark.