[PATCH] dt-bindings: timer: renesas,tmu: Document input capture interrupt

[PATCH] dt-bindings: timer: renesas,tmu: Document input capture interrupt

[Geert Uytterhoeven]

Some Timer Unit (TMU) instances with 3 channels support a fourth
interrupt: an input capture interrupt for the third channel.

While at it, document the meaning of the four interrupts, and add
"interrupt-names" for clarity.

Update the example to match reality.

Inspired by a patch by Yoshinori Sato for SH.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
The corresponding DTS updates can be found in series "[PATCH 0/2]
ARM/arm64: dts: renesas: Improve TMU interrupt descriptions".
https://lore.kernel.org/r/cover.1705325654.git.geert+renesas@glider.be

 .../devicetree/bindings/timer/renesas,tmu.yaml | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/renesas,tmu.yaml b/Documentation/devicetree/bindings/timer/renesas,tmu.yaml
index a67e427a9e7e22aa..9a823224c144f7d4 100644
--- a/Documentation/devicetree/bindings/timer/renesas,tmu.yaml
+++ b/Documentation/devicetree/bindings/timer/renesas,tmu.yaml
@@ -46,7 +46,19 @@ properties:
 
   interrupts:
     minItems: 2
-    maxItems: 3
+    items:
+      - description: Underflow interrupt 0
+      - description: Underflow interrupt 1
+      - description: Underflow interrupt 2
+      - description: Input capture interrupt 2
+
+  interrupt-names:
+    minItems: 2
+    items:
+      - const: tuni0
+      - const: tuni1
+      - const: tuni2
+      - const: ticpi2
 
   clocks:
     maxItems: 1
@@ -100,7 +112,9 @@ examples:
             reg = <0xffd80000 0x30>;
             interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>,
                          <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>,
-                         <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
+                         <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "tuni0", "tuni1", "tuni2", "ticpi2";
             clocks = <&mstp0_clks R8A7779_CLK_TMU0>;
             clock-names = "fck";
             power-domains = <&sysc R8A7779_PD_ALWAYS_ON>;
-- 
2.34.1

Re: [PATCH] dt-bindings: timer: renesas,tmu: Document input capture interrupt

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2484 bytes --]

On Mon, Jan 15, 2024 at 02:45:39PM +0100, Geert Uytterhoeven wrote:
> Some Timer Unit (TMU) instances with 3 channels support a fourth
> interrupt: an input capture interrupt for the third channel.
> 
> While at it, document the meaning of the four interrupts, and add
> "interrupt-names" for clarity.
> 
> Update the example to match reality.
> 
> Inspired by a patch by Yoshinori Sato for SH.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> The corresponding DTS updates can be found in series "[PATCH 0/2]
> ARM/arm64: dts: renesas: Improve TMU interrupt descriptions".
> https://lore.kernel.org/r/cover.1705325654.git.geert+renesas@glider.be
> 
>  .../devicetree/bindings/timer/renesas,tmu.yaml | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/timer/renesas,tmu.yaml b/Documentation/devicetree/bindings/timer/renesas,tmu.yaml
> index a67e427a9e7e22aa..9a823224c144f7d4 100644
> --- a/Documentation/devicetree/bindings/timer/renesas,tmu.yaml
> +++ b/Documentation/devicetree/bindings/timer/renesas,tmu.yaml
> @@ -46,7 +46,19 @@ properties:
>  
>    interrupts:
>      minItems: 2
> -    maxItems: 3
> +    items:
> +      - description: Underflow interrupt 0
> +      - description: Underflow interrupt 1
> +      - description: Underflow interrupt 2
> +      - description: Input capture interrupt 2

Seeing "input capture interrupt 2" makes me wonder, are there two (or
more!) other input capture interrupts that are still out there,
undocumented, and looking for a home?

Cheers,
Conor.

> +
> +  interrupt-names:
> +    minItems: 2
> +    items:
> +      - const: tuni0
> +      - const: tuni1
> +      - const: tuni2
> +      - const: ticpi2
>  
>    clocks:
>      maxItems: 1
> @@ -100,7 +112,9 @@ examples:
>              reg = <0xffd80000 0x30>;
>              interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>,
>                           <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>,
> -                         <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
> +                         <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-names = "tuni0", "tuni1", "tuni2", "ticpi2";
>              clocks = <&mstp0_clks R8A7779_CLK_TMU0>;
>              clock-names = "fck";
>              power-domains = <&sysc R8A7779_PD_ALWAYS_ON>;
> -- 
> 2.34.1
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: timer: renesas,tmu: Document input capture interrupt

[Geert Uytterhoeven]

Hi Conor,

On Mon, Jan 15, 2024 at 5:13 PM Conor Dooley <conor@kernel.org> wrote:
> On Mon, Jan 15, 2024 at 02:45:39PM +0100, Geert Uytterhoeven wrote:
> > Some Timer Unit (TMU) instances with 3 channels support a fourth
> > interrupt: an input capture interrupt for the third channel.
> >
> > While at it, document the meaning of the four interrupts, and add
> > "interrupt-names" for clarity.
> >
> > Update the example to match reality.
> >
> > Inspired by a patch by Yoshinori Sato for SH.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

> > --- a/Documentation/devicetree/bindings/timer/renesas,tmu.yaml
> > +++ b/Documentation/devicetree/bindings/timer/renesas,tmu.yaml
> > @@ -46,7 +46,19 @@ properties:
> >
> >    interrupts:
> >      minItems: 2
> > -    maxItems: 3
> > +    items:
> > +      - description: Underflow interrupt 0
> > +      - description: Underflow interrupt 1
> > +      - description: Underflow interrupt 2
> > +      - description: Input capture interrupt 2
>
> Seeing "input capture interrupt 2" makes me wonder, are there two (or
> more!) other input capture interrupts that are still out there,
> undocumented, and looking for a home?

SoCs can have multiple TMU instances.
Each TMU instance has 2 or 3 timer channels.
Each timer channel has an underflow interrupt.
Only the third channel may have an optional input capture interrupt
(which is not supported yet by the Linux driver).
Hence each instance can have 2, 3, or 4 interrupts.

See "RZ/G Series, 2nd Generation User's Manual: Hardware"[1],
Section 69 ("Timer Unit (TMU)":
  - Figure 69.2: Block Diagram of TMU,
  - Section 69: Interrupt

Note that the documentation uses a monotonic increasing numbering
of the channels, across all instances.

[1] https://www.renesas.com/us/en/products/microcontrollers-microprocessors/rz-mpus/rzg2h-ultra-high-performance-microprocessors-quad-core-arm-cortex-a57-and-quad-core-arm-cortex-a53-cpus-3d

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] dt-bindings: timer: renesas,tmu: Document input capture interrupt

[Laurent Pinchart]

On Mon, Jan 15, 2024 at 05:48:18PM +0100, Geert Uytterhoeven wrote:
> Hi Conor,
> 
> On Mon, Jan 15, 2024 at 5:13 PM Conor Dooley <conor@kernel.org> wrote:
> > On Mon, Jan 15, 2024 at 02:45:39PM +0100, Geert Uytterhoeven wrote:
> > > Some Timer Unit (TMU) instances with 3 channels support a fourth
> > > interrupt: an input capture interrupt for the third channel.
> > >
> > > While at it, document the meaning of the four interrupts, and add
> > > "interrupt-names" for clarity.
> > >
> > > Update the example to match reality.
> > >
> > > Inspired by a patch by Yoshinori Sato for SH.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > > --- a/Documentation/devicetree/bindings/timer/renesas,tmu.yaml
> > > +++ b/Documentation/devicetree/bindings/timer/renesas,tmu.yaml
> > > @@ -46,7 +46,19 @@ properties:
> > >
> > >    interrupts:
> > >      minItems: 2
> > > -    maxItems: 3
> > > +    items:
> > > +      - description: Underflow interrupt 0
> > > +      - description: Underflow interrupt 1
> > > +      - description: Underflow interrupt 2
> > > +      - description: Input capture interrupt 2
> >
> > Seeing "input capture interrupt 2" makes me wonder, are there two (or
> > more!) other input capture interrupts that are still out there,
> > undocumented, and looking for a home?

Maybe writing this as

      - description: Underflow interrupt, channel 0
      - description: Underflow interrupt, channel 1
      - description: Underflow interrupt, channel 2
      - description: Input capture interrupt, channel 2

would make it clearer ?

I'm also wondering if we really need to add interrupt-names. Drivers
can't depend on the names due to backward compatibility, what benefit
does it bring to add them to the bindings ?

> SoCs can have multiple TMU instances.
> Each TMU instance has 2 or 3 timer channels.
> Each timer channel has an underflow interrupt.
> Only the third channel may have an optional input capture interrupt
> (which is not supported yet by the Linux driver).
> Hence each instance can have 2, 3, or 4 interrupts.
> 
> See "RZ/G Series, 2nd Generation User's Manual: Hardware"[1],
> Section 69 ("Timer Unit (TMU)":
>   - Figure 69.2: Block Diagram of TMU,
>   - Section 69: Interrupt
> 
> Note that the documentation uses a monotonic increasing numbering
> of the channels, across all instances.
> 
> [1] https://www.renesas.com/us/en/products/microcontrollers-microprocessors/rz-mpus/rzg2h-ultra-high-performance-microprocessors-quad-core-arm-cortex-a57-and-quad-core-arm-cortex-a53-cpus-3d

-- 
Regards,

Laurent Pinchart

Re: [PATCH] dt-bindings: timer: renesas,tmu: Document input capture interrupt

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3113 bytes --]

On Mon, Jan 15, 2024 at 07:08:07PM +0200, Laurent Pinchart wrote:
> On Mon, Jan 15, 2024 at 05:48:18PM +0100, Geert Uytterhoeven wrote:
> > Hi Conor,
> > 
> > On Mon, Jan 15, 2024 at 5:13 PM Conor Dooley <conor@kernel.org> wrote:
> > > On Mon, Jan 15, 2024 at 02:45:39PM +0100, Geert Uytterhoeven wrote:
> > > > Some Timer Unit (TMU) instances with 3 channels support a fourth
> > > > interrupt: an input capture interrupt for the third channel.
> > > >
> > > > While at it, document the meaning of the four interrupts, and add
> > > > "interrupt-names" for clarity.
> > > >
> > > > Update the example to match reality.
> > > >
> > > > Inspired by a patch by Yoshinori Sato for SH.
> > > >
> > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > 
> > > > --- a/Documentation/devicetree/bindings/timer/renesas,tmu.yaml
> > > > +++ b/Documentation/devicetree/bindings/timer/renesas,tmu.yaml
> > > > @@ -46,7 +46,19 @@ properties:
> > > >
> > > >    interrupts:
> > > >      minItems: 2
> > > > -    maxItems: 3
> > > > +    items:
> > > > +      - description: Underflow interrupt 0
> > > > +      - description: Underflow interrupt 1
> > > > +      - description: Underflow interrupt 2
> > > > +      - description: Input capture interrupt 2
> > >
> > > Seeing "input capture interrupt 2" makes me wonder, are there two (or
> > > more!) other input capture interrupts that are still out there,
> > > undocumented, and looking for a home?
> 
> Maybe writing this as
> 
>       - description: Underflow interrupt, channel 0
>       - description: Underflow interrupt, channel 1
>       - description: Underflow interrupt, channel 2
>       - description: Input capture interrupt, channel 2

I, for one, prefer this wording.

> would make it clearer ?
> 
> I'm also wondering if we really need to add interrupt-names. Drivers
> can't depend on the names due to backward compatibility, what benefit
> does it bring to add them to the bindings ?

Adding a -names property and not making it required has always seemed
like a waste of time to me. Granted, making it required post-factum has
other problems, so I am inclined to agree that it adds nothing.

> 
> > SoCs can have multiple TMU instances.
> > Each TMU instance has 2 or 3 timer channels.
> > Each timer channel has an underflow interrupt.
> > Only the third channel may have an optional input capture interrupt
> > (which is not supported yet by the Linux driver).
> > Hence each instance can have 2, 3, or 4 interrupts.
> > 
> > See "RZ/G Series, 2nd Generation User's Manual: Hardware"[1],
> > Section 69 ("Timer Unit (TMU)":
> >   - Figure 69.2: Block Diagram of TMU,
> >   - Section 69: Interrupt
> > 
> > Note that the documentation uses a monotonic increasing numbering
> > of the channels, across all instances.
> > 
> > [1] https://www.renesas.com/us/en/products/microcontrollers-microprocessors/rz-mpus/rzg2h-ultra-high-performance-microprocessors-quad-core-arm-cortex-a57-and-quad-core-arm-cortex-a53-cpus-3d
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: timer: renesas,tmu: Document input capture interrupt

[Geert Uytterhoeven]

Hi Conor, Laurent,

On Tue, Jan 16, 2024 at 6:34 PM Conor Dooley <conor@kernel.org> wrote:
> On Mon, Jan 15, 2024 at 07:08:07PM +0200, Laurent Pinchart wrote:
> > On Mon, Jan 15, 2024 at 05:48:18PM +0100, Geert Uytterhoeven wrote:
> > > On Mon, Jan 15, 2024 at 5:13 PM Conor Dooley <conor@kernel.org> wrote:
> > > > On Mon, Jan 15, 2024 at 02:45:39PM +0100, Geert Uytterhoeven wrote:
> > > > > Some Timer Unit (TMU) instances with 3 channels support a fourth
> > > > > interrupt: an input capture interrupt for the third channel.
> > > > >
> > > > > While at it, document the meaning of the four interrupts, and add
> > > > > "interrupt-names" for clarity.
> > > > >
> > > > > Update the example to match reality.
> > > > >
> > > > > Inspired by a patch by Yoshinori Sato for SH.
> > > > >
> > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > >
> > > > > --- a/Documentation/devicetree/bindings/timer/renesas,tmu.yaml
> > > > > +++ b/Documentation/devicetree/bindings/timer/renesas,tmu.yaml
> > > > > @@ -46,7 +46,19 @@ properties:
> > > > >
> > > > >    interrupts:
> > > > >      minItems: 2
> > > > > -    maxItems: 3
> > > > > +    items:
> > > > > +      - description: Underflow interrupt 0
> > > > > +      - description: Underflow interrupt 1
> > > > > +      - description: Underflow interrupt 2
> > > > > +      - description: Input capture interrupt 2
> > > >
> > > > Seeing "input capture interrupt 2" makes me wonder, are there two (or
> > > > more!) other input capture interrupts that are still out there,
> > > > undocumented, and looking for a home?
> >
> > Maybe writing this as
> >
> >       - description: Underflow interrupt, channel 0
> >       - description: Underflow interrupt, channel 1
> >       - description: Underflow interrupt, channel 2
> >       - description: Input capture interrupt, channel 2
>
> I, for one, prefer this wording.

Fine for me.

> > I'm also wondering if we really need to add interrupt-names. Drivers
> > can't depend on the names due to backward compatibility, what benefit
> > does it bring to add them to the bindings ?

I like adding names if there are multiple items, especially if not all
of them have the same type.  Before, we just had a single interrupt
per channel.

I am also wary of another weird variant showing up eventually, with
e.g. 4 channels, or multiple input capture interrupts.

> Adding a -names property and not making it required has always seemed
> like a waste of time to me. Granted, making it required post-factum has
> other problems, so I am inclined to agree that it adds nothing.

I should have mentioned that I do have another local patch, to be
submitted after all DTS files have been updated ;-)

    [WIP] dt-bindings: timer: renesas,tmu: Make interrupt-names required

    Now all in-tree users have been updated, make interrupt-names required.

> > > SoCs can have multiple TMU instances.
> > > Each TMU instance has 2 or 3 timer channels.
> > > Each timer channel has an underflow interrupt.
> > > Only the third channel may have an optional input capture interrupt
> > > (which is not supported yet by the Linux driver).
> > > Hence each instance can have 2, 3, or 4 interrupts.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] dt-bindings: timer: renesas,tmu: Document input capture interrupt

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 321 bytes --]


> > Maybe writing this as
> > 
> >       - description: Underflow interrupt, channel 0
> >       - description: Underflow interrupt, channel 1
> >       - description: Underflow interrupt, channel 2
> >       - description: Input capture interrupt, channel 2
> 
> I, for one, prefer this wording.

I agree.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]