Re: [PATCH 1/2] dt-bindings: timer: Add Realtek SYSTIMER binding

[Krzysztof Kozlowski <krzk@kernel.org>]

On 11/11/2025 10:29, Hao-Wen Ting wrote:
> Add device tree binding documentation for the Realtek SYSTIMER, a 64-bit
> timer that can be used as a tick broadcast timer on multi-core Realtek
> SoCs.
> 
> The SYSTIMER remains active during deep CPU idle states where local

Systimer or SYSTIMER? What is this acronym about?

> timers are powered off, allowing all CPUs to enter power-cut idle states
> simultaneously for improved power efficiency. The timer operates at a
> fixed 1MHz frequency and supports oneshot mode for tick broadcast
> functionality.
> 
> This binding defines the required properties for memory-mapped register
> access and interrupt configuration needed by the timer driver.
Drop sentence, completely redundant. We can read the diff.

> 
> Signed-off-by: Hao-Wen Ting <haowen.ting@realtek.com>
> ---
>  .../bindings/timer/realtek,systimer.yaml      | 54 +++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/timer/realtek,systimer.yaml
> 
> diff --git a/Documentation/devicetree/bindings/timer/realtek,systimer.yaml b/Documentation/devicetree/bindings/timer/realtek,systimer.yaml
> new file mode 100644
> index 000000000000..28ab9b91f45d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/realtek,systimer.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/timer/realtek,systimer.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Realtek SYSTIMER
> +
> +maintainers:
> +  - Hao-Wen Ting <hao-wen.ting@realtek.com>
> +
> +description: |
> +  The Realtek SYSTIMER is a 64-bit timer that can be used as a tick
> +  broadcast timer on multi-core Realtek SoCs. It remains active during
> +  deep CPU idle states where local timers are powered off, allowing all
> +  CPUs to enter power-cut idle states simultaneously for better power
> +  efficiency.
> +
> +  The timer operates at a fixed 1MHz frequency and supports oneshot mode
> +  for tick broadcast functionality.
> +
> +properties:
> +  compatible:
> +    const: realtek,systimer

This is clearly wrong, please read writing bindings. You ALWAYS need SoC
compatible.

See also guidelines for beginners in DTS.


> +
> +  reg:
> +    description: |
> +      Physical base address and length of the timer's memory mapped
> +      registers. The register range contains the 64-bit timestamp counter,
> +      compare value registers, control and status registers.

Drop. Please use recent bindings as your starting work. If you generated
this with LLM, it would mean it is huge waste of our time.

NAK

> +    maxItems: 1
> +
> +  interrupts:
> +    description: |
> +      Timer interrupt specifier. This interrupt is triggered when the
> +      timer compare value matches the current timestamp counter.
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    systimer: systimer@89420 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
If you cannot find a name matching your device, please check in kernel
sources for similar cases or you can grow the spec (via pull request to
DT spec repo).

Drop label, you do not use it.

Best regards,
Krzysztof