Re: [PATCH 1/6] dt-bindings: timer: Add Tegra186 & Tegra234 Timer

[Kartik <kkartik@nvidia.com>]

On 14/04/2022 18:20, Rob Herring wrote:
>> timestamp counter. The Tegra234 timer provides sixteen 29-bit timer
>> counters and one 32-bit timestamp counter. Each NV timer selects its
>> timing reference signal from the 1 MHz reference generated by USEC,
>> TSC or either clk_m or OSC. Each TMR can be programmed to generate
>> one-shot, periodic, or watchdog interrupts.
>> 
>> Signed-off-by: Kartik <kkartik@nvidia.com>
>
>Full name please.

This is my legal name.

>
>> ---
>>  .../bindings/timer/nvidia,tegra186-timer.yaml | 116 ++++++++++++++++++
>>  1 file changed, 116 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/timer/nvidia,tegra186-timer.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/timer/nvidia,tegra186-timer.yaml b/Documentation/devicetree/bindings/timer/nvidia,tegra186-timer.yaml
>> new file mode 100644
>> index 000000000000..7841a68d19f3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/timer/nvidia,tegra186-timer.yaml
>> @@ -0,0 +1,116 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/timer/nvidia,tegra186-timer.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: NVIDIA Tegra186 timer
>> +
>> +maintainers:
>> +  - Thierry Reding <treding@nvidia.com>
>> +
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: nvidia,tegra186-timer
>> +    then:
>> +      properties:
>> +        interrupts:
>> +          # Either a single combined interrupt or up to 14 individual interrupts
>
>This can be part of 'description'

I will include this in the description in v2.

>
>> +          minItems: 1
>> +          maxItems: 10
>> +          description: >
>> +            A list of 10 interrupts; one per each timer channels 0 through 9.
>
>Is it 10 or 14? I'm confused.

My bad, it is 10.

>
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: nvidia,tegra234-timer
>> +    then:
>> +      properties:
>> +        interrupts:
>> +          # Either a single combined interrupt or up to 16 individual interrupts
>> +          minItems: 1
>> +          maxItems: 16
>> +          description: >
>> +            A list of 16 interrupts; one per each timer channels 0 through 15.
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - const: nvidia,tegra186-timer
>> +        description: >
>> +          The Tegra186 timer provides ten 29-bit timer counters and one 32-bit
>> +          timestamp counter. Each NV timer selects its timing reference signal
>> +          from the 1 MHz reference generated by USEC, TSC or either clk_m or
>> +          OSC. Each TMR can be programmed to generate one-shot, periodic, or
>> +          watchdog interrupts.
>> +      - const: nvidia,tegra234-timer
>> +        description: >
>> +          The Tegra234 timer provides sixteen 29-bit timer counters and one 32-bit
>> +          timestamp counter. Each NV timer selects its timing reference signal
>> +          from the 1 MHz reference generated by USEC, TSC or either clk_m or
>> +          OSC. Each TMR can be programmed to generate one-shot, periodic, or
>> +          watchdog interrupts.
>
>Move all this description to top-level description leaving out the exact 
>number of counters (as the schema defines that).
>

Do you mean it to be this way?

title: NVIDIA Tegra186 timer

maintainers:
  - Thierry Reding <treding@nvidia.com>

description: >
  The Tegra timer provides 29-bit timer counters and a 32-bit timestamp
  counter. Each NV timer selects its timing reference signal from the 1 MHz
  reference generated by USEC, TSC or either clk_m or OSC. Each TMR can be
  programmed to generate one-shot, periodic, or watchdog interrupts.


properties:
  compatible:
    oneOf:
      - const: nvidia,tegra186-timer
        description: >
          The Tegra186 timer provides ten 29-bit timer counters.
      - const: nvidia,tegra234-timer
        description: >
          The Tegra234 timer provides sixteen 29-bit timer counters.

>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts: true
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +    timer@3010000 {
>> +        compatible = "nvidia,tegra186-timer";
>> +        reg = <0x03010000 0x000e0000>;
>> +        interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
>> +        status = "disabled";
>
>Drop status.
>

I will remove this in v2.

>> +    };
>> +
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +    timer@2080000 {
>> +        compatible = "nvidia,tegra234-timer";
>> +        reg = <0x02080000 0x00121000>;
>> +        interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>,
>> +                     <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
>> +        status = "disabled";
>> +    };
>> -- 
>> 2.17.1
>> 
>>