Re: [PATCH v3 5/9] dt-bindings: timer: Add schema for realtek,otto-timer

From: Sander Vanheule
Date: Sat Jun 29 2024 - 16:46:56 EST


Hi Chris,

Thanks for submitting these patches!

On Thu, 2024-06-27 at 16:33 +1200, Chris Packham wrote:
> Add the devicetree schema for the realtek,otto-timer present on a number
> of Realtek SoCs.
>
> Signed-off-by: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx>
> ---
[...]

> +
> +  reg:
> +    items:
> +      - description: timer0 registers
> +      - description: timer1 registers
> +      - description: timer2 registers
> +      - description: timer3 registers
> +      - description: timer4 registers
> +
> +  clocks:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: timer0 interrupt
> +      - description: timer1 interrupt
> +      - description: timer2 interrupt
> +      - description: timer3 interrupt
> +      - description: timer4 interrupt

Instead of providing a (SoC dependent) number of reg and interrupt items, can't we just
provide one reg+interrupt per timer and instantiate one block for however many timers the
SoC has?

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    timer@3200 {
> +      compatible = "realtek,rtl9302-timer", "realtek,otto-timer";
> +      reg = <0x3200 0x10>, <0x3210 0x10>, <0x3220 0x10>,
> +            <0x3230 0x10>, <0x3240 0x10>;
> +
> +      interrupt-parent = <&intc>;
> +      interrupts = <7>, <8>, <9>, <10>, <11>;
> +      clocks = <&lx_clk>;
> +    };

So this would become:
timer@3200 {
compatible = ...
reg = <0x3200 0x10>;
interrupt-parent = <&intc>;
interrupts = <7>;
...
};
timer@3210 {
compatible = ...
reg = <0x3210 0x10>;
interrupt-parent = <&intc>;
interrupts = <8>;
...
};
...

More verbose, but it also makes the binding a bit simpler. The driver can then still do
whatever it wants with all the timers that are registered, although some more resource
locking might be required.

Best,
Sander