diff mbox series

[v3,2/3] dt-bindings: interrupt-controller: renesas,rzg2l-irqc: Update description for '#interrupt-cells' property

Message ID 20220722151155.21100-3-prabhakar.mahadev-lad.rj@bp.renesas.com
State Accepted
Commit cfa1f9db6d6088118ef311c0927c66072665b47e
Headers show
Series Add PHY interrupt support for ETH{0,1} on RZ/G2L and RZ/V2L SMARC EVK | expand

Commit Message

Prabhakar Mahadev Lad July 22, 2022, 3:11 p.m. UTC
Update description for '#interrupt-cells' property to utilize the
RZG2L_{NMI,IRQX} for the first cell defined in the
include/dt-bindings/interrupt-controller/irqc-rzg2l.h file.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v3:
* New patch
---
 .../bindings/interrupt-controller/renesas,rzg2l-irqc.yaml    | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Marc Zyngier July 24, 2022, 11:01 a.m. UTC | #1
On Fri, 22 Jul 2022 16:11:54 +0100,
Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> 
> Update description for '#interrupt-cells' property to utilize the
> RZG2L_{NMI,IRQX} for the first cell defined in the
> include/dt-bindings/interrupt-controller/irqc-rzg2l.h file.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v3:
> * New patch
> ---
>  .../bindings/interrupt-controller/renesas,rzg2l-irqc.yaml    | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> index 33b90e975e33..ea7db3618b23 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> @@ -31,8 +31,9 @@ properties:
>        - const: renesas,rzg2l-irqc
>  
>    '#interrupt-cells':
> -    description: The first cell should contain external interrupt number (IRQ0-7) and the
> -                 second cell is used to specify the flag.
> +    description: The first cell should contain a macro RZG2L_{NMI,IRQX} included in the
> +                 include/dt-bindings/interrupt-controller/irqc-rzg2l.h and the second
> +                 cell is used to specify the flag.

I think a binding should be self describing, and not rely on an opaque
macro. Mentioning that there is a macro that encodes it is fine, but
the values are what matter, specially when considering that other OSs
could (and should be able to) write their own DTs from scratch without
depending on something that is very much Linux-specific.

Thanks,

	M.
Geert Uytterhoeven Aug. 11, 2022, 2:50 p.m. UTC | #2
Hi Marc,

On Sun, Jul 24, 2022 at 1:01 PM Marc Zyngier <maz@kernel.org> wrote:
> On Fri, 22 Jul 2022 16:11:54 +0100,
> Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> >
> > Update description for '#interrupt-cells' property to utilize the
> > RZG2L_{NMI,IRQX} for the first cell defined in the
> > include/dt-bindings/interrupt-controller/irqc-rzg2l.h file.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v3:
> > * New patch

> > --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > @@ -31,8 +31,9 @@ properties:
> >        - const: renesas,rzg2l-irqc
> >
> >    '#interrupt-cells':
> > -    description: The first cell should contain external interrupt number (IRQ0-7) and the
> > -                 second cell is used to specify the flag.
> > +    description: The first cell should contain a macro RZG2L_{NMI,IRQX} included in the
> > +                 include/dt-bindings/interrupt-controller/irqc-rzg2l.h and the second
> > +                 cell is used to specify the flag.
>
> I think a binding should be self describing, and not rely on an opaque
> macro. Mentioning that there is a macro that encodes it is fine, but
> the values are what matter, specially when considering that other OSs
> could (and should be able to) write their own DTs from scratch without
> depending on something that is very much Linux-specific.

The macros are not Linux-specific, and are part of the bindings.
But the only hard dependency on <dt-bindings/interrupt-controller/irqc-rzg2l.h>
is the DT source file describing the board.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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
Geert Uytterhoeven Sept. 8, 2023, 9:31 a.m. UTC | #3
Hi Marc,

On Thu, Aug 11, 2022 at 4:50 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Sun, Jul 24, 2022 at 1:01 PM Marc Zyngier <maz@kernel.org> wrote:
> > On Fri, 22 Jul 2022 16:11:54 +0100,
> > Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > >
> > > Update description for '#interrupt-cells' property to utilize the
> > > RZG2L_{NMI,IRQX} for the first cell defined in the
> > > include/dt-bindings/interrupt-controller/irqc-rzg2l.h file.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > > v3:
> > > * New patch
>
> > > --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > > @@ -31,8 +31,9 @@ properties:
> > >        - const: renesas,rzg2l-irqc
> > >
> > >    '#interrupt-cells':
> > > -    description: The first cell should contain external interrupt number (IRQ0-7) and the
> > > -                 second cell is used to specify the flag.
> > > +    description: The first cell should contain a macro RZG2L_{NMI,IRQX} included in the
> > > +                 include/dt-bindings/interrupt-controller/irqc-rzg2l.h and the second
> > > +                 cell is used to specify the flag.
> >
> > I think a binding should be self describing, and not rely on an opaque
> > macro. Mentioning that there is a macro that encodes it is fine, but
> > the values are what matter, specially when considering that other OSs
> > could (and should be able to) write their own DTs from scratch without
> > depending on something that is very much Linux-specific.
>
> The macros are not Linux-specific, and are part of the bindings.
> But the only hard dependency on <dt-bindings/interrupt-controller/irqc-rzg2l.h>
> is the DT source file describing the board.
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Looks like this fell through the cracks?
The two other patches from this series were applied in v6.1.

Note that the current DT bindings are incorrect, as they do not take into
account that the value of zero is used to represent the NMI.

Fixes: 96fed779d3d4cb3c ("dt-bindings: interrupt-controller: Add
Renesas RZ/G2L Interrupt Controller")

Should we resend instead?
Thanks!

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
index 33b90e975e33..ea7db3618b23 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
@@ -31,8 +31,9 @@  properties:
       - const: renesas,rzg2l-irqc
 
   '#interrupt-cells':
-    description: The first cell should contain external interrupt number (IRQ0-7) and the
-                 second cell is used to specify the flag.
+    description: The first cell should contain a macro RZG2L_{NMI,IRQX} included in the
+                 include/dt-bindings/interrupt-controller/irqc-rzg2l.h and the second
+                 cell is used to specify the flag.
     const: 2
 
   '#address-cells':