Tickless timer regression in v5.18.3 on riscv

[Anup Patel]

On Fri, Jun 24, 2022 at 1:15 PM Eva Kotova <nyandarknessgirl at gmail.com> wrote:
>
> On Thu, Jun 23, 2022 at 8:59 PM Anup Patel <anup at brainfault.org> wrote:
>  > I suggest we introduce an optional "riscv,timer-always-on" DT
>  > property for each HART. If all HART DT nodes have this property then
>  > we don't set C3STOP flag in clockevent device, else we set it.
>
> Is there any particular reason not to implement the opposite DT property
> "riscv,timer-s3stop" for timers that stop in suspend?

The DT property definition should be about describing HW behavior and
not about describing SW features to be used.

The "riscv,timer-always-on" DT property set in a HART means the timer
in the corresponding HART is always on irrespective to idle state of HART.

Whereas, the "riscv,timer-c3stop" DT property set in a HART means SW
should use the C3STOP flag for the clock_event_device of this HART.

Clearly, "riscv,timer-always-on" DT property is more appropriate since
it describes HW behaviour.

> Right now, this patch breaks the behavior of previously fine working
> boards (worse sleep precision, higher idle consumption), and these
> boards have none of the mentioned issues, so why should they degrade
> after a kernel update?

The breakage was unintentional.

What we quickly need is a patch series (2 patches) for setting C3STOP
based on "riscv,timer-always-on" (Just like ARM arch timer driver). The
first patch will add documentation of "riscv,timer-always-on" in DT bindings
and second patch will update drivers/clocksource/timer-riscv.c

> The CLINT timer is already well-defined and worked for years, i don't
> see how suspend issues on particular hardware can justify going against
> the official spec.
>
> I do really think this should be addressed by the platform SBI
> implementation (that's why SBI exists after all?..) or DT properties for
> suspend-affected hardware, not finding holes in SBI spec and introducing
> fragmentation in the SW ecosystem.

Whether, timer will be kept running or be turned-off in a non-retentive
suspend state is a microarchitecture detail (or CPU implementation detail).

This can't be enforced by RISC-V SBI specification because SBI calls are
runtime services and do not describe HW. The best way is to discover this
HW behaviour (or CPU implementation detail) is through DT or ACPI.

Regards,
Anup