| Message ID | fb1e38c93e62221f94304edd980a2fb79c1f2995.1705325608.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | dt-bindings: timer: renesas,tmu: Document input capture interrupt | expand |
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 > >
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
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
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 >
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
> > 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.
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>;
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(-)