On Wed, Jul 27, 2022 at 6:05 PM Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > On 27/07/2022 14:21, Anup Patel wrote: > > On Wed, Jul 27, 2022 at 5:37 PM Krzysztof Kozlowski > > <krzysztof.kozlowski@xxxxxxxxxx> 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. > > > > Historically (since the early days), we never had a timer node in the > > RISC-V world. > > > >> > >> Now that's actual problem: why the RISC-V timer is bound to "riscv" > >> compatible, not to dedicated timer node? How is it related to actual CPU > >> (not SoC)? > > > > The RISC-V timer is always present on all RISC-V platforms because > > Timer is always present also on ARMv8 (and ARMv7) yet it has its node. > > > the "time" CSR is defined by RISC-V privileged specification. The method > > to program per-CPU timer events in either using SBI call or Sstc CSRs. > > Timer is still not part of CPU. Otherwise you are claiming here that CPU > can wakeup CPU... The clocksource (i.e. "time" register) is part of the CPU but it is an alias/copy of a free running system counter whereas clockevent devices (i.e. "mtimecmp" or "stimecmp" registers) are tightly coupled-with/part-of-the CPU. Some of the CPU suspend/idle states may or may not preserve state of timer registers so we might not get a timer interrupt to wake up the cpu from idle state. > > > > > Since, there is no dedicated timer node, we use CPU compatible string > > for probing the per-CPU timer. > > Next time you add a properties: > riscv,saata-can-wake-cpu > riscv,usb-can-wake-cpu > riscv,interrupt-controller-can-wake-cpu > > and so on and keep explaining that "historically" you did not define > separate nodes, so thus must be in CPU node. This is a one-of-case with RISC-V DeviceTree where we are living with the fact that there is no timer DT node. If we add a timer DT node now then we have to deal with compatibility for existing platforms. > > You need to properly reflect hardware in the DTS instead of such hacks. > > Best regards, > Krzysztof Regards, Anup
