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

Hi Conor, Laurent,

On Tue, Jan 16, 2024 at 6:34 PM Conor Dooley <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > >
> > > > > --- 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 -- [email protected]

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