Re: [PATCH v2 1/2] dt-bindings: riscv: Add optional DT property riscv,timer-can-wake-cpu

[Sudeep Holla]

On Wed, Jul 27, 2022 at 07:15:28PM +0530, Anup Patel wrote:
> Hi Sudeep,
> 
> On Wed, Jul 27, 2022 at 6:16 PM Sudeep Holla <sudeep.holla@xxxxxxx> wrote:
> >
> > On Wed, Jul 27, 2022 at 02:07:50PM +0200, Krzysztof Kozlowski wrote:
> > > On 27/07/2022 13:43, Anup Patel wrote:
> > > > We add an optional DT property riscv,timer-can-wake-cpu which if present
> > > > in CPU DT node then CPU timer is always powered-on and never loses context.
> > > >
> > > > Signed-off-by: Anup Patel <apatel@xxxxxxxxxxxxxxxx>
> > > > ---
> > > >  Documentation/devicetree/bindings/riscv/cpus.yaml | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > > > index d632ac76532e..b60b64b4113a 100644
> > > > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> > > > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > > > @@ -78,6 +78,12 @@ properties:
> > > >        - rv64imac
> > > >        - rv64imafdc
> > > >
> > > > +  riscv,timer-can-wake-cpu:
> > > > +    type: boolean
> > > > +    description:
> > > > +      If present, the timer interrupt can wake up the CPU from
> > > > +      suspend/idle state.
> > >
> > > Isn't this a property of a timer, not CPU? IOW, your timer node should
> > > have "wakeup-source" property.
> > >
> >
> > I agree on the concept that this is property of the timer and not CPU.
> > However we generally don't need to use wakeup-source property for timer
> > as we ideally use this for waking up from system sleep state and we don't
> > want to be running timer when we enter the state.
> 
> It seems ARM is using two separate timer DT properties: one for
> system suspend (i.e. arm,no-tick-in-suspend) and another for CPU
> system (i.e. always-on). Is this understanding correct ?
>

TBH, I hadn't looked at "arm,no-tick-in-suspend" before though it seem to
be there for few years now. Based on the log, I see this as quirk for some
platform and not used on many platforms. Ideally to account how much time
was spent in suspend, we could use the counter as it continues to tick
during the suspend as well i.e. just the timers are not available if
not always. This was added to handle systems that are not conformant to
the expectation of always available counter.

-- 
Regards,
Sudeep